Skip to content

Commit 829e694

Browse files
committed
Merge tag 'nfs-for-5.6-2' of git://git.linux-nfs.org/projects/anna/linux-nfs
Pull NFS client bugfixes from Anna Schumaker: "The only stable fix this time is the DMA scatter-gather list bug fixed by Chuck. The rest fix up races and refcounting issues that have been found during testing. Stable fix: - fix DMA scatter-gather list mapping imbalance The rest: - fix directory verifier races - fix races between open and dentry revalidation - fix revalidation of dentries with delegations - fix "cachethis" setting for writes - fix delegation and delegation cred pinning" * tag 'nfs-for-5.6-2' of git://git.linux-nfs.org/projects/anna/linux-nfs: NFSv4: Ensure the delegation cred is pinned when we call delegreturn NFSv4: Ensure the delegation is pinned in nfs_do_return_delegation() NFSv4.1 make cachethis=no for writes xprtrdma: Fix DMA scatter-gather list mapping imbalance NFSv4: Fix revalidation of dentries with delegations NFSv4: Fix races between open and dentry revalidation NFS: Fix up directory verifier races
2 parents cf556ed + 5d63944 commit 829e694

File tree

8 files changed

+188
-50
lines changed

8 files changed

+188
-50
lines changed

fs/nfs/delegation.c

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,27 @@ static void nfs_mark_delegation_revoked(struct nfs_delegation *delegation)
4242
if (!test_and_set_bit(NFS_DELEGATION_REVOKED, &delegation->flags)) {
4343
delegation->stateid.type = NFS4_INVALID_STATEID_TYPE;
4444
atomic_long_dec(&nfs_active_delegations);
45+
if (!test_bit(NFS_DELEGATION_RETURNING, &delegation->flags))
46+
nfs_clear_verifier_delegated(delegation->inode);
4547
}
4648
}
4749

50+
static struct nfs_delegation *nfs_get_delegation(struct nfs_delegation *delegation)
51+
{
52+
refcount_inc(&delegation->refcount);
53+
return delegation;
54+
}
55+
56+
static void nfs_put_delegation(struct nfs_delegation *delegation)
57+
{
58+
if (refcount_dec_and_test(&delegation->refcount))
59+
__nfs_free_delegation(delegation);
60+
}
61+
4862
static void nfs_free_delegation(struct nfs_delegation *delegation)
4963
{
5064
nfs_mark_delegation_revoked(delegation);
51-
__nfs_free_delegation(delegation);
65+
nfs_put_delegation(delegation);
5266
}
5367

