loop: don't destroy lo->workqueue in __loop_clr_fd

There is no need to destroy the workqueue when clearing unbinding
a loop device from a backing file.  Not doing so on the other hand
avoid creating a complex lock dependency chain involving the global
system_transition_mutex.

Based on a patch from Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>.

Reported-by: syzbot+6479585dfd4dedd3f7e1@syzkaller.appspotmail.com
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jan Kara <jack@suse.cz>
Tested-by: syzbot+6479585dfd4dedd3f7e1@syzkaller.appspotmail.com
Link: https://lore.kernel.org/r/20220330052917.2566582-16-hch@lst.de
Signed-off-by: Jens Axboe <axboe@kernel.dk>

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 204558d..0c7f036 100644 (file)
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -812,7 +812,6 @@ struct loop_worker {
 };
 
 static void loop_workfn(struct work_struct *work);
-static void loop_rootcg_workfn(struct work_struct *work);
 
 #ifdef CONFIG_BLK_CGROUP
 static inline int queue_on_root_worker(struct cgroup_subsys_state *css)
@@ -1050,20 +1049,19 @@ static int loop_configure(struct loop_device *lo, fmode_t mode,
            !file->f_op->write_iter)
                lo->lo_flags |= LO_FLAGS_READ_ONLY;
 
-       lo->workqueue = alloc_workqueue("loop%d",
-                                       WQ_UNBOUND | WQ_FREEZABLE,
-                                       0,
-                                       lo->lo_number);
        if (!lo->workqueue) {
-               error = -ENOMEM;
-               goto out_unlock;
+               lo->workqueue = alloc_workqueue("loop%d",
+                                               WQ_UNBOUND | WQ_FREEZABLE,
+                                               0, lo->lo_number);
+               if (!lo->workqueue) {
+                       error = -ENOMEM;
+                       goto out_unlock;
+               }
        }
 
        disk_force_media_change(lo->lo_disk, DISK_EVENT_MEDIA_CHANGE);
        set_disk_ro(lo->lo_disk, (lo->lo_flags & LO_FLAGS_READ_ONLY) != 0);
 
-       INIT_WORK(&lo->rootcg_work, loop_rootcg_workfn);
-       INIT_LIST_HEAD(&lo->rootcg_cmd_list);
        lo->use_dio = lo->lo_flags & LO_FLAGS_DIRECT_IO;
        lo->lo_device = bdev;
        lo->lo_backing_file = file;
@@ -1143,10 +1141,6 @@ static void __loop_clr_fd(struct loop_device *lo, bool release)
        if (!release)
                blk_mq_freeze_queue(lo->lo_queue);
 
-       destroy_workqueue(lo->workqueue);
-       loop_free_idle_workers(lo, true);
-       del_timer_sync(&lo->timer);
-
        spin_lock_irq(&lo->lo_lock);
        filp = lo->lo_backing_file;
        lo->lo_backing_file = NULL;
@@ -1750,6 +1744,10 @@ static void lo_free_disk(struct gendisk *disk)
 {
        struct loop_device *lo = disk->private_data;
 
+       if (lo->workqueue)
+               destroy_workqueue(lo->workqueue);
+       loop_free_idle_workers(lo, true);
+       del_timer_sync(&lo->timer);
        mutex_destroy(&lo->lo_mutex);
        kfree(lo);
 }
@@ -2013,6 +2011,8 @@ static int loop_add(int i)
        lo->lo_number           = i;
        spin_lock_init(&lo->lo_lock);
        spin_lock_init(&lo->lo_work_lock);
+       INIT_WORK(&lo->rootcg_work, loop_rootcg_workfn);
+       INIT_LIST_HEAD(&lo->rootcg_cmd_list);
        disk->major             = LOOP_MAJOR;
        disk->first_minor       = i << part_shift;
        disk->minors            = 1 << part_shift;