Re: md bitmap writes random memory over disks' bitmap sectors

[Su Yue <l@damenly.org>]

On Fri 28 Feb 2025 at 15:46, Xiao Ni <xni@redhat.com> wrote:

> On Fri, Feb 28, 2025 at 10:07 AM Yu Kuai 
> <yukuai1@huaweicloud.com> wrote:
>>
>> Hi,
>>
>> 在 2025/02/25 23:32, Nigel Croxon 写道:
>> > -       md_super_write(mddev, rdev, sboff + ps, (int) size, 
>> > page);
>> > +       md_super_write(mddev, rdev, sboff + ps, 
>> > (int)min(size,
>> > bitmap_limit), page);
>> >          return 0;
>> >
>> > This patch still will attempt to send writes greater than a 
>> > page using
>> > only a single page pointer for multi-page bitmaps. The bitmap 
>> > uses an
>> > array (the filemap) of pages when the bitmap cannot fit in a 
>> > single
>> > page. These pages are allocated separately and not guaranteed 
>> > to be
>> > contiguous. So this patch will keep writes in a multi-page 
>> > bitmap from
>> > trashing data beyond the bitmap, but can create writes which 
>> > corrupt
>> > other parts of the bitmap with random memory.
>>
>> Is this problem introduced by:
>>
>> 8745faa95611 ("md: Use optimal I/O size for last bitmap page")
>
> I think so.
>
>>
>> >
>> > The opt using logic in this function is fundamentally flawed 
>> > as
>> > __write_sb_page should never send a write bigger than a page 
>> > at a time.
>> > It would need to use a new interface which can build 
>> > multi-page bio and
>> > not md_super_write() if it wanted to send multi-page I/Os.
>>
>> I argree. And I don't understand that patch yet, it said:
>>
>> If the bitmap space has enough room, size the I/O for the last 
>> bitmap
>> page write to the optimal I/O size for the storage device.
>>
>> Does this mean, for example, if bitmap space is 128k, while 
>> there is
>> only one page, means 124k is not used. In this case, if device 
>> opt io
>> size is 128k, this patch will choose to issue 128k IO instead 
>> of just
>> 4k IO? And how can this improve performance ...
>
> It looks like it does as you mentioned above. Through the commit
> 8745faa95611 message, the io size(3584 bytes, 7 sectors) is 
> smaller
> than 4K. Without the patch 8745faa95611,  the io size is round 
> up with
> bdev_logical_block_size(bdev). If we round up io size with 
> PAGE_SIZE,
> can it fix the performance problem? Because bitmap space is
> 4K/64K/128K, if it doesn't affect performance only changing the 
> round
> up value to PAGE_SIZE, it'll easy to fix this problem.
>

I'm afiraid that the perf will drop if rounding up io size to 4K 
for devices
optimal I/O size smaller than 4K. IMO better version of 
md_super_write to is
necessary to handle bitmap pages as Nigel said.

--
Su

> Best Regards
> Xiao
>
>> Thanks,
>> Kuai
>>
>>