Re: [PATCH net-next 1/2] rtnetlink: move rtnl_lock handling out of af_netlink

[Kuniyuki Iwashima <kuniyu@amazon.com>]

From: Jakub Kicinski <kuba@kernel.org>
Date: Thu, 6 Jun 2024 17:04:53 -0700
> On Thu, 6 Jun 2024 16:33:03 -0700 Kuniyuki Iwashima wrote:
> > > +	if (needs_lock)
> > > +		rtnl_lock();
> > >  	err = dumpit(skb, cb);
> > > +	if (needs_lock)
> > > +		rtnl_unlock();  
> > 
> > This calls netdev_run_todo() now, is this change intended ?
> 
> Nice catch / careful thinking, indeed we're moving from pure unlock to
> run_todo. I don't really recall if I thought of this when writing the
> change (it was few days back). My guess is that the fact we weren't
> calling full rtnl_unlock() was unintentional / out of laziness in the
> first place. It didn't matter since dumps are unlikely to changes /
> unregister / free things.

This makes sense.  Probably due to cb_mutex interface constraint.


> But still, someone may get caught off guard
> as some point that we're holding rtnl but won't go via the usual unlock
> path.
> 
> Would you like me to add a note to the commit message?

That would be nice, but I'm fine with this version :)

Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>

Thanks!