On Wed, Jan 26, 2022 at 06:21:59AM +0100, Christoph Hellwig wrote:
> On Wed, Jan 26, 2022 at 08:47:17AM +0900, Tetsuo Handa wrote:
> > > As far as I can tell we do not need the freeze at all for given that
> > > by the time release is called I/O is quiesced.
> >
> > Why? lo_release() is called when close() is called. But (periodically-scheduled
> > or triggered-on-demand) writeback of previously executed buffered write() calls
> > can start while lo_release() or __loop_clr_fd() is running. Then, why not to
> > wait for I/O requests to complete?
>
> Let's refine my wording, the above should be "... by the time the final
> lo_release is called". blkdev_put_whole ensures all writeback has finished
> and all buffers are gone by writing all data back and waiting for it and then
> truncating the pages from blkdev_flush_mapping.
>
> > Isn't that the reason of
> >
> > } else if (lo->lo_state == Lo_bound) {
> > /*
> > * Otherwise keep thread (if running) and config,
> > * but flush possible ongoing bios in thread.
> > */
> > blk_mq_freeze_queue(lo->lo_queue);
> > blk_mq_unfreeze_queue(lo->lo_queue);
> > }
> >
> > path in lo_release() being there?
>
> This looks completely spurious to me. Adding Ming who added it.
It was added when converting to blk-mq.
I remember it was to replace original loop_flush() which uses
wait_for_completion() for draining all inflight bios, but seems
the flush isn't needed in lo_release().
Thanks,
Ming
