Skip to content

Commit

Permalink
nfsd: Push mnt_want_write() outside of i_mutex
Browse files Browse the repository at this point in the history
When mnt_want_write() starts to handle freezing it will get a full lock
semantics requiring proper lock ordering. So push mnt_want_write() call
consistently outside of i_mutex.

CC: linux-nfs@vger.kernel.org
CC: "J. Bruce Fields" <bfields@fieldses.org>
Signed-off-by: Jan Kara <jack@suse.cz>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
  • Loading branch information
Jan Kara authored and Al Viro committed Jul 30, 2012
1 parent e784868 commit 4a55c10
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 46 deletions.
9 changes: 5 additions & 4 deletions fs/nfsd/nfs4recover.c
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,10 @@ nfsd4_create_clid_dir(struct nfs4_client *clp)
if (status < 0)
return;

status = mnt_want_write_file(rec_file);
if (status)
return;

dir = rec_file->f_path.dentry;
/* lock the parent */
mutex_lock(&dir->d_inode->i_mutex);
Expand All @@ -173,11 +177,7 @@ nfsd4_create_clid_dir(struct nfs4_client *clp)
* as well be forgiving and just succeed silently.
*/
goto out_put;
status = mnt_want_write_file(rec_file);
if (status)
goto out_put;
status = vfs_mkdir(dir->d_inode, dentry, S_IRWXU);
mnt_drop_write_file(rec_file);
out_put:
dput(dentry);
out_unlock:
Expand All @@ -189,6 +189,7 @@ nfsd4_create_clid_dir(struct nfs4_client *clp)
" (err %d); please check that %s exists"
" and is writeable", status,
user_recovery_dirname);
mnt_drop_write_file(rec_file);
nfs4_reset_creds(original_cred);
}

