mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3 0/4] md: Fix two raid5/raid10 bugs
@ 2026-09-23 11:21 Zhihao Cheng
  2026-09-23 11:21 ` [PATCH v3 1/4] md/raid5: Hide the origin mddev->thread before takeover Zhihao Cheng
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Zhihao Cheng @ 2026-09-23 11:21 UTC (permalink / raw)
  To: song, yukuai, shli, neil
  Cc: linux-raid, linux-kernel, chengzhihao1, yangerkun, yi.zhang

Fix two raid5/raid10 bugs in error injection case.

v1->v2:
 Handle pers->run failure in level_store
v2->v3:
 Patch 2: Release sysfs group in 'pers->run' error handling path

Zhihao Cheng (4):
  md/raid5: Hide the origin mddev->thread before takeover
  md: Handle pers->run failure in level_store
  md/raid5: Don't free conf on raid5_run failure
  md/raid10: Don't free conf on raid10_run failure

 drivers/md/md.c     | 29 ++++++++++++++++++++++++++++-
 drivers/md/raid10.c | 22 ++++++++++------------
 drivers/md/raid5.c  | 39 ++++++++++++++++++++++++++++-----------
 3 files changed, 66 insertions(+), 24 deletions(-)

-- 
2.52.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 1/4] md/raid5: Hide the origin mddev->thread before takeover
  2026-09-23 11:21 [PATCH v3 0/4] md: Fix two raid5/raid10 bugs Zhihao Cheng
@ 2026-09-23 11:21 ` Zhihao Cheng
  2026-10-09  4:31   ` yu kuai
  2026-09-23 11:21 ` [PATCH v3 2/4] md: Handle pers->run failure in level_store Zhihao Cheng
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Zhihao Cheng @ 2026-09-23 11:21 UTC (permalink / raw)
  To: song, yukuai, shli, neil
  Cc: linux-raid, linux-kernel, chengzhihao1, yangerkun, yi.zhang

The raid5 takeover invokes setup_conf and allocates strip heads, but
it wakes up the wrong thread, which lefts strip heads in the list
'conf->released_stripes' and not being processed. If raid5_run fails,
the strip heads won't be released, which triggers the following slab
warnings (CONFIG_SLUB_DEBUG):
 BUG raid5-md0 (Not tainted): Objects remaining on __kmem_cache_shutdown()
 Object 0x0000000062fad548 @offset=3968
 Object 0x000000007f74683c @offset=4960
 WARNING: mm/slub.c:1268 at __slab_err+0x31/0x40, CPU#0: bash/865
 RIP: 0010:__slab_err+0x31
 Call Trace:
  __kmem_cache_shutdown.cold+0x15b
  kmem_cache_destroy+0x71
  free_conf+0xf8
  raid5_run.cold+0x463
  level_store+0x64e
  md_attr_store+0xd7

The detailed triggering process is as follows:
 mdadm --create /dev/md0 --level=1 --raid-devices=2 /dev/sda /dev/sdb
 --force --assume-clean # create raid1, mddev->thread is raid1d
 echo 5 > /sys/block/md0/md/level
  level_store
   raid5_takeover_raid1
     setup_conf
      grow_stripes
       grow_one_stripe
        sh = alloc_stripe
        raid5_release_stripe
          md_wakeup_thread(conf->mddev->thread) // wakeup raid1d
     raid5_run
      ENOMEM = raid5_create_ctx_pool
      free_conf
       shrink_stripes
        drop_one_stripe // no strips found from the conf->inactive_list
        kmem_cache_destroy(conf->slab_cache)
	 __kmem_cache_shutdown
	  free_partial
	   list_slab_objects // some entries are not released !

Fix it by hiding the origin mddev->thread before takeover, so that
new allocating strip heads can be put into 'conf->inactive_list',
which can be found by drop_one_stripe().

