Thread (2 messages) flat view 2 messages, 2 authors, 1d ago
WARM1d

[PATCH v2] md: remove legacy async del_gendisk

From: Li Youhong <hidden>
Date: 2026-09-22 02:16:11
Also in: lkml
Subsystem: software raid (multiple disks) support, the rest · Maintainers: Song Liu, Yu Kuai, Linus Torvalds

From: Li Youhong <redacted>

md_alloc() is called from md_probe() while blk_probe_dev() still holds
major_names_lock. It flushes md_misc_wq only to wait for the previous
mddev_delayed_delete() to finish.

md_misc_wq also runs sync_work (md_start_sync), which 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

legacy_async_del_gendisk has been in tree long enough that the async
path can be removed. del_gendisk is done synchronously from
mddev_unlock() after dropping reconfig_mutex, and the last kobject_put()
runs from mddev_put() after dropping all_mddevs_lock. del_work and the
flush in md_alloc() are then unnecessary.

mdadm 4.5+ is required.

Reported-by: syzbot+68e1f51046d68329940f@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=68e1f51046d68329940f
Fixes: e804ac780e2f ("md: fix and update workqueue usage")
Suggested-by: Yu Kuai <redacted>
Signed-off-by: Li Youhong <redacted>
---
v2:
- Drop legacy_async_del_gendisk, del_work, mddev_delayed_delete(),
  and the flush in md_alloc().
- del_gendisk is synchronous: mddev_unlock() after dropping reconfig_mutex,
  and the last kobject_put() from mddev_put() after dropping all_mddevs_lock.
- __mddev_put() returns whether the caller must complete_delete after
  dropping all_mddevs_lock. md_seq_show() stashes last-put mddev in
  seq->private and complete_delete after the iterator has moved on.
  md_notify_reboot()/md_exit() walk with list_for_each_entry_safe().
- v1: link: https://lore.kernel.org/linux-raid/20260915083354.1603416-1-dayou5941@163.com/ (local) 

---
 drivers/md/md.c | 175 +++++++++++++++++++++++++-----------------------
 drivers/md/md.h |   2 -
 2 files changed, 91 insertions(+), 86 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 680b34a63cb3..5d9e76f3256b 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -87,7 +87,7 @@ 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.
+ * event_work that is only set by dm-raid.
  *
  * Noted that sync_work will grab reconfig_mutex, hence never flush this
  * workqueue whith reconfig_mutex grabbed.
@@ -338,7 +338,6 @@ static int start_readonly;
  * so all the races disappear.
  */
 static bool create_on_open = true;
-static bool legacy_async_del_gendisk = true;
 static bool check_new_feature = true;
 
 /*
@@ -633,40 +632,62 @@ static inline struct mddev *mddev_get(struct mddev *mddev)
 	return mddev;
 }
 
-static void mddev_delayed_delete(struct work_struct *ws);
+static void mddev_delete_gendisk(struct mddev *mddev)
+{
+	/*
+	 * Call del_gendisk after releasing reconfig_mutex to avoid
+	 * deadlock (e.g. del_gendisk under the lock while a sysfs
+	 * access waits for the lock).
+	 * MD_DELETED is only used for md raid, set in do_md_stop().
+	 * dm-raid uses md_stop and does not need MD_DELETED.
+	 */
+	if (!test_bit(MD_DELETED, &mddev->flags) ||
+	    test_and_set_bit(MD_DO_DELETE, &mddev->flags))
+		return;
 
