Messages in this thread Patch in this message | | | Date | Fri, 21 Jan 2022 11:56:37 +0800 | From | Hangbin Liu <> | Subject | Re: [PATCH RFC V1 net-next 1/4] net: ethtool: Refactor identical get_ts_info implementations. |
| |
On Thu, Jan 20, 2022 at 04:13:29PM +0000, Vladimir Oltean wrote: > > diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c > > index b60e22f6394a..f28b88b67b9e 100644 > > --- a/drivers/net/bonding/bond_main.c > > +++ b/drivers/net/bonding/bond_main.c > > @@ -5353,23 +5353,13 @@ static int bond_ethtool_get_ts_info(struct net_device *bond_dev, > > struct ethtool_ts_info *info) > > { > > struct bonding *bond = netdev_priv(bond_dev); > > - const struct ethtool_ops *ops; > > struct net_device *real_dev; > > - struct phy_device *phydev; > > > > rcu_read_lock(); > > real_dev = bond_option_active_slave_get_rcu(bond); > > rcu_read_unlock(); > > Side note: I'm a bit confused about this rcu_read_lock() -> > rcu_dereference_protected() -> rcu_read_unlock() pattern, and use of the > real_dev outside the RCU critical section. Isn't ->get_ts_info() > protected by the rtnl_mutex? Shouldn't there be a > bond_option_active_slave_get() which uses rtnl_dereference()? > I see the code has been recently added by Hangbin Liu.
Hi Vladimir,
Yes, it should be enough to use rtnl_dereference() since ->get_ts_info is protected by the rtnl_lock. I just thought there is an existing get active slave function and rcu read should be OK to be used here. So I just used the existing one.
Hi Jay,
Do you think if there is a need to add a rtnl version of bond_option_active_slave_get()?
Thanks Hangbin
| |