summaryrefslogtreecommitdiff
path: root/fs/nfsd/vfs.c
diff options
context:
space:
mode:
authorChuck Lever <chuck.lever@oracle.com>2023-04-20 11:02:33 -0400
committerChuck Lever <chuck.lever@oracle.com>2023-04-27 18:49:24 -0400
commit22b620ec0bf454cfd1c464f57cfce9afb3fb1e70 (patch)
treefa4f345a66c54e2e344c7c590abd4353a9446e66 /fs/nfsd/vfs.c
parent147abcacee33781e75588869e944ddb07528a897 (diff)
NFSD: Clean up xattr memory allocation flags
Tetsuo Handa points out: > Since GFP_KERNEL is "GFP_NOFS | __GFP_FS", usage like > "GFP_KERNEL | GFP_NOFS" does not make sense. The original intent was to hold the inode lock while estimating the buffer requirements for the requested information. Frank van der Linden, the author of NFSD's xattr code, says: > ... you need inode_lock to get an atomic view of an xattr. Since > both nfsd_getxattr and nfsd_listxattr to the standard trick of > querying the xattr length with a NULL buf argument (just getting > the length back), allocating the right buffer size, and then > querying again, they need to hold the inode lock to avoid having > the xattr changed from under them while doing that. > > From that then flows the requirement that GFP_FS could cause > problems while holding i_rwsem, so I added GFP_NOFS. However, Dave Chinner states: > You can do GFP_KERNEL allocations holding the i_rwsem just fine. > All that it requires is the caller holds a reference to the > inode ... Since these code paths acquire a dentry, they do indeed hold a reference. It is therefore safe to use GFP_KERNEL for these memory allocations. In particular, that's what this code is already doing; but now the C source code looks sane too. At a later time we can revisit in order to remove the inode lock in favor of simply retrying if the estimated buffer size is too small. Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Diffstat (limited to 'fs/nfsd/vfs.c')
-rw-r--r--fs/nfsd/vfs.c7
1 files changed, 2 insertions, 5 deletions
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 10aa68ca82ef..bb9d47172162 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -2168,7 +2168,7 @@ nfsd_getxattr(struct svc_rqst *rqstp, struct svc_fh *fhp, char *name,
goto out;
}
- buf = kvmalloc(len, GFP_KERNEL | GFP_NOFS);
+ buf = kvmalloc(len, GFP_KERNEL);
if (buf == NULL) {
err = nfserr_jukebox;
goto out;
@@ -2231,10 +2231,7 @@ nfsd_listxattr(struct svc_rqst *rqstp, struct svc_fh *fhp, char **bufp,
goto out;
}
- /*
- * We're holding i_rwsem - use GFP_NOFS.
- */
- buf = kvmalloc(len, GFP_KERNEL | GFP_NOFS);
+ buf = kvmalloc(len, GFP_KERNEL);
if (buf == NULL) {
err = nfserr_jukebox;
goto out;