Re: [PATCH v5 11/11] md/md-llbitmap: introduce new lockless bitmap

[Yu Kuai <yukuai@kernel.org>]

在 2025/8/3 17:48, Li Nan 写道:
>
>
> 在 2025/8/1 15:03, Yu Kuai 写道:
>> From: Yu Kuai <yukuai3@huawei.com>
>>
>> Redundant data is used to enhance data fault tolerance, and the storage
>> method for redundant data vary depending on the RAID levels. And it's
>> important to maintain the consistency of redundant data.
>>
>> Bitmap is used to record which data blocks have been synchronized and 
>> which
>> ones need to be resynchronized or recovered. Each bit in the bitmap
>> represents a segment of data in the array. When a bit is set, it 
>> indicates
>> that the multiple redundant copies of that data segment may not be
>> consistent. Data synchronization can be performed based on the bitmap 
>> after
>> power failure or readding a disk. If there is no bitmap, a full disk
>> synchronization is required.
>
> This is a large patch, I've found a few minor issues so far.
> And I'm still working through it.
>
> [...]
>
>> +    [BitDirty] = {
>> +        [BitmapActionStartwrite]    = BitNone,
>> +        [BitmapActionStartsync]        = BitNone,
>> +        [BitmapActionEndsync]        = BitNone,
>> +        [BitmapActionAbortsync]        = BitNone,
>> +        [BitmapActionReload]        = BitNeedSync,
>> +        [BitmapActionDaemon]        = BitClean,
>> +        [BitmapActionDiscard]        = BitUnwritten,
>> +        [BitmapActionStale]        = BitNeedSync,
>> +    },
>
> Bits becomes BitDirt during degraded remains BitDirty even after 
> recover and re-write. Should we consider adjusting this state 
> transition, or maybe trigger the daemon after the recovery is complete?
We should keep this behavior, otherwise readd a disk will be broken, 
we'll have
do do a full resync for the disk instead.

>
> [...]
>
>> +
>> +static int llbitmap_create(struct mddev *mddev)
>> +{
>> +    struct llbitmap *llbitmap;
>> +    int ret;
>> +
>> +    ret = llbitmap_check_support(mddev);
>> +    if (ret)
>> +        return ret;
>> +
>> +    llbitmap = kzalloc(sizeof(*llbitmap), GFP_KERNEL);
>> +    if (!llbitmap)
>> +        return -ENOMEM;
>> +
>> +    llbitmap->mddev = mddev;
>> +    llbitmap->io_size = bdev_logical_block_size(mddev->gendisk->part0);
>> +    llbitmap->blocks_per_page = PAGE_SIZE / llbitmap->io_size;
>
> logical_block_size can > PAGE_SIZE, blocks_per_page is set to 0 which can
> cause issues in later computations.
>
I do not expect this can happen, it's right large lbs for raw disk is 
supported
now, however, mdraid still handles metadata by page, and should forbid large
lbz while assembling the array for now. mdraid need to switch page to folio
to support large lbs, and this will be done later.

Thanks,
Kuai