[PATCH] net: Disambiguate kernel message

[PATCH] net: Disambiguate kernel message

[Arun Sharma]

Some of our machines were reporting:

TCP: too many of orphaned sockets

even when the number of orphaned sockets was well below the
limit.

We print a different message depending on whether we're out
of TCP memory or there are too many orphan sockets.

Signed-off-by: Arun Sharma <asharma@fb.com>
Suggested-by: Mohan Srinivasan <mohan@fb.com>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: David Miller <davem@davemloft.net>
Cc: Glauber Costa <glommer@parallels.com>
Cc: Ingo Molnar <mingo@elte.hu>
---
 include/net/tcp.h    |   12 ++++++++----
 net/ipv4/tcp.c       |   14 +++++++++++++-
 net/ipv4/tcp_timer.c |    9 +++++++--
 3 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 0118ea9..92965dd 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -273,6 +273,14 @@ static inline int between(__u32 seq1, __u32 seq2, __u32 seq3)
 	return seq3 - seq2 >= seq1 - seq2;
 }
 
+static inline bool tcp_out_of_memory(struct sock *sk)
+{
+	if (sk->sk_wmem_queued > SOCK_MIN_SNDBUF &&
+	    sk_memory_allocated(sk) > sk_prot_mem_limits(sk, 2))
+		return true;
+	return false;
+}
+
 static inline bool tcp_too_many_orphans(struct sock *sk, int shift)
 {
 	struct percpu_counter *ocp = sk->sk_prot->orphan_count;
@@ -283,10 +291,6 @@ static inline bool tcp_too_many_orphans(struct sock *sk, int shift)
 		if (orphans << shift > sysctl_tcp_max_orphans)
 			return true;
 	}
