Commit 1cb34522 authored by Ben Hutchings's avatar Ben Hutchings

sfc: Hold efx_nic::stats_lock while reading efx_nic::mac_stats

efx_nic::stats_lock is used to serialise stats updates, but each
reader was dropping it before it finished reading efx_nic::mac_stats.

If there were concurrent stats reads using procfs, or one using procfs
and one using ethtool, an update could race with a read.  On a 32-bit
system, the reader could see word-tearing of 64-bit stats (32 bits of
the old value and 32 bits of the new).
Signed-off-by: default avatarBen Hutchings <bhutchings@solarflare.com>
parent 788ec41c
...@@ -1743,8 +1743,8 @@ static struct rtnl_link_stats64 *efx_net_stats(struct net_device *net_dev, struc ...@@ -1743,8 +1743,8 @@ static struct rtnl_link_stats64 *efx_net_stats(struct net_device *net_dev, struc
struct efx_mac_stats *mac_stats = &efx->mac_stats; struct efx_mac_stats *mac_stats = &efx->mac_stats;
spin_lock_bh(&efx->stats_lock); spin_lock_bh(&efx->stats_lock);
efx->type->update_stats(efx); efx->type->update_stats(efx);
spin_unlock_bh(&efx->stats_lock);
stats->rx_packets = mac_stats->rx_packets; stats->rx_packets = mac_stats->rx_packets;
stats->tx_packets = mac_stats->tx_packets; stats->tx_packets = mac_stats->tx_packets;
...@@ -1768,6 +1768,8 @@ static struct rtnl_link_stats64 *efx_net_stats(struct net_device *net_dev, struc ...@@ -1768,6 +1768,8 @@ static struct rtnl_link_stats64 *efx_net_stats(struct net_device *net_dev, struc
stats->tx_errors = (stats->tx_window_errors + stats->tx_errors = (stats->tx_window_errors +
mac_stats->tx_bad); mac_stats->tx_bad);
spin_unlock_bh(&efx->stats_lock);
return stats; return stats;
} }
......
...@@ -489,13 +489,14 @@ static void efx_ethtool_get_stats(struct net_device *net_dev, ...@@ -489,13 +489,14 @@ static void efx_ethtool_get_stats(struct net_device *net_dev,
const struct efx_ethtool_stat *stat; const struct efx_ethtool_stat *stat;
struct efx_channel *channel; struct efx_channel *channel;
struct efx_tx_queue *tx_queue; struct efx_tx_queue *tx_queue;
struct rtnl_link_stats64 temp;
int i; int i;
EFX_BUG_ON_PARANOID(stats->n_stats != EFX_ETHTOOL_NUM_STATS); EFX_BUG_ON_PARANOID(stats->n_stats != EFX_ETHTOOL_NUM_STATS);
spin_lock_bh(&efx->stats_lock);
/* Update MAC and NIC statistics */ /* Update MAC and NIC statistics */
dev_get_stats(net_dev, &temp); efx->type->update_stats(efx);
/* Fill detailed statistics buffer */ /* Fill detailed statistics buffer */
for (i = 0; i < EFX_ETHTOOL_NUM_STATS; i++) { for (i = 0; i < EFX_ETHTOOL_NUM_STATS; i++) {
...@@ -525,6 +526,8 @@ static void efx_ethtool_get_stats(struct net_device *net_dev, ...@@ -525,6 +526,8 @@ static void efx_ethtool_get_stats(struct net_device *net_dev,
break; break;
} }
} }
spin_unlock_bh(&efx->stats_lock);
} }
static void efx_ethtool_self_test(struct net_device *net_dev, static void efx_ethtool_self_test(struct net_device *net_dev,
......
...@@ -704,6 +704,7 @@ struct efx_filter_state; ...@@ -704,6 +704,7 @@ struct efx_filter_state;
* can provide. Generic code converts these into a standard * can provide. Generic code converts these into a standard
* &struct net_device_stats. * &struct net_device_stats.
* @stats_lock: Statistics update lock. Serialises statistics fetches * @stats_lock: Statistics update lock. Serialises statistics fetches
* and access to @mac_stats.
* *
* This is stored in the private area of the &struct net_device. * This is stored in the private area of the &struct net_device.
*/ */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment