Uninline the __inet_hash function

PATCH Applied Pavel Emelyanov 3 participants 9 messages Dec 19, 2007 – Dec 20, 2007

Subsystems (3)

Networking Network drivers TCP

lore

Share & tools

lore

Pavel Emelyanov

author

2007-12-19 10:50 UTC #0 lore raw

PATCH 1/3 Uninline the __inet_hash function

This one is used in quite many places in the networking code and seems to big to be inline.

After the patch net/ipv4/build-in.o loses 725 bytes:
add/remove: 1/0 grow/shrink: 0/5 up/down: 374/-1099 (-725)
function old new delta
__inet_hash - 374 +374
tcp_sacktag_write_queue 2255 2254 -1

__inet_lookup_listener                       284     274     -10
tcp_v4_syn_recv_sock                         755     495    -260
tcp_v4_hash                                  389      40    -349

inet_hash_connect 1165 686 -479

Exporting this is for dccp module.

Signed-off-by Pavel Emelyanov <[email protected]>

---

 include/net/inet_hashtables.h |   27 ++-------------------------
 net/ipv4/inet_hashtables.c    |   27 +++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 25 deletions(-)

include/net/inet_hashtables.h +2 ~~-25~~

index 37f6cb1..1a43125 100644
@@ -264,31 +264,8 @@ static inline void inet_listen_unlock(struct inet_hashinfo *hashinfo)
 		wake_up(&hashinfo->lhash_wait);
 }
 
-static inline void __inet_hash(struct inet_hashinfo *hashinfo,
-			       struct sock *sk, const int listen_possible)
-{
-	struct hlist_head *list;
-	rwlock_t *lock;
-
-	BUG_TRAP(sk_unhashed(sk));
-	if (listen_possible && sk->sk_state == TCP_LISTEN) {
-		list = &hashinfo->listening_hash[inet_sk_listen_hashfn(sk)];
-		lock = &hashinfo->lhash_lock;
-		inet_listen_wlock(hashinfo);
-	} else {
-		struct inet_ehash_bucket *head;
-		sk->sk_hash = inet_sk_ehashfn(sk);
-		head = inet_ehash_bucket(hashinfo, sk->sk_hash);
-		list = &head->chain;
-		lock = inet_ehash_lockp(hashinfo, sk->sk_hash);
-		write_lock(lock);
-	}
-	__sk_add_node(sk, list);
-	sock_prot_inc_use(sk->sk_prot);
-	write_unlock(lock);
-	if (listen_possible && sk->sk_state == TCP_LISTEN)
-		wake_up(&hashinfo->lhash_wait);
-}
+extern void __inet_hash(struct inet_hashinfo *hashinfo, struct sock *sk,
+		const int listen_possible);
 
 static inline void inet_hash(struct inet_hashinfo *hashinfo, struct sock *sk)
 {

net/ipv4/inet_hashtables.c +27 -0

index 67704da..46f899b 100644
@@ -267,6 +267,33 @@ static inline u32 inet_sk_port_offset(const struct sock *sk)
 					  inet->dport);
 }
 
+void __inet_hash(struct inet_hashinfo *hashinfo, struct sock *sk,
+		const int listen_possible)
+{
+	struct hlist_head *list;
+	rwlock_t *lock;
+
+	BUG_TRAP(sk_unhashed(sk));
+	if (listen_possible && sk->sk_state == TCP_LISTEN) {
+		list = &hashinfo->listening_hash[inet_sk_listen_hashfn(sk)];
+		lock = &hashinfo->lhash_lock;
+		inet_listen_wlock(hashinfo);
+	} else {
+		struct inet_ehash_bucket *head;
+		sk->sk_hash = inet_sk_ehashfn(sk);
+		head = inet_ehash_bucket(hashinfo, sk->sk_hash);
+		list = &head->chain;
+		lock = inet_ehash_lockp(hashinfo, sk->sk_hash);
+		write_lock(lock);
+	}
+	__sk_add_node(sk, list);
+	sock_prot_inc_use(sk->sk_prot);
+	write_unlock(lock);
+	if (listen_possible && sk->sk_state == TCP_LISTEN)
+		wake_up(&hashinfo->lhash_wait);
+}
+EXPORT_SYMBOL_GPL(__inet_hash);
+
 /*
  * Bind a port for a connect operation and hash it.
  */

