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

[John Garry <john.g.garry@oracle.com>]

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

> 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.

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