Re: [PATCH 3/5] md: add fallback to correct bitmap_ops on version mismatch

[Su Yue <l@damenly.org>]

On Sat 14 Feb 2026 at 14:10, Yu Kuai <yukuai@fnnas.com> wrote:

> If default bitmap version and on-disk version doesn't match, and 
> mdadm
> is not the latest version to set bitmap_type, set bitmap_ops 
> based on
> the disk version.
>
Why not just let old version mdadm fails  since llbitmap is a new 
feature.

> Signed-off-by: Yu Kuai <yukuai@fnnas.com>
> ---
>  drivers/md/md.c | 103 
>  +++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 102 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 59cd303548de..d2607ed5c2e9 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -6447,15 +6447,116 @@ static void md_safemode_timeout(struct 
> timer_list *t)
>
>  static int start_dirty_degraded;
>
> +/*
> + * Read bitmap superblock and return the bitmap_id based on 
> disk version.
> + * This is used as fallback when default bitmap version and 
> on-disk version
> + * doesn't match, and mdadm is not the latest version to set 
> bitmap_type.
> + */
> +static enum md_submodule_id md_bitmap_get_id_from_sb(struct 
> mddev *mddev)
> +{
> +	struct md_rdev *rdev;
> +	struct page *sb_page;
> +	bitmap_super_t *sb;
> +	enum md_submodule_id id = ID_BITMAP_NONE;
> +	sector_t sector;
> +	u32 version;
> +
> +	if (!mddev->bitmap_info.offset)
> +		return ID_BITMAP_NONE;
> +
> +	sb_page = alloc_page(GFP_KERNEL);
> +	if (!sb_page)
> +		return ID_BITMAP_NONE;
> +
>
Personally I don't like the way treating error as ID_BITMAP_NONE.
When wrong things happen everything looks fine, no error code, no 
error message.

> +	sector = mddev->bitmap_info.offset;
> +
> +	rdev_for_each(rdev, mddev) {
> +		u32 iosize;
> +
> +		if (!test_bit(In_sync, &rdev->flags) ||
> +		    test_bit(Faulty, &rdev->flags) ||
> +		    test_bit(Bitmap_sync, &rdev->flags))
> +			continue;
> +
> +		iosize = roundup(sizeof(bitmap_super_t),
> +				 bdev_logical_block_size(rdev->bdev));
> +		if (sync_page_io(rdev, sector, iosize, sb_page, 
> REQ_OP_READ,
> +				 true))
> +			goto read_ok;
> +	}
>
And here.

> +	goto out;
> +
> +read_ok:
> +	sb = kmap_local_page(sb_page);
> +	if (sb->magic != cpu_to_le32(BITMAP_MAGIC))
> +		goto out_unmap;
> +
> +	version = le32_to_cpu(sb->version);
> +	switch (version) {
> +	case BITMAP_MAJOR_LO:
> +	case BITMAP_MAJOR_HI:
> +	case BITMAP_MAJOR_CLUSTERED:
>
For BITMAP_MAJOR_CLUSTERED, why not ID_CLUSTER ?

--
Su
> +		id = ID_BITMAP;
> +		break;
> +	case BITMAP_MAJOR_LOCKLESS:
> +		id = ID_LLBITMAP;
> +		break;
> +	default:
> +		pr_warn("md: %s: unknown bitmap version %u\n",
> +			mdname(mddev), version);
> +		break;
> +	}
> +
> +out_unmap:
> +	kunmap_local(sb);
> +out:
> +	__free_page(sb_page);
> +	return id;
> +}
> +
>  static int md_bitmap_create(struct mddev *mddev)
>  {
> +	enum md_submodule_id orig_id = mddev->bitmap_id;
> +	enum md_submodule_id sb_id;
> +	int err;
> +
>  	if (mddev->bitmap_id == ID_BITMAP_NONE)
>  		return -EINVAL;
>
>  	if (!mddev_set_bitmap_ops(mddev))
>  		return -ENOENT;
>
> -	return mddev->bitmap_ops->create(mddev);
> +	err = mddev->bitmap_ops->create(mddev);
> +	if (!err)
> +		return 0;
>
> +
> +	/*
> +	 * Create failed, if default bitmap version and on-disk 
> version
> +	 * doesn't match, and mdadm is not the latest version to set
> +	 * bitmap_type, set bitmap_ops based on the disk version.
> +	 */
> +	mddev_clear_bitmap_ops(mddev);
> +
> +	sb_id = md_bitmap_get_id_from_sb(mddev);
> +	if (sb_id == ID_BITMAP_NONE || sb_id == orig_id)
> +		return err;
> +
> +	pr_info("md: %s: bitmap version mismatch, switching from %d to 
> %d\n",
> +		mdname(mddev), orig_id, sb_id);
> +
> +	mddev->bitmap_id = sb_id;
> +	if (!mddev_set_bitmap_ops(mddev)) {
> +		mddev->bitmap_id = orig_id;
> +		return -ENOENT;
> +	}
> +
> +	err = mddev->bitmap_ops->create(mddev);
> +	if (err) {
> +		mddev_clear_bitmap_ops(mddev);
> +		mddev->bitmap_id = orig_id;
> +	}
> +
> +	return err;
>  }
>
>  static void md_bitmap_destroy(struct mddev *mddev)