Re: [PATCH v2 08/20] md/md-llbitmap: finish reshape geometry

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The entire bitmap is unnecessarily flushed to disk twice during reshape finish.
--

commit ae4a6d71e5ee77405793f20d5d615e089187608f
Author: Yu Kuai <yukuai@fygo.io>
md/md-llbitmap: finish reshape geometry

This commit introduces the final step for reshaping llbitmap arrays by
committing the staged geometry. It handles quiescing the device, updating
internal state, writing the superblock, and flushing the bitmap.

> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index cdced2126e975..ae2e87fd58874 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c

[ ... ]

> @@ -1680,6 +1680,30 @@ static void llbitmap_dirty_bits(struct mddev *mddev, unsigned long s,
>  	llbitmap_state_machine(mddev->bitmap, s, e, BitmapActionStartwrite);
>  }
>  
> +static void llbitmap_reshape_finish(struct mddev *mddev)
> +{
> +	struct llbitmap *llbitmap = mddev->bitmap;
> +
> +	if (mddev->pers->quiesce)
> +		mddev->pers->quiesce(mddev, 1);
> +
> +	mutex_lock(&mddev->bitmap_info.mutex);
> +	llbitmap_flush(mddev);
> +
> +	llbitmap->chunksize = llbitmap->reshape_chunksize;
> +	llbitmap->chunkshift = ffz(~llbitmap->chunksize);
> +	llbitmap->chunks = llbitmap->reshape_chunks;
> +	llbitmap->sync_size = llbitmap->reshape_sync_size;
> +	llbitmap_refresh_reshape(llbitmap);
> +	mddev->bitmap_info.chunksize = llbitmap->chunksize;
> +	llbitmap_update_sb(llbitmap);
> +	__llbitmap_flush(mddev);

[Severity: Low]
Is this second flush of the entire bitmap redundant?

The code calls llbitmap_flush() at the beginning of the lock section.
After updating the geometry and writing out the superblock via
llbitmap_update_sb(), __llbitmap_flush() is called again. Since no bitmap
data bits are modified in the interim, does this cause an unnecessary
synchronous I/O spike that could be avoided?

> +	mutex_unlock(&mddev->bitmap_info.mutex);
> +
> +	if (mddev->pers->quiesce)
> +		mddev->pers->quiesce(mddev, 0);
> +}

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