[PATCH] tcp: ensure epoll edge trigger wakeup when out of memory

[PATCH] tcp: ensure epoll edge trigger wakeup when out of memory

[Jason Baron]

From: Jason Baron <jbaron@akamai.com>

If we really can't get an skb allocated, make sure that we let
userspace know. This at least gives us a chance to re-try or abort.

Signed-off-by: Jason Baron <jbaron@akamai.com>
---
 net/ipv4/tcp.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index ecccfdc..d7d85e8 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -986,6 +986,9 @@ do_error:
 	if (copied)
 		goto out;
 out_err:
+	/* make sure we wake any epoll edge trigger waiter */
+	if (unlikely(sk->sk_write_queue == 0 && err == -EAGAIN))
+		sk->sk_write_space(sk);
 	return sk_stream_error(sk, flags, err);
 }
 
@@ -1274,6 +1277,9 @@ do_error:
 		goto out;
 out_err:
 	err = sk_stream_error(sk, flags, err);
+	/* make sure we wake any epoll edge trigger waiter */
+	if (unlikely(sk->sk_write_queue == 0 && err == -EAGAIN))
+		sk->sk_write_space(sk);
 	release_sock(sk);
 	return err;
 }
-- 
1.8.2.rc2

Re: [PATCH] tcp: ensure epoll edge trigger wakeup when out of memory

[Eric Dumazet]

On Fri, 2015-05-15 at 21:17 +0000, Jason Baron wrote:
> From: Jason Baron <jbaron@akamai.com>
> 
> If we really can't get an skb allocated, make sure that we let
> userspace know. This at least gives us a chance to re-try or abort.
> 
> Signed-off-by: Jason Baron <jbaron@akamai.com>
> ---
>  net/ipv4/tcp.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index ecccfdc..d7d85e8 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -986,6 +986,9 @@ do_error:
>  	if (copied)
>  		goto out;
>  out_err:
> +	/* make sure we wake any epoll edge trigger waiter */
> +	if (unlikely(sk->sk_write_queue == 0 && err == -EAGAIN))
> +		sk->sk_write_space(sk);
>  	return sk_stream_error(sk, flags, err);
>  }
>  
> @@ -1274,6 +1277,9 @@ do_error:
>  		goto out;
>  out_err:
>  	err = sk_stream_error(sk, flags, err);
> +	/* make sure we wake any epoll edge trigger waiter */
> +	if (unlikely(sk->sk_write_queue == 0 && err == -EAGAIN))
> +		sk->sk_write_space(sk);
>  	release_sock(sk);
>  	return err;
>  }

I wonder if we should return ENOMEM here, otherwise application has no
way to avoid to spin forever ?

Re: [PATCH] tcp: ensure epoll edge trigger wakeup when out of memory

[David Miller]

From: Jason Baron <jbaron@akamai.com>
Date: Fri, 15 May 2015 21:17:07 +0000 (GMT)

> From: Jason Baron <jbaron@akamai.com>
> 
> If we really can't get an skb allocated, make sure that we let
> userspace know. This at least gives us a chance to re-try or abort.
> 
> Signed-off-by: Jason Baron <jbaron@akamai.com>

Applied to net-next, thanks Jason.

Eric asked if we should perhaps return -ENOMEM here, and I'm not so
sure that's a good idea.  It is in a way a temporary failure that
should be retried.

Re: [PATCH] tcp: ensure epoll edge trigger wakeup when out of memory

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Tue, 19 May 2015 00:09:45 -0400 (EDT)

> From: Jason Baron <jbaron@akamai.com>
> Date: Fri, 15 May 2015 21:17:07 +0000 (GMT)
> 
>> From: Jason Baron <jbaron@akamai.com>
>> 
>> If we really can't get an skb allocated, make sure that we let
>> userspace know. This at least gives us a chance to re-try or abort.
>> 
>> Signed-off-by: Jason Baron <jbaron@akamai.com>
> 
> Applied to net-next, thanks Jason.

Nevermind, reverted, you didn't compile let alone test this:

net/ipv4/tcp.c: In function ‘do_tcp_sendpages’:
net/ipv4/tcp.c:1001:34: error: invalid operands to binary == (have ‘struct sk_buff_head’ and ‘int’)
  if (unlikely(sk->sk_write_queue == 0 && err == -EAGAIN))
                                  ^
include/linux/compiler.h:164:42: note: in definition of macro ‘unlikely’
 # define unlikely(x) __builtin_expect(!!(x), 0)
                                          ^
net/ipv4/tcp.c: In function ‘tcp_sendmsg’:
net/ipv4/tcp.c:1292:34: error: invalid operands to binary == (have ‘struct sk_buff_head’ and ‘int’)
  if (unlikely(sk->sk_write_queue == 0 && err == -EAGAIN))
                                  ^
include/linux/compiler.h:164:42: note: in definition of macro ‘unlikely’
 # define unlikely(x) __builtin_expect(!!(x), 0)
                                          ^

Re: [PATCH] tcp: ensure epoll edge trigger wakeup when out of memory

[Jason Baron]

On 05/19/2015 12:10 AM, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Tue, 19 May 2015 00:09:45 -0400 (EDT)
>
>> From: Jason Baron <jbaron@akamai.com>
>> Date: Fri, 15 May 2015 21:17:07 +0000 (GMT)
>>
>>> From: Jason Baron <jbaron@akamai.com>
>>>
>>> If we really can't get an skb allocated, make sure that we let
>>> userspace know. This at least gives us a chance to re-try or abort.
>>>
>>> Signed-off-by: Jason Baron <jbaron@akamai.com>
>> Applied to net-next, thanks Jason.
> Nevermind, reverted, you didn't compile let alone test this:
>

Hi David,

This was meant as: "do you think this is a good idea? If so, I will
go off and test it. I mentioned it was *not* tested in the separate
thread where we were discussing it:

http://marc.info/?l=linux-netdev&m=143172482802074&w=2

However, I didn't make that at all clear in *this* thread. And I'm
sorry for the inconvenience.

I meant to do:

+       if (unlikely(skb_queue_len(&sk->sk_write_queue) == 0 && err ==
-EAGAIN))
+               sk->sk_write_space(sk);

not:

+       if (unlikely(sk->sk_write_queue == 0 && err == -EAGAIN))
+               sk->sk_write_space(sk);

Sounds like you are ok with doing the wakeup here- so I will go
off and test that approach, and report back my findings.

Thanks,

-Jason