Signature · 1 line

1.5.3.4

Eric Dumazet

2007-12-19 12:21 UTC #1 lore raw

Quoted 89 lines · Pavel Emelyanov

This one is used in quite many places in the networking code and seems to big to be inline.

__inet_lookup_listener                       284     274     -10
tcp_v4_syn_recv_sock                         755     495    -260
tcp_v4_hash                                  389      40    -349

inet_hash_connect 1165 686 -479

Exporting this is for dccp module.

Signed-off-by Pavel Emelyanov <[email protected]>

---

 include/net/inet_hashtables.h |   27 ++-------------------------
 net/ipv4/inet_hashtables.c    |   27 +++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 25 deletions(-)

include/net/inet_hashtables.h +2 ~~-25~~

index 37f6cb1..1a43125 100644
@@ -264,31 +264,8 @@ static inline void inet_listen_unlock(struct inet_hashinfo *hashinfo)
 		wake_up(&hashinfo->lhash_wait);
 }
 
-static inline void __inet_hash(struct inet_hashinfo *hashinfo,
-			       struct sock *sk, const int listen_possible)
-{
-	struct hlist_head *list;
-	rwlock_t *lock;
-
-	BUG_TRAP(sk_unhashed(sk));
-	if (listen_possible && sk->sk_state == TCP_LISTEN) {
-		list = &hashinfo->listening_hash[inet_sk_listen_hashfn(sk)];
-		lock = &hashinfo->lhash_lock;
-		inet_listen_wlock(hashinfo);
-	} else {
-		struct inet_ehash_bucket *head;
-		sk->sk_hash = inet_sk_ehashfn(sk);
-		head = inet_ehash_bucket(hashinfo, sk->sk_hash);
-		list = &head->chain;
-		lock = inet_ehash_lockp(hashinfo, sk->sk_hash);
-		write_lock(lock);
-	}
-	__sk_add_node(sk, list);
-	sock_prot_inc_use(sk->sk_prot);
-	write_unlock(lock);
-	if (listen_possible && sk->sk_state == TCP_LISTEN)
-		wake_up(&hashinfo->lhash_wait);
-}
+extern void __inet_hash(struct inet_hashinfo *hashinfo, struct sock *sk,
+		const int listen_possible);
 
 static inline void inet_hash(struct inet_hashinfo *hashinfo, struct sock *sk)
 {

net/ipv4/inet_hashtables.c +20 -0

index 67704da..46f899b 100644
@@ -267,6 +267,33 @@ static inline u32 inet_sk_port_offset(const struct sock *sk)
 					  inet->dport);
 }
 
+void __inet_hash(struct inet_hashinfo *hashinfo, struct sock *sk,
+		const int listen_possible)
+{
+	struct hlist_head *list;
+	rwlock_t *lock;
+
+	BUG_TRAP(sk_unhashed(sk));
+	if (listen_possible && sk->sk_state == TCP_LISTEN) {
+		list = &hashinfo->listening_hash[inet_sk_listen_hashfn(sk)];
+		lock = &hashinfo->lhash_lock;
+		inet_listen_wlock(hashinfo);
+	} else {
+		struct inet_ehash_bucket *head;
+		sk->sk_hash = inet_sk_ehashfn(sk);
+		head = inet_ehash_bucket(hashinfo, sk->sk_hash);
+		list = &head->chain;
+		lock = inet_ehash_lockp(hashinfo, sk->sk_hash);
+		write_lock(lock);
+	}
+	__sk_add_node(sk, list);

net/ipv4/inet_hashtables.c static inline u32 inet_sk_port_offset(const struct sock *sk) · line 299

Quoted 10 lines

net/ipv4/inet_hashtables.c +7 -0

+	sock_prot_inc_use(sk->sk_prot);
+	write_unlock(lock);
+	if (listen_possible && sk->sk_state == TCP_LISTEN)
+		wake_up(&hashinfo->lhash_wait);
+}
+EXPORT_SYMBOL_GPL(__inet_hash);
+
 /*
  * Bind a port for a connect operation and hash it.
  */

If you un-inline this (good idea), I am not sure we still need listen_possible argument.

