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

John Garry
2025-07-08T12:34:54Z

On 08/07/2025 13:01, Ojaswin Mujoo wrote:
>> You could do the statx call on the mounted bdev, and see if atomics are
>> supported there. If not, skip the test.
>>
>> But even for xfs when HW support is available, we can still use software
>> fallback when HW support is not possible, i.e. misaligned. So, in that
>> regard, we really should only test this on ext4.
> Hmm, okay so I'm a bit confused now. What is different in ext4 vs xfs.
> Why does the software fallback for xfs matter in this case?

For xfs, software fallback-based atomic writes are not even serialized
against other software fallback-based atomic writes. They can trample on
one another. That is a reason I say that this test should only be for
HW-based atomic writes.

But even if the bs <= HW limit, on xfs may rarely still use atomic write
software fallback if HW-based atomic write is not possible, e.g.
misaligned, multiple extents, etc. I will emphasis that this would be
rare, but it is still possible.

For ext4, since alignment is always guaranteed, and HW-based atomics
will only ever be used.

> If we limit
> io to 16KB hoping that reads don't break, then either ways the fio
> verify should work. And if reads break then both ext4 and xfs suffer
> irrespective of hardware or software fallback. (We also saw that
> hardware atomic writes also suffer with same issue when max_sectors is
> low)
>
>>> Im not sure if returning non-zero for ext4 is also correct as per the
>>> documented behavior from 5d894321c49e613.
>>>
>>> When zero, it means that there is no such performance boundary.
>>>
>>> Anyways, I think there might be a simpler solution...
>>>>> The simplest way seems to be to just limit the $blocksize to
>>>>> something like $(_min 16KB awu_max) and hope that 16KB is small enough
>>>>> to not be split during reads. We anyways have other tests like
>>>>> generic/1230 that can be used to test larger atomic write sizes
>>>>>
>>>>> Thoughts?
>>>> As I said at the start, we never had guarantees of serialization of reads
>>>> and atomic writes.
>>>>
>>>> However I still think that this is a useful test. It's just it is
>>>> theoretically possible to give false positives.
>>>>
>>>> You could get the test to read max_sectors_kb, and check whether it is
>>>> greater than the bs. Again, more complexity.
>>> I think this might work. So i can set the iosize as 16kb which is
>>> already low enought. Incase the max_sectors is < than 16kb then we bail
>>> out. (but then maybe there is a very small chance of read split still)
>> Sure, that seems fine.
>>
>>> OR another approach is to do the verify at the very end when all threads
>>> are done writing so the reads don't race. The tradeoff is that this
>>> will reduce the effictiveness of the test though to some extent.
>> Yeah, right. We would want this test to prove that atomic writes are not
>> getting split by the block stack, so racing reads can help prove that.
> Yeahh, but now i'm leaning towards this approach of keeping verify at
> the very end instead of parallel.

I don't think that fio supports such a mode. Or does it?

> Yes, it is a bit less effective but
> then having a data integrity test that can theoretically return false
> positives is not good, it'll always keep me guessing about any failure.
> Having verify at end will not have false failures for sure. Even though
> it comes at a cost of less coverage.
>
> Maybe we can also get some other prespectives on this.
> @Zorro, @Darrick, can you please give your thoughts on this?
>
> As a sidenote, with this issue of fio's verify read splitting, it seems
> like the parallel verify feature in fio itself is flawed for non-atomic
> writes as well since the read can always split and race with the writes.

fio does warn that multiple read and writers in verify mode is unsafe.

>
> Not sure what the solution is here, maybe time for non splittable atomic
> writes (:p)

I just think that fio in verify mode for ext4 is ok, as long as
max_sectors_kb is large.

But fundamentally we are trying to test a feature of RWF_ATOMIC which is
not guaranteed.

Thanks,
John