Re: [RFC PATCH] llc: convert the socket list to RCU locking (was Re: [PATCH 3/4] llc: use a device based hash table to speed up multicast delivery)

[Octavian Purdila <opurdila@ixiacom.com>]

On Wednesday 09 December 2009 22:19:24 you wrote:

> > +     rcu_read_lock();
> > +     sk_nulls_for_each_rcu(rc, node, &sap->sk_list) {
> > +             if (llc_estab_match(sap, daddr, laddr, rc)) {
> > +                     /* Extra checks required by SLAB_DESTROY_BY_RCU */
> > +                     if (unlikely(!atomic_inc_not_zero(&rc->sk_refcnt)))
> > +                             continue;
> 
> Hmm, this wont work in fact, because if we have several llc_sap allocated
>  on machine, we have no guarantee a freed socket wont be reused and
>  inserted in another llc_sap list.
> 
> So before calling llc_estab_match() (and/or llc_listener_match()) you
>  should check if 'rc' really belong to current sap.
> 
> If not, you must restart the lookup (you hit a socket that was moved to
>  another sap)
> 

Right ! 

>> 
>> One doubt before pasting the code: In slab.h comment and in udp.c I see the 
lookup is restarted if an improper object is returned. Is that really 
required?
>> 
>
>
>Its needed only if you convert to a hash table (more than one chain),
>and I guess you definitly want a fanout of your XXXX items ?
>
>Check Documentation/RCU/rculist_nulls.txt for details :)

And of course I'll still want to add the hash table ;) I was just trying to 
get my head around something simpler first.

I think I have everything  (in my head :) ) that I need now to come back with 
a v2 llc patch set now. Thanks for your help !