Commit debf16f0 authored by NeilBrown's avatar NeilBrown Committed by Chuck Lever

NFSD: use explicit lock/unlock for directory ops

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: default avatarJeff Layton <jlayton@kernel.org>
Signed-off-by: default avatarNeilBrown <neilb@suse.de>
Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
parent 19d008b4
...@@ -260,7 +260,7 @@ nfsd3_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp, ...@@ -260,7 +260,7 @@ nfsd3_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
if (host_err) if (host_err)
return nfserrno(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); child = lookup_one_len(argp->name, parent, argp->len);
if (IS_ERR(child)) { if (IS_ERR(child)) {
...@@ -318,11 +318,13 @@ nfsd3_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp, ...@@ -318,11 +318,13 @@ nfsd3_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
if (!IS_POSIXACL(inode)) if (!IS_POSIXACL(inode))
iap->ia_mode &= ~current_umask(); iap->ia_mode &= ~current_umask();
fh_fill_pre_attrs(fhp);
host_err = vfs_create(&init_user_ns, inode, child, iap->ia_mode, true); host_err = vfs_create(&init_user_ns, inode, child, iap->ia_mode, true);
if (host_err < 0) { if (host_err < 0) {
status = nfserrno(host_err); status = nfserrno(host_err);
goto out; goto out;
} }
fh_fill_post_attrs(fhp);
/* A newly created file already has a file size of zero. */ /* A newly created file already has a file size of zero. */
if ((iap->ia_valid & ATTR_SIZE) && (iap->ia_size == 0)) if ((iap->ia_valid & ATTR_SIZE) && (iap->ia_size == 0))
...@@ -340,7 +342,7 @@ nfsd3_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp, ...@@ -340,7 +342,7 @@ nfsd3_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
status = nfsd_create_setattr(rqstp, fhp, resfhp, &attrs); status = nfsd_create_setattr(rqstp, fhp, resfhp, &attrs);
out: out:
fh_unlock(fhp); inode_unlock(inode);
if (child && !IS_ERR(child)) if (child && !IS_ERR(child))
dput(child); dput(child);
fh_drop_write(fhp); fh_drop_write(fhp);
......
...@@ -264,7 +264,7 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp, ...@@ -264,7 +264,7 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
if (is_create_with_attrs(open)) if (is_create_with_attrs(open))
nfsd4_acl_to_attr(NF4REG, open->op_acl, &attrs); 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); child = lookup_one_len(open->op_fname, parent, open->op_fnamelen);
if (IS_ERR(child)) { if (IS_ERR(child)) {
...@@ -348,10 +348,12 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp, ...@@ -348,10 +348,12 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
if (!IS_POSIXACL(inode)) if (!IS_POSIXACL(inode))
iap->ia_mode &= ~current_umask(); iap->ia_mode &= ~current_umask();
fh_fill_pre_attrs(fhp);
status = nfsd4_vfs_create(fhp, child, open); status = nfsd4_vfs_create(fhp, child, open);
if (status != nfs_ok) if (status != nfs_ok)
goto out; goto out;
open->op_created = true; open->op_created = true;
fh_fill_post_attrs(fhp);
/* A newly created file already has a file size of zero. */ /* A newly created file already has a file size of zero. */
if ((iap->ia_valid & ATTR_SIZE) && (iap->ia_size == 0)) if ((iap->ia_valid & ATTR_SIZE) && (iap->ia_size == 0))
...@@ -373,7 +375,7 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp, ...@@ -373,7 +375,7 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
if (attrs.na_aclerr) if (attrs.na_aclerr)
open->op_bmval[0] &= ~FATTR4_WORD0_ACL; open->op_bmval[0] &= ~FATTR4_WORD0_ACL;
out: out:
fh_unlock(fhp); inode_unlock(inode);
nfsd_attrs_free(&attrs); nfsd_attrs_free(&attrs);
if (child && !IS_ERR(child)) if (child && !IS_ERR(child))
dput(child); dput(child);
......
...@@ -291,7 +291,7 @@ nfsd_proc_create(struct svc_rqst *rqstp) ...@@ -291,7 +291,7 @@ nfsd_proc_create(struct svc_rqst *rqstp)
goto done; 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); dchild = lookup_one_len(argp->name, dirfhp->fh_dentry, argp->len);
if (IS_ERR(dchild)) { if (IS_ERR(dchild)) {
resp->status = nfserrno(PTR_ERR(dchild)); resp->status = nfserrno(PTR_ERR(dchild));
...@@ -407,8 +407,7 @@ nfsd_proc_create(struct svc_rqst *rqstp) ...@@ -407,8 +407,7 @@ nfsd_proc_create(struct svc_rqst *rqstp)
} }
out_unlock: out_unlock:
/* We don't really need to unlock, as fh_put does it. */ inode_unlock(dirfhp->fh_dentry->d_inode);
fh_unlock(dirfhp);
fh_drop_write(dirfhp); fh_drop_write(dirfhp);
done: done:
fh_put(dirfhp); fh_put(dirfhp);
......
...@@ -1370,7 +1370,7 @@ nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp, ...@@ -1370,7 +1370,7 @@ nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
if (host_err) if (host_err)
return nfserrno(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); dchild = lookup_one_len(fname, dentry, flen);
host_err = PTR_ERR(dchild); host_err = PTR_ERR(dchild);
if (IS_ERR(dchild)) { if (IS_ERR(dchild)) {
...@@ -1385,10 +1385,12 @@ nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp, ...@@ -1385,10 +1385,12 @@ nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
dput(dchild); dput(dchild);
if (err) if (err)
goto out_unlock; goto out_unlock;
fh_fill_pre_attrs(fhp);
err = nfsd_create_locked(rqstp, fhp, fname, flen, attrs, type, err = nfsd_create_locked(rqstp, fhp, fname, flen, attrs, type,
rdev, resfhp); rdev, resfhp);
fh_fill_post_attrs(fhp);
out_unlock: out_unlock:
fh_unlock(fhp); inode_unlock(dentry->d_inode);
return err; return err;
} }
...@@ -1471,20 +1473,22 @@ nfsd_symlink(struct svc_rqst *rqstp, struct svc_fh *fhp, ...@@ -1471,20 +1473,22 @@ nfsd_symlink(struct svc_rqst *rqstp, struct svc_fh *fhp,
goto out; goto out;
} }
fh_lock(fhp);
dentry = fhp->fh_dentry; dentry = fhp->fh_dentry;
inode_lock_nested(dentry->d_inode, I_MUTEX_PARENT);
dnew = lookup_one_len(fname, dentry, flen); dnew = lookup_one_len(fname, dentry, flen);
if (IS_ERR(dnew)) { if (IS_ERR(dnew)) {
err = nfserrno(PTR_ERR(dnew)); err = nfserrno(PTR_ERR(dnew));
fh_unlock(fhp); inode_unlock(dentry->d_inode);
goto out_drop_write; goto out_drop_write;
} }
fh_fill_pre_attrs(fhp);
host_err = vfs_symlink(&init_user_ns, d_inode(dentry), dnew, path); host_err = vfs_symlink(&init_user_ns, d_inode(dentry), dnew, path);
err = nfserrno(host_err); err = nfserrno(host_err);
cerr = fh_compose(resfhp, fhp->fh_export, dnew, fhp); cerr = fh_compose(resfhp, fhp->fh_export, dnew, fhp);
if (!err) if (!err)
nfsd_create_setattr(rqstp, fhp, resfhp, attrs); nfsd_create_setattr(rqstp, fhp, resfhp, attrs);
fh_unlock(fhp); fh_fill_post_attrs(fhp);
inode_unlock(dentry->d_inode);
if (!err) if (!err)
err = nfserrno(commit_metadata(fhp)); err = nfserrno(commit_metadata(fhp));
dput(dnew); dput(dnew);
...@@ -1530,9 +1534,9 @@ nfsd_link(struct svc_rqst *rqstp, struct svc_fh *ffhp, ...@@ -1530,9 +1534,9 @@ nfsd_link(struct svc_rqst *rqstp, struct svc_fh *ffhp,
goto out; goto out;
} }
fh_lock_nested(ffhp, I_MUTEX_PARENT);
ddir = ffhp->fh_dentry; ddir = ffhp->fh_dentry;
dirp = d_inode(ddir); dirp = d_inode(ddir);
inode_lock_nested(dirp, I_MUTEX_PARENT);
dnew = lookup_one_len(name, ddir, len); dnew = lookup_one_len(name, ddir, len);
if (IS_ERR(dnew)) { if (IS_ERR(dnew)) {
...@@ -1545,8 +1549,10 @@ nfsd_link(struct svc_rqst *rqstp, struct svc_fh *ffhp, ...@@ -1545,8 +1549,10 @@ nfsd_link(struct svc_rqst *rqstp, struct svc_fh *ffhp,
err = nfserr_noent; err = nfserr_noent;
if (d_really_is_negative(dold)) if (d_really_is_negative(dold))
goto out_dput; goto out_dput;
fh_fill_pre_attrs(ffhp);
host_err = vfs_link(dold, &init_user_ns, dirp, dnew, NULL); 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) { if (!host_err) {
err = nfserrno(commit_metadata(ffhp)); err = nfserrno(commit_metadata(ffhp));
if (!err) if (!err)
...@@ -1566,7 +1572,7 @@ nfsd_link(struct svc_rqst *rqstp, struct svc_fh *ffhp, ...@@ -1566,7 +1572,7 @@ nfsd_link(struct svc_rqst *rqstp, struct svc_fh *ffhp,
out_dput: out_dput:
dput(dnew); dput(dnew);
out_unlock: out_unlock:
fh_unlock(ffhp); inode_unlock(dirp);
goto out_drop_write; goto out_drop_write;
} }
...@@ -1741,9 +1747,9 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type, ...@@ -1741,9 +1747,9 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type,
if (host_err) if (host_err)
goto out_nfserr; goto out_nfserr;
fh_lock_nested(fhp, I_MUTEX_PARENT);
dentry = fhp->fh_dentry; dentry = fhp->fh_dentry;
dirp = d_inode(dentry); dirp = d_inode(dentry);
inode_lock_nested(dirp, I_MUTEX_PARENT);
rdentry = lookup_one_len(fname, dentry, flen); rdentry = lookup_one_len(fname, dentry, flen);
host_err = PTR_ERR(rdentry); host_err = PTR_ERR(rdentry);
...@@ -1761,6 +1767,7 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type, ...@@ -1761,6 +1767,7 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type,
if (!type) if (!type)
type = d_inode(rdentry)->i_mode & S_IFMT; type = d_inode(rdentry)->i_mode & S_IFMT;
fh_fill_pre_attrs(fhp);
if (type != S_IFDIR) { if (type != S_IFDIR) {
if (rdentry->d_sb->s_export_op->flags & EXPORT_OP_CLOSE_BEFORE_UNLINK) if (rdentry->d_sb->s_export_op->flags & EXPORT_OP_CLOSE_BEFORE_UNLINK)
nfsd_close_cached_files(rdentry); nfsd_close_cached_files(rdentry);
...@@ -1768,8 +1775,9 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type, ...@@ -1768,8 +1775,9 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type,
} else { } else {
host_err = vfs_rmdir(&init_user_ns, dirp, rdentry); host_err = vfs_rmdir(&init_user_ns, dirp, rdentry);
} }
fh_fill_post_attrs(fhp);
fh_unlock(fhp); inode_unlock(dirp);
if (!host_err) if (!host_err)
host_err = commit_metadata(fhp); host_err = commit_metadata(fhp);
dput(rdentry); dput(rdentry);
...@@ -1792,7 +1800,7 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type, ...@@ -1792,7 +1800,7 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type,
out: out:
return err; return err;
out_unlock: out_unlock:
fh_unlock(fhp); inode_unlock(dirp);
goto out_drop_write; goto out_drop_write;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment