[PATCH] md: don't flush md_misc_wq from md_alloc()

[Li Youhong]

From: Li Youhong <liyouhong@xxxxxxxxxx>

md_alloc() is called from md_probe(), while blk_probe_dev() still holds
major_names_lock. The flush of md_misc_wq in md_alloc() is only meant to
wait for the previous mddev_delayed_delete() to finish.

md_misc_wq also runs sync_work (md_start_sync), and md_start_sync takes
reconfig_mutex. On the other path, md_ioctl() already holds
reconfig_mutex when md_import_device() opens a bdev and takes
major_names_lock.

Flushing md_misc_wq under major_names_lock therefore creates a lockdep
cycle:

  major_names_lock -> md_misc_wq -> reconfig_mutex -> major_names_lock

Move del_work to a dedicated workqueue and flush only that queue from
md_alloc(). Leave sync_work on md_misc_wq.

Reported-by: syzbot+68e1f51046d68329940f@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://syzkaller.appspot.com/bug?extid=68e1f51046d68329940f
Fixes: e804ac780e2f ("md: fix and update workqueue usage")
Signed-off-by: Li Youhong <liyouhong@xxxxxxxxxx>
---
 drivers/md/md.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 680b34a63cb3..87a851466892 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -86,13 +86,16 @@ static const struct kobj_type md_ktype;
 static DECLARE_WAIT_QUEUE_HEAD(resync_wait);
 
 /*
- * This workqueue is used for sync_work to register new sync_thread, and for
- * del_work to remove rdev, and for event_work that is only set by dm-raid.
+ * md_misc_wq runs sync_work (new sync_thread) and event_work (dm-raid).
+ * sync_work grabs reconfig_mutex, so this queue must not be flushed while
+ * holding reconfig_mutex, nor while holding major_names_lock (md_probe /
+ * md_alloc is called from blk_probe_dev with that lock held).
  *
- * Noted that sync_work will grab reconfig_mutex, hence never flush this
- * workqueue whith reconfig_mutex grabbed.
+ * md_del_wq runs only del_work (mddev_delayed_delete). md_alloc() flushes
+ * this queue to wait for a previous instance of the same device to go away.
  */
 static struct workqueue_struct *md_misc_wq;
+static struct workqueue_struct *md_del_wq;
 
 static int remove_and_add_spares(struct mddev *mddev,
 				 struct md_rdev *this);
@@ -651,7 +654,7 @@ static void __mddev_put(struct mddev *mddev)
 	 * Call queue_work inside the spinlock so that flush_workqueue() after
 	 * mddev_find will succeed in waiting for the work to be done.
 	 */
-	queue_work(md_misc_wq, &mddev->del_work);
+	queue_work(md_del_wq, &mddev->del_work);
 }
 
 static void mddev_put_locked(struct mddev *mddev)
@@ -6329,9 +6332,12 @@ struct mddev *md_alloc(dev_t dev, char *name)
 
 	/*
 	 * Wait for any previous instance of this device to be completely
-	 * removed (mddev_delayed_delete).
+	 * removed (mddev_delayed_delete). Do not flush md_misc_wq: it also
+	 * runs sync_work, which takes reconfig_mutex and can take
+	 * major_names_lock via md_import_device(). md_alloc() is called from
+	 * md_probe() while blk_probe_dev() already holds major_names_lock.
 	 */
-	flush_workqueue(md_misc_wq);
+	flush_workqueue(md_del_wq);
 
 	mutex_lock(&disks_mutex);
 	mddev = mddev_alloc(dev);
@@ -10765,6 +10771,10 @@ static int __init md_init(void)
 	if (!md_misc_wq)
 		goto err_misc_wq;
 
+	md_del_wq = alloc_workqueue("md_del", WQ_PERCPU, 0);
+	if (!md_del_wq)
+		goto err_del_wq;
+
 	ret = __register_blkdev(MD_MAJOR, "md", md_probe);
 	if (ret < 0)
 		goto err_md;
@@ -10783,6 +10793,8 @@ static int __init md_init(void)
 err_mdp:
 	unregister_blkdev(MD_MAJOR, "md");
 err_md:
+	destroy_workqueue(md_del_wq);
+err_del_wq:
 	destroy_workqueue(md_misc_wq);
 err_misc_wq:
 	md_llbitmap_exit();
@@ -11093,6 +11105,7 @@ static __exit void md_exit(void)
 	spin_unlock(&all_mddevs_lock);
 
 	destroy_workqueue(md_misc_wq);
+	destroy_workqueue(md_del_wq);
 	md_bitmap_exit();
 }
 
-- 
2.43.0