Skip to contentSkip to conversation

performance problem with established hash

PATCH In review Anton Blanchard 3 participants 3 messages Feb 28, 2004

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 performance problem with established hash

Hi,

An HTTP stress test on latest 2.6 BK shows up a problem in tcp_r4_rcv:

15.0125  tcp_v4_rcv
 8.2719  e1000_xmit_frame
 3.2153  qdisc_restart
 3.1675  skb_release_data

15% in tcp_v4_rcv, ouch. It turns out almost all of this is walking the established hash looking for stuff in time wait:

        /* Must check for a TIME_WAIT'er before going to listener hash. */
        sk_for_each(sk, node, &(head + tcp_ehash_size)->chain) {
                if (TCP_IPV4_TW_MATCH(sk, acookie, saddr, daddr, ports, dif))
                        goto hit;
        }

Looking at the logic for sizing the established hash, we clamp at 10 pages. That doesnt sound right:

        if (num_physpages >= (128 * 1024))
                goal = num_physpages >> (21 - PAGE_SHIFT);
        else
                goal = num_physpages >> (23 - PAGE_SHIFT);

        if (!thash_entries)
                goal = min(10UL, goal);
        else
                goal = (thash_entries * sizeof(struct tcp_ehash_bucket)) >> PAGE_SHIFT;

Sure enough things arent happy:

Older 2.6 kernel:
TCP: Hash tables configured (established 262144 bind 65536)

Current 2.6 BK:
TCP: Hash tables configured (established 4096 bind 4096)

It looks like the hash clamping patches wanted to clamp at 1 << 10, not
10. Patch below.

Anton

---

 foobar2-anton/net/ipv4/tcp.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)
net/ipv4/tcp.c renamed from net/ipv4/tcp.c~ehashfix +1 -1
diff -puN net/ipv4/tcp.c~ehashfix net/ipv4/tcp.c
@@ -2622,7 +2622,7 @@ void __init tcp_init(void)
 		goal = num_physpages >> (23 - PAGE_SHIFT);
 
 	if (!thash_entries)
-		goal = min(10UL, goal);
+		goal = min(1UL << 10, goal);
 	else
 		goal = (thash_entries * sizeof(struct tcp_ehash_bucket)) >> PAGE_SHIFT;
 	for (order = 0; (1UL << order) < goal; order++)
#1 lore raw

Review

Quoted 3 lines · Anton Blanchard
(quoted diff) +0 -1
-		goal = min(10UL, goal);
 +		goal = min(1UL << 10, goal);

oops. Better fix the route cache too.

I'm not sure what went wrong in there, sorry for letting that slip through. Obviously, doing

	if (a)
		foo = bar;
	else
		foo = zot;

	if (b)
		foo = rab;
	else
		foo = toz;

does not make a ton of sense.

This should fix it up. We keep the table sizing identical to that which we had in 2.6.earlier, with a boot option override.

 net/ipv4/route.c |    4 +---
 net/ipv4/tcp.c   |    4 +---
 2 files changed, 2 insertions(+), 6 deletions(-)
net/ipv4/route.c renamed from net/ipv4/route.c~ip_rt_init-sizing-fix +1 -3
diff -puN net/ipv4/route.c~ip_rt_init-sizing-fix net/ipv4/route.c
@@ -2753,9 +2753,7 @@ int __init ip_rt_init(void)
 		panic("IP: failed to allocate ip_dst_cache\n");
 
 	goal = num_physpages >> (26 - PAGE_SHIFT);
-	if (!rhash_entries)
-		goal = min(10, goal);
-	else
+	if (rhash_entries)
 		goal = (rhash_entries * sizeof(struct rt_hash_bucket)) >> PAGE_SHIFT;
 	for (order = 0; (1UL << order) < goal; order++)
 		/* NOTHING */;
net/ipv4/tcp.c renamed from net/ipv4/tcp.c~ip_rt_init-sizing-fix +1 -3
diff -puN net/ipv4/tcp.c~ip_rt_init-sizing-fix net/ipv4/tcp.c
@@ -2621,9 +2621,7 @@ void __init tcp_init(void)
 	else
 		goal = num_physpages >> (23 - PAGE_SHIFT);
 
-	if (!thash_entries)
-		goal = min(10UL, goal);
-	else
+	if (thash_entries)
 		goal = (thash_entries * sizeof(struct tcp_ehash_bucket)) >> PAGE_SHIFT;
 	for (order = 0; (1UL << order) < goal; order++)
 		;

_

#2 lore raw
Quoted 2 lines · On Fri, 27 Feb 2004 19:55:48 -0800 Andrew Morton <[email protected]> wrote:

This should fix it up. We keep the table sizing identical to that which we had in 2.6.earlier, with a boot option override.

Andrew, just push this to Linus now if you haven't already :)