Re: [PATCH 1/2] net: dsa: RCU-protect dsa_ptr in struct net_device

[Gur Stavi <gur.stavi@huawei.com>]

> Hello Gur!
>
> On Tue, 2024-09-17 at 11:10 +0300, Gur Stavi wrote:
> > > @@ -1594,10 +1592,11 @@ void dsa_switch_shutdown(struct dsa_switch *ds)
> > >   	}
> > >
> > >   	/* Disconnect from further netdevice notifiers on the conduit,
> > > -	 * since netdev_uses_dsa() will now return false.
> > > +	 * from now on, netdev_uses_dsa_currently() will return false.
> > >   	 */
> > >   	dsa_switch_for_each_cpu_port(dp, ds)
> > > -		dp->conduit->dsa_ptr = NULL;
> > > +		rcu_assign_pointer(dp->conduit->dsa_ptr, NULL);
> > > +	synchronize_rcu();
> > >
> > >   	rtnl_unlock();
> > >   out:
> >
> > Hi, I am a newbie here. Thanks for the opportunity for learning more
> > about rcu.
> > Wouldn't it make more sense to call synchronize_rcu after rtnl_unlock?
>
> This is indeed a question which is usually resolved other way around
> (making locked section shorter), but in this particular case I thought that:
> - we actually don't need giving rtnl lock sooner, which would potentially
>   make synchronize_rcu() call longer (if another thread manages to wake up
>   and claim the rtnl lock before synchronize_rcu())
> - we are in shutdown phase, we don't need to minimize lock contention, we
>   need to minimize the overall shutdown time

But isn't shutdown still multithreaded?
10 threads may have similar shutdown task: remove objects from different
rcu protected data structures while holding rtnl. Then synchronize RCU
and release the objects.
Synchronizing RCU from within the lock will completely serialize all
threads and postpone shutdown whereas outside the lock multiple
synchronize_rcu could run in parallel.