Re: [PATCH] nvme-multipath: don't wait for a grace period in nvme_mpath_set_live()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] nvme-multipath: unlocked requeue_srcu_cookie update in
  nvme_mpath_set_live()
It also noticed 1 related pre-existing issue this patch does not
introduce:
- [High] nvme-multipath: stranded I/O in delayed_removal_secs_store()
  [1]
--

commit c9b236e98759e76a4ecd5a5f54a6bd1f8d9ab7be
Author: Qiliang Yuan <qiliang.yuan@linux.dev>

nvme-multipath: don't wait for a grace period in nvme_mpath_set_live()

This commit replaces the synchronous synchronize_srcu() call in
nvme_mpath_set_live() with start_poll_synchronize_srcu() and defers
waiting for the SRCU grace period to nvme_requeue_work().

> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
> index ae8059366b57d..4bc94f165c17d 100644
> --- a/drivers/nvme/host/multipath.c
> +++ b/drivers/nvme/host/multipath.c

[ ... ]

> @@ -851,7 +857,13 @@ static void nvme_mpath_set_live(struct nvme_ns *ns)
>  	}
>  	mutex_unlock(&head->lock);
>  
> -	synchronize_srcu(&head->srcu);
> +	/*
> +	 * Have nvme_requeue_work() wait for the grace period instead of
> +	 * blocking here, so that namespaces going live together, e.g. on
> +	 * an ANA change, do not wait for one grace period each.
> +	 */
> +	WRITE_ONCE(head->requeue_srcu_cookie,
> +		   start_poll_synchronize_srcu(&head->srcu));
>  	nvme_mpath_revalidate_zones(head);
>  	kblockd_schedule_work(&head->requeue_work);
>  }

[Severity: Medium]
Can concurrent nvme_mpath_set_live() calls on the same head overwrite a
newer head->requeue_srcu_cookie with an older, already-completed cookie?

CPU0                              CPU1
nvme_mpath_set_live(ns1)
  mutex_unlock(&head->lock)
  c1 = start_poll_synchronize_srcu()
  // grace period c1 completes;
  // ns1 fails over again, and a
  // reader enters srcu_read_lock()
  // seeing no usable path
                                  nvme_mpath_set_live(ns2)
                                    mutex_unlock(&head->lock)
                                    c2 = start_poll_synchronize_srcu()
                                    WRITE_ONCE(requeue_srcu_cookie, c2)
  WRITE_ONCE(requeue_srcu_cookie, c1)
                                    kblockd_schedule_work(&requeue_work)
                                  nvme_requeue_work()
                                    poll_state_synchronize_srcu(..., c1)
                                      // returns true, skips sync
                                    bio_list_get(&head->requeue_list)
                                      <-- misses in-flight reader's bio

When that happens, nvme_requeue_work() drains head->requeue_list before
the in-flight nvme_ns_head_submit_bio() reader finishes queueing its bio,
leaving the bio stranded on head->requeue_list until a later path event.
Updating head->requeue_srcu_cookie before mutex_unlock(&head->lock) would
serialize the cookie generation and store.

[Severity: High]
This problem wasn't introduced by this patch, but in the same file,
delayed_removal_secs_store() [1] can leave parked bios stranded on
head->requeue_list when delayed_removal_secs is set to 0:

drivers/nvme/host/multipath.c:delayed_removal_secs_store() {
	...
	mutex_lock(&head->subsys->lock);
	head->delayed_removal_secs = sec;
	assign_bit(NVME_NSHEAD_QUEUE_IF_NO_PATH, &head->flags, sec);
	mutex_unlock(&head->subsys->lock);
	/*
	 * Ensure that update to NVME_NSHEAD_QUEUE_IF_NO_PATH is seen
	 * by its reader.
	 */
	synchronize_srcu(&head->srcu);

	return count;
}

When writing 0 clears NVME_NSHEAD_QUEUE_IF_NO_PATH while no paths are
usable, any bios already queued on head->requeue_list need to be drained
and failed, but delayed_removal_secs_store() returns without calling
kblockd_schedule_work(&head->requeue_work), leaving tasks waiting on
those bios hung until a separate path event schedules requeue_work.

[1] https://sashiko.dev/#/bug/linux-7d2cc5ea-2752-4473-b874-a955225beb1b

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110-bug-nvme-mpath-set-live-srcu-v1-1-1f58b405b0f8@linux.dev?part=1