* [PATCH] md: avoid modifying spares while the array is not suspended
@ 2026-06-30 7:56 Abd-Alrhman Masalkhi
2026-06-30 10:18 ` sashiko-bot
2026-07-05 16:16 ` yu kuai
0 siblings, 2 replies; 8+ messages in thread
From: Abd-Alrhman Masalkhi @ 2026-06-30 7:56 UTC (permalink / raw)
To: song, yukuai, magiclinan, xiao
Cc: linux-raid, linux-kernel, Abd-Alrhman Masalkhi, sashiko-bot
remove_spares() and remove_and_add_spares() modify the array's rdev
configuration. These operations are only safe after the array has been
suspended.
Today, md_start_sync() can call md_choose_sync_action() even when the
array has not been suspended. As a result, md_choose_sync_action() may
remove or replace rdevs while normal I/O is still accessing them.
The race can occur as follows:
raid10d Worker Normal IO
____________ _______________________ ______________________
raid10_write_request()
wait_blocked_dev()
set Blocked
set Faulty
Skip Faulty rdev
rrdev->nr_pending++
.repl_bio = bio
removeable_rdev = false .
array not suspended .
lock mddev goto err_handle
lock mddev (wait)
.
update sb .
clear Blocked .
.
unlock mddev .
lock mddev (acquires)
remove_spares()
removeable_rdev = true
raid10_remove_disk()
rdev = replacement
replacement = NULL
rdev_dec_pending(NULL)
unlock mddev (NULL)->nr_pending--
In this case, rdev_dec_pending() is called with a NULL pointer,
resulting in a NULL pointer dereference when attempting to decrement
nr_pending.
Fix this by ensuring that remove_spares() and remove_and_add_spares()
are only called after the array has been suspended, preventing
concurrent rdev configuration changes while normal I/O is in progress.
Fixes: bc08041b32ab ("md: suspend array in md_start_sync() if array need reconfiguration")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260628142420.1051027-1-abd.masalkhi@gmail.com?part=3
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
drivers/md/md.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 66a41d482e59..c85ebb59535b 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -10116,7 +10116,8 @@ static int remove_and_add_spares(struct mddev *mddev,
return spares;
}
-static bool md_choose_sync_action(struct mddev *mddev, int *spares)
+static bool md_choose_sync_action(struct mddev *mddev, int *spares,
+ bool array_suspended)
{
/* Check if reshape is in progress first. */
if (mddev->reshape_position != MaxSector) {
@@ -10132,7 +10133,9 @@ static bool md_choose_sync_action(struct mddev *mddev, int *spares)
/* Check if resync is in progress. */
if (mddev->resync_offset < MaxSector) {
- remove_spares(mddev, NULL);
+ if (array_suspended)
+ remove_spares(mddev, NULL);
+
set_bit(MD_RECOVERY_SYNC, &mddev->recovery);
clear_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
clear_bit(MD_RECOVERY_LAZY_RECOVER, &mddev->recovery);
@@ -10144,7 +10147,11 @@ static bool md_choose_sync_action(struct mddev *mddev, int *spares)
* also removed and re-added, to allow the personality to fail the
* re-add.
*/
- *spares = remove_and_add_spares(mddev, NULL);
+ if (array_suspended)
+ *spares = remove_and_add_spares(mddev, NULL);
+ else
+ *spares = 0;
+
if (*spares || test_bit(MD_RECOVERY_LAZY_RECOVER, &mddev->recovery)) {
clear_bit(MD_RECOVERY_SYNC, &mddev->recovery);
clear_bit(MD_RECOVERY_CHECK, &mddev->recovery);
@@ -10189,11 +10196,12 @@ static void md_start_sync(struct work_struct *ws)
* As we only add devices that are already in-sync, we can
* activate the spares immediately.
*/
- remove_and_add_spares(mddev, NULL);
+ if (suspend)
+ remove_and_add_spares(mddev, NULL);
goto not_running;
}
- if (!md_choose_sync_action(mddev, &spares))
+ if (!md_choose_sync_action(mddev, &spares, suspend))
goto not_running;
if (!mddev->pers->sync_request)
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] md: avoid modifying spares while the array is not suspended 2026-06-30 7:56 [PATCH] md: avoid modifying spares while the array is not suspended Abd-Alrhman Masalkhi @ 2026-06-30 10:18 ` sashiko-bot 2026-07-05 16:16 ` yu kuai 1 sibling, 0 replies; 8+ messages in thread From: sashiko-bot @ 2026-06-30 10:18 UTC (permalink / raw) To: Abd-Alrhman Masalkhi; +Cc: linux-raid, yukuai Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: New issues: - [High] The patch prevents the recovery thread from starting or resuming for existing rebuilding spares, silently abandoning recovery and leaving the array permanently degraded. Pre-existing issues: - [High] The patch addresses a TOCTOU race for `state_store` but misses other vulnerable paths like `slot_store` and cluster updates, which still call `remove_and_add_spares()` without array suspension. -- commit 16cf64f20d25bc671dc52422ec0a44672674e524 Author: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com> md: avoid modifying spares while the array is not suspended This commit ensures that remove_spares() and remove_and_add_spares() are only called after the array has been suspended. This prevents a race condition where normal I/O could trigger a NULL pointer dereference when rdevs are removed or replaced concurrently. Closes: https://sashiko.dev/#/patchset/20260628142420.1051027-1-abd.masalkhi@gmail.com?part=3 > diff --git a/drivers/md/md.c b/drivers/md/md.c > index d1465bcd86c81..f555eb5aca5d8 100644 > --- a/drivers/md/md.c > +++ b/drivers/md/md.c > @@ -10151,7 +10154,11 @@ static bool md_choose_sync_action(struct mddev *mddev, int *spares) > * also removed and re-added, to allow the personality to fail the > * re-add. > */ > - *spares = remove_and_add_spares(mddev, NULL); > + if (array_suspended) > + *spares = remove_and_add_spares(mddev, NULL); > + else > + *spares = 0; > + [Severity: High] Does this else branch prevent the recovery thread from starting or resuming for existing rebuilding spares? In md_start_sync(), suspend is determined by checking md_spares_need_change(mddev). For an existing recovering spare receiving normal I/O (where nr_pending > 0), both rdev_removeable() and rdev_addable() can evaluate to false. This causes md_spares_need_change() to return false, leaving suspend as false. When md_choose_sync_action() is called with array_suspended == false, this new else branch forces *spares = 0. By completely bypassing the counting of existing recovering spares, md_choose_sync_action() will return false and skip starting the sync thread. Since nr_pending dropping to zero on a non-faulty spare doesn't trigger a new MD_RECOVERY_NEEDED event, could this silently abandon recovery and leave the array permanently degraded? [Severity: High] This is a pre-existing issue, but does restricting remove_and_add_spares() calls to suspended arrays in md_choose_sync_action() leave other vulnerable paths exposed? The commit addresses a TOCTOU race by ensuring remove_and_add_spares() is only called when the array is suspended, preventing concurrent I/O from encountering a cleared replacement pointer. However, slot_store() can still call remove_and_add_spares() without an array suspension when a user writes "none" (slot == -1): drivers/md/md.c:slot_store() { ... clear_bit(Blocked, &rdev->flags); remove_and_add_spares(rdev->mddev, rdev); if (rdev->raid_disk >= 0) return -EBUSY; ... } Similarly, the MD cluster component updates the array configuration in check_sb_changes() by calling remove_and_add_spares() while the array is not suspended. Could normal I/O still increment nr_pending on a replacement drive just before remove_spares() removes the faulty drive and clears the replacement pointer in these paths, reproducing the exact TOCTOU crash this patch aims to fix? > @@ -10196,11 +10203,12 @@ static void md_start_sync(struct work_struct *ws) > * As we only add devices that are already in-sync, we can > * activate the spares immediately. > */ > - remove_and_add_spares(mddev, NULL); > + if (suspend) > + remove_and_add_spares(mddev, NULL); > goto not_running; > } > > - if (!md_choose_sync_action(mddev, &spares)) > + if (!md_choose_sync_action(mddev, &spares, suspend)) > goto not_running; -- Sashiko AI review · https://sashiko.dev/#/patchset/20260630075640.1081634-1-abd.masalkhi@gmail.com?part=1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] md: avoid modifying spares while the array is not suspended 2026-06-30 7:56 [PATCH] md: avoid modifying spares while the array is not suspended Abd-Alrhman Masalkhi 2026-06-30 10:18 ` sashiko-bot @ 2026-07-05 16:16 ` yu kuai 2026-07-05 19:58 ` Abd-Alrhman Masalkhi 1 sibling, 1 reply; 8+ messages in thread From: yu kuai @ 2026-07-05 16:16 UTC (permalink / raw) To: Abd-Alrhman Masalkhi, song, magiclinan, xiao, yukuai Cc: linux-raid, linux-kernel, sashiko-bot Hi, 在 2026/6/30 15:56, Abd-Alrhman Masalkhi 写道: > remove_spares() and remove_and_add_spares() modify the array's rdev > configuration. These operations are only safe after the array has been > suspended. > > Today, md_start_sync() can call md_choose_sync_action() even when the > array has not been suspended. As a result, md_choose_sync_action() may > remove or replace rdevs while normal I/O is still accessing them. > > The race can occur as follows: > > raid10d Worker Normal IO > ____________ _______________________ ______________________ > > raid10_write_request() > wait_blocked_dev() > set Blocked > set Faulty > Skip Faulty rdev > rrdev->nr_pending++ > .repl_bio = bio > removeable_rdev = false . > array not suspended . > lock mddev goto err_handle > lock mddev (wait) > . > update sb . > clear Blocked . > . > unlock mddev . > lock mddev (acquires) > remove_spares() > removeable_rdev = true > > raid10_remove_disk() > rdev = replacement > replacement = NULL > rdev_dec_pending(NULL) > unlock mddev (NULL)->nr_pending-- > > In this case, rdev_dec_pending() is called with a NULL pointer, > resulting in a NULL pointer dereference when attempting to decrement > nr_pending. > > Fix this by ensuring that remove_spares() and remove_and_add_spares() > are only called after the array has been suspended, preventing > concurrent rdev configuration changes while normal I/O is in progress. > > Fixes: bc08041b32ab ("md: suspend array in md_start_sync() if array need reconfiguration") > Reported-by: sashiko-bot <sashiko-bot@kernel.org> > Closes: https://sashiko.dev/#/patchset/20260628142420.1051027-1-abd.masalkhi@gmail.com?part=3 > Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com> > --- > drivers/md/md.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/drivers/md/md.c b/drivers/md/md.c > index 66a41d482e59..c85ebb59535b 100644 > --- a/drivers/md/md.c > +++ b/drivers/md/md.c > @@ -10116,7 +10116,8 @@ static int remove_and_add_spares(struct mddev *mddev, > return spares; > } > > -static bool md_choose_sync_action(struct mddev *mddev, int *spares) > +static bool md_choose_sync_action(struct mddev *mddev, int *spares, > + bool array_suspended) > { > /* Check if reshape is in progress first. */ > if (mddev->reshape_position != MaxSector) { > @@ -10132,7 +10133,9 @@ static bool md_choose_sync_action(struct mddev *mddev, int *spares) > > /* Check if resync is in progress. */ > if (mddev->resync_offset < MaxSector) { > - remove_spares(mddev, NULL); > + if (array_suspended) > + remove_spares(mddev, NULL); The problem looks real, however, I think this will cause a change that user will be awared, if there are really spares that can be removed from conf, but array is not suspended here, user will still expect rdev will be removed from conf automatically. In md_start_sync, if suspend is false, can we check again after mddev_lock? If suspend is supposed to be true, we can release the lock and retry with suspend = true. > + > set_bit(MD_RECOVERY_SYNC, &mddev->recovery); > clear_bit(MD_RECOVERY_RECOVER, &mddev->recovery); > clear_bit(MD_RECOVERY_LAZY_RECOVER, &mddev->recovery); > @@ -10144,7 +10147,11 @@ static bool md_choose_sync_action(struct mddev *mddev, int *spares) > * also removed and re-added, to allow the personality to fail the > * re-add. > */ > - *spares = remove_and_add_spares(mddev, NULL); > + if (array_suspended) > + *spares = remove_and_add_spares(mddev, NULL); > + else > + *spares = 0; > + > if (*spares || test_bit(MD_RECOVERY_LAZY_RECOVER, &mddev->recovery)) { > clear_bit(MD_RECOVERY_SYNC, &mddev->recovery); > clear_bit(MD_RECOVERY_CHECK, &mddev->recovery); > @@ -10189,11 +10196,12 @@ static void md_start_sync(struct work_struct *ws) > * As we only add devices that are already in-sync, we can > * activate the spares immediately. > */ > - remove_and_add_spares(mddev, NULL); > + if (suspend) > + remove_and_add_spares(mddev, NULL); > goto not_running; > } > > - if (!md_choose_sync_action(mddev, &spares)) > + if (!md_choose_sync_action(mddev, &spares, suspend)) > goto not_running; > > if (!mddev->pers->sync_request) -- Thanks, Kuai ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] md: avoid modifying spares while the array is not suspended 2026-07-05 16:16 ` yu kuai @ 2026-07-05 19:58 ` Abd-Alrhman Masalkhi 2026-07-07 1:12 ` yu kuai 0 siblings, 1 reply; 8+ messages in thread From: Abd-Alrhman Masalkhi @ 2026-07-05 19:58 UTC (permalink / raw) To: yu kuai, song, magiclinan, xiao, yukuai Cc: linux-raid, linux-kernel, sashiko-bot Hi Kuai, On Mon, Jul 06, 2026 at 00:16 +0800, yu kuai wrote: > Hi, > > 在 2026/6/30 15:56, Abd-Alrhman Masalkhi 写道: >> remove_spares() and remove_and_add_spares() modify the array's rdev >> configuration. These operations are only safe after the array has been >> suspended. >> >> Today, md_start_sync() can call md_choose_sync_action() even when the >> array has not been suspended. As a result, md_choose_sync_action() may >> remove or replace rdevs while normal I/O is still accessing them. >> >> The race can occur as follows: >> >> raid10d Worker Normal IO >> ____________ _______________________ ______________________ >> >> raid10_write_request() >> wait_blocked_dev() >> set Blocked >> set Faulty >> Skip Faulty rdev >> rrdev->nr_pending++ >> .repl_bio = bio >> removeable_rdev = false . >> array not suspended . >> lock mddev goto err_handle >> lock mddev (wait) >> . >> update sb . >> clear Blocked . >> . >> unlock mddev . >> lock mddev (acquires) >> remove_spares() >> removeable_rdev = true >> >> raid10_remove_disk() >> rdev = replacement >> replacement = NULL >> rdev_dec_pending(NULL) >> unlock mddev (NULL)->nr_pending-- >> >> In this case, rdev_dec_pending() is called with a NULL pointer, >> resulting in a NULL pointer dereference when attempting to decrement >> nr_pending. >> >> Fix this by ensuring that remove_spares() and remove_and_add_spares() >> are only called after the array has been suspended, preventing >> concurrent rdev configuration changes while normal I/O is in progress. >> >> Fixes: bc08041b32ab ("md: suspend array in md_start_sync() if array need reconfiguration") >> Reported-by: sashiko-bot <sashiko-bot@kernel.org> >> Closes: https://sashiko.dev/#/patchset/20260628142420.1051027-1-abd.masalkhi@gmail.com?part=3 >> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com> >> --- >> drivers/md/md.c | 18 +++++++++++++----- >> 1 file changed, 13 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/md/md.c b/drivers/md/md.c >> index 66a41d482e59..c85ebb59535b 100644 >> --- a/drivers/md/md.c >> +++ b/drivers/md/md.c >> @@ -10116,7 +10116,8 @@ static int remove_and_add_spares(struct mddev *mddev, >> return spares; >> } >> >> -static bool md_choose_sync_action(struct mddev *mddev, int *spares) >> +static bool md_choose_sync_action(struct mddev *mddev, int *spares, >> + bool array_suspended) >> { >> /* Check if reshape is in progress first. */ >> if (mddev->reshape_position != MaxSector) { >> @@ -10132,7 +10133,9 @@ static bool md_choose_sync_action(struct mddev *mddev, int *spares) >> >> /* Check if resync is in progress. */ >> if (mddev->resync_offset < MaxSector) { >> - remove_spares(mddev, NULL); >> + if (array_suspended) >> + remove_spares(mddev, NULL); > > The problem looks real, however, I think this will cause a change that user will be awared, > if there are really spares that can be removed from conf, but array is not suspended here, > user will still expect rdev will be removed from conf automatically. > > In md_start_sync, if suspend is false, can we check again after mddev_lock? If suspend is > supposed to be true, we can release the lock and retry with suspend = true. > Yes, I see, and your approach is much better. But what do you think about taking the lock first and then checking only once? >> + >> set_bit(MD_RECOVERY_SYNC, &mddev->recovery); >> clear_bit(MD_RECOVERY_RECOVER, &mddev->recovery); >> clear_bit(MD_RECOVERY_LAZY_RECOVER, &mddev->recovery); >> @@ -10144,7 +10147,11 @@ static bool md_choose_sync_action(struct mddev *mddev, int *spares) >> * also removed and re-added, to allow the personality to fail the >> * re-add. >> */ >> - *spares = remove_and_add_spares(mddev, NULL); >> + if (array_suspended) >> + *spares = remove_and_add_spares(mddev, NULL); >> + else >> + *spares = 0; >> + >> if (*spares || test_bit(MD_RECOVERY_LAZY_RECOVER, &mddev->recovery)) { >> clear_bit(MD_RECOVERY_SYNC, &mddev->recovery); >> clear_bit(MD_RECOVERY_CHECK, &mddev->recovery); >> @@ -10189,11 +10196,12 @@ static void md_start_sync(struct work_struct *ws) >> * As we only add devices that are already in-sync, we can >> * activate the spares immediately. >> */ >> - remove_and_add_spares(mddev, NULL); >> + if (suspend) >> + remove_and_add_spares(mddev, NULL); >> goto not_running; >> } >> >> - if (!md_choose_sync_action(mddev, &spares)) >> + if (!md_choose_sync_action(mddev, &spares, suspend)) >> goto not_running; >> >> if (!mddev->pers->sync_request) > > -- > Thanks, > Kuai -- Best Regards, Abd-Alrhman ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] md: avoid modifying spares while the array is not suspended 2026-07-05 19:58 ` Abd-Alrhman Masalkhi @ 2026-07-07 1:12 ` yu kuai 2026-07-07 10:35 ` Abd-Alrhman Masalkhi 0 siblings, 1 reply; 8+ messages in thread From: yu kuai @ 2026-07-07 1:12 UTC (permalink / raw) To: Abd-Alrhman Masalkhi, yu kuai, song, magiclinan, xiao Cc: linux-raid, linux-kernel, sashiko-bot Hi, 在 2026/7/6 3:58, Abd-Alrhman Masalkhi 写道: >> The problem looks real, however, I think this will cause a change that user will be awared, >> if there are really spares that can be removed from conf, but array is not suspended here, >> user will still expect rdev will be removed from conf automatically. >> >> In md_start_sync, if suspend is false, can we check again after mddev_lock? If suspend is >> supposed to be true, we can release the lock and retry with suspend = true. >> > Yes, I see, and your approach is much better. But what do you think > about taking the lock first and then checking only once? I don't get what you mean. If we take the lock and then check that array should suspend, we still have to release the lock before we suspend the array. -- Thanks, Kuai ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] md: avoid modifying spares while the array is not suspended 2026-07-07 1:12 ` yu kuai @ 2026-07-07 10:35 ` Abd-Alrhman Masalkhi 2026-07-08 8:32 ` yu kuai 0 siblings, 1 reply; 8+ messages in thread From: Abd-Alrhman Masalkhi @ 2026-07-07 10:35 UTC (permalink / raw) To: yu kuai, yu kuai, song, magiclinan, xiao Cc: linux-raid, linux-kernel, sashiko-bot Hi Kuai, On Tue, Jul 07, 2026 at 09:12 +0800, yu kuai wrote: > Hi, > > 在 2026/7/6 3:58, Abd-Alrhman Masalkhi 写道: >>> The problem looks real, however, I think this will cause a change that user will be awared, >>> if there are really spares that can be removed from conf, but array is not suspended here, >>> user will still expect rdev will be removed from conf automatically. >>> >>> In md_start_sync, if suspend is false, can we check again after mddev_lock? If suspend is >>> supposed to be true, we can release the lock and retry with suspend = true. >>> >> Yes, I see, and your approach is much better. But what do you think >> about taking the lock first and then checking only once? > > I don't get what you mean. If we take the lock and then check that array should > suspend, we still have to release the lock before we suspend the array. > Sorry, I was not clear. I meant, do we need to check twice, once before taking the lock and once after? It seems that the check before taking the lock is redundant. since the result would need to be checked again after taking the lock anyway. Could we drop the check before taking the lock and only check whether suspension is needed once while holding it? If suspension is needed, we would release the lock, suspend the array, and then reacquire the lock. > -- > Thanks, > Kuai -- Best Regards, Abd-Alrhman ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] md: avoid modifying spares while the array is not suspended 2026-07-07 10:35 ` Abd-Alrhman Masalkhi @ 2026-07-08 8:32 ` yu kuai 2026-07-08 8:59 ` Abd-Alrhman Masalkhi 0 siblings, 1 reply; 8+ messages in thread From: yu kuai @ 2026-07-08 8:32 UTC (permalink / raw) To: Abd-Alrhman Masalkhi, yu kuai, song, magiclinan, xiao Cc: linux-raid, linux-kernel, sashiko-bot Hi, 在 2026/7/7 18:35, Abd-Alrhman Masalkhi 写道: > Hi Kuai, > > On Tue, Jul 07, 2026 at 09:12 +0800, yu kuai wrote: >> Hi, >> >> 在 2026/7/6 3:58, Abd-Alrhman Masalkhi 写道: >>>> The problem looks real, however, I think this will cause a change that user will be awared, >>>> if there are really spares that can be removed from conf, but array is not suspended here, >>>> user will still expect rdev will be removed from conf automatically. >>>> >>>> In md_start_sync, if suspend is false, can we check again after mddev_lock? If suspend is >>>> supposed to be true, we can release the lock and retry with suspend = true. >>>> >>> Yes, I see, and your approach is much better. But what do you think >>> about taking the lock first and then checking only once? >> I don't get what you mean. If we take the lock and then check that array should >> suspend, we still have to release the lock before we suspend the array. >> > Sorry, I was not clear. I meant, do we need to check twice, once before > taking the lock and once after? It seems that the check before taking the > lock is redundant. since the result would need to be checked again after > taking the lock anyway. > > Could we drop the check before taking the lock and only check whether > suspension is needed once while holding it? If suspension is needed, we > would release the lock, suspend the array, and then reacquire the lock. Thanks for the explanation, I understand now. Howerver, I still prefer to check first before holding the lock. Because the checking is much lower overhead than acquire reconfig_mutex, and the race window that rdev become spare is small, so it's unlikely we'll acquire reconfig_mtuex twice. > >> -- >> Thanks, >> Kuai -- Thanks, Kuai ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] md: avoid modifying spares while the array is not suspended 2026-07-08 8:32 ` yu kuai @ 2026-07-08 8:59 ` Abd-Alrhman Masalkhi 0 siblings, 0 replies; 8+ messages in thread From: Abd-Alrhman Masalkhi @ 2026-07-08 8:59 UTC (permalink / raw) To: yu kuai, yu kuai, song, magiclinan, xiao Cc: linux-raid, linux-kernel, sashiko-bot Hi Kuai, On Wed, Jul 08, 2026 at 16:32 +0800, yu kuai wrote: > Hi, > > 在 2026/7/7 18:35, Abd-Alrhman Masalkhi 写道: >> Hi Kuai, >> >> On Tue, Jul 07, 2026 at 09:12 +0800, yu kuai wrote: >>> Hi, >>> >>> 在 2026/7/6 3:58, Abd-Alrhman Masalkhi 写道: >>>>> The problem looks real, however, I think this will cause a change that user will be awared, >>>>> if there are really spares that can be removed from conf, but array is not suspended here, >>>>> user will still expect rdev will be removed from conf automatically. >>>>> >>>>> In md_start_sync, if suspend is false, can we check again after mddev_lock? If suspend is >>>>> supposed to be true, we can release the lock and retry with suspend = true. >>>>> >>>> Yes, I see, and your approach is much better. But what do you think >>>> about taking the lock first and then checking only once? >>> I don't get what you mean. If we take the lock and then check that array should >>> suspend, we still have to release the lock before we suspend the array. >>> >> Sorry, I was not clear. I meant, do we need to check twice, once before >> taking the lock and once after? It seems that the check before taking the >> lock is redundant. since the result would need to be checked again after >> taking the lock anyway. >> >> Could we drop the check before taking the lock and only check whether >> suspension is needed once while holding it? If suspension is needed, we >> would release the lock, suspend the array, and then reacquire the lock. > > Thanks for the explanation, I understand now. Howerver, I still prefer to check > first before holding the lock. Because the checking is much lower overhead than > acquire reconfig_mutex, and the race window that rdev become spare is small, so > it's unlikely we'll acquire reconfig_mtuex twice. > I see, thanks for the explanation. I'll send a v2 shortly. >> >>> -- >>> Thanks, >>> Kuai > > -- > Thanks, > Kuai -- Best Regards, Abd-Alrhman ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-07-08 8:59 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-06-30 7:56 [PATCH] md: avoid modifying spares while the array is not suspended Abd-Alrhman Masalkhi 2026-06-30 10:18 ` sashiko-bot 2026-07-05 16:16 ` yu kuai 2026-07-05 19:58 ` Abd-Alrhman Masalkhi 2026-07-07 1:12 ` yu kuai 2026-07-07 10:35 ` Abd-Alrhman Masalkhi 2026-07-08 8:32 ` yu kuai 2026-07-08 8:59 ` Abd-Alrhman Masalkhi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox