Re: [tbench regression fixes]: digging out smelly deadmen.

Eric Dumazet
2008-10-31T10:46:40Z

[-- Attachment #1: Type: text/plain, Size: 1359 bytes --]

David Miller a écrit :
> From: "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi>
> Date: Fri, 31 Oct 2008 11:40:16 +0200 (EET)
>
>> Let me remind that it is just a single process, so no ping-pong & other
>> lock related cache effects should play any significant role here, no? (I'm
>> no expert though :-)).
>
> Not locks or ping-pongs perhaps, I guess. So it just sends and
> receives over a socket, implementing both ends of the communication
> in the same process?
>
> If hash chain conflicts do happen for those 2 sockets, just traversing
> the chain 2 entries deep could show up.

tbench is very sensible to cache line ping-pongs (on SMP machines of course)

Just to prove my point, I coded the following patch and tried it
on a HP BL460c G1. This machine has 2 quad cores cpu
(Intel(R) Xeon(R) CPU E5450 @3.00GHz)

tbench 8 went from 2240 MB/s to 2310 MB/s after this patch applied

[PATCH] net: Introduce netif_set_last_rx() helper

On SMP machine, loopback device (and possibly others net device)
should try to avoid dirty the memory cache line containing "last_rx"
field. Got 3% increase on tbench on a 8 cpus machine.

Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>
---
drivers/net/loopback.c | 2 +-
include/linux/netdevice.h | 16 ++++++++++++++++
2 files changed, 17 insertions(+), 1 deletion(-)

[-- Attachment #2: netif_set_last_rx.patch --]
[-- Type: text/plain, Size: 1232 bytes --]

diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
index 3b43bfd..cf17238 100644
--- a/drivers/net/loopback.c
+++ b/drivers/net/loopback.c
@@ -85,7 +85,7 @@ static int loopback_xmit(struct sk_buff *skb, struct net_device *dev)
return 0;
}
#endif
- dev->last_rx = jiffies;
+ netif_set_last_rx(dev);

/* it's OK to use per_cpu_ptr() because BHs are off */
pcpu_lstats = dev->ml_priv;
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index c8bcb59..6729865 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -849,6 +849,22 @@ static inline void *netdev_priv(const struct net_device *dev)
#define SET_NETDEV_DEV(net, pdev) ((net)->dev.parent = (pdev))

/**
+ * netif_set_last_rx - Set last_rx field of a device
+ * @dev: network device
+ *
+ * Instead of setting net->last_rx to jiffies, drivers should call this helper
+ * to avoid dirtying a cache line if last_rx already has the current jiffies
+ */
+static inline void netif_set_last_rx(struct net_device *dev)
+{
+#ifdef CONFIG_SMP
+ if (dev->last_rx == jiffies)
+ return;
+#endif
+ dev->last_rx = jiffies;
+}
+
+/**
* netif_napi_add - initialize a napi context
* @dev: network device
* @napi: napi context