David S. Miller a écrit :
From: Eric Dumazet <[EMAIL PROTECTED]>
Date: Wed, 08 Feb 2006 16:06:31 +0100
loopback driver carefully uses per_cpu storage for statistics but updates
loopback_dev.last_rx
This has been discussed before, this is an attribute every
driver must keep uptodate. Things like bonding use it,
for example.
Yes, loopback isn't usable with bonding, but it's a bad
precedence to set that a useful metric is set for some
devices and not for others.
Find a cleaner way to fix this SMP cacheline pingpong,
thanks.
OK David, thanks for your (always excellent :) ) feedback.
What do you think of this patch then ?
[PATCH] : NET : Introduce netif_last_rx_update() method
last_rx is changed each time a packet is received on an net device.
This can cause cache line ping pongs in SMP machines, reducing performance.
This effect is particularly bad on loopback device since no IRQ affinity can
solve the problem (Ethernet devices can be tuned so that all incoming frames
are handled by one CPU, avoiding this cache line ping pong. But loopback
driver is used by user processes on all cpus)
Instead of letting each driver doing plain "dev->last_rx = jiffies;" we can do
some SMP optimizations once in include/linux/netdevice.h, and adapt this
method if necessary. (Current uses of last_rx are limited to bonding, but this
could change in the future, or for debugging purposes)
This patch :
- Defines netif_last_rx_update() inline function.
Currently, we avoid a change on last_rx if the device is not a slave, or
if last_rx value is already 'jiffies'.
For slave devices, the number of ping-pongs is maxed to NR_CPUS*HZ per
second. Loopback is not a slave candidate, and IRQ affinity should be the
answer for ethernet drivers.
- Changes drivers/net/loopback.c to :
-- use this function.
-- (and last_rx should be updated even if LOOPBACK_TSO is defined.)
-- kmalloc/kzalloc conversion in loopback_init()
Signed-off-by: Eric Dumazet <[EMAIL PROTECTED]>
--- a/include/linux/netdevice.h 2006-02-07 11:55:42.000000000 +0100
+++ b/include/linux/netdevice.h 2006-02-09 09:23:15.000000000 +0100
@@ -649,6 +649,19 @@
return test_bit(__LINK_STATE_START, &dev->state);
}
+static inline void netif_last_rx_update(struct net_device *dev)
+{
+#ifdef CONFIG_SMP
+ /*
+ * In order to avoid cache line ping pongs on last_rx, we check
+ * if the device is a slave,
+ * and if last_rx really has to be modified
+ */
+ if (!(dev->flags & IFF_SLAVE) || (dev->last_rx == jiffies))
+ return;
+#endif
+ dev->last_rx = jiffies;
+}
/* Use this variant when it is known for sure that it
* is executing from interrupt context.
--- a/drivers/net/loopback.c 2006-02-07 12:10:55.000000000 +0100
+++ b/drivers/net/loopback.c 2006-02-09 09:40:23.000000000 +0100
@@ -138,6 +138,7 @@
skb->ip_summed = CHECKSUM_UNNECESSARY;
#endif
+ netif_last_rx_update(dev);
#ifdef LOOPBACK_TSO
if (skb_shinfo(skb)->tso_size) {
BUG_ON(skb->protocol != htons(ETH_P_IP));
@@ -147,7 +148,6 @@
return 0;
}
#endif
- dev->last_rx = jiffies;
lb_stats = &per_cpu(loopback_stats, get_cpu());
lb_stats->rx_bytes += skb->len;
@@ -226,9 +226,8 @@
struct net_device_stats *stats;
/* Can survive without statistics */
- stats = kmalloc(sizeof(struct net_device_stats), GFP_KERNEL);
+ stats = kzalloc(sizeof(struct net_device_stats), GFP_KERNEL);
if (stats) {
- memset(stats, 0, sizeof(struct net_device_stats));
loopback_dev.priv = stats;
loopback_dev.get_stats = &get_stats;
}
