diff mbox series

[RFC,net-next,06/13] net_failover: hold the netdev lists lock when retrieving device statistics

Message ID 20201206235919.393158-7-vladimir.oltean@nxp.com (mailing list archive)
State RFC
Delegated to: Netdev Maintainers
Headers show
Series Make .ndo_get_stats64 sleepable | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit fail Errors and warnings before: 0 this patch: 2
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 48 lines checked
netdev/build_allmodconfig_warn fail Errors and warnings before: 0 this patch: 2
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

Vladimir Oltean Dec. 6, 2020, 11:59 p.m. UTC
In the effort of making .ndo_get_stats64 be able to sleep, we need to
ensure the callers of dev_get_stats do not use atomic context.

The net_failover driver makes copious abuse of RCU protection for the
slave interfaces, which is probably unnecessary given the fact that it
already calls dev_hold on slave interfaces. Nonetheless, to avoid
regressions, we still need to offer the same level of protection against
unregistering the standby and primary devices. We can achieve this by
holding the netns mutex, which gives us the sleepable context that
dev_get_stats() wants to see. Holding this mutex also removes the need
for a separate lock for statistics.

Cc: Sridhar Samudrala <sridhar.samudrala@intel.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/net_failover.c | 15 +++++++--------
 include/net/net_failover.h |  3 ---
 2 files changed, 7 insertions(+), 11 deletions(-)
diff mbox series

Patch

diff --git a/drivers/net/net_failover.c b/drivers/net/net_failover.c
index 2a4892402ed8..90db0358bc1d 100644
--- a/drivers/net/net_failover.c
+++ b/drivers/net/net_failover.c
@@ -184,32 +184,31 @@  static void net_failover_get_stats(struct net_device *dev,
 {
 	struct net_failover_info *nfo_info = netdev_priv(dev);
 	const struct rtnl_link_stats64 *new;
+	struct net *net = dev_net(dev);
 	struct rtnl_link_stats64 temp;
 	struct net_device *slave_dev;
 
-	spin_lock(&nfo_info->stats_lock);
-	memcpy(stats, &nfo_info->failover_stats, sizeof(*stats));
+	mutex_lock(&net->netdev_lists_lock);
 
-	rcu_read_lock();
+	memcpy(stats, &nfo_info->failover_stats, sizeof(*stats));
 
-	slave_dev = rcu_dereference(nfo_info->primary_dev);
+	slave_dev = nfo_info->primary_dev;
 	if (slave_dev) {
 		new = dev_get_stats(slave_dev, &temp);
 		net_failover_fold_stats(stats, new, &nfo_info->primary_stats);
 		memcpy(&nfo_info->primary_stats, new, sizeof(*new));
 	}
 
-	slave_dev = rcu_dereference(nfo_info->standby_dev);
+	slave_dev = nfo_info->standby_dev;
 	if (slave_dev) {
 		new = dev_get_stats(slave_dev, &temp);
 		net_failover_fold_stats(stats, new, &nfo_info->standby_stats);
 		memcpy(&nfo_info->standby_stats, new, sizeof(*new));
 	}
 
-	rcu_read_unlock();
-
 	memcpy(&nfo_info->failover_stats, stats, sizeof(*stats));
-	spin_unlock(&nfo_info->stats_lock);
+
+	mutex_unlock(&net->netdev_lists_lock);
 }
 
 static int net_failover_change_mtu(struct net_device *dev, int new_mtu)
diff --git a/include/net/net_failover.h b/include/net/net_failover.h
index b12a1c469d1c..1e0089800a28 100644
--- a/include/net/net_failover.h
+++ b/include/net/net_failover.h
@@ -22,9 +22,6 @@  struct net_failover_info {
 
 	/* aggregated stats */
 	struct rtnl_link_stats64 failover_stats;
-
-	/* spinlock while updating stats */
-	spinlock_t stats_lock;
 };
 
 struct failover *net_failover_create(struct net_device *standby_dev);