All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] md: allow growing logical_block_size on a running array
@ 2026-10-14 11:48 Jack Wang
  2026-10-14 12:00 ` sashiko-bot
  0 siblings, 1 reply; 3+ messages in thread
From: Jack Wang @ 2026-10-14 11:48 UTC (permalink / raw)
  To: Song Liu, Yu Kuai
  Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel, Jack Wang,
	Florian-Ewald Mueller

From: Jack Wang <jinpu.wang@cloud.ionos.com>

Growing logical_block_size on a running array required stopping it
first. Handle it directly in lbs_store(), similar to
mddev_set_closing_and_sync_blockdev(): under open_mutex, refuse with
-EBUSY if the array is closing or has openers, otherwise apply the
bigger LBS.

mddev_lock() is not taken on this path, to avoid holding reconfig_mutex
while blocking on the queue limits lock.

md_release() drops openers without open_mutex, so a bio of a just-closed
opener may not yet be accounted in active_io. Commit the limits with
queue_limits_commit_update_frozen() to drain it. Also take open_mutex
around the existing "enable LBS" branch so it cannot race with the grow
path on mddev->logical_block_size.

Suggested-by: Florian-Ewald Mueller <florian-ewald.mueller@ionos.com>
Signed-off-by: Jack Wang <jinpu.wang@cloud.ionos.com>
---
 drivers/md/md.c | 40 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 35 insertions(+), 5 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 03e35386a802..cd9c73f46786 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -6013,12 +6013,12 @@ static int mddev_set_logical_block_size(struct mddev *mddev,
 	lim.logical_block_size = lbs;
 	pr_info("%s: logical_block_size is changed, data may be lost\n",
 		mdname(mddev));
-	err = queue_limits_commit_update(mddev->gendisk->queue, &lim);
+	err = queue_limits_commit_update_frozen(mddev->gendisk->queue, &lim);
 	if (err)
 		return err;
 
 	mddev->logical_block_size = lbs;
-	/* New lbs will be written to superblock after array is running */
+	/* New lbs will be written to superblock on the next update */
 	set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
 	return 0;
 }
@@ -6046,8 +6046,36 @@ lbs_store(struct mddev *mddev, const char *buf, size_t len)
 	if (mddev->pers) {
 		unsigned int curr_lbs;
 
-		if (mddev->logical_block_size)
+		/*
+		 * open_mutex serialises both paths below on
+		 * mddev->logical_block_size and keeps new openers out.
+		 * MD_CLOSING means a stop is under way. mddev_lock() is not
+		 * taken, to avoid holding reconfig_mutex while blocking on
+		 * the queue limits lock.
+		 */
+		mutex_lock(&mddev->open_mutex);
+		curr_lbs = queue_logical_block_size(mddev->gendisk->queue);
+		if (lbs > curr_lbs) {
+			/*
+			 * A bigger LBS can be applied to a running array if
+			 * nobody has it open, as there is no I/O through the
+			 * MD gendisk queue without openers. The queue is
+			 * still frozen for the limits update to drain a bio
+			 * of an opener that has just closed.
+			 */
+			if (test_bit(MD_CLOSING, &mddev->flags) ||
+			    atomic_read(&mddev->openers) > 0)
+				err = -EBUSY;
+			else
+				err = mddev_set_logical_block_size(mddev, lbs);
+			mutex_unlock(&mddev->open_mutex);
+			return err ?: len;
+		}
+
+		if (mddev->logical_block_size) {
+			mutex_unlock(&mddev->open_mutex);
 			return -EBUSY;
+		}
 		/*
 		 * To fix forward compatibility issues, LBS is not
 		 * configured for arrays from old kernels (<=6.18) by default.
@@ -6055,12 +6083,14 @@ lbs_store(struct mddev *mddev, const char *buf, size_t len)
 		 * enable LBS by writing current LBS — to prevent data
 		 * loss from LBS changes.
 		 */
-		curr_lbs = queue_logical_block_size(mddev->gendisk->queue);
-		if (lbs != curr_lbs)
+		if (lbs != curr_lbs) {
+			mutex_unlock(&mddev->open_mutex);
 			return -EINVAL;
+		}
 
 		mddev->logical_block_size = curr_lbs;
 		set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
+		mutex_unlock(&mddev->open_mutex);
 		pr_info("%s: logical block size configured successfully, array will not be assembled in old kernels (<= 6.18)\n",
 			mdname(mddev));
 		return len;
-- 
2.43.0


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

* Re: [PATCH] md: allow growing logical_block_size on a running array
  2026-10-14 11:48 [PATCH] md: allow growing logical_block_size on a running array Jack Wang
@ 2026-10-14 12:00 ` sashiko-bot
  2026-10-14 12:33   ` Jinpu Wang
  0 siblings, 1 reply; 3+ messages in thread
From: sashiko-bot @ 2026-10-14 12:00 UTC (permalink / raw)
  To: Jack Wang; +Cc: linux-raid, yukuai

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] md: Unbalanced mutex unlock on mddev_lock() failure in lbs_store()
- [High] md: TOCTOU race in lbs_store() bypasses opener check and deadlocks via queue freeze
--

commit 4267b7426d911d22ecfdc4aa203d879ce4f9a759
Author: Jack Wang <jinpu.wang@cloud.ionos.com>

    md: allow growing logical_block_size on a running array
    
    Growing logical_block_size on a running array required stopping it
    first. Handle it directly in lbs_store(), similar to
    mddev_set_closing_and_sync_blockdev(): under open_mutex, refuse with
    -EBUSY if the array is closing or has openers, otherwise apply the
    bigger LBS.

> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 680b34a63cb3d..c885d405bf466 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c

[ ... ]

> @@ -6026,8 +6026,36 @@ lbs_store(struct mddev *mddev, const char *buf, size_t len)
>  	if (mddev->pers) {

[Severity: High]
Since this check of mddev->pers is performed locklessly, can a concurrent
thread starting the array race with it?

If mddev->pers is null, we fall through to the bottom path and block on
mddev_lock(mddev). Concurrently, another thread executing do_md_run() could
acquire reconfig_mutex, set mddev->pers to non-null, start the array, and
release the lock.

When lbs_store() wakes up and acquires reconfig_mutex, it does not re-verify
mddev->pers or check atomic_read(&mddev->openers) > 0 before calling
mddev_set_logical_block_size():

drivers/md/md.c:lbs_store() {
    ...
	if (mddev->pers) {
        ...
		return len;
	}

	err = mddev_lock(mddev);
	if (err)
		goto unlock;

	err = mddev_set_logical_block_size(mddev, lbs);
    ...
}

Does this blindly apply a new logical block size to a running array with
active openers? Furthermore, since mddev_set_logical_block_size() invokes
queue_limits_commit_update_frozen() which waits for all active I/O to drain
while holding reconfig_mutex, could this deadlock if active I/O completion
requires reconfig_mutex?

[Severity: High]
If mddev_lock() fails (e.g., interrupted by a signal, returning -EINTR),
does it jump to an unbalanced unlock?

drivers/md/md.c:lbs_store() {
    ...
	err = mddev_lock(mddev);
	if (err)
		goto unlock;

	err = mddev_set_logical_block_size(mddev, lbs);

unlock:
	mddev_unlock(mddev);
	return err ?: len;
}

Because mddev_lock() failed, reconfig_mutex was not acquired, but the unlock
label unconditionally calls mddev_unlock(mddev) to release it. Could this
attempt to unlock an unheld mutex and trigger a lockdep splat or kernel
panic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014114830.615589-1-jinpu.wang@ionos.com?part=1

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

* Re: [PATCH] md: allow growing logical_block_size on a running array
  2026-10-14 12:00 ` sashiko-bot
