Thread (9 messages) flat view 9 messages, 3 authors, 2012-04-24

Re: [PATCH net-next] tcp: introduce tcp_try_coalesce

From: David Miller <davem@davemloft.net>
Date: 2012-04-24 02:46:15

From: Neal Cardwell <ncardwell@google.com>
Date: Mon, 23 Apr 2012 22:39:10 -0400
On Mon, Apr 23, 2012 at 1:11 PM, Eric Dumazet [off-list ref] wrote:
quoted
From: Eric Dumazet <edumazet@google.com>

commit c8628155ece3 (tcp: reduce out_of_order memory use) took care of
coalescing tcp segments provided by legacy devices (linear skbs)

We extend this idea to fragged skbs, as their truesize can be heavy.

ixgbe for example uses 256+1024+PAGE_SIZE/2 = 3328 bytes per segment.

Use this coalescing strategy for receive queue too.

This contributes to reduce number of tcp collapses, at minimal cost, and
reduces memory overhead and packets drops.
Acked-by: Neal Cardwell <ncardwell@google.com>

Thanks for the background info, Eric.
Applied, thanks Eric.

Although I'd like to ask you to clean up tcp_try_coalesce() a bit.

It effectively returns a boolean, but you've clouded this up by
returning an int and defining it in the comment to return "> 0" or
not.

Just make it return a real bool.

I know why you did this, it makes the "eaten" code somewhat simpler in
tcp_data_queue(), but overall it's more confusing how it is now.

People look at how the tcp_try_coalesce() return value is interpreted
and say "in what cases can it return a negative value?"  We both know
it can't, but you have to read the entire function to figure that out.

And that's by definition not intuitive.

Thanks.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help