[Linux-cachefs] [RFC PATCH] cifs: Transition from ->readpages() to ->readahead()
Matthew Wilcox
willy at infradead.org
Mon Jan 24 15:33:08 UTC 2022
On Mon, Jan 24, 2022 at 03:14:41PM +0000, David Howells wrote:
> Matthew Wilcox <willy at infradead.org> wrote:
>
> > > Questions for Willy:
> > > - Can we get a function to return the number of pages in a readahead
> > > batch?
> > >
> > > - Can we get a function to commit a readahead batch? Currently, this is
> > > done when we call __readahead_batch(), but that means ractl->_nr_pages
> > > isn't up to date at the point we need it to be. However, we want to
> > > check to see if the ractl is empty, then get server credits and only
> > > *then* call __readahead_batch() as we don't know how big a batch we're
> > > allowed till we have the credits.
> >
> > If you insist on using the primitives in a way that nobody else uses
> > them, you're going to find they don't work. What's wrong with the
> > way that FUSE uses them in fuse_readahead()?
>
> You mean doing this?
>
> nr_pages = readahead_count(rac) - nr_pages;
>
> that would seem to indicate that the readahead interface is wrong. Why should
> readahead_count() need correction? I think I can see *why* the batching stuff
> is hidden, but it seems that the comment for readahead_count() needs to
> mention this if you aren't going to fix it.
>
> Would it be possible to make readahead_count() do:
>
> return rac->_nr_pages - rac->_batch_count;
>
> maybe?
Yes, I think that would make sense. Do we also need to change
readhead_length()? It seems to me that it's only ever called once at
initialisation, so it should be possible to keep the two in sync.
Can you write & test such a patch? I'll support it going upstream
(either by taking it myself or giving you a R-b to take it through your
tree).
More information about the Linux-cachefs
mailing list