Re: [PATCH][XFRM] Replace rwlock on xfrm_policy_afinfo with rcu

[Eric Dumazet <eric.dumazet@gmail.com>]

On Tue, 2012-08-07 at 10:51 +0530, Priyanka Jain wrote:
> xfrm_policy_afinfo is read mosly data structure.
> Write on xfrm_policy_afinfo is done only at the
> time of configuration.
> So rwlocks can be safely replaced with RCU.
> 
> RCUs usage optimizes the performance.


>  static struct xfrm_policy_afinfo *xfrm_policy_get_afinfo(unsigned short family)
> @@ -2530,16 +2535,16 @@ static struct xfrm_policy_afinfo *xfrm_policy_get_afinfo(unsigned short family)
>  	struct xfrm_policy_afinfo *afinfo;
>  	if (unlikely(family >= NPROTO))
>  		return NULL;
> -	read_lock(&xfrm_policy_afinfo_lock);
> -	afinfo = xfrm_policy_afinfo[family];
> +	rcu_read_lock();
> +	afinfo = rcu_dereference(xfrm_policy_afinfo[family]);
>  	if (unlikely(!afinfo))
> -		read_unlock(&xfrm_policy_afinfo_lock);
> +		rcu_read_unlock();

This makes no sense to me : We cant safely return afinfo here. 

Note the current code is buggy as well, this is worrying.

As soon as we exit from xfrm_policy_get_afinfo(), pointer might be
invalid.

Really, RCU conversion should be the right moment to spot those bugs and
first fix them (for stable trees)