NFSD: use explicit lock/unlock for directory ops

[ Upstream commit debf16f0c671cb8db154a9ebcd6014cfff683b80 ]

When creating or unlinking a name in a directory use explicit
inode_lock_nested() instead of fh_lock(), and explicit calls to
fh_fill_pre_attrs() and fh_fill_post_attrs().  This is already done
for renames, with lock_rename() as the explicit locking.

Also move the 'fill' calls closer to the operation that might change the
attributes.  This way they are avoided on some error paths.

For the v2-only code in nfsproc.c, the fill calls are not replaced as
they aren't needed.

Making the locking explicit will simplify proposed future changes to
locking for directories.  It also makes it easily visible exactly where
pre/post attributes are used - not all callers of fh_lock() actually
need the pre/post attributes.

Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: NeilBrown <neilb@suse.de>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>

diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
index fbdc109fbd06736213b4c7ea17ab22faaa23ef09..5b1e771238b352f0724249f360431b6352f597c3 100644 (file)
--- a/fs/nfsd/nfs3proc.c
+++ b/fs/nfsd/nfs3proc.c
@@ -260,7 +260,7 @@ nfsd3_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
        if (host_err)
                return nfserrno(host_err);
 
-       fh_lock_nested(fhp, I_MUTEX_PARENT);
+       inode_lock_nested(inode, I_MUTEX_PARENT);
 
        child = lookup_one_len(argp->name, parent, argp->len);
        if (IS_ERR(child)) {
@@ -318,11 +318,13 @@ nfsd3_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
        if (!IS_POSIXACL(inode))
                iap->ia_mode &= ~current_umask();
 
+       fh_fill_pre_attrs(fhp);
        host_err = vfs_create(&init_user_ns, inode, child, iap->ia_mode, true);
        if (host_err < 0) {
                status = nfserrno(host_err);
                goto out;
        }
+       fh_fill_post_attrs(fhp);
 
        /* A newly created file already has a file size of zero. */
        if ((iap->ia_valid & ATTR_SIZE) && (iap->ia_size == 0))
@@ -340,7 +342,7 @@ set_attr:
        status = nfsd_create_setattr(rqstp, fhp, resfhp, &attrs);
 
 out:
-       fh_unlock(fhp);
+       inode_unlock(inode);
        if (child && !IS_ERR(child))
                dput(child);
        fh_drop_write(fhp);

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index b6df56fb6755de853b4f198f4bec4b641c9fbab1..5e4b7858b2e501dfccebb618c346d87e6bd0db2e 100644 (file)
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -264,7 +264,7 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
        if (is_create_with_attrs(open))
                nfsd4_acl_to_attr(NF4REG, open->op_acl, &attrs);
 
-       fh_lock_nested(fhp, I_MUTEX_PARENT);
+       inode_lock_nested(inode, I_MUTEX_PARENT);
 
        child = lookup_one_len(open->op_fname, parent, open->op_fnamelen);
        if (IS_ERR(child)) {
@@ -348,10 +348,12 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
        if (!IS_POSIXACL(inode))
                iap->ia_mode &= ~current_umask();
 
+       fh_fill_pre_attrs(fhp);
        status = nfsd4_vfs_create(fhp, child, open);
        if (status != nfs_ok)
                goto out;
        open->op_created = true;
+       fh_fill_post_attrs(fhp);
 
        /* A newly created file already has a file size of zero. */
        if ((iap->ia_valid & ATTR_SIZE) && (iap->ia_size == 0))
@@ -373,7 +375,7 @@ set_attr:
        if (attrs.na_aclerr)
                open->op_bmval[0] &= ~FATTR4_WORD0_ACL;
 out:
-       fh_unlock(fhp);
+       inode_unlock(inode);
        nfsd_attrs_free(&attrs);
        if (child && !IS_ERR(child))
                dput(child);

diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c
index 09afd188099be8c998a218dbbbb9a1904f0e6328..4b19cc727ea50b1a2a3f796cbf831992e8a7357e 100644 (file)
--- a/fs/nfsd/nfsproc.c
+++ b/fs/nfsd/nfsproc.c
@@ -292,7 +292,7 @@ nfsd_proc_create(struct svc_rqst *rqstp)
                goto done;
        }
 
-       fh_lock_nested(dirfhp, I_MUTEX_PARENT);
+       inode_lock_nested(dirfhp->fh_dentry->d_inode, I_MUTEX_PARENT);
        dchild = lookup_one_len(argp->name, dirfhp->fh_dentry, argp->len);
        if (IS_ERR(dchild)) {
                resp->status = nfserrno(PTR_ERR(dchild));
@@ -408,8 +408,7 @@ nfsd_proc_create(struct svc_rqst *rqstp)
        }
 
 out_unlock:
-       /* We don't really need to unlock, as fh_put does it. */
-       fh_unlock(dirfhp);
+       inode_unlock(dirfhp->fh_dentry->d_inode);
        fh_drop_write(dirfhp);
 done:
        fh_put(dirfhp);

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index c07fe50d6bdfb9821c6cdb1f5412ab230a3d67c1..7de76b37a9bc23773c21c6a915f2e4765ccc3e50 100644 (file)
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1371,7 +1371,7 @@ nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
        if (host_err)
                return nfserrno(host_err);
 
-       fh_lock_nested(fhp, I_MUTEX_PARENT);
+       inode_lock_nested(dentry->d_inode, I_MUTEX_PARENT);
        dchild = lookup_one_len(fname, dentry, flen);
        host_err = PTR_ERR(dchild);
        if (IS_ERR(dchild)) {
@@ -1386,10 +1386,12 @@ nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
        dput(dchild);
        if (err)
                goto out_unlock;
+       fh_fill_pre_attrs(fhp);
        err = nfsd_create_locked(rqstp, fhp, fname, flen, attrs, type,
                                 rdev, resfhp);
+       fh_fill_post_attrs(fhp);
 out_unlock:
-       fh_unlock(fhp);
+       inode_unlock(dentry->d_inode);
        return err;
 }
 
@@ -1472,20 +1474,22 @@ nfsd_symlink(struct svc_rqst *rqstp, struct svc_fh *fhp,
                goto out;
        }
 
-       fh_lock(fhp);
        dentry = fhp->fh_dentry;
+       inode_lock_nested(dentry->d_inode, I_MUTEX_PARENT);
        dnew = lookup_one_len(fname, dentry, flen);
        if (IS_ERR(dnew)) {
                err = nfserrno(PTR_ERR(dnew));
-               fh_unlock(fhp);
+               inode_unlock(dentry->d_inode);
                goto out_drop_write;
        }
+       fh_fill_pre_attrs(fhp);
        host_err = vfs_symlink(&init_user_ns, d_inode(dentry), dnew, path);
        err = nfserrno(host_err);
        cerr = fh_compose(resfhp, fhp->fh_export, dnew, fhp);
        if (!err)
                nfsd_create_setattr(rqstp, fhp, resfhp, attrs);
-       fh_unlock(fhp);
+       fh_fill_post_attrs(fhp);
+       inode_unlock(dentry->d_inode);
        if (!err)
                err = nfserrno(commit_metadata(fhp));
        dput(dnew);
@@ -1531,9 +1535,9 @@ nfsd_link(struct svc_rqst *rqstp, struct svc_fh *ffhp,
                goto out;
        }
 
-       fh_lock_nested(ffhp, I_MUTEX_PARENT);
        ddir = ffhp->fh_dentry;
        dirp = d_inode(ddir);
+       inode_lock_nested(dirp, I_MUTEX_PARENT);
 
        dnew = lookup_one_len(name, ddir, len);
        if (IS_ERR(dnew)) {
@@ -1546,8 +1550,10 @@ nfsd_link(struct svc_rqst *rqstp, struct svc_fh *ffhp,
        err = nfserr_noent;
        if (d_really_is_negative(dold))
                goto out_dput;
+       fh_fill_pre_attrs(ffhp);
        host_err = vfs_link(dold, &init_user_ns, dirp, dnew, NULL);
-       fh_unlock(ffhp);
+       fh_fill_post_attrs(ffhp);
+       inode_unlock(dirp);
        if (!host_err) {
                err = nfserrno(commit_metadata(ffhp));
                if (!err)
@@ -1567,7 +1573,7 @@ out:
 out_dput:
        dput(dnew);
 out_unlock:
-       fh_unlock(ffhp);
+       inode_unlock(dirp);
        goto out_drop_write;
 }
 
@@ -1742,9 +1748,9 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type,
        if (host_err)
                goto out_nfserr;
 
-       fh_lock_nested(fhp, I_MUTEX_PARENT);
        dentry = fhp->fh_dentry;
        dirp = d_inode(dentry);
+       inode_lock_nested(dirp, I_MUTEX_PARENT);
 
        rdentry = lookup_one_len(fname, dentry, flen);
        host_err = PTR_ERR(rdentry);
@@ -1762,6 +1768,7 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type,
        if (!type)
                type = d_inode(rdentry)->i_mode & S_IFMT;
 
+       fh_fill_pre_attrs(fhp);
        if (type != S_IFDIR) {
                if (rdentry->d_sb->s_export_op->flags & EXPORT_OP_CLOSE_BEFORE_UNLINK)
                        nfsd_close_cached_files(rdentry);
@@ -1769,8 +1776,9 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type,
        } else {
                host_err = vfs_rmdir(&init_user_ns, dirp, rdentry);
        }
+       fh_fill_post_attrs(fhp);
 
-       fh_unlock(fhp);
+       inode_unlock(dirp);
        if (!host_err)
                host_err = commit_metadata(fhp);
        dput(rdentry);
@@ -1793,7 +1801,7 @@ out_nfserr:
 out:
        return err;
 out_unlock:
-       fh_unlock(fhp);
+       inode_unlock(dirp);
        goto out_drop_write;
 }