On 3/27/17 4:54 PM, Eric W. Biederman wrote: > It is absolutely a no-brainer to change rt_nhn to a u8. And I very much > appreciate all work to keep mpls_route into a single cache line. As in > practices that is one of the most important parts to performance. > > Which leads to the functions mpls_ifup, mpls_ifdown, and > mpls_select_multipath. > > To make this all work correctly we need a couple of things. > - A big fat comment on struct mpls_route and mpls_nh about how > and why these structures are modified and not replaced during > nexthop processing. Including the fact that it all modifications > may only happen with rntl_lock held. > > - The use of READ_ONCE and WRITE_ONCE on all rt->rt_nhn_alive accesses, > that happen after the route is installed (and is thus rcu reachable). > > - The use of READ_ONCE and WRITE_ONCE on all nh->nh_flags accesses, > that happen after the route is installed (and is thus rcu reachable).
For both of these, mpls_select_multipath does need to use READ_ONCE to read the nh_flags and rt_nhn_alive. In this case it is reading a value that could change behind its back. The READ_ONCE is not necessary for mpls_ifdown or mpls_ifup as these are the functions that change the values. These 2 functions only need a WRITE_ONCE for both struct members.
