Re: [PATCH] loop: replace freezing queue with quiesce when changing loop specific setting

[Christoph Hellwig <hch@lst.de>]

On Mon, Apr 07, 2025 at 10:54:01PM +0800, Ming Lei wrote:
> > What are "internal settings" please?  If you change the loop backing
> > file outstanding I/O is relevant.  If you change NVMe ANA or retry
> > policies are relevant as they are checked in the I/O completion handler.
> 
> internal setting means the driver internal setting, which is only visible
> for driver.
> 
> Here this setting(lo_offset, backing file, dio, ...) won't be used in
> completion handler, so it is fine to use quiesce here for updating
> these loop specific setting.

The backing file is used during I/O.  So you certainly can't change
it while I/O is in flight.

But the important point is that there is nothing inherent about
"internal" attributes needing different synchronization.  Maybe some
field don't need a full I/O quiesce, but you need to explain that
for each and every single one of them.

> 
> > 
> > > > This also misses an explanation of what setting this protects and why
> > > > you think this is safe and the sound fix.
> > > 
> > > 1) it is typical queue quiesce use case
> > 
> > What is the typical queue quiesce use case?  Why is it "typical" and
> > why is it safe.
> 
> typical quiesce provides sync with driver's ->queue_rq(), and it is typical
> that these driver settings are only used in driver submission code path.

"typical" does not matter.  The required synchronization needs to be
provided even for non-typical use cases.

> > > 3) for driver, quiesce is always preferred over freeze, and freeze is
> > > easily mis-used by driver, you know we have bad driver uses for freeze.
> > 
> > I am actually much more worried about quiesce.  It is much less well
> > defined.
> 
> It is widely used, and please see document of blk_mq_quiesce_queue().

I did not say it isn't widely used.  Which is part of the problem.