leandrof/linux
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

inet: frags: Remove unnecessary smp_store_release/READ_ONCE

Browse Source 
The smp_store_release call in fqdir_exit cannot protect the setting
of fqdir->dead as claimed because its memory barrier is only
guaranteed to be one-way and the barrier precedes the setting of
fqdir->dead.

IOW it doesn't provide any barriers between fq->dir and the following
hash table destruction.

In fact, the code is safe anyway because call_rcu does provide both
the memory barrier as well as a guarantee that when the destruction
work starts executing all RCU readers will see the updated value for
fqdir->dead.

Therefore this patch removes the unnecessary smp_store_release call
as well as the corresponding READ_ONCE on the read-side in order to
not confuse future readers of this code.  Comments have been added
in their places.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
Herbert Xu
2019-05-29 13:40:26 +08:00
committed by David S. Miller
parent 5080e28d94
commit 32707c4dfa
1 changed files with 10 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
net/ipv4/inet_fragment.c
View File

@@ -193,10 +193,12 @@ void fqdir_exit(struct fqdir *fqdir)
{ {
fqdir->high_thresh = 0; /* prevent creation of new frags */ fqdir->high_thresh = 0; /* prevent creation of new frags */
/* paired with READ_ONCE() in inet_frag_kill() : fqdir->dead = true;
* We want to prevent rhashtable_remove_fast() calls
/* call_rcu is supposed to provide memory barrier semantics,
* separating the setting of fqdir->dead with the destruction
* work. This implicit barrier is paired with inet_frag_kill().
*/ */
smp_store_release(&fqdir->dead, true);
INIT_RCU_WORK(&fqdir->destroy_rwork, fqdir_rwork_fn); INIT_RCU_WORK(&fqdir->destroy_rwork, fqdir_rwork_fn);
queue_rcu_work(system_wq, &fqdir->destroy_rwork); queue_rcu_work(system_wq, &fqdir->destroy_rwork);
@@ -214,10 +216,12 @@ void inet_frag_kill(struct inet_frag_queue *fq)
fq->flags |= INET_FRAG_COMPLETE; fq->flags |= INET_FRAG_COMPLETE;
rcu_read_lock(); rcu_read_lock();
/* This READ_ONCE() is paired with smp_store_release() /* The RCU read lock provides a memory barrier
* in inet_frags_exit_net(). * guaranteeing that if fqdir->dead is false then
* the hash table destruction will not start until
* after we unlock. Paired with inet_frags_exit_net().
*/ */
if (!READ_ONCE(fqdir->dead)) { if (!fqdir->dead) {
rhashtable_remove_fast(&fqdir->rhashtable, &fq->node, rhashtable_remove_fast(&fqdir->rhashtable, &fq->node,
fqdir->f->rhash_params); fqdir->f->rhash_params);
refcount_dec(&fq->refcnt); refcount_dec(&fq->refcnt);