Re: [PATCH] md: prevent incoreect update of resync/recovery offset

[Paul Menzel]

Dear Nan,


Thank you for your patch. I have some formal comments. In the 
summary/title: incor*r*ect.

Am 30.08.25 um 11:02 schrieb linan666@xxxxxxxxxxxxxxx:
> From: Li Nan <linan122@xxxxxxxxxx>
>
> In md_do_sync(), when md_sync_action returns ACTION_FROZEN, subsequent
> call to md_sync_position() will return MaxSector. This causes
> 'curr_resync' (and later 'recovery_offset') to be set to MaxSector too,
> which incorrectly signals that recovery/resync has completed even though
> disk data has not actually been updated.
>
> To fix this issue, skip updating any offset values when the sync acion

ac*t*ion

> is either FROZEN or IDLE.

Maybe state that the same holds true for IDLE?

Should these two cases be handled differently in `md_sync_position()`. 
Does it semantically make sense, that the default of MaxSector is returned?

> Fixes: 7d9f107a4e94 ("md: use new helpers in md_do_sync()")
> Signed-off-by: Li Nan <linan122@xxxxxxxxxx>
> ---
>   drivers/md/md.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index e78f80d39271..6828a569e819 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -9397,6 +9397,9 @@ void md_do_sync(struct md_thread *thread)
>   	}
>   
>   	action = md_sync_action(mddev);
> +	if (action == ACTION_FROZEN || action == ACTION_IDLE)
> +		goto skip;
> +
>   	desc = md_sync_action_name(action);
>   	mddev->last_sync_action = action;
>   


Kind regards,

Paul