On Tue, Sep 2, 2014, at 20:04, Cong Wang wrote: > On Tue, Sep 2, 2014 at 10:58 AM, Hannes Frederic Sowa > <hannes@xxxxxxxxxxxxxxxxxxx> wrote: > > Hi Cong, > > > > On Tue, Sep 2, 2014, at 18:50, Cong Wang wrote: > >> On Fri, Aug 29, 2014 at 6:51 PM, Hannes Frederic Sowa > >> <hannes@xxxxxxxxxxxxxxxxxxx> wrote: > >> > > >> > Also rtnl_lock and rcu_read_lock compose in that order, so we don't need > >> > to change dev_get_by_flags, but as this is the only user it sure is > >> > possible. RCU locked version is just easier composeable, so I wouldn't > >> > touch that if needed in future, just also take rcu lock as before. > >> > >> There is no point to keep RCU read lock if we have rtnl lock, > >> I don't know why you don't want to change dev_get_by_flags(), > >> it is pretty easy to do since it only has one caller. > > > > I definitely don't have a problem cleaning this up in net-next. I wanted > > a minimal patch for stable because I didn't check history where and when > > additional users of dev_get_by_flags_rcu were removed. > > `git grep` should show you we only have one caller. Apparently we don't > care about any out-of-tree module. Sure, I don't care about out-of-tree modules either. I just wanted to make it easier to backport. Current patch is almost headache free to backport. > >> Even if you really need RCU in future, you are always welcome > >> to bring it back when you do, sorry we should never be blocked by > >> code NOT merged yet. > >> > >> > > >> > Also we should move ASSERT_RTNL checks from addrconf_join_solict to > >> > ipv6_dev_mc_inc/dec. > >> > > >> > >> Make it another patch. > > > > It is just one logical change, moving ASSERT_RTNLs to places where they > > better catch invalid callstacks. > > > > Conflicts with what you claimed above. :) Those ASSERT_RTNLs were misplaced and only caught the callers mostly from addrconf.c. I don't mind getting reports from stable kernel users and fixing those, too (or help fixing those). ASSERT_RTNL is not dangerous. We had a long history in not correctly using rtnl lock in ipv6/multicast code and those wrongfully placed ASSERT_RTNLs were my bad when I fixed the duplicate address detection handling. If enough multicast addresses are subscribed to an interface we might again get those splats because enabling promisc mode on an interface will also check for rtnl lock. Bye, Hannes -- To unsubscribe from this list: send the line "unsubscribe trinity" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
