Can the GFP flags to releasepage() be trusted? -- was Re: [PATCH v2 3/8] nfs: Move to using the alternate fallback fscache I/O API

[David Howells <dhowells@redhat.com>]

Trond Myklebust <trondmy@hammerspace.com> wrote:

> > > @@ -432,7 +432,12 @@ static int nfs_release_page(struct page *page, gfp_t gfp)
> > >  	/* If PagePrivate() is set, then the page is not freeable */
> > >  	if (PagePrivate(page))
> > >  		return 0;
> > > -	return nfs_fscache_release_page(page, gfp);
> > > +	if (PageFsCache(page)) {
> > > +		if (!(gfp & __GFP_DIRECT_RECLAIM) || !(gfp & __GFP_FS))
> > > +			return false;
> > > +		wait_on_page_fscache(page);
> > > +	}
> > > +	return true;
> > >  }
> 
> I've found this generally not to be safe. The VM calls ->release_page()
> from a variety of contexts, and often fails to report it correctly in
> the gfp flags. That's particularly true of the stuff in mm/vmscan.c.
> This is why we have the check above that vetos page removal upon
> PagePrivate() being set.

[Adding Willy and the mm crew to the cc list]

I wonder if that matters in this case.  In the worst case, we'll wait for the
page to cease being DMA'd - but we won't return true if it is.

But if vmscan is generating the wrong VM flags, we should look at fixing that.

David