[PATCH 2/3] nvme-core: don't check non-mdts for disc ctrl

Chaitanya Kulkarni chaitanyak at nvidia.com
Mon Apr 11 13:44:59 PDT 2022


On 4/11/22 07:14, Keith Busch wrote:
> On Mon, Apr 11, 2022 at 12:40:22PM +0000, Chaitanya Kulkarni wrote:
>> @@ -3101,9 +3101,12 @@ int nvme_init_ctrl_finish(struct nvme_ctrl *ctrl)
>>    	if (ret)
>>    		return ret;
>>
>> -	ret = nvme_init_non_mdts_limits(ctrl);
>> -	if (ret < 0)
>> -		return ret;
>> +	/* only check non mdts for the I/O controller */
>> +	if (ctrl->tagset) {
>> +		ret = nvme_init_non_mdts_limits(ctrl);
>> +		if (ret < 0)
>> +			return ret;
>> +	}
> 
> The nvme pci driver doesn't initialize the ctrl->tagset until after
> nvme_init_ctrl_finish(), so this won't work.

I think we should move nvme_init_non_mdts_limits() from
nvme_init_ctrl_finish() before ctrl actually allocate the I/O tagset ?

Since tagset allocation happens only once, it is guarded by the
new ctrl check in every transport and before each transport calls the
nvme_scan_work() from nvme_start_ctrl() path then proceed to the ns
allocation where these limits max_discard_{segments|sectors}
and max_write_zeroes_sectors actually used in :-

nvme_scan_work()
...
nvme_validate_or_alloc_ns()
  nvme_alloc_ns()
   nvme_update_ns_info()
    nvme_update_disk_info()

1. FC:-
nvme_fc_create_association()
  nvme_fc_create_io_queues()
   allocate ctrl->tagset. <-- call nvme_init_mdts_limits() before this.
  nvme_start_ctrl()
   nvme_scan_queue()
    nvme_scan_work()
     path to namespace allocation and accessing ctrl non mdts limits.

2. PCIe:-
nvme_reset_work()
  nvme_dev_add()
   allocate ctrl->tagset. <-- call nvme_init_mdts_limits() before this.
  nvme_start_ctrl()
   nvme_scan_queue()
    nvme_scan_work()
     path to namespace allocation and accessing ctrl non mdts limits.

3. RDMA :-
nvme_rdma_setup_ctrl
  nvme_rdma_configure_io_queues
   allocate ctrl->tagset.  <-- call nvme_init_mdts_limits() before this.
  nvme_start_ctrl()
   nvme_scan_queue()
    nvme_scan_work()
     path to namespace allocation and accessing ctrl non mdts limits.

4. TCP :-
nvme_tcp_setup_ctrl
  nvme_tcp_configure_io_queues
   allocate ctrl->tagset.  <-- call nvme_init_mdts_limits() before this.
  nvme_start_ctrl()
   nvme_scan_queue()
    nvme_scan_work()
     path to namespace allocation and accessing ctrl non mdts limits. 


-ck

Based on this something like :-

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index e8d6a1e52083..3af2abd67e60 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2872,7 +2872,7 @@ static inline u32 nvme_mps_to_sectors(struct 
nvme_ctrl *ctrl, u32 units)
         return val;
  }

-static int nvme_init_non_mdts_limits(struct nvme_ctrl *ctrl)
+int nvme_init_non_mdts_limits(struct nvme_ctrl *ctrl)
  {
         struct nvme_command c = { };
         struct nvme_id_ctrl_nvm *id;
@@ -2924,6 +2924,7 @@ static int nvme_init_non_mdts_limits(struct 
nvme_ctrl *ctrl)
         kfree(id);
         return ret;
  }
