All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ben Hutchings <bhutchings@solarflare.com>
To: David Miller <davem@davemloft.net>
Cc: <netdev@vger.kernel.org>, <linux-net-drivers@solarflare.com>
Subject: [PATCH net-next 10/11] sfc: Fix interface statistics running backward
Date: Thu, 12 Jul 2012 00:18:55 +0100	[thread overview]
Message-ID: <1342048735.2613.69.camel@bwh-desktop.uk.solarflarecom.com> (raw)
In-Reply-To: <1342048389.2613.59.camel@bwh-desktop.uk.solarflarecom.com>

Some interface statistics are computed in such a way that they can
sometimes decrease (and even underflow).  Since the computed value
will never be greater than the true value, we fix this by only storing
the computed value when it increases.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/ethernet/sfc/falcon_xmac.c |   12 ++++++------
 drivers/net/ethernet/sfc/nic.h         |   18 ++++++++++++++++++
 drivers/net/ethernet/sfc/siena.c       |    8 ++++----
 3 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/sfc/falcon_xmac.c b/drivers/net/ethernet/sfc/falcon_xmac.c
index 6106ef1..8333865 100644
--- a/drivers/net/ethernet/sfc/falcon_xmac.c
+++ b/drivers/net/ethernet/sfc/falcon_xmac.c
@@ -341,12 +341,12 @@ void falcon_update_stats_xmac(struct efx_nic *efx)
 	FALCON_STAT(efx, XgTxIpSrcErrPkt, tx_ip_src_error);
 
 	/* Update derived statistics */
-	mac_stats->tx_good_bytes =
-		(mac_stats->tx_bytes - mac_stats->tx_bad_bytes -
-		 mac_stats->tx_control * 64);
-	mac_stats->rx_bad_bytes =
-		(mac_stats->rx_bytes - mac_stats->rx_good_bytes -
-		 mac_stats->rx_control * 64);
+	efx_update_diff_stat(&mac_stats->tx_good_bytes,
+			     mac_stats->tx_bytes - mac_stats->tx_bad_bytes -
+			     mac_stats->tx_control * 64);
+	efx_update_diff_stat(&mac_stats->rx_bad_bytes,
+			     mac_stats->rx_bytes - mac_stats->rx_good_bytes -
+			     mac_stats->rx_control * 64);
 }
 
 void falcon_poll_xmac(struct efx_nic *efx)
diff --git a/drivers/net/ethernet/sfc/nic.h b/drivers/net/ethernet/sfc/nic.h
index f48ccf6..bab5cd9 100644
--- a/drivers/net/ethernet/sfc/nic.h
+++ b/drivers/net/ethernet/sfc/nic.h
@@ -294,6 +294,24 @@ extern bool falcon_xmac_check_fault(struct efx_nic *efx);
 extern int falcon_reconfigure_xmac(struct efx_nic *efx);
 extern void falcon_update_stats_xmac(struct efx_nic *efx);
 
+/* Some statistics are computed as A - B where A and B each increase
+ * linearly with some hardware counter(s) and the counters are read
+ * asynchronously.  If the counters contributing to B are always read
+ * after those contributing to A, the computed value may be lower than
+ * the true value by some variable amount, and may decrease between
+ * subsequent computations.
+ *
+ * We should never allow statistics to decrease or to exceed the true
+ * value.  Since the computed value will never be greater than the
+ * true value, we can achieve this by only storing the computed value
+ * when it increases.
+ */
+static inline void efx_update_diff_stat(u64 *stat, u64 diff)
+{
+	if ((s64)(diff - *stat) > 0)
+		*stat = diff;
+}
+
 /* Interrupts and test events */
 extern int efx_nic_init_interrupt(struct efx_nic *efx);
 extern void efx_nic_enable_interrupts(struct efx_nic *efx);
