[PATCH] tcp: tcp_prequeue() cleanup

[PATCH] tcp: tcp_prequeue() cleanup

[Eric Dumazet]

Small cleanup patch to reduce line lengths, before a change in tcp_prequeue().

Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>

diff --git a/include/net/tcp.h b/include/net/tcp.h
index b55b489..ac37228 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -890,30 +890,31 @@ static inline int tcp_prequeue(struct sock *sk, struct sk_buff *skb)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 
-	if (!sysctl_tcp_low_latency && tp->ucopy.task) {
-		__skb_queue_tail(&tp->ucopy.prequeue, skb);
-		tp->ucopy.memory += skb->truesize;
-		if (tp->ucopy.memory > sk->sk_rcvbuf) {
-			struct sk_buff *skb1;
-
-			BUG_ON(sock_owned_by_user(sk));
-
-			while ((skb1 = __skb_dequeue(&tp->ucopy.prequeue)) != NULL) {
-				sk_backlog_rcv(sk, skb1);
-				NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_TCPPREQUEUEDROPPED);
-			}
-
-			tp->ucopy.memory = 0;
-		} else if (skb_queue_len(&tp->ucopy.prequeue) == 1) {
-			wake_up_interruptible(sk->sk_sleep);
-			if (!inet_csk_ack_scheduled(sk))
-				inet_csk_reset_xmit_timer(sk, ICSK_TIME_DACK,
-						          (3 * TCP_RTO_MIN) / 4,
-							  TCP_RTO_MAX);
+	if (sysctl_tcp_low_latency || !tp->ucopy.task)
+		return 0;
+
+	__skb_queue_tail(&tp->ucopy.prequeue, skb);
+	tp->ucopy.memory += skb->truesize;
+	if (tp->ucopy.memory > sk->sk_rcvbuf) {
+		struct sk_buff *skb1;
+
+		BUG_ON(sock_owned_by_user(sk));
+
+		while ((skb1 = __skb_dequeue(&tp->ucopy.prequeue)) != NULL) {
+			sk_backlog_rcv(sk, skb1);
+			NET_INC_STATS_BH(sock_net(sk),
+					 LINUX_MIB_TCPPREQUEUEDROPPED);
 		}
-		return 1;
+
+		tp->ucopy.memory = 0;
+	} else if (skb_queue_len(&tp->ucopy.prequeue) == 1) {
+		wake_up_interruptible(sk->sk_sleep);
+		if (!inet_csk_ack_scheduled(sk))
+			inet_csk_reset_xmit_timer(sk, ICSK_TIME_DACK,
+						  (3 * TCP_RTO_MIN) / 4,
+						  TCP_RTO_MAX);
 	}
-	return 0;
+	return 1;
 }
 
 

[PATCH] tcp: tcp_prequeue() can use keyed wakeups

[Eric Dumazet]

We can avoid waking up tasks not interested in receive notifications,
using wake_up_interruptible_poll() instead of wake_up_interruptible()

Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>