It was usefull only to help compiler to zap dead code (since it was known at compile time), now it only adds some extra test and argument passing.

Thank you

Pavel Emelyanov

author

2007-12-19 13:23 UTC #2 lore raw

Quoted 104 lines · Eric Dumazet

Quoted 89 lines · Pavel Emelyanov

This one is used in quite many places in the networking code and seems to big to be inline.

__inet_lookup_listener                       284     274     -10
tcp_v4_syn_recv_sock                         755     495    -260
tcp_v4_hash                                  389      40    -349

inet_hash_connect 1165 686 -479

Exporting this is for dccp module.

Signed-off-by Pavel Emelyanov <[email protected]>

---

 include/net/inet_hashtables.h |   27 ++-------------------------
 net/ipv4/inet_hashtables.c    |   27 +++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 25 deletions(-)

include/net/inet_hashtables.h +2 ~~-25~~

index 37f6cb1..1a43125 100644
@@ -264,31 +264,8 @@ static inline void inet_listen_unlock(struct inet_hashinfo *hashinfo)
 		wake_up(&hashinfo->lhash_wait);
 }
 
-static inline void __inet_hash(struct inet_hashinfo *hashinfo,
-			       struct sock *sk, const int listen_possible)
-{
-	struct hlist_head *list;
-	rwlock_t *lock;
-
-	BUG_TRAP(sk_unhashed(sk));
-	if (listen_possible && sk->sk_state == TCP_LISTEN) {
-		list = &hashinfo->listening_hash[inet_sk_listen_hashfn(sk)];
-		lock = &hashinfo->lhash_lock;
-		inet_listen_wlock(hashinfo);
-	} else {
-		struct inet_ehash_bucket *head;
-		sk->sk_hash = inet_sk_ehashfn(sk);
-		head = inet_ehash_bucket(hashinfo, sk->sk_hash);
-		list = &head->chain;
-		lock = inet_ehash_lockp(hashinfo, sk->sk_hash);
-		write_lock(lock);
-	}
-	__sk_add_node(sk, list);
-	sock_prot_inc_use(sk->sk_prot);
-	write_unlock(lock);
-	if (listen_possible && sk->sk_state == TCP_LISTEN)
-		wake_up(&hashinfo->lhash_wait);
-}
+extern void __inet_hash(struct inet_hashinfo *hashinfo, struct sock *sk,
+		const int listen_possible);
 
 static inline void inet_hash(struct inet_hashinfo *hashinfo, struct sock *sk)
 {

net/ipv4/inet_hashtables.c +20 -0

index 67704da..46f899b 100644
@@ -267,6 +267,33 @@ static inline u32 inet_sk_port_offset(const struct sock *sk)
 					  inet->dport);
 }
 
+void __inet_hash(struct inet_hashinfo *hashinfo, struct sock *sk,
+		const int listen_possible)
+{
+	struct hlist_head *list;
+	rwlock_t *lock;
+
+	BUG_TRAP(sk_unhashed(sk));
+	if (listen_possible && sk->sk_state == TCP_LISTEN) {
+		list = &hashinfo->listening_hash[inet_sk_listen_hashfn(sk)];
+		lock = &hashinfo->lhash_lock;
+		inet_listen_wlock(hashinfo);
+	} else {
+		struct inet_ehash_bucket *head;
+		sk->sk_hash = inet_sk_ehashfn(sk);
+		head = inet_ehash_bucket(hashinfo, sk->sk_hash);
+		list = &head->chain;
+		lock = inet_ehash_lockp(hashinfo, sk->sk_hash);
+		write_lock(lock);
+	}
+	__sk_add_node(sk, list);

net/ipv4/inet_hashtables.c static inline u32 inet_sk_port_offset(const struct sock *sk) · line 299

Quoted 10 lines

net/ipv4/inet_hashtables.c +7 -0

+	sock_prot_inc_use(sk->sk_prot);
+	write_unlock(lock);
+	if (listen_possible && sk->sk_state == TCP_LISTEN)
+		wake_up(&hashinfo->lhash_wait);
+}
+EXPORT_SYMBOL_GPL(__inet_hash);
+
 /*
  * Bind a port for a connect operation and hash it.
  */

If you un-inline this (good idea), I am not sure we still need listen_possible argument.

Quoted 2 lines