Fixes: 773ca82fa1ee ("raid5: make release_stripe lockless")
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 drivers/md/raid5.c | 37 ++++++++++++++++++++++++++++---------
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index c091bba95c31..7e87e8a60f5f 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -9039,19 +9039,38 @@ static void *raid5_takeover(struct mddev *mddev)
 	 *  raid4 - trivial - just use a raid4 layout.
 	 *  raid6 - Providing it is a *_6 layout
 	 */
-	if (mddev->level == 0)
-		return raid45_takeover_raid0(mddev, 5);
-	if (mddev->level == 1)
-		return raid5_takeover_raid1(mddev);
-	if (mddev->level == 4) {
+	void *ret = ERR_PTR(-EINVAL);
+	struct md_thread *thread;
+
+	thread = rcu_dereference_protected(mddev->thread,
+				lockdep_is_held(&mddev->reconfig_mutex));
+	/*
+	 * Set mddev->thread to NULL before setup_conf() to avoid waking up
+	 * wrong thread(eg. raid1), which can prevent the strips from being
+	 * left unreleased in the error handling path(free_conf) of raid5_run.
+	 */
+	rcu_assign_pointer(mddev->thread, NULL);
+
+	switch (mddev->level) {
+	case 0:
+		ret = raid45_takeover_raid0(mddev, 5);
+		break;
+	case 1:
+		ret = raid5_takeover_raid1(mddev);
+		break;
+	case 4:
 		mddev->new_layout = ALGORITHM_PARITY_N;
 		mddev->new_level = 5;
-		return setup_conf(mddev);
+		ret = setup_conf(mddev);
+		break;
+	case 6:
+		ret = raid5_takeover_raid6(mddev);
+		break;
 	}
-	if (mddev->level == 6)
-		return raid5_takeover_raid6(mddev);
 
-	return ERR_PTR(-EINVAL);
+	rcu_assign_pointer(mddev->thread, thread);
+
+	return ret;
 }
 
 static void *raid4_takeover(struct mddev *mddev)
