Re: [PATCH v3 1/5] task_work: export task_work_add()

[Ming Lei <ming.lei@redhat.com>]

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