[PATCH V2 0/2] md: call del_gendisk in sync way

[PATCH V2 0/2] md: call del_gendisk in sync way

[Xiao Ni]

Now del_gendisk is called in a queue work which has a small window
that mdadm --stop command exits but the device node still exists.
It causes trouble in regression tests. This patch set tries to resolve
this problem.

v2: don't remove MD_DELETED

Xiao Ni (2):
  md: Don't clear MD_CLOSING until mddev is freed
  md: call del_gendisk in control path

 drivers/md/md.c | 68 ++++++++++++++++++++++++++++++++++++-------------
 drivers/md/md.h | 16 +++++++++++-
 2 files changed, 65 insertions(+), 19 deletions(-)

-- 
2.32.0 (Apple Git-132)

[PATCH 1/2] md: Don't clear MD_CLOSING until mddev is freed

[Xiao Ni]

UNTIL_STOP is used to avoid mddev is freed on the last close before adding
disks to mddev. And it should be cleared when stopping an array which is
mentioned in commit efeb53c0e572 ("md: Allow md devices to be created by
name."). So reset ->hold_active to 0 in md_clean.

And MD_CLOSING should be kept until mddev is freed to avoid reopen.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 drivers/md/md.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 9daa78c5fe33..9b9950ed6ee9 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -6360,15 +6360,10 @@ static void md_clean(struct mddev *mddev)
 	mddev->persistent = 0;
 	mddev->level = LEVEL_NONE;
 	mddev->clevel[0] = 0;
-	/*
-	 * Don't clear MD_CLOSING, or mddev can be opened again.
-	 * 'hold_active != 0' means mddev is still in the creation
-	 * process and will be used later.
-	 */
-	if (mddev->hold_active)
-		mddev->flags = 0;
-	else
-		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;
@@ -6595,8 +6590,6 @@ static int do_md_stop(struct mddev *mddev, int mode)
 		export_array(mddev);
 
 		md_clean(mddev);
-		if (mddev->hold_active == UNTIL_STOP)
-			mddev->hold_active = 0;
 	}
 	md_new_event();
 	sysfs_notify_dirent_safe(mddev->sysfs_state);
-- 
2.32.0 (Apple Git-132)

[PATCH 2/2] md: call del_gendisk in control path

[Xiao Ni]

Now del_gendisk and put_disk are called asynchronously in workqueue work.
The asynchronous way also has a problem that the device node can still
exist after mdadm --stop command returns in a short window. So udev rule
can open this device node and create the struct mddev in kernel again.

So put del_gendisk in control path and still leave put_disk in
md_kobj_release to avoid uaf.

But there is a window that sysfs can be accessed between mddev_unlock and
del_gendisk. So some actions (add disk, change level, .e.g) can happen
which lead unexpected results. And if we delete MD_DELETED and only use
MD_CLOSING in stop control path, the sysfs files can't be accessed if
do_md_stop stuck when io hange. So we keep MD_DELETED here and set
MD_DELETED before mddev_unlock.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 drivers/md/md.c | 53 ++++++++++++++++++++++++++++++++++++++++++-------
 drivers/md/md.h | 16 ++++++++++++++-
 2 files changed, 61 insertions(+), 8 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 9b9950ed6ee9..a62867f34aa8 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -606,15 +606,13 @@ static inline struct mddev *mddev_get(struct mddev *mddev)
 }
 
 static void mddev_delayed_delete(struct work_struct *ws);
