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

Ojaswin Mujoo
2025-07-08T06:51:01Z

On Mon, Jul 07, 2025 at 09:18:04AM +0100, John Garry wrote:
> On 04/07/2025 16:23, Ojaswin Mujoo wrote:
> > > Hmm yep this makes sense but seems like we don't expose it yet in
> > > xfs_io. Will need to add that support as well.
> > Okay so thinking about it a bit more, Im not sure if capping to
> > awu_max_opt is the right thing to do.
> >
> > There is no guarantee in kernel that awu_max_opt or even awu_max would
> > be smaller than lim->max_sectors so why to derive our io size based on
> > that.
>
> Absolutely. There is no guarantee. But generally it would not be less than
> lim->max_sectors. But lim->max_sectors kernel default is 1280KB, which is
> quite large. However, there are other reasons why splits could be broken. I
> am just giving the simple example in lim->max_sectors, as that is
> configurable from userspace (so useful for experimenting).

Got it.
>
> > Further, handling awu_max vs awu_max_opt and their different
> > interpretations for XFS vs EXT4 etc are adding uneccessary complexity.
>
> You could assume awu max opt == awu max for ext4, as we no there are no
> software-based atomic writes. That could be simpler than my previous
> suggestion, which involves a kernel change.

I want to avoid adding something like

iosize = FSTYP == "ext4" ? awu_max : awu_max_opt

in the test iteslf, because it is a bit difficult to remember changing
this in case things change in the kernel later, eg if ext4 gets a
fallback.

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)

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.

>
> As an alternative, maybe it's better to maintain this test out-of-tree. I'm
> not sure.
That'll be a hassle for everyone :p Does any of the above 2 approaches
seem acceptable for upstreaming this test?

Thanks for the reviews,
ojaswin

P.S: I'm not seeing anything weird with the line endings, but thanks,
i'll try to look into it.
>
> Thanks,
> John
>