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

[Matthew Wilcox <willy@infradead.org>]

On Wed, Oct 28, 2020 at 03:22:35PM -0700, 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?

I agree we want that improvement!  I don't understand how this improves
4k buffered reads though.  It should improve 8k to 60k io sizes, but
it should be the same for 4k.  Honestly, it should be better for 4097
bytes on up (but nobody tests 4097 byte block sizes).

I've been working on redoing my patchset on top of current -next for the
past two days.  It's mostly there, but I've hit a problem when testing
with the readahead error injector.  The good news is that the performance
improvement should be cumulative -- Kent's improvement mean that we
get pages out of the page cache in batches, while mine will do fewer
atomic ops (on page->refcount).

Funnily, this has led to finding a bug in copy_page_to_iter_iovec()
and, honestly, a lot of the other iov_iter code, but I don't think
that's going to hold us up.