Re: [PATCH v5 18/29] 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] The newly added reshape callbacks (`reshape_can_start`, `reshape_finish`, and `reshape_mark`) are completely unreferenced by the core MD subsystem, rendering the patch ineffective.
- [Medium] llbitmap_reshape_can_start() ignores proactive sync states for unwritten regions, which would allow reshape to conflict with ongoing or pending syncs if the function were called.
--

--- Patch [18]: [PATCH v5 18/29] md/md-llbitmap: refuse reshape while llbitmap still needs sync ---
commit 4b812e132deb0c081e68cf2925523d86a890d935
Author: Yu Kuai <yukuai@fygo.io>

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

This commit rejects reshape when llbitmap still contains NeedSync or
Syncing bits. This is intended to keep 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 a7c229db3058d..f8a1b0f79be6a 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
> @@ -1718,6 +1718,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: Medium]
Does this check omit the unwritten equivalents of these sync states?

It appears that BitNeedSyncUnwritten and BitSyncingUnwritten are not
checked here. 

Since these states also represent active or pending parity syncs, would
omitting them allow a reshape to conflict with an ongoing sync?

> +			ret = -EBUSY;
> +			break;
> +		}
> +	}
> +	mutex_unlock(&mddev->bitmap_info.mutex);
> +
> +	return ret;
> +}
> +
>  static void llbitmap_reshape_finish(struct mddev *mddev)
>  {
>  	struct llbitmap *llbitmap = mddev->bitmap;

[ ... ]

> @@ -2034,6 +2057,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]
Are there any callers that actually use this new callback?

This patch populates the reshape_can_start callback in llbitmap_ops, but
does not appear to update the core MD subsystem (such as drivers/md/md.c)
to invoke mddev->bitmap_ops->reshape_can_start().

Without a caller, does this validation logic become unreachable dead code
that fails to reject the reshape?

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260802195038.164272-1-yukuai@kernel.org?part=18