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

[Jens Axboe <axboe@kernel.dk>]

On 11/12/24 1:21 PM, Brian Foster wrote:
> 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.

OK good, will give that a spin.

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

Yeah makes sense, unloved.

-- 
Jens Axboe