On Mon, Mar 3, 2025 at 11:16 PM Su Yue <l@xxxxxxxxxxx> wrote:
>
> On Fri 28 Feb 2025 at 15:46, Xiao Ni <xni@xxxxxxxxxx> wrote:
>
> > On Fri, Feb 28, 2025 at 10:07 AM Yu Kuai
> > <yukuai1@xxxxxxxxxxxxxxx> 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
Hi Su
You're right. Thanks for pointing out this.
Best Regards
Xiao
>
> > Best Regards
> > Xiao
> >
> >> Thanks,
> >> Kuai
> >>
> >>
>