5468
/**
@@ -241,13 +255,18 @@ void nfs_inode_reclaim_delegation(struct inode *inode, const struct cred *cred,
241255

242256
static int nfs_do_return_delegation(struct inode *inode, struct nfs_delegation *delegation, int issync)
243257
{
258+
const struct cred *cred;
244259
int res = 0;
245260

246-
if (!test_bit(NFS_DELEGATION_REVOKED, &delegation->flags))
247-
res = nfs4_proc_delegreturn(inode,
248-
delegation->cred,
261+
if (!test_bit(NFS_DELEGATION_REVOKED, &delegation->flags)) {
262+
spin_lock(&delegation->lock);
263+
cred = get_cred(delegation->cred);
264+
spin_unlock(&delegation->lock);
265+
res = nfs4_proc_delegreturn(inode, cred,
249266
&delegation->stateid,
250267
issync);
268+
put_cred(cred);
269+
}
251270
return res;
252271
}
253272

@@ -273,9 +292,13 @@ nfs_start_delegation_return_locked(struct nfs_inode *nfsi)
273292
if (delegation == NULL)
274293
goto out;
275294
spin_lock(&delegation->lock);
276-
if (!test_and_set_bit(NFS_DELEGATION_RETURNING, &delegation->flags))
277-
ret = delegation;
295+
if (!test_and_set_bit(NFS_DELEGATION_RETURNING, &delegation->flags)) {
296+
/* Refcount matched in nfs_end_delegation_return() */
297+
ret = nfs_get_delegation(delegation);
298+
}
278299
spin_unlock(&delegation->lock);
300+
if (ret)
301+
nfs_clear_verifier_delegated(&nfsi->vfs_inode);
279302
out:
280303
return ret;
281304
}
@@ -393,6 +416,7 @@ int nfs_inode_set_delegation(struct inode *inode, const struct cred *cred,
393416
if (delegation == NULL)
394417
return -ENOMEM;
395418
nfs4_stateid_copy(&delegation->stateid, stateid);
419+
refcount_set(&delegation->refcount, 1);
396420
delegation->type = type;
397421
delegation->pagemod_limit = pagemod_limit;
398422
delegation->change_attr = inode_peek_iversion_raw(inode);
@@ -492,6 +516,8 @@ static int nfs_end_delegation_return(struct inode *inode, struct nfs_delegation
492516

493517
err = nfs_do_return_delegation(inode, delegation, issync);
494518
out:
519+
/* Refcount matched in nfs_start_delegation_return_locked() */
520+
nfs_put_delegation(delegation);
495521
return err;
496522
}
497523

@@ -686,9 +712,12 @@ void nfs4_inode_return_delegation_on_close(struct inode *inode)
686712
list_empty(&NFS_I(inode)->open_files) &&
687713
!test_and_set_bit(NFS_DELEGATION_RETURNING, &delegation->flags)) {
688714
clear_bit(NFS_DELEGATION_RETURN_IF_CLOSED, &delegation->flags);
689-
ret = delegation;
715+
/* Refcount matched in nfs_end_delegation_return() */
716+
ret = nfs_get_delegation(delegation);
690717
}
691718
spin_unlock(&delegation->lock);
719+
if (ret)
720+
nfs_clear_verifier_delegated(inode);
692721
}
693722
out:
694723
rcu_read_unlock();
@@ -1088,10 +1117,11 @@ void nfs_delegation_reap_unclaimed(struct nfs_client *clp)
10881117
delegation = nfs_start_delegation_return_locked(NFS_I(inode));
10891118
rcu_read_unlock();
10901119
if (delegation != NULL) {
1091-
delegation = nfs_detach_delegation(NFS_I(inode),
1092-
delegation, server);
1093-
if (delegation != NULL)
1120+
if (nfs_detach_delegation(NFS_I(inode), delegation,
1121+
server) != NULL)
10941122
nfs_free_delegation(delegation);
1123+
/* Match nfs_start_delegation_return_locked */
1124+
nfs_put_delegation(delegation);
10951125
}
10961126
iput(inode);
10971127
nfs_sb_deactive(server->super);

fs/nfs/delegation.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ struct nfs_delegation {
2222
unsigned long pagemod_limit;
2323
__u64 change_attr;
2424
unsigned long flags;
25+
refcount_t refcount;
2526
spinlock_t lock;
2627
struct rcu_head rcu;
2728
};

fs/nfs/dir.c

