From: Christoph Hellwig <hch@lst.de>
To: Qiliang Yuan <odys.yuan@gmail.com>
Cc: Keith Busch <kbusch@kernel.org>, Jens Axboe <axboe@kernel.dk>,
Christoph Hellwig <hch@lst.de>, Sagi Grimberg <sagi@grimberg.me>,
linux-nvme@lists.infradead.org, linux-kernel@vger.kernel.org,
stable@vger.kernel.org, rcu@vger.kernel.org
Subject: Re: [PATCH] nvme: wait for SRCU readers before splicing the namespace list
Date: Tue, 3 Nov 2026 09:14:18 +0100 [thread overview]
Message-ID: <20261103081418.GB4301@lst.de> (raw)
In-Reply-To: <20261103-bug-nvme-remove-ns-srcu-v1-1-e239087d63ac@gmail.com>
On Tue, Nov 03, 2026 at 01:30:35AM -0500, Qiliang Yuan wrote:
> Readers walk ctrl->namespaces under ctrl->srcu, and some of them sleep
> there, e.g. nvme_wait_freeze() waiting for I/O to drain.
>
> nvme_remove_namespaces() moves the list onto its on-stack ns_list with
> list_splice_init_rcu(), which only waits for RCU readers before
> relinking the entries. A reader still on the last namespace then follows
> its next pointer to ns_list instead of ctrl->namespaces and treats the
> stack as a namespace. Deleting a controller while a Format NVM
> passthrough waits for a delayed read on the last namespace gives:
>
> BUG: KASAN: stack-out-of-bounds in nvme_wait_freeze+0x102/0x120
> Read of size 8 at addr ff110001d2f3fc38 by task nvme/4413
> Call Trace:
> nvme_wait_freeze
> nvme_passthru_start
> nvme_submit_user_cmd
> nvme_user_cmd
> nvme_dev_ioctl
> BUG: unable to handle page fault for address: ffffffff8af04850
>
> Open-code the splice and wait for SRCU readers instead. This also takes
> the RCU grace period out of every controller deletion:
This feels like a design issue with list_splice_init_rcu or rather
using list_splice_init_rcu with SRCU. Do we need to encapsulate
and verify this logic in a list_splice_init_srcu so that other
potential users won't get it wrong again?
>
> 64 tcp controllers, 1 namespace each before after
> nvme disconnect-all (ms) 3543 / 3474 3221 / 3146
> nvme_remove_namespaces() avg (ms) 39.6 / 38.9 34.4 / 33.7
>
> Fixes: be647e2c76b2 ("nvme: use srcu for iterating namespace list")
> Cc: stable@vger.kernel.org
> Signed-off-by: Qiliang Yuan <odys.yuan@gmail.com>
> ---
> drivers/nvme/host/core.c | 19 +++++++++++++++++--
> 1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index a7a4d09644c13..8f032b3edde9a 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -4754,10 +4754,25 @@ void nvme_remove_namespaces(struct nvme_ctrl *ctrl)
> /* this is a no-op when called from the controller reset handler */
> nvme_change_ctrl_state(ctrl, NVME_CTRL_DELETING_NOIO);
>
> + /*
> + * Readers walk the list under ctrl->srcu, so hide the namespaces from
> + * new readers and wait for the current ones before moving them over.
> + * list_splice_init_rcu() can't be used as it only waits for RCU
> + * readers.
> + */
> mutex_lock(&ctrl->namespaces_lock);
> - list_splice_init_rcu(&ctrl->namespaces, &ns_list, synchronize_rcu);
> + if (!list_empty(&ctrl->namespaces)) {
> + struct list_head *first = ctrl->namespaces.next;
> + struct list_head *last = ctrl->namespaces.prev;
> +
> + INIT_LIST_HEAD_RCU(&ctrl->namespaces);
> + synchronize_srcu(&ctrl->srcu);
> + ns_list.next = first;
> + ns_list.prev = last;
> + first->prev = &ns_list;
> + last->next = &ns_list;
> + }
> mutex_unlock(&ctrl->namespaces_lock);
> - synchronize_srcu(&ctrl->srcu);
>
> list_for_each_entry_safe(ns, next, &ns_list, list)
> nvme_ns_remove(ns);
>
> ---
> base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
> change-id: 20261102-bug-nvme-remove-ns-srcu-93d8a75a9b1b
>
> Best regards,
> --
> Qiliang Yuan <odys.yuan@gmail.com>
---end quoted text---
next prev parent reply other threads:[~2026-11-03 8:14 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-03 6:30 Qiliang Yuan
2026-11-03 8:14 ` Christoph Hellwig [this message]
2026-11-03 8:57 ` Qiliang Yuan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261103081418.GB4301@lst.de \
--to=hch@lst.de \
--cc=axboe@kernel.dk \
--cc=kbusch@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nvme@lists.infradead.org \
--cc=odys.yuan@gmail.com \
--cc=rcu@vger.kernel.org \
--cc=sagi@grimberg.me \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®