-static void __mddev_put(struct mddev *mddev)
+	kobject_del(&mddev->kobj);
+	del_gendisk(mddev->gendisk);
+}
+
+static void mddev_complete_delete(struct mddev *mddev)
+{
+	mddev_delete_gendisk(mddev);
+	kobject_put(&mddev->kobj);
+}
+
+static bool __mddev_put(struct mddev *mddev)
 {
 	if (mddev->raid_disks || !list_empty(&mddev->disks) ||
 	    mddev->ctime || mddev->hold_active)
-		return;
+		return false;
 
 	/*
 	 * If array is freed by stopping array, MD_DELETED is set by
-	 * do_md_stop(), MD_DELETED is still set here in case mddev is freed
+	 * do_md_stop(). Still set it here in case mddev is freed
 	 * directly by closing a mddev that is created by create_on_open.
 	 */
 	set_bit(MD_DELETED, &mddev->flags);
-	/*
-	 * 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);
+	return true;
 }
 
-static void mddev_put_locked(struct mddev *mddev)
+static bool mddev_put_locked(struct mddev *mddev)
 {
 	if (atomic_dec_and_test(&mddev->active))
-		__mddev_put(mddev);
+		return __mddev_put(mddev);
+	return false;
 }
 
 void mddev_put(struct mddev *mddev)
 {
+	bool complete_delete;
+
 	if (!atomic_dec_and_lock(&mddev->active, &all_mddevs_lock))
 		return;
 
-	__mddev_put(mddev);
+	complete_delete = __mddev_put(mddev);
 	spin_unlock(&all_mddevs_lock);
+	if (complete_delete)
+		mddev_complete_delete(mddev);
 }
 
 static void md_safemode_timeout(struct timer_list *t);
@@ -794,7 +815,6 @@ int mddev_init(struct mddev *mddev)
 	mddev->level = LEVEL_NONE;
 
 	INIT_WORK(&mddev->sync_work, md_start_sync);
-	INIT_WORK(&mddev->del_work, mddev_delayed_delete);
 
 	return 0;
 
@@ -969,21 +989,7 @@ void mddev_unlock(struct mddev *mddev)
 		export_rdev(rdev);
 	}
 
-	if (!legacy_async_del_gendisk) {
-		/*
-		 * Call del_gendisk after release reconfig_mutex to avoid
-		 * deadlock (e.g. call del_gendisk under the lock and an
-		 * access to sysfs files waits the lock)
-		 * And MD_DELETED is only used for md raid which is set in
-		 * do_md_stop. dm raid only uses md_stop to stop. So dm raid
-		 * doesn't need to check MD_DELETED when getting reconfig lock
-		 */
-		if (test_bit(MD_DELETED, &mddev->flags) &&
-		    !test_and_set_bit(MD_DO_DELETE, &mddev->flags)) {
-			kobject_del(&mddev->kobj);
-			del_gendisk(mddev->gendisk);
-		}
-	}
+	mddev_delete_gendisk(mddev);
 }
 EXPORT_SYMBOL_GPL(mddev_unlock);
 
@@ -6174,13 +6180,10 @@ static void md_kobj_release(struct kobject *ko)
 {
 	struct mddev *mddev = container_of(ko, struct mddev, kobj);
 
-	if (legacy_async_del_gendisk) {
-		if (mddev->sysfs_state)
-			sysfs_put(mddev->sysfs_state);
-		if (mddev->sysfs_level)
-			sysfs_put(mddev->sysfs_level);
-		del_gendisk(mddev->gendisk);
-	}
+	if (mddev->sysfs_state)
+		sysfs_put(mddev->sysfs_state);
+	if (mddev->sysfs_level)
+		sysfs_put(mddev->sysfs_level);
 	put_disk(mddev->gendisk);
 }
 
@@ -6293,13 +6296,6 @@ void mddev_update_io_opt(struct mddev *mddev, unsigned int nr_stripes)
 }
 EXPORT_SYMBOL_GPL(mddev_update_io_opt);
 
