Skip to content

Commit f642404

Browse files
dhowellsdhowells
committed
afs: Make vnode->cb_interest RCU safe
Use RCU-based freeing for afs_cb_interest struct objects and use RCU on vnode->cb_interest. Use that change to allow afs_check_validity() to use read_seqbegin_or_lock() instead of read_seqlock_excl(). This also requires the caller of afs_check_validity() to hold the RCU read lock across the call. Signed-off-by: David Howells <[email protected]>
1 parent c925bd0 commit f642404

File tree

7 files changed

+100
-59
lines changed

7 files changed

+100
-59
lines changed

‎fs/afs/callback.c‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -94,15 +94,15 @@ int afs_register_server_cb_interest(struct afs_vnode *vnode,
9494
struct afs_server *server = entry->server;
9595

9696
again:
97-
if (vnode->cb_interest &&
98-
likely(vnode->cb_interest == entry->cb_interest))
97+
vcbi = rcu_dereference_protected(vnode->cb_interest,
98+
lockdep_is_held(&vnode->io_lock));
99+
if (vcbi && likely(vcbi == entry->cb_interest))
99100
return 0;
100101

101102
read_lock(&slist->lock);
102103
cbi = afs_get_cb_interest(entry->cb_interest);
103104
read_unlock(&slist->lock);
104105

105-
vcbi = vnode->cb_interest;
106106
if (vcbi) {
107107
if (vcbi == cbi) {
108108
afs_put_cb_interest(afs_v2net(vnode), cbi);
@@ -114,8 +114,9 @@ int afs_register_server_cb_interest(struct afs_vnode *vnode,
114114
*/
115115
if (cbi && vcbi->server == cbi->server) {
116116
write_seqlock(&vnode->cb_lock);
117-
old = vnode->cb_interest;
118-
vnode->cb_interest = cbi;
117+
old = rcu_dereference_protected(vnode->cb_interest,
118+
lockdep_is_held(&vnode->cb_lock.lock));
119+
rcu_assign_pointer(vnode->cb_interest, cbi);
119120
write_sequnlock(&vnode->cb_lock);
120121
afs_put_cb_interest(afs_v2net(vnode), old);
121122
return 0;
@@ -160,8 +161,9 @@ int afs_register_server_cb_interest(struct afs_vnode *vnode,
160161
*/
161162
write_seqlock(&vnode->cb_lock);
162163

163-
old = vnode->cb_interest;
164-
vnode->cb_interest = cbi;
164+
old = rcu_dereference_protected(vnode->cb_interest,
165+
lockdep_is_held(&vnode->cb_lock.lock));
166+
rcu_assign_pointer(vnode->cb_interest, cbi);
165167
vnode->cb_s_break = cbi->server->cb_s_break;
166168
vnode->cb_v_break = vnode->volume->cb_v_break;
167169
clear_bit(AFS_VNODE_CB_PROMISED, &vnode->flags);
@@ -191,10 +193,11 @@ void afs_put_cb_interest(struct afs_net *net, struct afs_cb_interest *cbi)
191193
vi = NULL;
192194

193195
write_unlock(&cbi->server->cb_break_lock);
194-
kfree(vi);
196+
if (vi)
197+
kfree_rcu(vi, rcu);
195198
afs_put_server(net, cbi->server);
196199
}
197-
kfree(cbi);
200+
kfree_rcu(cbi, rcu);
198201
}
199202
}
200203

‎fs/afs/dir.c‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -638,11 +638,12 @@ static struct inode *afs_do_lookup(struct inode *dir, struct dentry *dentry,
638638
struct key *key)
639639
{
640640
struct afs_lookup_cookie *cookie;
641-
struct afs_cb_interest *cbi = NULL;
641+
struct afs_cb_interest *dcbi, *cbi = NULL;
642642
struct afs_super_info *as = dir->i_sb->s_fs_info;
643643
struct afs_status_cb *scb;
644644
struct afs_iget_data data;
645645
struct afs_fs_cursor fc;
646+
struct afs_server *server;
646647
struct afs_vnode *dvnode = AFS_FS_I(dir);
647648
struct inode *inode = NULL;
648649
int ret, i;
@@ -658,10 +659,14 @@ static struct inode *afs_do_lookup(struct inode *dir, struct dentry *dentry,
658659
cookie->nr_fids = 1; /* slot 0 is saved for the fid we actually want */
659660

660661
read_seqlock_excl(&dvnode->cb_lock);
661-
if (dvnode->cb_interest &&
662-
dvnode->cb_interest->server &&
663-
test_bit(AFS_SERVER_FL_NO_IBULK, &dvnode->cb_interest->server->flags))
664-
cookie->one_only = true;
662+
dcbi = rcu_dereference_protected(dvnode->cb_interest,
663+
lockdep_is_held(&dvnode->cb_lock.lock));
664+
if (dcbi) {
665+
server = dcbi->server;
666+
if (server &&
667+
test_bit(AFS_SERVER_FL_NO_IBULK, &server->flags))
668+
cookie->one_only = true;
669+
}
665670
read_sequnlock_excl(&dvnode->cb_lock);
666671

667672
for (i = 0; i < 50; i++)

‎fs/afs/inode.c‎

Lines changed: 50 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -139,9 +139,10 @@ static int afs_inode_init_from_status(struct afs_vnode *vnode, struct key *key,
139139
vnode->cb_expires_at = ktime_get_real_seconds();
140140
} else {
141141
vnode->cb_expires_at = scb->callback.expires_at;
142-
old_cbi = vnode->cb_interest;
142+
old_cbi = rcu_dereference_protected(vnode->cb_interest,
143+
lockdep_is_held(&vnode->cb_lock.lock));
143144
if (cbi != old_cbi)
144-
vnode->cb_interest = afs_get_cb_interest(cbi);
145+
rcu_assign_pointer(vnode->cb_interest, afs_get_cb_interest(cbi));
145146
else
146147
old_cbi = NULL;
147148
set_bit(AFS_VNODE_CB_PROMISED, &vnode->flags);
@@ -245,9 +246,10 @@ static void afs_apply_callback(struct afs_fs_cursor *fc,
245246

246247
if (!afs_cb_is_broken(cb_break, vnode, fc->cbi)) {
247248
vnode->cb_expires_at = cb->expires_at;
248-
old = vnode->cb_interest;
249+
old = rcu_dereference_protected(vnode->cb_interest,
250+
lockdep_is_held(&vnode->cb_lock.lock));
249251
if (old != fc->cbi) {
250-
vnode->cb_interest = afs_get_cb_interest(fc->cbi);
252+
rcu_assign_pointer(vnode->cb_interest, afs_get_cb_interest(fc->cbi));
251253
afs_put_cb_interest(afs_v2net(vnode), old);
252254
}
253255
set_bit(AFS_VNODE_CB_PROMISED, &vnode->flags);
@@ -565,36 +567,46 @@ void afs_zap_data(struct afs_vnode *vnode)
565567
*/
566568
bool afs_check_validity(struct afs_vnode *vnode)
567569
{
570+
struct afs_cb_interest *cbi;
571+
struct afs_server *server;
572+
struct afs_volume *volume = vnode->volume;
568573
time64_t now = ktime_get_real_seconds();
569574
bool valid;
575+
unsigned int cb_break, cb_s_break, cb_v_break;
576+
int seq = 0;
570577

571-
/* Quickly check the callback state. Ideally, we'd use read_seqbegin
572-
* here, but we have no way to pass the net namespace to the RCU
573-
* cleanup for the server record.
574-
*/
575-
read_seqlock_excl(&vnode->cb_lock);
576-
577-
if (test_bit(AFS_VNODE_CB_PROMISED, &vnode->flags)) {
578-
if (vnode->cb_s_break != vnode->cb_interest->server->cb_s_break ||
579-
vnode->cb_v_break != vnode->volume->cb_v_break) {
580-
vnode->cb_s_break = vnode->cb_interest->server->cb_s_break;
581-
vnode->cb_v_break = vnode->volume->cb_v_break;
582-
valid = false;
583-
} else if (test_bit(AFS_VNODE_ZAP_DATA, &vnode->flags)) {
584-
valid = false;
585-
} else if (vnode->cb_expires_at - 10 <= now) {
586-
valid = false;
587-
} else {
578+
do {
579+
read_seqbegin_or_lock(&vnode->cb_lock, &seq);
580+
cb_v_break = READ_ONCE(volume->cb_v_break);
581+
cb_break = vnode->cb_break;
582+
583+
if (test_bit(AFS_VNODE_CB_PROMISED, &vnode->flags)) {
584+
cbi = rcu_dereference(vnode->cb_interest);
585+
server = rcu_dereference(cbi->server);
586+
cb_s_break = READ_ONCE(server->cb_s_break);
587+
588+
if (vnode->cb_s_break != cb_s_break ||
589+
vnode->cb_v_break != cb_v_break) {
590+
vnode->cb_s_break = cb_s_break;
591+
vnode->cb_v_break = cb_v_break;
592+
valid = false;
593+
} else if (test_bit(AFS_VNODE_ZAP_DATA, &vnode->flags)) {
594+
valid = false;
595+
} else if (vnode->cb_expires_at - 10 <= now) {
596+
valid = false;
597+
} else {
598+
valid = true;
599+
}
600+
} else if (test_bit(AFS_VNODE_DELETED, &vnode->flags)) {
588601
valid = true;
602+
} else {
603+
vnode->cb_v_break = cb_v_break;
604+
valid = false;
589605
}
590-
} else if (test_bit(AFS_VNODE_DELETED, &vnode->flags)) {
591-
valid = true;
592-
} else {
593-
vnode->cb_v_break = vnode->volume->cb_v_break;
594-
valid = false;
595-
}
596606

597-
read_sequnlock_excl(&vnode->cb_lock);
607+
} while (need_seqretry(&vnode->cb_lock, seq));
608+
609+
done_seqretry(&vnode->cb_lock, seq);
598610
return valid;
599611
}
600612

@@ -616,7 +628,9 @@ int afs_validate(struct afs_vnode *vnode, struct key *key)
616628
vnode->fid.vid, vnode->fid.vnode, vnode->flags,
617629
key_serial(key));
618630

631+
rcu_read_lock();
619632
valid = afs_check_validity(vnode);
633+
rcu_read_unlock();
620634

621635
if (test_bit(AFS_VNODE_DELETED, &vnode->flags))
622636
clear_nlink(&vnode->vfs_inode);
@@ -703,6 +717,7 @@ int afs_drop_inode(struct inode *inode)
703717
*/
704718
void afs_evict_inode(struct inode *inode)
705719
{
720+
struct afs_cb_interest *cbi;
706721
struct afs_vnode *vnode;
707722

708723
vnode = AFS_FS_I(inode);
@@ -719,10 +734,14 @@ void afs_evict_inode(struct inode *inode)
719734
truncate_inode_pages_final(&inode->i_data);
720735
clear_inode(inode);
721736

722-
if (vnode->cb_interest) {
723-
afs_put_cb_interest(afs_i2net(inode), vnode->cb_interest);
724-
vnode->cb_interest = NULL;
737+
write_seqlock(&vnode->cb_lock);
738+
cbi = rcu_dereference_protected(vnode->cb_interest,
739+
lockdep_is_held(&vnode->cb_lock.lock));
740+
if (cbi) {
741+
afs_put_cb_interest(afs_i2net(inode), cbi);
742+
rcu_assign_pointer(vnode->cb_interest, NULL);
725743
}
744+
write_sequnlock(&vnode->cb_lock);
726745

727746
while (!list_empty(&vnode->wb_keys)) {
728747
struct afs_wb_key *wbk = list_entry(vnode->wb_keys.next,

‎fs/afs/internal.h‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -554,7 +554,10 @@ struct afs_server {
554554
struct afs_vol_interest {
555555
struct hlist_node srv_link; /* Link in server->cb_volumes */
556556
struct hlist_head cb_interests; /* List of callback interests on the server */
557-
afs_volid_t vid; /* Volume ID to match */
557+
union {
558+
struct rcu_head rcu;
559+
afs_volid_t vid; /* Volume ID to match */
560+
};
558561
unsigned int usage;
559562
};
560563

@@ -566,7 +569,10 @@ struct afs_cb_interest {
566569
struct afs_vol_interest *vol_interest;
567570
struct afs_server *server; /* Server on which this interest resides */
568571
struct super_block *sb; /* Superblock on which inodes reside */
569-
afs_volid_t vid; /* Volume ID to match */
572+
union {
573+
struct rcu_head rcu;
574+
afs_volid_t vid; /* Volume ID to match */
575+
};
570576
refcount_t usage;
571577
};
572578

@@ -676,7 +682,7 @@ struct afs_vnode {
676682
afs_lock_type_t lock_type : 8;
677683

678684
/* outstanding callback notification on this file */
679-
struct afs_cb_interest *cb_interest; /* Server on which this resides */
685+
struct afs_cb_interest __rcu *cb_interest; /* Server on which this resides */
680686
unsigned int cb_s_break; /* Mass break counter on ->server */
681687
unsigned int cb_v_break; /* Mass break counter on ->volume */
682688
unsigned int cb_break; /* Break counter on vnode */

‎fs/afs/rotate.c‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,8 @@ static bool afs_start_fs_iteration(struct afs_fs_cursor *fc,
6666
fc->untried = (1UL << fc->server_list->nr_servers) - 1;
6767
fc->index = READ_ONCE(fc->server_list->preferred);
6868

69-
cbi = vnode->cb_interest;
69+
cbi = rcu_dereference_protected(vnode->cb_interest,
70+
lockdep_is_held(&vnode->io_lock));
7071
if (cbi) {
7172
/* See if the vnode's preferred record is still available */
7273
for (i = 0; i < fc->server_list->nr_servers; i++) {
@@ -87,8 +88,8 @@ static bool afs_start_fs_iteration(struct afs_fs_cursor *fc,
8788

8889
/* Note that the callback promise is effectively broken */
8990
write_seqlock(&vnode->cb_lock);
90-
ASSERTCMP(cbi, ==, vnode->cb_interest);
91-
vnode->cb_interest = NULL;
91+
ASSERTCMP(cbi, ==, rcu_access_pointer(vnode->cb_interest));
92+
rcu_assign_pointer(vnode->cb_interest, NULL);
9293
if (test_and_clear_bit(AFS_VNODE_CB_PROMISED, &vnode->flags))
9394
vnode->cb_break++;
9495
write_sequnlock(&vnode->cb_lock);
@@ -417,7 +418,9 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc)
417418
if (error < 0)
418419
goto failed_set_error;
419420

420-
fc->cbi = afs_get_cb_interest(vnode->cb_interest);
421+
fc->cbi = afs_get_cb_interest(
422+
rcu_dereference_protected(vnode->cb_interest,
423+
lockdep_is_held(&vnode->io_lock)));
421424

422425
read_lock(&server->fs_lock);
423426
alist = rcu_dereference_protected(server->addresses,
@@ -487,12 +490,15 @@ bool afs_select_fileserver(struct afs_fs_cursor *fc)
487490
bool afs_select_current_fileserver(struct afs_fs_cursor *fc)
488491
{
489492
struct afs_vnode *vnode = fc->vnode;
490-
struct afs_cb_interest *cbi = vnode->cb_interest;
493+
struct afs_cb_interest *cbi;
491494
struct afs_addr_list *alist;
492495
int error = fc->ac.error;
493496

494497
_enter("");
495498

499+
cbi = rcu_dereference_protected(vnode->cb_interest,
500+
lockdep_is_held(&vnode->io_lock));
501+
496502
switch (error) {
497503
case SHRT_MAX:
498504
if (!cbi) {
@@ -501,7 +507,7 @@ bool afs_select_current_fileserver(struct afs_fs_cursor *fc)
501507
return false;
502508
}
503509

504-
fc->cbi = afs_get_cb_interest(vnode->cb_interest);
510+
fc->cbi = afs_get_cb_interest(cbi);
505511

506512
read_lock(&cbi->server->fs_lock);
507513
alist = rcu_dereference_protected(cbi->server->addresses,

‎fs/afs/security.c‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ void afs_cache_permit(struct afs_vnode *vnode, struct key *key,
146146
}
147147

148148
if (afs_cb_is_broken(cb_break, vnode,
149-
vnode->cb_interest)) {
149+
rcu_dereference(vnode->cb_interest))) {
150150
changed = true;
151151
break;
152152
}
@@ -176,7 +176,7 @@ void afs_cache_permit(struct afs_vnode *vnode, struct key *key,
176176
}
177177
}
178178

179-
if (afs_cb_is_broken(cb_break, vnode, vnode->cb_interest))
179+
if (afs_cb_is_broken(cb_break, vnode, rcu_dereference(vnode->cb_interest)))
180180
goto someone_else_changed_it;
181181

182182
/* We need a ref on any permits list we want to copy as we'll have to
@@ -253,14 +253,16 @@ void afs_cache_permit(struct afs_vnode *vnode, struct key *key,
253253

254254
kfree(new);
255255

256+
rcu_read_lock();
256257
spin_lock(&vnode->lock);
257258
zap = rcu_access_pointer(vnode->permit_cache);
258-
if (!afs_cb_is_broken(cb_break, vnode, vnode->cb_interest) &&
259+
if (!afs_cb_is_broken(cb_break, vnode, rcu_dereference(vnode->cb_interest)) &&
259260
zap == permits)
260261
rcu_assign_pointer(vnode->permit_cache, replacement);
261262
else
262263
zap = replacement;
263264
spin_unlock(&vnode->lock);
265+
rcu_read_unlock();
264266
afs_put_permits(zap);
265267
out_put:
266268
afs_put_permits(permits);

‎fs/afs/super.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -677,7 +677,7 @@ static struct inode *afs_alloc_inode(struct super_block *sb)
677677
vnode->volume = NULL;
678678
vnode->lock_key = NULL;
679679
vnode->permit_cache = NULL;
680-
vnode->cb_interest = NULL;
680+
RCU_INIT_POINTER(vnode->cb_interest, NULL);
681681
#ifdef CONFIG_AFS_FSCACHE
682682
vnode->cache = NULL;
683683
#endif
@@ -707,7 +707,7 @@ static void afs_destroy_inode(struct inode *inode)
707707

708708
_debug("DESTROY INODE %p", inode);
709709

710-
ASSERTCMP(vnode->cb_interest, ==, NULL);
710+
ASSERTCMP(rcu_access_pointer(vnode->cb_interest), ==, NULL);
711711

712712
atomic_dec(&afs_count_active_inodes);
713713
}

0 commit comments

Comments
 (0)