It was usefull only to help compiler to zap dead code (since it was known at compile time), now it only adds some extra test and argument passing.

Hm... I've tried to address this issue and got worse result - minus 600 bytes (vs minus 725). So, what would be more preferable - get a smaller code with one extra 'if' or get a bit larger code without it?

Quoted 1 line

Thank you

Thanks,
Pavel

Eric Dumazet

2007-12-19 16:09 UTC #3 lore raw

Quoted 103 lines · Pavel Emelyanov

Quoted 102 lines · Eric Dumazet

Quoted 89 lines · Pavel Emelyanov

This one is used in quite many places in the networking code and seems to big to be inline.

__inet_lookup_listener                       284     274     -10
tcp_v4_syn_recv_sock                         755     495    -260
tcp_v4_hash                                  389      40    -349

inet_hash_connect 1165 686 -479

Exporting this is for dccp module.

Signed-off-by Pavel Emelyanov <[email protected]>

---

 include/net/inet_hashtables.h |   27 ++-------------------------
 net/ipv4/inet_hashtables.c    |   27 +++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 25 deletions(-)

include/net/inet_hashtables.h +2 ~~-25~~

index 37f6cb1..1a43125 100644
@@ -264,31 +264,8 @@ static inline void inet_listen_unlock(struct inet_hashinfo *hashinfo)
 		wake_up(&hashinfo->lhash_wait);
 }
 
-static inline void __inet_hash(struct inet_hashinfo *hashinfo,
-			       struct sock *sk, const int listen_possible)
-{
-	struct hlist_head *list;
-	rwlock_t *lock;
-
-	BUG_TRAP(sk_unhashed(sk));
-	if (listen_possible && sk->sk_state == TCP_LISTEN) {
-		list = &hashinfo->listening_hash[inet_sk_listen_hashfn(sk)];
-		lock = &hashinfo->lhash_lock;
-		inet_listen_wlock(hashinfo);
-	} else {
-		struct inet_ehash_bucket *head;
-		sk->sk_hash = inet_sk_ehashfn(sk);
-		head = inet_ehash_bucket(hashinfo, sk->sk_hash);
-		list = &head->chain;
-		lock = inet_ehash_lockp(hashinfo, sk->sk_hash);
-		write_lock(lock);
-	}
-	__sk_add_node(sk, list);
-	sock_prot_inc_use(sk->sk_prot);
-	write_unlock(lock);
-	if (listen_possible && sk->sk_state == TCP_LISTEN)
-		wake_up(&hashinfo->lhash_wait);
-}
+extern void __inet_hash(struct inet_hashinfo *hashinfo, struct sock *sk,
+		const int listen_possible);
 
 static inline void inet_hash(struct inet_hashinfo *hashinfo, struct sock *sk)
 {

net/ipv4/inet_hashtables.c +20 -0

index 67704da..46f899b 100644
@@ -267,6 +267,33 @@ static inline u32 inet_sk_port_offset(const struct sock *sk)
 					  inet->dport);
 }
 
+void __inet_hash(struct inet_hashinfo *hashinfo, struct sock *sk,
+		const int listen_possible)
+{
+	struct hlist_head *list;
+	rwlock_t *lock;
+
+	BUG_TRAP(sk_unhashed(sk));
+	if (listen_possible && sk->sk_state == TCP_LISTEN) {
+		list = &hashinfo->listening_hash[inet_sk_listen_hashfn(sk)];
+		lock = &hashinfo->lhash_lock;
+		inet_listen_wlock(hashinfo);
+	} else {
+		struct inet_ehash_bucket *head;
+		sk->sk_hash = inet_sk_ehashfn(sk);
+		head = inet_ehash_bucket(hashinfo, sk->sk_hash);
+		list = &head->chain;
+		lock = inet_ehash_lockp(hashinfo, sk->sk_hash);
+		write_lock(lock);
+	}
+	__sk_add_node(sk, list);

net/ipv4/inet_hashtables.c static inline u32 inet_sk_port_offset(const struct sock *sk) · line 299

Quoted 10 lines

net/ipv4/inet_hashtables.c +7 -0

+	sock_prot_inc_use(sk->sk_prot);
+	write_unlock(lock);
+	if (listen_possible && sk->sk_state == TCP_LISTEN)
+		wake_up(&hashinfo->lhash_wait);
+}
+EXPORT_SYMBOL_GPL(__inet_hash);
+
 /*
  * Bind a port for a connect operation and hash it.
  */

