Re: [RFC PATCH] cifs: Transition from ->readpages() to ->readahead()

From: Matthew Wilcox <willy@infradead.org>
To: David Howells <dhowells@redhat.com>
Cc: smfrench@gmail.com, nspmangalore@gmail.com,
	Jeff Layton <jlayton@kernel.org>,
	linux-cifs@vger.kernel.org, linux-cachefs@redhat.com
Subject: Re: [RFC PATCH] cifs: Transition from ->readpages() to ->readahead()
Date: Mon, 24 Jan 2022 15:58:43 +0000	[thread overview]
Message-ID: <Ye7Ms67MA0kycc/x@casper.infradead.org> (raw)
In-Reply-To: <2270964.1643039187@warthog.procyon.org.uk>

On Mon, Jan 24, 2022 at 03:46:27PM +0000, David Howells wrote:
> Matthew Wilcox <willy@infradead.org> wrote:
> 
> > > 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).
> 
> It seems I also have a problem with readahead_index() needing compensation
> too.  I'm guessing that's more of a problem, however, as I think that's
> expected to refer to the beginning of the current batch.

Well, that's the problem isn't it?  You're expecting to mutate the state
and then refer to its previous state instead of its current state,
whereas the other users refer to the current state instead of the
previous state.  Can't you pull readahead_index() out of the ractl
ahead of the mutation?