mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
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---

  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®