On Wed, May 20, 2020 at 10:10:47AM -0700, Dongli Zhang wrote: > On 5/20/20 4:56 AM, Ming Lei wrote: > > +static bool nvme_wait_freeze_and_check(struct nvme_dev *dev) > > +{ > > + bool frozen; > > + > > + while (true) { > > + frozen = nvme_frozen(&dev->ctrl); > > + if (frozen) > > + break; > > + if (!dev->online_queues) > > + break; > > + msleep(5); > > + } > > + > > + return frozen; > > +} > > + > > static void nvme_reset_work(struct work_struct *work) > > { > > struct nvme_dev *dev = > > container_of(work, struct nvme_dev, ctrl.reset_work); > > bool was_suspend = !!(dev->ctrl.ctrl_config & NVME_CC_SHN_NORMAL); > > int result; > > + bool reset_done = true; > > > > if (WARN_ON(dev->ctrl.state != NVME_CTRL_RESETTING)) { > > result = -ENODEV; > > @@ -2606,8 +2622,9 @@ static void nvme_reset_work(struct work_struct *work) > > nvme_free_tagset(dev); > > } else { > > nvme_start_queues(&dev->ctrl); > > - nvme_wait_freeze(&dev->ctrl); > > - nvme_dev_add(dev); > > + reset_done = nvme_wait_freeze_and_check(dev); > > Once we arrive at here, it indicates "dev->online_queues >= 2". > > 2601 if (dev->online_queues < 2) { > 2602 dev_warn(dev->ctrl.device, "IO queues not created\n"); > 2603 nvme_kill_queues(&dev->ctrl); > 2604 nvme_remove_namespaces(&dev->ctrl); > 2605 nvme_free_tagset(dev); > 2606 } else { > 2607 nvme_start_queues(&dev->ctrl); > 2608 nvme_wait_freeze(&dev->ctrl); > 2609 nvme_dev_add(dev); > 2610 nvme_unfreeze(&dev->ctrl); > 2611 } > > Is there any reason to check "if (!dev->online_queues)" in > nvme_wait_freeze_and_check()? Looks correct to me. If the queues fail to freeze, that means a timeout occured, and the nvme timeout handler tears down all online queues, so this patch uses that for the criteria to break out of the loop.