Re: [PATCH v4 1/3] md/raid5: freeze and interrupt reshape when handling failed reshape

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] md: stale did_freeze clears MD_RECOVERY_FROZEN requested by failed reshape
--

commit 0538f5adec4b4e67f586325e5d4064322224a7a9
Author: Doug V Johnson <me@dougvj.me>

md/raid5: freeze and interrupt reshape when handling failed reshape

This commit adds a freeze flag to stop reshape operations when an
unrecoverable read occurs during a failed reshape. It prevents automatic
reshaping resumption which otherwise leads to block writes being cancelled
while the reshape continues, corrupting data.

Link: https://github.com/dougvj/md-raid-reshape-badblock-corruption-test
Link: https://patch.msgid.link/20260624075824.2601110-1-chencheng@fnnas.com

> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index b91545ce090de..86ea65559576a 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -3773,6 +3773,10 @@ static void handle_failed_reshape(struct r5conf *conf, struct stripe_head *sh,
>  	bool was_expanding = test_and_clear_bit(STRIPE_EXPANDING, &sh->state);
>  	bool was_ready = test_and_clear_bit(STRIPE_EXPAND_READY, &sh->state);
>  
> +	/* Freeze reshaping to prevent data loss */
> +	set_bit(MD_RECOVERY_FROZEN, &conf->mddev->recovery);

[Severity: High]
Does setting the frozen state locklessly from the asynchronous worker thread
race with other configuration operations that clear it based on a stale
local variable?

If a user initiates an array teardown or read-only transition via sysfs
concurrently with a failing reshape:

1. do_md_stop() or md_set_readonly() observe MD_RECOVERY_FROZEN is clear,
   set it, and cache did_freeze = 1 locally.

2. They call stop_sync_thread() which intentionally drops
   mddev->reconfig_mutex while waiting.

3. Concurrently, the asynchronous raid5d worker executes this new patch
   logic in handle_failed_reshape() and sets MD_RECOVERY_FROZEN to prevent
   the reshape from resuming.

4. stop_sync_thread() returns and reacquires mddev->reconfig_mutex.

5. In error paths of do_md_stop() (or unconditionally in
   md_set_readonly()), the stale did_freeze variable is evaluated and
   MD_RECOVERY_FROZEN is cleared:

drivers/md/md.c:do_md_stop() {
    ...
    if (did_freeze) {
        clear_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
        set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
    }
    ...
}

Will this unconditionally clear the critical freeze requested by the worker
thread, causing MD_RECOVERY_NEEDED to be set and the array to automatically
resume the failed reshape, leading to data corruption?

> +	md_sync_error(conf->mddev);
> +
>  	if (was_expanding || was_ready) {
>  		atomic_dec(&conf->reshape_stripes);
>  		wake_up(&conf->wait_for_reshape);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/010101a142f57e37-cd14b78d-54cd-447b-9888-87f166a937ab-000000@us-west-2.amazonses.com?part=1