[PATCH] nvmet: support reservation feature
Chaitanya Kulkarni
chaitanyak at nvidia.com
Wed Jan 10 00:31:32 PST 2024
On 1/9/24 21:58, Guixin Liu wrote:
>
> 在 2024/1/10 12:34, Chaitanya Kulkarni 写道:
>> On 1/9/24 04:10, Guixin Liu wrote:
>>> This patch implements the reservation feature, includes:
>>> 1. reservation register(register, unregister and replace).
>>> 2. reservation acquire(acquire, preempt, preempt and abort).
>>> 3. reservation release(release and clear).
>>> 4. reservation report.
>>>
>>> And also make reservation configurable, one can set ns to support
>>> reservation before enable ns. The default of resv_enable is false.
>>>
>>> Signed-off-by: Guixin Liu <kanie at linux.alibaba.com>
>>> ---
>>> Hi guys,
>>> I've implemented the NVMe reservation feature. Please review
>>> it, all
>>> comments are welcome.
>>> In addtion, I didn't implement event reporting because I
>>> didn't see
>>> any handling of these events on the host side. If these events are
>>> mandatory
>>> to report, please let me know so that I can implement them.
>>>
>>> drivers/nvme/target/Makefile | 2 +-
>>> drivers/nvme/target/admin-cmd.c | 14 +-
>>> drivers/nvme/target/configfs.c | 27 ++
>>> drivers/nvme/target/core.c | 37 +-
>>> drivers/nvme/target/nvmet.h | 26 ++
>>> drivers/nvme/target/pr.c | 806
>>> ++++++++++++++++++++++++++++++++
>>> include/linux/nvme.h | 30 ++
>>> 7 files changed, 939 insertions(+), 3 deletions(-)
>>> create mode 100644 drivers/nvme/target/pr.c
>>>
>>> diff --git a/drivers/nvme/target/Makefile
>>> b/drivers/nvme/target/Makefile
>>> index c66820102493..f9bfc904a5b3 100644
>>> --- a/drivers/nvme/target/Makefile
>>> +++ b/drivers/nvme/target/Makefile
>>> @@ -10,7 +10,7 @@ obj-$(CONFIG_NVME_TARGET_FCLOOP) += nvme-fcloop.o
>>> obj-$(CONFIG_NVME_TARGET_TCP) += nvmet-tcp.o
>>> nvmet-y += core.o configfs.o admin-cmd.o fabrics-cmd.o \
>>> - discovery.o io-cmd-file.o io-cmd-bdev.o
>>> + discovery.o io-cmd-file.o io-cmd-bdev.o pr.o
>>> nvmet-$(CONFIG_NVME_TARGET_PASSTHRU) += passthru.o
>>> nvmet-$(CONFIG_BLK_DEV_ZONED) += zns.o
>>> nvmet-$(CONFIG_NVME_TARGET_AUTH) += fabrics-cmd-auth.o auth.o
>>> diff --git a/drivers/nvme/target/admin-cmd.c
>>> b/drivers/nvme/target/admin-cmd.c
>>> index 39cb570f833d..7da6f3085a4c 100644
>>> --- a/drivers/nvme/target/admin-cmd.c
>>> +++ b/drivers/nvme/target/admin-cmd.c
>>> @@ -550,7 +550,13 @@ static void nvmet_execute_identify_ns(struct
>>> nvmet_req *req)
>>> */
>>> id->nmic = NVME_NS_NMIC_SHARED;
>>> id->anagrpid = cpu_to_le32(req->ns->anagrpid);
>>> -
>>> + id->rescap = NVME_PR_SUPPORT_WRITE_EXCLUSIVE |
>>> + NVME_PR_SUPPORT_EXCLUSIVE_ACCESS |
>>> + NVME_PR_SUPPORT_WRITE_EXCLUSIVE_REG_ONLY |
>>> + NVME_PR_SUPPORT_EXCLUSIVE_ACCESS_REG_ONLY |
>>> + NVME_PR_SUPPORT_WRITE_EXCLUSIVE_ALL_REGS |
>>> + NVME_PR_SUPPORT_EXCLUSIVE_ACCESS_ALL_REGS |
>>> + NVME_PR_SUPPORT_IEKEY_DEF_LATER_VER_1_3;
>>> memcpy(&id->nguid, &req->ns->nguid, sizeof(id->nguid));
>>> id->lbaf[0].ds = req->ns->blksize_shift;
>>> @@ -1017,6 +1023,12 @@ u16 nvmet_parse_admin_cmd(struct nvmet_req *req)
>>> if (nvmet_is_passthru_req(req))
>>> return nvmet_parse_passthru_admin_cmd(req);
>>> + ret = nvmet_pr_check_cmd_access(req);
>>> + if (unlikely(ret)) {
>>> + req->error_loc = offsetof(struct nvme_common_command, opcode);
>>> + return ret;
>>> + }
>>> +
>>> switch (cmd->common.opcode) {
>>> case nvme_admin_get_log_page:
>>> req->execute = nvmet_execute_get_log_page;
>>> diff --git a/drivers/nvme/target/configfs.c
>>> b/drivers/nvme/target/configfs.c
>>> index d937fe05129e..1ac4802ec818 100644
>>> --- a/drivers/nvme/target/configfs.c
>>> +++ b/drivers/nvme/target/configfs.c
>>> @@ -714,6 +714,32 @@ static ssize_t
>>> nvmet_ns_revalidate_size_store(struct config_item *item,
>>> CONFIGFS_ATTR_WO(nvmet_ns_, revalidate_size);
>>> +static ssize_t nvmet_ns_resv_enable_show(struct config_item
>>> *item, char *page)
>>> +{
>>> + return sprintf(page, "%d\n", to_nvmet_ns(item)->pr.enable);
>>> +}
>>> +
>>> +static ssize_t nvmet_ns_resv_enable_store(struct config_item *item,
>>> + const char *page, size_t count)
>>> +{
>>> + struct nvmet_ns *ns = to_nvmet_ns(item);
>>> + bool val;
>>> +
>>> + if (kstrtobool(page, &val))
>>> + return -EINVAL;
>>> +
>>> + mutex_lock(&ns->subsys->lock);
>>> + if (ns->enabled) {
>>> + pr_err("the ns:%d is already enabled.\n", ns->nsid);
>>> + mutex_unlock(&ns->subsys->lock);
>>> + return -EINVAL;
>>> + }
>>> + ns->pr.enable = val;
>>> + mutex_unlock(&ns->subsys->lock);
>>> + return count;
>>> +}
>>> +CONFIGFS_ATTR(nvmet_ns_, resv_enable);
>>> +
>>> static struct configfs_attribute *nvmet_ns_attrs[] = {
>>> &nvmet_ns_attr_device_path,
>>> &nvmet_ns_attr_device_nguid,
>>> @@ -722,6 +748,7 @@ static struct configfs_attribute
>>> *nvmet_ns_attrs[] = {
>>> &nvmet_ns_attr_enable,
>>> &nvmet_ns_attr_buffered_io,
>>> &nvmet_ns_attr_revalidate_size,
>>> + &nvmet_ns_attr_resv_enable,
>>> #ifdef CONFIG_PCI_P2PDMA
>>> &nvmet_ns_attr_p2pmem,
>>> #endif
>>> diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
>>> index 3935165048e7..8eab81804b14 100644
>>> --- a/drivers/nvme/target/core.c
>>> +++ b/drivers/nvme/target/core.c
>>> @@ -598,6 +598,7 @@ int nvmet_ns_enable(struct nvmet_ns *ns)
>>> subsys->nr_namespaces++;
>>> nvmet_ns_changed(subsys, ns->nsid);
>>> + nvmet_pr_init_ns(ns);
>>> ns->enabled = true;
>>> ret = 0;
>>> out_unlock:
>>> @@ -651,6 +652,7 @@ void nvmet_ns_disable(struct nvmet_ns *ns)
>>> subsys->nr_namespaces--;
>>> nvmet_ns_changed(subsys, ns->nsid);
>>> + nvmet_pr_clean_all_registrants(&ns->pr);
>>> nvmet_ns_dev_disable(ns);
>>> out_unlock:
>>> mutex_unlock(&subsys->lock);
>>> @@ -904,6 +906,16 @@ static u16 nvmet_parse_io_cmd(struct nvmet_req
>>> *req)
>>> return ret;
>>> }
>>> + ret = nvmet_pr_check_cmd_access(req);
>>> + if (unlikely(ret)) {
>>> + req->error_loc = offsetof(struct nvme_common_command, opcode);
>>> + return ret;
>>> + }
>>> +
>>> + ret = nvmet_parse_pr_cmd(req);
>>> + if (!ret)
>>> + return ret;
>>> +
>> Can we make this feature configurable via Kconfig? If someone doesn't
>> want to
>> use PR, they will have to bear the cost of these checks in the fast
>> path.
>
> Yeah, I have added a resv_enable in configfs, the default is false,
> one can
>
> make reservation enable before enable namespace.
Why can't we make it KConfig option ? Is there any particular reason for
not doing that ? That will also allow user to avoid kernel compilation
of code if they want to turn it off.
-ck
More information about the Linux-nvme
mailing list