@ 2026-10-14 12:33   ` Jinpu Wang
  0 siblings, 0 replies; 3+ messages in thread
From: Jinpu Wang @ 2026-10-14 12:33 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: linux-raid, yukuai

On Wed, Oct 14, 2026 at 2:00 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] md: Unbalanced mutex unlock on mddev_lock() failure in lbs_store()
This one is fixed in
https://lore.kernel.org/linux-raid/20261013141241.2257621-1-yukuai@kernel.org/T/#u
> - [High] md: TOCTOU race in lbs_store() bypasses opener check and deadlocks via queue freeze
Second is valid, seems no simple solution, safer to lbs change when
array is inactive. I will drop the patch
> --
>
> commit 4267b7426d911d22ecfdc4aa203d879ce4f9a759
> Author: Jack Wang <jinpu.wang@cloud.ionos.com>
>
>     md: allow growing logical_block_size on a running array
>
>     Growing logical_block_size on a running array required stopping it
>     first. Handle it directly in lbs_store(), similar to
>     mddev_set_closing_and_sync_blockdev(): under open_mutex, refuse with
>     -EBUSY if the array is closing or has openers, otherwise apply the
>     bigger LBS.
>
> > diff --git a/drivers/md/md.c b/drivers/md/md.c
> > index 680b34a63cb3d..c885d405bf466 100644
> > --- a/drivers/md/md.c
> > +++ b/drivers/md/md.c
>
> [ ... ]
>
> > @@ -6026,8 +6026,36 @@ lbs_store(struct mddev *mddev, const char *buf, size_t len)
> >       if (mddev->pers) {
>
> [Severity: High]
> Since this check of mddev->pers is performed locklessly, can a concurrent
> thread starting the array race with it?
>
> If mddev->pers is null, we fall through to the bottom path and block on
> mddev_lock(mddev). Concurrently, another thread executing do_md_run() could
> acquire reconfig_mutex, set mddev->pers to non-null, start the array, and
> release the lock.
>
> When lbs_store() wakes up and acquires reconfig_mutex, it does not re-verify
> mddev->pers or check atomic_read(&mddev->openers) > 0 before calling
> mddev_set_logical_block_size():
>
> drivers/md/md.c:lbs_store() {
>     ...
>         if (mddev->pers) {
>         ...
>                 return len;
>         }
>
>         err = mddev_lock(mddev);
>         if (err)
>                 goto unlock;
>
>         err = mddev_set_logical_block_size(mddev, lbs);
>     ...
> }
>
> Does this blindly apply a new logical block size to a running array with
> active openers? Furthermore, since mddev_set_logical_block_size() invokes
> queue_limits_commit_update_frozen() which waits for all active I/O to drain
> while holding reconfig_mutex, could this deadlock if active I/O completion
> requires reconfig_mutex?
>
> [Severity: High]
> If mddev_lock() fails (e.g., interrupted by a signal, returning -EINTR),
> does it jump to an unbalanced unlock?
>
> drivers/md/md.c:lbs_store() {
>     ...
>         err = mddev_lock(mddev);
>         if (err)
>                 goto unlock;
>
>         err = mddev_set_logical_block_size(mddev, lbs);
>
> unlock:
>         mddev_unlock(mddev);
>         return err ?: len;
> }
>
> Because mddev_lock() failed, reconfig_mutex was not acquired, but the unlock
> label unconditionally calls mddev_unlock(mddev) to release it. Could this
> attempt to unlock an unheld mutex and trigger a lockdep splat or kernel
> panic?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261014114830.615589-1-jinpu.wang@ionos.com?part=1

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

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

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-14 11:48 [PATCH] md: allow growing logical_block_size on a running array Jack Wang
2026-10-14 12:00 ` sashiko-bot
2026-10-14 12:33   ` Jinpu Wang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.