1. Patchew
  2. linux
  3. View series

[PATCH] md-cluster: check pers->resize() return value in update_size()

ghuicao@163.com posted 1 patch 1 month ago 
drivers/md/md-cluster.c | 2 ++
1 file changed, 2 insertions(+)
[PATCH] md-cluster: check pers->resize() return value in update_size()
Posted by ghuicao@163.com 1 month ago 
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
[PATCH v2 1/3] md-cluster: fix error handling and superblock update in update_size revert
Posted by ghuicao@163.com 1 month ago 
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
[PATCH v3 1/3] md-cluster: fix lock_comm leak and __sendmsg error path
Posted by ghuicao@163.com 1 month ago 
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
[PATCH v3 2/3] md-cluster: fix error handling and superblock consistency in update_size
Posted by ghuicao@163.com 1 month ago 
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
[PATCH v3 3/3] md-cluster: revert local resize and propagate cluster errors
Posted by ghuicao@163.com 1 month ago 
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
[PATCH v2 2/3] md-cluster: propagate update_size() errors to callers
Posted by ghuicao@163.com 1 month ago 
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
[PATCH v2 3/3] md-cluster: fix ack_lockres leak in __sendmsg error path
Posted by ghuicao@163.com 1 month ago 
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

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.