Linux RAID subsystem development
 help / color / mirror / Atom feed
From: "Yu Kuai" <yukuai@fnnas.com>
To: "Xiao Ni" <xni@redhat.com>, <yukuai@fnnas.com>
Cc: <song@kernel.org>, <linan122@huawei.com>, <colyli@fnnas.com>,
	 <linux-raid@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 3/5] md: add fallback to correct bitmap_ops on version mismatch
Date: Mon, 16 Mar 2026 01:01:55 +0800	[thread overview]
Message-ID: <3af32442-5783-41fb-85c4-1e68d2ef2149@fnnas.com> (raw)
In-Reply-To: <CALTww28rtki9J=_GB1zf4N5FBcdz+vzSvB1bNb-O2-AgrUzgSA@mail.gmail.com>

Hi,

在 2026/3/10 9:06, Xiao Ni 写道:
> On Mon, Feb 23, 2026 at 10:43 AM 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.
> Hi Kuai
>
> How can I do test to check if this patch works?
>
> 1. Create array with llbitmap
> 2. Stop the array
> 3. uninstall mdadm with llbitmap support and install mdadm without llbitmap
> 4. assemble the array

At first, my colleague met this problem during power off and reboot test, however
I can't reproduce this, after adding some debug info, I'm quite sure this is still
an mdadm issue, that somewhere write llbitmap to sysfs bitmap_type file is missing.

Also, this way can be check as well if this patch works.

>
> Is it the case you want to fix?
>
> Regards
> Xiao
>> Signed-off-by: Yu Kuai <yukuai@fnnas.com>
>> ---
>>   drivers/md/md.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 110 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index 72a1c7267851..245785ad0ffd 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -6447,15 +6447,124 @@ 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) {
>> +               pr_warn("md: %s: failed to allocate memory for bitmap\n",
>> +                       mdname(mddev));
>> +               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;
>> +       }
>> +       pr_warn("md: %s: failed to read bitmap from any device\n",
>> +               mdname(mddev));
>> +       goto out;
>> +
>> +read_ok:
>> +       sb = kmap_local_page(sb_page);
>> +       if (sb->magic != cpu_to_le32(BITMAP_MAGIC)) {
>> +               pr_warn("md: %s: invalid bitmap magic 0x%x\n",
>> +                       mdname(mddev), le32_to_cpu(sb->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
>>
>>
-- 
Thansk,
Kuai

  reply	other threads:[~2026-03-15 17:02 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-23  2:40 [PATCH v2 0/5] md/md-llbitmap: fixes and proactive parity building support Yu Kuai
2026-02-23  2:40 ` [PATCH v2 1/5] md/md-llbitmap: skip reading rdevs that are not in_sync Yu Kuai
2026-03-09  7:32   ` Xiao Ni
2026-02-23  2:40 ` [PATCH v2 2/5] md/md-llbitmap: raise barrier before state machine transition Yu Kuai
2026-03-09 13:05   ` Xiao Ni
2026-03-15 16:57     ` Yu Kuai
2026-02-23  2:40 ` [PATCH v2 3/5] md: add fallback to correct bitmap_ops on version mismatch Yu Kuai
2026-03-10  1:06   ` Xiao Ni
2026-03-15 17:01     ` Yu Kuai [this message]
2026-02-23  2:40 ` [PATCH v2 4/5] md/md-llbitmap: add CleanUnwritten state for RAID-5 proactive parity building Yu Kuai
2026-03-13  3:16   ` Xiao Ni
2026-03-17  3:36     ` Yu Kuai
2026-02-23  2:40 ` [PATCH v2 5/5] md/md-llbitmap: optimize initial sync with write_zeroes_unmap support Yu Kuai
2026-03-13  8:12   ` Xiao Ni
2026-03-22 18:30 ` [PATCH v2 0/5] md/md-llbitmap: fixes and proactive parity building support Yu Kuai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3af32442-5783-41fb-85c4-1e68d2ef2149@fnnas.com \
    --to=yukuai@fnnas.com \
    --cc=colyli@fnnas.com \
    --cc=linan122@huawei.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-raid@vger.kernel.org \
    --cc=song@kernel.org \
    --cc=xni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox