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

Reply via email to