Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Organization: Red Hat UK Ltd. Registered Address: Red Hat UK Ltd, Amberley
        Place, 107-111 Peascod Street, Windsor, Berkshire, SI4 1TE, United
        Kingdom.
        Registered in England and Wales under Company Registration No. 3798903
From:   David Howells <dhowells@redhat.com>
In-Reply-To: <97eb17f51c8fd9a89f10d9dd0bf35f1075f6b236.camel@hammerspace.com>
References: <97eb17f51c8fd9a89f10d9dd0bf35f1075f6b236.camel@hammerspace.com> <163189104510.2509237.10805032055807259087.stgit@warthog.procyon.org.uk> <163189108292.2509237.12615909591150927232.stgit@warthog.procyon.org.uk> <CALF+zO=165sRYRaxPpDS7DaQCpTe-YOa4FamSoMy5FV2uuG5Yg@mail.gmail.com>
To:     Trond Myklebust <trondmy@hammerspace.com>
Cc:     dhowells@redhat.com, dwysocha@redhat.com,
        anna.schumaker@netapp.com, Matthew Wilcox <willy@infradead.org>,
        linux-cachefs@redhat.com, linux-nfs@vger.kernel.org,
        linux-mm@kvack.org
Subject: 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
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <81119.1633099916.1@warthog.procyon.org.uk>
Content-Transfer-Encoding: quoted-printable
Date:   Fri, 01 Oct 2021 15:51:56 +0100
Message-ID: <81120.1633099916@warthog.procyon.org.uk>
Precedence: bulk

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 t=
hat.

David