[PATCH] nvme: ensure disabling pairs with unquiesce

[Keith Busch]

On Thu, Jul 06, 2023 at 02:51:34PM +0200, Christoph Hellwig wrote:
> > +static int nvme_disable_prepare_reset(struct nvme_dev *dev, bool shutdown);
> 
> Can we rename nvme_disable_prepare_reset while we're at it?  The
> name already is a bit suboptimal in the xisting code, but with
> the new calles becomes really confusing.
> 
> nvme_pci_wait_for_reset_and_disable is what I could come up with.
> It feels a bit too verbose, but at least it's descriptive.
> 
> > +disable:
> > +	if (!nvme_disable_prepare_reset(dev, false) &&
> > +	    nvme_try_sched_reset(&dev->ctrl))
> 
> Why do we wait for a reset just to schedule another reset here?

nvme_disable_prepare_reset() just waits for the ctrl->state to become
"RESETTING" so the code path knows no one else can schedule a reset
during disable. The actual reset has to enable happens later.

The following nvme_try_sched_reset() just does the required 2nd part if
the state change was successful. The only way that should fail is if the
controller was removed during the disabling.

There is a problem with multi-namespace, though: the reset_work
synchronizes each request_queue while in the RESETTING state, so if each
request_queue's timeout handler blocks for the RESETTING state, then
it's a deadlock. I will need to reconsider how to fix this.

> Also I feel splitting out the changes to wait for reset from those
> that add the unquiesce when we can't reset into another patch
> would probably help beeing able to understand the changes here.