Re: [PATCHv2 6/6] nvme: Wait for reset state when required

Keith Busch
2019-09-20T18:26:44Z

On Fri, Sep 20, 2019 at 11:13:06AM -0700, Sagi Grimberg wrote:
> On 9/19/19 2:34 PM, kbusch@kernel.org wrote:
> > static void nvme_reset_done(struct pci_dev *pdev)
> > {
> > struct nvme_dev *dev = pci_get_drvdata(pdev);
> > - nvme_reset_ctrl_sync(&dev->ctrl);
> > +
> > + if (!nvme_reset_continue(&dev->ctrl))
> > + flush_work(&dev->ctrl.reset_work);
>
> Is nvme_reset_continue allowed to fail here?
> As I see it, the flush must complete regardless doesn't it?

I was thinking a surprise hot-removal, but no, the pci driver is holding
the device lock across the prepare/done sequence, so I don't think it's
possible for this reset_continue() to fail to schedule.

> > }
> > static void nvme_shutdown(struct pci_dev *pdev)
> > {
> > struct nvme_dev *dev = pci_get_drvdata(pdev);
> > - nvme_dev_disable(dev, true);
> > + nvme_disable_prepare_reset(dev, true);
> > }
> > /*
> > @@ -2898,7 +2920,7 @@ static int nvme_resume(struct device *dev)
> > if (ndev->last_ps == U32_MAX ||
> > nvme_set_power_state(ctrl, ndev->last_ps) != 0)
> > - nvme_reset_ctrl(ctrl);
> > + return nvme_reset_continue(&ndev->ctrl);
>
> Is the controller allowed not to be in RESETTING in this stage?
> do we need a WARN_ON if it is?

This one, though, I think a hot removal can cause this to fail since the
device lock isn't held from suspend all the way through resume, so we
could be in the DELETING or DEAD state.