* [PATCH 2/3] md: Don't clear MD_CLOSING until mddev is freed
2025-06-03 5:20 [PATCH V3 0/2] md: call del_gendisk in sync way Xiao Ni
@ 2025-06-03 5:20 ` Xiao Ni
0 siblings, 0 replies; 11+ messages in thread
From: Xiao Ni @ 2025-06-03 5:20 UTC (permalink / raw)
To: linux-raid; +Cc: yukuai3, song, ncroxon
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 | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index fd6c5ff0e3c1..0913b8236471 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -6377,15 +6377,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;
@@ -6612,9 +6607,6 @@ static int do_md_stop(struct mddev *mddev, int mode)
export_array(mddev);
md_clean(mddev);
set_bit(MD_DELETED, &mddev->flags);
-
- 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)
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH V4 0/3] md: call del_gendisk in sync way
@ 2025-06-04 9:07 Xiao Ni
2025-06-04 9:07 ` [PATCH 1/3] md: call del_gendisk in control path Xiao Ni
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Xiao Ni @ 2025-06-04 9:07 UTC (permalink / raw)
To: linux-raid; +Cc: yukuai3, ncroxon, song, yukuai1
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.
v1: replace MD_DELETED with MD_CLOSING
v2: keep MD_CLOSING
v3: call den_gendisk in mddev_unlock, and remove ->to_remove in stop path
and adjust the order of patches
v4: only remove the codes in stop path.
Xiao Ni (3):
md: call del_gendisk in control path
md: Don't clear MD_CLOSING until mddev is freed
md: remove/add redundancy group only in level change
drivers/md/md.c | 44 ++++++++++++++++++++++++++------------------
drivers/md/md.h | 26 ++++++++++++++++++++++++--
2 files changed, 50 insertions(+), 20 deletions(-)
--
2.32.0 (Apple Git-132)
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] md: call del_gendisk in control path
2025-06-04 9:07 [PATCH V4 0/3] md: call del_gendisk in sync way Xiao Ni
@ 2025-06-04 9:07 ` Xiao Ni
2025-06-06 8:46 ` Yu Kuai
2025-06-04 9:07 ` [PATCH 2/3] md: Don't clear MD_CLOSING until mddev is freed Xiao Ni
2025-06-04 9:07 ` [PATCH 3/3] md: remove/add redundancy group only in level change Xiao Ni
2 siblings, 1 reply; 11+ messages in thread
From: Xiao Ni @ 2025-06-04 9:07 UTC (permalink / raw)
To: linux-raid; +Cc: yukuai3, ncroxon, song, yukuai1
Now del_gendisk and put_disk are called asynchronously in workqueue work.
The asynchronous way 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 of gendisk.
Function del_gendisk can't be called with reconfig_mutex. If it's called
with reconfig mutex, a deadlock can happen. del_gendisk waits all sysfs
files access to finish and sysfs file access waits reconfig mutex. So
put del_gendisk after releasing reconfig mutex.
But there is still 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. MD_DELETED is used to resolve this problem.
MD_DELETED is set before releasing reconfig mutex and it should be checked
for these sysfs access which need reconfig mutex. For sysfs access which
don't need reconfig mutex, del_gendisk will wait them to finish.
But it doesn't need to do this in function mddev_lock_nointr. There are
ten places that call it.
* Five of them are in dm raid which we don't need to care. MD_DELETED is
only used for md raid.
* stop_sync_thread, md_do_sync and md_start_sync are related sync request,
and it needs to wait sync thread to finish before stopping an array.
* md_ioctl: md_open is called before md_ioctl, so ->openers is added. It
will fail to stop the array. So it doesn't need to check MD_DELETED here
* md_set_readonly:
It needs to call mddev_set_closing_and_sync_blockdev when setting readonly
or read_auto. So it will fail to stop the array too because MD_CLOSING is
already set.
Signed-off-by: Xiao Ni <xni@redhat.com>
---
drivers/md/md.c | 28 +++++++++++++++++++++++-----
drivers/md/md.h | 26 ++++++++++++++++++++++++--
2 files changed, 47 insertions(+), 7 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 0fde115e921f..e58bb80bf2e9 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -636,9 +636,6 @@ static void __mddev_put(struct mddev *mddev)
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);
-
/*
* Call queue_work inside the spinlock so that flush_workqueue() after
* mddev_find will succeed in waiting for the work to be done.
@@ -873,6 +870,16 @@ void mddev_unlock(struct mddev *mddev)
kobject_del(&rdev->kobj);
export_rdev(rdev, mddev);
}
+
+ /* 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))
+ del_gendisk(mddev->gendisk);
}
EXPORT_SYMBOL_GPL(mddev_unlock);
@@ -5774,19 +5781,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;
}
@@ -5799,7 +5817,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);
}
@@ -6646,8 +6663,9 @@ 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);
+
if (mddev->hold_active == UNTIL_STOP)
mddev->hold_active = 0;
}
diff --git a/drivers/md/md.h b/drivers/md/md.h
index d45a9e6ead80..de47b2500ef0 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -700,11 +700,26 @@ 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.
+ */
+ 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. And all paths can't
+ * call this function after do_md_stop.
*/
static inline void mddev_lock_nointr(struct mddev *mddev)
{
@@ -713,7 +728,14 @@ static inline void mddev_lock_nointr(struct mddev *mddev)
static inline int mddev_trylock(struct mddev *mddev)
{
- return mutex_trylock(&mddev->reconfig_mutex);
+ int ret = 0;
+
+ ret = mutex_trylock(&mddev->reconfig_mutex);
+ if (!ret && test_bit(MD_DELETED, &mddev->flags)) {
+ ret = -EBUSY;
+ mutex_unlock(&mddev->reconfig_mutex);
+ }
+ return ret;
}
extern void mddev_unlock(struct mddev *mddev);
--
2.32.0 (Apple Git-132)
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/3] md: Don't clear MD_CLOSING until mddev is freed
2025-06-04 9:07 [PATCH V4 0/3] md: call del_gendisk in sync way Xiao Ni
2025-06-04 9:07 ` [PATCH 1/3] md: call del_gendisk in control path Xiao Ni
@ 2025-06-04 9:07 ` Xiao Ni
2025-06-04 9:07 ` [PATCH 3/3] md: remove/add redundancy group only in level change Xiao Ni
2 siblings, 0 replies; 11+ messages in thread
From: Xiao Ni @ 2025-06-04 9:07 UTC (permalink / raw)
To: linux-raid; +Cc: yukuai3, ncroxon, song, yukuai1
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 | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index e58bb80bf2e9..8824ce64eee2 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -6430,15 +6430,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;
@@ -6665,9 +6660,6 @@ static int do_md_stop(struct mddev *mddev, int mode)
export_array(mddev);
md_clean(mddev);
set_bit(MD_DELETED, &mddev->flags);
-
- 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)
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] md: remove/add redundancy group only in level change
2025-06-04 9:07 [PATCH V4 0/3] md: call del_gendisk in sync way Xiao Ni
2025-06-04 9:07 ` [PATCH 1/3] md: call del_gendisk in control path Xiao Ni
2025-06-04 9:07 ` [PATCH 2/3] md: Don't clear MD_CLOSING until mddev is freed Xiao Ni
@ 2025-06-04 9:07 ` Xiao Ni
2 siblings, 0 replies; 11+ messages in thread
From: Xiao Ni @ 2025-06-04 9:07 UTC (permalink / raw)
To: linux-raid; +Cc: yukuai3, ncroxon, song, yukuai1
del_gendisk is called in synchronous way now. So it doesn't need to handle
redundancy group in stop path separately.
Signed-off-by: Xiao Ni <xni@redhat.com>
---
drivers/md/md.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 8824ce64eee2..e3bb8c725bca 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -6528,8 +6528,6 @@ static void __md_stop(struct mddev *mddev)
if (mddev->private)
pers->free(mddev, mddev->private);
mddev->private = NULL;
- if (pers->sync_request && mddev->to_remove == NULL)
- mddev->to_remove = &md_redundancy_group;
put_pers(pers);
clear_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
--
2.32.0 (Apple Git-132)
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] md: call del_gendisk in control path
2025-06-04 9:07 ` [PATCH 1/3] md: call del_gendisk in control path Xiao Ni
@ 2025-06-06 8:46 ` Yu Kuai
2025-06-06 8:48 ` Yu Kuai
0 siblings, 1 reply; 11+ messages in thread
From: Yu Kuai @ 2025-06-06 8:46 UTC (permalink / raw)
To: Xiao Ni, linux-raid; +Cc: ncroxon, song, yukuai1, yukuai (C)
Hi,
在 2025/06/04 17:07, Xiao Ni 写道:
> Now del_gendisk and put_disk are called asynchronously in workqueue work.
> The asynchronous way 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 of gendisk.
>
> Function del_gendisk can't be called with reconfig_mutex. If it's called
> with reconfig mutex, a deadlock can happen. del_gendisk waits all sysfs
> files access to finish and sysfs file access waits reconfig mutex. So
> put del_gendisk after releasing reconfig mutex.
>
> But there is still 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. MD_DELETED is used to resolve this problem.
> MD_DELETED is set before releasing reconfig mutex and it should be checked
> for these sysfs access which need reconfig mutex. For sysfs access which
> don't need reconfig mutex, del_gendisk will wait them to finish.
>
> But it doesn't need to do this in function mddev_lock_nointr. There are
> ten places that call it.
> * Five of them are in dm raid which we don't need to care. MD_DELETED is
> only used for md raid.
> * stop_sync_thread, md_do_sync and md_start_sync are related sync request,
> and it needs to wait sync thread to finish before stopping an array.
> * md_ioctl: md_open is called before md_ioctl, so ->openers is added. It
> will fail to stop the array. So it doesn't need to check MD_DELETED here
> * md_set_readonly:
> It needs to call mddev_set_closing_and_sync_blockdev when setting readonly
> or read_auto. So it will fail to stop the array too because MD_CLOSING is
> already set.
>
> Signed-off-by: Xiao Ni <xni@redhat.com>
> ---
> drivers/md/md.c | 28 +++++++++++++++++++++++-----
> drivers/md/md.h | 26 ++++++++++++++++++++++++--
> 2 files changed, 47 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 0fde115e921f..e58bb80bf2e9 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -636,9 +636,6 @@ static void __mddev_put(struct mddev *mddev)
> 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);
> -
> /*
> * Call queue_work inside the spinlock so that flush_workqueue() after
> * mddev_find will succeed in waiting for the work to be done.
> @@ -873,6 +870,16 @@ void mddev_unlock(struct mddev *mddev)
> kobject_del(&rdev->kobj);
> export_rdev(rdev, mddev);
> }
> +
/*
* xxx
*/
> + /* 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))
> + del_gendisk(mddev->gendisk);
> }
> EXPORT_SYMBOL_GPL(mddev_unlock);
>
> @@ -5774,19 +5781,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;
> }
>
> @@ -5799,7 +5817,6 @@ static void md_kobj_release(struct kobject *ko)
> if (mddev->sysfs_level)
> sysfs_put(mddev->sysfs_level);
Since del_gendisk will remove the sysfs_file, should the above sysfs_put
and all other sysfs entries be removed before del_gendisk?
Thanks,
Kuai
>
> - del_gendisk(mddev->gendisk);
> put_disk(mddev->gendisk);
> }
>
> @@ -6646,8 +6663,9 @@ 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);
> +
> if (mddev->hold_active == UNTIL_STOP)
> mddev->hold_active = 0;
> }
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index d45a9e6ead80..de47b2500ef0 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -700,11 +700,26 @@ 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.
> + */
> + 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. And all paths can't
> + * call this function after do_md_stop.
> */
> static inline void mddev_lock_nointr(struct mddev *mddev)
> {
> @@ -713,7 +728,14 @@ static inline void mddev_lock_nointr(struct mddev *mddev)
>
> static inline int mddev_trylock(struct mddev *mddev)
> {
> - return mutex_trylock(&mddev->reconfig_mutex);
> + int ret = 0;
> +
> + ret = mutex_trylock(&mddev->reconfig_mutex);
> + if (!ret && test_bit(MD_DELETED, &mddev->flags)) {
> + ret = -EBUSY;
> + mutex_unlock(&mddev->reconfig_mutex);
> + }
> + return ret;
> }
> extern void mddev_unlock(struct mddev *mddev);
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] md: call del_gendisk in control path
2025-06-06 8:46 ` Yu Kuai
@ 2025-06-06 8:48 ` Yu Kuai
2025-06-09 9:15 ` Xiao Ni
0 siblings, 1 reply; 11+ messages in thread
From: Yu Kuai @ 2025-06-06 8:48 UTC (permalink / raw)
To: Yu Kuai, Xiao Ni, linux-raid; +Cc: ncroxon, song, yukuai (C)
Hi,
在 2025/06/06 16:46, Yu Kuai 写道:
> + if (!ret && test_bit(MD_DELETED, &mddev->flags)) {
> + ret = -EBUSY;
And I think ENODEV is better here.
Thanks,
Kuai
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] md: call del_gendisk in control path
2025-06-06 8:48 ` Yu Kuai
@ 2025-06-09 9:15 ` Xiao Ni
0 siblings, 0 replies; 11+ messages in thread
From: Xiao Ni @ 2025-06-09 9:15 UTC (permalink / raw)
To: Yu Kuai, linux-raid; +Cc: ncroxon, song, yukuai (C)
在 2025/6/6 下午4:48, Yu Kuai 写道:
> Hi,
>
> 在 2025/06/06 16:46, Yu Kuai 写道:
>> + if (!ret && test_bit(MD_DELETED, &mddev->flags)) {
>> + ret = -EBUSY;
>
> And I think ENODEV is better here.
>
> Thanks,
> Kuai
>
Hi Kuai
Thanks for pointing out these problems.
I modifed the patch (remove the sysfs remove also). The regression test
is running now. I'll send v5 if regression can pass.
Regards
Xiao
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/3] md: Don't clear MD_CLOSING until mddev is freed
2025-06-10 7:20 [PATCH V5 0/3] md: call del_gendisk in sync way Xiao Ni
@ 2025-06-10 7:20 ` Xiao Ni
2025-06-11 6:11 ` Yu Kuai
0 siblings, 1 reply; 11+ messages in thread
From: Xiao Ni @ 2025-06-10 7:20 UTC (permalink / raw)
To: linux-raid; +Cc: yukuai3, ncroxon, song, yukuai1
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 | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 7445e44eabff..dde3d2bfd34d 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -6425,15 +6425,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;
@@ -6660,9 +6655,6 @@ static int do_md_stop(struct mddev *mddev, int mode)
export_array(mddev);
md_clean(mddev);
set_bit(MD_DELETED, &mddev->flags);
-
- 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)
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] md: Don't clear MD_CLOSING until mddev is freed
2025-06-10 7:20 ` [PATCH 2/3] md: Don't clear MD_CLOSING until mddev is freed Xiao Ni
@ 2025-06-11 6:11 ` Yu Kuai
0 siblings, 0 replies; 11+ messages in thread
From: Yu Kuai @ 2025-06-11 6:11 UTC (permalink / raw)
To: Xiao Ni, linux-raid; +Cc: ncroxon, song, yukuai1, yukuai (C)
在 2025/06/10 15:20, 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 | 16 ++++------------
> 1 file changed, 4 insertions(+), 12 deletions(-)
Reviewed-by: Yu Kuai <yukuai3@huawei.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/3] md: Don't clear MD_CLOSING until mddev is freed
2025-06-11 7:31 [PATCH V6 0/3] md: call del_gendisk in sync way Xiao Ni
@ 2025-06-11 7:31 ` Xiao Ni
0 siblings, 0 replies; 11+ messages in thread
From: Xiao Ni @ 2025-06-11 7:31 UTC (permalink / raw)
To: linux-raid; +Cc: yukuai3, ncroxon, song, yukuai1
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.
Reviewed-by: Yu Kuai <yukuai3@huawei.com>
Signed-off-by: Xiao Ni <xni@redhat.com>
---
drivers/md/md.c | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 7445e44eabff..dde3d2bfd34d 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -6425,15 +6425,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;
@@ -6660,9 +6655,6 @@ static int do_md_stop(struct mddev *mddev, int mode)
export_array(mddev);
md_clean(mddev);
set_bit(MD_DELETED, &mddev->flags);
-
- 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)
^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-06-11 7:31 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-04 9:07 [PATCH V4 0/3] md: call del_gendisk in sync way Xiao Ni
2025-06-04 9:07 ` [PATCH 1/3] md: call del_gendisk in control path Xiao Ni
2025-06-06 8:46 ` Yu Kuai
2025-06-06 8:48 ` Yu Kuai
2025-06-09 9:15 ` Xiao Ni
2025-06-04 9:07 ` [PATCH 2/3] md: Don't clear MD_CLOSING until mddev is freed Xiao Ni
2025-06-04 9:07 ` [PATCH 3/3] md: remove/add redundancy group only in level change Xiao Ni
-- strict thread matches above, loose matches on Subject: below --
2025-06-11 7:31 [PATCH V6 0/3] md: call del_gendisk in sync way Xiao Ni
2025-06-11 7:31 ` [PATCH 2/3] md: Don't clear MD_CLOSING until mddev is freed Xiao Ni
2025-06-10 7:20 [PATCH V5 0/3] md: call del_gendisk in sync way Xiao Ni
2025-06-10 7:20 ` [PATCH 2/3] md: Don't clear MD_CLOSING until mddev is freed Xiao Ni
2025-06-11 6:11 ` Yu Kuai
2025-06-03 5:20 [PATCH V3 0/2] md: call del_gendisk in sync way Xiao Ni
2025-06-03 5:20 ` [PATCH 2/3] md: Don't clear MD_CLOSING until mddev is freed Xiao Ni
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).