[PATCH] nvme: swap synchronization ordering in nvme_remove_head()

[PATCH] nvme: swap synchronization ordering in nvme_remove_head()

[John Garry]

From: John Garry <john.g.garry@oracle.com>

sashiko bot reported a potential issue in the requeue handling in [0] -
the code there is same as the NVMe driver.

The issue is that when we schedule the requeue work, if a bio is added to
the requeue list afterwards in nvme_ns_head_submit_bio(), it is missed by
the requeue worker.

This issue can be recreated by hacking a large delay in the bio submission
requeue path:

        } else if (nvme_available_path(head)) {
                dev_warn_ratelimited(dev, "no usable path - requeuing I/O\n");

+               msleep(30000);
                spin_lock_irq(&head->requeue_lock);
                bio_list_add(&head->requeue_list, bio);
                spin_unlock_irq(&head->requeue_lock);


Then if we issue a write after removing all paths, a hang can be seen:

# echo 20 > /sys/devices/virtual/nvme-subsystem/nvme-subsys1/nvme1n1/delayed_removal_secs
#
# ./ini_nvme_teardown.sh
[   25.877224] nvme nvme1: Removing ctrl: NQN "nvme-test-target"
[   25.939569] nvme nvme2: Removing ctrl: NQN "nvme-test-target"
#
# xfs_io -d -C "pwrite -b 64k -V 1 -D 0 64k" /dev/nvme1n1p1
[   29.883653] block nvme1n1: no usable path - requeuing I/O

Fix by re-ordering the SRCU synchronization and scheduling the requeue
work.

[0] https://lore.kernel.org/linux-scsi/20260703102918.3723667-1-john.g.garry@oracle.com/T/#m72af1f29deb0ebfb2973464207f201f1be1f660c

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
I am not sure if we still require the synchronize_srcu() after nvme_cdev_del().

diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 016b6b0128c7..0b017eeb82b2 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -690,14 +690,15 @@ static void nvme_remove_head(struct nvme_ns_head *head)
 {
 	if (test_and_clear_bit(NVME_NSHEAD_DISK_LIVE, &head->flags)) {
 		/*
-		 * requeue I/O after NVME_NSHEAD_DISK_LIVE has been cleared
-		 * to allow multipath to fail all I/O.
+		 * Requeue I/O after NVME_NSHEAD_DISK_LIVE has been cleared
+		 * to allow multipath to fail all I/O. First synchronize to
+		 * add any bios to the requeue list.
 		 */
+		synchronize_srcu(&head->srcu);
 		kblockd_schedule_work(&head->requeue_work);
 
 		if (test_and_clear_bit(NVME_NSHEAD_CDEV_LIVE, &head->flags))
 			nvme_cdev_del(&head->cdev, &head->cdev_device);
-		synchronize_srcu(&head->srcu);
 		del_gendisk(head->disk);
 	}
 	nvme_put_ns_head(head);
-- 
2.43.0

Re: [PATCH] nvme: swap synchronization ordering in nvme_remove_head()

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH] nvme: swap synchronization ordering in nvme_remove_head()

[Nilay Shroff]

Hi John,

On 7/7/26 7:27 PM, John Garry wrote:
> From: John Garry <john.g.garry@oracle.com>
> 
> sashiko bot reported a potential issue in the requeue handling in [0] -
> the code there is same as the NVMe driver.
> 
> The issue is that when we schedule the requeue work, if a bio is added to
> the requeue list afterwards in nvme_ns_head_submit_bio(), it is missed by
> the requeue worker.
> 
> This issue can be recreated by hacking a large delay in the bio submission
> requeue path:
> 
>          } else if (nvme_available_path(head)) {
>                  dev_warn_ratelimited(dev, "no usable path - requeuing I/O\n");
> 
> +               msleep(30000);
>                  spin_lock_irq(&head->requeue_lock);
>                  bio_list_add(&head->requeue_list, bio);
>                  spin_unlock_irq(&head->requeue_lock);
> 
> 
> Then if we issue a write after removing all paths, a hang can be seen:
> 
> # echo 20 > /sys/devices/virtual/nvme-subsystem/nvme-subsys1/nvme1n1/delayed_removal_secs
> #
> # ./ini_nvme_teardown.sh
> [   25.877224] nvme nvme1: Removing ctrl: NQN "nvme-test-target"
> [   25.939569] nvme nvme2: Removing ctrl: NQN "nvme-test-target"
> #
> # xfs_io -d -C "pwrite -b 64k -V 1 -D 0 64k" /dev/nvme1n1p1
> [   29.883653] block nvme1n1: no usable path - requeuing I/O
> 
> Fix by re-ordering the SRCU synchronization and scheduling the requeue
> work.
> 
> [0] https://lore.kernel.org/linux-scsi/20260703102918.3723667-1-john.g.garry@oracle.com/T/#m72af1f29deb0ebfb2973464207f201f1be1f660c
> 
> Signed-off-by: John Garry <john.g.garry@oracle.com>
> ---
> I am not sure if we still require the synchronize_srcu() after nvme_cdev_del().
> 
> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
> index 016b6b0128c7..0b017eeb82b2 100644
> --- a/drivers/nvme/host/multipath.c
> +++ b/drivers/nvme/host/multipath.c
> @@ -690,14 +690,15 @@ static void nvme_remove_head(struct nvme_ns_head *head)
>   {
>   	if (test_and_clear_bit(NVME_NSHEAD_DISK_LIVE, &head->flags)) {
>   		/*
> -		 * requeue I/O after NVME_NSHEAD_DISK_LIVE has been cleared
> -		 * to allow multipath to fail all I/O.
> +		 * Requeue I/O after NVME_NSHEAD_DISK_LIVE has been cleared
> +		 * to allow multipath to fail all I/O. First synchronize to
> +		 * add any bios to the requeue list.
>   		 */
> +		synchronize_srcu(&head->srcu);
>   		kblockd_schedule_work(&head->requeue_work);
>   
>   		if (test_and_clear_bit(NVME_NSHEAD_CDEV_LIVE, &head->flags))
>   			nvme_cdev_del(&head->cdev, &head->cdev_device);
> -		synchronize_srcu(&head->srcu);
>   		del_gendisk(head->disk);
>   	}
>   	nvme_put_ns_head(head);

With the above change now I wonder do we really need to schedule ->requeue_work
from nvme_mpath_put_disk()? Maybe we still want to keep it as a defensive
"safety net". Though having it doesn't harm but looks redundant.

Otherwise, this change look good to me,

Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>

Re: [PATCH] nvme: swap synchronization ordering in nvme_remove_head()

[Keith Busch]

On Tue, Jul 07, 2026 at 02:57:44PM +0100, John Garry wrote:
> From: John Garry <john.g.garry@oracle.com>
> 
> sashiko bot reported a potential issue in the requeue handling in [0] -
> the code there is same as the NVMe driver.
> 
> The issue is that when we schedule the requeue work, if a bio is added to
> the requeue list afterwards in nvme_ns_head_submit_bio(), it is missed by
> the requeue worker.

Thanks, applied to nvme-7.3.