* [PATCH net-next v1 1/9] forcedeth: Add messages to indicate using MSI or MSI-X
2011-11-09 22:09 [PATCH net-next v1 0/9] forcedeth: stats & debug enhancements David Decotigny
@ 2011-11-09 22:09 ` David Decotigny
2011-11-09 22:09 ` [PATCH net-next v1 2/9] forcedeth: allow to silence "TX timeout" debug messages David Decotigny
` (8 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: David Decotigny @ 2011-11-09 22:09 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: David S. Miller, Ian Campbell, Eric Dumazet, Jeff Kirsher,
Ben Hutchings, Mike Ditto, David Decotigny
From: Mike Ditto <mditto@google.com>
This adds a few debug messages to indicate whether PCIe interrupts are
signaled with MSI or MSI-X.
Signed-off-by: David Decotigny <david.decotigny@google.com>
---
drivers/net/ethernet/nvidia/forcedeth.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
index d24c45b..39781c1b 100644
--- a/drivers/net/ethernet/nvidia/forcedeth.c
+++ b/drivers/net/ethernet/nvidia/forcedeth.c
@@ -3711,6 +3711,7 @@ static int nv_request_irq(struct net_device *dev, int intr_test)
writel(0, base + NvRegMSIXMap0);
writel(0, base + NvRegMSIXMap1);
}
+ netdev_info(dev, "MSI-X enabled\n");
}
}
if (ret != 0 && np->msi_flags & NV_MSI_CAPABLE) {
@@ -3732,6 +3733,7 @@ static int nv_request_irq(struct net_device *dev, int intr_test)
writel(0, base + NvRegMSIMap1);
/* enable msi vector 0 */
writel(NVREG_MSI_VECTOR_0_ENABLED, base + NvRegMSIIrqMask);
+ netdev_info(dev, "MSI enabled\n");
}
}
if (ret != 0) {
--
1.7.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH net-next v1 2/9] forcedeth: allow to silence "TX timeout" debug messages
2011-11-09 22:09 [PATCH net-next v1 0/9] forcedeth: stats & debug enhancements David Decotigny
2011-11-09 22:09 ` [PATCH net-next v1 1/9] forcedeth: Add messages to indicate using MSI or MSI-X David Decotigny
@ 2011-11-09 22:09 ` David Decotigny
2011-11-09 22:09 ` [PATCH net-next v1 3/9] forcedeth: expose module parameters in /sys/module David Decotigny
` (7 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: David Decotigny @ 2011-11-09 22:09 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: David S. Miller, Ian Campbell, Eric Dumazet, Jeff Kirsher,
Ben Hutchings, Sameer Nanda, David Decotigny
From: Sameer Nanda <snanda@google.com>
This adds a new module parameter "debug_tx_timeout" to silence most
debug messages in case of TX timeout. These messages don't provide a
signal/noise ratio high enough for production systems and, with ~30kB
logged each time, they tend to add to a cascade effect if the system
is already under stress (memory pressure, disk, etc.).
By default, the parameter is clear, meaning that only a single warning
will be reported (the other more detailed debug messages are not
displayed).
Signed-off-by: David Decotigny <david.decotigny@google.com>
---
drivers/net/ethernet/nvidia/forcedeth.c | 98 ++++++++++++++++++-------------
1 files changed, 57 insertions(+), 41 deletions(-)
diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
index 39781c1b..763863d 100644
--- a/drivers/net/ethernet/nvidia/forcedeth.c
+++ b/drivers/net/ethernet/nvidia/forcedeth.c
@@ -892,6 +892,11 @@ enum {
static int dma_64bit = NV_DMA_64BIT_ENABLED;
/*
+ * Debug output control for tx_timeout
+ */
+static bool debug_tx_timeout = false;
+
+/*
* Crossover Detection
* Realtek 8201 phy + some OEM boards do not work properly.
*/
@@ -2461,56 +2466,64 @@ static void nv_tx_timeout(struct net_device *dev)
u32 status;
union ring_type put_tx;
int saved_tx_limit;
- int i;
if (np->msi_flags & NV_MSI_X_ENABLED)
status = readl(base + NvRegMSIXIrqStatus) & NVREG_IRQSTAT_MASK;
else
status = readl(base + NvRegIrqStatus) & NVREG_IRQSTAT_MASK;
- netdev_info(dev, "Got tx_timeout. irq: %08x\n", status);
+ netdev_warn(dev, "Got tx_timeout. irq status: %08x\n", status);
- netdev_info(dev, "Ring at %lx\n", (unsigned long)np->ring_addr);
- netdev_info(dev, "Dumping tx registers\n");
- for (i = 0; i <= np->register_size; i += 32) {
- netdev_info(dev,
- "%3x: %08x %08x %08x %08x %08x %08x %08x %08x\n",
- i,
- readl(base + i + 0), readl(base + i + 4),
- readl(base + i + 8), readl(base + i + 12),
- readl(base + i + 16), readl(base + i + 20),
- readl(base + i + 24), readl(base + i + 28));
- }
- netdev_info(dev, "Dumping tx ring\n");
- for (i = 0; i < np->tx_ring_size; i += 4) {
- if (!nv_optimized(np)) {
- netdev_info(dev,
- "%03x: %08x %08x // %08x %08x // %08x %08x // %08x %08x\n",
- i,
- le32_to_cpu(np->tx_ring.orig[i].buf),
- le32_to_cpu(np->tx_ring.orig[i].flaglen),
- le32_to_cpu(np->tx_ring.orig[i+1].buf),
- le32_to_cpu(np->tx_ring.orig[i+1].flaglen),
- le32_to_cpu(np->tx_ring.orig[i+2].buf),
- le32_to_cpu(np->tx_ring.orig[i+2].flaglen),
- le32_to_cpu(np->tx_ring.orig[i+3].buf),
- le32_to_cpu(np->tx_ring.orig[i+3].flaglen));
- } else {
+ if (unlikely(debug_tx_timeout)) {
+ int i;
+
+ netdev_info(dev, "Ring at %lx\n", (unsigned long)np->ring_addr);
+ netdev_info(dev, "Dumping tx registers\n");
+ for (i = 0; i <= np->register_size; i += 32) {
netdev_info(dev,
- "%03x: %08x %08x %08x // %08x %08x %08x // %08x %08x %08x // %08x %08x %08x\n",
+ "%3x: %08x %08x %08x %08x "
+ "%08x %08x %08x %08x\n",
i,
- le32_to_cpu(np->tx_ring.ex[i].bufhigh),
- le32_to_cpu(np->tx_ring.ex[i].buflow),
- le32_to_cpu(np->tx_ring.ex[i].flaglen),
- le32_to_cpu(np->tx_ring.ex[i+1].bufhigh),
- le32_to_cpu(np->tx_ring.ex[i+1].buflow),
- le32_to_cpu(np->tx_ring.ex[i+1].flaglen),
- le32_to_cpu(np->tx_ring.ex[i+2].bufhigh),
- le32_to_cpu(np->tx_ring.ex[i+2].buflow),
- le32_to_cpu(np->tx_ring.ex[i+2].flaglen),
- le32_to_cpu(np->tx_ring.ex[i+3].bufhigh),
- le32_to_cpu(np->tx_ring.ex[i+3].buflow),
- le32_to_cpu(np->tx_ring.ex[i+3].flaglen));
+ readl(base + i + 0), readl(base + i + 4),
+ readl(base + i + 8), readl(base + i + 12),
+ readl(base + i + 16), readl(base + i + 20),
+ readl(base + i + 24), readl(base + i + 28));
+ }
+ netdev_info(dev, "Dumping tx ring\n");
+ for (i = 0; i < np->tx_ring_size; i += 4) {
+ if (!nv_optimized(np)) {
+ netdev_info(dev,
+ "%03x: %08x %08x // %08x %08x "
+ "// %08x %08x // %08x %08x\n",
+ i,
+ le32_to_cpu(np->tx_ring.orig[i].buf),
+ le32_to_cpu(np->tx_ring.orig[i].flaglen),
+ le32_to_cpu(np->tx_ring.orig[i+1].buf),
+ le32_to_cpu(np->tx_ring.orig[i+1].flaglen),
+ le32_to_cpu(np->tx_ring.orig[i+2].buf),
+ le32_to_cpu(np->tx_ring.orig[i+2].flaglen),
+ le32_to_cpu(np->tx_ring.orig[i+3].buf),
+ le32_to_cpu(np->tx_ring.orig[i+3].flaglen));
+ } else {
+ netdev_info(dev,
+ "%03x: %08x %08x %08x "
+ "// %08x %08x %08x "
+ "// %08x %08x %08x "
+ "// %08x %08x %08x\n",
+ i,
+ le32_to_cpu(np->tx_ring.ex[i].bufhigh),
+ le32_to_cpu(np->tx_ring.ex[i].buflow),
+ le32_to_cpu(np->tx_ring.ex[i].flaglen),
+ le32_to_cpu(np->tx_ring.ex[i+1].bufhigh),
+ le32_to_cpu(np->tx_ring.ex[i+1].buflow),
+ le32_to_cpu(np->tx_ring.ex[i+1].flaglen),
+ le32_to_cpu(np->tx_ring.ex[i+2].bufhigh),
+ le32_to_cpu(np->tx_ring.ex[i+2].buflow),
+ le32_to_cpu(np->tx_ring.ex[i+2].flaglen),
+ le32_to_cpu(np->tx_ring.ex[i+3].bufhigh),
+ le32_to_cpu(np->tx_ring.ex[i+3].buflow),
+ le32_to_cpu(np->tx_ring.ex[i+3].flaglen));
+ }
}
}
@@ -5984,6 +5997,9 @@ module_param(phy_cross, int, 0);
MODULE_PARM_DESC(phy_cross, "Phy crossover detection for Realtek 8201 phy is enabled by setting to 1 and disabled by setting to 0.");
module_param(phy_power_down, int, 0);
MODULE_PARM_DESC(phy_power_down, "Power down phy and disable link when interface is down (1), or leave phy powered up (0).");
+module_param(debug_tx_timeout, bool, 0);
+MODULE_PARM_DESC(debug_tx_timeout,
+ "Dump tx related registers and ring when tx_timeout happens");
MODULE_AUTHOR("Manfred Spraul <manfred@colorfullife.com>");
MODULE_DESCRIPTION("Reverse Engineered nForce ethernet driver");
--
1.7.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH net-next v1 3/9] forcedeth: expose module parameters in /sys/module
2011-11-09 22:09 [PATCH net-next v1 0/9] forcedeth: stats & debug enhancements David Decotigny
2011-11-09 22:09 ` [PATCH net-next v1 1/9] forcedeth: Add messages to indicate using MSI or MSI-X David Decotigny
2011-11-09 22:09 ` [PATCH net-next v1 2/9] forcedeth: allow to silence "TX timeout" debug messages David Decotigny
@ 2011-11-09 22:09 ` David Decotigny
2011-11-09 22:09 ` [PATCH net-next v1 4/9] forcedeth: stats for rx_packets based on hardware registers David Decotigny
` (6 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: David Decotigny @ 2011-11-09 22:09 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: David S. Miller, Ian Campbell, Eric Dumazet, Jeff Kirsher,
Ben Hutchings, David Decotigny
In particular, debug_tx_timeout can be updated at runtime.
Signed-off-by: David Decotigny <david.decotigny@google.com>
---
drivers/net/ethernet/nvidia/forcedeth.c | 18 +++++++++---------
1 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
index 763863d..cd7f83d 100644
--- a/drivers/net/ethernet/nvidia/forcedeth.c
+++ b/drivers/net/ethernet/nvidia/forcedeth.c
@@ -5981,23 +5981,23 @@ static void __exit exit_nic(void)
pci_unregister_driver(&driver);
}
-module_param(max_interrupt_work, int, 0);
+module_param(max_interrupt_work, int, S_IRUGO);
MODULE_PARM_DESC(max_interrupt_work, "forcedeth maximum events handled per interrupt");
-module_param(optimization_mode, int, 0);
+module_param(optimization_mode, int, S_IRUGO);
MODULE_PARM_DESC(optimization_mode, "In throughput mode (0), every tx & rx packet will generate an interrupt. In CPU mode (1), interrupts are controlled by a timer. In dynamic mode (2), the mode toggles between throughput and CPU mode based on network load.");
-module_param(poll_interval, int, 0);
+module_param(poll_interval, int, S_IRUGO);
MODULE_PARM_DESC(poll_interval, "Interval determines how frequent timer interrupt is generated by [(time_in_micro_secs * 100) / (2^10)]. Min is 0 and Max is 65535.");
-module_param(msi, int, 0);
+module_param(msi, int, S_IRUGO);
MODULE_PARM_DESC(msi, "MSI interrupts are enabled by setting to 1 and disabled by setting to 0.");
-module_param(msix, int, 0);
+module_param(msix, int, S_IRUGO);
MODULE_PARM_DESC(msix, "MSIX interrupts are enabled by setting to 1 and disabled by setting to 0.");
-module_param(dma_64bit, int, 0);
+module_param(dma_64bit, int, S_IRUGO);
MODULE_PARM_DESC(dma_64bit, "High DMA is enabled by setting to 1 and disabled by setting to 0.");
-module_param(phy_cross, int, 0);
+module_param(phy_cross, int, S_IRUGO);
MODULE_PARM_DESC(phy_cross, "Phy crossover detection for Realtek 8201 phy is enabled by setting to 1 and disabled by setting to 0.");
-module_param(phy_power_down, int, 0);
+module_param(phy_power_down, int, S_IRUGO);
MODULE_PARM_DESC(phy_power_down, "Power down phy and disable link when interface is down (1), or leave phy powered up (0).");
-module_param(debug_tx_timeout, bool, 0);
+module_param(debug_tx_timeout, bool, S_IRUGO|S_IWUSR);
MODULE_PARM_DESC(debug_tx_timeout,
"Dump tx related registers and ring when tx_timeout happens");
--
1.7.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH net-next v1 4/9] forcedeth: stats for rx_packets based on hardware registers
2011-11-09 22:09 [PATCH net-next v1 0/9] forcedeth: stats & debug enhancements David Decotigny
` (2 preceding siblings ...)
2011-11-09 22:09 ` [PATCH net-next v1 3/9] forcedeth: expose module parameters in /sys/module David Decotigny
@ 2011-11-09 22:09 ` David Decotigny
2011-11-10 18:45 ` David Decotigny
2011-11-09 22:09 ` [PATCH net-next v1 5/9] forcedeth: implement ndo_get_stats64() API David Decotigny
` (5 subsequent siblings)
9 siblings, 1 reply; 13+ messages in thread
From: David Decotigny @ 2011-11-09 22:09 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: David S. Miller, Ian Campbell, Eric Dumazet, Jeff Kirsher,
Ben Hutchings, David Decotigny
Use the hardware registers instead of a software implementation to
account for the number of RX packets.
Signed-off-by: David Decotigny <david.decotigny@google.com>
---
drivers/net/ethernet/nvidia/forcedeth.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
index cd7f83d..0071d5c 100644
--- a/drivers/net/ethernet/nvidia/forcedeth.c
+++ b/drivers/net/ethernet/nvidia/forcedeth.c
@@ -1712,6 +1712,7 @@ static struct net_device_stats *nv_get_stats(struct net_device *dev)
nv_get_hw_stats(dev);
/* copy to net_device stats */
+ dev->stats.rx_packets = np->estats.rx_packets;
dev->stats.tx_packets = np->estats.tx_packets;
dev->stats.rx_bytes = np->estats.rx_bytes;
dev->stats.tx_bytes = np->estats.tx_bytes;
@@ -2690,7 +2691,6 @@ static int nv_rx_process(struct net_device *dev, int limit)
skb_put(skb, len);
skb->protocol = eth_type_trans(skb, dev);
napi_gro_receive(&np->napi, skb);
- dev->stats.rx_packets++;
next_pkt:
if (unlikely(np->get_rx.orig++ == np->last_rx.orig))
np->get_rx.orig = np->first_rx.orig;
@@ -2773,7 +2773,6 @@ static int nv_rx_process_optimized(struct net_device *dev, int limit)
__vlan_hwaccel_put_tag(skb, vid);
}
napi_gro_receive(&np->napi, skb);
- dev->stats.rx_packets++;
} else {
dev_kfree_skb(skb);
}
--
1.7.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH net-next v1 4/9] forcedeth: stats for rx_packets based on hardware registers
2011-11-09 22:09 ` [PATCH net-next v1 4/9] forcedeth: stats for rx_packets based on hardware registers David Decotigny
@ 2011-11-10 18:45 ` David Decotigny
0 siblings, 0 replies; 13+ messages in thread
From: David Decotigny @ 2011-11-10 18:45 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: David S. Miller, Ian Campbell, Eric Dumazet, Jeff Kirsher,
Ben Hutchings, David Decotigny
Hi all,
After discussion with original author of patch "forcedeth: statistics
optimization" (21828163b2b31e0675cb3e66a2a4a231dec06236), I will
revise this strategy, and step back to using software counters to make
the accounting of packets received at software level more accurate.
Will send an update to this series shortly.
Regards,
--
David Decotigny
On Wed, Nov 9, 2011 at 2:09 PM, David Decotigny
<david.decotigny@google.com> wrote:
> Use the hardware registers instead of a software implementation to
> account for the number of RX packets.
>
>
>
> Signed-off-by: David Decotigny <david.decotigny@google.com>
> ---
> drivers/net/ethernet/nvidia/forcedeth.c | 3 +--
> 1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
> index cd7f83d..0071d5c 100644
> --- a/drivers/net/ethernet/nvidia/forcedeth.c
> +++ b/drivers/net/ethernet/nvidia/forcedeth.c
> @@ -1712,6 +1712,7 @@ static struct net_device_stats *nv_get_stats(struct net_device *dev)
> nv_get_hw_stats(dev);
>
> /* copy to net_device stats */
> + dev->stats.rx_packets = np->estats.rx_packets;
> dev->stats.tx_packets = np->estats.tx_packets;
> dev->stats.rx_bytes = np->estats.rx_bytes;
> dev->stats.tx_bytes = np->estats.tx_bytes;
> @@ -2690,7 +2691,6 @@ static int nv_rx_process(struct net_device *dev, int limit)
> skb_put(skb, len);
> skb->protocol = eth_type_trans(skb, dev);
> napi_gro_receive(&np->napi, skb);
> - dev->stats.rx_packets++;
> next_pkt:
> if (unlikely(np->get_rx.orig++ == np->last_rx.orig))
> np->get_rx.orig = np->first_rx.orig;
> @@ -2773,7 +2773,6 @@ static int nv_rx_process_optimized(struct net_device *dev, int limit)
> __vlan_hwaccel_put_tag(skb, vid);
> }
> napi_gro_receive(&np->napi, skb);
> - dev->stats.rx_packets++;
> } else {
> dev_kfree_skb(skb);
> }
> --
> 1.7.3.1
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net-next v1 5/9] forcedeth: implement ndo_get_stats64() API
2011-11-09 22:09 [PATCH net-next v1 0/9] forcedeth: stats & debug enhancements David Decotigny
` (3 preceding siblings ...)
2011-11-09 22:09 ` [PATCH net-next v1 4/9] forcedeth: stats for rx_packets based on hardware registers David Decotigny
@ 2011-11-09 22:09 ` David Decotigny
2011-11-09 23:43 ` Ben Hutchings
2011-11-09 22:09 ` [PATCH net-next v1 6/9] forcedeth: account for dropped RX frames David Decotigny
` (4 subsequent siblings)
9 siblings, 1 reply; 13+ messages in thread
From: David Decotigny @ 2011-11-09 22:09 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: David S. Miller, Ian Campbell, Eric Dumazet, Jeff Kirsher,
Ben Hutchings, David Decotigny
This commit implements the ndo_get_stats64() API for forcedeth. Since
these stats are being updated from different contexts (process and
timer), this commit adds protection (locking + atomic variables).
Tested:
16-way SMP x86_64 ->
RX bytes:7244556582 (7.2 GB) TX bytes:181904254 (181.9 MB)
Signed-off-by: David Decotigny <david.decotigny@google.com>
---
drivers/net/ethernet/nvidia/forcedeth.c | 132 +++++++++++++++++++++++--------
1 files changed, 99 insertions(+), 33 deletions(-)
diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
index 0071d5c..0b4fd1a 100644
--- a/drivers/net/ethernet/nvidia/forcedeth.c
+++ b/drivers/net/ethernet/nvidia/forcedeth.c
@@ -692,6 +692,19 @@ struct nv_ethtool_stats {
#define NV_DEV_STATISTICS_V2_COUNT (NV_DEV_STATISTICS_V3_COUNT - 3)
#define NV_DEV_STATISTICS_V1_COUNT (NV_DEV_STATISTICS_V2_COUNT - 6)
+/* driver statistics */
+struct nv_driver_stat {
+ atomic_t delta; /* increase since last nv_update_stats() */
+ u64 total; /* cumulative, requires netdev_priv(dev)->stats_lock */
+};
+
+#define NV_DRIVER_STAT_ATOMIC_INC(ptr_stat) /* atomic */ \
+ ({ atomic_inc(&(ptr_stat)->delta); })
+#define NV_DRIVER_STAT_UPDATE_TOTAL(ptr_stat) /* requires stats_lock */ \
+ ({ (ptr_stat)->total += atomic_xchg(&(ptr_stat)->delta, 0); })
+#define NV_DRIVER_STAT_GET_TOTAL(ptr_stat) /* requires stats_lock */ \
+ ((ptr_stat)->total)
+
/* diagnostics */
#define NV_TEST_COUNT_BASE 3
#define NV_TEST_COUNT_EXTENDED 4
@@ -736,6 +749,12 @@ struct nv_skb_map {
* - tx setup is lockless: it relies on netif_tx_lock. Actual submission
* needs netdev_priv(dev)->lock :-(
* - set_multicast_list: preparation lockless, relies on netif_tx_lock.
+ *
+ * Stats are protected with stats_lock:
+ * - updated by nv_do_stats_poll (timer). This is meant to avoid
+ * integer wraparound in the NIC stats registers, at low frequency
+ * (0.1 Hz)
+ * - updated by nv_get_ethtool_stats + nv_get_stats64
*/
/* in dev: base, irq */
@@ -745,9 +764,10 @@ struct fe_priv {
struct net_device *dev;
struct napi_struct napi;
- /* General data:
- * Locking: spin_lock(&np->lock); */
+ /* stats are updated in syscall and timer */
+ spinlock_t stats_lock;
struct nv_ethtool_stats estats;
+
int in_shutdown;
u32 linkspeed;
int duplex;
@@ -797,6 +817,7 @@ struct fe_priv {
struct timer_list stats_poll;
u32 nic_poll_irq;
int rx_ring_size;
+ struct nv_driver_stat stat_rx_missed_errors;
/* media detection workaround.
* Locking: Within irq hander or disable_irq+spin_lock(&np->lock);
@@ -819,6 +840,7 @@ struct fe_priv {
struct nv_skb_map *tx_change_owner;
struct nv_skb_map *tx_end_flip;
int tx_stop;
+ struct nv_driver_stat stat_tx_dropped;
/* msi/msi-x fields */
u32 msi_flags;
@@ -1635,11 +1657,19 @@ static void nv_mac_reset(struct net_device *dev)
pci_push(base);
}
-static void nv_get_hw_stats(struct net_device *dev)
+/* Caller must appropriately lock netdev_priv(dev)->stats_lock */
+static void nv_update_stats(struct net_device *dev)
{
struct fe_priv *np = netdev_priv(dev);
u8 __iomem *base = get_hwbase(dev);
+ /* If it happens that this is run in top-half context, then
+ * replace the spin_lock of stats_lock with
+ * spin_lock_irqsave() in calling functions. */
+ WARN_ONCE(in_irq(), "forcedeth: estats spin_lock(_bh) from top-half");
+ assert_spin_locked(&np->stats_lock);
+
+ /* query hardware */
np->estats.tx_bytes += readl(base + NvRegTxCnt);
np->estats.tx_zero_rexmt += readl(base + NvRegTxZeroReXmt);
np->estats.tx_one_rexmt += readl(base + NvRegTxOneReXmt);
@@ -1695,37 +1725,61 @@ static void nv_get_hw_stats(struct net_device *dev)
np->estats.tx_multicast += readl(base + NvRegTxMulticast);
np->estats.tx_broadcast += readl(base + NvRegTxBroadcast);
}
+
+ /* update software stats */
+ NV_DRIVER_STAT_UPDATE_TOTAL(&np->stat_tx_dropped);
+ NV_DRIVER_STAT_UPDATE_TOTAL(&np->stat_rx_missed_errors);
}
/*
- * nv_get_stats: dev->get_stats function
+ * nv_get_stats64: dev->ndo_get_stats64 function
* Get latest stats value from the nic.
* Called with read_lock(&dev_base_lock) held for read -
* only synchronized against unregister_netdevice.
*/
-static struct net_device_stats *nv_get_stats(struct net_device *dev)
+static struct rtnl_link_stats64*
+nv_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *storage)
+ __acquires(&netdev_priv(dev)->stats_lock)
+ __releases(&netdev_priv(dev)->stats_lock)
{
struct fe_priv *np = netdev_priv(dev);
/* If the nic supports hw counters then retrieve latest values */
- if (np->driver_data & (DEV_HAS_STATISTICS_V1|DEV_HAS_STATISTICS_V2|DEV_HAS_STATISTICS_V3)) {
- nv_get_hw_stats(dev);
-
- /* copy to net_device stats */
- dev->stats.rx_packets = np->estats.rx_packets;
- dev->stats.tx_packets = np->estats.tx_packets;
- dev->stats.rx_bytes = np->estats.rx_bytes;
- dev->stats.tx_bytes = np->estats.tx_bytes;
- dev->stats.tx_fifo_errors = np->estats.tx_fifo_errors;
- dev->stats.tx_carrier_errors = np->estats.tx_carrier_errors;
- dev->stats.rx_crc_errors = np->estats.rx_crc_errors;
- dev->stats.rx_over_errors = np->estats.rx_over_errors;
- dev->stats.rx_fifo_errors = np->estats.rx_drop_frame;
- dev->stats.rx_errors = np->estats.rx_errors_total;
- dev->stats.tx_errors = np->estats.tx_errors_total;
- }
-
- return &dev->stats;
+ if (np->driver_data & (DEV_HAS_STATISTICS_V1
+ | DEV_HAS_STATISTICS_V2
+ | DEV_HAS_STATISTICS_V3)) {
+ spin_lock_bh(&np->stats_lock);
+
+ nv_update_stats(dev);
+
+ /* generic stats */
+ storage->rx_packets = np->estats.rx_packets;
+ storage->tx_packets = np->estats.tx_packets;
+ storage->rx_bytes = np->estats.rx_bytes;
+ storage->tx_bytes = np->estats.tx_bytes;
+ storage->rx_errors = np->estats.rx_errors_total;
+ storage->tx_errors = np->estats.tx_errors_total;
+ storage->tx_dropped = NV_DRIVER_STAT_GET_TOTAL(
+ &np->stat_tx_dropped);
+ storage->multicast = np->estats.rx_multicast;
+
+ /* detailed rx_errors */
+ storage->rx_length_errors = np->estats.rx_length_error;
+ storage->rx_over_errors = np->estats.rx_over_errors;
+ storage->rx_crc_errors = np->estats.rx_crc_errors;
+ storage->rx_frame_errors = np->estats.rx_frame_align_error;
+ storage->rx_fifo_errors = np->estats.rx_drop_frame;
+ storage->rx_missed_errors = NV_DRIVER_STAT_GET_TOTAL(
+ &np->stat_rx_missed_errors);
+
+ /* detailed tx_errors */
+ storage->tx_carrier_errors = np->estats.tx_carrier_errors;
+ storage->tx_fifo_errors = np->estats.tx_fifo_errors;
+
+ spin_unlock_bh(&np->stats_lock);
+ }
+
+ return storage;
}
/*
@@ -1927,7 +1981,7 @@ static void nv_drain_tx(struct net_device *dev)
np->tx_ring.ex[i].buflow = 0;
}
if (nv_release_txskb(np, &np->tx_skb[i]))
- dev->stats.tx_dropped++;
+ NV_DRIVER_STAT_ATOMIC_INC(&np->stat_tx_dropped);
np->tx_skb[i].dma = 0;
np->tx_skb[i].dma_len = 0;
np->tx_skb[i].dma_single = 0;
@@ -2648,7 +2702,7 @@ static int nv_rx_process(struct net_device *dev, int limit)
/* the rest are hard errors */
else {
if (flags & NV_RX_MISSEDFRAME)
- dev->stats.rx_missed_errors++;
+ NV_DRIVER_STAT_ATOMIC_INC(&np->stat_rx_missed_errors);
dev_kfree_skb(skb);
goto next_pkt;
}
@@ -3900,11 +3954,18 @@ static void nv_poll_controller(struct net_device *dev)
#endif
static void nv_do_stats_poll(unsigned long data)
+ __acquires(&netdev_priv(dev)->stats_lock)
+ __releases(&netdev_priv(dev)->stats_lock)
{
struct net_device *dev = (struct net_device *) data;
struct fe_priv *np = netdev_priv(dev);
- nv_get_hw_stats(dev);
+ /* If lock is currently taken, the stats are being refreshed
+ * and hence fresh enough */
+ if (spin_trylock(&np->stats_lock)) {
+ nv_update_stats(dev);
+ spin_unlock(&np->stats_lock);
+ }
if (!np->in_shutdown)
mod_timer(&np->stats_poll,
@@ -4549,14 +4610,18 @@ static int nv_get_sset_count(struct net_device *dev, int sset)
}
}
-static void nv_get_ethtool_stats(struct net_device *dev, struct ethtool_stats *estats, u64 *buffer)
+static void nv_get_ethtool_stats(struct net_device *dev,
+ struct ethtool_stats *estats, u64 *buffer)
+ __acquires(&netdev_priv(dev)->stats_lock)
+ __releases(&netdev_priv(dev)->stats_lock)
{
struct fe_priv *np = netdev_priv(dev);
- /* update stats */
- nv_get_hw_stats(dev);
-
- memcpy(buffer, &np->estats, nv_get_sset_count(dev, ETH_SS_STATS)*sizeof(u64));
+ spin_lock_bh(&np->stats_lock);
+ nv_update_stats(dev);
+ memcpy(buffer, &np->estats,
+ nv_get_sset_count(dev, ETH_SS_STATS)*sizeof(u64));
+ spin_unlock_bh(&np->stats_lock);
}
static int nv_link_test(struct net_device *dev)
@@ -5194,7 +5259,7 @@ static int nv_close(struct net_device *dev)
static const struct net_device_ops nv_netdev_ops = {
.ndo_open = nv_open,
.ndo_stop = nv_close,
- .ndo_get_stats = nv_get_stats,
+ .ndo_get_stats64 = nv_get_stats64,
.ndo_start_xmit = nv_start_xmit,
.ndo_tx_timeout = nv_tx_timeout,
.ndo_change_mtu = nv_change_mtu,
@@ -5211,7 +5276,7 @@ static const struct net_device_ops nv_netdev_ops = {
static const struct net_device_ops nv_netdev_ops_optimized = {
.ndo_open = nv_open,
.ndo_stop = nv_close,
- .ndo_get_stats = nv_get_stats,
+ .ndo_get_stats64 = nv_get_stats64,
.ndo_start_xmit = nv_start_xmit_optimized,
.ndo_tx_timeout = nv_tx_timeout,
.ndo_change_mtu = nv_change_mtu,
@@ -5250,6 +5315,7 @@ static int __devinit nv_probe(struct pci_dev *pci_dev, const struct pci_device_i
np->dev = dev;
np->pci_dev = pci_dev;
spin_lock_init(&np->lock);
+ spin_lock_init(&np->stats_lock);
SET_NETDEV_DEV(dev, &pci_dev->dev);
init_timer(&np->oom_kick);
--
1.7.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH net-next v1 5/9] forcedeth: implement ndo_get_stats64() API
2011-11-09 22:09 ` [PATCH net-next v1 5/9] forcedeth: implement ndo_get_stats64() API David Decotigny
@ 2011-11-09 23:43 ` Ben Hutchings
0 siblings, 0 replies; 13+ messages in thread
From: Ben Hutchings @ 2011-11-09 23:43 UTC (permalink / raw)
To: David Decotigny
Cc: netdev, linux-kernel, David S. Miller, Ian Campbell, Eric Dumazet,
Jeff Kirsher
On Wed, 2011-11-09 at 14:09 -0800, David Decotigny wrote:
> This commit implements the ndo_get_stats64() API for forcedeth. Since
> these stats are being updated from different contexts (process and
> timer), this commit adds protection (locking + atomic variables).
[...]
I don't claim to have reviewed this properly, but it looks reasonable.
Ben.
--
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net-next v1 6/9] forcedeth: account for dropped RX frames
2011-11-09 22:09 [PATCH net-next v1 0/9] forcedeth: stats & debug enhancements David Decotigny
` (4 preceding siblings ...)
2011-11-09 22:09 ` [PATCH net-next v1 5/9] forcedeth: implement ndo_get_stats64() API David Decotigny
@ 2011-11-09 22:09 ` David Decotigny
2011-11-09 22:09 ` [PATCH net-next v1 7/9] forcedeth: new ethtool stat counter for TX timeouts David Decotigny
` (3 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: David Decotigny @ 2011-11-09 22:09 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: David S. Miller, Ian Campbell, Eric Dumazet, Jeff Kirsher,
Ben Hutchings, David Decotigny
This adds the stats counter for dropped RX frames.
Signed-off-by: David Decotigny <david.decotigny@google.com>
---
drivers/net/ethernet/nvidia/forcedeth.c | 12 ++++++++++--
1 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
index 0b4fd1a..85e34b6c 100644
--- a/drivers/net/ethernet/nvidia/forcedeth.c
+++ b/drivers/net/ethernet/nvidia/forcedeth.c
@@ -818,6 +818,7 @@ struct fe_priv {
u32 nic_poll_irq;
int rx_ring_size;
struct nv_driver_stat stat_rx_missed_errors;
+ struct nv_driver_stat stat_rx_dropped;
/* media detection workaround.
* Locking: Within irq hander or disable_irq+spin_lock(&np->lock);
@@ -1727,6 +1728,7 @@ static void nv_update_stats(struct net_device *dev)
}
/* update software stats */
+ NV_DRIVER_STAT_UPDATE_TOTAL(&np->stat_rx_dropped);
NV_DRIVER_STAT_UPDATE_TOTAL(&np->stat_tx_dropped);
NV_DRIVER_STAT_UPDATE_TOTAL(&np->stat_rx_missed_errors);
}
@@ -1759,6 +1761,8 @@ nv_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *storage)
storage->tx_bytes = np->estats.tx_bytes;
storage->rx_errors = np->estats.rx_errors_total;
storage->tx_errors = np->estats.tx_errors_total;
+ storage->rx_dropped = NV_DRIVER_STAT_GET_TOTAL(
+ &np->stat_rx_dropped);
storage->tx_dropped = NV_DRIVER_STAT_GET_TOTAL(
&np->stat_tx_dropped);
storage->multicast = np->estats.rx_multicast;
@@ -1812,8 +1816,10 @@ static int nv_alloc_rx(struct net_device *dev)
np->put_rx.orig = np->first_rx.orig;
if (unlikely(np->put_rx_ctx++ == np->last_rx_ctx))
np->put_rx_ctx = np->first_rx_ctx;
- } else
+ } else {
+ NV_DRIVER_STAT_ATOMIC_INC(&np->stat_rx_dropped);
return 1;
+ }
}
return 0;
}
@@ -1844,8 +1850,10 @@ static int nv_alloc_rx_optimized(struct net_device *dev)
np->put_rx.ex = np->first_rx.ex;
if (unlikely(np->put_rx_ctx++ == np->last_rx_ctx))
np->put_rx_ctx = np->first_rx_ctx;
- } else
+ } else {
+ NV_DRIVER_STAT_ATOMIC_INC(&np->stat_rx_dropped);
return 1;
+ }
}
return 0;
}
--
1.7.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH net-next v1 7/9] forcedeth: new ethtool stat counter for TX timeouts
2011-11-09 22:09 [PATCH net-next v1 0/9] forcedeth: stats & debug enhancements David Decotigny
` (5 preceding siblings ...)
2011-11-09 22:09 ` [PATCH net-next v1 6/9] forcedeth: account for dropped RX frames David Decotigny
@ 2011-11-09 22:09 ` David Decotigny
2011-11-09 22:09 ` [PATCH net-next v1 8/9] forcedeth: stats updated with a deferrable timer David Decotigny
` (2 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: David Decotigny @ 2011-11-09 22:09 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: David S. Miller, Ian Campbell, Eric Dumazet, Jeff Kirsher,
Ben Hutchings, Sameer Nanda, David Decotigny
From: Sameer Nanda <snanda@google.com>
This change publishes a new ethtool stats: tx_timeout that counts the
number of times the tx_timeout callback was triggered.
Signed-off-by: David Decotigny <david.decotigny@google.com>
---
drivers/net/ethernet/nvidia/forcedeth.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
index 85e34b6c..6a6604a 100644
--- a/drivers/net/ethernet/nvidia/forcedeth.c
+++ b/drivers/net/ethernet/nvidia/forcedeth.c
@@ -633,6 +633,7 @@ static const struct nv_ethtool_str nv_estats_str[] = {
{ "rx_packets" },
{ "rx_errors_total" },
{ "tx_errors_total" },
+ { "tx_timeout" },
/* version 2 stats */
{ "tx_deferral" },
@@ -673,6 +674,7 @@ struct nv_ethtool_stats {
u64 rx_packets;
u64 rx_errors_total;
u64 tx_errors_total;
+ u64 tx_timeout;
/* version 2 stats */
u64 tx_deferral;
@@ -842,6 +844,7 @@ struct fe_priv {
struct nv_skb_map *tx_end_flip;
int tx_stop;
struct nv_driver_stat stat_tx_dropped;
+ atomic_t stat_tx_timeout; /* TX timeouts since last nv_update_stats */
/* msi/msi-x fields */
u32 msi_flags;
@@ -1728,6 +1731,7 @@ static void nv_update_stats(struct net_device *dev)
}
/* update software stats */
+ np->estats.tx_timeout += atomic_xchg(&np->stat_tx_timeout, 0);
NV_DRIVER_STAT_UPDATE_TOTAL(&np->stat_rx_dropped);
NV_DRIVER_STAT_UPDATE_TOTAL(&np->stat_tx_dropped);
NV_DRIVER_STAT_UPDATE_TOTAL(&np->stat_rx_missed_errors);
@@ -2590,6 +2594,8 @@ static void nv_tx_timeout(struct net_device *dev)
}
}
+ atomic_inc(&np->stat_tx_timeout);
+
spin_lock_irq(&np->lock);
/* 1) stop tx engine */
--
1.7.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH net-next v1 8/9] forcedeth: stats updated with a deferrable timer
2011-11-09 22:09 [PATCH net-next v1 0/9] forcedeth: stats & debug enhancements David Decotigny
` (6 preceding siblings ...)
2011-11-09 22:09 ` [PATCH net-next v1 7/9] forcedeth: new ethtool stat counter for TX timeouts David Decotigny
@ 2011-11-09 22:09 ` David Decotigny
2011-11-09 22:09 ` [PATCH net-next v1 9/9] forcedeth: whitespace/indentation fixes David Decotigny
2011-11-09 23:41 ` [PATCH net-next v1 0/9] forcedeth: stats & debug enhancements Ben Hutchings
9 siblings, 0 replies; 13+ messages in thread
From: David Decotigny @ 2011-11-09 22:09 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: David S. Miller, Ian Campbell, Eric Dumazet, Jeff Kirsher,
Ben Hutchings, David Decotigny
Stats don't need to be updated periodically unless there is at least
an IRQ: mark stats timer as deferrable.
Signed-off-by: David Decotigny <david.decotigny@google.com>
---
drivers/net/ethernet/nvidia/forcedeth.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
index 6a6604a..2e45a45 100644
--- a/drivers/net/ethernet/nvidia/forcedeth.c
+++ b/drivers/net/ethernet/nvidia/forcedeth.c
@@ -5338,7 +5338,7 @@ static int __devinit nv_probe(struct pci_dev *pci_dev, const struct pci_device_i
init_timer(&np->nic_poll);
np->nic_poll.data = (unsigned long) dev;
np->nic_poll.function = nv_do_nic_poll; /* timer handler */
- init_timer(&np->stats_poll);
+ init_timer_deferrable(&np->stats_poll);
np->stats_poll.data = (unsigned long) dev;
np->stats_poll.function = nv_do_stats_poll; /* timer handler */
--
1.7.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH net-next v1 9/9] forcedeth: whitespace/indentation fixes
2011-11-09 22:09 [PATCH net-next v1 0/9] forcedeth: stats & debug enhancements David Decotigny
` (7 preceding siblings ...)
2011-11-09 22:09 ` [PATCH net-next v1 8/9] forcedeth: stats updated with a deferrable timer David Decotigny
@ 2011-11-09 22:09 ` David Decotigny
2011-11-09 23:41 ` [PATCH net-next v1 0/9] forcedeth: stats & debug enhancements Ben Hutchings
9 siblings, 0 replies; 13+ messages in thread
From: David Decotigny @ 2011-11-09 22:09 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: David S. Miller, Ian Campbell, Eric Dumazet, Jeff Kirsher,
Ben Hutchings, David Decotigny
Signed-off-by: David Decotigny <david.decotigny@google.com>
---
drivers/net/ethernet/nvidia/forcedeth.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
index 2e45a45..669578f 100644
--- a/drivers/net/ethernet/nvidia/forcedeth.c
+++ b/drivers/net/ethernet/nvidia/forcedeth.c
@@ -65,7 +65,7 @@
#include <linux/slab.h>
#include <linux/uaccess.h>
#include <linux/prefetch.h>
-#include <linux/io.h>
+#include <linux/io.h>
#include <asm/irq.h>
#include <asm/system.h>
--
1.7.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH net-next v1 0/9] forcedeth: stats & debug enhancements
2011-11-09 22:09 [PATCH net-next v1 0/9] forcedeth: stats & debug enhancements David Decotigny
` (8 preceding siblings ...)
2011-11-09 22:09 ` [PATCH net-next v1 9/9] forcedeth: whitespace/indentation fixes David Decotigny
@ 2011-11-09 23:41 ` Ben Hutchings
9 siblings, 0 replies; 13+ messages in thread
From: Ben Hutchings @ 2011-11-09 23:41 UTC (permalink / raw)
To: David Decotigny
Cc: netdev, linux-kernel, David S. Miller, Ian Campbell, Eric Dumazet,
Jeff Kirsher
On Wed, 2011-11-09 at 14:09 -0800, David Decotigny wrote:
> These changes implement the ndo_get_stats64 API and add a few more
> stats and debugging features for forcedeth. They also ensure that
> stats updates are correct in SMP systems, 32 or 64-bits.
>
> Regarding the "implement ndo_get_stats64() API" patch, I'm not sure
> I'm using the right way to protect the 64b stats. Ideally, I would
> like them to be non-blocking (u64_stats_sync.h), but as there are
> several sources for updates, I don't think I can do without locking or
> per-CPU stats.
The important thing is that the fast path remains fast, and I think
you've achieved that (due to patch 4/9). The data path doesn't have to
acquire the stats lock and it only has to use atomic operations in some
error cases.
> Would per-CPU stats be better here (note: I expect the
> contention on netdev_priv(dev)->stats_lock to be _VERY_ low)?
[...]
Only queue-less software devices should maintain per-CPU stats. For any
device with queues, servicing of each queue must already be serialised
and software stats can be maintained per-queue. In the single-queue
case this means per-device.
Ben.
--
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.
^ permalink raw reply [flat|nested] 13+ messages in thread