On 2022/02/08 22:45, Jan Kara wrote: > On Sat 05-02-22 09:28:33, Tetsuo Handa wrote: >> Ping? >> >> I sent https://lkml.kernel.org/r/20220129071500.3566-1-penguin-kernel@xxxxxxxxxxxxxxxxxxx >> based on ideas from your series. >> >> Since automated kernel tests are failing, can't we apply >> [PATCH 1/7] loop: revert "make autoclear operation asynchronous" >> for now if we can't come to a conclusion? > > That's certainly a good start so feel free to add my Acked-by to the > revert. I agree it should be merged quickly as I think it is better to have > a theoretical deadlock in the code than userspace breakage hit in the wild. > I'll find some more time to look into this but it will take a while. Did you get a chance to look into this? As far as I tested, I found two problems ( https://lkml.kernel.org/r/a72c59c6-298b-e4ba-b1f5-2275afab49a1@xxxxxxxxxxxxxxxxxxx ). That is, waiting for lo->lo_mutex upon open is not only for /bin/mount but for other programs. I found that we can use anon_inodes approach (basic idea is shown below) as a way to use task_work context. If we can agree with this approach, I can re-implement https://lkml.kernel.org/r/20220121114006.3633-1-penguin-kernel@xxxxxxxxxxxxxxxxxxx without exporting task_work_add(). diff --git a/drivers/block/loop.c b/drivers/block/loop.c index 19fe19eaa50e..6bd6af1836c6 100644 --- a/drivers/block/loop.c +++ b/drivers/block/loop.c @@ -80,6 +80,7 @@ #include <linux/blk-cgroup.h> #include <linux/sched/mm.h> #include <linux/statfs.h> +#include <linux/anon_inodes.h> #include "loop.h" @@ -1736,6 +1737,25 @@ static int lo_open(struct block_device *bdev, fmode_t mode) return err; } +static int loop_no_open(struct inode *inode, struct file *file) +{ + return -ENXIO; +} + +static int loop_post_release(struct inode *inode, struct file *file) +{ + struct loop_device *lo = file->private_data; + + pr_info("Performing autoclear operation.\n"); + __loop_clr_fd(lo, false); + return 0; +} + +static const struct file_operations loop_close_fops = { + .open = loop_no_open, + .release = loop_post_release, +}; + static void lo_release(struct gendisk *disk, fmode_t mode) { struct loop_device *lo = disk->private_data; @@ -1745,6 +1765,8 @@ static void lo_release(struct gendisk *disk, fmode_t mode) goto out_unlock; if (lo->lo_flags & LO_FLAGS_AUTOCLEAR) { + struct file *file; + if (lo->lo_state != Lo_bound) goto out_unlock; lo->lo_state = Lo_rundown; @@ -1753,7 +1775,9 @@ static void lo_release(struct gendisk *disk, fmode_t mode) * In autoclear mode, stop the loop thread * and remove configuration after last close. */ - __loop_clr_fd(lo, true); + file = anon_inode_getfile("loop.close", &loop_close_fops, lo, O_CLOEXEC); + if (!IS_ERR(file)) + fput(file); return; } else if (lo->lo_state == Lo_bound) { /*
