sorting out the freeze / quiesce mess

sorting out the freeze / quiesce mess

[Christoph Hellwig]

Hi Jens and Ming,

I've been looking into properly supporting queue freezing for bio based
drivers (that is only release q_usage_counter on bio completion for them).
And the deeper I look into the code the more I'm confused by us having
the blk_mq_quiesce* interface in addition to blk_freeze_queue.  What
is a good reason to do a quiesce separately from a freeze?

Re: sorting out the freeze / quiesce mess

[Ming Lei]

On Wed, Nov 10, 2021 at 10:14:07AM +0100, Christoph Hellwig wrote:
> Hi Jens and Ming,
> 
> I've been looking into properly supporting queue freezing for bio based
> drivers (that is only release q_usage_counter on bio completion for them).
> And the deeper I look into the code the more I'm confused by us having
> the blk_mq_quiesce* interface in addition to blk_freeze_queue.  What
> is a good reason to do a quiesce separately from a freeze?

freeze can make sure that all requests are done, quiesce can make sure that
dispatch critical area(covered by hctx lock/unlock) is done.


Thanks,
Ming

Re: sorting out the freeze / quiesce mess

[Ming Lei]

On Wed, Nov 10, 2021 at 05:29:26PM +0800, Ming Lei wrote:
> On Wed, Nov 10, 2021 at 10:14:07AM +0100, Christoph Hellwig wrote:
> > Hi Jens and Ming,
> > 
> > I've been looking into properly supporting queue freezing for bio based
> > drivers (that is only release q_usage_counter on bio completion for them).
> > And the deeper I look into the code the more I'm confused by us having
> > the blk_mq_quiesce* interface in addition to blk_freeze_queue.  What
> > is a good reason to do a quiesce separately from a freeze?
> 
> freeze can make sure that all requests are done, quiesce can make sure that
> dispatch critical area(covered by hctx lock/unlock) is done.

Another difference: quiesce usually is used to stop to queue requests to
LLD, and driver needs no requets queued any more after the interface returns,
freeze can't do that.

Thanks,
Ming

Re: sorting out the freeze / quiesce mess

[Christoph Hellwig]

On Wed, Nov 10, 2021 at 05:29:26PM +0800, Ming Lei wrote:
> On Wed, Nov 10, 2021 at 10:14:07AM +0100, Christoph Hellwig wrote:
> > Hi Jens and Ming,
> > 
> > I've been looking into properly supporting queue freezing for bio based
> > drivers (that is only release q_usage_counter on bio completion for them).
> > And the deeper I look into the code the more I'm confused by us having
> > the blk_mq_quiesce* interface in addition to blk_freeze_queue.  What
> > is a good reason to do a quiesce separately from a freeze?
> 
> freeze can make sure that all requests are done, quiesce can make sure that
> dispatch critical area(covered by hctx lock/unlock) is done.

Yeah, but why do we need to still call quiesce after we just did a
freeze, which is about half of the users?

Re: sorting out the freeze / quiesce mess

[Ming Lei]

On Wed, Nov 10, 2021 at 01:58:56PM +0100, Christoph Hellwig wrote:
> On Wed, Nov 10, 2021 at 05:29:26PM +0800, Ming Lei wrote:
> > On Wed, Nov 10, 2021 at 10:14:07AM +0100, Christoph Hellwig wrote:
> > > Hi Jens and Ming,
> > > 
> > > I've been looking into properly supporting queue freezing for bio based
> > > drivers (that is only release q_usage_counter on bio completion for them).
> > > And the deeper I look into the code the more I'm confused by us having
> > > the blk_mq_quiesce* interface in addition to blk_freeze_queue.  What
> > > is a good reason to do a quiesce separately from a freeze?
> > 
> > freeze can make sure that all requests are done, quiesce can make sure that
> > dispatch critical area(covered by hctx lock/unlock) is done.
> 
> Yeah, but why do we need to still call quiesce after we just did a
> freeze, which is about half of the users?

Because the caller need to make sure that dispatch critical area is
gone, otherwise dispatch code path may see intermediate state of the
change, such as switching elevator, and __blk_mq_update_nr_hw_queues()
may need quiesce too, I remember that there was kernel panic log in
some test.

