Re: [PATCH 03/11] MM: improve cleanup when ->readpages doesn't process all pages.
From: NeilBrown
Date: Thu Mar 03 2022 - 21:23:36 EST
On Wed, 02 Mar 2022, Miklos Szeredi wrote:
> On Tue, 22 Feb 2022 at 04:18, NeilBrown <neilb@xxxxxxx> wrote:
> >
> > If ->readpages doesn't process all the pages, then it is best to act as
> > though they weren't requested so that a subsequent readahead can try
> > again.
> > So:
> > - remove any 'ahead' pages from the page cache so they can be loaded
> > with ->readahead() rather then multiple ->read()s
> > - update the file_ra_state to reflect the reads that were actually
> > submitted.
> >
> > This allows ->readpages() to abort early due e.g. to congestion, which
> > will then allow us to remove the inode_read_congested() test from
> > page_Cache_async_ra().
> >
> > Signed-off-by: NeilBrown <neilb@xxxxxxx>
> > ---
> > mm/readahead.c | 19 +++++++++++++++++--
> > 1 file changed, 17 insertions(+), 2 deletions(-)
> >
> > diff --git a/mm/readahead.c b/mm/readahead.c
> > index 73b2bc5302e0..8a97bd408cf6 100644
> > --- a/mm/readahead.c
> > +++ b/mm/readahead.c
> > @@ -104,7 +104,13 @@
> > * for necessary resources (e.g. memory or indexing information) to
> > * become available. Pages in the final ``async_size`` may be
> > * considered less urgent and failure to read them is more acceptable.
> > - * They will eventually be read individually using ->readpage().
> > + * In this case it is best to use delete_from_page_cache() to remove the
> > + * pages from the page cache as is automatically done for pages that
> > + * were not fetched with readahead_page(). This will allow a
> > + * subsequent synchronous read ahead request to try them again. If they
> > + * are left in the page cache, then they will be read individually using
> > + * ->readpage().
> > + *
> > */
> >
> > #include <linux/kernel.h>
> > @@ -226,8 +232,17 @@ static void read_pages(struct readahead_control *rac, struct list_head *pages,
> >
> > if (aops->readahead) {
> > aops->readahead(rac);
> > - /* Clean up the remaining pages */
> > + /*
> > + * Clean up the remaining pages. The sizes in ->ra
> > + * maybe be used to size next read-ahead, so make sure
> > + * they accurately reflect what happened.
> > + */
> > while ((page = readahead_page(rac))) {
> > + rac->ra->size -= 1;
> > + if (rac->ra->async_size > 0) {
> > + rac->ra->async_size -= 1;
> > + delete_from_page_cache(page);
> > + }
>
> Does the above imply that filesystem should submit at least ra->size
> pages, regardless of congestion?
ra->size - ra_async_size
pages should be submitted reguardless of congestion.
NeilBrown