If you un-inline this (good idea), I am not sure we still need listen_possible argument.

Quoted 5 lines

Quoted 1 line
It was usefull only to help compiler to zap dead code (since it was known at compile time), now it only adds some extra test and argument passing.
Hm... I've tried to address this issue and got worse result - minus
600 bytes (vs minus 725). So, what would be more preferable - get a smaller code with one extra 'if' or get a bit larger code without it?

Strange... What I meant is always assume listen_possible is true.

The if (sk->sk_state == TCP_LISTEN) will finally see the truth.

I did a test here on x86 gcc-4.2.2 and saved 32 bytes.

Pavel Emelyanov

author

2007-12-19 17:07 UTC #4 lore raw

Quoted 105 lines · Eric Dumazet

Quoted 104 lines · Pavel Emelyanov

Quoted 103 lines · Eric Dumazet

Quoted 89 lines · Pavel Emelyanov

This one is used in quite many places in the networking code and seems to big to be inline.

__inet_lookup_listener                       284     274     -10
tcp_v4_syn_recv_sock                         755     495    -260
tcp_v4_hash                                  389      40    -349

inet_hash_connect 1165 686 -479

Exporting this is for dccp module.

Signed-off-by Pavel Emelyanov <[email protected]>

---

 include/net/inet_hashtables.h |   27 ++-------------------------
 net/ipv4/inet_hashtables.c    |   27 +++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 25 deletions(-)

include/net/inet_hashtables.h +2 ~~-25~~

index 37f6cb1..1a43125 100644
@@ -264,31 +264,8 @@ static inline void inet_listen_unlock(struct inet_hashinfo *hashinfo)
 		wake_up(&hashinfo->lhash_wait);
 }
 
-static inline void __inet_hash(struct inet_hashinfo *hashinfo,
-			       struct sock *sk, const int listen_possible)
-{
-	struct hlist_head *list;
-	rwlock_t *lock;
-
-	BUG_TRAP(sk_unhashed(sk));
-	if (listen_possible && sk->sk_state == TCP_LISTEN) {
-		list = &hashinfo->listening_hash[inet_sk_listen_hashfn(sk)];
-		lock = &hashinfo->lhash_lock;
-		inet_listen_wlock(hashinfo);
-	} else {
-		struct inet_ehash_bucket *head;
-		sk->sk_hash = inet_sk_ehashfn(sk);
-		head = inet_ehash_bucket(hashinfo, sk->sk_hash);
-		list = &head->chain;
-		lock = inet_ehash_lockp(hashinfo, sk->sk_hash);
-		write_lock(lock);
-	}
-	__sk_add_node(sk, list);
-	sock_prot_inc_use(sk->sk_prot);
-	write_unlock(lock);
-	if (listen_possible && sk->sk_state == TCP_LISTEN)
-		wake_up(&hashinfo->lhash_wait);
-}
+extern void __inet_hash(struct inet_hashinfo *hashinfo, struct sock *sk,
+		const int listen_possible);
 
 static inline void inet_hash(struct inet_hashinfo *hashinfo, struct sock *sk)
 {

net/ipv4/inet_hashtables.c +20 -0

index 67704da..46f899b 100644
@@ -267,6 +267,33 @@ static inline u32 inet_sk_port_offset(const struct sock *sk)
 					  inet->dport);
 }
 
