Re: [PATCH v3] nvme: core: shorten duration of multipath namespace rescan

[Martin Wilck <mwilck@suse.com>]

On Tue, 2024-08-27 at 08:33 +0200, Christoph Hellwig wrote:
> > +	/*
> > +	 * The controller queue is going to be frozen in
> > +	 * nvme_update_ns_info_{generic,block}(). Every freeze
> > implies waiting
> > +	 * for an RCU grace period to pass. For multipath devices,
> > we
> > +	 * need to freeze the multipath queue, too. Start freezing
> > the
> > +	 * multipath queue now, lest we need to wait for two grace
> > periods.
> > +	 */
> > +	if (nvme_ns_head_multipath(ns->head))
> > +		blk_freeze_queue_start(ns->head->disk->queue);
> 
> I really don't like how we keep this state around the calls into
> the command set specific nvme_update_ns_info_* calls, and how it
> feels very open coded.
> 
> And while looking at it, I think we might even want the multipath
> queue frozen over the actual update as well, as there is no point
> in sending the I/O even to other paths while we are updating the
> information, as they'll probably fail.
> 
> Here is what might make sense:
> 
>  1) move the code past the csi switch statement in
> nvme_update_ns_info
>     into a new nvme_update_ns_info_common helper and all that
>     from nvme_update_ns_info_block and nvme_update_ns_info_generic.
>     Maybe having another helper just for the multipath updates might
>     also be worthwhile.
>  2) add new nvme_freeze_ns helpers that freeze the ns and nshead
>     queues and use those in nvme_update_ns_info_block and
>     nvme_update_ns_info_generic over the entire update

Not sure if I understood you correctly, but a single helper for
unfreezing both the path and the multipath queues won't work.The
following code block

        if (blk_queue_is_zoned(ns->queue)) {
                ret = blk_revalidate_disk_zones(ns->disk);
                if (ret && !nvme_first_scan(ns->disk))
                        goto out;
        }

must be called from nvme_update_ns_info_block() after unfreezing the
path queue, but its result is used in the code that must run with the
multipath queue still frozen. So we can freeze both queues at the same
time, but we must unfreeze them separately.

Still trying to work this out.

Martin