-- 
2.52.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 2/4] md: Handle pers->run failure in level_store
  2026-09-23 11:21 [PATCH v3 0/4] md: Fix two raid5/raid10 bugs Zhihao Cheng
  2026-09-23 11:21 ` [PATCH v3 1/4] md/raid5: Hide the origin mddev->thread before takeover Zhihao Cheng
@ 2026-09-23 11:21 ` Zhihao Cheng
  2026-10-09  5:05   ` yu kuai
  2026-09-23 11:21 ` [PATCH v3 3/4] md/raid5: Don't free conf on raid5_run failure Zhihao Cheng
  2026-09-23 11:21 ` [PATCH v3 4/4] md/raid10: Don't free conf on raid10_run failure Zhihao Cheng
  3 siblings, 1 reply; 8+ messages in thread
From: Zhihao Cheng @ 2026-09-23 11:21 UTC (permalink / raw)
  To: song, yukuai, shli, neil
  Cc: linux-raid, linux-kernel, chengzhihao1, yangerkun, yi.zhang

Set 'raid_disks' after raid5_run() failure will trigger an
null-ptr-deref problem:
 BUG: kernel NULL pointer dereference, address: 0000000000000038
 RIP: 0010:raid5_check_reshape+0xad
 Call Trace:
  update_raid_disks+0x124
  raid_disks_store+0x145
  md_attr_store+0xd7
  sysfs_kf_write+0x7c

The trigger process is simple:
 mdadm --create /dev/md0 --level=1 --raid-devices=2 /dev/sda /dev/sdb
 --force --assume-clean # create raid1
 echo 5 > /sys/block/md0/md/level
  level_store
   mddev->pers = pers
   mddev->private = priv
   raid5_run
    fail to abort (eg. raid5_create_ctx_pool fails)
    mddev->private = NULL
 echo 10 > /sys/block/md0/md/raid_disks
  raid_disks_store
   if (mddev->pers) // true
    update_raid_disks
     raid5_check_reshape
      conf = mddev->private
       conf->algorithm = mddev->new_layout // null-ptr-deref !

Similar process exists in do_md_stop->__md_stop_writes->raid5_quiesce.
Similar process exists in raid10 too.

Fix it by handling the error from pers->run, next active-type order will
restart the mddev.

Fixes: 245f46c2c221e ("md: add ->takeover method to support changing the personality managing an array")
Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 drivers/md/md.c | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 02798f2dbf0e..ff78bf8656ff 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -97,6 +97,7 @@ static struct workqueue_struct *md_misc_wq;
 static int remove_and_add_spares(struct mddev *mddev,
 				 struct md_rdev *this);
 static void mddev_detach(struct mddev *mddev);
+static void __md_stop(struct mddev *mddev);
 static void export_rdev(struct md_rdev *rdev);
 static void md_wakeup_thread_directly(struct md_thread __rcu **thread);
 
@@ -4243,7 +4244,33 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
 		mddev->in_sync = 1;
 		timer_delete_sync(&mddev->safemode_timer);
 	}
-	pers->run(mddev);
+	rv = pers->run(mddev);
+	if (rv) {
+		/*
+		 * ->run() has released the private data of the new personality,
+		 * while the old one has already been released as well. There is
+		 * nothing to fall back to, so stop the array to avoid leaving
+		 * 'mddev->pers' pointing to a personality which has no private
+		 * data, and reminds user to try to active the mddev again.
+		 */
+		pr_warn("md: %s: failed to run %s after takeover, please try to active\n",
+			mdname(mddev), pers->head.name);
+		if (mddev->pers->sync_request && mddev->to_remove == NULL)
+			mddev->to_remove = &md_redundancy_group;
+		if (md_bitmap_enabled(mddev, true))
+			mddev->bitmap_ops->flush(mddev);
+		clear_bit(MD_SERIALIZE_POLICY, &mddev->flags);
+		mddev_destroy_serial_pool(mddev, NULL);
+		__md_stop(mddev);
+		rdev_for_each(rdev, mddev)
+			if (rdev->raid_disk >= 0)
+				sysfs_unlink_rdev(mddev, rdev);
+		set_capacity_and_notify(mddev->gendisk, 0);
+		mddev->changed = 1;
+		md_new_event();
+		sysfs_notify_dirent_safe(mddev->sysfs_state);
+		goto out_unlock;
+	}
 	set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
 	if (!mddev->thread)
 		md_update_sb(mddev, 1);
-- 
2.52.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 3/4] md/raid5: Don't free conf on raid5_run failure
  2026-09-23 11:21 [PATCH v3 0/4] md: Fix two raid5/raid10 bugs Zhihao Cheng
  2026-09-23 11:21 ` [PATCH v3 1/4] md/raid5: Hide the origin mddev->thread before takeover Zhihao Cheng
  2026-09-23 11:21 ` [PATCH v3 2/4] md: Handle pers->run failure in level_store Zhihao Cheng
@ 2026-09-23 11:21 ` Zhihao Cheng
  2026-09-23 11:21 ` [PATCH v3 4/4] md/raid10: Don't free conf on raid10_run failure Zhihao Cheng
  3 siblings, 0 replies; 8+ messages in thread
From: Zhihao Cheng @ 2026-09-23 11:21 UTC (permalink / raw)
  To: song, yukuai, shli, neil
  Cc: linux-raid, linux-kernel, chengzhihao1, yangerkun, yi.zhang

Since all pers->run() callers handle the error case, no need to free
conf when raid5_run() fails.

Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 drivers/md/raid5.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 7e87e8a60f5f..913e5a709e35 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -8263,8 +8263,6 @@ static int raid5_run(struct mddev *mddev)
 abort:
 	md_unregister_thread(mddev, &mddev->thread);
 	print_raid5_conf(conf);
-	free_conf(conf);
-	mddev->private = NULL;
 	pr_warn("md/raid:%s: failed to run raid set.\n", mdname(mddev));
 	return ret;
 }
-- 
2.52.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 4/4] md/raid10: Don't free conf on raid10_run failure
  2026-09-23 11:21 [PATCH v3 0/4] md: Fix two raid5/raid10 bugs Zhihao Cheng
                   ` (2 preceding siblings ...)
  2026-09-23 11:21 ` [PATCH v3 3/4] md/raid5: Don't free conf on raid5_run failure Zhihao Cheng
@ 2026-09-23 11:21 ` Zhihao Cheng
  3 siblings, 0 replies; 8+ messages in thread
