| Message ID | 20120717120358.16611.98190.sendpatchset@localhost.localdomain |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, 2012-07-17 at 17:33 +0530, Krishna Kumar wrote: > Do not throttle if sysctl_tcp_limit_output_bytes==0. > > Maybe it is better to throttle earlier in the loop, after > calling tcp_init_tso_segs(). > I wonder why, and why you put this question in a changelog instead of outside of it... Idea was to avoid setting TSQ_THROTTLED if we break out the loop. About disabling TSQ, my initial intent was to instead use a negative sysctl_tcp_limit_output_bytes value. Thats why I have in tcp_transmit_skb() : skb->destructor = (sysctl_tcp_limit_output_bytes > 0) ? tcp_wfree : sock_wfree; So I suggest you change the tcp_write_xmit(() test to a single unsigned compare : if (atomic_read(&sk->sk_wmem_alloc) >= (unsigned) sysctl_tcp_limit_output_bytes) { Also use : skb->destructor = (sysctl_tcp_limit_output_bytes >= 0) ? tcp_wfree : sock_wfree; and document the 'negative value disables TSQ' in Documentation/networking/ip-sysctl.txt -- 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 <eric.dumazet@gmail.com> wrote on 07/17/2012 06:40:54 PM: > > Do not throttle if sysctl_tcp_limit_output_bytes==0. > > > > Maybe it is better to throttle earlier in the loop, after > > calling tcp_init_tso_segs(). > > > > I wonder why, and why you put this question in a changelog instead of > outside of it... > > Idea was to avoid setting TSQ_THROTTLED if we break out the loop. The reason I mentioned it (in the wrong place) is because I thought this is a likely case and the checks before that might all pass only to get throttled. Some of the checks are quite lengthy. > About disabling TSQ, my initial intent was to instead use a negative > sysctl_tcp_limit_output_bytes value. > > Thats why I have in tcp_transmit_skb() : > > skb->destructor = (sysctl_tcp_limit_output_bytes > 0) ? > tcp_wfree : sock_wfree; > > So I suggest you change the tcp_write_xmit(() test to a single unsigned > compare : > > if (atomic_read(&sk->sk_wmem_alloc) >= > (unsigned) sysctl_tcp_limit_output_bytes) { > > Also use : > > skb->destructor = (sysctl_tcp_limit_output_bytes >= 0) ? > tcp_wfree : sock_wfree; > > and document the 'negative value disables TSQ' in > Documentation/networking/ip-sysctl.txt Sure, will post with this change. thanks, - KK -- 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
diff -ruNp org/net/ipv4/tcp_output.c new/net/ipv4/tcp_output.c --- org/net/ipv4/tcp_output.c 2012-07-17 09:56:12.000000000 +0530 +++ new/net/ipv4/tcp_output.c 2012-07-17 13:02:12.476111697 +0530 @@ -1948,7 +1948,8 @@ static bool tcp_write_xmit(struct sock * /* TSQ : sk_wmem_alloc accounts skb truesize, * including skb overhead. But thats OK. */ - if (atomic_read(&sk->sk_wmem_alloc) >= sysctl_tcp_limit_output_bytes) { + if (sysctl_tcp_limit_output_bytes > 0 && + atomic_read(&sk->sk_wmem_alloc) >= sysctl_tcp_limit_output_bytes) { set_bit(TSQ_THROTTLED, &tp->tsq_flags); break; }
Do not throttle if sysctl_tcp_limit_output_bytes==0. Maybe it is better to throttle earlier in the loop, after calling tcp_init_tso_segs(). Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com> --- tcp_output.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) -- 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