Patchwork inet_peer: Optimize inet_getid()

login
register
mail settings
Submitter Eric Dumazet
Date Sept. 24, 2009, 7:04 p.m.
Message ID <4ABBC2D8.2040901@gmail.com>
Download mbox | patch
Permalink /patch/34229/
State Superseded
Delegated to: David Miller
Headers show

Comments

Eric Dumazet - Sept. 24, 2009, 7:04 p.m.
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@gmail.com>
---
 include/net/inetpeer.h |   16 ++++++++--------
 net/ipv4/inetpeer.c    |    3 ---
 2 files changed, 8 insertions(+), 11 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
stephen hemminger - Sept. 24, 2009, 7:30 p.m.
On Thu, 24 Sep 2009 21:04:56 +0200
Eric Dumazet <eric.dumazet@gmail.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@gmail.com>

I thought cmpxchg was not available on all architectures.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Dumazet - Sept. 24, 2009, 7:57 p.m.
Stephen Hemminger a écrit :
> On Thu, 24 Sep 2009 21:04:56 +0200
> Eric Dumazet <eric.dumazet@gmail.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@gmail.com>
> 
> I thought cmpxchg was not available on all architectures.

Good point Stephen, I forgot about non x86 arches ;)

I'll send an update with cmpxchg() if available, and atomic_t as fallback.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/include/net/inetpeer.h b/include/net/inetpeer.h
index 15e1f8f..952f0ad 100644
--- a/include/net/inetpeer.h
+++ b/include/net/inetpeer.h
@@ -37,17 +37,17 @@  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;
-
-	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;
+	__u16 old;
+
+	while (1) {
+		old = p->ip_id_count;
+		if (cmpxchg(&p->ip_id_count, old, old + 1 + more) == old)
+			break;
+	}
+	return old;
 }
 
 #endif /* _NET_INETPEER_H */
diff --git a/net/ipv4/inetpeer.c b/net/ipv4/inetpeer.c
index b1fbe18..5dc29b8 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