+void __inet_hash(struct inet_hashinfo *hashinfo, struct sock *sk,
+		const int listen_possible)
+{
+	struct hlist_head *list;
+	rwlock_t *lock;
+
+	BUG_TRAP(sk_unhashed(sk));
+	if (listen_possible && sk->sk_state == TCP_LISTEN) {
+		list = &hashinfo->listening_hash[inet_sk_listen_hashfn(sk)];
+		lock = &hashinfo->lhash_lock;
+		inet_listen_wlock(hashinfo);
+	} else {
+		struct inet_ehash_bucket *head;
+		sk->sk_hash = inet_sk_ehashfn(sk);
+		head = inet_ehash_bucket(hashinfo, sk->sk_hash);
+		list = &head->chain;
+		lock = inet_ehash_lockp(hashinfo, sk->sk_hash);
+		write_lock(lock);
+	}
+	__sk_add_node(sk, list);

net/ipv4/inet_hashtables.c static inline u32 inet_sk_port_offset(const struct sock *sk) · line 299

Quoted 10 lines

net/ipv4/inet_hashtables.c +7 -0

+	sock_prot_inc_use(sk->sk_prot);
+	write_unlock(lock);
+	if (listen_possible && sk->sk_state == TCP_LISTEN)
+		wake_up(&hashinfo->lhash_wait);
+}
+EXPORT_SYMBOL_GPL(__inet_hash);
+
 /*
  * Bind a port for a connect operation and hash it.
  */

If you un-inline this (good idea), I am not sure we still need listen_possible argument.

Quoted 8 lines

Quoted 6 lines
Quoted 2 lines
It was usefull only to help compiler to zap dead code (since it was known at compile time), now it only adds some extra test and argument passing.
Hm... I've tried to address this issue and got worse result - minus 600 bytes (vs minus 725). So, what would be more preferable - get a smaller code with one extra 'if' or get a bit larger code without it?
Strange... What I meant is always assume listen_possible is true.

That's not truth, if I get you right. The __inet_hash() is called with 0, from all the places except for the inet_hash() one.

Quoted 6 lines

The if (sk->sk_state == TCP_LISTEN) will finally see the truth.
I did a test here on x86 gcc-4.2.2 and saved 32 bytes.

Thanks,
Pavel

Eric Dumazet

2007-12-19 17:15 UTC #5 lore raw

Quoted 5 lines · Pavel Emelyanov

Quoted 4 lines · Eric Dumazet
Quoted 3 lines · Pavel Emelyanov
Quoted 2 lines · Eric Dumazet
If you un-inline this (good idea), I am not sure we still need listen_possible argument.

Quoted 8 lines

Quoted 5 lines
Quoted 4 lines
Quoted 1 line
It was usefull only to help compiler to zap dead code (since it was known at compile time), now it only adds some extra test and argument passing.
Hm... I've tried to address this issue and got worse result - minus
600 bytes (vs minus 725). So, what would be more preferable - get a smaller code with one extra 'if' or get a bit larger code without it?
Strange... What I meant is always assume listen_possible is true.
That's not truth, if I get you right. The __inet_hash() is called with 0, from all the places except for the inet_hash() one.

OK, but on cases with 0, sk->sk_state is != TCP_LISTEN, unless I am mistaken.

Quoted 12 lines

Quoted 7 lines
The if (sk->sk_state == TCP_LISTEN) will finally see the truth.
I did a test here on x86 gcc-4.2.2 and saved 32 bytes.
Thanks,
Pavel

David Miller

maintainer

2007-12-20 08:30 UTC #6 lore raw

From: Eric Dumazet <[email protected]>
Date: Wed, 19 Dec 2007 18:15:20 +0100

Quoted 5 lines

Quoted 2 lines · Pavel Emelyanov
That's not truth, if I get you right. The __inet_hash() is called with 0, from all the places except for the inet_hash() one.
OK, but on cases with 0, sk->sk_state is != TCP_LISTEN, unless I am mistaken.

This is true.

Pavel Emelyanov

author

2007-12-20 09:46 UTC #7 lore raw

PATCH 1/3 Uninline the __inet_hash function

This one is used in quite many places in the networking code and seems to big to be inline.

After the patch net/ipv4/build-in.o loses ~650 bytes:
add/remove: 2/0 grow/shrink: 0/5 up/down: 461/-1114 (-653)

function                                     old     new   delta
__inet_hash_nolisten                           -     282    +282
__inet_hash                                    -     179    +179
tcp_sacktag_write_queue                     2255    2254      -1
__inet_lookup_listener                       284     274     -10
tcp_v4_syn_recv_sock                         755     493    -262
tcp_v4_hash                                  389      35    -354

inet_hash_connect 1086 599 -487

This version addresses the issue pointed by Eric, that while being inline this function was optimized by gcc in respect to the 'listen_possible' argument.

(Patches 2 and 3 in this series are still applied after this)

Signed-off-by Pavel Emelyanov <[email protected]>