+static bool can_delete_gendisk(struct mddev *mddev);
 
 static void __mddev_put(struct mddev *mddev)
 {
-	if (mddev->raid_disks || !list_empty(&mddev->disks) ||
-	    mddev->ctime || mddev->hold_active)
-		return;
 
-	/* Array is not configured at all, and not held active, so destroy it */
-	set_bit(MD_DELETED, &mddev->flags);
+	if (can_delete_gendisk(mddev) == false)
+		return;
 
 	/*
 	 * Call queue_work inside the spinlock so that flush_workqueue() after
@@ -4400,6 +4398,7 @@ array_state_show(struct mddev *mddev, char *page)
 	return sprintf(page, "%s\n", array_states[st]);
 }
 
+static void delete_gendisk(struct mddev *mddev);
 static int do_md_stop(struct mddev *mddev, int ro);
 static int md_set_readonly(struct mddev *mddev);
 static int restart_array(struct mddev *mddev);
@@ -4533,6 +4532,9 @@ array_state_store(struct mddev *mddev, const char *buf, size_t len)
 	    (err && st == clear))
 		clear_bit(MD_CLOSING, &mddev->flags);
 
+	if ((st == clear || st == inactive) && !err)
+		delete_gendisk(mddev);
+
 	return err ?: len;
 }
 static struct md_sysfs_entry md_array_state =
@@ -5721,19 +5723,30 @@ md_attr_store(struct kobject *kobj, struct attribute *attr,
 	struct md_sysfs_entry *entry = container_of(attr, struct md_sysfs_entry, attr);
 	struct mddev *mddev = container_of(kobj, struct mddev, kobj);
 	ssize_t rv;
+	struct kernfs_node *kn = NULL;
 
 	if (!entry->store)
 		return -EIO;
 	if (!capable(CAP_SYS_ADMIN))
 		return -EACCES;
+
+	if (entry->store == array_state_store && cmd_match(page, "clear"))
+		kn = sysfs_break_active_protection(kobj, attr);
+
 	spin_lock(&all_mddevs_lock);
 	if (!mddev_get(mddev)) {
 		spin_unlock(&all_mddevs_lock);
+		if (kn)
+			sysfs_unbreak_active_protection(kn);
 		return -EBUSY;
 	}
 	spin_unlock(&all_mddevs_lock);
 	rv = entry->store(mddev, page, length);
 	mddev_put(mddev);
+
+	if (kn)
+		sysfs_unbreak_active_protection(kn);
+
 	return rv;
 }
 
@@ -5746,7 +5759,6 @@ static void md_kobj_release(struct kobject *ko)
 	if (mddev->sysfs_level)
 		sysfs_put(mddev->sysfs_level);
 
-	del_gendisk(mddev->gendisk);
 	put_disk(mddev->gendisk);
 }
 
@@ -6526,6 +6538,28 @@ static int md_set_readonly(struct mddev *mddev)
 	return err;
 }
 
+static bool can_delete_gendisk(struct mddev *mddev)
+{
+	if (mddev->raid_disks || !list_empty(&mddev->disks) ||
+	    mddev->ctime || mddev->hold_active)
+		return false;
+
+	return true;
+}
+
+/* Call this function after do_md_stop with mode 0.
+ * And it can't call this function under reconfig_mutex to
+ * avoid deadlock(e.g. call del_gendisk under the lock and
+ * an access to sysfs files waits the lock)
+ */
+static void delete_gendisk(struct mddev *mddev)
+{
+	if (can_delete_gendisk(mddev) == false)
+		return;
+
+	del_gendisk(mddev->gendisk);
+}
+
 /* mode:
  *   0 - completely stop and dis-assemble array
  *   2 - stop but do not disassemble array
@@ -6588,8 +6622,8 @@ static int do_md_stop(struct mddev *mddev, int mode)
 		mddev->bitmap_info.offset = 0;
 
 		export_array(mddev);
-
 		md_clean(mddev);
+		set_bit(MD_DELETED, &mddev->flags);
 	}
 	md_new_event();
 	sysfs_notify_dirent_safe(mddev->sysfs_state);
@@ -6616,6 +6650,7 @@ static void autorun_array(struct mddev *mddev)
 	if (err) {
 		pr_warn("md: do_md_run() returned %d\n", err);
 		do_md_stop(mddev, 0);
+		delete_gendisk(mddev);
 	}
 }
 
@@ -7886,6 +7921,10 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
 out:
 	if (cmd == STOP_ARRAY_RO || (err && cmd == STOP_ARRAY))
 		clear_bit(MD_CLOSING, &mddev->flags);
+
+	if (cmd == STOP_ARRAY && err == 0)
+		delete_gendisk(mddev);
+
 	return err;
 }
 #ifdef CONFIG_COMPAT
diff --git a/drivers/md/md.h b/drivers/md/md.h
index 1cf00a04bcdd..45f1027986e4 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -697,11 +697,25 @@ static inline bool reshape_interrupted(struct mddev *mddev)
 
 static inline int __must_check mddev_lock(struct mddev *mddev)
 {
-	return mutex_lock_interruptible(&mddev->reconfig_mutex);
+	int ret = 0;
+
+	ret = mutex_lock_interruptible(&mddev->reconfig_mutex);
+
+	/* MD_DELETED is set in do_md_stop with reconfig_mutex
+	 * So check it here also.
+	 */
+	if (!ret && test_bit(MD_DELETED, &mddev->flags)) {
+		ret = -EBUSY;
+		mutex_unlock(&mddev->reconfig_mutex);
+	}
+
+	return ret;
 }
 
 /* Sometimes we need to take the lock in a situation where
  * failure due to interrupts is not acceptable.
+ * It doesn't need to check MD_DELETED here, the owner which
+ * holds the lock here can't be stopped.
  */
 static inline void mddev_lock_nointr(struct mddev *mddev)
 {
-- 
2.32.0 (Apple Git-132)

Re: [PATCH 1/2] md: Don't clear MD_CLOSING until mddev is freed

[Yu Kuai]

在 2025/05/15 17:08, Xiao Ni 写道:
> UNTIL_STOP is used to avoid mddev is freed on the last close before adding
> disks to mddev. And it should be cleared when stopping an array which is
> mentioned in commit efeb53c0e572 ("md: Allow md devices to be created by
> name."). So reset ->hold_active to 0 in md_clean.
> 
> And MD_CLOSING should be kept until mddev is freed to avoid reopen.
> 
> Signed-off-by: Xiao Ni <xni@redhat.com>
> ---
>   drivers/md/md.c | 15 ++++-----------
>   1 file changed, 4 insertions(+), 11 deletions(-)
> 

Reviewed-by: Yu Kuai <yukuai3@huawei.com>

BTW, please send me by address yukuai3@huawei.com, or I may miss the
patches.
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 9daa78c5fe33..9b9950ed6ee9 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -6360,15 +6360,10 @@ static void md_clean(struct mddev *mddev)
>   	mddev->persistent = 0;
>   	mddev->level = LEVEL_NONE;
>   	mddev->clevel[0] = 0;
> -	/*
> -	 * Don't clear MD_CLOSING, or mddev can be opened again.
> -	 * 'hold_active != 0' means mddev is still in the creation
> -	 * process and will be used later.
> -	 */
> -	if (mddev->hold_active)
> -		mddev->flags = 0;
> -	else
> -		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;
> @@ -6595,8 +6590,6 @@ static int do_md_stop(struct mddev *mddev, int mode)
>   		export_array(mddev);
>   
>   		md_clean(mddev);
> -		if (mddev->hold_active == UNTIL_STOP)
> -			mddev->hold_active = 0;
>   	}
>   	md_new_event();
>   	sysfs_notify_dirent_safe(mddev->sysfs_state);
> 

Re: [PATCH 2/2] md: call del_gendisk in control path

[Yu Kuai]

Hi,

在 2025/05/15 17:08, Xiao Ni 写道:
> Now del_gendisk and put_disk are called asynchronously in workqueue work.
> The asynchronous way also has a problem that the device node can still
> exist after mdadm --stop command returns in a short window. So udev rule
> can open this device node and create the struct mddev in kernel again.
> 
> So put del_gendisk in control path and still leave put_disk in
> md_kobj_release to avoid uaf.
> 
> But there is a window that sysfs can be accessed between mddev_unlock and
> del_gendisk. So some actions (add disk, change level, .e.g) can happen
> which lead unexpected results. And if we delete MD_DELETED and only use
> MD_CLOSING in stop control path, the sysfs files can't be accessed if
> do_md_stop stuck when io hange. So we keep MD_DELETED here and set
> MD_DELETED before mddev_unlock.
> 
> Signed-off-by: Xiao Ni <xni@redhat.com>
> ---
>   drivers/md/md.c | 53 ++++++++++++++++++++++++++++++++++++++++++-------
>   drivers/md/md.h | 16 ++++++++++++++-
>   2 files changed, 61 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 9b9950ed6ee9..a62867f34aa8 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -606,15 +606,13 @@ static inline struct mddev *mddev_get(struct mddev *mddev)
>   }
>   
>   static void mddev_delayed_delete(struct work_struct *ws);
> +static bool can_delete_gendisk(struct mddev *mddev);