From: Zhihao Cheng @ 2026-09-23 11:21 UTC (permalink / raw)
  To: song, yukuai, shli, neil
  Cc: linux-raid, linux-kernel, chengzhihao1, yangerkun, yi.zhang

Since all pers->run() callers handle the error case, no need to free
conf when raid10_run() fails.

Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
---
 drivers/md/raid10.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 1093c798d9dd..9e4d4202f35c 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -3972,7 +3972,7 @@ static int raid10_run(struct mddev *mddev)
 		if (fc > 1 || fo > 0) {
 			pr_err("only near layout is supported by clustered"
 				" raid10\n");
-			goto out_free_conf;
+			goto out_unregister_thread;
 		}
 	}
 
@@ -3989,11 +3989,11 @@ static int raid10_run(struct mddev *mddev)
 
 		if (test_bit(Replacement, &rdev->flags)) {
 			if (disk->replacement)
-				goto out_free_conf;
+				goto out_unregister_thread;
 			disk->replacement = rdev;
 		} else {
 			if (disk->rdev)
-				goto out_free_conf;
+				goto out_unregister_thread;
 			disk->rdev = rdev;
 		}
 		diff = (rdev->new_data_offset - rdev->data_offset);
@@ -4013,7 +4013,7 @@ static int raid10_run(struct mddev *mddev)
 
 		if (err) {
 			ret = err;
-			goto out_free_conf;
+			goto out_unregister_thread;
 		}
 	}
 
@@ -4021,17 +4021,17 @@ static int raid10_run(struct mddev *mddev)
 	if (!enough(conf, -1)) {
 		pr_err("md/raid10:%s: not enough operational mirrors.\n",
 		       mdname(mddev));
-		goto out_free_conf;
+		goto out_unregister_thread;
 	}
 
 	if (conf->reshape_progress != MaxSector) {
 		/* must ensure that shape change is supported */
 		if (conf->geo.far_copies != 1 &&
 		    conf->geo.far_offset == 0)
-			goto out_free_conf;
+			goto out_unregister_thread;
 		if (conf->prev.far_copies != 1 &&
 		    conf->prev.far_offset == 0)
-			goto out_free_conf;
+			goto out_unregister_thread;
 	}
 
 	mddev->degraded = 0;
@@ -4081,7 +4081,7 @@ static int raid10_run(struct mddev *mddev)
 	set_bit(MD_FAILFAST_SUPPORTED, &mddev->flags);
 
 	if (md_integrity_register(mddev))
-		goto out_free_conf;
+		goto out_unregister_thread;
 
 	if (conf->reshape_progress != MaxSector) {
 		unsigned long before_length, after_length;
@@ -4094,7 +4094,7 @@ static int raid10_run(struct mddev *mddev)
 		if (max(before_length, after_length) > min_offset_diff) {
 			/* This cannot work */
 			pr_warn("md/raid10: offset difference not enough to continue reshape\n");
-			goto out_free_conf;
+			goto out_unregister_thread;
 		}
 		conf->offset_diff = min_offset_diff;
 
@@ -4106,10 +4106,8 @@ static int raid10_run(struct mddev *mddev)
 
 	return 0;
 
-out_free_conf:
+out_unregister_thread:
 	md_unregister_thread(mddev, &mddev->thread);
-	raid10_free_conf(conf);
-	mddev->private = NULL;
 out:
 	return ret;
 }
