Re: [PATCH 1/2] nvme: remove namespaces in parallel

[Hannes Reinecke <hare@suse.de>]

On 11/2/26 7:14 AM, Qiliang Yuan wrote:
> nvme_remove_namespaces() removes the namespaces of a controller one
> after another. Each removal waits for RCU grace periods in
> del_gendisk() for the namespace and for its multipath head, about
> 20 ms in all.
> 
But that isn't changed by calling async_schedule_domain, isn't it?

> Deleting a controller, such as disconnecting from a fabrics target,
> thus takes time proportional to its namespace count, over a second for
> 64 namespaces. Scanning had the same problem until commit 4e893ca81170
> ("nvme_core: scan namespaces asynchronously") made it add namespaces
> in parallel.
> 
> Remove the namespaces through an async domain the same way, and wait
> for all of them before returning.
> 
> Disconnect time from a target on the same host, over tcp to 127.0.0.1
> and over nvme-loop, with 4 I/O queues, median of 3 runs on each of two
> boots per kernel:
> 
>    namespaces  transport  before              after
>             8  tcp         170 /  171 ms        85 /   91 ms
>            32  tcp         628 /  628 ms       294 /  293 ms
>            64  tcp        1201 / 1308 ms       533 /  550 ms
>            64  loop       1988 / 1913 ms      1283 / 1200 ms
> 
> Signed-off-by: Qiliang Yuan <odys.yuan@gmail.com>
> ---
>   drivers/nvme/host/core.c | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index a7a4d09644c13..18948c2ba218d 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -4716,6 +4716,11 @@ static void nvme_scan_work(struct work_struct *work)
>   #endif
>   }
>   
> +static void nvme_ns_remove_async(void *data, async_cookie_t cookie)
> +{
> +	nvme_ns_remove(data);
> +}
> +
>   /*
>    * This function iterates the namespace list unlocked to allow recovery from
>    * controller failure. It is up to the caller to ensure the namespace list is
> @@ -4725,6 +4730,7 @@ void nvme_remove_namespaces(struct nvme_ctrl *ctrl)
>   {
>   	struct nvme_ns *ns, *next;
>   	LIST_HEAD(ns_list);
> +	ASYNC_DOMAIN(domain);
>   
>   	/*
>   	 * make sure to requeue I/O to all namespaces as these
> @@ -4759,8 +4765,13 @@ void nvme_remove_namespaces(struct nvme_ctrl *ctrl)
>   	mutex_unlock(&ctrl->namespaces_lock);
>   	synchronize_srcu(&ctrl->srcu);
>   
> +	/*
> +	 * Remove the namespaces in parallel, as each removal waits for several
> +	 * RCU grace periods.
> +	 */
>   	list_for_each_entry_safe(ns, next, &ns_list, list)
> -		nvme_ns_remove(ns);
> +		async_schedule_domain(nvme_ns_remove_async, ns, &domain);
> +	async_synchronize_full_domain(&domain);
>   }
>   EXPORT_SYMBOL_GPL(nvme_remove_namespaces);
>   
> 
Other than that it looks fine.

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