diff --git a/include/net/tcp.h b/include/net/tcp.h
index ac37228..87d210b 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -908,7 +908,8 @@ static inline int tcp_prequeue(struct sock *sk, struct sk_buff *skb)
 
 		tp->ucopy.memory = 0;
 	} else if (skb_queue_len(&tp->ucopy.prequeue) == 1) {
-		wake_up_interruptible(sk->sk_sleep);
+		wake_up_interruptible_poll(sk->sk_sleep,
+					   POLLIN | POLLRDNORM | POLLRDBAND);
 		if (!inet_csk_ack_scheduled(sk))
 			inet_csk_reset_xmit_timer(sk, ICSK_TIME_DACK,
 						  (3 * TCP_RTO_MIN) / 4,

Re: [PATCH] tcp: tcp_prequeue() cleanup

[David Miller]

From: Eric Dumazet <dada1@cosmosbay.com>
Date: Thu, 07 May 2009 19:08:38 +0200

> Small cleanup patch to reduce line lengths, before a change in tcp_prequeue().
> 
> Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>

Applied.

Re: [PATCH] tcp: tcp_prequeue() can use keyed wakeups

[David Miller]

From: Eric Dumazet <dada1@cosmosbay.com>
Date: Thu, 07 May 2009 19:20:39 +0200

> We can avoid waking up tasks not interested in receive notifications,
> using wake_up_interruptible_poll() instead of wake_up_interruptible()
> 
> Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>

Nice work Eric, applied!

Re: [PATCH] tcp: tcp_prequeue() can use keyed wakeups

[John Dykstra]

On Thu, 2009-05-07 at 19:20 +0200, Eric Dumazet wrote:
> We can avoid waking up tasks not interested in receive notifications,
> using wake_up_interruptible_poll() instead of wake_up_interruptible()

Is there any reason why we shouldn't do the same thing on the write
side?

Compile-tested only.

  --  John

---
[PATCH net-next-2.6] tcp: Don't wake up reading threads on write space 

When TCP frees up write buffer space, avoid waking up tasks that have
done a poll() or select() on the same socket specifying read-side
events.

This is an extension of a read-side patch by Eric Dumazet.

Signed-off-by: John Dykstra <john.dykstra1@gmail.com>
---
 net/core/stream.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/core/stream.c b/net/core/stream.c
index 8727cea..a37debf 100644
--- a/net/core/stream.c
+++ b/net/core/stream.c
@@ -33,7 +33,8 @@ void sk_stream_write_space(struct sock *sk)
 		clear_bit(SOCK_NOSPACE, &sock->flags);
 
 		if (sk->sk_sleep && waitqueue_active(sk->sk_sleep))
-			wake_up_interruptible(sk->sk_sleep);
+			wake_up_interruptible_poll(sk->sk_sleep, POLLOUT |
+						POLLWRNORM | POLLWRBAND);
 		if (sock->fasync_list && !(sk->sk_shutdown & SEND_SHUTDOWN))
 			sock_wake_async(sock, SOCK_WAKE_SPACE, POLL_OUT);
 	}
-- 
1.5.4.3

Re: [PATCH] tcp: tcp_prequeue() can use keyed wakeups

[Eric Dumazet]

John Dykstra a écrit :
> On Thu, 2009-05-07 at 19:20 +0200, Eric Dumazet wrote:
>> We can avoid waking up tasks not interested in receive notifications,
>> using wake_up_interruptible_poll() instead of wake_up_interruptible()
> 
> Is there any reason why we shouldn't do the same thing on the write
> side?

No special reason, apart that this event is not so frequent.

(For tcp, sock_wfree() doesnt call ->sk_write_space())

I wonder if sk_stream_write_space() is actually called at all, I never got it in profiles :)

I see it is called from setsockopt(SO_SNFBUF),
and from tcp_new_space() only if SOCK_NOSPACE was set.

To see a benefit of this patch, we would need two threads, one blocked in a recv(),
one blocked in a send() on same socket (or a poll/epoll user with same bi-directional trafic)

Acked-by: Eric Dumazet <dada1@cosmosbay.com>

> 
> Compile-tested only.
> 
>   --  John
> 
> ---
> [PATCH net-next-2.6] tcp: Don't wake up reading threads on write space 
> 
> When TCP frees up write buffer space, avoid waking up tasks that have
> done a poll() or select() on the same socket specifying read-side
> events.
> 
> This is an extension of a read-side patch by Eric Dumazet.
> 
> Signed-off-by: John Dykstra <john.dykstra1@gmail.com>
> ---
>  net/core/stream.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/net/core/stream.c b/net/core/stream.c
> index 8727cea..a37debf 100644
> --- a/net/core/stream.c
> +++ b/net/core/stream.c
> @@ -33,7 +33,8 @@ void sk_stream_write_space(struct sock *sk)
>  		clear_bit(SOCK_NOSPACE, &sock->flags);
>  
>  		if (sk->sk_sleep && waitqueue_active(sk->sk_sleep))
> -			wake_up_interruptible(sk->sk_sleep);
> +			wake_up_interruptible_poll(sk->sk_sleep, POLLOUT |
> +						POLLWRNORM | POLLWRBAND);
>  		if (sock->fasync_list && !(sk->sk_shutdown & SEND_SHUTDOWN))
>  			sock_wake_async(sock, SOCK_WAKE_SPACE, POLL_OUT);
>  	}

Re: [PATCH] tcp: tcp_prequeue() can use keyed wakeups

[John Dykstra]

On Wed, 2009-05-13 at 05:28 +0200, Eric Dumazet wrote:

> To see a benefit of this patch, we would need two threads, one blocked
> in a recv(),
> one blocked in a send() on same socket (or a poll/epoll user with same
> bi-directional trafic)

Agreed.

  --  John

Re: [PATCH] tcp: tcp_prequeue() can use keyed wakeups

[David Miller]

From: John Dykstra <john.dykstra1@gmail.com>
Date: Wed, 13 May 2009 10:05:07 -0500

> On Wed, 2009-05-13 at 05:28 +0200, Eric Dumazet wrote:
> 
>> To see a benefit of this patch, we would need two threads, one blocked
>> in a recv(),
>> one blocked in a send() on same socket (or a poll/epoll user with same
>> bi-directional trafic)
> 
> Agreed.

I've applied this to net-next-2.6, thanks.