* [PATCH] net: Disambiguate kernel message @ 2012-01-30 22:41 Arun Sharma 2012-01-31 4:22 ` Bjorn Helgaas 2012-01-31 8:51 ` Christoph Paasch 0 siblings, 2 replies; 14+ messages in thread From: Arun Sharma @ 2012-01-30 22:41 UTC (permalink / raw) To: linux-kernel Cc: Arun Sharma, netdev, David Miller, Glauber Costa, Ingo Molnar 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] net: Disambiguate kernel message 2012-01-30 22:41 [PATCH] net: Disambiguate kernel message Arun Sharma @ 2012-01-31 4:22 ` Bjorn Helgaas 2012-01-31 18:15 ` Arun Sharma 2012-01-31 8:51 ` Christoph Paasch 1 sibling, 1 reply; 14+ messages in thread From: Bjorn Helgaas @ 2012-01-31 4:22 UTC (permalink / raw) To: Arun Sharma Cc: linux-kernel, netdev, David Miller, Glauber Costa, Ingo Molnar 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] net: Disambiguate kernel message 2012-01-31 4:22 ` Bjorn Helgaas @ 2012-01-31 18:15 ` Arun Sharma 2012-01-31 18:50 ` Joe Perches 2012-01-31 19:44 ` Ingo Molnar 0 siblings, 2 replies; 14+ messages in thread From: Arun Sharma @ 2012-01-31 18:15 UTC (permalink / raw) To: Bjorn Helgaas Cc: Arun Sharma, linux-kernel, netdev, David Miller, Glauber Costa, Ingo Molnar, christoph.paasch 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] net: Disambiguate kernel message 2012-01-31 18:15 ` Arun Sharma @ 2012-01-31 18:50 ` Joe Perches 2012-01-31 19:47 ` Arun Sharma 2012-01-31 19:44 ` Ingo Molnar 1 sibling, 1 reply; 14+ messages in thread From: Joe Perches @ 2012-01-31 18:50 UTC (permalink / raw) To: Arun Sharma Cc: Bjorn Helgaas, linux-kernel, netdev, David Miller, Glauber Costa, Ingo Molnar, christoph.paasch 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"); } ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] net: Disambiguate kernel message 2012-01-31 18:50 ` Joe Perches @ 2012-01-31 19:47 ` Arun Sharma 2012-01-31 20:09 ` Joe Perches 0 siblings, 1 reply; 14+ messages in thread From: Arun Sharma @ 2012-01-31 19:47 UTC (permalink / raw) To: Joe Perches Cc: Arun Sharma, Bjorn Helgaas, linux-kernel, netdev, David Miller, Glauber Costa, Ingo Molnar, christoph.paasch 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] net: Disambiguate kernel message 2012-01-31 19:47 ` Arun Sharma @ 2012-01-31 20:09 ` Joe Perches 2012-01-31 20:46 ` Arun Sharma 0 siblings, 1 reply; 14+ messages in thread From: Joe Perches @ 2012-01-31 20:09 UTC (permalink / raw) To: Arun Sharma Cc: Bjorn Helgaas, linux-kernel, netdev, David Miller, Glauber Costa, Ingo Molnar, christoph.paasch 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; } ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] net: Disambiguate kernel message 2012-01-31 20:09 ` Joe Perches @ 2012-01-31 20:46 ` Arun Sharma 2012-01-31 20:55 ` Joe Perches 0 siblings, 1 reply; 14+ messages in thread From: Arun Sharma @ 2012-01-31 20:46 UTC (permalink / raw) To: Joe Perches Cc: Bjorn Helgaas, linux-kernel, netdev, David Miller, Glauber Costa, Ingo Molnar, christoph.paasch 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] net: Disambiguate kernel message 2012-01-31 20:46 ` Arun Sharma @ 2012-01-31 20:55 ` Joe Perches 2012-01-31 21:15 ` David Miller 0 siblings, 1 reply; 14+ messages in thread From: Joe Perches @ 2012-01-31 20:55 UTC (permalink / raw) To: Arun Sharma Cc: Bjorn Helgaas, linux-kernel, netdev, David Miller, Glauber Costa, Ingo Molnar, christoph.paasch 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] net: Disambiguate kernel message 2012-01-31 20:55 ` Joe Perches @ 2012-01-31 21:15 ` David Miller 2012-01-31 22:05 ` Arun Sharma 0 siblings, 1 reply; 14+ messages in thread From: David Miller @ 2012-01-31 21:15 UTC (permalink / raw) To: joe Cc: asharma, bhelgaas, linux-kernel, netdev, glommer, mingo, christoph.paasch 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] net: Disambiguate kernel message 2012-01-31 21:15 ` David Miller @ 2012-01-31 22:05 ` Arun Sharma 2012-01-31 22:12 ` Joe Perches 2012-02-01 19:42 ` David Miller 0 siblings, 2 replies; 14+ messages in thread From: Arun Sharma @ 2012-01-31 22:05 UTC (permalink / raw) To: David Miller Cc: joe, asharma, bhelgaas, linux-kernel, netdev, glommer, mingo, christoph.paasch 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] net: Disambiguate kernel message 2012-01-31 22:05 ` Arun Sharma @ 2012-01-31 22:12 ` Joe Perches 2012-02-01 19:42 ` David Miller 1 sibling, 0 replies; 14+ messages in thread From: Joe Perches @ 2012-01-31 22:12 UTC (permalink / raw) To: Arun Sharma Cc: David Miller, bhelgaas, linux-kernel, netdev, glommer, mingo, christoph.paasch On Tue, 2012-01-31 at 14:05 -0800, Arun Sharma wrote: > Updated patch below. Thanks. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] net: Disambiguate kernel message 2012-01-31 22:05 ` Arun Sharma 2012-01-31 22:12 ` Joe Perches @ 2012-02-01 19:42 ` David Miller 1 sibling, 0 replies; 14+ messages in thread From: David Miller @ 2012-02-01 19:42 UTC (permalink / raw) To: asharma Cc: joe, bhelgaas, linux-kernel, netdev, glommer, mingo, christoph.paasch 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] net: Disambiguate kernel message 2012-01-31 18:15 ` Arun Sharma 2012-01-31 18:50 ` Joe Perches @ 2012-01-31 19:44 ` Ingo Molnar 1 sibling, 0 replies; 14+ messages in thread From: Ingo Molnar @ 2012-01-31 19:44 UTC (permalink / raw) To: Arun Sharma Cc: Bjorn Helgaas, linux-kernel, netdev, David Miller, Glauber Costa, christoph.paasch * 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] net: Disambiguate kernel message 2012-01-30 22:41 [PATCH] net: Disambiguate kernel message Arun Sharma 2012-01-31 4:22 ` Bjorn Helgaas @ 2012-01-31 8:51 ` Christoph Paasch 1 sibling, 0 replies; 14+ messages in thread From: Christoph Paasch @ 2012-01-31 8:51 UTC (permalink / raw) To: Arun Sharma Cc: linux-kernel, netdev, David Miller, Glauber Costa, Ingo Molnar 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 -- ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2012-02-01 19:43 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-01-30 22:41 [PATCH] net: Disambiguate kernel message Arun Sharma 2012-01-31 4:22 ` Bjorn Helgaas 2012-01-31 18:15 ` Arun Sharma 2012-01-31 18:50 ` Joe Perches 2012-01-31 19:47 ` Arun Sharma 2012-01-31 20:09 ` Joe Perches 2012-01-31 20:46 ` Arun Sharma 2012-01-31 20:55 ` Joe Perches 2012-01-31 21:15 ` David Miller 2012-01-31 22:05 ` Arun Sharma 2012-01-31 22:12 ` Joe Perches 2012-02-01 19:42 ` David Miller 2012-01-31 19:44 ` Ingo Molnar 2012-01-31 8:51 ` Christoph Paasch
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).