Expand Down
1 change: 1 addition & 0 deletions fs/nfsd/nfsfh.c
Original file line number Diff line number Diff line change
Expand Up @@ -635,6 +635,7 @@ fh_put(struct svc_fh *fhp)
fhp->fh_post_saved = 0;
#endif
}
fh_drop_write(fhp);
if (exp) {
exp_put(exp);
fhp->fh_export = NULL;
Expand Down
9 changes: 8 additions & 1 deletion fs/nfsd/nfsproc.c
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ nfsd_proc_create(struct svc_rqst *rqstp, struct nfsd_createargs *argp,
struct dentry *dchild;
int type, mode;
__be32 nfserr;
int hosterr;
dev_t rdev = 0, wanted = new_decode_dev(attr->ia_size);

dprintk("nfsd: CREATE %s %.*s\n",
Expand All @@ -214,6 +215,12 @@ nfsd_proc_create(struct svc_rqst *rqstp, struct nfsd_createargs *argp,
nfserr = nfserr_exist;
if (isdotent(argp->name, argp->len))
goto done;
hosterr = fh_want_write(dirfhp);
if (hosterr) {
nfserr = nfserrno(hosterr);
goto done;
}

fh_lock_nested(dirfhp, I_MUTEX_PARENT);
dchild = lookup_one_len(argp->name, dirfhp->fh_dentry, argp->len);
if (IS_ERR(dchild)) {
Expand Down Expand Up @@ -330,7 +337,7 @@ nfsd_proc_create(struct svc_rqst *rqstp, struct nfsd_createargs *argp,
out_unlock:
/* We don't really need to unlock, as fh_put does it. */
fh_unlock(dirfhp);

fh_drop_write(dirfhp);
done:
fh_put(dirfhp);
return nfsd_return_dirop(nfserr, resp);
Expand Down
79 changes: 40 additions & 39 deletions fs/nfsd/vfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -1276,6 +1276,10 @@ nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
* If it has, the parent directory should already be locked.
*/
if (!resfhp->fh_dentry) {
host_err = fh_want_write(fhp);
if (host_err)
goto out_nfserr;

/* called from nfsd_proc_mkdir, or possibly nfsd3_proc_create */
fh_lock_nested(fhp, I_MUTEX_PARENT);
dchild = lookup_one_len(fname, dentry, flen);
Expand Down Expand Up @@ -1319,14 +1323,11 @@ nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
goto out;
}

host_err = fh_want_write(fhp);
if (host_err)
goto out_nfserr;

/*
* Get the dir op function pointer.
*/
err = 0;
host_err = 0;
switch (type) {
case S_IFREG:
host_err = vfs_create(dirp, dchild, iap->ia_mode, true);
Expand All @@ -1343,10 +1344,8 @@ nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
host_err = vfs_mknod(dirp, dchild, iap->ia_mode, rdev);
break;
}
if (host_err < 0) {
fh_drop_write(fhp);
if (host_err < 0)
goto out_nfserr;
}

err = nfsd_create_setattr(rqstp, resfhp, iap);

Expand All @@ -1358,7 +1357,6 @@ nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
err2 = nfserrno(commit_metadata(fhp));
if (err2)
err = err2;
fh_drop_write(fhp);
/*
* Update the file handle to get the new inode info.
*/
Expand Down Expand Up @@ -1417,6 +1415,11 @@ do_nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
err = nfserr_notdir;
if (!dirp->i_op->lookup)
goto out;

host_err = fh_want_write(fhp);
if (host_err)
goto out_nfserr;

fh_lock_nested(fhp, I_MUTEX_PARENT);

/*
Expand Down Expand Up @@ -1449,9 +1452,6 @@ do_nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
v_atime = verifier[1]&0x7fffffff;
}

host_err = fh_want_write(fhp);
if (host_err)
goto out_nfserr;
if (dchild->d_inode) {
err = 0;

Expand Down Expand Up @@ -1522,7 +1522,6 @@ do_nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
if (!err)
err = nfserrno(commit_metadata(fhp));

fh_drop_write(fhp);
/*
* Update the filehandle to get the new inode info.
*/
Expand All @@ -1533,6 +1532,7 @@ do_nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
fh_unlock(fhp);
if (dchild && !IS_ERR(dchild))
dput(dchild);
fh_drop_write(fhp);
return err;

out_nfserr:
Expand Down Expand Up @@ -1613,17 +1613,18 @@ nfsd_symlink(struct svc_rqst *rqstp, struct svc_fh *fhp,
err = fh_verify(rqstp, fhp, S_IFDIR, NFSD_MAY_CREATE);
if (err)
goto out;

host_err = fh_want_write(fhp);
if (host_err)
goto out_nfserr;

fh_lock(fhp);
dentry = fhp->fh_dentry;
dnew = lookup_one_len(fname, dentry, flen);
host_err = PTR_ERR(dnew);
if (IS_ERR(dnew))
goto out_nfserr;

host_err = fh_want_write(fhp);
if (host_err)
goto out_nfserr;

if (unlikely(path[plen] != 0)) {
char *path_alloced = kmalloc(plen+1, GFP_KERNEL);
if (path_alloced == NULL)
Expand Down Expand Up @@ -1683,6 +1684,12 @@ nfsd_link(struct svc_rqst *rqstp, struct svc_fh *ffhp,
if (isdotent(name, len))
goto out;

host_err = fh_want_write(tfhp);
if (host_err) {
err = nfserrno(host_err);
goto out;
}

fh_lock_nested(ffhp, I_MUTEX_PARENT);
ddir = ffhp->fh_dentry;
dirp = ddir->d_inode;
Expand All @@ -1694,18 +1701,13 @@ nfsd_link(struct svc_rqst *rqstp, struct svc_fh *ffhp,

dold = tfhp->fh_dentry;

host_err = fh_want_write(tfhp);
if (host_err) {
err = nfserrno(host_err);
goto out_dput;
}
err = nfserr_noent;
if (!dold->d_inode)
goto out_drop_write;
goto out_dput;
host_err = nfsd_break_lease(dold->d_inode);
if (host_err) {
err = nfserrno(host_err);
goto out_drop_write;
goto out_dput;
}
host_err = vfs_link(dold, dirp, dnew);
if (!host_err) {
Expand All @@ -1718,12 +1720,11 @@ nfsd_link(struct svc_rqst *rqstp, struct svc_fh *ffhp,
else
err = nfserrno(host_err);
}
out_drop_write:
fh_drop_write(tfhp);
out_dput:
dput(dnew);
out_unlock:
fh_unlock(ffhp);
fh_drop_write(tfhp);
out:
return err;

Expand Down Expand Up @@ -1766,6 +1767,12 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
if (!flen || isdotent(fname, flen) || !tlen || isdotent(tname, tlen))
goto out;

host_err = fh_want_write(ffhp);
if (host_err) {
err = nfserrno(host_err);
goto out;
}

/* cannot use fh_lock as we need deadlock protective ordering
* so do it by hand */
trap = lock_rename(tdentry, fdentry);
Expand Down Expand Up @@ -1796,26 +1803,21 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
host_err = -EXDEV;
if (ffhp->fh_export->ex_path.mnt != tfhp->fh_export->ex_path.mnt)
goto out_dput_new;
host_err = fh_want_write(ffhp);
if (host_err)
goto out_dput_new;

host_err = nfsd_break_lease(odentry->d_inode);
if (host_err)
goto out_drop_write;
goto out_dput_new;
if (ndentry->d_inode) {
host_err = nfsd_break_lease(ndentry->d_inode);
if (host_err)
goto out_drop_write;
goto out_dput_new;
}
host_err = vfs_rename(fdir, odentry, tdir, ndentry);
if (!host_err) {
host_err = commit_metadata(tfhp);
if (!host_err)
host_err = commit_metadata(ffhp);
}
out_drop_write:
fh_drop_write(ffhp);
out_dput_new:
dput(ndentry);
out_dput_old:
Expand All @@ -1831,6 +1833,7 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
fill_post_wcc(tfhp);
unlock_rename(tdentry, fdentry);
ffhp->fh_locked = tfhp->fh_locked = 0;
fh_drop_write(ffhp);

out:
return err;
Expand All @@ -1856,6 +1859,10 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type,
if (err)
goto out;

host_err = fh_want_write(fhp);
if (host_err)
goto out_nfserr;

fh_lock_nested(fhp, I_MUTEX_PARENT);
dentry = fhp->fh_dentry;
dirp = dentry->d_inode;
Expand All @@ -1874,21 +1881,15 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type,
if (!type)
type = rdentry->d_inode->i_mode & S_IFMT;

host_err = fh_want_write(fhp);
if (host_err)
goto out_put;

host_err = nfsd_break_lease(rdentry->d_inode);
if (host_err)
goto out_drop_write;
goto out_put;
if (type != S_IFDIR)
host_err = vfs_unlink(dirp, rdentry);
else
host_err = vfs_rmdir(dirp, rdentry);
if (!host_err)
host_err = commit_metadata(fhp);
out_drop_write:
fh_drop_write(fhp);
out_put:
dput(rdentry);

Expand Down
11 changes: 9 additions & 2 deletions fs/nfsd/vfs.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,19 @@ int nfsd_set_posix_acl(struct svc_fh *, int, struct posix_acl *);

static inline int fh_want_write(struct svc_fh *fh)
{
return mnt_want_write(fh->fh_export->ex_path.mnt);
int ret = mnt_want_write(fh->fh_export->ex_path.mnt);

if (!ret)
fh->fh_want_write = 1;
return ret;
}

static inline void fh_drop_write(struct svc_fh *fh)
{
mnt_drop_write(fh->fh_export->ex_path.mnt);
if (fh->fh_want_write) {
fh->fh_want_write = 0;
mnt_drop_write(fh->fh_export->ex_path.mnt);
}
}

#endif /* LINUX_NFSD_VFS_H */
1 change: 1 addition & 0 deletions include/linux/nfsd/nfsfh.h
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ typedef struct svc_fh {
int fh_maxsize; /* max size for fh_handle */

unsigned char fh_locked; /* inode locked by us */
unsigned char fh_want_write; /* remount protection taken */

#ifdef CONFIG_NFSD_V3
unsigned char fh_post_saved; /* post-op attrs saved */
Expand Down

0 comments on commit 4a55c10

Please sign in to comment.