On Thu 02-12-21 23:39:42, Tetsuo Handa wrote:

On 2021/12/02 21:16, Jan Kara wrote:

quoted

Why not scheduling this using task_work_add()? It solves the locking
context problems, has generally lower overhead than normal work (no need to
schedule), and avoids possible unexpected side-effects of releasing
loopback device later. Also task work is specifically designed so that one
task work can queue another task work so we should be fine using it.

Indeed. But that will make really no difference between synchronous approach
( https://lkml.kernel.org/r/fb6adcdc-fb56-3b90-355b-3f5a81220f2b@i-love.sakura.ne.jp )
and asynchronous approach
( https://lkml.kernel.org/r/d1f760f9-cdb2-f40d-33d8-bfa517c731be@i-love.sakura.ne.jp ), for
disk->open_mutex is the only lock held when lo_release() is called.

Both approaches allow __loop_clr_fd() to run with no lock held, and both
approaches need to be aware of what actions are taken by blkdev_put()
before and after dropping disk->open_mutex. And
bdev->bd_disk->fops->release() is the last action taken before dropping
disk->open_mutex.

What is so happier with preventing what will be done after
disk->open_mutex is dropped by blkdev_put() (i.e. __module_get() +
kobject_get() before blkdev_put() calls kobject_put() + module_put(), and
kobject_put() + module_put() upon task_work_run()), compared to doing
things that can be done without disk->open_mutex (i.e. calling
__loop_clr_fd() without disk->open_mutex) ?

So the advantage of using task work instead of just dropping open_mutex
before calling __loop_clr_fd() is that if something in block/bdev.c ever
changes and starts relying on open_mutex being held throughout blkdev_put()
then loop device handling will not suddently become broken. Generally it is
a bad practice to drop locks (even temporarily) upper layers have acquired.
Sometimes it is inevitable in in this case we can avoid that... So I'd
prefer if we used task work instead of dropping open_mutex inside loop
driver. Not sure what's Christoph's opinion though, I don't feel *that*
strongly about it.

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