Re: + fs-break-generic_file_buffered_read-up-into-multiple-functions.patch added to -mm tree

[Jens Axboe <axboe@kernel.dk>]

On 10/28/20 4:22 PM, Andrew Morton wrote:
> On Tue, 27 Oct 2020 13:35:51 +0000 Matthew Wilcox <willy@infradead.org> wrote:
> 
>> On Sun, Oct 25, 2020 at 03:08:17PM -0700, akpm@linux-foundation.org wrote:
>>> The patch titled
>>>      Subject: mm/filemap/c: freak generic_file_buffered_read up into multiple functions
>>> has been added to the -mm tree.  Its filename is
>>>      fs-break-generic_file_buffered_read-up-into-multiple-functions.patch
>>
>> Can we back this out?  It really makes the THP patchset unhappy.  I think
>> we can do something like this afterwards, but doing it this way round is
>> ridiculously hard.
> 
> Two concerns:
> 
> : On my test box, 4k buffered random reads go from ~150k to ~250k iops,
> : and the improvements to big sequential reads are even bigger.
> 
> That's a big improvement!  We want that improvement.  Throwing it away
> on behalf of an as-yet-unmerged feature patchset hurts.  Can we expect that
> this improvement will be available post-that-patchset?  And when?
> 
> (This improvment is rather hard to believe, really - more details on the
> test environment would be useful.  Can we expect that people will in
> general see similar benefits or was there something special about the
> testing?)

I did see some wins when I tested this. I'll try and run some testing
tomorrow and report back. If there's something specifically you want to
see tested, let me know.

> Secondly,
> 
> : This incorporates the fix for IOCB_WAITQ handling that Jens just
> : posted as well
> 
> Is this (mysterious, unreferenced!) fix against v1 of Kent's patchset,
> or is it against mainline?  If it's against mainline then it's
> presumably first priority.  Can we please get that sent out in a
> standalone form asap?

It's this one:

commit 13bd691421bc191a402d2e0d3da5f248d170a632
Author: Jens Axboe <axboe@kernel.dk>
Date:   Sat Oct 17 08:31:29 2020 -0600

    mm: mark async iocb read as NOWAIT once some data has been copied

which is in Linus's tree. Kent spotted the issue and had it fixed in
his 5.9 based patchset, which is probably where this confusion comes
from.

-- 
Jens Axboe