-static void mddev_delayed_delete(struct work_struct *ws)
-{
-	struct mddev *mddev = container_of(ws, struct mddev, del_work);
-
-	kobject_put(&mddev->kobj);
-}
-
 void md_init_stacking_limits(struct queue_limits *lim)
 {
 	blk_set_stacking_limits(lim);
@@ -6327,12 +6323,6 @@ struct mddev *md_alloc(dev_t dev, char *name)
 	int unit;
 	int error;
 
-	/*
-	 * Wait for any previous instance of this device to be completely
-	 * removed (mddev_delayed_delete).
-	 */
-	flush_workqueue(md_misc_wq);
-
 	mutex_lock(&disks_mutex);
 	mddev = mddev_alloc(dev);
 	if (IS_ERR(mddev)) {
@@ -6422,9 +6412,6 @@ static int md_alloc_and_put(dev_t dev, char *name)
 {
 	struct mddev *mddev = md_alloc(dev, name);
 
-	if (legacy_async_del_gendisk)
-		pr_warn("md: async del_gendisk mode will be removed in future, please upgrade to mdadm-4.5+\n");
-
 	if (IS_ERR(mddev))
 		return PTR_ERR(mddev);
 	mddev_put(mddev);
@@ -6979,21 +6966,10 @@ static void md_clean(struct mddev *mddev)
 	mddev->level = LEVEL_NONE;
 	mddev->clevel[0] = 0;
 
-	/*
-	 * For legacy_async_del_gendisk mode, it can stop the array in the
-	 * middle of assembling it, then it still can access the array. So
-	 * it needs to clear MD_CLOSING. If not legacy_async_del_gendisk,
-	 * it can't open the array again after stopping it. So it doesn't
-	 * clear MD_CLOSING.
-	 */
-	if (legacy_async_del_gendisk && mddev->hold_active) {
-		clear_bit(MD_CLOSING, &mddev->flags);
-	} else {
-		/* if UNTIL_STOP is set, it's cleared here */
-		mddev->hold_active = 0;
-		/* Don't clear MD_CLOSING, or mddev can be opened again. */
-		mddev->flags &= BIT_ULL_MASK(MD_CLOSING);
-	}
+	/* if UNTIL_STOP is set, it's cleared here */
+	mddev->hold_active = 0;
+	/* Don't clear MD_CLOSING, or mddev can be opened again. */
+	mddev->flags &= BIT_ULL_MASK(MD_CLOSING);
 	mddev->sb_flags = 0;
 	mddev->ro = MD_RDWR;
 	mddev->metadata_type[0] = 0;
@@ -7222,8 +7198,7 @@ static int do_md_stop(struct mddev *mddev, int mode)
 
 		export_array(mddev);
 		md_clean(mddev);
-		if (!legacy_async_del_gendisk)
-			set_bit(MD_DELETED, &mddev->flags);
+		set_bit(MD_DELETED, &mddev->flags);
 	}
 	md_new_event();
 	sysfs_notify_dirent_safe(mddev->sysfs_state);
@@ -8977,10 +8952,31 @@ static int status_resync(struct seq_file *seq, struct mddev *mddev)
 	return 1;
 }
 
+/*
+ * seq_file walks all_mddevs with the lock held across next().  Last put
+ * cannot complete_delete in show() or the current list node is freed
+ * before next().  Stash it in seq->private and delete after the iterator
+ * has moved on: the next show() pins the new current first, stop()
+ * handles the last one.
+ */
+static void md_seq_flush_delete(struct seq_file *seq)
+{
+	struct mddev *mddev = seq->private;
+
+	if (!mddev)
+		return;
+
+	seq->private = NULL;
+	spin_unlock(&all_mddevs_lock);
+	mddev_complete_delete(mddev);
+	spin_lock(&all_mddevs_lock);
+}
+
 static void *md_seq_start(struct seq_file *seq, loff_t *pos)
 	__acquires(&all_mddevs_lock)
 {
 	seq->poll_event = atomic_read(&md_event_count);
+	seq->private = NULL;
 	spin_lock(&all_mddevs_lock);
 
 	return seq_list_start_head(&all_mddevs, *pos);
@@ -8994,7 +8990,12 @@ static void *md_seq_next(struct seq_file *seq, void *v, loff_t *pos)
 static void md_seq_stop(struct seq_file *seq, void *v)
 	__releases(&all_mddevs_lock)
 {
+	struct mddev *mddev = seq->private;
+
+	seq->private = NULL;
 	spin_unlock(&all_mddevs_lock);
+	if (mddev)
+		mddev_complete_delete(mddev);
 }
 
 static void md_bitmap_status(struct seq_file *seq, struct mddev *mddev)
@@ -9041,8 +9042,13 @@ static int md_seq_show(struct seq_file *seq, void *v)
 	}
 
 	mddev = list_entry(v, struct mddev, all_mddevs);
-	if (!mddev_get(mddev))
+	if (!mddev_get(mddev)) {
+		md_seq_flush_delete(seq);
 		return 0;
+	}
+
+	/* Previous node is no longer the iterator; delete it if we last-put it. */
+	md_seq_flush_delete(seq);
 
 	spin_unlock(&all_mddevs_lock);
 
@@ -9130,7 +9136,8 @@ static int md_seq_show(struct seq_file *seq, void *v)
 	if (mddev == list_last_entry(&all_mddevs, struct mddev, all_mddevs))
 		status_unused(seq);
 
-	mddev_put_locked(mddev);
+	if (mddev_put_locked(mddev))
+		seq->private = mddev;
 	return 0;
 }
 
