tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

sunrpc/cache: improve RCU safety in cache_list walking.

1/ consistently use hlist_add_head_rcu() when adding to
the cachelist to reflect the fact that it can be concurrently
walked using RCU. In fact hlist_add_head() has all the needed
barriers so this is no safety issue, primarily a clarity issue.

2/ call cache_get() *before* adding the list with hlist_add_head_rcu().
It is generally safest to inc the refcount before publishing a
reference. In this case it doesn't have any behavioural effect
as code which does an RCU walk does not depend on precision of
the refcount, and it will always be at least one. But it looks
more correct to use this order.

3/ avoid possible races between NULL tests and hlist_entry_safe()
calls. It is possible that a test will find that .next or .head
is not NULL, but hlist_entry_safe() will find that it is NULL.
This can lead to incorrect behaviour with the list-walk terminating
early.
It is safest to always call hlist_entry_safe() and test the result.

Also simplify the *ppos calculation by simply assigning the hash
shifted 32, rather than masking out low bits and incrementing high
bits.

Signed-off-by: NeilBrown <neil@brown.name>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>

authored by

NeilBrown and committed by
Chuck Lever 7b546bd8 322ecd01

+29 -33
+29 -33
net/sunrpc/cache.c
··· 134 134 return tmp; 135 135 } 136 136 137 + cache_get(new); 137 138 hlist_add_head_rcu(&new->cache_list, head); 138 139 detail->entries++; 139 140 if (detail->nextcheck > new->expiry_time) 140 141 detail->nextcheck = new->expiry_time + 1; 141 - cache_get(new); 142 142 spin_unlock(&detail->hash_lock); 143 143 144 144 if (freeme) ··· 233 233 234 234 spin_lock(&detail->hash_lock); 235 235 cache_entry_update(detail, tmp, new); 236 - hlist_add_head(&tmp->cache_list, &detail->hash_table[hash]); 237 - detail->entries++; 238 236 cache_get(tmp); 237 + hlist_add_head_rcu(&tmp->cache_list, &detail->hash_table[hash]); 238 + detail->entries++; 239 239 cache_fresh_locked(tmp, new->expiry_time, detail); 240 240 cache_fresh_locked(old, 0, detail); 241 241 spin_unlock(&detail->hash_lock); ··· 1378 1378 hlist_for_each_entry_rcu(ch, &cd->hash_table[hash], cache_list) 1379 1379 if (!entry--) 1380 1380 return ch; 1381 - n &= ~((1LL<<32) - 1); 1382 - do { 1383 - hash++; 1384 - n += 1LL<<32; 1385 - } while(hash < cd->hash_size && 1386 - hlist_empty(&cd->hash_table[hash])); 1387 - if (hash >= cd->hash_size) 1388 - return NULL; 1389 - *pos = n+1; 1390 - return hlist_entry_safe(rcu_dereference_raw( 1381 + ch = NULL; 1382 + while (!ch && ++hash < cd->hash_size) 1383 + ch = hlist_entry_safe(rcu_dereference( 1391 1384 hlist_first_rcu(&cd->hash_table[hash])), 1392 1385 struct cache_head, cache_list); 1386 + 1387 + *pos = ((long long)hash << 32) + 1; 1388 + return ch; 1393 1389 } 1394 1390 1395 1391 static void *cache_seq_next(struct seq_file *m, void *p, loff_t *pos) ··· 1394 1398 int hash = (*pos >> 32); 1395 1399 struct cache_detail *cd = m->private; 1396 1400 1397 - if (p == SEQ_START_TOKEN) 1401 + if (p == SEQ_START_TOKEN) { 1398 1402 hash = 0; 1399 - else if (ch->cache_list.next == NULL) { 1400 - hash++; 1401 - *pos += 1LL<<32; 1402 - } else { 1403 - ++*pos; 1404 - return hlist_entry_safe(rcu_dereference_raw( 1403 + ch = NULL; 1404 + } 1405 + while (hash < cd->hash_size) { 1406 + if (ch) 1407 + ch = hlist_entry_safe( 1408 + rcu_dereference( 1405 1409 hlist_next_rcu(&ch->cache_list)), 1406 - struct cache_head, cache_list); 1407 - } 1408 - *pos &= ~((1LL<<32) - 1); 1409 - while (hash < cd->hash_size && 1410 - hlist_empty(&cd->hash_table[hash])) { 1411 - hash++; 1412 - *pos += 1LL<<32; 1413 - } 1414 - if (hash >= cd->hash_size) 1415 - return NULL; 1416 - ++*pos; 1417 - return hlist_entry_safe(rcu_dereference_raw( 1418 - hlist_first_rcu(&cd->hash_table[hash])), 1419 1410 struct cache_head, cache_list); 1411 + else 1412 + ch = hlist_entry_safe( 1413 + rcu_dereference( 1414 + hlist_first_rcu(&cd->hash_table[hash])), 1415 + struct cache_head, cache_list); 1416 + if (ch) { 1417 + ++*pos; 1418 + return ch; 1419 + } 1420 + hash++; 1421 + *pos = (long long)hash << 32; 1422 + } 1423 + return NULL; 1420 1424 } 1421 1425 1422 1426 void *cache_seq_start_rcu(struct seq_file *m, loff_t *pos)