diff mbox series

[5/7] loop: don't call destroy_workqueue() from lo_release()

Message ID 20220129071500.3566-6-penguin-kernel@I-love.SAKURA.ne.jp (mailing list archive)
State New, archived
Headers show
Series [1/7] loop: revert "make autoclear operation asynchronous" | expand

Commit Message

Tetsuo Handa Jan. 29, 2022, 7:14 a.m. UTC
syzbot is reporting circular locking problem at __loop_clr_fd() [1], for
commit 87579e9b7d8dc36e ("loop: use worker per cgroup instead of kworker")
is calling destroy_workqueue() with disk->open_mutex held.

But it turned out that there is no need to call flush_workqueue() from
lo_release(), for all pending I/O requests are flushed by the block core
layer before "struct block_device_operations"->release() is called.

In order to silence lockdep warning, defer calling destroy_workqueue()
till loop_remove(). Note that the root cause is that lo_open()/lo_release()
waits for I/O requests with disk->open_mutex held, and we need to avoid
holding lo->lo_mutex from lo_open()/lo_release() paths.

Link: https://syzkaller.appspot.com/bug?extid=643e4ce4b6ad1347d372 [1]
Reported-by: syzbot <syzbot+643e4ce4b6ad1347d372@syzkaller.appspotmail.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 drivers/block/loop.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
diff mbox series

Patch

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index b6435f803061..481d2371864a 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1003,7 +1003,8 @@  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",
+	if (!lo->workqueue)
+		lo->workqueue = alloc_workqueue("loop%d",
 					WQ_UNBOUND | WQ_FREEZABLE,
 					0,
 					lo->lo_number);
@@ -1108,10 +1109,11 @@  static void __loop_clr_fd(struct loop_device *lo, bool release)
 		blk_queue_write_cache(lo->lo_queue, false, false);
 
 	/* freeze request queue during the transition */
-	if (!release)
+	if (!release) {
 		blk_mq_freeze_queue(lo->lo_queue);
-
-	destroy_workqueue(lo->workqueue);
+		destroy_workqueue(lo->workqueue);
+		lo->workqueue = NULL;
+	}
 	spin_lock_irq(&lo->lo_work_lock);
 	list_for_each_entry_safe(worker, pos, &lo->idle_worker_list,
 				idle_list) {
@@ -2050,6 +2052,8 @@  static void loop_remove(struct loop_device *lo)
 	mutex_lock(&loop_ctl_mutex);
 	idr_remove(&loop_index_idr, lo->lo_number);
 	mutex_unlock(&loop_ctl_mutex);
+	if (lo->workqueue)
+		destroy_workqueue(lo->workqueue);
 	/* There is no route which can find this loop device. */
 	mutex_destroy(&lo->lo_mutex);
 	kfree(lo);