Plese define new helper here directly.
>   
>   static void __mddev_put(struct mddev *mddev)
>   {
> -	if (mddev->raid_disks || !list_empty(&mddev->disks) ||
> -	    mddev->ctime || mddev->hold_active)
> -		return;
>   
> -	/* Array is not configured at all, and not held active, so destroy it */
> -	set_bit(MD_DELETED, &mddev->flags);
> +	if (can_delete_gendisk(mddev) == false)
> +		return;
>   
>   	/*
>   	 * Call queue_work inside the spinlock so that flush_workqueue() after
> @@ -4400,6 +4398,7 @@ array_state_show(struct mddev *mddev, char *page)
>   	return sprintf(page, "%s\n", array_states[st]);
>   }
>   
> +static void delete_gendisk(struct mddev *mddev);

This helper looks unnecessary, it doesn't make code cleaner.

>   static int do_md_stop(struct mddev *mddev, int ro);
>   static int md_set_readonly(struct mddev *mddev);
>   static int restart_array(struct mddev *mddev);
> @@ -4533,6 +4532,9 @@ array_state_store(struct mddev *mddev, const char *buf, size_t len)
>   	    (err && st == clear))
>   		clear_bit(MD_CLOSING, &mddev->flags);
>   
> +	if ((st == clear || st == inactive) && !err)
> +		delete_gendisk(mddev);

I think it's better to do this in mddev_unlock() if MD_DELETED is set,
like what we did for rdev. Meanwhile, mddev->to_remove and all related
hacks can be removed now.
> +
>   	return err ?: len;
>   }
>   static struct md_sysfs_entry md_array_state =
> @@ -5721,19 +5723,30 @@ md_attr_store(struct kobject *kobj, struct attribute *attr,
>   	struct md_sysfs_entry *entry = container_of(attr, struct md_sysfs_entry, attr);
>   	struct mddev *mddev = container_of(kobj, struct mddev, kobj);
>   	ssize_t rv;
> +	struct kernfs_node *kn = NULL;
>   
>   	if (!entry->store)
>   		return -EIO;
>   	if (!capable(CAP_SYS_ADMIN))
>   		return -EACCES;
> +
> +	if (entry->store == array_state_store && cmd_match(page, "clear"))
> +		kn = sysfs_break_active_protection(kobj, attr);
> +
>   	spin_lock(&all_mddevs_lock);
>   	if (!mddev_get(mddev)) {
>   		spin_unlock(&all_mddevs_lock);
> +		if (kn)
> +			sysfs_unbreak_active_protection(kn);
>   		return -EBUSY;
>   	}
>   	spin_unlock(&all_mddevs_lock);
>   	rv = entry->store(mddev, page, length);
>   	mddev_put(mddev);
> +
> +	if (kn)
> +		sysfs_unbreak_active_protection(kn);
> +
>   	return rv;
>   }
>   
> @@ -5746,7 +5759,6 @@ static void md_kobj_release(struct kobject *ko)
>   	if (mddev->sysfs_level)
>   		sysfs_put(mddev->sysfs_level);
>   
> -	del_gendisk(mddev->gendisk);
>   	put_disk(mddev->gendisk);
>   }
>   
> @@ -6526,6 +6538,28 @@ static int md_set_readonly(struct mddev *mddev)
>   	return err;
>   }
>   
> +static bool can_delete_gendisk(struct mddev *mddev)
> +{
> +	if (mddev->raid_disks || !list_empty(&mddev->disks) ||
> +	    mddev->ctime || mddev->hold_active)
> +		return false;
> +
> +	return true;
> +}
> +
> +/* Call this function after do_md_stop with mode 0.
> + * And it can't call this function under reconfig_mutex to
> + * avoid deadlock(e.g. call del_gendisk under the lock and
> + * an access to sysfs files waits the lock)
> + */
> +static void delete_gendisk(struct mddev *mddev)
> +{
> +	if (can_delete_gendisk(mddev) == false)
> +		return;
> +
> +	del_gendisk(mddev->gendisk);
> +}
> +
>   /* mode:
>    *   0 - completely stop and dis-assemble array
>    *   2 - stop but do not disassemble array
> @@ -6588,8 +6622,8 @@ static int do_md_stop(struct mddev *mddev, int mode)
>   		mddev->bitmap_info.offset = 0;
>   
>   		export_array(mddev);
> -
>   		md_clean(mddev);
> +		set_bit(MD_DELETED, &mddev->flags);
>   	}
>   	md_new_event();
>   	sysfs_notify_dirent_safe(mddev->sysfs_state);
> @@ -6616,6 +6650,7 @@ static void autorun_array(struct mddev *mddev)
>   	if (err) {
>   		pr_warn("md: do_md_run() returned %d\n", err);
>   		do_md_stop(mddev, 0);
> +		delete_gendisk(mddev);
>   	}
>   }
>   
> @@ -7886,6 +7921,10 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
>   out:
>   	if (cmd == STOP_ARRAY_RO || (err && cmd == STOP_ARRAY))
>   		clear_bit(MD_CLOSING, &mddev->flags);
> +
> +	if (cmd == STOP_ARRAY && err == 0)
> +		delete_gendisk(mddev);
> +
>   	return err;
>   }
>   #ifdef CONFIG_COMPAT
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index 1cf00a04bcdd..45f1027986e4 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -697,11 +697,25 @@ static inline bool reshape_interrupted(struct mddev *mddev)
>   
>   static inline int __must_check mddev_lock(struct mddev *mddev)
>   {
> -	return mutex_lock_interruptible(&mddev->reconfig_mutex);
> +	int ret = 0;
> +
> +	ret = mutex_lock_interruptible(&mddev->reconfig_mutex);
> +
> +	/* MD_DELETED is set in do_md_stop with reconfig_mutex
> +	 * So check it here also.
> +	 */
> +	if (!ret && test_bit(MD_DELETED, &mddev->flags)) {
> +		ret = -EBUSY;
> +		mutex_unlock(&mddev->reconfig_mutex);
> +	}
> +
> +	return ret;
>   }

