linux-kernel - Re: [PATCH 07/23] md/md-bitmap: delay registration of bitmap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <82ed0b5d-d1ba-4e58-7d3b-adeb10b1ad24@huaweicloud.com>
Date: Tue, 27 May 2025 10:49:50 +0800
From: Yu Kuai <yukuai1@...weicloud.com>
To: Xiao Ni <xni@...hat.com>, Yu Kuai <yukuai1@...weicloud.com>
Cc: hch@....de, colyli@...nel.org, song@...nel.org,
 linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-raid@...r.kernel.org, yi.zhang@...wei.com, yangerkun@...wei.com,
 johnny.chenyi@...wei.com, "yukuai (C)" <yukuai3@...wei.com>
Subject: Re: [PATCH 07/23] md/md-bitmap: delay registration of bitmap_ops
 until creating bitmap

Hi,

在 2025/05/27 10:15, Xiao Ni 写道:
> 
> 在 2025/5/26 下午3:57, Yu Kuai 写道:
>> Hi,
>>
>> 在 2025/05/26 14:52, Xiao Ni 写道:
>>> On Sat, May 24, 2025 at 2:18 PM Yu Kuai <yukuai1@...weicloud.com> wrote:
>>>>
>>>> From: Yu Kuai <yukuai3@...wei.com>
>>>>
>>>> Currently bitmap_ops is registered while allocating mddev, this is fine
>>>> when there is only one bitmap_ops, however, after introduing a new
>>>> bitmap_ops, user space need a time window to choose which bitmap_ops to
>>>> use while creating new array.
>>>
>>> Could you give more explanation about what the time window is? Is it
>>> between setting llbitmap by bitmap_type and md_bitmap_create?
>>
>> The window after this patch is that user can write the new sysfs after
>> allocating mddev, and before running the array.
> 
> 
> Thanks for the explanation. Is it ok to add it in the commit log message?

ok
> 
>>>
>>>>
>>>> Signed-off-by: Yu Kuai <yukuai3@...wei.com>
>>>> ---
>>>>   drivers/md/md.c | 86 
>>>> +++++++++++++++++++++++++++++++------------------
>>>>   1 file changed, 55 insertions(+), 31 deletions(-)
>>>>
>>>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>>>> index 4eb0c6effd5b..dc4b85f30e13 100644
>>>> --- a/drivers/md/md.c
>>>> +++ b/drivers/md/md.c
>>>> @@ -674,39 +674,50 @@ static void no_op(struct percpu_ref *r) {}
>>>>
>>>>   static bool mddev_set_bitmap_ops(struct mddev *mddev)
>>>>   {
>>>> +       struct bitmap_operations *old = mddev->bitmap_ops;
>>>> +       struct md_submodule_head *head;
>>>> +
>>>> +       if (mddev->bitmap_id == ID_BITMAP_NONE ||
>>>> +           (old && old->head.id == mddev->bitmap_id))
>>>> +               return true;
>>>> +
>>>>          xa_lock(&md_submodule);
>>>> -       mddev->bitmap_ops = xa_load(&md_submodule, mddev->bitmap_id);
>>>> +       head = xa_load(&md_submodule, mddev->bitmap_id);
>>>>          xa_unlock(&md_submodule);
>>>>
>>>> -       if (!mddev->bitmap_ops) {
>>>> -               pr_warn_once("md: can't find bitmap id %d\n", 
>>>> mddev->bitmap_id);
>>>> +       if (WARN_ON_ONCE(!head || head->type != MD_BITMAP)) {
>>>> +               pr_err("md: can't find bitmap id %d\n", 
>>>> mddev->bitmap_id);
>>>>                  return false;
>>>>          }
>>>>
>>>> +       if (old && old->group)
>>>> +               sysfs_remove_group(&mddev->kobj, old->group);
>>>
>>> I think you're handling a competition problem here. But I don't know
>>> how the old/old->group is already created when creating an array.
>>> Could you explain this?
>>
>> It's not possible now, this is because I think we want to be able to
>> switch existing array with old bitmap to new bitmap.
> 
> 
> Can we add the check of old when we really want it?

I'm fine, and there is no doubt we will want it.

Thanks,
Kuai

> 
> Regards
> 
> Xiao
> 
>>
>> Thanks,
>> Kuai
>>
>>>
>>> Regards
>>> Xiao
>>
> 
> 
> .
>