Re: [PATCH v3 14/21] md/md-llbitmap: clamp state-machine walks to tracked bits

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] `llbitmap_cond_end_sync` prematurely clears the sync state for partially synced chunks, leading to skipped sectors and data corruption.
--

--- Patch [14]: [PATCH v3 14/21] md/md-llbitmap: clamp state-machine walks to tracked bits ---
commit d30fecfeaabb2957753288e01ac48b29a04d716a
Author: Yu Kuai <[email protected]>

md/md-llbitmap: clamp state-machine walks to tracked bits

This commit clamps the state-machine range to llbitmap->chunks to
prevent it from walking past the tracked bitmap when handling boundary
conditions.

> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index 794ac0eaf4379..bb83d15247d23 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
> @@ -1011,7 +1011,10 @@ static enum llbitmap_state llbitmap_state_machine(struct llbitmap *llbitmap,
>  		llbitmap_init_state(llbitmap);
>  		return BitNone;
>  	}
> -
> +	if (start >= llbitmap->chunks)
> +		return BitNone;
> +	if (end >= llbitmap->chunks)
> +		end = llbitmap->chunks - 1;

[Severity: Critical]
This is a pre-existing issue, but does llbitmap_cond_end_sync() prematurely
clear the sync state for partially synced chunks?

During array resync or initial build, the current resync position (sector)
typically advances by stripe size or resync sectors, which is smaller than
the bitmap chunk size. This means sector almost always falls in the middle of
a chunk.

When llbitmap_cond_end_sync() calls:

drivers/md/md-llbitmap.c:llbitmap_cond_end_sync() {
    ...
	llbitmap_state_machine(llbitmap, 0, sector >> llbitmap->chunkshift,
			       BitmapActionEndsync);
    ...
}

it calculates the inclusive end bound for Endsync but fails to ensure only
fully completed chunks are processed, applying BitmapActionEndsync to the
partially completed chunk that sector resides in.

Because the state machine transitions partially-synced chunks to BitDirty or
BitCleanUnwritten, the next call to md_bitmap_start_sync() in the same chunk
will evaluate to BitNone and return false.

Will this cause the sync thread to skip the remaining sectors of that chunk
without resyncing them, leaving stale parity or unsynced mirrors on the disk?

>  	while (start <= end) {
>  		enum llbitmap_state c = llbitmap_read(llbitmap, start);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=14