There are also other helpers like mddev_lock_nointr,
mddev_suspend_and_lock_nointr, mddev_trylock.

Thanks,
Kuai

>   
>   /* Sometimes we need to take the lock in a situation where
>    * failure due to interrupts is not acceptable.
> + * It doesn't need to check MD_DELETED here, the owner which
> + * holds the lock here can't be stopped.
>    */
>   static inline void mddev_lock_nointr(struct mddev *mddev)
>   {
> 

Re: [PATCH 1/2] md: Don't clear MD_CLOSING until mddev is freed

[Yu Kuai]

在 2025/05/15 17:08, Xiao Ni 写道:
> UNTIL_STOP is used to avoid mddev is freed on the last close before adding
> disks to mddev. And it should be cleared when stopping an array which is
> mentioned in commit efeb53c0e572 ("md: Allow md devices to be created by
> name."). So reset ->hold_active to 0 in md_clean.
> 
> And MD_CLOSING should be kept until mddev is freed to avoid reopen.
> 
> Signed-off-by: Xiao Ni <xni@redhat.com>
> ---
>   drivers/md/md.c | 15 ++++-----------
>   1 file changed, 4 insertions(+), 11 deletions(-)
> 

Patch 1 applied to md-6.16

BTW, please send a new version for patch 2, we might consider it for
the next merge window.

Thanks,
Kuai

> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 9daa78c5fe33..9b9950ed6ee9 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -6360,15 +6360,10 @@ static void md_clean(struct mddev *mddev)
>   	mddev->persistent = 0;
>   	mddev->level = LEVEL_NONE;
>   	mddev->clevel[0] = 0;
> -	/*
> -	 * Don't clear MD_CLOSING, or mddev can be opened again.
> -	 * 'hold_active != 0' means mddev is still in the creation
> -	 * process and will be used later.
> -	 */
> -	if (mddev->hold_active)
> -		mddev->flags = 0;
> -	else
> -		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;
> @@ -6595,8 +6590,6 @@ static int do_md_stop(struct mddev *mddev, int mode)
>   		export_array(mddev);
>   
>   		md_clean(mddev);
> -		if (mddev->hold_active == UNTIL_STOP)
> -			mddev->hold_active = 0;
>   	}
>   	md_new_event();
>   	sysfs_notify_dirent_safe(mddev->sysfs_state);
> 

Re: [PATCH 1/2] md: Don't clear MD_CLOSING until mddev is freed

[Xiao Ni]

On Fri, May 30, 2025 at 2:48 PM Yu Kuai <yukuai1@huaweicloud.com> wrote:
>
> 在 2025/05/15 17:08, Xiao Ni 写道:
> > UNTIL_STOP is used to avoid mddev is freed on the last close before adding
> > disks to mddev. And it should be cleared when stopping an array which is
> > mentioned in commit efeb53c0e572 ("md: Allow md devices to be created by
> > name."). So reset ->hold_active to 0 in md_clean.
> >
> > And MD_CLOSING should be kept until mddev is freed to avoid reopen.
> >
> > Signed-off-by: Xiao Ni <xni@redhat.com>
> > ---
> >   drivers/md/md.c | 15 ++++-----------
> >   1 file changed, 4 insertions(+), 11 deletions(-)
> >
>
> Patch 1 applied to md-6.16
>
> BTW, please send a new version for patch 2, we might consider it for
> the next merge window.

Thanks. I'm preparing patches for other changes.

Regards
Xiao
>
> Thanks,
> Kuai
>
> > diff --git a/drivers/md/md.c b/drivers/md/md.c
> > index 9daa78c5fe33..9b9950ed6ee9 100644
> > --- a/drivers/md/md.c
> > +++ b/drivers/md/md.c
> > @@ -6360,15 +6360,10 @@ static void md_clean(struct mddev *mddev)
> >       mddev->persistent = 0;
> >       mddev->level = LEVEL_NONE;
> >       mddev->clevel[0] = 0;
> > -     /*
> > -      * Don't clear MD_CLOSING, or mddev can be opened again.
> > -      * 'hold_active != 0' means mddev is still in the creation
> > -      * process and will be used later.
> > -      */
> > -     if (mddev->hold_active)
> > -             mddev->flags = 0;
> > -     else
> > -             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;
> > @@ -6595,8 +6590,6 @@ static int do_md_stop(struct mddev *mddev, int mode)
> >               export_array(mddev);
> >
> >               md_clean(mddev);
> > -             if (mddev->hold_active == UNTIL_STOP)
> > -                     mddev->hold_active = 0;
> >       }
> >       md_new_event();
> >       sysfs_notify_dirent_safe(mddev->sysfs_state);
> >
>

Re: [PATCH 1/2] md: Don't clear MD_CLOSING until mddev is freed

[Xiao Ni]

On Fri, May 30, 2025 at 2:48 PM Yu Kuai <yukuai1@huaweicloud.com> wrote:
>
> 在 2025/05/15 17:08, Xiao Ni 写道:
> > UNTIL_STOP is used to avoid mddev is freed on the last close before adding
> > disks to mddev. And it should be cleared when stopping an array which is
> > mentioned in commit efeb53c0e572 ("md: Allow md devices to be created by
> > name."). So reset ->hold_active to 0 in md_clean.
> >
> > And MD_CLOSING should be kept until mddev is freed to avoid reopen.
> >
> > Signed-off-by: Xiao Ni <xni@redhat.com>
> > ---
> >   drivers/md/md.c | 15 ++++-----------
> >   1 file changed, 4 insertions(+), 11 deletions(-)
> >
>
> Patch 1 applied to md-6.16
>
> BTW, please send a new version for patch 2, we might consider it for
> the next merge window.

Hi Kuai

The first patch isn't used to resolve the problem that /dev/md0 can't
be removed. So it's not useful to merge itself. I'll send all patches
in v2, so please remove it from this PR.

Regards
Xiao
>
> Thanks,
> Kuai
>
> > diff --git a/drivers/md/md.c b/drivers/md/md.c
> > index 9daa78c5fe33..9b9950ed6ee9 100644
> > --- a/drivers/md/md.c
> > +++ b/drivers/md/md.c
> > @@ -6360,15 +6360,10 @@ static void md_clean(struct mddev *mddev)
> >       mddev->persistent = 0;
> >       mddev->level = LEVEL_NONE;
> >       mddev->clevel[0] = 0;
> > -     /*
> > -      * Don't clear MD_CLOSING, or mddev can be opened again.
> > -      * 'hold_active != 0' means mddev is still in the creation
> > -      * process and will be used later.
> > -      */
> > -     if (mddev->hold_active)
> > -             mddev->flags = 0;
> > -     else
> > -             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;
> > @@ -6595,8 +6590,6 @@ static int do_md_stop(struct mddev *mddev, int mode)
> >               export_array(mddev);
> >
> >               md_clean(mddev);
> > -             if (mddev->hold_active == UNTIL_STOP)
> > -                     mddev->hold_active = 0;
> >       }
> >       md_new_event();
> >       sysfs_notify_dirent_safe(mddev->sysfs_state);
> >
>

Re: [PATCH 1/2] md: Don't clear MD_CLOSING until mddev is freed

[Yu Kuai]

Hi,

在 2025/05/30 15:57, Xiao Ni 写道:
> The first patch isn't used to resolve the problem that /dev/md0 can't
> be removed. So it's not useful to merge itself. I'll send all patches
> in v2, so please remove it from this PR.

Now dropped.

Thanks,
Kuai