Stephen Hemminger writes:
> I have a merged patch set in the works. with Robert et al.
> Some of what my differences are:
> > +struct fib_alias *fib_find_alias_rcu(struct list_head *fah, u8 tos,
> > u32 prio) +{
> > + if (fah) {
> > + struct fib_alias *fa;
> > + list_for_each_entry_rcu(fa, fah, fa_list) {
> > + if (fa->fa_tos > tos)
> > + continue;
> > + if (fa->fa_info->fib_priority >= prio ||
> > + fa->fa_tos < tos)
> > + return fa;
> > + }
> > + }
> > + return NULL;
> > +}
>
> fib_find_alias is only used in path's that have RT netlink
> mutex held, so RCU is not needed.
Yes true for the moment but shouldn't we use RCU consequently?
> > +int fib_semantic_match_rcu(struct list_head *head, const struct
> > flowi *flp,
> > + struct fib_result *res, __u32 zone, __u32
> > mask,
> > + int prefixlen)
> > +{
>
>
> Why duplicate the code of of fib_semantic_match? It would be
> better to just change fib_semantic_match to use the for_each_rcu().
> The resulting code will be the same (except alpha) because the
> only addition is a read_barrier_depends().
>
> > int fib_semantic_match(struct list_head *head, const struct flowi
> > *flp, struct fib_result *res, __u32 zone, __u32 mask,
> > int prefixlen)
Yes it duplication as we didn't want to touch any existing FIB code for
the moment also Patrick was thinking of RCU version of fib_hash later on.
You might have a point though...
But it raises the next question ;) when the difference is so minimal.
Why do we keep two sets of (h)list functions? Both RCU and non-RCU
> Also, please don't bother adding RCU to routines that are modifying
> the tree. Only the lookup, dump, and /proc code paths should
> use RCU.
Lists are rcu-procteced. So shouldn't we use ie RCU consistenly even
in the "writer" functions as insert/delete? Is this what you mean?
And lookup is of course run from softirq.
Cheers.
--ro
-
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