-
-	if (sk->sk_wmem_queued > SOCK_MIN_SNDBUF &&
-	    sk_memory_allocated(sk) > sk_prot_mem_limits(sk, 2))
-		return true;
 	return false;
 }
 
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 9bcdec3..395a4ef 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2015,10 +2015,22 @@ adjudge_to_death:
 	}
 	if (sk->sk_state != TCP_CLOSE) {
 		sk_mem_reclaim(sk);
-		if (tcp_too_many_orphans(sk, 0)) {
+		bool too_many_orphans = tcp_too_many_orphans(sk, 0);
+		bool out_of_socket_memory = tcp_out_of_memory(sk);
+
+		if (too_many_orphans) {
 			if (net_ratelimit())
 				printk(KERN_INFO "TCP: too many of orphaned "
 				       "sockets\n");
+		}
+
+		if (out_of_socket_memory) {
+			if (net_ratelimit())
+				printk(KERN_INFO "TCP: out of memory. "
+				       "Consider tuning tcp_mem\n");
+		}
+
+		if (too_many_orphans || out_of_socket_memory) {
 			tcp_set_state(sk, TCP_CLOSE);
 			tcp_send_active_reset(sk, GFP_ATOMIC);
 			NET_INC_STATS_BH(sock_net(sk),
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index a516d1e..89907c3 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -67,6 +67,7 @@ static int tcp_out_of_resources(struct sock *sk, int do_reset)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 	int shift = 0;
+	bool too_many_orphans, out_of_socket_memory;
 
 	/* If peer does not open window for long time, or did not transmit
 	 * anything for long time, penalize it. */
@@ -77,9 +78,13 @@ static int tcp_out_of_resources(struct sock *sk, int do_reset)
 	if (sk->sk_err_soft)
 		shift++;
 
-	if (tcp_too_many_orphans(sk, shift)) {
-		if (net_ratelimit())
+	too_many_orphans = tcp_too_many_orphans(sk, shift);
+	out_of_socket_memory = tcp_out_of_memory(sk);
+	if (too_many_orphans || out_of_socket_memory) {
+		if (out_of_socket_memory && net_ratelimit())
 			printk(KERN_INFO "Out of socket memory\n");
+		if (too_many_orphans && net_ratelimit())
+			printk(KERN_INFO "TCP: too many orphaned sockets\n");
 
 		/* Catch exceptional cases, when connection requires reset.
 		 *      1. Last segment was sent recently. */
-- 
1.7.4

Re: [PATCH] net: Disambiguate kernel message

[Bjorn Helgaas]

On Mon, Jan 30, 2012 at 2:41 PM, Arun Sharma <asharma@fb.com> wrote:
> Some of our machines were reporting:
>
> TCP: too many of orphaned sockets
>
> even when the number of orphaned sockets was well below the
> limit.
>
> We print a different message depending on whether we're out
> of TCP memory or there are too many orphan sockets.

This patch mentions pairs of messages that are almost the same, but
not quite.  If they're supposed to be different, I'd suggest making
them clearly different.  As it is, the differences look like careless
mistakes:

>                                printk(KERN_INFO "TCP: too many of orphaned "
>                                       "sockets\n");
> +                       printk(KERN_INFO "TCP: too many orphaned sockets\n");

> +                               printk(KERN_INFO "TCP: out of memory. "
> +                                      "Consider tuning tcp_mem\n");
>                        printk(KERN_INFO "Out of socket memory\n");

"too many orphaned sockets" is better English usage than "too many of ..."

Bjorn

Re: [PATCH] net: Disambiguate kernel message

[Christoph Paasch]

On 01/30/2012 11:41 PM, Arun Sharma wrote:
> Some of our machines were reporting:
> 
> TCP: too many of orphaned sockets
> 
> even when the number of orphaned sockets was well below the
> limit.
> 
> We print a different message depending on whether we're out
> of TCP memory or there are too many orphan sockets.
> 
> Signed-off-by: Arun Sharma <asharma@fb.com>
> Suggested-by: Mohan Srinivasan <mohan@fb.com>
> Cc: netdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: David Miller <davem@davemloft.net>
> Cc: Glauber Costa <glommer@parallels.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> ---
>  include/net/tcp.h    |   12 ++++++++----
>  net/ipv4/tcp.c       |   14 +++++++++++++-
>  net/ipv4/tcp_timer.c |    9 +++++++--
>  3 files changed, 28 insertions(+), 7 deletions(-)
> 
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 0118ea9..92965dd 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -273,6 +273,14 @@ static inline int between(__u32 seq1, __u32 seq2, __u32 seq3)
>  	return seq3 - seq2 >= seq1 - seq2;
>  }
>  
> +static inline bool tcp_out_of_memory(struct sock *sk)
> +{
> +	if (sk->sk_wmem_queued > SOCK_MIN_SNDBUF &&
> +	    sk_memory_allocated(sk) > sk_prot_mem_limits(sk, 2))
> +		return true;
> +	return false;
> +}
> +
>  static inline bool tcp_too_many_orphans(struct sock *sk, int shift)
>  {
>  	struct percpu_counter *ocp = sk->sk_prot->orphan_count;
> @@ -283,10 +291,6 @@ static inline bool tcp_too_many_orphans(struct sock *sk, int shift)
>  		if (orphans << shift > sysctl_tcp_max_orphans)
>  			return true;
>  	}
> -
> -	if (sk->sk_wmem_queued > SOCK_MIN_SNDBUF &&
> -	    sk_memory_allocated(sk) > sk_prot_mem_limits(sk, 2))
> -		return true;
>  	return false;
>  }
>  
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 9bcdec3..395a4ef 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -2015,10 +2015,22 @@ adjudge_to_death:
>  	}
>  	if (sk->sk_state != TCP_CLOSE) {
>  		sk_mem_reclaim(sk);
> -		if (tcp_too_many_orphans(sk, 0)) {
> +		bool too_many_orphans = tcp_too_many_orphans(sk, 0);
> +		bool out_of_socket_memory = tcp_out_of_memory(sk);

This introduces a compiler warning because you have the declarations
after the code (sk_mem_reclaim()):

Cheers,
Christoph



-- 
Christoph Paasch
PhD Student

IP Networking Lab --- http://inl.info.ucl.ac.be
MultiPath TCP in the Linux Kernel --- http://mptcp.info.ucl.ac.be
Université Catholique de Louvain
-- 

Re: [PATCH] net: Disambiguate kernel message

[Arun Sharma]

On Mon, Jan 30, 2012 at 08:22:03PM -0800, Bjorn Helgaas wrote:
> > We print a different message depending on whether we're out
> > of TCP memory or there are too many orphan sockets.
> 
> This patch mentions pairs of messages that are almost the same, but
> not quite.  If they're supposed to be different, I'd suggest making
> them clearly different.  As it is, the differences look like careless
> mistakes:

Good point. Updated patch changes the existing printks to be
the same as well.

On 1/31/12 12:51 AM, Christoph Paasch wrote:

>> --- a/net/ipv4/tcp.c
>> +++ b/net/ipv4/tcp.c
>> @@ -2015,10 +2015,22 @@ adjudge_to_death:
>>   	}
>>   	if (sk->sk_state != TCP_CLOSE) {
>>   		sk_mem_reclaim(sk);
>> -		if (tcp_too_many_orphans(sk, 0)) {
>> +		bool too_many_orphans = tcp_too_many_orphans(sk, 0);
>> +		bool out_of_socket_memory = tcp_out_of_memory(sk);
> 
> This introduces a compiler warning because you have the declarations
> after the code (sk_mem_reclaim()):
> 

Fixed by the patch below.

 -Arun

>From 505717e51b15304b0bf50fc9dfed43fd06fad09a Mon Sep 17 00:00:00 2001
From: Arun Sharma <asharma@fb.com>
Date: Mon, 30 Jan 2012 14:16:06 -0800
Subject: [PATCH] net: Disambiguate kernel message

Some of our machines were reporting:

TCP: too many of orphaned sockets

even when the number of orphaned sockets was well below the
limit.

We print a different message depending on whether we're out
of TCP memory or there are too many orphaned sockets.

Signed-off-by: Arun Sharma <asharma@fb.com>
Suggested-by: Mohan Srinivasan <mohan@fb.com>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: David Miller <davem@davemloft.net>
Cc: Glauber Costa <glommer@parallels.com>
Cc: Ingo Molnar <mingo@elte.hu>
---
 include/net/tcp.h    |   12 ++++++++----
 net/ipv4/tcp.c       |   17 +++++++++++++++--
 net/ipv4/tcp_timer.c |   12 +++++++++---
 3 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 0118ea9..92965dd 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -273,6 +273,14 @@ static inline int between(__u32 seq1, __u32 seq2, __u32 seq3)
 	return seq3 - seq2 >= seq1 - seq2;
 }
 
+static inline bool tcp_out_of_memory(struct sock *sk)
+{
+	if (sk->sk_wmem_queued > SOCK_MIN_SNDBUF &&
+	    sk_memory_allocated(sk) > sk_prot_mem_limits(sk, 2))
+		return true;
+	return false;
+}
+
 static inline bool tcp_too_many_orphans(struct sock *sk, int shift)
 {
 	struct percpu_counter *ocp = sk->sk_prot->orphan_count;
@@ -283,10 +291,6 @@ static inline bool tcp_too_many_orphans(struct sock *sk, int shift)
 		if (orphans << shift > sysctl_tcp_max_orphans)
 			return true;
 	}
-
-	if (sk->sk_wmem_queued > SOCK_MIN_SNDBUF &&
-	    sk_memory_allocated(sk) > sk_prot_mem_limits(sk, 2))
-		return true;
 	return false;
 }
 
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 9bcdec3..27de5e4 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2014,11 +2014,24 @@ adjudge_to_death:
 		}
 	}
 	if (sk->sk_state != TCP_CLOSE) {
+		bool too_many_orphans, out_of_socket_memory;
+
 		sk_mem_reclaim(sk);
-		if (tcp_too_many_orphans(sk, 0)) {
+		too_many_orphans = tcp_too_many_orphans(sk, 0);
+		out_of_socket_memory = tcp_out_of_memory(sk);
+		if (too_many_orphans) {
 			if (net_ratelimit())
-				printk(KERN_INFO "TCP: too many of orphaned "
+				printk(KERN_INFO "TCP: too many orphaned "
 				       "sockets\n");
+		}
+
+		if (out_of_socket_memory) {
+			if (net_ratelimit())
+				printk(KERN_INFO "TCP: out of memory. "
+				       "Consider tuning tcp_mem\n");
+		}
+
+		if (too_many_orphans || out_of_socket_memory) {
 			tcp_set_state(sk, TCP_CLOSE);
 			tcp_send_active_reset(sk, GFP_ATOMIC);
 			NET_INC_STATS_BH(sock_net(sk),
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index a516d1e..0ac006b 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -67,6 +67,7 @@ static int tcp_out_of_resources(struct sock *sk, int do_reset)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 	int shift = 0;
+	bool too_many_orphans, out_of_socket_memory;
 
 	/* If peer does not open window for long time, or did not transmit
 	 * anything for long time, penalize it. */
@@ -77,9 +78,14 @@ static int tcp_out_of_resources(struct sock *sk, int do_reset)
 	if (sk->sk_err_soft)
 		shift++;
 
-	if (tcp_too_many_orphans(sk, shift)) {
-		if (net_ratelimit())
-			printk(KERN_INFO "Out of socket memory\n");
+	too_many_orphans = tcp_too_many_orphans(sk, shift);
+	out_of_socket_memory = tcp_out_of_memory(sk);
+	if (too_many_orphans || out_of_socket_memory) {
+		if (out_of_socket_memory && net_ratelimit())
+			printk(KERN_INFO "TCP: out of memory. "
+					 "Consider tuning tcp_mem\n");
+		if (too_many_orphans && net_ratelimit())
+			printk(KERN_INFO "TCP: too many orphaned sockets\n");
 
 		/* Catch exceptional cases, when connection requires reset.
 		 *      1. Last segment was sent recently. */
-- 
1.7.4

Re: [PATCH] net: Disambiguate kernel message

[Joe Perches]

On Tue, 2012-01-31 at 10:15 -0800, Arun Sharma wrote:
> On Mon, Jan 30, 2012 at 08:22:03PM -0800, Bjorn Helgaas wrote:
> > > We print a different message depending on whether we're out
> > > of TCP memory or there are too many orphan sockets.
> > 
> > This patch mentions pairs of messages that are almost the same, but
> > not quite.  If they're supposed to be different, I'd suggest making
> > them clearly different.  As it is, the differences look like careless
> > mistakes:
> 
> Good point. Updated patch changes the existing printks to be
> the same as well.
[]
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
[]
> @@ -2014,11 +2014,24 @@ adjudge_to_death:
[]
> +		if (too_many_orphans) {
>  			if (net_ratelimit())
> -				printk(KERN_INFO "TCP: too many of orphaned "
> +				printk(KERN_INFO "TCP: too many orphaned "
>  				       "sockets\n");
> +		}
> +
> +		if (out_of_socket_memory) {
> +			if (net_ratelimit())
> +				printk(KERN_INFO "TCP: out of memory. "
> +				       "Consider tuning tcp_mem\n");
> +		}
> +
[]
> diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
[]
> @@ -77,9 +78,14 @@ static int tcp_out_of_resources(struct sock *sk, int do_reset)
[]
> +	too_many_orphans = tcp_too_many_orphans(sk, shift);
> +	out_of_socket_memory = tcp_out_of_memory(sk);
> +	if (too_many_orphans || out_of_socket_memory) {
> +		if (out_of_socket_memory && net_ratelimit())
> +			printk(KERN_INFO "TCP: out of memory. "
> +					 "Consider tuning tcp_mem\n");
> +		if (too_many_orphans && net_ratelimit())
> +			printk(KERN_INFO "TCP: too many orphaned sockets\n");

These 2 blocks emit the messages in different order.

It might be useful to use a generic routine.

void tcp_log_oom(bool orphans, bool socket_memory)
{
	if (!net_ratelimit())
		return;
	if (orphans)
		pr_info("too many orphaned sockets\n");
	if (socket_memory)
		pr_info("out of socket memory - consider tuning tcp_mem\n");
}

Re: [PATCH] net: Disambiguate kernel message

[Ingo Molnar]

* Arun Sharma <asharma@fb.com> wrote:

> +				printk(KERN_INFO "TCP: too many orphaned "
>  				       "sockets\n");

> +				printk(KERN_INFO "TCP: out of memory. "
> +				       "Consider tuning tcp_mem\n");

> +			printk(KERN_INFO "TCP: out of memory. "
> +					 "Consider tuning tcp_mem\n");
> +		if (too_many_orphans && net_ratelimit())
> +			printk(KERN_INFO "TCP: too many orphaned sockets\n");

A small detail: please don't line break user-visible strings in 
mid sentence. Just keep the line long. Makes it much easier to 
search for the source of a kernel message:

   git grep "TCP: too many orphaned sockets"

Thanks,

	Ingo

Re: [PATCH] net: Disambiguate kernel message

[Arun Sharma]

On Tue, Jan 31, 2012 at 10:50:57AM -0800, Joe Perches wrote:
> 
> These 2 blocks emit the messages in different order.
> 
> It might be useful to use a generic routine.

Refactored slightly differently below.

On 1/31/12 11:44 AM, Ingo Molnar wrote:

> A small detail: please don't line break user-visible strings in
> mid sentence. Just keep the line long. Makes it much easier to
> search for the source of a kernel message:
> 

pr_info() on a single line now.

>From 08f36b8fd9f2439001a4192c0d1ff390d13e67e0 Mon Sep 17 00:00:00 2001
From: Arun Sharma <asharma@fb.com>
Date: Mon, 30 Jan 2012 14:16:06 -0800
Subject: [PATCH] net: Disambiguate kernel message

Some of our machines were reporting:

TCP: too many of orphaned sockets

even when the number of orphaned sockets was well below the
limit.

We print a different message depending on whether we're out
of TCP memory or there are too many orphaned sockets.

Signed-off-by: Arun Sharma <asharma@fb.com>
Suggested-by: Mohan Srinivasan <mohan@fb.com>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: David Miller <davem@davemloft.net>
Cc: Glauber Costa <glommer@parallels.com>
Cc: Ingo Molnar <mingo@elte.hu>
---
 include/net/tcp.h    |   20 ++++++++++++++++----
 net/ipv4/tcp.c       |   11 +++++++----
 net/ipv4/tcp_timer.c |    9 +++++----
 3 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 0118ea9..0dba9d6 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -273,6 +273,14 @@ static inline int between(__u32 seq1, __u32 seq2, __u32 seq3)
 	return seq3 - seq2 >= seq1 - seq2;
 }
 
+static inline bool tcp_out_of_memory(struct sock *sk)
+{
+	if (sk->sk_wmem_queued > SOCK_MIN_SNDBUF &&
+	    sk_memory_allocated(sk) > sk_prot_mem_limits(sk, 2))
+		return true;
+	return false;
+}
+
 static inline bool tcp_too_many_orphans(struct sock *sk, int shift)
 {
 	struct percpu_counter *ocp = sk->sk_prot->orphan_count;
@@ -283,13 +291,17 @@ static inline bool tcp_too_many_orphans(struct sock *sk, int shift)
 		if (orphans << shift > sysctl_tcp_max_orphans)
 			return true;
 	}
-
-	if (sk->sk_wmem_queued > SOCK_MIN_SNDBUF &&
-	    sk_memory_allocated(sk) > sk_prot_mem_limits(sk, 2))
-		return true;
 	return false;
 }
 
+static inline void tcp_log_oom(bool orphans, bool socket_memory)
+{
+	if (orphans && net_ratelimit())
+		pr_info("TCP: too many orphaned sockets\n");
+	if (socket_memory && net_ratelimit())
+		pr_info("TCP: out of memory -- consider tuning tcp_mem\n");
+}
+
 /* syncookies: remember time of last synqueue overflow */
 static inline void tcp_synq_overflow(struct sock *sk)
 {
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 9bcdec3..9c38b3a 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2014,11 +2014,14 @@ adjudge_to_death:
 		}
 	}
 	if (sk->sk_state != TCP_CLOSE) {
+		bool too_many_orphans, out_of_socket_memory;
+
 		sk_mem_reclaim(sk);
-		if (tcp_too_many_orphans(sk, 0)) {
-			if (net_ratelimit())
-				printk(KERN_INFO "TCP: too many of orphaned "
-				       "sockets\n");
+		too_many_orphans = tcp_too_many_orphans(sk, 0);
+		out_of_socket_memory = tcp_out_of_memory(sk);
+		tcp_log_oom(too_many_orphans, out_of_socket_memory);
+
+		if (too_many_orphans || out_of_socket_memory) {
 			tcp_set_state(sk, TCP_CLOSE);
 			tcp_send_active_reset(sk, GFP_ATOMIC);
 			NET_INC_STATS_BH(sock_net(sk),
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index a516d1e..c47fcb6 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -67,6 +67,7 @@ static int tcp_out_of_resources(struct sock *sk, int do_reset)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 	int shift = 0;
+	bool too_many_orphans, out_of_socket_memory;
 
 	/* If peer does not open window for long time, or did not transmit
 	 * anything for long time, penalize it. */
@@ -77,10 +78,10 @@ static int tcp_out_of_resources(struct sock *sk, int do_reset)
 	if (sk->sk_err_soft)
 		shift++;
 
-	if (tcp_too_many_orphans(sk, shift)) {
-		if (net_ratelimit())
-			printk(KERN_INFO "Out of socket memory\n");
-
+	too_many_orphans = tcp_too_many_orphans(sk, shift);
+	out_of_socket_memory = tcp_out_of_memory(sk);
+	tcp_log_oom(too_many_orphans, out_of_socket_memory);
+	if (too_many_orphans || out_of_socket_memory) {
 		/* Catch exceptional cases, when connection requires reset.
 		 *      1. Last segment was sent recently. */
 		if ((s32)(tcp_time_stamp - tp->lsndtime) <= TCP_TIMEWAIT_LEN ||
-- 
1.7.4

Re: [PATCH] net: Disambiguate kernel message

[Joe Perches]

On Tue, 2012-01-31 at 11:47 -0800, Arun Sharma wrote:
> On Tue, Jan 31, 2012 at 10:50:57AM -0800, Joe Perches wrote:
> > It might be useful to use a generic routine.
[]
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
[]
> @@ -2014,11 +2014,14 @@ adjudge_to_death:
[]
> +		too_many_orphans = tcp_too_many_orphans(sk, 0);
> +		out_of_socket_memory = tcp_out_of_memory(sk);
> +		tcp_log_oom(too_many_orphans, out_of_socket_memory);
[]
> diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
[]
> @@ -77,10 +78,10 @@ static int tcp_out_of_resources(struct sock *sk, int do_reset)
[]
> +	too_many_orphans = tcp_too_many_orphans(sk, shift);
> +	out_of_socket_memory = tcp_out_of_memory(sk);
> +	tcp_log_oom(too_many_orphans, out_of_socket_memory);
> +	if (too_many_orphans || out_of_socket_memory) {

Perhaps these repeated three lines should be a routine like:

bool tcp_check_oom(struct sock *sk, int shift)
{
	bool tcp_orphans = tcp_too_many_orphans(sk, shift);
	bool tcp_oom = tcp_out_of_memory(sk);

	printks...

	return tcp_orphans || tcp_oom;
}

Re: [PATCH] net: Disambiguate kernel message

[Arun Sharma]

On 1/31/12 12:09 PM, Joe Perches wrote:

>> +	too_many_orphans = tcp_too_many_orphans(sk, shift);
>> +	out_of_socket_memory = tcp_out_of_memory(sk);
>> +	tcp_log_oom(too_many_orphans, out_of_socket_memory);
>> +	if (too_many_orphans || out_of_socket_memory) {
>
> Perhaps these repeated three lines should be a routine like:
>
> bool tcp_check_oom(struct sock *sk, int shift)
> {
> 	bool tcp_orphans = tcp_too_many_orphans(sk, shift);
> 	bool tcp_oom = tcp_out_of_memory(sk);
>
> 	printks...
>
> 	return tcp_orphans || tcp_oom;
> }

I like your previous suggestion better. It preserves the ability to write:

if (too_many_orphans) {
   do_something();
}

if (out_of_socket_memory) {
   do_something_else();
}

  -Arun

Re: [PATCH] net: Disambiguate kernel message

[Joe Perches]

On Tue, 2012-01-31 at 12:46 -0800, Arun Sharma wrote:
> On 1/31/12 12:09 PM, Joe Perches wrote:
> >> +	too_many_orphans = tcp_too_many_orphans(sk, shift);
> >> +	out_of_socket_memory = tcp_out_of_memory(sk);
> >> +	tcp_log_oom(too_many_orphans, out_of_socket_memory);
> >> +	if (too_many_orphans || out_of_socket_memory) {
> > Perhaps these repeated three lines should be a routine like:
> > bool tcp_check_oom(struct sock *sk, int shift)
> > {
> > 	bool tcp_orphans = tcp_too_many_orphans(sk, shift);
> > 	bool tcp_oom = tcp_out_of_memory(sk);
> >
> > 	printks...
> >
> > 	return tcp_orphans || tcp_oom;
> > }
> I like your previous suggestion better. It preserves the ability to write:
> if (too_many_orphans) {
>    do_something();
> }
> if (out_of_socket_memory) {
>    do_something_else();
> }

shrug. That isn't currently used and 
tcp_too_many_orphans and tcp_out_of_memory
could still be checked.

I think the routine could be moved out-of-line.

Re: [PATCH] net: Disambiguate kernel message

[David Miller]

From: Joe Perches <joe@perches.com>
Date: Tue, 31 Jan 2012 12:55:41 -0800

> On Tue, 2012-01-31 at 12:46 -0800, Arun Sharma wrote:
>> On 1/31/12 12:09 PM, Joe Perches wrote:
>> >> +	too_many_orphans = tcp_too_many_orphans(sk, shift);
>> >> +	out_of_socket_memory = tcp_out_of_memory(sk);
>> >> +	tcp_log_oom(too_many_orphans, out_of_socket_memory);
>> >> +	if (too_many_orphans || out_of_socket_memory) {
>> > Perhaps these repeated three lines should be a routine like:
>> > bool tcp_check_oom(struct sock *sk, int shift)
>> > {
>> > 	bool tcp_orphans = tcp_too_many_orphans(sk, shift);
>> > 	bool tcp_oom = tcp_out_of_memory(sk);
>> >
>> > 	printks...
>> >
>> > 	return tcp_orphans || tcp_oom;
>> > }
>> I like your previous suggestion better. It preserves the ability to write:
>> if (too_many_orphans) {
>>    do_something();
>> }
>> if (out_of_socket_memory) {
>>    do_something_else();
>> }
> 
> shrug. That isn't currently used and 
> tcp_too_many_orphans and tcp_out_of_memory
> could still be checked.
> 
> I think the routine could be moved out-of-line.

Indeed, and make the out-of-line combined routine (that does the test
as well as the conditional logging) return a boolean that determines
if the "if (too_many_orphans || out_of_socket_memory)" test should
pass.

Re: [PATCH] net: Disambiguate kernel message

[Arun Sharma]

On Tue, Jan 31, 2012 at 04:15:35PM -0500, David Miller wrote:
> > 
> > shrug. That isn't currently used and 
> > tcp_too_many_orphans and tcp_out_of_memory
> > could still be checked.
> > 
> > I think the routine could be moved out-of-line.
> 
> Indeed, and make the out-of-line combined routine (that does the test
> as well as the conditional logging) return a boolean that determines
> if the "if (too_many_orphans || out_of_socket_memory)" test should
> pass.

Updated patch below. No change in the size of .text
.rodata is a bit larger.

 -Arun

>From f4a973dfbc9c624c9b93d2662f77eeae04e84497 Mon Sep 17 00:00:00 2001
From: Arun Sharma <asharma@fb.com>
Date: Mon, 30 Jan 2012 14:16:06 -0800
Subject: [PATCH] net: Disambiguate kernel message

Some of our machines were reporting:

TCP: too many of orphaned sockets

even when the number of orphaned sockets was well below the
limit.

We print a different message depending on whether we're out
of TCP memory or there are too many orphaned sockets.

Also move the check out of line and cleanup the messages
that were printed.

Signed-off-by: Arun Sharma <asharma@fb.com>
Suggested-by: Mohan Srinivasan <mohan@fb.com>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: David Miller <davem@davemloft.net>
Cc: Glauber Costa <glommer@parallels.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Joe Perches <joe@perches.com>
---
 include/net/tcp.h    |   14 ++++++++++----
 net/ipv4/tcp.c       |   19 +++++++++++++++----
 net/ipv4/tcp_timer.c |    5 +----
 3 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 0118ea9..9e158f1 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -273,6 +273,14 @@ static inline int between(__u32 seq1, __u32 seq2, __u32 seq3)
 	return seq3 - seq2 >= seq1 - seq2;
 }
 
+static inline bool tcp_out_of_memory(struct sock *sk)
+{
+	if (sk->sk_wmem_queued > SOCK_MIN_SNDBUF &&
+	    sk_memory_allocated(sk) > sk_prot_mem_limits(sk, 2))
+		return true;
+	return false;
+}
+
 static inline bool tcp_too_many_orphans(struct sock *sk, int shift)
 {
 	struct percpu_counter *ocp = sk->sk_prot->orphan_count;
@@ -283,13 +291,11 @@ static inline bool tcp_too_many_orphans(struct sock *sk, int shift)
 		if (orphans << shift > sysctl_tcp_max_orphans)
 			return true;
 	}
-
-	if (sk->sk_wmem_queued > SOCK_MIN_SNDBUF &&
-	    sk_memory_allocated(sk) > sk_prot_mem_limits(sk, 2))
-		return true;
 	return false;
 }
 
+extern bool tcp_check_oom(struct sock *sk, int shift);
+
 /* syncookies: remember time of last synqueue overflow */
 static inline void tcp_synq_overflow(struct sock *sk)
 {
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 9bcdec3..5c1cb35 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1876,6 +1876,20 @@ void tcp_shutdown(struct sock *sk, int how)
 }
 EXPORT_SYMBOL(tcp_shutdown);
 
+bool tcp_check_oom(struct sock *sk, int shift)
+{
+	bool too_many_orphans, out_of_socket_memory;
+
+	too_many_orphans = tcp_too_many_orphans(sk, shift);
+	out_of_socket_memory = tcp_out_of_memory(sk);
+
+	if (too_many_orphans && net_ratelimit())
+		pr_info("TCP: too many orphaned sockets\n");
+	if (out_of_socket_memory && net_ratelimit())
+		pr_info("TCP: out of memory -- consider tuning tcp_mem\n");
+	return too_many_orphans || out_of_socket_memory;
+}
+
 void tcp_close(struct sock *sk, long timeout)
 {
 	struct sk_buff *skb;
@@ -2015,10 +2029,7 @@ adjudge_to_death:
 	}
 	if (sk->sk_state != TCP_CLOSE) {
 		sk_mem_reclaim(sk);
-		if (tcp_too_many_orphans(sk, 0)) {
-			if (net_ratelimit())
-				printk(KERN_INFO "TCP: too many of orphaned "
-				       "sockets\n");
+		if (tcp_check_oom(sk, 0)) {
 			tcp_set_state(sk, TCP_CLOSE);
 			tcp_send_active_reset(sk, GFP_ATOMIC);
 			NET_INC_STATS_BH(sock_net(sk),
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index a516d1e..cd2e072 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -77,10 +77,7 @@ static int tcp_out_of_resources(struct sock *sk, int do_reset)
 	if (sk->sk_err_soft)
 		shift++;
 
-	if (tcp_too_many_orphans(sk, shift)) {
-		if (net_ratelimit())
-			printk(KERN_INFO "Out of socket memory\n");
-
+	if (tcp_check_oom(sk, shift)) {
 		/* Catch exceptional cases, when connection requires reset.
 		 *      1. Last segment was sent recently. */
 		if ((s32)(tcp_time_stamp - tp->lsndtime) <= TCP_TIMEWAIT_LEN ||
-- 
1.7.4

Re: [PATCH] net: Disambiguate kernel message

[Joe Perches]

On Tue, 2012-01-31 at 14:05 -0800, Arun Sharma wrote:
> Updated patch below.

Thanks.

Re: [PATCH] net: Disambiguate kernel message

[David Miller]

From: Arun Sharma <asharma@fb.com>
Date: Tue, 31 Jan 2012 14:05:22 -0800

> From: Arun Sharma <asharma@fb.com>
> Date: Mon, 30 Jan 2012 14:16:06 -0800
> Subject: [PATCH] net: Disambiguate kernel message
> 
> Some of our machines were reporting:
> 
> TCP: too many of orphaned sockets
> 
> even when the number of orphaned sockets was well below the
> limit.
> 
> We print a different message depending on whether we're out
> of TCP memory or there are too many orphaned sockets.
> 
> Also move the check out of line and cleanup the messages
> that were printed.
> 
> Signed-off-by: Arun Sharma <asharma@fb.com>
> Suggested-by: Mohan Srinivasan <mohan@fb.com>

Applied, thanks.