diff --git a/drivers/net/ethernet/sfc/siena.c b/drivers/net/ethernet/sfc/siena.c
index 2354886..6bafd21 100644
--- a/drivers/net/ethernet/sfc/siena.c
+++ b/drivers/net/ethernet/sfc/siena.c
@@ -458,8 +458,8 @@ static int siena_try_update_nic_stats(struct efx_nic *efx)
 
 	MAC_STAT(tx_bytes, TX_BYTES);
 	MAC_STAT(tx_bad_bytes, TX_BAD_BYTES);
-	mac_stats->tx_good_bytes = (mac_stats->tx_bytes -
-				    mac_stats->tx_bad_bytes);
+	efx_update_diff_stat(&mac_stats->tx_good_bytes,
+			     mac_stats->tx_bytes - mac_stats->tx_bad_bytes);
 	MAC_STAT(tx_packets, TX_PKTS);
 	MAC_STAT(tx_bad, TX_BAD_FCS_PKTS);
 	MAC_STAT(tx_pause, TX_PAUSE_PKTS);
@@ -492,8 +492,8 @@ static int siena_try_update_nic_stats(struct efx_nic *efx)
 	MAC_STAT(tx_ip_src_error, TX_IP_SRC_ERR_PKTS);
 	MAC_STAT(rx_bytes, RX_BYTES);
 	MAC_STAT(rx_bad_bytes, RX_BAD_BYTES);
-	mac_stats->rx_good_bytes = (mac_stats->rx_bytes -
-				    mac_stats->rx_bad_bytes);
+	efx_update_diff_stat(&mac_stats->rx_good_bytes,
+			     mac_stats->rx_bytes - mac_stats->rx_bad_bytes);
 	MAC_STAT(rx_packets, RX_PKTS);
 	MAC_STAT(rx_good, RX_GOOD_PKTS);
 	MAC_STAT(rx_bad, RX_BAD_FCS_PKTS);
-- 
1.7.7.6



-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

  parent reply	other threads:[~2012-07-11 23:18 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-11 23:13 pull request: sfc-next 2012-07-11 Ben Hutchings
2012-07-11 23:15 ` [PATCH net-next 01/11] sfc: Implement 128-bit writes for efx_writeo_page Ben Hutchings
2012-07-12  8:45   ` David Laight
2012-07-12 16:10     ` Ben Hutchings
2012-07-12 15:13   ` David Miller
2012-07-12 16:17     ` Ben Hutchings
2012-07-11 23:15 ` [PATCH net-next 02/11] sfc: Work around bogus 'uninitialised variable' warning Ben Hutchings
2012-07-11 23:16 ` [PATCH net-next 03/11] sfc: Use generic DMA API, not PCI-DMA API Ben Hutchings
2012-07-11 23:16 ` [PATCH net-next 04/11] sfc: Remove dead write to tso_state::packet_space Ben Hutchings
2012-07-11 23:16 ` [PATCH net-next 05/11] sfc: Stop changing header offsets on TX Ben Hutchings
2012-07-11 23:16 ` [PATCH net-next 06/11] sfc: Use strlcpy() to copy ethtool stats names Ben Hutchings
2012-07-11 23:17 ` [PATCH net-next 07/11] sfc: Use dev_kfree_skb() in efx_end_loopback() Ben Hutchings
2012-07-11 23:17 ` [PATCH net-next 08/11] sfc: Explain why efx_mcdi_exit_assertion() ignores result of efx_mcdi_rpc() Ben Hutchings
2012-07-11 23:18 ` [PATCH net-next 09/11] sfc: Disable VF queues during register self-test Ben Hutchings
2012-07-11 23:18 ` Ben Hutchings [this message]
2012-07-11 23:19 ` [PATCH net-next 11/11] sfc: Correct some comments on enum reset_type Ben Hutchings

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1342048735.2613.69.camel@bwh-desktop.uk.solarflarecom.com \
    --to=bhutchings@solarflare.com \
    --cc=davem@davemloft.net \
    --cc=linux-net-drivers@solarflare.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.