---

include/net/inet_hashtables.h +3 ~~-26~~

index fef4442..65ddb25 100644
@@ -264,37 +264,14 @@ static inline void inet_listen_unlock(struct inet_hashinfo *hashinfo)
 		wake_up(&hashinfo->lhash_wait);
 }
 
-static inline void __inet_hash(struct inet_hashinfo *hashinfo,
-			       struct sock *sk, const int listen_possible)
-{
-	struct hlist_head *list;
-	rwlock_t *lock;
-
-	BUG_TRAP(sk_unhashed(sk));
-	if (listen_possible && sk->sk_state == TCP_LISTEN) {
-		list = &hashinfo->listening_hash[inet_sk_listen_hashfn(sk)];
-		lock = &hashinfo->lhash_lock;
-		inet_listen_wlock(hashinfo);
-	} else {
-		struct inet_ehash_bucket *head;
-		sk->sk_hash = inet_sk_ehashfn(sk);
-		head = inet_ehash_bucket(hashinfo, sk->sk_hash);
-		list = &head->chain;
-		lock = inet_ehash_lockp(hashinfo, sk->sk_hash);
-		write_lock(lock);
-	}
-	__sk_add_node(sk, list);
-	sock_prot_inc_use(sk->sk_prot);
-	write_unlock(lock);
-	if (listen_possible && sk->sk_state == TCP_LISTEN)
-		wake_up(&hashinfo->lhash_wait);
-}
+extern void __inet_hash(struct inet_hashinfo *hashinfo, struct sock *sk);
+extern void __inet_hash_nolisten(struct inet_hashinfo *hinfo, struct sock *sk);
 
 static inline void inet_hash(struct inet_hashinfo *hashinfo, struct sock *sk)
 {
 	if (sk->sk_state != TCP_CLOSE) {
 		local_bh_disable();
-		__inet_hash(hashinfo, sk, 1);
+		__inet_hash(hashinfo, sk);
 		local_bh_enable();
 	}
 }

net/dccp/ipv4.c +1 -1

index 02fc91c..f450df2 100644
@@ -408,7 +408,7 @@ struct sock *dccp_v4_request_recv_sock(struct sock *sk, struct sk_buff *skb,
 
 	dccp_sync_mss(newsk, dst_mtu(dst));
 
-	__inet_hash(&dccp_hashinfo, newsk, 0);
+	__inet_hash_nolisten(&dccp_hashinfo, newsk);
 	__inet_inherit_port(&dccp_hashinfo, sk, newsk);
 
 	return newsk;

net/ipv4/inet_hashtables.c +44 -2

64 lines not on this page · raw message

net/ipv4/tcp_ipv4.c +1 -1

index 652c323..fc9bdd8 100644
@@ -1478,7 +1478,7 @@ struct sock *tcp_v4_syn_recv_sock(struct sock *sk, struct sk_buff *skb,
 	}
 #endif
 
-	__inet_hash(&tcp_hashinfo, newsk, 0);
+	__inet_hash_nolisten(&tcp_hashinfo, newsk);
 	__inet_inherit_port(&tcp_hashinfo, sk, newsk);
 
 	return newsk;

David Miller

maintainer Applied

2007-12-20 23:31 UTC #8 lore raw

From: Pavel Emelyanov <[email protected]>
Date: Thu, 20 Dec 2007 12:46:07 +0300

Quoted 14 lines

This one is used in quite many places in the networking code and seems to big to be inline.
After the patch net/ipv4/build-in.o loses ~650 bytes:
add/remove: 2/0 grow/shrink: 0/5 up/down: 461/-1114 (-653)
function                                     old     new   delta
__inet_hash_nolisten                           -     282    +282
__inet_hash                                    -     179    +179
tcp_sacktag_write_queue                     2255    2254      -1
__inet_lookup_listener                       284     274     -10
tcp_v4_syn_recv_sock                         755     493    -262
tcp_v4_hash                                  389      35    -354
inet_hash_connect 1086 599 -487

Quoted 7 lines

This version addresses the issue pointed by Eric, that while being inline this function was optimized by gcc in respect to the 'listen_possible' argument.
(Patches 2 and 3 in this series are still applied after this)
Signed-off-by Pavel Emelyanov <[email protected]>

Applied.