[v1] md/md-llbitmap: fixes and proactive parity building support

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

Posted by Yu Kuai 1 month, 2 weeks ago

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.

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;
+
+	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;
+	}
+	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:
+		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)
-- 
2.51.0

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

Posted by Su Yue 1 month, 2 weeks ago

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)

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

Posted by Yu Kuai 1 month, 1 week ago

Hi,

在 2026/2/17 16:54, Su Yue 写道:
> 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.

The original use case is that we found llbitmap array fails to assemble in
some corner cases, and with the respect I'm not quite familiar with mdadm
code, so I think this patch is the best solution for now.

On the other hand, this should also be helpful if we decide to make llbitmap
the default option in the future.

>
>> 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.

Ok, sounds reasonable.

>
>> +    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 ?

Because there is no optional bitmap_ops for md-cluster, it's still
the old bitmap, and llbitmap does not support md-cluster for now.

>
> -- 
> 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)

-- 
Thansk,
Kuai

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

Posted by Su Yue 1 month, 1 week ago

On Mon 23 Feb 2026 at 10:22, "Yu Kuai" <yukuai@fnnas.com> wrote:

> Hi,
>
> 在 2026/2/17 16:54, Su Yue 写道:
>> 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.
>
> The original use case is that we found llbitmap array fails to 
> assemble in
> some corner cases, and with the respect I'm not quite familiar 
> with mdadm
> code, so I think this patch is the best solution for now.
>
Would you please elaborate which corner cases that llbitmap array 
fails to assemble
in? Do they happen in mdadm <= 4.5?

> On the other hand, this should also be helpful if we decide to 
> make llbitmap
> the default option in the future.
>
But it's so far, right? llbitmap support is still on the way(mdadm 
4.6 is not released).

I am not opposed to the patch. It just looks strange to me that 
changing kernel code to
let old userspace work with *new* feature.
Maybe the mdadm maintainers have words in another angles?

--
Su
>
>>
>>> 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.
>
> Ok, sounds reasonable.
>
>>
>>> +    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 ?
>
> Because there is no optional bitmap_ops for md-cluster, it's 
> still
> the old bitmap, and llbitmap does not support md-cluster for 
> now.
>
>>
>> --
>> 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)

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

Posted by Xiao Ni 3 weeks, 2 days ago

在 2026/2/24 09:52, Su Yue 写道:
> On Mon 23 Feb 2026 at 10:22, "Yu Kuai" <yukuai@fnnas.com> wrote:
>
>> Hi,
>>
>> 在 2026/2/17 16:54, Su Yue 写道:
>>> 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.
>>
>> The original use case is that we found llbitmap array fails to 
>> assemble in
>> some corner cases, and with the respect I'm not quite familiar with 
>> mdadm
>> code, so I think this patch is the best solution for now.
>>
> Would you please elaborate which corner cases that llbitmap array 
> fails to assemble
> in? Do they happen in mdadm <= 4.5?
>
>> On the other hand, this should also be helpful if we decide to make 
>> llbitmap
>> the default option in the future.
>>
> But it's so far, right? llbitmap support is still on the way(mdadm 4.6 
> is not released).
>
> I am not opposed to the patch. It just looks strange to me that 
> changing kernel code to
> let old userspace work with *new* feature.
> Maybe the mdadm maintainers have words in another angles?


Yes. Is it better to upgrade mdadm to the version which supports llbitmap?


Regards

Xiao

>
> -- 
> Su
>>
>>>
>>>> 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.
>>
>> Ok, sounds reasonable.
>>
>>>
>>>> +    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 ?
>>
>> Because there is no optional bitmap_ops for md-cluster, it's still
>> the old bitmap, and llbitmap does not support md-cluster for now.
>>
>>>
>>> -- 
>>> 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)
>

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

Posted by Su Yue 3 weeks, 2 days ago

On Tue 10 Mar 2026 at 09:15, Xiao Ni <xni@redhat.com> wrote:

> 在 2026/2/24 09:52, Su Yue 写道:
>> On Mon 23 Feb 2026 at 10:22, "Yu Kuai" <yukuai@fnnas.com> 
>> wrote:
>>
>>> Hi,
>>>
>>> 在 2026/2/17 16:54, Su Yue 写道:
>>>> 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.
>>>
>>> The original use case is that we found llbitmap array fails to 
>>> assemble in
>>> some corner cases, and with the respect I'm not quite familiar 
>>> with mdadm
>>> code, so I think this patch is the best solution for now.
>>>
>> Would you please elaborate which corner cases that llbitmap 
>> array fails to
>> assemble
>> in? Do they happen in mdadm <= 4.5?
>>
>>> On the other hand, this should also be helpful if we decide to 
>>> make llbitmap
>>> the default option in the future.
>>>
>> But it's so far, right? llbitmap support is still on the 
>> way(mdadm 4.6 is not
>> released).
>>
>> I am not opposed to the patch. It just looks strange to me that 
>> changing
>> kernel code to
>> let old userspace work with *new* feature.
>> Maybe the mdadm maintainers have words in another angles?
>
>
> Yes. Is it better to upgrade mdadm to the version which supports 
> llbitmap?
>

Yes. This's what I mean.

--
Su
>
> Regards
>
> Xiao
>
>>
>> -- Su
>>>
>>>>
>>>>> 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.
>>>
>>> Ok, sounds reasonable.
>>>
>>>>
>>>>> +    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 ?
>>>
>>> Because there is no optional bitmap_ops for md-cluster, it's 
>>> still
>>> the old bitmap, and llbitmap does not support md-cluster for 
>>> now.
>>>
>>>>
>>>> -- 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)
>>

[PATCH 1/5] md/md-llbitmap: skip reading rdevs that are not in_sync
[PATCH 2/5] md/md-llbitmap: raise barrier before state machine transition
[PATCH 3/5] md: add fallback to correct bitmap_ops on version mismatch
[PATCH 4/5] md/md-llbitmap: add CleanUnwritten state for RAID-5 proactive parity building
[PATCH 5/5] md/md-llbitmap: optimize initial sync with write_zeroes_unmap support