| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4ABBDA45.2040109@gmail.com>
Date: Thu, 24 Sep 2009 22:44:53 +0200
From: Eric Dumazet <eric.dumazet@...il.com>
To: Stephen Hemminger <shemminger@...tta.com>
CC: "David S. Miller" <davem@...emloft.net>,
Linux Netdev List <netdev@...r.kernel.org>
Subject: Re: [PATCH] inet_peer: Optimize inet_getid()
Stephen Hemminger a écrit :
> On Thu, 24 Sep 2009 21:04:56 +0200
> Eric Dumazet <eric.dumazet@...il.com> wrote:
>
>> While investigating for network latencies, I found inet_getid() was a contention point
>> for some workloads.
>>
>> Fix is straightforward, using cmpxchg() instead of
>> a spin_lock_bh()/spin_unlock_bh() pair on a central lock.
>>
>> Another possibility was to use an atomic_t and atomic_add_return() but
>> the size of struct inet_peer object would had doubled on x86_64 because of
>> SLAB_HWCACHE_ALIGN constraint.
>>
>> Signed-off-by: Eric Dumazet <eric.dumazet@...il.com>
>
> I thought cmpxchg was not available on all architectures.
Here is the second version of the patch, thanks Stephen
[PATCH] inet_peer: Optimize inet_getid()
While investigating for network latencies, I found inet_getid() was a contention point
for some workloads.
If __HAVE_ARCH_CMPXCHG is defined, we can use cmpxchg() instead of
a spin_lock_bh()/spin_unlock_bh() pair on a central lock.
On other arches, we can use an atomic_t instead of a u16,
and atomic_add_return(). This might grow memory usage a bit, unless
someone invents atomic16_t :)
Signed-off-by: Eric Dumazet <eric.dumazet@...il.com>
---
include/net/inetpeer.h | 43 ++++++++++++++++++++++++++++++++-------
net/ipv4/inetpeer.c | 5 ----
net/ipv4/route.c | 2 -
3 files changed, 38 insertions(+), 12 deletions(-)
diff --git a/include/net/inetpeer.h b/include/net/inetpeer.h
index 15e1f8f..f1cde51 100644
--- a/include/net/inetpeer.h
+++ b/include/net/inetpeer.h
@@ -19,7 +19,11 @@ struct inet_peer
struct inet_peer *avl_left, *avl_right;
__be32 v4daddr; /* peer's address */
__u16 avl_height;
+#if defined(__HAVE_ARCH_CMPXCHG)
__u16 ip_id_count; /* IP ID for the next packet */
+#else
+ atomic_t ip_id_count;
+#endif
struct list_head unused;
__u32 dtime; /* the time of last use of not
* referenced entries */
@@ -37,17 +41,42 @@ struct inet_peer *inet_getpeer(__be32 daddr, int create);
/* can be called from BH context or outside */
extern void inet_putpeer(struct inet_peer *p);
-extern spinlock_t inet_peer_idlock;
/* can be called with or without local BH being disabled */
static inline __u16 inet_getid(struct inet_peer *p, int more)
{
- __u16 id;
+ __u16 old;
- spin_lock_bh(&inet_peer_idlock);
- id = p->ip_id_count;
- p->ip_id_count += 1 + more;
- spin_unlock_bh(&inet_peer_idlock);
- return id;
+ more++;
+#if defined(__HAVE_ARCH_CMPXCHG)
+ while (1) {
+ old = p->ip_id_count;
+ if (cmpxchg(&p->ip_id_count, old, old + more) == old)
+ break;
+ }
+#else
+ old = atomic_add_return(more, &p->ip_id_count) - more;
+#endif
+ return old;
}
+static inline void inet_id_set(struct inet_peer *p, int val)
+{
+#if defined(__HAVE_ARCH_CMPXCHG)
+ p->ip_id_count = val;
+#else
+ atomic_set(&p->ip_id_count, val);
+#endif
+}
+
+static inline __u16 inet_id_read(const struct inet_peer *p)
+{
+#if defined(__HAVE_ARCH_CMPXCHG)
+ return p->ip_id_count;
+#else
+ return atomic_read(&p->ip_id_count);
+#endif
+}
+
+
+
#endif /* _NET_INETPEER_H */
diff --git a/net/ipv4/inetpeer.c b/net/ipv4/inetpeer.c
index b1fbe18..3641831 100644
--- a/net/ipv4/inetpeer.c
+++ b/net/ipv4/inetpeer.c
@@ -67,9 +67,6 @@
* ip_id_count: idlock
*/
-/* Exported for inet_getid inline function. */
-DEFINE_SPINLOCK(inet_peer_idlock);
-
static struct kmem_cache *peer_cachep __read_mostly;
#define node_height(x) x->avl_height
@@ -390,7 +387,7 @@ struct inet_peer *inet_getpeer(__be32 daddr, int create)
n->v4daddr = daddr;
atomic_set(&n->refcnt, 1);
atomic_set(&n->rid, 0);
- n->ip_id_count = secure_ip_id(daddr);
+ inet_id_set(n, secure_ip_id(daddr));
n->tcp_ts_stamp = 0;
write_lock_bh(&peer_pool_lock);
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index df93473..a090fcf 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2855,7 +2855,7 @@ static int rt_fill_info(struct net *net,
error = rt->u.dst.error;
expires = rt->u.dst.expires ? rt->u.dst.expires - jiffies : 0;
if (rt->peer) {
- id = rt->peer->ip_id_count;
+ id = inet_id_read(rt->peer);
if (rt->peer->tcp_ts_stamp) {
ts = rt->peer->tcp_ts;
tsage = get_seconds() - rt->peer->tcp_ts_stamp;
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists