Re: [PATCH 08/15] mm/filemap: add read support for RWF_UNCACHED

[Brian Foster <bfoster@redhat.com>]

On Tue, Nov 12, 2024 at 12:45:58PM -0700, Jens Axboe wrote:
> On 11/12/24 12:39 PM, Brian Foster wrote:
> > On Tue, Nov 12, 2024 at 12:08:45PM -0700, Jens Axboe wrote:
> >> On 11/12/24 11:44 AM, Brian Foster wrote:
> >>> On Tue, Nov 12, 2024 at 10:19:02AM -0700, Jens Axboe wrote:
> >>>> On 11/12/24 10:06 AM, Jens Axboe wrote:
> >>>>> On 11/12/24 9:39 AM, Brian Foster wrote:
> >>>>>> On Tue, Nov 12, 2024 at 08:14:28AM -0700, Jens Axboe wrote:
> >>>>>>> On 11/11/24 10:13 PM, Christoph Hellwig wrote:
> >>>>>>>> On Mon, Nov 11, 2024 at 04:42:25PM -0700, Jens Axboe wrote:
> >>>>>>>>> Here's the slightly cleaned up version, this is the one I ran testing
> >>>>>>>>> with.
> >>>>>>>>
> >>>>>>>> Looks reasonable to me, but you probably get better reviews on the
> >>>>>>>> fstests lists.
> >>>>>>>
> >>>>>>> I'll send it out once this patchset is a bit closer to integration,
> >>>>>>> there's the usual chicken and egg situation with it. For now, it's quite
> >>>>>>> handy for my testing, found a few issues with this version. So thanks
> >>>>>>> for the suggestion, sure beats writing more of your own test cases :-)
> >>>>>>>
> >>>>>>
> >>>>>> fsx support is probably a good idea as well. It's similar in idea to
> >>>>>> fsstress, but bashes the same file with mixed operations and includes
> >>>>>> data integrity validation checks as well. It's pretty useful for
> >>>>>> uncovering subtle corner case issues or bad interactions..
> >>>>>
> >>>>> Indeed, I did that too. Re-running xfstests right now with that too.
> >>>>
> >>>> Here's what I'm running right now, fwiw. It adds RWF_UNCACHED support
> >>>> for both the sync read/write and io_uring paths.
> >>>>
> >>>
> >>> Nice, thanks. Looks reasonable to me at first glance. A few randomish
> >>> comments inlined below.
> >>>
> >>> BTW, I should have also mentioned that fsx is also useful for longer
> >>> soak testing. I.e., fstests will provide a decent amount of coverage as
> >>> is via the various preexisting tests, but I'll occasionally run fsx
> >>> directly and let it run overnight or something to get the op count at
> >>> least up in the 100 millions or so to have a little more confidence
> >>> there isn't some rare/subtle bug lurking. That might be helpful with
> >>> something like this. JFYI.
> >>
> >> Good suggestion, I can leave it running overnight here as well. Since
> >> I'm not super familiar with it, what would be a good set of parameters
> >> to run it with?
> >>
> > 
> > Most things are on by default, so I'd probably just go with that. -p is
> > useful to get occasional status output on how many operations have
> > completed and you could consider increasing the max file size with -l,
> > but usually I don't use more than a few MB or so if I increase it at
> > all.
> 
> When you say default, I'd run it without arguments. And then it does
> nothing :-)
> 
> Not an fs guy, I never run fsx. I run xfstests if I make changes that
> may impact the page cache, writeback, or file systems.
> 
> IOW, consider this a "I'm asking my mom to run fsx, I need to be pretty
> specific" ;-)
> 

Heh. In that case I'd just run something like this:

	fsx -p 100000 <file>

... and see how long it survives. It may not necessarily be an uncached
I/O problem if it fails, but depending on how reproducible a failure is,
that's where a cli knob comes in handy.

> > Random other thought: I also wonder if uncached I/O should be an
> > exclusive mode more similar to like how O_DIRECT or AIO is implemented.
> > But I dunno, maybe it doesn't matter that much (or maybe others will
> > have opinions on the fstests list).
> 
> Should probably exclude it with DIO, as it should not do anything there
> anyway. Eg if you ask for DIO, it gets turned off. For some of the other
> exclusions, they seem kind of wonky to me. Why can you use libaio and
> io_uring at the same time, for example?
> 

To your earlier point, if I had to guess it's probably just because it's
grotty test code with sharp edges.

Brian

> io_uring will work just fine with both buffered and direct IO, and it'll
> do the right thing with uncached as well. AIO is really a DIO only
> thing, not useful for anything else.
> 
> -- 
> Jens Axboe
>