* [PATCH V3] nvme: set non-mdts limits in nvme_scan_work
@ 2022-05-18 15:51 Chaitanya Kulkarni
2022-05-19 19:28 ` Keith Busch
0 siblings, 1 reply; 3+ messages in thread
From: Chaitanya Kulkarni @ 2022-05-18 15:51 UTC (permalink / raw)
To: hch, kbusch, sagi, james.smart; +Cc: linux-nvme, Chaitanya Kulkarni
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"
Move the call of nvme_init_non_mdts_limits() to nvme_scan_work() after
we verify that I/O queues are created since that is a converging point
for each transport where these limits are actually used.
1. FC :
nvme_fc_create_association()
...
nvme_fc_create_io_queues(ctrl);
...
nvme_start_ctrl()
nvme_scan_queue()
nvme_scan_work()
2. PCIe:-
nvme_reset_work()
...
nvme_setup_io_queues()
nvme_create_io_queues()
nvme_alloc_queue()
...
nvme_start_ctrl()
nvme_scan_queue()
nvme_scan_work()
3. RDMA :-
nvme_rdma_setup_ctrl
...
nvme_rdma_configure_io_queues
...
nvme_start_ctrl()
nvme_scan_queue()
nvme_scan_work()
4. TCP :-
nvme_tcp_setup_ctrl
...
nvme_tcp_configure_io_queues
...
nvme_start_ctrl()
nvme_scan_queue()
nvme_scan_work()
* nvme_scan_work()
...
nvme_validate_or_alloc_ns()
nvme_alloc_ns()
nvme_update_ns_info()
nvme_update_disk_info()
nvme_config_discard() <---
blk_queue_max_write_zeroes_sectors() <---
Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
---
* changes from V2:-
1. Add a comment why we need to re-read the non-mdts limits at the time
of every ns allocation.
---
drivers/nvme/host/core.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 42f9772abc4d..140f589305d1 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3082,10 +3082,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;
@@ -4239,11 +4235,26 @@ static void nvme_scan_work(struct work_struct *work)
{
struct nvme_ctrl *ctrl =
container_of(work, struct nvme_ctrl, scan_work);
+ int ret;
/* No tagset on a live ctrl means IO queues could not created */
if (ctrl->state != NVME_CTRL_LIVE || !ctrl->tagset)
return;
+ /**
+ * Identify controller limits can change at controller reset due to
+ * new firmware download, even though it is not common we cannot ignore
+ * such scenario. Controller's non-mdts limits are reported in the unit
+ * of logical blocks that is dependent on the format of attached
+ * namespace. Hence re-read the limits at the time of ns allocation.
+ */
+ ret = nvme_init_non_mdts_limits(ctrl);
+ if (ret < 0) {
+ dev_warn(ctrl->device,
+ "reading non-mdts-limits failed: %d\n", ret);
+ return;
+ }
+
if (test_and_clear_bit(NVME_AER_NOTICE_NS_CHANGED, &ctrl->events)) {
dev_info(ctrl->device, "rescanning namespaces.\n");
nvme_clear_changed_ns_log(ctrl);
--
2.29.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH V3] nvme: set non-mdts limits in nvme_scan_work
2022-05-18 15:51 [PATCH V3] nvme: set non-mdts limits in nvme_scan_work Chaitanya Kulkarni
@ 2022-05-19 19:28 ` Keith Busch
2022-05-19 19:44 ` Chaitanya Kulkarni
0 siblings, 1 reply; 3+ messages in thread
From: Keith Busch @ 2022-05-19 19:28 UTC (permalink / raw)
To: Chaitanya Kulkarni; +Cc: hch, sagi, james.smart, linux-nvme
On Wed, May 18, 2022 at 08:51:38AM -0700, Chaitanya Kulkarni wrote:
> + /**
> + * Identify controller limits can change at controller reset due to
> + * new firmware download, even though it is not common we cannot ignore
> + * such scenario. Controller's non-mdts limits are reported in the unit
> + * of logical blocks that is dependent on the format of attached
> + * namespace. Hence re-read the limits at the time of ns allocation.
> + */
I think we usually see the "/**" comment style for kernel doc.
But otherwise, patch looks good.
Reviewed-by: Keith Busch <kbusch@kernel.org>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH V3] nvme: set non-mdts limits in nvme_scan_work
2022-05-19 19:28 ` Keith Busch
@ 2022-05-19 19:44 ` Chaitanya Kulkarni
0 siblings, 0 replies; 3+ messages in thread
From: Chaitanya Kulkarni @ 2022-05-19 19:44 UTC (permalink / raw)
To: Keith Busch, Chaitanya Kulkarni; +Cc: hch, sagi, james.smart, linux-nvme
On 5/19/22 12:28, Keith Busch wrote:
> On Wed, May 18, 2022 at 08:51:38AM -0700, Chaitanya Kulkarni wrote:
>> + /**
>> + * Identify controller limits can change at controller reset due to
>> + * new firmware download, even though it is not common we cannot ignore
>> + * such scenario. Controller's non-mdts limits are reported in the unit
>> + * of logical blocks that is dependent on the format of attached
>> + * namespace. Hence re-read the limits at the time of ns allocation.
>> + */
>
> I think we usually see the "/**" comment style for kernel doc.
>
> But otherwise, patch looks good.
>
> Reviewed-by: Keith Busch <kbusch@kernel.org>
Yes, I wanted it to be documented in kernel doc.
-ck
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-05-19 19:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-18 15:51 [PATCH V3] nvme: set non-mdts limits in nvme_scan_work Chaitanya Kulkarni
2022-05-19 19:28 ` Keith Busch
2022-05-19 19:44 ` Chaitanya Kulkarni
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.