Skip to content

Commit debf16f

Browse files
neilbrownchucklever
neilbrown
authored and
chucklever
committed
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: Jeff Layton <[email protected]> Signed-off-by: NeilBrown <[email protected]> Signed-off-by: Chuck Lever <[email protected]>
1 parent 19d008b commit debf16f

File tree

4 files changed

+29
-18
lines changed

4 files changed

+29
-18
lines changed

‎fs/nfsd/nfs3proc.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ nfsd3_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
260260
if (host_err)
261261
return nfserrno(host_err);
262262

263-
fh_lock_nested(fhp, I_MUTEX_PARENT);
263+
inode_lock_nested(inode, I_MUTEX_PARENT);
264264

265265
child = lookup_one_len(argp->name, parent, argp->len);
266266
if (IS_ERR(child)) {
@@ -318,11 +318,13 @@ nfsd3_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
318318
if (!IS_POSIXACL(inode))
319319
iap->ia_mode &= ~current_umask();
320320

321+
fh_fill_pre_attrs(fhp);
321322
host_err = vfs_create(&init_user_ns, inode, child, iap->ia_mode, true);
322323
if (host_err < 0) {
323324
status = nfserrno(host_err);
324325
goto out;
325326
}
327+
fh_fill_post_attrs(fhp);
326328

327329
/* A newly created file already has a file size of zero. */
328330
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,
340342
status = nfsd_create_setattr(rqstp, fhp, resfhp, &attrs);
341343

342344
out:
343-
fh_unlock(fhp);
345+
inode_unlock(inode);
344346
if (child && !IS_ERR(child))
345347
dput(child);
346348
fh_drop_write(fhp);

‎fs/nfsd/nfs4proc.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
264264
if (is_create_with_attrs(open))
265265
nfsd4_acl_to_attr(NF4REG, open->op_acl, &attrs);
266266

267-
fh_lock_nested(fhp, I_MUTEX_PARENT);
267+
inode_lock_nested(inode, I_MUTEX_PARENT);
268268

269269
child = lookup_one_len(open->op_fname, parent, open->op_fnamelen);
270270
if (IS_ERR(child)) {
@@ -348,10 +348,12 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
348348
if (!IS_POSIXACL(inode))
349349
iap->ia_mode &= ~current_umask();
350350

351+
fh_fill_pre_attrs(fhp);
351352
status = nfsd4_vfs_create(fhp, child, open);
352353
if (status != nfs_ok)
353354
goto out;
354355
open->op_created = true;
356+
fh_fill_post_attrs(fhp);
355357

356358
/* A newly created file already has a file size of zero. */
357359
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,
373375
if (attrs.na_aclerr)
374376
open->op_bmval[0] &= ~FATTR4_WORD0_ACL;
375377
out:
376-
fh_unlock(fhp);
378+
inode_unlock(inode);
377379
nfsd_attrs_free(&attrs);
378380
if (child && !IS_ERR(child))
379381
dput(child);

‎fs/nfsd/nfsproc.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ nfsd_proc_create(struct svc_rqst *rqstp)
291291
goto done;
292292
}
293293

294-
fh_lock_nested(dirfhp, I_MUTEX_PARENT);
294+
inode_lock_nested(dirfhp->fh_dentry->d_inode, I_MUTEX_PARENT);
295295
dchild = lookup_one_len(argp->name, dirfhp->fh_dentry, argp->len);
296296
if (IS_ERR(dchild)) {
297297
resp->status = nfserrno(PTR_ERR(dchild));
@@ -407,8 +407,7 @@ nfsd_proc_create(struct svc_rqst *rqstp)
407407
}
408408

409409
out_unlock:
410-
/* We don't really need to unlock, as fh_put does it. */
411-
fh_unlock(dirfhp);
410+
inode_unlock(dirfhp->fh_dentry->d_inode);
412411
fh_drop_write(dirfhp);
413412
done:
414413
fh_put(dirfhp);

‎fs/nfsd/vfs.c

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1370,7 +1370,7 @@ nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
13701370
if (host_err)
13711371
return nfserrno(host_err);
13721372

1373-
fh_lock_nested(fhp, I_MUTEX_PARENT);
1373+
inode_lock_nested(dentry->d_inode, I_MUTEX_PARENT);
13741374
dchild = lookup_one_len(fname, dentry, flen);
13751375
host_err = PTR_ERR(dchild);
13761376
if (IS_ERR(dchild)) {
@@ -1385,10 +1385,12 @@ nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
13851385
dput(dchild);
13861386
if (err)
13871387
goto out_unlock;
1388+
fh_fill_pre_attrs(fhp);
13881389
err = nfsd_create_locked(rqstp, fhp, fname, flen, attrs, type,
13891390
rdev, resfhp);
1391+
fh_fill_post_attrs(fhp);
13901392
out_unlock:
1391-
fh_unlock(fhp);
1393+
inode_unlock(dentry->d_inode);
13921394
return err;
13931395
}
13941396

@@ -1471,20 +1473,22 @@ nfsd_symlink(struct svc_rqst *rqstp, struct svc_fh *fhp,
14711473
goto out;
14721474
}
14731475

1474-
fh_lock(fhp);
14751476
dentry = fhp->fh_dentry;
1477+
inode_lock_nested(dentry->d_inode, I_MUTEX_PARENT);
14761478
dnew = lookup_one_len(fname, dentry, flen);
14771479
if (IS_ERR(dnew)) {
14781480
err = nfserrno(PTR_ERR(dnew));
1479-
fh_unlock(fhp);
1481+
inode_unlock(dentry->d_inode);
14801482
goto out_drop_write;
14811483
}
1484+
fh_fill_pre_attrs(fhp);
14821485
host_err = vfs_symlink(&init_user_ns, d_inode(dentry), dnew, path);
14831486
err = nfserrno(host_err);
14841487
cerr = fh_compose(resfhp, fhp->fh_export, dnew, fhp);
14851488
if (!err)
14861489
nfsd_create_setattr(rqstp, fhp, resfhp, attrs);
1487-
fh_unlock(fhp);
1490+
fh_fill_post_attrs(fhp);
1491+
inode_unlock(dentry->d_inode);
14881492
if (!err)
14891493
err = nfserrno(commit_metadata(fhp));
14901494
dput(dnew);
@@ -1530,9 +1534,9 @@ nfsd_link(struct svc_rqst *rqstp, struct svc_fh *ffhp,
15301534
goto out;
15311535
}
15321536

1533-
fh_lock_nested(ffhp, I_MUTEX_PARENT);
15341537
ddir = ffhp->fh_dentry;
15351538
dirp = d_inode(ddir);
1539+
inode_lock_nested(dirp, I_MUTEX_PARENT);
15361540

15371541
dnew = lookup_one_len(name, ddir, len);
15381542
if (IS_ERR(dnew)) {
@@ -1545,8 +1549,10 @@ nfsd_link(struct svc_rqst *rqstp, struct svc_fh *ffhp,
15451549
err = nfserr_noent;
15461550
if (d_really_is_negative(dold))
15471551
goto out_dput;
1552+
fh_fill_pre_attrs(ffhp);
15481553
host_err = vfs_link(dold, &init_user_ns, dirp, dnew, NULL);
1549-
fh_unlock(ffhp);
1554+
fh_fill_post_attrs(ffhp);
1555+
inode_unlock(dirp);
15501556
if (!host_err) {
15511557
err = nfserrno(commit_metadata(ffhp));
15521558
if (!err)
@@ -1566,7 +1572,7 @@ nfsd_link(struct svc_rqst *rqstp, struct svc_fh *ffhp,
15661572
out_dput:
15671573
dput(dnew);
15681574
out_unlock:
1569-
fh_unlock(ffhp);
1575+
inode_unlock(dirp);
15701576
goto out_drop_write;
15711577
}
15721578

@@ -1741,9 +1747,9 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type,
17411747
if (host_err)
17421748
goto out_nfserr;
17431749

1744-
fh_lock_nested(fhp, I_MUTEX_PARENT);
17451750
dentry = fhp->fh_dentry;
17461751
dirp = d_inode(dentry);
1752+
inode_lock_nested(dirp, I_MUTEX_PARENT);
17471753

17481754
rdentry = lookup_one_len(fname, dentry, flen);
17491755
host_err = PTR_ERR(rdentry);
@@ -1761,15 +1767,17 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type,
17611767
if (!type)
17621768
type = d_inode(rdentry)->i_mode & S_IFMT;
17631769

1770+
fh_fill_pre_attrs(fhp);
17641771
if (type != S_IFDIR) {
17651772
if (rdentry->d_sb->s_export_op->flags & EXPORT_OP_CLOSE_BEFORE_UNLINK)
17661773
nfsd_close_cached_files(rdentry);
17671774
host_err = vfs_unlink(&init_user_ns, dirp, rdentry, NULL);
17681775
} else {
17691776
host_err = vfs_rmdir(&init_user_ns, dirp, rdentry);
17701777
}
1778+
fh_fill_post_attrs(fhp);
17711779

1772-
fh_unlock(fhp);
1780+
inode_unlock(dirp);
17731781
if (!host_err)
17741782
host_err = commit_metadata(fhp);
17751783
dput(rdentry);
@@ -1792,7 +1800,7 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type,
17921800
out:
17931801
return err;
17941802
out_unlock:
1795-
fh_unlock(fhp);
1803+
inode_unlock(dirp);
17961804
goto out_drop_write;
17971805
}
17981806

0 commit comments

Comments
 (0)