@@ -10714,10 +10721,11 @@ EXPORT_SYMBOL_GPL(rdev_clear_badblocks);
 static int md_notify_reboot(struct notifier_block *this,
 			    unsigned long code, void *x)
 {
-	struct mddev *mddev;
+	struct mddev *mddev, *tmp;
+	bool complete_delete;
 
 	spin_lock(&all_mddevs_lock);
-	list_for_each_entry(mddev, &all_mddevs, all_mddevs) {
+	list_for_each_entry_safe(mddev, tmp, &all_mddevs, all_mddevs) {
 		if (!mddev_get(mddev))
 			continue;
 		spin_unlock(&all_mddevs_lock);
@@ -10729,7 +10737,12 @@ static int md_notify_reboot(struct notifier_block *this,
 			mddev_unlock(mddev);
 		}
 		spin_lock(&all_mddevs_lock);
-		mddev_put_locked(mddev);
+		complete_delete = mddev_put_locked(mddev);
+		if (complete_delete) {
+			spin_unlock(&all_mddevs_lock);
+			mddev_complete_delete(mddev);
+			spin_lock(&all_mddevs_lock);
+		}
 	}
 	spin_unlock(&all_mddevs_lock);
 
@@ -11054,7 +11067,7 @@ void md_autostart_arrays(int part)
 
 static __exit void md_exit(void)
 {
-	struct mddev *mddev;
+	struct mddev *mddev, *tmp;
 	int delay = 1;
 
 	unregister_blkdev(MD_MAJOR,"md");
@@ -11075,20 +11088,15 @@ static __exit void md_exit(void)
 	remove_proc_entry("mdstat", NULL);
 
 	spin_lock(&all_mddevs_lock);
-	list_for_each_entry(mddev, &all_mddevs, all_mddevs) {
+	list_for_each_entry_safe(mddev, tmp, &all_mddevs, all_mddevs) {
 		if (!mddev_get(mddev))
 			continue;
 		spin_unlock(&all_mddevs_lock);
 		export_array(mddev);
 		mddev->ctime = 0;
 		mddev->hold_active = 0;
-		/*
-		 * As the mddev is now fully clear, mddev_put will schedule
-		 * the mddev for destruction by a workqueue, and the
-		 * destroy_workqueue() below will wait for that to complete.
-		 */
+		mddev_put(mddev);
 		spin_lock(&all_mddevs_lock);
-		mddev_put_locked(mddev);
 	}
 	spin_unlock(&all_mddevs_lock);
 
@@ -11112,7 +11120,6 @@ module_param_call(start_ro, set_ro, get_ro, NULL, S_IRUSR|S_IWUSR);
 module_param(start_dirty_degraded, int, S_IRUGO|S_IWUSR);
 module_param_call(new_array, add_named_array, NULL, NULL, S_IWUSR);
 module_param(create_on_open, bool, S_IRUSR|S_IWUSR);
-module_param(legacy_async_del_gendisk, bool, 0600);
 module_param(check_new_feature, bool, 0600);
 
 MODULE_LICENSE("GPL");
diff --git a/drivers/md/md.h b/drivers/md/md.h
index b6d2e8929a0f..16a236bd37ce 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -549,8 +549,6 @@ struct mddev {
 	struct kernfs_node		*sysfs_degraded;	/*handle for 'degraded' */
 	struct kernfs_node		*sysfs_level;		/*handle for 'level' */
 
-	/* used for delayed sysfs removal */
-	struct work_struct del_work;
 	/* used for register new sync thread */
 	struct work_struct sync_work;
 
-- 
2.25.1
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help