From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9BBB8C433F5 for ; Fri, 1 Oct 2021 19:43:15 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 08E0E61A6F for ; Fri, 1 Oct 2021 19:43:14 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 08E0E61A6F Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 3F0F1940141; Fri, 1 Oct 2021 15:43:14 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 39FFE940121; Fri, 1 Oct 2021 15:43:14 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 28EF7940141; Fri, 1 Oct 2021 15:43:14 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0127.hostedemail.com [216.40.44.127]) by kanga.kvack.org (Postfix) with ESMTP id 1B0F7940121 for ; Fri, 1 Oct 2021 15:43:14 -0400 (EDT) Received: from smtpin14.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id BDECE180CF5F0 for ; Fri, 1 Oct 2021 19:43:13 +0000 (UTC) X-FDA: 78648892266.14.6F66A45 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf04.hostedemail.com (Postfix) with ESMTP id 1D461500106F for ; Fri, 1 Oct 2021 19:43:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Transfer-Encoding: Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date: Sender:Reply-To:Content-ID:Content-Description; bh=XiYRe7l+AkFXd4W8/qljO00jcN54Re+n4pMLJBImcn4=; b=iQI6FiASRn2habvKhbw6JQqj52 OKre4X2XWAD4zcFFS6NAZUIei9XMNcRLT85OPvTamROTSOr6+OC70n1Bre4z3l7ppfXJJwTbMBMu5 PS5B3glxN8RhlLhnHnQzECNI5VmCXUdW5fb4SThejwj9nxD3mMh9nLfO7S7PWzXwcTcVJ87Oya+qc RH8rjyCPRWBl5eHSAKwK5gmdomKGrz/LSgdNonQA+TSO3xUe1vSS0KAS//Bg5ifXw7WGWEOUMA925 /AB1+s3jE5kptfbxsYbi0NcK39gypiHAjjfRvN52ZirZVGRp1oK06oW+1RgQuWxg2l2B6pnRQTeHV ZSUTwNxQ==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1mWOQT-00EDKy-Sm; Fri, 01 Oct 2021 19:42:39 +0000 Date: Fri, 1 Oct 2021 20:42:33 +0100 From: Matthew Wilcox To: Trond Myklebust Cc: "dhowells@redhat.com" , "linux-cachefs@redhat.com" , "linux-mm@kvack.org" , "linux-nfs@vger.kernel.org" , "anna.schumaker@netapp.com" , "dwysocha@redhat.com" Subject: Re: 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 Message-ID: References: <97eb17f51c8fd9a89f10d9dd0bf35f1075f6b236.camel@hammerspace.com> <163189104510.2509237.10805032055807259087.stgit@warthog.procyon.org.uk> <163189108292.2509237.12615909591150927232.stgit@warthog.procyon.org.uk> <81120.1633099916@warthog.procyon.org.uk> <23033528036059af4633f60b8325e48eab95ac36.camel@hammerspace.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <23033528036059af4633f60b8325e48eab95ac36.camel@hammerspace.com> Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=iQI6FiAS; spf=none (imf04.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 1D461500106F X-Stat-Signature: kp5dntzmwq8rg3j4w9x7i64mw1iy7omg X-HE-Tag: 1633117393-542600 Content-Transfer-Encoding: quoted-printable X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Fri, Oct 01, 2021 at 03:04:08PM +0000, Trond Myklebust wrote: > On Fri, 2021-10-01 at 15:51 +0100, David Howells wrote: > > Trond Myklebust wrote: > >=20 > > > > > @@ -432,7 +432,12 @@ static int nfs_release_page(struct page > > > > > *page, gfp_t gfp) > > > > > =A0=A0=A0=A0=A0=A0=A0=A0/* If PagePrivate() is set, then the pa= ge is not > > > > > freeable */ > > > > > =A0=A0=A0=A0=A0=A0=A0=A0if (PagePrivate(page)) > > > > > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0return 0; > > > > > -=A0=A0=A0=A0=A0=A0=A0return nfs_fscache_release_page(page, gfp= ); > > > > > +=A0=A0=A0=A0=A0=A0=A0if (PageFsCache(page)) { > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0if (!(gfp & __GFP= _DIRECT_RECLAIM) || !(gfp & > > > > > __GFP_FS)) > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0return false; > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0wait_on_page_fsca= che(page); > > > > > +=A0=A0=A0=A0=A0=A0=A0} > > > > > +=A0=A0=A0=A0=A0=A0=A0return true; > > > > > =A0} > > >=20 > > > 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. > >=20 > > [Adding Willy and the mm crew to the cc list] > >=20 > > I wonder if that matters in this case.=A0 In the worst case, we'll wa= it > > for the > > page to cease being DMA'd - but we won't return true if it is. > >=20 > > But if vmscan is generating the wrong VM flags, we should look at > > fixing that. > >=20 > >=20 >=20 > To elaborate a bit: we used to have code here that would check whether > the page had been cleaned but was unstable, and if an argument of > GFP_KERNEL or above was set, we'd try to call COMMIT to ensure the page > was synched to disk on the server (and we'd wait for that call to > complete). >=20 > That code would end up deadlocking in all sorts of horrible ways, so we > ended up having to pull it. Based on having read zero code at all in this area ... Is it possible that you can wait for an existing operation to finish, but starting a new operation will take a lock that is already being held somewhere in your call chain? So it's not that the gfp flags are being set incorrectly, it's just that you're not in a context where you can start a new operation.