Lines changed: 116 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ typedef struct {
155155
loff_t current_index;
156156
decode_dirent_t decode;
157157

158+
unsigned long dir_verifier;
158159
unsigned long timestamp;
159160
unsigned long gencount;
160161
unsigned int cache_entry_index;
@@ -353,6 +354,7 @@ int nfs_readdir_xdr_filler(struct page **pages, nfs_readdir_descriptor_t *desc,
353354
again:
354355
timestamp = jiffies;
355356
gencount = nfs_inc_attr_generation_counter();
357+
desc->dir_verifier = nfs_save_change_attribute(inode);
356358
error = NFS_PROTO(inode)->readdir(file_dentry(file), cred, entry->cookie, pages,
357359
NFS_SERVER(inode)->dtsize, desc->plus);
358360
if (error < 0) {
@@ -455,13 +457,13 @@ void nfs_force_use_readdirplus(struct inode *dir)
455457
}
456458

457459
static
458-
void nfs_prime_dcache(struct dentry *parent, struct nfs_entry *entry)
460+
void nfs_prime_dcache(struct dentry *parent, struct nfs_entry *entry,
461+
unsigned long dir_verifier)
459462
{
460463
struct qstr filename = QSTR_INIT(entry->name, entry->len);
461464
DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wq);
462465
struct dentry *dentry;
463466
struct dentry *alias;
464-
struct inode *dir = d_inode(parent);
465467
struct inode *inode;
466468
int status;
467469

@@ -500,7 +502,7 @@ void nfs_prime_dcache(struct dentry *parent, struct nfs_entry *entry)
500502
if (nfs_same_file(dentry, entry)) {
501503
if (!entry->fh->size)
502504
goto out;
503-
nfs_set_verifier(dentry, nfs_save_change_attribute(dir));
505+
nfs_set_verifier(dentry, dir_verifier);
504506
status = nfs_refresh_inode(d_inode(dentry), entry->fattr);
505507
if (!status)
506508
nfs_setsecurity(d_inode(dentry), entry->fattr, entry->label);
@@ -526,7 +528,7 @@ void nfs_prime_dcache(struct dentry *parent, struct nfs_entry *entry)
526528
dput(dentry);
527529
dentry = alias;
528530
}
529-
nfs_set_verifier(dentry, nfs_save_change_attribute(dir));
531+
nfs_set_verifier(dentry, dir_verifier);
530532
out:
531533
dput(dentry);
532534
}
@@ -564,7 +566,8 @@ int nfs_readdir_page_filler(nfs_readdir_descriptor_t *desc, struct nfs_entry *en
564566
count++;
565567

566568
if (desc->plus)
567-
nfs_prime_dcache(file_dentry(desc->file), entry);
569+
nfs_prime_dcache(file_dentry(desc->file), entry,
570+
desc->dir_verifier);
568571

569572
status = nfs_readdir_add_to_array(entry, page);
570573
if (status != 0)
@@ -983,14 +986,113 @@ static int nfs_fsync_dir(struct file *filp, loff_t start, loff_t end,
983986
* full lookup on all child dentries of 'dir' whenever a change occurs
984987
* on the server that might have invalidated our dcache.
985988
*
989+
* Note that we reserve bit '0' as a tag to let us know when a dentry
990+
* was revalidated while holding a delegation on its inode.
991+
*
986992
* The caller should be holding dir->i_lock
987993
*/
988994
void nfs_force_lookup_revalidate(struct inode *dir)
989995
{
990-
NFS_I(dir)->cache_change_attribute++;
996+
NFS_I(dir)->cache_change_attribute += 2;
991997
}
992998
EXPORT_SYMBOL_GPL(nfs_force_lookup_revalidate);
993999

1000+
/**
1001+
* nfs_verify_change_attribute - Detects NFS remote directory changes
1002+
* @dir: pointer to parent directory inode
1003+
* @verf: previously saved change attribute
1004+
*
1005+
* Return "false" if the verifiers doesn't match the change attribute.
1006+
* This would usually indicate that the directory contents have changed on
1007+
* the server, and that any dentries need revalidating.
1008+
*/
1009+
static bool nfs_verify_change_attribute(struct inode *dir, unsigned long verf)
1010+
{
1011+
return (verf & ~1UL) == nfs_save_change_attribute(dir);
1012+
}
1013+
1014+
static void nfs_set_verifier_delegated(unsigned long *verf)
1015+
{
1016+
*verf |= 1UL;
1017+
}
1018+
1019+
#if IS_ENABLED(CONFIG_NFS_V4)
1020+
static void nfs_unset_verifier_delegated(unsigned long *verf)
1021+
{
1022+
*verf &= ~1UL;
1023+
}
1024+
#endif /* IS_ENABLED(CONFIG_NFS_V4) */
1025+
1026+
static bool nfs_test_verifier_delegated(unsigned long verf)
1027+
{
1028+
return verf & 1;
1029+
}
1030+
1031+
static bool nfs_verifier_is_delegated(struct dentry *dentry)
1032+
{
1033+
return nfs_test_verifier_delegated(dentry->d_time);
1034+
}
1035+
1036+
static void nfs_set_verifier_locked(struct dentry *dentry, unsigned long verf)
1037+
{
1038+
struct inode *inode = d_inode(dentry);
1039+
1040+
if (!nfs_verifier_is_delegated(dentry) &&
1041+
!nfs_verify_change_attribute(d_inode(dentry->d_parent), verf))
1042+
goto out;
1043+
if (inode && NFS_PROTO(inode)->have_delegation(inode, FMODE_READ))
1044+
nfs_set_verifier_delegated(&verf);
1045+
out:
1046+
dentry->d_time = verf;
1047+
}
1048+
1049+
/**
1050+
* nfs_set_verifier - save a parent directory verifier in the dentry
1051+
* @dentry: pointer to dentry
1052+
* @verf: verifier to save
1053+
*
1054+
* Saves the parent directory verifier in @dentry. If the inode has
1055+
* a delegation, we also tag the dentry as having been revalidated
1056+
* while holding a delegation so that we know we don't have to
1057+
* look it up again after a directory change.
1058+
*/
1059+
void nfs_set_verifier(struct dentry *dentry, unsigned long verf)
1060+
{
1061+
1062+
spin_lock(&dentry->d_lock);
1063+
nfs_set_verifier_locked(dentry, verf);
1064+
spin_unlock(&dentry->d_lock);
1065+
}
1066+
EXPORT_SYMBOL_GPL(nfs_set_verifier);
1067+
1068+
#if IS_ENABLED(CONFIG_NFS_V4)
1069+
/**
1070+
* nfs_clear_verifier_delegated - clear the dir verifier delegation tag
1071+
* @inode: pointer to inode
1072+
*
1073+
* Iterates through the dentries in the inode alias list and clears
1074+
* the tag used to indicate that the dentry has been revalidated
1075+
* while holding a delegation.
1076+
* This function is intended for use when the delegation is being
1077+
* returned or revoked.
1078+
*/
1079+
void nfs_clear_verifier_delegated(struct inode *inode)
1080+
{
1081+
struct dentry *alias;
1082+
1083+
if (!inode)
1084+
return;
1085+
spin_lock(&inode->i_lock);
1086+
hlist_for_each_entry(alias, &inode->i_dentry, d_u.d_alias) {
1087+
spin_lock(&alias->d_lock);
1088+
nfs_unset_verifier_delegated(&alias->d_time);
1089+
spin_unlock(&alias->d_lock);
1090+
}
1091+
spin_unlock(&inode->i_lock);
1092+
}
1093+
EXPORT_SYMBOL_GPL(nfs_clear_verifier_delegated);
1094+
#endif /* IS_ENABLED(CONFIG_NFS_V4) */
1095+
9941096
/*
9951097
* A check for whether or not the parent directory has changed.
9961098
* In the case it has, we assume that the dentries are untrustworthy
@@ -1159,6 +1261,7 @@ nfs_lookup_revalidate_dentry(struct inode *dir, struct dentry *dentry,
11591261
struct nfs_fh *fhandle;
11601262
struct nfs_fattr *fattr;
11611263
struct nfs4_label *label;
1264+
unsigned long dir_verifier;
11621265
int ret;
11631266

11641267
ret = -ENOMEM;
@@ -1168,6 +1271,7 @@ nfs_lookup_revalidate_dentry(struct inode *dir, struct dentry *dentry,
11681271
if (fhandle == NULL || fattr == NULL || IS_ERR(label))
11691272
goto out;
11701273

1274+
dir_verifier = nfs_save_change_attribute(dir);
11711275
ret = NFS_PROTO(dir)->lookup(dir, dentry, fhandle, fattr, label);
11721276
if (ret < 0) {
11731277
switch (ret) {
@@ -1188,7 +1292,7 @@ nfs_lookup_revalidate_dentry(struct inode *dir, struct dentry *dentry,
11881292
goto out;
11891293

11901294
nfs_setsecurity(inode, fattr, label);
1191-
nfs_set_verifier(dentry, nfs_save_change_attribute(dir));
1295+
nfs_set_verifier(dentry, dir_verifier);
11921296

11931297
/* set a readdirplus hint that we had a cache miss */
11941298
nfs_force_use_readdirplus(dir);
@@ -1230,7 +1334,7 @@ nfs_do_lookup_revalidate(struct inode *dir, struct dentry *dentry,
12301334
goto out_bad;
12311335
}
12321336

1233-
if (NFS_PROTO(dir)->have_delegation(inode, FMODE_READ))
1337+
if (nfs_verifier_is_delegated(dentry))
12341338
return nfs_lookup_revalidate_delegated(dir, dentry, inode);
12351339

12361340
/* Force a full look up iff the parent directory has changed */
@@ -1415,6 +1519,7 @@ struct dentry *nfs_lookup(struct inode *dir, struct dentry * dentry, unsigned in
14151519
struct nfs_fh *fhandle = NULL;
14161520
struct nfs_fattr *fattr = NULL;
14171521
struct nfs4_label *label = NULL;
1522+
unsigned long dir_verifier;
14181523
int error;
14191524

14201525
dfprintk(VFS, "NFS: lookup(%pd2)\n", dentry);
@@ -1440,6 +1545,7 @@ struct dentry *nfs_lookup(struct inode *dir, struct dentry * dentry, unsigned in
14401545
if (IS_ERR(label))
14411546
goto out;
14421547

1548+
dir_verifier = nfs_save_change_attribute(dir);
14431549
trace_nfs_lookup_enter(dir, dentry, flags);
14441550
error = NFS_PROTO(dir)->lookup(dir, dentry, fhandle, fattr, label);
14451551
if (error == -ENOENT)
@@ -1463,7 +1569,7 @@ struct dentry *nfs_lookup(struct inode *dir, struct dentry * dentry, unsigned in
14631569
goto out_label;
14641570
dentry = res;
14651571
}
1466-
nfs_set_verifier(dentry, nfs_save_change_attribute(dir));
1572+
nfs_set_verifier(dentry, dir_verifier);
14671573
out_label:
14681574
trace_nfs_lookup_exit(dir, dentry, flags, error);
14691575
nfs4_label_free(label);
@@ -1668,7 +1774,7 @@ nfs4_do_lookup_revalidate(struct inode *dir, struct dentry *dentry,
16681774
if (inode == NULL)
16691775
goto full_reval;
16701776

1671-
if (NFS_PROTO(dir)->have_delegation(inode, FMODE_READ))
1777+
if (nfs_verifier_is_delegated(dentry))
16721778
return nfs_lookup_revalidate_delegated(dir, dentry, inode);
16731779

16741780
/* NFS only supports OPEN on regular files */

fs/nfs/inode.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2114,6 +2114,7 @@ static void init_once(void *foo)
21142114
init_rwsem(&nfsi->rmdir_sem);
21152115
mutex_init(&nfsi->commit_mutex);
21162116
nfs4_init_once(nfsi);
2117+
nfsi->cache_change_attribute = 0;
21172118
}
21182119

21192120
static int __init nfs_init_inodecache(void)

fs/nfs/nfs4file.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,6 @@ nfs4_file_open(struct inode *inode, struct file *filp)
8787
if (inode != d_inode(dentry))
8888
goto out_drop;
8989

90-
nfs_set_verifier(dentry, nfs_save_change_attribute(dir));
9190
nfs_file_set_open_context(filp, ctx);
9291
nfs_fscache_open_file(inode, filp);
9392
err = 0;

0 commit comments

Comments
 (0)