Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <aee35d52ab19e7e95f69742be8329764db72cbf8.camel@kernel.org>
Subject: Re: [PATCH] nfsd: don't use GFP_KERNEL from
 nfsd_getxattr()/nfsd_listxattr()
From:   Jeff Layton <jlayton@kernel.org>
To:     Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
        Chuck Lever III <chuck.lever@oracle.com>
Cc:     Frank van der Linden <fllinden@amazon.com>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Date:   Sat, 15 Apr 2023 14:40:17 -0400
In-Reply-To: <b014047a-4a70-b38f-c5bb-01bc3c53d6f2@I-love.SAKURA.ne.jp>
References: <72bf692e-bb6b-c1f2-d1ba-3205ab649b43@I-love.SAKURA.ne.jp>
         <4BC7955B-40E4-4A43-B2D1-2E9302E84337@oracle.com>
         <b014047a-4a70-b38f-c5bb-01bc3c53d6f2@I-love.SAKURA.ne.jp>
Content-Type: text/plain; charset="ISO-8859-15"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.46.4 (3.46.4-1.fc37) 
MIME-Version: 1.0
Precedence: bulk

On Sun, 2023-04-16 at 02:11 +0900, Tetsuo Handa wrote:
> On 2023/04/16 1:13, Chuck Lever III wrote:
> > > On Apr 15, 2023, at 7:07 AM, Tetsuo Handa <penguin-kernel@I-love.SAKU=
RA.ne.jp> wrote:
> > >=20
> > > Since GFP_KERNEL is GFP_NOFS | __GFP_FS, usage like GFP_KERNEL | GFP_=
NOFS
> > > does not make sense. Drop __GFP_FS flag in order to avoid deadlock.
> >=20
> > The server side threads run in process context. GFP_KERNEL
> > is safe to use here -- as Jeff said, this code is not in
> > the server's reclaim path. Plenty of other call sites in
> > the NFS server code use GFP_KERNEL.
>=20
> GFP_KERNEL memory allocation calls filesystem's shrinker functions
> because of __GFP_FS flag. My understanding is
>=20
>   Whether this code is in memory reclaim path or not is irrelevant.
>   Whether memory reclaim path might hold lock or not is relevant.
>=20
> . Therefore, question is, does nfsd hold i_rwsem during memory reclaim pa=
th?
>=20

No. At the time of these allocations, the i_rwsem is not held.

> >=20
> > But I agree that the flag combination doesn't make sense.
> > Maybe drop GFP_NOFS instead and call it only a clean-up?
>=20

Yeah, I think that's the right thing to do here.

--=20
Jeff Layton <jlayton@kernel.org>