| Message ID | 20220324075119.1556334-14-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/13] nbd: use the correct block_device in nbd_ioctl | expand |
On Thu 24-03-22 08:51:19, Christoph Hellwig wrote: > 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> Looks good. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > drivers/block/loop.c | 26 +++++++++++++------------- > 1 file changed, 13 insertions(+), 13 deletions(-) > > diff --git a/drivers/block/loop.c b/drivers/block/loop.c > index e1eb925d3f855..84613eb2fdd57 100644 > --- a/drivers/block/loop.c > +++ b/drivers/block/loop.c > @@ -808,7 +808,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) > @@ -1043,20 +1042,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; > @@ -1152,10 +1150,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; > @@ -1749,6 +1743,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); > } > @@ -2012,6 +2010,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; > -- > 2.30.2 >
diff --git a/drivers/block/loop.c b/drivers/block/loop.c index e1eb925d3f855..84613eb2fdd57 100644 --- a/drivers/block/loop.c +++ b/drivers/block/loop.c @@ -808,7 +808,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) @@ -1043,20 +1042,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; @@ -1152,10 +1150,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; @@ -1749,6 +1743,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); } @@ -2012,6 +2010,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;
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> --- drivers/block/loop.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-)