-- 
2.52.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/4] md/raid5: Hide the origin mddev->thread before takeover
  2026-09-23 11:21 ` [PATCH v3 1/4] md/raid5: Hide the origin mddev->thread before takeover Zhihao Cheng
@ 2026-10-09  4:31   ` yu kuai
  2026-10-09  7:12     ` Zhihao Cheng
  0 siblings, 1 reply; 8+ messages in thread
From: yu kuai @ 2026-10-09  4:31 UTC (permalink / raw)
  To: Zhihao Cheng, song, shli, neil, yu kuai
  Cc: linux-raid, linux-kernel, yangerkun, yi.zhang

Hi,

在 2026/9/23 19:21, Zhihao Cheng 写道:
> The raid5 takeover invokes setup_conf and allocates strip heads, but
> it wakes up the wrong thread, which lefts strip heads in the list
> 'conf->released_stripes' and not being processed. If raid5_run fails,
> the strip heads won't be released, which triggers the following slab
> warnings (CONFIG_SLUB_DEBUG):
>   BUG raid5-md0 (Not tainted): Objects remaining on __kmem_cache_shutdown()
>   Object 0x0000000062fad548 @offset=3968
>   Object 0x000000007f74683c @offset=4960
>   WARNING: mm/slub.c:1268 at __slab_err+0x31/0x40, CPU#0: bash/865
>   RIP: 0010:__slab_err+0x31
>   Call Trace:
>    __kmem_cache_shutdown.cold+0x15b
>    kmem_cache_destroy+0x71
>    free_conf+0xf8
>    raid5_run.cold+0x463
>    level_store+0x64e
>    md_attr_store+0xd7

Is this still a problem with following patch?

[PATCH] md/raid5: drain released_stripes before destroying the cache - 
Li Youhong 
<https://lore.kernel.org/all/20260929083916.50620-1-dayou5941@163.com/>

>
> The detailed triggering process is as follows:
>   mdadm --create /dev/md0 --level=1 --raid-devices=2 /dev/sda /dev/sdb
>   --force --assume-clean # create raid1, mddev->thread is raid1d
>   echo 5 > /sys/block/md0/md/level
>    level_store
>     raid5_takeover_raid1
>       setup_conf
>        grow_stripes
>         grow_one_stripe
>          sh = alloc_stripe
>          raid5_release_stripe
>            md_wakeup_thread(conf->mddev->thread) // wakeup raid1d
>       raid5_run
>        ENOMEM = raid5_create_ctx_pool
>        free_conf
>         shrink_stripes
>          drop_one_stripe // no strips found from the conf->inactive_list
>          kmem_cache_destroy(conf->slab_cache)
> 	 __kmem_cache_shutdown
> 	  free_partial
> 	   list_slab_objects // some entries are not released !
>
> Fix it by hiding the origin mddev->thread before takeover, so that
> new allocating strip heads can be put into 'conf->inactive_list',
> which can be found by drop_one_stripe().
>
> Fixes: 773ca82fa1ee ("raid5: make release_stripe lockless")
> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
> ---
>   drivers/md/raid5.c | 37 ++++++++++++++++++++++++++++---------
>   1 file changed, 28 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index c091bba95c31..7e87e8a60f5f 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -9039,19 +9039,38 @@ static void *raid5_takeover(struct mddev *mddev)
>   	 *  raid4 - trivial - just use a raid4 layout.
>   	 *  raid6 - Providing it is a *_6 layout
>   	 */
> -	if (mddev->level == 0)
> -		return raid45_takeover_raid0(mddev, 5);
> -	if (mddev->level == 1)
> -		return raid5_takeover_raid1(mddev);
> -	if (mddev->level == 4) {
> +	void *ret = ERR_PTR(-EINVAL);
> +	struct md_thread *thread;
> +
> +	thread = rcu_dereference_protected(mddev->thread,
> +				lockdep_is_held(&mddev->reconfig_mutex));
> +	/*
> +	 * Set mddev->thread to NULL before setup_conf() to avoid waking up
> +	 * wrong thread(eg. raid1), which can prevent the strips from being
> +	 * left unreleased in the error handling path(free_conf) of raid5_run.
> +	 */
> +	rcu_assign_pointer(mddev->thread, NULL);
> +
> +	switch (mddev->level) {
> +	case 0:
> +		ret = raid45_takeover_raid0(mddev, 5);
> +		break;
> +	case 1:
> +		ret = raid5_takeover_raid1(mddev);
> +		break;
> +	case 4:
>   		mddev->new_layout = ALGORITHM_PARITY_N;
>   		mddev->new_level = 5;
> -		return setup_conf(mddev);
> +		ret = setup_conf(mddev);
> +		break;
> +	case 6:
> +		ret = raid5_takeover_raid6(mddev);
> +		break;
>   	}
> -	if (mddev->level == 6)
> -		return raid5_takeover_raid6(mddev);
>   
> -	return ERR_PTR(-EINVAL);
> +	rcu_assign_pointer(mddev->thread, thread);
> +
> +	return ret;
>   }
>   
>   static void *raid4_takeover(struct mddev *mddev)

-- 
Thanks,
Kuai

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 2/4] md: Handle pers->run failure in level_store
  2026-09-23 11:21 ` [PATCH v3 2/4] md: Handle pers->run failure in level_store Zhihao Cheng
