Re: [PATCH net-2.6.26] fib_trie: RCU optimizations

["Paul E. McKenney" <paulmck@linux.vnet.ibm.com>]

On Fri, Mar 21, 2008 at 07:55:21AM -0700, Stephen Hemminger wrote:
> Small performance improvements.
> 
> Eliminate unneeded barrier on deletion. The first pointer to update
> the head of the list is ordered by the second call to rcu_assign_pointer.
> See hlist_add_after_rcu or comparision.
> 
> Move rcu_derference to the loop check (like hlist_for_each_rcu), and
> add a prefetch.

Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Justification below.

> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> 
> --- a/net/ipv4/route.c	2008-03-19 08:45:32.000000000 -0700
> +++ b/net/ipv4/route.c	2008-03-19 08:54:57.000000000 -0700
> @@ -977,8 +977,8 @@ restart:
>  			 * must be visible to another weakly ordered CPU before
>  			 * the insertion at the start of the hash chain.
>  			 */
> -			rcu_assign_pointer(rth->u.dst.rt_next,
> -					   rt_hash_table[hash].chain);
> +			rth->u.dst.rt_next = rt_hash_table[hash].chain;
> +

This is OK because it is finalizing a deletion.  If this were instead
an insertion, this would of course be grossly illegal and dangerous.

>  			/*
>  			 * Since lookup is lockfree, the update writes
>  			 * must be ordered for consistency on SMP.
> @@ -2076,8 +2076,9 @@ int ip_route_input(struct sk_buff *skb, 
>  	hash = rt_hash(daddr, saddr, iif);
> 
>  	rcu_read_lock();
> -	for (rth = rcu_dereference(rt_hash_table[hash].chain); rth;
> -	     rth = rcu_dereference(rth->u.dst.rt_next)) {
> +	for (rth = rt_hash_table[hash].chain; rcu_dereference(rth);
> +	     rth = rth->u.dst.rt_next) {
> +		prefetch(rth->u.dst.rt_next);
>  		if (rth->fl.fl4_dst == daddr &&
>  		    rth->fl.fl4_src == saddr &&
>  		    rth->fl.iif == iif &&

Works, though I would guess that increasingly aggressive compiler
optimization will eventually force us to change the list.h macros
to look like what you had to begin with...  Sigh!!!