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: <e8cbe0fb8292b872ce40b2471a8878478fb71530.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 <chuck.lever@oracle.com>,
        Frank van der Linden <fllinden@amazon.com>,
        "open list:NFS, SUNRPC, AND..." <linux-nfs@vger.kernel.org>
Date:   Sat, 15 Apr 2023 07:42:19 -0400
In-Reply-To: <72bf692e-bb6b-c1f2-d1ba-3205ab649b43@I-love.SAKURA.ne.jp>
References: <72bf692e-bb6b-c1f2-d1ba-3205ab649b43@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 Sat, 2023-04-15 at 20:07 +0900, Tetsuo Handa wrote:
> 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
> Fixes: 32119446bb65 ("nfsd: define xattr functions to call into their vfs=
 counterparts")
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ---
>  fs/nfsd/vfs.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>=20
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 5783209f17fc..109b31246666 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -2164,7 +2164,7 @@ nfsd_getxattr(struct svc_rqst *rqstp, struct svc_fh=
 *fhp, char *name,
>  		goto out;
>  	}
> =20
> -	buf =3D kvmalloc(len, GFP_KERNEL | GFP_NOFS);
> +	buf =3D kvmalloc(len, GFP_NOFS);
>  	if (buf =3D=3D NULL) {
>  		err =3D nfserr_jukebox;
>  		goto out;
> @@ -2230,7 +2230,7 @@ nfsd_listxattr(struct svc_rqst *rqstp, struct svc_f=
h *fhp, char **bufp,
>  	/*
>  	 * We're holding i_rwsem - use GFP_NOFS.
>  	 */
> -	buf =3D kvmalloc(len, GFP_KERNEL | GFP_NOFS);
> +	buf =3D kvmalloc(len, GFP_NOFS);
>  	if (buf =3D=3D NULL) {
>  		err =3D nfserr_jukebox;
>  		goto out;

I don't get it.

These are the NFS server handlers for the GETXATTR and LISTXATTR
operations. Basically the client is making a call to the server to fetch
or list xattrs. I don't really see a huge issue with them recursing into
reclaim. It's not like this code is in the writeback path.
--=20
Jeff Layton <jlayton@kernel.org>