@ 2026-10-09  5:05   ` yu kuai
  0 siblings, 0 replies; 8+ messages in thread
From: yu kuai @ 2026-10-09  5:05 UTC (permalink / raw)
  To: Zhihao Cheng, song, shli, neil, yu kuai
  Cc: linux-raid, linux-kernel, yangerkun, yi.zhang

Hi,

在 2026/9/23 19:21, Zhihao Cheng 写道:
> Set 'raid_disks' after raid5_run() failure will trigger an
> null-ptr-deref problem:
>   BUG: kernel NULL pointer dereference, address: 0000000000000038
>   RIP: 0010:raid5_check_reshape+0xad
>   Call Trace:
>    update_raid_disks+0x124
>    raid_disks_store+0x145
>    md_attr_store+0xd7
>    sysfs_kf_write+0x7c
>
> The trigger process is simple:
>   mdadm --create /dev/md0 --level=1 --raid-devices=2 /dev/sda /dev/sdb
>   --force --assume-clean # create raid1
>   echo 5 > /sys/block/md0/md/level
>    level_store
>     mddev->pers = pers
>     mddev->private = priv
>     raid5_run
>      fail to abort (eg. raid5_create_ctx_pool fails)
>      mddev->private = NULL
>   echo 10 > /sys/block/md0/md/raid_disks
>    raid_disks_store
>     if (mddev->pers) // true
>      update_raid_disks
>       raid5_check_reshape
>        conf = mddev->private
>         conf->algorithm = mddev->new_layout // null-ptr-deref !
>
> Similar process exists in do_md_stop->__md_stop_writes->raid5_quiesce.
> Similar process exists in raid10 too.
>
> Fix it by handling the error from pers->run, next active-type order will
> restart the mddev.
>
> Fixes: 245f46c2c221e ("md: add ->takeover method to support changing the personality managing an array")
> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
> ---
>   drivers/md/md.c | 29 ++++++++++++++++++++++++++++-
>   1 file changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 02798f2dbf0e..ff78bf8656ff 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -97,6 +97,7 @@ static struct workqueue_struct *md_misc_wq;
>   static int remove_and_add_spares(struct mddev *mddev,
>   				 struct md_rdev *this);
>   static void mddev_detach(struct mddev *mddev);
> +static void __md_stop(struct mddev *mddev);
>   static void export_rdev(struct md_rdev *rdev);
>   static void md_wakeup_thread_directly(struct md_thread __rcu **thread);
>   
> @@ -4243,7 +4244,33 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
>   		mddev->in_sync = 1;
>   		timer_delete_sync(&mddev->safemode_timer);
>   	}
> -	pers->run(mddev);
> +	rv = pers->run(mddev);
> +	if (rv) {
> +		/*
> +		 * ->run() has released the private data of the new personality,
> +		 * while the old one has already been released as well. There is
> +		 * nothing to fall back to, so stop the array to avoid leaving
> +		 * 'mddev->pers' pointing to a personality which has no private
> +		 * data, and reminds user to try to active the mddev again.
> +		 */
> +		pr_warn("md: %s: failed to run %s after takeover, please try to active\n",
> +			mdname(mddev), pers->head.name);

Usually takeover can be performed on a live array, and with a live fs mounted, so
I don't think it's acceptable for the array to be in this state in the case
pers->run failed.

Is it possible to split pers->run into two parts? One for static checking and
memory allocation, so the next pers->run that will never fail. Which means we can
keep the old pers active until we can make sure the new pers activation callback will
not fail.

> +		if (mddev->pers->sync_request && mddev->to_remove == NULL)
> +			mddev->to_remove = &md_redundancy_group;
> +		if (md_bitmap_enabled(mddev, true))
> +			mddev->bitmap_ops->flush(mddev);
> +		clear_bit(MD_SERIALIZE_POLICY, &mddev->flags);
> +		mddev_destroy_serial_pool(mddev, NULL);
> +		__md_stop(mddev);
> +		rdev_for_each(rdev, mddev)
> +			if (rdev->raid_disk >= 0)
> +				sysfs_unlink_rdev(mddev, rdev);
> +		set_capacity_and_notify(mddev->gendisk, 0);
> +		mddev->changed = 1;
> +		md_new_event();
> +		sysfs_notify_dirent_safe(mddev->sysfs_state);
> +		goto out_unlock;
> +	}
>   	set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
>   	if (!mddev->thread)
>   		md_update_sb(mddev, 1);

-- 
Thanks,
Kuai

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/4] md/raid5: Hide the origin mddev->thread before takeover
  2026-10-09  4:31   ` yu kuai