Please see the point in commit 662156641bc4 ("block: don't drain
in-progress dispatch in blk_cleanup_queue()").


Thanks,
Ming

Re: sorting out the freeze / quiesce mess

[Hannes Reinecke]

On 11/10/21 10:14 AM, Christoph Hellwig wrote:
> Hi Jens and Ming,
> 
> I've been looking into properly supporting queue freezing for bio based
> drivers (that is only release q_usage_counter on bio completion for them).
> And the deeper I look into the code the more I'm confused by us having
> the blk_mq_quiesce* interface in addition to blk_freeze_queue.  What
> is a good reason to do a quiesce separately from a freeze?
> 
IIRC the 'quiesce' interface was an abstraction from the SCSI 'quiesce'
operation, where we had to stop all I/O except for TMFs and scanning.
And 'freeze' was designed fro stopping all I/O.

But I'm not sure if that ever was the distinction, or if it still
applies today.

And yeah, I've been wondering myself.

Probably we should just kill the 'quiesce' stuff and see where we end up :-)

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer

Re: sorting out the freeze / quiesce mess

[Ming Lei]

On Wed, Nov 10, 2021 at 11:22:05AM +0100, Hannes Reinecke wrote:
> On 11/10/21 10:14 AM, Christoph Hellwig wrote:
> > Hi Jens and Ming,
> > 
> > I've been looking into properly supporting queue freezing for bio based
> > drivers (that is only release q_usage_counter on bio completion for them).
> > And the deeper I look into the code the more I'm confused by us having
> > the blk_mq_quiesce* interface in addition to blk_freeze_queue.  What
> > is a good reason to do a quiesce separately from a freeze?
> > 
> IIRC the 'quiesce' interface was an abstraction from the SCSI 'quiesce'
> operation, where we had to stop all I/O except for TMFs and scanning.
> And 'freeze' was designed fro stopping all I/O.
> 
> But I'm not sure if that ever was the distinction, or if it still
> applies today.
> 
> And yeah, I've been wondering myself.
> 
> Probably we should just kill the 'quiesce' stuff and see where we end up :-)

In case of EH, no queued requests can be completed, however driver still
needs to stop queue and reset hardware, then how can you use freeze to stop
queue? See nvme_dev_disable().

Freeze can stop to allocate new request and drain all queued requests, but
it can't prevent IO from being queued to LLD. On the contrary,
blk_mq_freeze_queue_wait() requires LLD to handle IO for moving on,
otherwise it will wait forever.

Thanks,
Ming

Re: sorting out the freeze / quiesce mess

[James Bottomley]

On Wed, 2021-11-10 at 11:22 +0100, Hannes Reinecke wrote:
> On 11/10/21 10:14 AM, Christoph Hellwig wrote:
> > Hi Jens and Ming,
> > 
> > I've been looking into properly supporting queue freezing for bio
> > based
> > drivers (that is only release q_usage_counter on bio completion for
> > them).
> > And the deeper I look into the code the more I'm confused by us
> > having
> > the blk_mq_quiesce* interface in addition to
> > blk_freeze_queue.  What
> > is a good reason to do a quiesce separately from a freeze?
> > 
> IIRC the 'quiesce' interface was an abstraction from the SCSI
> 'quiesce'
> operation, where we had to stop all I/O except for TMFs and scanning.
> And 'freeze' was designed fro stopping all I/O.

Quiesce was a specific invention for SPI Domain Validation.  That's
where you try a specific set of patterns to the buffer and read them
back and try to jack up the transport parameters.  Pretty much every
transport does this type of retraining, but on all but SPI it's usually
hidden in hardware.

The point is you need to be able to stop all other I/O and then send
READ/WRITE_BUFFER commands to the device.

> But I'm not sure if that ever was the distinction, or if it still
> applies today.

As long as the SPI transport class exists, there'll be a distinction.
I'm not sure there's a use for quiesce beyond that.

> And yeah, I've been wondering myself.
> 
> Probably we should just kill the 'quiesce' stuff and see where we end
> up :-)

The SPI transport class would break.

James

Re: sorting out the freeze / quiesce mess

[Max Gurtovoy]

On 11/10/2021 11:14 AM, Christoph Hellwig wrote:
> Hi Jens and Ming,
>
> I've been looking into properly supporting queue freezing for bio based
> drivers (that is only release q_usage_counter on bio completion for them).
> And the deeper I look into the code the more I'm confused by us having
> the blk_mq_quiesce* interface in addition to blk_freeze_queue.  What
> is a good reason to do a quiesce separately from a freeze?

I think that quiesce_q will guarantee that no new requests will be 
passed to LLD (completions can still arrive from LLD).

And freeze_q will notify that the internal request_q state will not 
change and completions will not arrive from LLD (and if they will, the 
block layer can ignore it).

At least this is what we aim to do in other areas such as Live migration.