+EXPORT_SYMBOL_GPL(nvme_init_non_mdts_limits);

  static int nvme_init_identify(struct nvme_ctrl *ctrl)
  {
@@ -3101,10 +3102,6 @@ int nvme_init_ctrl_finish(struct nvme_ctrl *ctrl)
         if (ret)
                 return ret;

-       ret = nvme_init_non_mdts_limits(ctrl);
-       if (ret < 0)
-               return ret;
-
         ret = nvme_configure_apst(ctrl);
         if (ret < 0)
                 return ret;
diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index 080f85f4105f..89a3d1d4f48f 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -2882,6 +2882,10 @@ nvme_fc_create_io_queues(struct nvme_fc_ctrl *ctrl)
         unsigned int nr_io_queues;
         int ret;

+       ret = nvme_init_non_mdts_limits(&ctrl->ctrl);
+       if (ret < 0)
+               return ret;
+
         nr_io_queues = min(min(opts->nr_io_queues, num_online_cpus()),
                                 ctrl->lport->ops->max_hw_queues);
         ret = nvme_set_queue_count(&ctrl->ctrl, &nr_io_queues);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index a1f8403ffd78..1a661226fbb0 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -777,6 +777,7 @@ long nvme_ns_head_chr_ioctl(struct file *file, 
unsigned int cmd,
  long nvme_dev_ioctl(struct file *file, unsigned int cmd,
                 unsigned long arg);
  int nvme_getgeo(struct block_device *bdev, struct hd_geometry *geo);
+int nvme_init_non_mdts_limits(struct nvme_ctrl *ctrl);

  extern const struct attribute_group *nvme_ns_id_attr_groups[];
  extern const struct pr_ops nvme_pr_ops;
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 3554c20e78c3..ce4060c3d62e 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2525,6 +2525,13 @@ static void nvme_dev_add(struct nvme_dev *dev)
         int ret;

         if (!dev->ctrl.tagset) {
+               ret = nvme_init_non_mdts_limits(&dev->ctrl);
+               if (ret < 0) {
+                       dev_warn(dev->ctrl.device,
+                               "reading non mdts limit error: %d\n", ret);
+                       return;
+               }
+
                 dev->tagset.ops = &nvme_mq_ops;
                 dev->tagset.nr_hw_queues = dev->online_queues - 1;
                 dev->tagset.nr_maps = 2; /* default + read */
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index c49b9c3c46f2..f6cf28a3b4c1 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -972,6 +972,10 @@ static int nvme_rdma_configure_io_queues(struct 
nvme_rdma_ctrl *ctrl, bool new)
                 return ret;

         if (new) {
+               ret = nvme_init_non_mdts_limits(&ctrl->ctrl);
+               if (ret < 0)
+                       goto out_free_io_queues;
+
                 ctrl->ctrl.tagset = nvme_rdma_alloc_tagset(&ctrl->ctrl, 
false);
                 if (IS_ERR(ctrl->ctrl.tagset)) {
                         ret = PTR_ERR(ctrl->ctrl.tagset);
diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 10fc45d95b86..9147459580d4 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -1858,6 +1858,10 @@ static int nvme_tcp_configure_io_queues(struct 
nvme_ctrl *ctrl, bool new)
                 return ret;

         if (new) {
+               ret = nvme_init_non_mdts_limits(ctrl);
+               if (ret < 0)
+                       goto out_free_io_queues;
+
                 ctrl->tagset = nvme_tcp_alloc_tagset(ctrl, false);
                 if (IS_ERR(ctrl->tagset)) {
                         ret = PTR_ERR(ctrl->tagset);
diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
index 59024af2da2e..2ac9017e966a 100644
--- a/drivers/nvme/target/loop.c
+++ b/drivers/nvme/target/loop.c
@@ -522,6 +522,10 @@ static int nvme_loop_create_io_queues(struct 
nvme_loop_ctrl *ctrl)
  {
         int ret;

+       ret = nvme_init_non_mdts_limits(&ctrl->ctrl);
+       if (ret < 0)
+               return ret;
+
         ret = nvme_loop_init_io_queues(ctrl);
         if (ret)
                 return ret;


-ck





More information about the Linux-nvme mailing list