On Sat 15-01-22 00:50:32, Tetsuo Handa wrote:

On 2022/01/13 19:44, Jan Kara wrote:

quoted

OK, so I think I understand the lockdep complaint better. Lockdep
essentially complains about the following scenario:

blkdev_put()
  lock disk->open_mutex
  lo_release
    __loop_clr_fd()
        |
        | wait for IO to complete
        v
loop worker
  write to backing file
    sb_start_write(file)
        |
        | wait for fs with backing file to unfreeze
        v
process holding fs frozen
  freeze_super()
        |
        | wait for remaining writers on the fs so that fs can be frozen
        v
sendfile()
  sb_start_write()
  do_splice_direct()
        |
        | blocked on read from /sys/power/resume, waiting for kernfs file
        | lock
        v
write() "/dev/loop0" (in whatever form) to /sys/power/resume
  calls blkdev_get_by_dev("/dev/loop0")
    lock disk->open_mutex => deadlock

Then, not calling flush_workqueue() from destroy_workqueue() from
__loop_clr_fd() will not help because the reason we did not need to call
flush_workqueue() is that blk_mq_freeze_queue() waits until all "struct
work_struct" which flush_workqueue() would have waited completes?

Yes.

If flush_workqueue() cannot complete because an I/O request cannot
complete, blk_mq_freeze_queue() as well cannot complete because it waits
for I/O requests which flush_workqueue() would have waited?

Then, any attempt to revert commit 322c4293ecc58110 ("loop: make
autoclear operation asynchronous")
(e.g.
https://lkml.kernel.org/r/4e7b711f-744b-3a78-39be-c9432a3cecd2@i-love.sakura.ne.jp
) cannot be used?

Well, it could be used but the deadlock would be reintroduced. There are
still possibilities to fix it in some other way. But for now I think that
async loop cleanup is probably the least painful solution.

Now that commit 322c4293ecc58110 already arrived at linux.git, we need to
quickly send a fixup patch for these reported regressions. This "[PATCH
v2 2/2] loop: use task_work for autoclear operation" did not address "if
(lo->lo_state == Lo_bound) { }" part. If we address this part, something
like below diff?

Please no. That is too ugly to live. I'd go just with attached version of
your solution. That should fix the xfstests regression. The LTP regression
needs to be fixed in mount.

								Honza
-- 
Jan Kara [off-list ref]
SUSE Labs, CR