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

[Hannes Reinecke <hare@suse.de>]

On 11/11/26 2:10 PM, Qiliang Yuan wrote:
> When a path goes live, nvme_mpath_set_live() waits for an SRCU grace
> period of the namespace head before kicking the requeue work, so that
> bios which found no usable path are on the requeue list by the time it
> runs.
> 
> The ANA work calls nvme_mpath_set_live() for every namespace that
> becomes optimized or non-optimized, one after another, so an ANA change
> waits for one grace period per namespace.
> 
> Start the grace period with start_poll_synchronize_srcu() instead and
> have nvme_requeue_work() wait for it only if it has not completed yet.
> Take the cookie under head->lock, so that concurrent callers store
> their cookies in order and the requeue work never sees an older one.
> The grace periods of all namespaces then run in parallel, and requeue
> work kicked for other reasons finds the cookie completed and does not
> wait.
> 
> Flipping an ANA group from inaccessible to optimized ten times over
> nvme-tcp loopback, nvme_read_ana_log() takes (median, min-max in
> parentheses):
> 
>    namespaces  before (ms)          after (ms)        change
>           256  51.9 (42.5-85.4)     8.9 (2.6-12.0)      -83%
>          1024  157.5 (133.5-179.6)  49.5 (32.8-67.1)    -69%
> 
> Signed-off-by: Qiliang Yuan <qiliang.yuan@linux.dev>
> ---
>   drivers/nvme/host/multipath.c | 15 ++++++++++++++-
>   drivers/nvme/host/nvme.h      |  1 +
>   2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
> index a92a05b1f925a..9c1023102902c 100644
> --- a/drivers/nvme/host/multipath.c
> +++ b/drivers/nvme/host/multipath.c
> @@ -712,6 +712,11 @@ static void nvme_requeue_work(struct work_struct *work)
>   		container_of(work, struct nvme_ns_head, requeue_work);
>   	struct bio *bio, *next;
>   
> +	/* Let bios that found no usable path reach the requeue list first. */
> +	if (!poll_state_synchronize_srcu(&head->srcu,
> +					 READ_ONCE(head->requeue_srcu_cookie)))
> +		synchronize_srcu(&head->srcu);
> +
>   	spin_lock_irq(&head->requeue_lock);
>   	next = bio_list_get(&head->requeue_list);
>   	spin_unlock_irq(&head->requeue_lock);
> @@ -767,6 +772,7 @@ int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl, struct nvme_ns_head *head)
>   	mutex_init(&head->lock);
>   	spin_lock_init(&head->requeue_lock);
>   	INIT_WORK(&head->requeue_work, nvme_requeue_work);
> +	head->requeue_srcu_cookie = get_completed_synchronize_srcu();
>   	INIT_WORK(&head->partition_scan_work, nvme_partition_scan_work);
>   	INIT_DELAYED_WORK(&head->remove_work, nvme_remove_head_work);
>   
> @@ -849,9 +855,16 @@ static void nvme_mpath_set_live(struct nvme_ns *ns)
>   			__nvme_find_path(head, node);
>   		srcu_read_unlock(&head->srcu, srcu_idx);
>   	}
> +	/*
> +	 * 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. Start it
> +	 * under head->lock so that the newest cookie is stored last.
> +	 */
> +	WRITE_ONCE(head->requeue_srcu_cookie,
> +		   start_poll_synchronize_srcu(&head->srcu));
>   	mutex_unlock(&head->lock);
>   
> -	synchronize_srcu(&head->srcu);
>   	nvme_mpath_revalidate_zones(head);
>   	kblockd_schedule_work(&head->requeue_work);
>   }
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index 8833a463f1c84..3fff9b394b1cd 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -587,6 +587,7 @@ struct nvme_ns_head {
>   		__guarded_by(&requeue_lock);
>   	spinlock_t		requeue_lock;
>   	struct work_struct	requeue_work;
> +	unsigned long		requeue_srcu_cookie;
>   	struct work_struct	partition_scan_work;
>   	struct mutex		lock;
>   	unsigned long		flags;
> 
Looks good.

Reviewed-by: Hannes Reinecke <hare@kernel.org>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich