Subject: Re: [PATCH] net: implement emergency route cache rebulds when gc_elasticity is exceeded

From: Eric Dumazet <[email-masked]>
Date: Thu, 16 Oct 2008 11:19:38 +0200

David Miller a écrit :
> From: Neil Horman <[email-masked]>
> Date: Mon, 13 Oct 2008 14:26:55 -0400
> 
>> If this meets everyones approval I think we can follow up with a
>> patch to remove the secret interval code entirely.
> 
> This patch looks pretty good to me.
> 
> Just some minor coding style nits:
> 
>> +static void rt_secret_rebuild_oneshot(struct net *net) {
> 
> Openning brace on new line please.
> 
>> +static void rt_emergency_hash_rebuild(struct net *net) {
> 
> Likewise.
> 
> 

While browsing Neil patch, I found one missing rcu_assign_pointer()
in current code. I added a comment similar to other rcu_assign_pointer()
in rt_intern_hash() function.

[PATCH] ip: adds a missing rcu_assign_pointer()

rt_intern_hash() is doing an update of a RCU guarded hash chain
without using rcu_assign_pointer() or equivalent barrier.

Signed-off-by: Eric Dumazet <[email-masked]>

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index a6d7c58..377c6f9 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1109,7 +1109,12 @@ restart:
 		printk("\n");
 	}
 #endif
-	rt_hash_table[hash].chain = rt;
+	/*
+	 * Since lookup is lockfree, we must make sure
+	 * previous writes to rt are comitted to memory
+	 * before making rt visible to other CPUS.
+	 */	
+	rcu_assign_pointer(rt_hash_table[hash].chain, rt);
 	spin_unlock_bh(rt_hash_lock_addr(hash));
 	*rp = rt;
 	return 0;