Re: [PATCH v2 05/13] generic/1226: Add atomic write test using fio crc check verifier

Ojaswin Mujoo
2025-07-03T06:42:24Z

On Wed, Jul 02, 2025 at 08:46:39AM +0100, John Garry wrote:
> On 01/07/2025 17:18, Ojaswin Mujoo wrote:
> > On Fri, Jun 27, 2025 at 03:09:09PM +0100, John Garry wrote:
> > >
> > > > +touch "$SCRATCH_MNT/f1"
> > > > +awu_min_write=$(_get_atomic_write_unit_min "$SCRATCH_MNT/f1")
> > > > +awu_max_write=$(_get_atomic_write_unit_max "$SCRATCH_MNT/f1")
> > > > +blocksize=$(_max "$awu_min_write" "$((awu_max_write/2))")
> > > > +
> > > > +# XFS can have high awu_max_write due to software fallback. Cap it at 64k
> > >
> > > This test fails on xfs due for this reason. software fallback -based atomic
> > > writes have no serialization against reads or other writes.
> >
> > Okay so Im still not understanding why serialization is affecting
> > anything here? Lets assume the IO never breaks,
>
> That's a false assumption.
>
> An atomic write should never be split into multiple requests for REQ_ATOMIC
> set. However, regular reads and writes may be split in the block stack for
> any reason. We can easily force a split in regular reads and writes by
> setting max_sectors_kb to a value less than the read/write size.
>
> Check blk_mq_submit_bio() -> __bio_split_to_limits() -> bio_split_rw()
>
> > from what I understand
> > in fio code, we do the write and then once it is complete we try to read
> > it back and verify it.
> >
> > Why serialization matters here, because even if device reorders our read
> > before the atomic write, we still fetch an older $blocksize chunk which
> > should have the data and the header.
>
> Yes, we fetch $blocksize, but it may be as multiple read requests,
> so we can have:
>
> partial read $blocksize request, atomic write request, partial read
> $blocksize request

Ahh now I get the picture. Since software atomic writes might have a
value way higher than the max_sectors that the bio may split at, this
causes the reads to split which is the issue happnes, as you mentioned.

>
> > The data crc should match the
> > header because it would have gone atomically. What am I missing?
> >
> > (Or do you mean that max_sectors_kb is not big enough to support
> > reads after software atomic writes and thats the issue?)
> >
> > >
> > > Can you cap at atomic write unit max opt? That will mean that we get
> > > serialization from atomic write HW support.
> > >
> > > BTW, to repeat what I said before, it can also fail for atomic write bios
> > > using HW support, as reads may be split.
> >
> > This one I can understand, the fio's read is split causing a short read
> > and seems like fio is not requeing short read properly for the verify
> > step(?) so it tries to verify against short read itself.
>
> this is not an fio problem, as above.

Yep got it, its the block layer splitting the reads.

So as a solution, do you think it is okay to cap $blocksize to
max_sectors_kb or would you suggest it would be better to just stick to
awu max opt?

Thanks,
ojaswin
>
> I would like to repeat that the test should be ok to use, as long as we
> don't try to use an atomic write size > HW support (and we also have
> max_sectors_kb > $blocksize, which it would normally be).
>
> Thanks,
> John