Re: [PATCH v2 09/20] md/md-llbitmap: refuse reshape while llbitmap still needs sync

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] llbitmap_reshape_can_start() fails to reject reshape when chunks are in BitNeedSyncUnwritten or BitSyncingUnwritten states.
- [High] The newly introduced reshape_can_start callback is never invoked by the RAID5 subsystem, rendering the reshape refusal logic non-functional for RAID5/6.
--

commit fc17ecb84d03fe66b3e1048dc17b6b8022da7f9b
Author: Yu Kuai <yukuai@fygo.io>

md/md-llbitmap: refuse reshape while llbitmap still needs sync

Reject reshape when llbitmap still contains NeedSync or Syncing bits.

This keeps reshape from starting until the current llbitmap state has
been reconciled.

> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index ae2e87fd58874..11fede1fc79fd 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
> @@ -1680,6 +1680,29 @@ static void llbitmap_dirty_bits(struct mddev *mddev, unsigned long s,
>  	llbitmap_state_machine(mddev->bitmap, s, e, BitmapActionStartwrite);
>  }
>  
> +static int llbitmap_reshape_can_start(struct mddev *mddev)
> +{
> +	struct llbitmap *llbitmap = mddev->bitmap;
> +	unsigned long chunk;
> +	int ret = 0;
> +
> +	if (!llbitmap)
> +		return 0;
> +
> +	mutex_lock(&mddev->bitmap_info.mutex);
> +	for (chunk = 0; chunk < llbitmap->chunks; chunk++) {
> +		enum llbitmap_state state = llbitmap_read(llbitmap, chunk);
> +
> +		if (state == BitNeedSync || state == BitSyncing) {

[Severity: High]
Should this check also include BitNeedSyncUnwritten and BitSyncingUnwritten
states?

Reshaping an array while it has regions pending or undergoing proactive parity
synchronization could change the array layout while the background sync process
is running, potentially leading to corrupt parity.

> +			ret = -EBUSY;
> +			break;
> +		}
> +	}
> +	mutex_unlock(&mddev->bitmap_info.mutex);
> +
> +	return ret;
> +}
> +

[ ... ]

> @@ -2002,6 +2025,7 @@ static struct bitmap_operations llbitmap_ops = {
>  	.dirty_bits		= llbitmap_dirty_bits,
>  	.prepare_range		= llbitmap_prepare_range,
>  	.reshape_finish		= llbitmap_reshape_finish,
> +	.reshape_can_start	= llbitmap_reshape_can_start,

[Severity: High]
Is the reshape_can_start callback invoked by the RAID5 subsystem when starting
a reshape?

Looking at raid5_start_reshape() in drivers/md/raid5.c:

    if (md_bitmap_enabled(mddev, false) &&
        mddev->bitmap_id == ID_LLBITMAP) {
        i = mddev->bitmap_ops->resize(mddev, mddev->dev_sectors, 0);

It doesn't appear to call mddev->bitmap_ops->reshape_can_start. This could
allow RAID5/6 arrays to start a reshape while the bitmap still contains
pending sync bits, causing the array layout to change before parity is
reconciled.

>  	.write_all		= llbitmap_write_all,
>  
>  	.groups			= md_llbitmap_groups,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1782282042.git.yukuai@kernel.org?part=9