Re: [PATCH] nvme: set non-mdts limits at I/O queue creation

[Chaitanya Kulkarni <chaitanyak@nvidia.com>]

On 5/17/22 02:56, Sagi Grimberg wrote:
> 
> 
> On 5/17/22 08:53, Chaitanya Kulkarni wrote:
>> In current implementation we set the non-mdts limits by calling
>> nvme_init_non_mdts_limits() from nvme_init_ctrl_finish().
>> This also tries to set the limits for the discovery controller which
>> has no I/O queues resulting in the warning message reported by the
>> nvme_log_error() when running blktest nvme/002: -
>>
>> [ 2005.155946] run blktests nvme/002 at 2022-04-09 16:57:47
>> [ 2005.192223] loop: module loaded
>> [ 2005.196429] nvmet: adding nsid 1 to subsystem blktests-subsystem-0
>> [ 2005.200334] nvmet: adding nsid 1 to subsystem blktests-subsystem-1
>>
>> <------------------------------SNIP---------------------------------->
>>
>> [ 2008.958108] nvmet: adding nsid 1 to subsystem blktests-subsystem-997
>> [ 2008.962082] nvmet: adding nsid 1 to subsystem blktests-subsystem-998
>> [ 2008.966102] nvmet: adding nsid 1 to subsystem blktests-subsystem-999
>> [ 2008.973132] nvmet: creating discovery controller 1 for subsystem 
>> nqn.2014-08.org.nvmexpress.discovery for NQN testhostnqn.
>> *[ 2008.973196] nvme1: Identify(0x6), Invalid Field in Command (sct 
>> 0x0 / sc 0x2) MORE DNR*
>> [ 2008.974595] nvme nvme1: new ctrl: 
>> "nqn.2014-08.org.nvmexpress.discovery"
>> [ 2009.103248] nvme nvme1: Removing ctrl: NQN 
>> "nqn.2014-08.org.nvmexpress.discovery"
>>
>> Export the nvme_init_non_mdts_limits() and move the same call from
>> nvme_ctrl_finish() to each transport right before each transport calls
>> the nvme_scan_work() from nvme_start_ctrl() path, then proceeds to the
>> ns allocation where these limits max_discard_{segments|sectors} and
>> max_write_zeroes_sectors are actually used in folllwing path :-
> 
> Why not just check the ctrl->queue_count and call it accordingly?

nvme_init_ctrl_finish() is called before the I/O queues are allocated
so queue_count value will be always equal to 1 when checked in the
nvme_init_ctrl_finish() before calling the
nvme_init_non_mdts_limits() for PCIe :-

nvme_reset_work()
...
  nvme_init_ctrl_finish()
   nvme_init_non_mdts_limits() <--- ctrl->queue_count = 1
  nvme_dev_add()
  nvme_setup_io_queues()
   nvme_create_io_queues()
    nvme_alloc_queue()
     dev->ctrl.queue_count++ <---
...

Also, this patch groups reading ctrl non-mdts limits right before I/O
queue allocation for each transport, since these limits are only
used for I/O queue allocation.

-ck