Removing readpages aop

Removing readpages aop

[Matthew Wilcox]

In Linus' current tree, there are just three filesystems left using the
readpages address_space_operation:

$ git grep '\.readpages'
fs/9p/vfs_addr.c:       .readpages = v9fs_vfs_readpages,
fs/cifs/file.c: .readpages = cifs_readpages,
fs/nfs/file.c:  .readpages = nfs_readpages,

I'd love to finish getting rid of ->readpages as it would simplify
the VFS.  AFS and Ceph were both converted since 5.12 to use
netfs_readahead().  Is there any chance we might get the remaining three
filesystems converted in the next merge window?

Re: Removing readpages aop

[Steve French]

I don't have any objections as long as:
- we see at least mild performance benefit (or at least we are
confident that no performance loss)
- it passes regression tests (the usual xfstest bucket)
- it doesn't complicate the code too much (sounds like it actually
might simplify it, but needs a little more work)
- make sure that the usual tuning parms still work (e.g. "rsize" and
"rasize" mount options) or we can figure out a sane way to autotune
readhead so those wouldn't be needed for any workload

But currently since we get the most benefit from multichannel (as that
allows even better parallelization of i/o) ... I have been focused on
various multichannel issues (low credit situations, reconnect, fall
back to different channels when weird errors, adjusting channels
dynamically when server adds or removes adapters on the fly) for the
short term

On Wed, May 12, 2021 at 10:31 AM Matthew Wilcox <willy@infradead.org> wrote:
>
> In Linus' current tree, there are just three filesystems left using the
> readpages address_space_operation:
>
> $ git grep '\.readpages'
> fs/9p/vfs_addr.c:       .readpages = v9fs_vfs_readpages,
> fs/cifs/file.c: .readpages = cifs_readpages,
> fs/nfs/file.c:  .readpages = nfs_readpages,
>
> I'd love to finish getting rid of ->readpages as it would simplify
> the VFS.  AFS and Ceph were both converted since 5.12 to use
> netfs_readahead().  Is there any chance we might get the remaining three
> filesystems converted in the next merge window?
>


-- 
Thanks,

Steve

Re: Removing readpages aop

[Matthew Wilcox]

On Wed, May 12, 2021 at 02:28:41PM -0500, Steve French wrote:
> I don't have any objections as long as:
> - we see at least mild performance benefit (or at least we are
> confident that no performance loss)

Nobody's complained of a performance loss in the other ~30 filesystems
which have already been converted (some almost a year ago).  And CIFS
has one of the more convoluted readpages implementation, so I'd expect
a higher likelihood of a performance gain from CIFS.

> - it passes regression tests (the usual xfstest bucket)
> - it doesn't complicate the code too much (sounds like it actually
> might simplify it, but needs a little more work)
> - make sure that the usual tuning parms still work (e.g. "rsize" and
> "rasize" mount options) or we can figure out a sane way to autotune
> readhead so those wouldn't be needed for any workload

One of the enhancements added as part of the recent netfs merge
was readahead_expand().  Take a look at it and see if it works for you.

> But currently since we get the most benefit from multichannel (as that
> allows even better parallelization of i/o) ... I have been focused on
> various multichannel issues (low credit situations, reconnect, fall
> back to different channels when weird errors, adjusting channels
> dynamically when server adds or removes adapters on the fly) for the
> short term

Understood.  Only so many hours in the day.

I think
https://lore.kernel.org/linux-fsdevel/1794123.1605713481@warthog.procyon.org.uk/
is the most recent version, but as Dave notes, it needs attention from
somebody who knows the CIFS code better.

Re: [V9fs-developer] Removing readpages aop

[Dominique Martinet]

Matthew Wilcox wrote on Wed, May 12, 2021 at 04:12:22PM +0100:
> In Linus' current tree, there are just three filesystems left using the
> readpages address_space_operation:
> 
> $ git grep '\.readpages'
> fs/9p/vfs_addr.c:       .readpages = v9fs_vfs_readpages,
> fs/cifs/file.c: .readpages = cifs_readpages,
> fs/nfs/file.c:  .readpages = nfs_readpages,
> 
> I'd love to finish getting rid of ->readpages as it would simplify
> the VFS.  AFS and Ceph were both converted since 5.12 to use
> netfs_readahead().  Is there any chance we might get the remaining three
> filesystems converted in the next merge window?

David sent me a mostly-working implementation for netfs and it does get
rid of readpages, so it's just a matter of finding time for thorough
tests and cleanups...
I'd also like to let it sit in -next for a while (let's say at least one
month), so realistically I need to look at it within the next few weeks
and I honestly probably won't have time with my current schedule... But
it'll definitely be done for 5.15 (next's next merge window), and
probably in -next in ~2ish months if that's good enough for you.


If you can convince me both cifs and nfs will get it done before then I
might reconsider priorities :-D

-- 
Dominique