On 10/08/2013 11:23 PM, Eric Dumazet wrote: > On Tue, 2013-10-08 at 18:04 +0800, Gao feng wrote: >> Though I don't face an oops, but it is more safer to >> set table's subsys through rcu_assign_pointer. >> >> Signed-off-by: Gao feng <gaofeng@xxxxxxxxxxxxxx> >> --- >> net/netfilter/nfnetlink.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c >> index 572d87d..3cd2fe6 100644 >> --- a/net/netfilter/nfnetlink.c >> +++ b/net/netfilter/nfnetlink.c >> @@ -78,7 +78,7 @@ EXPORT_SYMBOL_GPL(nfnetlink_subsys_register); >> int nfnetlink_subsys_unregister(const struct nfnetlink_subsystem *n) >> { >> nfnl_lock(n->subsys_id); >> - table[n->subsys_id].subsys = NULL; >> + rcu_assign_pointer(table[n->subsys_id].subsys, NULL); >> nfnl_unlock(n->subsys_id); >> synchronize_rcu(); >> return 0; > > Certainly not. > > Assigning a NULL pointer do not require rcu_assign_pointer() but this, > if you want to be really really clean. > The reason assigning a NULL pointer do not require rcu_assign_pointer is it's impossible for a reader to access to uninitialized content? So we don't need write barrier here to make sure the resource being initialized before reader accessing it. Right? > diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c > index 572d87d..649958b 100644 > --- a/net/netfilter/nfnetlink.c > +++ b/net/netfilter/nfnetlink.c > @@ -78,7 +78,7 @@ EXPORT_SYMBOL_GPL(nfnetlink_subsys_register); > int nfnetlink_subsys_unregister(const struct nfnetlink_subsystem *n) > { > nfnl_lock(n->subsys_id); > - table[n->subsys_id].subsys = NULL; > + RCU_INIT_POINTER(table[n->subsys_id].subsys, NULL); > nfnl_unlock(n->subsys_id); > synchronize_rcu(); > return 0; > > > > > -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html