@ 2026-10-09  7:12     ` Zhihao Cheng
  0 siblings, 0 replies; 8+ messages in thread
From: Zhihao Cheng @ 2026-10-09  7:12 UTC (permalink / raw)
  To: yukuai, song, shli, neil; +Cc: linux-raid, linux-kernel, yangerkun, yi.zhang

在 2026/10/9 12:31, yu kuai 写道:
> Hi,
> 
> 在 2026/9/23 19:21, Zhihao Cheng 写道:
>> The raid5 takeover invokes setup_conf and allocates strip heads, but
>> it wakes up the wrong thread, which lefts strip heads in the list
>> 'conf->released_stripes' and not being processed. If raid5_run fails,
>> the strip heads won't be released, which triggers the following slab
>> warnings (CONFIG_SLUB_DEBUG):
>>    BUG raid5-md0 (Not tainted): Objects remaining on __kmem_cache_shutdown()
>>    Object 0x0000000062fad548 @offset=3968
>>    Object 0x000000007f74683c @offset=4960
>>    WARNING: mm/slub.c:1268 at __slab_err+0x31/0x40, CPU#0: bash/865
>>    RIP: 0010:__slab_err+0x31
>>    Call Trace:
>>     __kmem_cache_shutdown.cold+0x15b
>>     kmem_cache_destroy+0x71
>>     free_conf+0xf8
>>     raid5_run.cold+0x463
>>     level_store+0x64e
>>     md_attr_store+0xd7
> 
> Is this still a problem with following patch?
> 
> [PATCH] md/raid5: drain released_stripes before destroying the cache -
> Li Youhong
> <https://lore.kernel.org/all/20260929083916.50620-1-dayou5941@163.com/>
Hi,

Youhong's patch could fix the problem, please take it, his solution is 
better.
> 
>>
>> The detailed triggering process is as follows:
>>    mdadm --create /dev/md0 --level=1 --raid-devices=2 /dev/sda /dev/sdb
>>    --force --assume-clean # create raid1, mddev->thread is raid1d
>>    echo 5 > /sys/block/md0/md/level
>>     level_store
>>      raid5_takeover_raid1
>>        setup_conf
>>         grow_stripes
>>          grow_one_stripe
>>           sh = alloc_stripe
>>           raid5_release_stripe
>>             md_wakeup_thread(conf->mddev->thread) // wakeup raid1d
>>        raid5_run
>>         ENOMEM = raid5_create_ctx_pool
>>         free_conf
>>          shrink_stripes
>>           drop_one_stripe // no strips found from the conf->inactive_list
>>           kmem_cache_destroy(conf->slab_cache)
>> 	 __kmem_cache_shutdown
>> 	  free_partial
>> 	   list_slab_objects // some entries are not released !
>>
>> Fix it by hiding the origin mddev->thread before takeover, so that
>> new allocating strip heads can be put into 'conf->inactive_list',
>> which can be found by drop_one_stripe().
>>
>> Fixes: 773ca82fa1ee ("raid5: make release_stripe lockless")
>> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
>> ---
>>    drivers/md/raid5.c | 37 ++++++++++++++++++++++++++++---------
>>    1 file changed, 28 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
>> index c091bba95c31..7e87e8a60f5f 100644
>> --- a/drivers/md/raid5.c
>> +++ b/drivers/md/raid5.c
>> @@ -9039,19 +9039,38 @@ static void *raid5_takeover(struct mddev *mddev)
>>    	 *  raid4 - trivial - just use a raid4 layout.
>>    	 *  raid6 - Providing it is a *_6 layout
>>    	 */
>> -	if (mddev->level == 0)
>> -		return raid45_takeover_raid0(mddev, 5);
>> -	if (mddev->level == 1)
>> -		return raid5_takeover_raid1(mddev);
>> -	if (mddev->level == 4) {
>> +	void *ret = ERR_PTR(-EINVAL);
>> +	struct md_thread *thread;
>> +
>> +	thread = rcu_dereference_protected(mddev->thread,
>> +				lockdep_is_held(&mddev->reconfig_mutex));
>> +	/*
>> +	 * Set mddev->thread to NULL before setup_conf() to avoid waking up
>> +	 * wrong thread(eg. raid1), which can prevent the strips from being
>> +	 * left unreleased in the error handling path(free_conf) of raid5_run.
>> +	 */
>> +	rcu_assign_pointer(mddev->thread, NULL);
>> +
>> +	switch (mddev->level) {
>> +	case 0:
>> +		ret = raid45_takeover_raid0(mddev, 5);
>> +		break;
>> +	case 1:
>> +		ret = raid5_takeover_raid1(mddev);
>> +		break;
>> +	case 4:
>>    		mddev->new_layout = ALGORITHM_PARITY_N;
>>    		mddev->new_level = 5;
>> -		return setup_conf(mddev);
>> +		ret = setup_conf(mddev);
>> +		break;
>> +	case 6:
>> +		ret = raid5_takeover_raid6(mddev);
>> +		break;
>>    	}
>> -	if (mddev->level == 6)
>> -		return raid5_takeover_raid6(mddev);
>>    
>> -	return ERR_PTR(-EINVAL);
>> +	rcu_assign_pointer(mddev->thread, thread);
>> +
>> +	return ret;
>>    }
>>    
>>    static void *raid4_takeover(struct mddev *mddev)
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-10-09  7:12 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-23 11:21 [PATCH v3 0/4] md: Fix two raid5/raid10 bugs Zhihao Cheng
2026-09-23 11:21 ` [PATCH v3 1/4] md/raid5: Hide the origin mddev->thread before takeover Zhihao Cheng
2026-10-09  4:31   ` yu kuai
2026-10-09  7:12     ` Zhihao Cheng
2026-09-23 11:21 ` [PATCH v3 2/4] md: Handle pers->run failure in level_store Zhihao Cheng
2026-10-09  5:05   ` yu kuai
2026-09-23 11:21 ` [PATCH v3 3/4] md/raid5: Don't free conf on raid5_run failure Zhihao Cheng
2026-09-23 11:21 ` [PATCH v3 4/4] md/raid10: Don't free conf on raid10_run failure Zhihao Cheng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®