On Fri, Jan 11, 2008 at 09:30:10AM +0100, Jarek Poplawski wrote: > On Fri, Jan 11, 2008 at 11:00:20AM +1100, Herbert Xu wrote: > > On Fri, Jan 11, 2008 at 12:10:42AM +0100, Jarek Poplawski wrote: > > > > > > It seems this optimization could've a side effect: if during such a > > > loop updates are done, and r is seen !NULL during while() check, but > > > NULL after rcu_dereference(), the listing/counting could stop too > > > soon. So, IMHO, probably the first version of this patch is more > > > reliable. (Or alternatively additional check is needed before return.) > > > > No, while the value of r->u.dst.rt_next can change between two readings, > > the value of r cannot. > > ...Then, of course, it's O.K.! > > It looks like I'm really too lazy and/or these selfdocumenting features > of RCU are a bit overrated: one can never be sure which pointer is > really RCU protected without checking a few places?! So, after looking > at this rt_cache_get_next() and this patch only, it's looks like the > third candidate after seq->private and rtable...
OOPS! ...it seems we are talking about the same, properly documented (second) poiner yet... So, IOW: strictly speaking you are right, r can't change here, but I meant r vs. the returned value! Before the patch the returned value couldn't be NULL unless all elements of the list were looped. After this patch it seems possible... Jarek P. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
