From mboxrd@z Thu Jan 1 00:00:00 1970 From: ming.lei@redhat.com (Ming Lei) Date: Tue, 17 Jul 2018 15:21:37 +0800 Subject: [PATCHv4 3/4] nvme: Introduce frozen controller state In-Reply-To: References: <20180713205609.19701-1-keith.busch@intel.com> <20180713205609.19701-4-keith.busch@intel.com> <20180716110903.GC25386@ming.t460p> <20180716133640.GC19967@localhost.localdomain> <20180717012301.GB22593@ming.t460p> Message-ID: <20180717072136.GB26324@ming.t460p> On Tue, Jul 17, 2018@01:49:17PM +0800, jianchao.wang wrote: > > > On 07/17/2018 09:23 AM, Ming Lei wrote: > > On Mon, Jul 16, 2018@07:36:40AM -0600, Keith Busch wrote: > >> On Mon, Jul 16, 2018@07:09:04PM +0800, Ming Lei wrote: > >>> On Fri, Jul 13, 2018@02:56:08PM -0600, Keith Busch wrote: > >>>> + if (ctrl->state == NVME_CTRL_FROZEN) { > >>>> + nvme_wait_freeze(ctrl); > >>>> + blk_mq_update_nr_hw_queues(ctrl->tagset, ctrl->queue_count - 1); > >>>> + nvme_unfreeze(ctrl); > >>>> + if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE)) > >>>> + return; > >>>> + } > >>>> + > >>> > >>> Now you move unfreezing into start work function, seems this way can't > >>> guarantee that nvme_unfreeze() is strictly paired with nvme_start_freeze() done > >>> in nvme_dev_disable(). > >> > >> The state machine is supposed to guarantee this. We only start freeze > >> in certain states from nvme_dev_disable, and transition to frozen when > >> exiting those states. I agree it's not the easiest API to follow, and > >> I'll double check to see if it's correct. I think I have the > >> reset/connecting/frozen/live transitions okay for nvme-pci, but I need > >> to look again at the deleting/dead states. > > > > I am afraid that may not work well wrt. reset vs. timeout because > > timeout still may happen during reset on admin queue, and nvme_dev_disable() > > may call nvme_start_freeze() one more time, meantime reset_work won't be > > scheduled. > > > > Hi Ming > > In nvme_dev_disable, except for the shutdown case which will try to drain the IO, > what we want is to stop new IO coming in. freeze queue interfaces looks too heavy, > after freeze, we have to wait for the q_usage_counter to be drained. > Is there any chance to just stop new IO coming in or just drain IO ? nvme_start_freeze() is just for stopping new IO. However, if you want to drain IO, new IO may have to be prevented from being entering queue first, then that is nvme_start_freeze() + nvme_wait_freeze(). Thanks, Ming