* [PATCH] md-cluster: check pers->resize() return value in update_size()
@ 2026-08-27 2:52 ghuicao
2026-08-27 3:07 ` sashiko-bot
2026-08-27 6:11 ` [PATCH v2 1/3] md-cluster: fix error handling and superblock update in update_size revert ghuicao
0 siblings, 2 replies; 14+ messages in thread
From: ghuicao @ 2026-08-27 2:52 UTC (permalink / raw)
To: Song Liu; +Cc: Yu Kuai, Li Nan, Xiao Ni, linux-raid, linux-kernel, Cao Guanghui
From: Cao Guanghui <caoguanghui@kylinos.cn>
In update_size(), when cluster_check_sync_size() detects that not all
nodes have updated their sync_size, the initiator reverts the array
size via pers->resize(). However, the return value of resize() is
immediately overwritten by __sendmsg(), so a resize failure is
silently lost.
If the revert resize fails, the array remains at the new (larger)
size while other nodes have not confirmed the change, leaving the
cluster in an inconsistent state with no error logged.
Check the resize() return value and log an error before sending the
METADATA_UPDATED message, so that a resize failure is visible to the
user.
Signed-off-by: Cao Guanghui <caoguanghui@kylinos.cn>
---
drivers/md/md-cluster.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -1349,7 +1349,10 @@ static void update_size(struct mddev *mddev, sector_t old_dev_sectors)
} else {
/* revert to previous sectors */
ret = mddev->pers->resize(mddev, old_dev_sectors);
+ if (ret)
+ pr_err("%s:%d: failed to revert array size\n",
+ __func__, __LINE__);
ret = __sendmsg(cinfo, &cmsg);
if (ret)
pr_err("%s:%d: failed to send METADATA_UPDATED msg\n",
__func__, __LINE__);
--
2.34.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] md-cluster: check pers->resize() return value in update_size()
2026-08-27 2:52 [PATCH] md-cluster: check pers->resize() return value in update_size() ghuicao
@ 2026-08-27 3:07 ` sashiko-bot
2026-08-27 6:11 ` [PATCH v2 1/3] md-cluster: fix error handling and superblock update in update_size revert ghuicao
1 sibling, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-08-27 3:07 UTC (permalink / raw)
To: ghuicao; +Cc: linux-raid, yukuai
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Missing superblock update after reverting array size leads to cluster-wide metadata inconsistency.
- [High] Cluster capacity change failures are incorrectly masked from userspace.
- [High] DLM lock leaked in EX state if down-conversion fails in `__sendmsg()`.
--
commit 716b34ca47de8d73645de2da63206339cac7a868
Author: Cao Guanghui <caoguanghui@kylinos.cn>
md-cluster: check pers->resize() return value in update_size()
This commit checks the return value of pers->resize() during an array capacity
revert and logs an error if it fails, ensuring the failure is visible.
> diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
> index 24dac6f09da82..0c6da16aecbfa 100644
> --- a/drivers/md/md-cluster.c
> +++ b/drivers/md/md-cluster.c
> @@ -1349,6 +1349,9 @@ static void update_size(struct mddev *mddev, sector_t old_dev_sectors)
> } else {
> /* revert to previous sectors */
> ret = mddev->pers->resize(mddev, old_dev_sectors);
> + if (ret)
> + pr_err("%s:%d: failed to revert array size\n",
> + __func__, __LINE__);
[Severity: High]
This is a pre-existing issue, but since update_size() has a void return type,
cluster capacity change failures like this are not propagated back to the core
MD layer.
In drivers/md/md.c:update_size(), the return state of the cluster size
update is ignored:
if (mddev_is_clustered(mddev))
mddev->cluster_ops->update_size(mddev, old_dev_sectors);
This allows the function to return success (0) even if the cluster resize
failed and was reverted here. Should we change the signature of update_size()
so userspace is accurately informed of the failure?
[Severity: Critical]
This is a pre-existing issue, but is there a missing superblock update before
sending the METADATA_UPDATED message?
If the resize operation fails and the in-memory size is reverted via
mddev->pers->resize(), the on-disk superblock still retains the new size
written earlier.
Because md_update_sb() is not called before broadcasting METADATA_UPDATED,
other nodes reading the on-disk superblock will adopt the new size while the
initiator runs with the old size. Can this lead to cluster-wide metadata
inconsistency and data corruption?
> ret = __sendmsg(cinfo, &cmsg);
[Severity: High]
This is a pre-existing issue, but I have a question about the __sendmsg()
error handling.
Looking at __sendmsg(), if the down-conversion of the lock fails:
/*down-convert EX to CR on Ack*/
error = dlm_lock_sync(cinfo->ack_lockres, DLM_LOCK_CR);
if (error) {
pr_err("md-cluster: failed to convert EX to CR on ACK(%d)\n",
error);
goto failed_ack;
}
failed_ack:
while ((unlock_error = dlm_unlock_sync(cinfo->message_lockres)))
pr_err("md-cluster: failed convert to NL on MESSAGE(%d)\n",
unlock_error);
return error;
The cleanup block handles message_lockres but completely ignores ack_lockres.
Does this permanently leak the DLM lock in EX state if the down-conversion
fails, potentially causing a cluster-wide deadlock?
> if (ret)
> pr_err("%s:%d: failed to send METADATA_UPDATED msg\n",
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260827025229.40138-1-ghuicao@163.com?part=1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/3] md-cluster: fix error handling and superblock update in update_size revert
2026-08-27 2:52 [PATCH] md-cluster: check pers->resize() return value in update_size() ghuicao
2026-08-27 3:07 ` sashiko-bot
@ 2026-08-27 6:11 ` ghuicao
2026-08-27 6:11 ` [PATCH v2 2/3] md-cluster: propagate update_size() errors to callers ghuicao
` (3 more replies)
1 sibling, 4 replies; 14+ messages in thread
From: ghuicao @ 2026-08-27 6:11 UTC (permalink / raw)
To: Song Liu
Cc: Yu Kuai, Li Nan, Xiao Ni, linux-raid, linux-kernel, stable,
Cao Guanghui
From: Cao Guanghui <caoguanghui@kylinos.cn>
In update_size(), when cluster_check_sync_size() detects that not all
nodes have updated their sync_size, the initiator reverts the array
size via pers->resize(). Two issues exist in this revert path:
1. The return value of resize() is immediately overwritten by
__sendmsg(), so a resize failure is silently lost. The array
remains at the new size while other nodes have not confirmed the
change, with no error logged.
2. The on-disk superblock still retains the new size written by the
earlier md_update_sb() call. Other nodes that receive the
METADATA_UPDATED message will re-read the on-disk superblock and
adopt the new size, while the initiator runs with the reverted old
size, causing a cluster-wide metadata inconsistency.
Fix by checking the resize() return value and logging an error, then
calling md_update_sb() so the on-disk superblock matches the in-memory
array size before broadcasting the message.
Fixes: 818da59f97d6 ("md-cluster: add the support for resize")
Cc: stable@vger.kernel.org
Signed-off-by: Cao Guanghui <caoguanghui@kylinos.cn>
---
Changes in v2:
- Fold in the md_update_sb() fix (was a separate patch in v1)
- Combine error check and superblock update into one coherent fix
drivers/md/md-cluster.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -1349,7 +1349,11 @@ static void update_size(struct mddev *mddev, sector_t old_dev_sectors)
} else {
/* revert to previous sectors */
ret = mddev->pers->resize(mddev, old_dev_sectors);
+ if (ret)
+ pr_err("%s:%d: failed to revert array size\n",
+ __func__, __LINE__);
+ md_update_sb(mddev, 1);
ret = __sendmsg(cinfo, &cmsg);
if (ret)
pr_err("%s:%d: failed to send METADATA_UPDATED msg\n",
__func__, __LINE__);
}
--
2.34.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 2/3] md-cluster: propagate update_size() errors to callers
2026-08-27 6:11 ` [PATCH v2 1/3] md-cluster: fix error handling and superblock update in update_size revert ghuicao
@ 2026-08-27 6:11 ` ghuicao
2026-08-27 6:23 ` sashiko-bot
2026-08-27 6:12 ` [PATCH v2 3/3] md-cluster: fix ack_lockres leak in __sendmsg error path ghuicao
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: ghuicao @ 2026-08-27 6:11 UTC (permalink / raw)
To: Song Liu
Cc: Yu Kuai, Li Nan, Xiao Ni, linux-raid, linux-kernel, stable,
Cao Guanghui
From: Cao Guanghui <caoguanghui@kylinos.cn>
update_size() in md-cluster.c has a void return type, so the caller in
md.c cannot detect cluster resize failures. A capacity revert or
message delivery failure is silently lost, and the MD layer reports
success to userspace even when the cluster operation failed.
Change the update_size() callback in struct md_cluster_ops and its
implementation to return int, and propagate the error at the call sites
in md.c.
Fixes: 818da59f97d6 ("md-cluster: add the support for resize")
Cc: stable@vger.kernel.org
Signed-off-by: Cao Guanghui <caoguanghui@kylinos.cn>
---
drivers/md/md-cluster.c | 11 ++++++-----
drivers/md/md-cluster.h | 2 +-
drivers/md/md.c | 7 +++++--
3 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -1292,7 +1292,7 @@ static int cluster_check_sync_size(struct mddev *mddev)
* let other nodes to perform it. If one node can't update sync_size
* accordingly, we need to revert to previous value.
*/
-static void update_size(struct mddev *mddev, sector_t old_dev_sectors)
+static int update_size(struct mddev *mddev, sector_t old_dev_sectors)
{
struct md_cluster_info *cinfo = mddev->cluster_info;
struct cluster_msg cmsg;
@@ -1303,7 +1303,7 @@ static int update_size(struct mddev *mddev, sector_t old_dev_sectors)
md_update_sb(mddev, 1);
if (lock_comm(cinfo, 1)) {
pr_err("%s: lock_comm failed\n", __func__);
- return;
+ return -1;
}
memset(&cmsg, 0, sizeof(cmsg));
@@ -1325,12 +1325,12 @@ static int update_size(struct mddev *mddev, sector_t old_dev_sectors)
pr_err("%s:%d: failed to send METADATA_UPDATED msg\n",
__func__, __LINE__);
unlock_comm(cinfo);
- return;
+ return ret;
}
} else {
pr_err("md-cluster: No good device id found to send\n");
unlock_comm(cinfo);
- return;
+ return -1;
}
/*
@@ -1355,6 +1355,7 @@ static int update_size(struct mddev *mddev, sector_t old_dev_sectors)
__func__, __LINE__);
}
unlock_comm(cinfo);
+ return ret;
}
static int resync_start(struct mddev *mddev)
diff --git a/drivers/md/md-cluster.h b/drivers/md/md-cluster.h
--- a/drivers/md/md-cluster.h
+++ b/drivers/md/md-cluster.h
@@ -34,7 +34,7 @@ struct md_cluster_operations {
int (*resize_bitmaps)(struct mddev *mddev, sector_t newsize, sector_t oldsize);
int (*lock_all_bitmaps)(struct mddev *mddev);
void (*unlock_all_bitmaps)(struct mddev *mddev);
- void (*update_size)(struct mddev *mddev, sector_t old_dev_sectors);
+ int (*update_size)(struct mddev *mddev, sector_t old_dev_sectors);
};
extern int md_setup_cluster(struct mddev *mddev, int nodes);
diff --git a/drivers/md/md.c b/drivers/md/md.c
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -8023,7 +8023,7 @@ static int update_size(struct mddev *mddev, sector_t num_sectors)
rv = mddev->pers->resize(mddev, num_sectors);
if (!rv) {
if (mddev_is_clustered(mddev))
- mddev->cluster_ops->update_size(mddev, old_dev_sectors);
+ rv = mddev->cluster_ops->update_size(mddev, old_dev_sectors);
else if (!mddev_is_dm(mddev))
set_capacity_and_notify(mddev->gendisk,
mddev->array_sectors);
@@ -10615,8 +10615,12 @@ void md_reap_sync_thread(struct mddev *mddev)
*/
if (mddev_is_clustered(mddev) && is_reshaped &&
mddev->pers->finish_reshape &&
- !test_bit(MD_CLOSING, &mddev->flags))
- mddev->cluster_ops->update_size(mddev, old_dev_sectors);
+ !test_bit(MD_CLOSING, &mddev->flags)) {
+ int ret = mddev->cluster_ops->update_size(mddev, old_dev_sectors);
+
+ if (ret)
+ pr_warn("md: cluster update_size failed after reshape: %d\n", ret);
+ }
/* flag recovery needed just to double check */
set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
sysfs_notify_dirent_safe(mddev->sysfs_completed);
--
2.34.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 3/3] md-cluster: fix ack_lockres leak in __sendmsg error path
2026-08-27 6:11 ` [PATCH v2 1/3] md-cluster: fix error handling and superblock update in update_size revert ghuicao
2026-08-27 6:11 ` [PATCH v2 2/3] md-cluster: propagate update_size() errors to callers ghuicao
@ 2026-08-27 6:12 ` ghuicao
2026-08-27 6:24 ` sashiko-bot
2026-08-27 6:24 ` [PATCH v2 1/3] md-cluster: fix error handling and superblock update in update_size revert sashiko-bot
2026-08-27 8:44 ` [PATCH v3 1/3] md-cluster: fix lock_comm leak and __sendmsg error path ghuicao
3 siblings, 1 reply; 14+ messages in thread
From: ghuicao @ 2026-08-27 6:12 UTC (permalink / raw)
To: Song Liu
Cc: Yu Kuai, Li Nan, Xiao Ni, linux-raid, linux-kernel, stable,
Cao Guanghui
From: Cao Guanghui <caoguanghui@kylinos.cn>
In __sendmsg(), if the down-conversion of ack_lockres from EX to CR
fails (step 5), the code jumps to failed_ack which only unlocks
message_lockres. The ack_lockres is left in EX state, causing a
cluster-wide deadlock as other nodes cannot acquire the ack lock.
Restore ack_lockres to CR in the failed_ack path when an error occurred,
so the lock is not leaked in EX state.
Fixes: 601b515c5dcc ("Communication Framework: Sending functions")
Cc: stable@vger.kernel.org
Signed-off-by: Cao Guanghui <caoguanghui@kylinos.cn>
---
drivers/md/md-cluster.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -801,8 +801,10 @@ static int __sendmsg(struct md_cluster_info *cinfo, struct cluster_msg *cmsg)
}
failed_ack:
+ if (error)
+ dlm_lock_sync(cinfo->ack_lockres, DLM_LOCK_CR);
while ((unlock_error = dlm_unlock_sync(cinfo->message_lockres)))
pr_err("md-cluster: failed convert to NL on MESSAGE(%d)\n",
unlock_error);
return error;
--
2.34.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/3] md-cluster: propagate update_size() errors to callers
2026-08-27 6:11 ` [PATCH v2 2/3] md-cluster: propagate update_size() errors to callers ghuicao
@ 2026-08-27 6:23 ` sashiko-bot
0 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-08-27 6:23 UTC (permalink / raw)
To: ghuicao; +Cc: yukuai, linux-raid
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
New issues:
- [High] Self-deadlock in error path of update_size() when calling md_update_sb() while holding cluster locks.
- [High] The patch fails its stated goal by returning success when a cluster capacity revert occurs.
- [High] Early cluster communication error paths return an error without reverting the local array size, leaving the cluster in a split-brain state.
- [Medium] The patch propagates hardcoded `-1` returns from update_size() which translates to `-EPERM` for userspace, violating standard kernel error codes.
Pre-existing issues:
- [High] lock_comm() leaks the MD_CLUSTER_SEND_LOCK bit on lock_token() failure, causing permanent hangs for future cluster operations.
--
commit 10a3e68b8ff2647112ced9ea72fb738bc9b14bb0
Author: Cao Guanghui <caoguanghui@kylinos.cn>
md-cluster: propagate update_size() errors to callers
This patch updates the update_size() callback in the md_cluster_ops structure
to return an integer instead of void. This allows error codes from cluster
operations to be propagated up to the caller in md.c, instead of failing
silently.
> diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
> index 9f404660a1fb7..bc4118d8c5a9f 100644
> --- a/drivers/md/md-cluster.c
> +++ b/drivers/md/md-cluster.c
> @@ -1292,7 +1292,7 @@ static int cluster_check_sync_size(struct mddev *mddev)
[ ... ]
> -static void update_size(struct mddev *mddev, sector_t old_dev_sectors)
> +static int update_size(struct mddev *mddev, sector_t old_dev_sectors)
> {
> struct md_cluster_info *cinfo = mddev->cluster_info;
> struct cluster_msg cmsg;
> @@ -1303,7 +1303,7 @@ static void update_size(struct mddev *mddev, sector_t old_dev_sectors)
> md_update_sb(mddev, 1);
> if (lock_comm(cinfo, 1)) {
[Severity: High]
This is a pre-existing issue, but does lock_comm() leak the
MD_CLUSTER_SEND_LOCK bit on failure?
Looking at lock_comm(), if test_and_set_bit(MD_CLUSTER_SEND_LOCK) succeeds,
but lock_token() fails, lock_comm() directly returns the error without
clearing the bit:
lock_comm() {
...
wait_event(cinfo->wait,
!test_and_set_bit(MD_CLUSTER_SEND_LOCK, &cinfo->state));
rv = lock_token(cinfo);
if (set_bit)
clear_bit_unlock(MD_CLUSTER_HOLDING_MUTEX_FOR_RECVD, &cinfo->state);
return rv;
}
If this occurs, will subsequent cluster operations permanently hang in
wait_event() waiting for the bit to clear?
> pr_err("%s: lock_comm failed\n", __func__);
> - return;
> + return -1;
[Severity: Medium]
Could returning -1 here cause userspace to see a confusing -EPERM error?
Since the patch changes this callback to return int, using standard error
codes like -EIO, -EAGAIN, or -ENODEV might be more appropriate than a
hardcoded -1.
[ ... ]
> @@ -1325,12 +1325,12 @@ static void update_size(struct mddev *mddev, sector_t old_dev_sectors)
> pr_err("%s:%d: failed to send METADATA_UPDATED msg\n",
> __func__, __LINE__);
> unlock_comm(cinfo);
> - return;
> + return ret;
> }
> } else {
> pr_err("md-cluster: No good device id found to send\n");
> unlock_comm(cinfo);
> - return;
> + return -1;
[Severity: Medium]
Does returning -1 here also similarly risk propagating -EPERM to userspace
for an internal configuration or device availability error?
> }
>
> /*
[ ... ]
> if (cluster_check_sync_size(mddev) == 0) {
> memset(&cmsg, 0, sizeof(cmsg));
> cmsg.type = cpu_to_le32(CHANGE_CAPACITY);
> ret = __sendmsg(cinfo, &cmsg);
> if (ret)
> pr_err("%s:%d: failed to send CHANGE_CAPACITY msg\n",
> __func__, __LINE__);
> set_capacity_and_notify(mddev->gendisk, mddev->array_sectors);
> } else {
> /* revert to previous sectors */
> ret = mddev->pers->resize(mddev, old_dev_sectors);
> if (ret)
> pr_err("%s:%d: failed to revert array size\n",
> __func__, __LINE__);
> md_update_sb(mddev, 1);
[Severity: High]
Could this call to md_update_sb() cause a self-deadlock?
The current thread holds the cluster send lock because lock_comm() was called
successfully at the start of update_size(). When md_update_sb() is called, it
triggers a cluster metadata update:
md_update_sb() -> mddev->cluster_ops->metadata_update_start()
The metadata_update_start() path appears to wait for MD_CLUSTER_SEND_LOCK to
be cleared. Since the lock is held by the same thread, will it wait forever?
> ret = __sendmsg(cinfo, &cmsg);
[Severity: High]
Does this __sendmsg() call overwrite the failure state from the capacity
revert?
If the cluster sync size check fails, the result of the local revert
operation is stored in 'ret' right above. By reassigning 'ret' to the result
of __sendmsg() here, would a successful broadcast incorrectly return 0
(success) to userspace instead of propagating the failure of the resize?
> if (ret)
> pr_err("%s:%d: failed to send METADATA_UPDATED msg\n",
> __func__, __LINE__);
> }
> unlock_comm(cinfo);
> + return ret;
> }
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 680b34a63cb3d..b723fd9eccc0b 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -8023,7 +8023,7 @@ static int update_size(struct mddev *mddev, sector_t num_sectors)
> rv = mddev->pers->resize(mddev, num_sectors);
> if (!rv) {
> if (mddev_is_clustered(mddev))
> - mddev->cluster_ops->update_size(mddev, old_dev_sectors);
> + rv = mddev->cluster_ops->update_size(mddev, old_dev_sectors);
[Severity: High]
If the cluster update_size() operation fails early (for example, if lock_comm
fails or it cannot find an active raid device), we now propagate the error
back to userspace.
However, the local array has already been resized by mddev->pers->resize()
right above this call. Does this local resize need to be reverted if the
cluster update fails, to prevent a split-brain state where the local node has
a different capacity than the rest of the cluster?
> else if (!mddev_is_dm(mddev))
> set_capacity_and_notify(mddev->gendisk,
> mddev->array_sectors);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260827061200.79753-1-ghuicao@163.com?part=2
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/3] md-cluster: fix ack_lockres leak in __sendmsg error path
2026-08-27 6:12 ` [PATCH v2 3/3] md-cluster: fix ack_lockres leak in __sendmsg error path ghuicao
@ 2026-08-27 6:24 ` sashiko-bot
0 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-08-27 6:24 UTC (permalink / raw)
To: ghuicao; +Cc: yukuai, linux-raid
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] The newly added error recovery logic silently fails if the original error was a timeout, leaving the lock leaked in EX state and defeating the patch's purpose.
Pre-existing issues:
- [High] An infinite loop and CPU soft lockup occur in the `failed_ack` cleanup path if a previous DLM lock operation timed out.
--
commit aed4dde7a766dd41cf7dc88c61e2ae9c8fd7a612
Author: Cao Guanghui <caoguanghui@kylinos.cn>
md-cluster: fix ack_lockres leak in __sendmsg error path
In __sendmsg(), if the down-conversion of ack_lockres from EX to CR fails, the
original code jumps to failed_ack and leaves ack_lockres in the EX state. This
commit attempts to fix this by adding a conversion of ack_lockres to CR in the
failed_ack path.
> diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
> index bc4118d8c5a9f..7c6aa759d0c23 100644
> --- a/drivers/md/md-cluster.c
> +++ b/drivers/md/md-cluster.c
> @@ -801,6 +801,8 @@ static int __sendmsg(struct md_cluster_info *cinfo, struct cluster_msg *cmsg)
> }
>
> failed_ack:
> + if (error)
> + dlm_lock_sync(cinfo->ack_lockres, DLM_LOCK_CR);
[Severity: High]
Will this error recovery logic silently fail if the original error was a
timeout?
If dlm_lock_sync() times out when acquiring DLM_LOCK_EX for ack_lockres
earlier in the function, it leaves a pending DLM lock request in the
background. Because of the uncancelled pending request, calling
dlm_lock_sync(cinfo->ack_lockres, DLM_LOCK_CR) here immediately fails with
-EBUSY and its return value is ignored.
When the background EX request eventually completes, the lock is left in the EX
state, which could still lead to a cluster-wide deadlock.
> while ((unlock_error = dlm_unlock_sync(cinfo->message_lockres)))
> pr_err("md-cluster: failed convert to NL on MESSAGE(%d)\n",
> unlock_error);
[Severity: High]
This is a pre-existing issue, but does this create an infinite loop and CPU
soft lockup if a previous DLM lock operation timed out?
If a timeout occurs during dlm_lock_sync(cinfo->message_lockres, DLM_LOCK_CW)
earlier in __sendmsg(), it leaves a pending lock conversion request in the DLM
lockspace.
Because of the pending request, dlm_unlock_sync() will immediately return
-EBUSY without blocking, causing this to become an infinite loop that floods
the kernel log and soft lockups the CPU core.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260827061200.79753-1-ghuicao@163.com?part=3
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/3] md-cluster: fix error handling and superblock update in update_size revert
2026-08-27 6:11 ` [PATCH v2 1/3] md-cluster: fix error handling and superblock update in update_size revert ghuicao
2026-08-27 6:11 ` [PATCH v2 2/3] md-cluster: propagate update_size() errors to callers ghuicao
2026-08-27 6:12 ` [PATCH v2 3/3] md-cluster: fix ack_lockres leak in __sendmsg error path ghuicao
@ 2026-08-27 6:24 ` sashiko-bot
2026-08-27 8:44 ` [PATCH v3 1/3] md-cluster: fix lock_comm leak and __sendmsg error path ghuicao
3 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-08-27 6:24 UTC (permalink / raw)
To: ghuicao; +Cc: yukuai, linux-raid
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Guaranteed self-deadlock and potential double-unlocks in `update_size` due to calling `md_update_sb()` while the cluster communication lock is held.
--
commit bae2db67348747b3864a225bb7b24be69937b7a2
Author: Cao Guanghui <caoguanghui@kylinos.cn>
md-cluster: fix error handling and superblock update in update_size revert
This commit addresses error handling and metadata updates when reverting an
array resize operation in a cluster. It checks the return value of resize()
and updates the superblock to match the in-memory array size.
> diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
> index 24dac6f09da82..9f404660a1fb7 100644
> --- a/drivers/md/md-cluster.c
> +++ b/drivers/md/md-cluster.c
> @@ -1349,6 +1349,10 @@ static void update_size(struct mddev *mddev, sector_t old_dev_sectors)
> } else {
> /* revert to previous sectors */
> ret = mddev->pers->resize(mddev, old_dev_sectors);
> + if (ret)
> + pr_err("%s:%d: failed to revert array size\n",
> + __func__, __LINE__);
> + md_update_sb(mddev, 1);
[Severity: High]
Does this call to md_update_sb() cause a self-deadlock?
At this point in update_size(), the cluster communication lock
(MD_CLUSTER_SEND_LOCK) is already held via lock_comm(). When md_update_sb()
is called, it enters mddev->cluster_ops->metadata_update_start(mddev), which
waits for MD_CLUSTER_SEND_LOCK to be cleared. Since the lock is held by the
same thread, this appears to result in a strict self-deadlock during a failed
array resize operation.
Additionally, if the deadlock could somehow be bypassed, md_update_sb() would
complete by calling metadata_update_finish(), which prematurely calls
unlock_comm(), leading to a fatal double-unlock when update_size() later
attempts to call unlock_comm() itself.
> ret = __sendmsg(cinfo, &cmsg);
> if (ret)
> pr_err("%s:%d: failed to send METADATA_UPDATED msg\n",
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260827061200.79753-1-ghuicao@163.com?part=1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 1/3] md-cluster: fix lock_comm leak and __sendmsg error path
2026-08-27 6:11 ` [PATCH v2 1/3] md-cluster: fix error handling and superblock update in update_size revert ghuicao
` (2 preceding siblings ...)
2026-08-27 6:24 ` [PATCH v2 1/3] md-cluster: fix error handling and superblock update in update_size revert sashiko-bot
@ 2026-08-27 8:44 ` ghuicao
2026-08-27 8:44 ` [PATCH v3 2/3] md-cluster: fix error handling and superblock consistency in update_size ghuicao
` (2 more replies)
3 siblings, 3 replies; 14+ messages in thread
From: ghuicao @ 2026-08-27 8:44 UTC (permalink / raw)
To: Song Liu
Cc: Yu Kuai, Li Nan, Xiao Ni, linux-raid, linux-kernel, stable,
Cao Guanghui
From: Cao Guanghui <caoguanghui@kylinos.cn>
Fix two error handling issues in cluster communication:
1. lock_comm() leaks MD_CLUSTER_SEND_LOCK if lock_token() fails.
The bit is set by test_and_set_bit but never cleared on error,
causing all subsequent cluster operations to hang permanently
in wait_event(). Clear the bit before returning the error.
2. __sendmsg() has two problems in the failed_ack cleanup path:
- ack_lockres is left in EX state if the down-conversion to CR
fails, causing a cluster-wide deadlock. Attempt to restore
it to CR and log if that also fails.
- The while loop for message_lockres unlock spins forever if
a previous DLM operation timed out and left a pending request
(dlm_unlock_sync returns -EBUSY immediately). Change to a
single attempt with error logging.
Fixes: 818da59f97d6 ("md-cluster: add the support for resize") (lock_comm)
Fixes: 601b515c5dcc ("Communication Framework: Sending functions") (__sendmsg)
Cc: stable@vger.kernel.org
Signed-off-by: Cao Guanghui <caoguanghui@kylinos.cn>
---
drivers/md/md-cluster.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -735,6 +735,8 @@ static int lock_comm(struct md_cluster_info *cinfo, bool mddev_locked)
wait_event(cinfo->wait,
!test_and_set_bit(MD_CLUSTER_SEND_LOCK, &cinfo->state));
rv = lock_token(cinfo);
+ if (rv)
+ clear_bit_unlock(MD_CLUSTER_SEND_LOCK, &cinfo->state);
if (set_bit)
clear_bit_unlock(MD_CLUSTER_HOLDING_MUTEX_FOR_RECVD, &cinfo->state);
return rv;
@@ -801,7 +803,15 @@ static int __sendmsg(struct md_cluster_info *cinfo, struct cluster_msg *cmsg)
}
failed_ack:
- while ((unlock_error = dlm_unlock_sync(cinfo->message_lockres)))
+ if (error) {
+ int ack_ret = dlm_lock_sync(cinfo->ack_lockres, DLM_LOCK_CR);
+
+ if (ack_ret)
+ pr_err("md-cluster: failed to restore ACK to CR (%d)\n",
+ ack_ret);
+ }
+ unlock_error = dlm_unlock_sync(cinfo->message_lockres);
+ if (unlock_error)
pr_err("md-cluster: failed convert to NL on MESSAGE(%d)\n",
unlock_error);
--
2.34.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 2/3] md-cluster: fix error handling and superblock consistency in update_size
2026-08-27 8:44 ` [PATCH v3 1/3] md-cluster: fix lock_comm leak and __sendmsg error path ghuicao
@ 2026-08-27 8:44 ` ghuicao
2026-08-27 8:58 ` sashiko-bot
2026-08-27 8:44 ` [PATCH v3 3/3] md-cluster: revert local resize and propagate cluster errors ghuicao
2026-08-27 8:58 ` [PATCH v3 1/3] md-cluster: fix lock_comm leak and __sendmsg error path sashiko-bot
2 siblings, 1 reply; 14+ messages in thread
From: ghuicao @ 2026-08-27 8:44 UTC (permalink / raw)
To: Song Liu
Cc: Yu Kuai, Li Nan, Xiao Ni, linux-raid, linux-kernel, stable,
Cao Guanghui
From: Cao Guanghui <caoguanghui@kylinos.cn>
update_size() has multiple issues in the revert path when
cluster_check_sync_size() detects that not all nodes have confirmed
the new size:
1. (pre-existing) The return value of resize() is immediately
overwritten by __sendmsg(), so a resize failure is silently lost.
2. (pre-existing) The on-disk superblock still retains the new size
from the earlier md_update_sb() call. Other nodes re-read it and
adopt the new size while the initiator runs with the reverted old
size.
3. (pre-existing) The function returns void, so callers cannot detect
failures.
Fix all of the above by:
- Using a separate variable for __sendmsg result so resize failure
is not overwritten
- Calling md_update_sb() after unlock_comm() to write the reverted
size back to disk. This must be outside the locked section because
md_update_sb() internally acquires MD_CLUSTER_SEND_LOCK via
metadata_update_start(), which would self-deadlock if already held.
- Changing return type from void to int with proper error codes
(-EIO for lock failure, -ENODEV for no device, ret for others)
Fixes: 818da59f97d6 ("md-cluster: add the support for resize")
Cc: stable@vger.kernel.org
Signed-off-by: Cao Guanghui <caoguanghui@kylinos.cn>
---
drivers/md/md-cluster.c | 28 +++++++++++++++++++++++++---
drivers/md/md-cluster.h | 2 +-
2 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
--- a/drivers/md/md-cluster.c
+++ b/drivers/md/md-cluster.c
@@ -1302,18 +1302,19 @@ static int cluster_check_sync_size(struct mddev *mddev)
* let other nodes to perform it. If one node can't update sync_size
* accordingly, we need to revert to previous value.
*/
-static void update_size(struct mddev *mddev, sector_t old_dev_sectors)
+static int update_size(struct mddev *mddev, sector_t old_dev_sectors)
{
struct md_cluster_info *cinfo = mddev->cluster_info;
+ bool reverted = false;
struct cluster_msg cmsg;
struct md_rdev *rdev;
- int ret = 0;
+ int ret = 0, msg_ret = 0;
int raid_slot = -1;
md_update_sb(mddev, 1);
if (lock_comm(cinfo, 1)) {
pr_err("%s: lock_comm failed\n", __func__);
- return;
+ return -EIO;
}
memset(&cmsg, 0, sizeof(cmsg));
@@ -1335,12 +1336,12 @@ static int update_size(struct mddev *mddev, sector_t old_dev_sectors)
pr_err("%s:%d: failed to send METADATA_UPDATED msg\n",
__func__, __LINE__);
unlock_comm(cinfo);
- return;
+ return ret;
}
} else {
pr_err("md-cluster: No good device id found to send\n");
unlock_comm(cinfo);
- return;
+ return -ENODEV;
}
/*
@@ -1359,12 +1360,28 @@ static int update_size(struct mddev *mddev, sector_t old_dev_sectors)
} else {
/* revert to previous sectors */
ret = mddev->pers->resize(mddev, old_dev_sectors);
- ret = __sendmsg(cinfo, &cmsg);
if (ret)
+ pr_err("%s:%d: failed to revert array size\n",
+ __func__, __LINE__);
+ reverted = true;
+ msg_ret = __sendmsg(cinfo, &cmsg);
+ if (msg_ret) {
pr_err("%s:%d: failed to send METADATA_UPDATED msg\n",
__func__, __LINE__);
+ if (!ret)
+ ret = msg_ret;
+ }
}
unlock_comm(cinfo);
+
+ if (reverted)
+ /* Update on-disk superblock to match reverted in-memory
+ * size. Must be after unlock_comm() to avoid self-deadlock
+ * since md_update_sb() acquires the cluster send lock.
+ */
+ md_update_sb(mddev, 1);
+
+ return ret;
}
static int resync_start(struct mddev *mddev)
diff --git a/drivers/md/md-cluster.h b/drivers/md/md-cluster.h
--- a/drivers/md/md-cluster.h
+++ b/drivers/md/md-cluster.h
@@ -34,7 +34,7 @@ struct md_cluster_operations {
int (*resize_bitmaps)(struct mddev *mddev, sector_t newsize, sector_t oldsize);
int (*lock_all_bitmaps)(struct mddev *mddev);
void (*unlock_all_bitmaps)(struct mddev *mddev);
- void (*update_size)(struct mddev *mddev, sector_t old_dev_sectors);
+ int (*update_size)(struct mddev *mddev, sector_t old_dev_sectors);
};
extern int md_setup_cluster(struct mddev *mddev, int nodes);
--
2.34.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 3/3] md-cluster: revert local resize and propagate cluster errors
2026-08-27 8:44 ` [PATCH v3 1/3] md-cluster: fix lock_comm leak and __sendmsg error path ghuicao
2026-08-27 8:44 ` [PATCH v3 2/3] md-cluster: fix error handling and superblock consistency in update_size ghuicao
@ 2026-08-27 8:44 ` ghuicao
2026-08-27 9:04 ` sashiko-bot
2026-08-27 8:58 ` [PATCH v3 1/3] md-cluster: fix lock_comm leak and __sendmsg error path sashiko-bot
2 siblings, 1 reply; 14+ messages in thread
From: ghuicao @ 2026-08-27 8:44 UTC (permalink / raw)
To: Song Liu
Cc: Yu Kuai, Li Nan, Xiao Ni, linux-raid, linux-kernel, stable,
Cao Guanghui
From: Cao Guanghui <caoguanghui@kylinos.cn>
In md.c:update_size(), the local array is resized via pers->resize()
before calling the cluster update_size() callback. If the cluster
operation fails, the local node has the new size while the rest of
the cluster does not, causing a split-brain state.
Propagate the cluster update_size() return value and revert the local
resize on failure. Also check the return value in the reshape
completion path (md_reap_sync_thread) and log a warning on failure.
Fixes: 818da59f97d6 ("md-cluster: add the support for resize")
Cc: stable@vger.kernel.org
Signed-off-by: Cao Guanghui <caoguanghui@kylinos.cn>
---
drivers/md/md.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -8022,9 +8022,11 @@ static int update_size(struct mddev *mddev, sector_t num_sectors)
}
rv = mddev->pers->resize(mddev, num_sectors);
if (!rv) {
- if (mddev_is_clustered(mddev))
- mddev->cluster_ops->update_size(mddev, old_dev_sectors);
- else if (!mddev_is_dm(mddev))
+ if (mddev_is_clustered(mddev)) {
+ rv = mddev->cluster_ops->update_size(mddev, old_dev_sectors);
+ if (rv)
+ mddev->pers->resize(mddev, old_dev_sectors);
+ } else if (!mddev_is_dm(mddev))
set_capacity_and_notify(mddev->gendisk,
mddev->array_sectors);
}
@@ -10615,8 +10617,12 @@ void md_reap_sync_thread(struct mddev *mddev)
*/
if (mddev_is_clustered(mddev) && is_reshaped &&
mddev->pers->finish_reshape &&
- !test_bit(MD_CLOSING, &mddev->flags))
- mddev->cluster_ops->update_size(mddev, old_dev_sectors);
+ !test_bit(MD_CLOSING, &mddev->flags)) {
+ int ret = mddev->cluster_ops->update_size(mddev, old_dev_sectors);
+
+ if (ret)
+ pr_warn("md: cluster update_size failed after reshape: %d\n", ret);
+ }
/* flag recovery needed just to double check */
set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
sysfs_notify_dirent_safe(mddev->sysfs_completed);
--
2.34.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/3] md-cluster: fix lock_comm leak and __sendmsg error path
2026-08-27 8:44 ` [PATCH v3 1/3] md-cluster: fix lock_comm leak and __sendmsg error path ghuicao
2026-08-27 8:44 ` [PATCH v3 2/3] md-cluster: fix error handling and superblock consistency in update_size ghuicao
2026-08-27 8:44 ` [PATCH v3 3/3] md-cluster: revert local resize and propagate cluster errors ghuicao
@ 2026-08-27 8:58 ` sashiko-bot
2 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-08-27 8:58 UTC (permalink / raw)
To: ghuicao; +Cc: yukuai, linux-raid
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [High] Missing `wake_up(&cinfo->wait)` after clearing `MD_CLUSTER_SEND_LOCK` on the error path in `lock_comm()`.
- [High] The single-attempt lock release strategy leaves the DLM lock permanently stuck when there is a pending timed-out conversion.
- [Medium] The fallback restoration of `ack_lockres` to `CR` incorrectly executes on all error paths, leading to redundant or synchronously failing requests.
Pre-existing issues:
- [High] `metadata_update_start()` suffers from the exact same lock leak pattern that the patch attempts to fix in `lock_comm()`.
--
commit ab42e35df8bfdb28c9d4472b6f7586349538824c
Author: Cao Guanghui <caoguanghui@kylinos.cn>
md-cluster: fix lock_comm leak and __sendmsg error path
This commit addresses error handling issues in cluster communication. It clears
MD_CLUSTER_SEND_LOCK when lock_token() fails in lock_comm() to avoid hangs.
It also modifies __sendmsg() to attempt restoring ack_lockres to CR and
replaces an infinite loop with a single attempt when unlocking message_lockres.
> diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
> index 24dac6f09da82..fb62fdb492350 100644
> --- a/drivers/md/md-cluster.c
> +++ b/drivers/md/md-cluster.c
> @@ -735,6 +735,8 @@ static int lock_comm(struct md_cluster_info *cinfo, bool mddev_locked)
> wait_event(cinfo->wait,
> !test_and_set_bit(MD_CLUSTER_SEND_LOCK, &cinfo->state));
> rv = lock_token(cinfo);
> + if (rv)
> + clear_bit_unlock(MD_CLUSTER_SEND_LOCK, &cinfo->state);
[Severity: High]
Does this clear operation also require a wake_up(&cinfo->wait)?
When lock_token() fails, this patch clears the lock bit but appears to miss
the wake_up() call. Any thread currently asleep in wait_event() waiting to
acquire MD_CLUSTER_SEND_LOCK could remain asleep indefinitely because
waitqueues require an explicit wakeup.
> if (set_bit)
> clear_bit_unlock(MD_CLUSTER_HOLDING_MUTEX_FOR_RECVD, &cinfo->state);
> return rv;
[ ... ]
> @@ -801,7 +803,15 @@ static int __sendmsg(struct md_cluster_info *cinfo, struct cluster_msg *cmsg)
> }
>
> failed_ack:
> - while ((unlock_error = dlm_unlock_sync(cinfo->message_lockres)))
> + if (error) {
> + int ack_ret = dlm_lock_sync(cinfo->ack_lockres, DLM_LOCK_CR);
[Severity: Medium]
Will this unconditional fallback restoration fail synchronously?
If __sendmsg() fails on message_lockres, ack_lockres is still in CR, making
this conversion redundant. However, if it fails on ack_lockres due to a
timeout, the DLM resource is already busy with a pending request. The
immediate dlm_lock_sync() fallback to CR will fail synchronously with -EBUSY
and fail to recover the lock state.
> +
> + if (ack_ret)
> + pr_err("md-cluster: failed to restore ACK to CR (%d)\n",
> + ack_ret);
> + }
> + unlock_error = dlm_unlock_sync(cinfo->message_lockres);
> + if (unlock_error)
> pr_err("md-cluster: failed convert to NL on MESSAGE(%d)\n",
> unlock_error);
[Severity: High]
Could this single-attempt lock release strategy leave the DLM lock stuck?
If dlm_lock_sync() times out (for example, when getting EX lock on MESSAGE),
it returns -EBUSY but leaves the lock request active in DLM. This single
dlm_unlock_sync() attempt translates to a new DLM lock request which is
immediately rejected with -EBUSY by DLM due to the pending request.
Proper recovery might require using dlm_unlock() with DLM_LKF_CANCEL to
cancel the pending operation.
[Severity: High]
This isn't a bug introduced by this patch, but metadata_update_start()
appears to suffer from the exact same lock leak pattern that this patch
attempts to fix in lock_comm().
Looking at metadata_update_start() in drivers/md/md-cluster.c:
ret = lock_token(cinfo);
clear_bit_unlock(MD_CLUSTER_HOLDING_MUTEX_FOR_RECVD, &cinfo->state);
return ret;
If lock_token() fails here, it returns without clearing MD_CLUSTER_SEND_LOCK
and without calling wake_up(&cinfo->wait), which could cause a permanent
cluster-wide hang during metadata updates.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260827084453.124629-1-ghuicao@163.com?part=1
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/3] md-cluster: fix error handling and superblock consistency in update_size
2026-08-27 8:44 ` [PATCH v3 2/3] md-cluster: fix error handling and superblock consistency in update_size ghuicao
@ 2026-08-27 8:58 ` sashiko-bot
0 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-08-27 8:58 UTC (permalink / raw)
To: ghuicao; +Cc: yukuai, linux-raid
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] When the cluster rejects a resize and the local revert succeeds, `update_size()` erroneously returns 0 (success), misleading userspace.
- [High] If the local revert resize fails after the cluster rejects the new size, the code unconditionally updates the on-disk superblock with the rejected new size.
Pre-existing issues:
- [High] Early failure paths in `update_size()` commit the new size to the on-disk superblock but fail to revert it on error, causing persistent cluster split-brain.
--
commit faab74a4740b5da9226b76ad0d7a819261f23143
Author: Cao Guanghui <caoguanghui@kylinos.cn>
Subject: md-cluster: fix error handling and superblock consistency in update_size
This commit attempts to fix error handling in update_size() by returning
proper error codes and ensuring the on-disk superblock is reverted if
other nodes in the cluster reject the size update.
> diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
> index fb62fdb492350..0ba25053198d5 100644
> --- a/drivers/md/md-cluster.c
> +++ b/drivers/md/md-cluster.c
> @@ -1302,18 +1302,19 @@ static int cluster_check_sync_size(struct mddev *mddev)
[ ... ]
> -static void update_size(struct mddev *mddev, sector_t old_dev_sectors)
> +static int update_size(struct mddev *mddev, sector_t old_dev_sectors)
> {
> struct md_cluster_info *cinfo = mddev->cluster_info;
> + bool reverted = false;
> struct cluster_msg cmsg;
> struct md_rdev *rdev;
> - int ret = 0;
> + int ret = 0, msg_ret = 0;
> int raid_slot = -1;
>
> md_update_sb(mddev, 1);
> if (lock_comm(cinfo, 1)) {
> pr_err("%s: lock_comm failed\n", __func__);
> - return;
> + return -EIO;
> }
[Severity: High]
This is a pre-existing issue, but since md_update_sb() has already
written the new array size to the on-disk superblock, does this early
error path need to revert that change?
If the cluster operation fails early here, or in the subsequent
__sendmsg() calls, the cluster is never notified of the size change but
the local node has already committed it to disk. On the next reboot,
this node might assemble the array with the new size while others use
the old size, causing a persistent cluster split-brain.
[ ... ]
> @@ -1359,12 +1360,28 @@ static void update_size(struct mddev *mddev, sector_t old_dev_sectors)
> } else {
> /* revert to previous sectors */
> ret = mddev->pers->resize(mddev, old_dev_sectors);
[Severity: High]
When the cluster rejects a resize and the code enters this else block to
revert the size, does a successful local revert (where
mddev->pers->resize() returns 0) cause the function to return 0?
Returning 0 incorrectly signals a successful resize to the caller. This
might mislead userspace tools and administrators into assuming the
storage capacity has increased, when it was actually rejected by the
cluster and rolled back locally.
> - ret = __sendmsg(cinfo, &cmsg);
> if (ret)
> + pr_err("%s:%d: failed to revert array size\n",
> + __func__, __LINE__);
> + reverted = true;
[Severity: High]
If the local revert resize fails (returning a non-zero error), the
reverted flag is still unconditionally set to true.
Because the reverted flag is true, md_update_sb() will be called at the
end of the function. Will this cause the local node to permanently store
the rejected new size in its on-disk superblock, leading to a cluster
split-brain?
> + msg_ret = __sendmsg(cinfo, &cmsg);
> + if (msg_ret) {
> pr_err("%s:%d: failed to send METADATA_UPDATED msg\n",
> __func__, __LINE__);
> + if (!ret)
> + ret = msg_ret;
> + }
> }
> unlock_comm(cinfo);
> +
> + if (reverted)
> + /* Update on-disk superblock to match reverted in-memory
> + * size. Must be after unlock_comm() to avoid self-deadlock
> + * since md_update_sb() acquires the cluster send lock.
> + */
> + md_update_sb(mddev, 1);
> +
> + return ret;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260827084453.124629-1-ghuicao@163.com?part=2
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 3/3] md-cluster: revert local resize and propagate cluster errors
2026-08-27 8:44 ` [PATCH v3 3/3] md-cluster: revert local resize and propagate cluster errors ghuicao
@ 2026-08-27 9:04 ` sashiko-bot
0 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-08-27 9:04 UTC (permalink / raw)
To: ghuicao; +Cc: linux-raid, yukuai
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [High] Reverting the local array resize on cluster failure leaves the on-disk superblock inconsistent with the in-memory state.
- [High] Missing revert of block device capacity when cluster broadcast fails creates a split-brain state between the block layer and the MD layer.
- [High] The return value of the revert call to `pers->resize()` is ignored, which can hide a failed revert and leave the array in an unexpected state.
Pre-existing issues:
- [High] The patch fails to propagate the error when the cluster resize is rejected by other nodes, resulting in a silent failure reported as success to user-space.
--
commit cc96df1db446b526c4a40bb6ed18c53c2fa44fb6
Author: Cao Guanghui <caoguanghui@kylinos.cn>
md-cluster: revert local resize and propagate cluster errors
This patch attempts to fix a split-brain state during an array resize by
propagating the return value of the cluster update_size() operation and
reverting the local resize if the cluster operation fails. It also logs
a warning if the cluster update fails during the reshape completion path.
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 680b34a63cb3d..10b9880e64580 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -8022,9 +8022,11 @@ static int update_size(struct mddev *mddev, sector_t num_sectors)
> }
> rv = mddev->pers->resize(mddev, num_sectors);
> if (!rv) {
> - if (mddev_is_clustered(mddev))
> - mddev->cluster_ops->update_size(mddev, old_dev_sectors);
> - else if (!mddev_is_dm(mddev))
> + if (mddev_is_clustered(mddev)) {
> + rv = mddev->cluster_ops->update_size(mddev, old_dev_sectors);
[Severity: High]
This is a pre-existing issue, but does mddev->cluster_ops->update_size()
correctly propagate errors?
If cluster_check_sync_size() returns an error, md-cluster.c:update_size()
executes a fallback path that overwrites the error state with the result of
the revert:
drivers/md/md-cluster.c:update_size() {
...
} else {
/* revert to previous sectors */
ret = mddev->pers->resize(mddev, old_dev_sectors);
if (ret)
pr_err("%s:%d: failed to revert array size\n",
__func__, __LINE__);
reverted = true;
...
}
If the local revert succeeds, ret becomes 0, silencing the error from
cluster_check_sync_size(). When this code receives rv = 0, it will
incorrectly assume the cluster update succeeded, bypassing the new revert
logic and reporting a false success to user-space.
> + if (rv)
[Severity: High]
The new revert logic handles the in-memory array size, but misses correcting
the on-disk superblock if cluster communication fails.
In md-cluster.c:update_size(), md_update_sb() synchronously writes the new
size to disk. If lock_comm() fails, it returns -EIO without reverting:
drivers/md/md-cluster.c:update_size() {
md_update_sb(mddev, 1);
if (lock_comm(cinfo, 1)) {
pr_err("%s: lock_comm failed\n", __func__);
return -EIO;
}
...
}
If cluster_ops->update_size() returns -EIO, we revert the internal MD array
size here, but shouldn't we also call md_update_sb(mddev, 1) to avoid
leaving the array in a split-brain state with a corrupted on-disk
superblock?
[Severity: High]
Does this sequence cause a split-brain state between the block layer and the
MD layer?
If the cluster broadcast fails right after checking the sync size,
md-cluster.c:update_size() unconditionally updates the block capacity before
returning the error:
drivers/md/md-cluster.c:update_size() {
...
if (cluster_check_sync_size(mddev) == 0) {
memset(&cmsg, 0, sizeof(cmsg));
cmsg.type = cpu_to_le32(CHANGE_CAPACITY);
ret = __sendmsg(cinfo, &cmsg);
if (ret)
pr_err("%s:%d: failed to send CHANGE_CAPACITY msg\n",
__func__, __LINE__);
set_capacity_and_notify(mddev->gendisk, mddev->array_sectors);
}
...
}
This code catches the __sendmsg() error and reverts the MD layer capacity
via mddev->pers->resize(), but there is no corresponding call to revert the
block layer capacity. Could this lead to out-of-bounds IOs if the block
layer exposes more capacity than the MD array limits?
> + mddev->pers->resize(mddev, old_dev_sectors);
[Severity: High]
Is it safe to ignore the return value of the revert operation here?
If mddev->pers->resize() fails (e.g., raid1_resize() returning -ENOMEM due
to bitmap allocation failure), the MD array will silently remain stuck at
the new size internally while user-space receives an error code. Shouldn't
we check the return value to handle a failed revert properly?
> + } else if (!mddev_is_dm(mddev))
> set_capacity_and_notify(mddev->gendisk,
> mddev->array_sectors);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260827084453.124629-1-ghuicao@163.com?part=3
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2026-08-27 9:05 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-27 2:52 [PATCH] md-cluster: check pers->resize() return value in update_size() ghuicao
2026-08-27 3:07 ` sashiko-bot
2026-08-27 6:11 ` [PATCH v2 1/3] md-cluster: fix error handling and superblock update in update_size revert ghuicao
2026-08-27 6:11 ` [PATCH v2 2/3] md-cluster: propagate update_size() errors to callers ghuicao
2026-08-27 6:23 ` sashiko-bot
2026-08-27 6:12 ` [PATCH v2 3/3] md-cluster: fix ack_lockres leak in __sendmsg error path ghuicao
2026-08-27 6:24 ` sashiko-bot
2026-08-27 6:24 ` [PATCH v2 1/3] md-cluster: fix error handling and superblock update in update_size revert sashiko-bot
2026-08-27 8:44 ` [PATCH v3 1/3] md-cluster: fix lock_comm leak and __sendmsg error path ghuicao
2026-08-27 8:44 ` [PATCH v3 2/3] md-cluster: fix error handling and superblock consistency in update_size ghuicao
2026-08-27 8:58 ` sashiko-bot
2026-08-27 8:44 ` [PATCH v3 3/3] md-cluster: revert local resize and propagate cluster errors ghuicao
2026-08-27 9:04 ` sashiko-bot
2026-08-27 8:58 ` [PATCH v3 1/3] md-cluster: fix lock_comm leak and __sendmsg error path sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox