All of lore.kernel.org
 help / color / mirror / Atom feed
* [Patch v2] skbuff: Hide GFP_ATOMIC page allocation failures for dropped packets
@ 2013-05-26 20:45 atomlin
  2013-05-27 18:18 ` Rafael Aquini
  2013-05-27 22:41 ` Francois Romieu
  0 siblings, 2 replies; 23+ messages in thread
From: atomlin @ 2013-05-26 20:45 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, pshelar, mst, alexander.h.duyck, riel, aquini,
	sergei.shtylyov, linux-kernel, Aaron Tomlin

From: Aaron Tomlin <atomlin@redhat.com>

Since v1:
 - Removed unnecessary parentheses (sergei.shtylyov)

---8<---

Failed GFP_ATOMIC allocations by the network stack result in dropped
packets, which will be received on a subsequent retransmit, and an
unnecessary, noisy warning with a kernel backtrace.

These warnings are harmless, but they still cause users to panic and
file bug reports over dropped packets. It would be better to hide the
failed allocation warnings and backtraces, and let retransmits handle
dropped packets quietly.

Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
---
 net/core/skbuff.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index af9185d..84aa870 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -236,7 +236,7 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t gfp_mask,
 		? skbuff_fclone_cache : skbuff_head_cache;
 
 	if (sk_memalloc_socks() && (flags & SKB_ALLOC_RX))
-		gfp_mask |= __GFP_MEMALLOC;
+		gfp_mask |= __GFP_MEMALLOC | __GFP_NOWARN;
 
 	/* Get the HEAD */
 	skb = kmem_cache_alloc_node(cache, gfp_mask & ~__GFP_DMA, node);
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [Patch v2] skbuff: Hide GFP_ATOMIC page allocation failures for dropped packets
  2013-05-26 20:45 [Patch v2] skbuff: Hide GFP_ATOMIC page allocation failures for dropped packets atomlin
@ 2013-05-27 18:18 ` Rafael Aquini
  2013-05-27 22:41 ` Francois Romieu
  1 sibling, 0 replies; 23+ messages in thread
From: Rafael Aquini @ 2013-05-27 18:18 UTC (permalink / raw)
  To: atomlin
  Cc: netdev, davem, edumazet, pshelar, mst, alexander.h.duyck, riel,
	sergei.shtylyov, linux-kernel

On Sun, May 26, 2013 at 09:45:01PM +0100, atomlin@redhat.com wrote:
> From: Aaron Tomlin <atomlin@redhat.com>
> 
> Since v1:
>  - Removed unnecessary parentheses (sergei.shtylyov)
> 
> ---8<---
> 
> Failed GFP_ATOMIC allocations by the network stack result in dropped
> packets, which will be received on a subsequent retransmit, and an
> unnecessary, noisy warning with a kernel backtrace.
> 
> These warnings are harmless, but they still cause users to panic and
> file bug reports over dropped packets. It would be better to hide the
> failed allocation warnings and backtraces, and let retransmits handle
> dropped packets quietly.
> 
> Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
> ---

Acked-by: Rafael Aquini <aquini@redhat.com>


>  net/core/skbuff.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index af9185d..84aa870 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -236,7 +236,7 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t gfp_mask,
>  		? skbuff_fclone_cache : skbuff_head_cache;
>  
>  	if (sk_memalloc_socks() && (flags & SKB_ALLOC_RX))
> -		gfp_mask |= __GFP_MEMALLOC;
> +		gfp_mask |= __GFP_MEMALLOC | __GFP_NOWARN;
>  
>  	/* Get the HEAD */
>  	skb = kmem_cache_alloc_node(cache, gfp_mask & ~__GFP_DMA, node);
> -- 
> 1.8.1.4
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Patch v2] skbuff: Hide GFP_ATOMIC page allocation failures for dropped packets
  2013-05-26 20:45 [Patch v2] skbuff: Hide GFP_ATOMIC page allocation failures for dropped packets atomlin
  2013-05-27 18:18 ` Rafael Aquini
@ 2013-05-27 22:41 ` Francois Romieu
  2013-05-28 16:00   ` Ben Greear
  2013-05-28 21:30   ` Rik van Riel
  1 sibling, 2 replies; 23+ messages in thread
From: Francois Romieu @ 2013-05-27 22:41 UTC (permalink / raw)
  To: atomlin
  Cc: netdev, davem, edumazet, pshelar, mst, alexander.h.duyck, riel,
	aquini, sergei.shtylyov, linux-kernel

atomlin@redhat.com <atomlin@redhat.com> :
[...]
> Failed GFP_ATOMIC allocations by the network stack result in dropped
> packets, which will be received on a subsequent retransmit, and an
> unnecessary, noisy warning with a kernel backtrace.
> 
> These warnings are harmless, but they still cause users to panic and
> file bug reports over dropped packets. It would be better to hide the
> failed allocation warnings and backtraces, and let retransmits handle
> dropped packets quietly.

Linux VM may be perfect but device drivers do stupid things.

Please don't paper over it just because some shit ends in your backyard.

-- 
Ueimor

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Patch v2] skbuff: Hide GFP_ATOMIC page allocation failures for dropped packets
  2013-05-27 22:41 ` Francois Romieu
@ 2013-05-28 16:00   ` Ben Greear
  2013-05-28 16:15     ` Rafael Aquini
  2013-05-28 17:29     ` Joe Perches
  2013-05-28 21:30   ` Rik van Riel
  1 sibling, 2 replies; 23+ messages in thread
From: Ben Greear @ 2013-05-28 16:00 UTC (permalink / raw)
  To: Francois Romieu
  Cc: atomlin, netdev, davem, edumazet, pshelar, mst,
	alexander.h.duyck, riel, aquini, sergei.shtylyov, linux-kernel

On 05/27/2013 03:41 PM, Francois Romieu wrote:
> atomlin@redhat.com <atomlin@redhat.com> :
> [...]
>> Failed GFP_ATOMIC allocations by the network stack result in dropped
>> packets, which will be received on a subsequent retransmit, and an
>> unnecessary, noisy warning with a kernel backtrace.
>>
>> These warnings are harmless, but they still cause users to panic and
>> file bug reports over dropped packets. It would be better to hide the
>> failed allocation warnings and backtraces, and let retransmits handle
>> dropped packets quietly.
>
> Linux VM may be perfect but device drivers do stupid things.
>
> Please don't paper over it just because some shit ends in your backyard.

We should rate-limit these messages at least.  When a system is low on memory
the logs can quickly fill up with useless OOM messages, further slowing
the system...

Ben

>


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Patch v2] skbuff: Hide GFP_ATOMIC page allocation failures for dropped packets
  2013-05-28 16:00   ` Ben Greear
@ 2013-05-28 16:15     ` Rafael Aquini
  2013-05-28 16:19       ` Ben Greear
  2013-05-28 16:29       ` Eric Dumazet
  2013-05-28 17:29     ` Joe Perches
  1 sibling, 2 replies; 23+ messages in thread
From: Rafael Aquini @ 2013-05-28 16:15 UTC (permalink / raw)
  To: Ben Greear
  Cc: Francois Romieu, atomlin, netdev, davem, edumazet, pshelar, mst,
	alexander.h.duyck, riel, sergei.shtylyov, linux-kernel

On Tue, May 28, 2013 at 09:00:45AM -0700, Ben Greear wrote:
> On 05/27/2013 03:41 PM, Francois Romieu wrote:
> >atomlin@redhat.com <atomlin@redhat.com> :
> >[...]
> >>Failed GFP_ATOMIC allocations by the network stack result in dropped
> >>packets, which will be received on a subsequent retransmit, and an
> >>unnecessary, noisy warning with a kernel backtrace.
> >>
> >>These warnings are harmless, but they still cause users to panic and
> >>file bug reports over dropped packets. It would be better to hide the
> >>failed allocation warnings and backtraces, and let retransmits handle
> >>dropped packets quietly.
> >
> >Linux VM may be perfect but device drivers do stupid things.
> >
> >Please don't paper over it just because some shit ends in your backyard.
> 
> We should rate-limit these messages at least.  When a system is low on memory
> the logs can quickly fill up with useless OOM messages, further slowing
> the system...
>

The real problem seems to be that more and more the network stack (drivers, perhaps)
is relying on chunks of contiguous page-blocks without a fallback mechanism to
order-0 page allocations. When memory gets fragmented, these alloc failures
start to pop up more often and they scare ordinary sysadmins out of their paints.

The big point of this change was to attempt to relief some of these warnings 
which we believed as being useless, since the net stack would recover from it
by re-transmissions.
We might have misjudged the scenario, though. Perhaps a better approach would be
making the warning less verbose for all page-alloc failures. We could, perhaps,
only print a stack-dump out, if some debug flag is passed along, either as
reference, or by some CONFIG_DEBUG_ preprocessor directive.

Rafael
 
> Ben
> 
> >
> 
> 
> -- 
> Ben Greear <greearb@candelatech.com>
> Candela Technologies Inc  http://www.candelatech.com
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Patch v2] skbuff: Hide GFP_ATOMIC page allocation failures for dropped packets
  2013-05-28 16:15     ` Rafael Aquini
@ 2013-05-28 16:19       ` Ben Greear
  2013-05-28 16:29       ` Eric Dumazet
  1 sibling, 0 replies; 23+ messages in thread
From: Ben Greear @ 2013-05-28 16:19 UTC (permalink / raw)
  To: Rafael Aquini
  Cc: Francois Romieu, atomlin, netdev, davem, edumazet, pshelar, mst,
	alexander.h.duyck, riel, sergei.shtylyov, linux-kernel

On 05/28/2013 09:15 AM, Rafael Aquini wrote:
> On Tue, May 28, 2013 at 09:00:45AM -0700, Ben Greear wrote:
>> On 05/27/2013 03:41 PM, Francois Romieu wrote:
>>> atomlin@redhat.com <atomlin@redhat.com> :
>>> [...]
>>>> Failed GFP_ATOMIC allocations by the network stack result in dropped
>>>> packets, which will be received on a subsequent retransmit, and an
>>>> unnecessary, noisy warning with a kernel backtrace.
>>>>
>>>> These warnings are harmless, but they still cause users to panic and
>>>> file bug reports over dropped packets. It would be better to hide the
>>>> failed allocation warnings and backtraces, and let retransmits handle
>>>> dropped packets quietly.
>>>
>>> Linux VM may be perfect but device drivers do stupid things.
>>>
>>> Please don't paper over it just because some shit ends in your backyard.
>>
>> We should rate-limit these messages at least.  When a system is low on memory
>> the logs can quickly fill up with useless OOM messages, further slowing
>> the system...
>>
>
> The real problem seems to be that more and more the network stack (drivers, perhaps)
> is relying on chunks of contiguous page-blocks without a fallback mechanism to
> order-0 page allocations. When memory gets fragmented, these alloc failures
> start to pop up more often and they scare ordinary sysadmins out of their paints.
>
> The big point of this change was to attempt to relief some of these warnings
> which we believed as being useless, since the net stack would recover from it
> by re-transmissions.
> We might have misjudged the scenario, though. Perhaps a better approach would be
> making the warning less verbose for all page-alloc failures. We could, perhaps,
> only print a stack-dump out, if some debug flag is passed along, either as
> reference, or by some CONFIG_DEBUG_ preprocessor directive.

I have seen the logs spam with 0rder-0 allocation errors.  Maybe the system had
legitimate issues, but continuously spamming made it even harder to figure out
the problem, and constantly trying to write that much text to the serial console
has a big performance impact, further slowing the system when it should instead
be clearing it's packet backlog or whatever.

Maybe print the first OOM message with lots of details, and then use
some rate-limiting stuff to print out summary details at most every 5 seconds
or so after that.  Could reset the verbose timer after some period of no
OOM messages.

Ben

>
> Rafael
>
>> Ben
>>
>>>
>>
>>
>> --
>> Ben Greear <greearb@candelatech.com>
>> Candela Technologies Inc  http://www.candelatech.com
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Patch v2] skbuff: Hide GFP_ATOMIC page allocation failures for dropped packets
  2013-05-28 16:15     ` Rafael Aquini
  2013-05-28 16:19       ` Ben Greear
@ 2013-05-28 16:29       ` Eric Dumazet
  2013-05-28 17:43         ` Rafael Aquini
                           ` (2 more replies)
  1 sibling, 3 replies; 23+ messages in thread
From: Eric Dumazet @ 2013-05-28 16:29 UTC (permalink / raw)
  To: Rafael Aquini
  Cc: Ben Greear, Francois Romieu, atomlin, netdev, davem, edumazet,
	pshelar, mst, alexander.h.duyck, riel, sergei.shtylyov,
	linux-kernel

On Tue, 2013-05-28 at 13:15 -0300, Rafael Aquini wrote:

> The real problem seems to be that more and more the network stack (drivers, perhaps)
> is relying on chunks of contiguous page-blocks without a fallback mechanism to
> order-0 page allocations. When memory gets fragmented, these alloc failures
> start to pop up more often and they scare ordinary sysadmins out of their paints.
> 

Where do you see that ?

I see exactly the opposite trend. 

We have less and less buggy drivers, and we want to catch last
offenders.

> The big point of this change was to attempt to relief some of these warnings 
> which we believed as being useless, since the net stack would recover from it
> by re-transmissions.
> We might have misjudged the scenario, though. Perhaps a better approach would be
> making the warning less verbose for all page-alloc failures. We could, perhaps,
> only print a stack-dump out, if some debug flag is passed along, either as
> reference, or by some CONFIG_DEBUG_ preprocessor directive.


warn_alloc_failed() uses the standard DEFAULT_RATELIMIT_INTERVAL which
is very small (5 * HZ)

I would bump nopage_rs to somethin more reasonable, like one hour or one
day.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Patch v2] skbuff: Hide GFP_ATOMIC page allocation failures for dropped packets
  2013-05-28 16:00   ` Ben Greear
  2013-05-28 16:15     ` Rafael Aquini
@ 2013-05-28 17:29     ` Joe Perches
  1 sibling, 0 replies; 23+ messages in thread
From: Joe Perches @ 2013-05-28 17:29 UTC (permalink / raw)
  To: Ben Greear
  Cc: Francois Romieu, atomlin, netdev, davem, edumazet, pshelar, mst,
	alexander.h.duyck, riel, aquini, sergei.shtylyov, linux-kernel

On Tue, 2013-05-28 at 09:00 -0700, Ben Greear wrote:
> On 05/27/2013 03:41 PM, Francois Romieu wrote:
> > atomlin@redhat.com <atomlin@redhat.com> :
> > [...]
> >> Failed GFP_ATOMIC allocations by the network stack result in dropped
> >> packets, which will be received on a subsequent retransmit, and an
> >> unnecessary, noisy warning with a kernel backtrace.
[]
> > Please don't paper over it just because some shit ends in your backyard.
> We should rate-limit these messages at least.

Already done.
Look in mm/page_alloc:warn_alloc_failed()

void warn_alloc_failed(gfp_t gfp_mask, int order, const char *fmt, ...)
{
	unsigned int filter = SHOW_MEM_FILTER_NODES;

	if ((gfp_mask & __GFP_NOWARN) || !__ratelimit(&nopage_rs) ||
	    debug_guardpage_minorder() > 0)
		return;



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Patch v2] skbuff: Hide GFP_ATOMIC page allocation failures for dropped packets
  2013-05-28 16:29       ` Eric Dumazet
@ 2013-05-28 17:43         ` Rafael Aquini
  2013-05-28 17:46           ` Eric Dumazet
  2013-05-28 18:03           ` Eric Dumazet
  2013-05-28 18:19         ` Joe Perches
  2013-05-28 21:32         ` Rik van Riel
  2 siblings, 2 replies; 23+ messages in thread
From: Rafael Aquini @ 2013-05-28 17:43 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Ben Greear, Francois Romieu, atomlin, netdev, davem, edumazet,
	pshelar, mst, alexander.h.duyck, riel, sergei.shtylyov,
	linux-kernel

On Tue, May 28, 2013 at 09:29:37AM -0700, Eric Dumazet wrote:
> On Tue, 2013-05-28 at 13:15 -0300, Rafael Aquini wrote:
> 
> > The real problem seems to be that more and more the network stack (drivers, perhaps)
> > is relying on chunks of contiguous page-blocks without a fallback mechanism to
> > order-0 page allocations. When memory gets fragmented, these alloc failures
> > start to pop up more often and they scare ordinary sysadmins out of their paints.
> > 
> 
> Where do you see that ?
>
> I see exactly the opposite trend. 
> 
> We have less and less buggy drivers, and we want to catch last
> offenders.
> 

Perhaps the explanation is because we're looking into old stuff bad effects,
then. But just to list a few for your appreciation:
--------------------------------------------------------
Apr 23 11:25:31 217-IDC kernel: httpd: page allocation failure. order:1,
mode:0x20 Apr 23 11:25:31 217-IDC kernel: Pid: 19747, comm: httpd Not tainted
2.6.32-358.2.1.el6.x86_64 #1 Apr 23 11:25:31 217-IDC kernel: Call Trace: Apr 23
11:25:31 217-IDC kernel: <IRQ> [<ffffffff8112c207>] ?
__alloc_pages_nodemask+0x757/0x8d0 Apr 23 11:25:31 217-IDC kernel:
[<ffffffffa0337361>] ? bond_start_xmit+0x2f1/0x5d0 [bonding]
....
--------------------------------------------------------
Apr  4 18:51:32 exton kernel: swapper: page allocation failure. order:1,
mode:0x20
Apr  4 18:51:32 exton kernel: Pid: 0, comm: swapper Not tainted
2.6.32-279.19.1.el6.x86_64 #1
Apr  4 18:51:32 exton kernel: Call Trace:
Apr  4 18:51:32 exton kernel: <IRQ>  [<ffffffff811231ff>] ?
__alloc_pages_nodemask+0x77f/0x940
Apr  4 18:51:32 exton kernel: [<ffffffff8115d1a2>] ? kmem_getpages+0x62/0x170
Apr  4 18:51:32 exton kernel: [<ffffffff8115ddba>] ? fallback_alloc+0x1ba/0x270
Apr  4 18:51:32 exton kernel: [<ffffffff8115d80f>] ? cache_grow+0x2cf/0x320
Apr  4 18:51:32 exton kernel: [<ffffffff8115db39>] ?
____cache_alloc_node+0x99/0x160
Apr  4 18:51:32 exton kernel: [<ffffffff8115ed00>] ?
kmem_cache_alloc_node_trace+0x90/0x200
Apr  4 18:51:32 exton kernel: [<ffffffff8115ef1d>] ? __kmalloc_node+0x4d/0x60
Apr  4 18:51:32 exton kernel: [<ffffffff8141ea1d>] ? __alloc_skb+0x6d/0x190
Apr  4 18:51:32 exton kernel: [<ffffffff8141eb5d>] ? dev_alloc_skb+0x1d/0x40
Apr  4 18:51:32 exton kernel: [<ffffffffa04f5f50>] ?
ipoib_cm_alloc_rx_skb+0x30/0x430 [ib_ipoib]
Apr  4 18:51:32 exton kernel: [<ffffffffa04f71ef>] ?
ipoib_cm_handle_rx_wc+0x29f/0x770 [ib_ipoib]
Apr  4 18:51:32 exton kernel: [<ffffffffa03c6a46>] ? mlx4_ib_poll_cq+0x2c6/0x7f0
[mlx4_ib]
....
--------------------------------------------------------
May 14 09:00:34 ifil03 kernel: swapper: page allocation failure. order:1,
mode:0x20
May 14 09:00:34 ifil03 kernel: Pid: 0, comm: swapper Not tainted
2.6.32-220.el6.x86_64 #1
May 14 09:00:34 ifil03 kernel: Call Trace:
May 14 09:00:34 ifil03 kernel: <IRQ>  [<ffffffff81123f0f>] ?
__alloc_pages_nodemask+0x77f/0x940
May 14 09:00:34 ifil03 kernel: [<ffffffff8115ddc2>] ? kmem_getpages+0x62/0x170
May 14 09:00:34 ifil03 kernel: [<ffffffff8115e9da>] ? fallback_alloc+0x1ba/0x270
May 14 09:00:34 ifil03 kernel: [<ffffffff8115e42f>] ? cache_grow+0x2cf/0x320
May 14 09:00:34 ifil03 kernel: [<ffffffff8115e759>] ?
____cache_alloc_node+0x99/0x160
May 14 09:00:34 ifil03 kernel: [<ffffffff8115f53b>] ?
kmem_cache_alloc+0x11b/0x190
May 14 09:00:34 ifil03 kernel: [<ffffffff8141f528>] ? sk_prot_alloc+0x48/0x1c0
May 14 09:00:34 ifil03 kernel: [<ffffffff8141f7b2>] ? sk_clone+0x22/0x2e0
May 14 09:00:34 ifil03 kernel: [<ffffffff8146ca26>] ? inet_csk_clone+0x16/0xd0
May 14 09:00:34 ifil03 kernel: [<ffffffff814858f3>] ?
tcp_create_openreq_child+0x23/0x450
May 14 09:00:34 ifil03 kernel: [<ffffffff814832dd>] ?
tcp_v4_syn_recv_sock+0x4d/0x2a0
May 14 09:00:34 ifil03 kernel: [<ffffffff814856b1>] ? tcp_check_req+0x201/0x420
May 14 09:00:34 ifil03 kernel: [<ffffffff8147b166>] ?
tcp_rcv_state_process+0x116/0xa30
May 14 09:00:34 ifil03 kernel: [<ffffffff81482cfb>] ? tcp_v4_do_rcv+0x35b/0x430
May 14 09:00:34 ifil03 kernel: [<ffffffff81484471>] ? tcp_v4_rcv+0x4e1/0x860
May 14 09:00:34 ifil03 kernel: [<ffffffff814621fd>] ?
ip_local_deliver_finish+0xdd/0x2d0
May 14 09:00:34 ifil03 kernel: [<ffffffff81462488>] ? ip_local_deliver+0x98/0xa0
May 14 09:00:34 ifil03 kernel: [<ffffffff8146194d>] ? ip_rcv_finish+0x12d/0x440
May 14 09:00:34 ifil03 kernel: [<ffffffff8101bd86>] ?
intel_pmu_enable_all+0xa6/0x150
May 14 09:00:34 ifil03 kernel: [<ffffffff81461ed5>] ? ip_rcv+0x275/0x350
May 14 09:00:34 ifil03 kernel: [<ffffffff8142bedb>] ?
__netif_receive_skb+0x49b/0x6e0
May 14 09:00:34 ifil03 kernel: [<ffffffff8142df88>] ?
netif_receive_skb+0x58/0x60
May 14 09:00:34 ifil03 kernel: [<ffffffffa00a0a9e>] ?
vmxnet3_rq_rx_complete+0x36e/0x880 [vmxnet3]
....
--------------------------------------------------------


 
> > The big point of this change was to attempt to relief some of these warnings 
> > which we believed as being useless, since the net stack would recover from it
> > by re-transmissions.
> > We might have misjudged the scenario, though. Perhaps a better approach would be
> > making the warning less verbose for all page-alloc failures. We could, perhaps,
> > only print a stack-dump out, if some debug flag is passed along, either as
> > reference, or by some CONFIG_DEBUG_ preprocessor directive.
> 
> 
> warn_alloc_failed() uses the standard DEFAULT_RATELIMIT_INTERVAL which
> is very small (5 * HZ)
> 
> I would bump nopage_rs to somethin more reasonable, like one hour or one
> day.
>

Neat! Worth to try, no doubts about that. Aaron?

Cheers!
-- Rafael 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Patch v2] skbuff: Hide GFP_ATOMIC page allocation failures for dropped packets
  2013-05-28 17:43         ` Rafael Aquini
@ 2013-05-28 17:46           ` Eric Dumazet
  2013-05-28 18:03           ` Eric Dumazet
  1 sibling, 0 replies; 23+ messages in thread
From: Eric Dumazet @ 2013-05-28 17:46 UTC (permalink / raw)
  To: Rafael Aquini
  Cc: Ben Greear, Francois Romieu, atomlin, netdev, davem, edumazet,
	pshelar, mst, alexander.h.duyck, riel, sergei.shtylyov,
	linux-kernel

On Tue, 2013-05-28 at 14:43 -0300, Rafael Aquini wrote:

> 
> Perhaps the explanation is because we're looking into old stuff bad effects,
> then. But just to list a few for your appreciation:
> --------------------------------------------------------


> --------------------------------------------------------
> May 14 09:00:34 ifil03 kernel: swapper: page allocation failure. order:1,
> mode:0x20
> May 14 09:00:34 ifil03 kernel: Pid: 0, comm: swapper Not tainted
> 2.6.32-220.el6.x86_64 #1
> May 14 09:00:34 ifil03 kernel: Call Trace:
> May 14 09:00:34 ifil03 kernel: <IRQ>  [<ffffffff81123f0f>] ?
> __alloc_pages_nodemask+0x77f/0x940
> May 14 09:00:34 ifil03 kernel: [<ffffffff8115ddc2>] ? kmem_getpages+0x62/0x170
> May 14 09:00:34 ifil03 kernel: [<ffffffff8115e9da>] ? fallback_alloc+0x1ba/0x270
> May 14 09:00:34 ifil03 kernel: [<ffffffff8115e42f>] ? cache_grow+0x2cf/0x320
> May 14 09:00:34 ifil03 kernel: [<ffffffff8115e759>] ?
> ____cache_alloc_node+0x99/0x160
> May 14 09:00:34 ifil03 kernel: [<ffffffff8115f53b>] ?
> kmem_cache_alloc+0x11b/0x190
> May 14 09:00:34 ifil03 kernel: [<ffffffff8141f528>] ? sk_prot_alloc+0x48/0x1c0
> May 14 09:00:34 ifil03 kernel: [<ffffffff8141f7b2>] ? sk_clone+0x22/0x2e0
> May 14 09:00:34 ifil03 kernel: [<ffffffff8146ca26>] ? inet_csk_clone+0x16/0xd0
> May 14 09:00:34 ifil03 kernel: [<ffffffff814858f3>] ?
> tcp_create_openreq_child+0x23/0x450
> May 14 09:00:34 ifil03 kernel: [<ffffffff814832dd>] ?
> tcp_v4_syn_recv_sock+0x4d/0x2a0
> May 14 09:00:34 ifil03 kernel: [<ffffffff814856b1>] ? tcp_check_req+0x201/0x420
> May 14 09:00:34 ifil03 kernel: [<ffffffff8147b166>] ?
> tcp_rcv_state_process+0x116/0xa30
> May 14 09:00:34 ifil03 kernel: [<ffffffff81482cfb>] ? tcp_v4_do_rcv+0x35b/0x430
> May 14 09:00:34 ifil03 kernel: [<ffffffff81484471>] ? tcp_v4_rcv+0x4e1/0x860
> May 14 09:00:34 ifil03 kernel: [<ffffffff814621fd>] ?
> ip_local_deliver_finish+0xdd/0x2d0
> May 14 09:00:34 ifil03 kernel: [<ffffffff81462488>] ? ip_local_deliver+0x98/0xa0
> May 14 09:00:34 ifil03 kernel: [<ffffffff8146194d>] ? ip_rcv_finish+0x12d/0x440
> May 14 09:00:34 ifil03 kernel: [<ffffffff8101bd86>] ?
> intel_pmu_enable_all+0xa6/0x150
> May 14 09:00:34 ifil03 kernel: [<ffffffff81461ed5>] ? ip_rcv+0x275/0x350
> May 14 09:00:34 ifil03 kernel: [<ffffffff8142bedb>] ?
> __netif_receive_skb+0x49b/0x6e0
> May 14 09:00:34 ifil03 kernel: [<ffffffff8142df88>] ?
> netif_receive_skb+0x58/0x60
> May 14 09:00:34 ifil03 kernel: [<ffffffffa00a0a9e>] ?
> vmxnet3_rq_rx_complete+0x36e/0x880 [vmxnet3]
> ....
> --

I hope you do realize this path has nothing to do with skb allocation ,
but socket cloning ?




^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Patch v2] skbuff: Hide GFP_ATOMIC page allocation failures for dropped packets
  2013-05-28 17:43         ` Rafael Aquini
  2013-05-28 17:46           ` Eric Dumazet
@ 2013-05-28 18:03           ` Eric Dumazet
  1 sibling, 0 replies; 23+ messages in thread
From: Eric Dumazet @ 2013-05-28 18:03 UTC (permalink / raw)
  To: Rafael Aquini, Roland Dreier
  Cc: Ben Greear, Francois Romieu, atomlin, netdev, davem, edumazet,
	pshelar, mst, alexander.h.duyck, riel, sergei.shtylyov,
	linux-kernel

On Tue, 2013-05-28 at 14:43 -0300, Rafael Aquini wrote:

> Perhaps the explanation is because we're looking into old stuff bad effects,
> then. But just to list a few for your appreciation:
> --------------------------------------------------------
> Apr 23 11:25:31 217-IDC kernel: httpd: page allocation failure. order:1,
> mode:0x20 Apr 23 11:25:31 217-IDC kernel: Pid: 19747, comm: httpd Not tainted
> 2.6.32-358.2.1.el6.x86_64 #1 Apr 23 11:25:31 217-IDC kernel: Call Trace: Apr 23
> 11:25:31 217-IDC kernel: <IRQ> [<ffffffff8112c207>] ?
> __alloc_pages_nodemask+0x757/0x8d0 Apr 23 11:25:31 217-IDC kernel:
> [<ffffffffa0337361>] ? bond_start_xmit+0x2f1/0x5d0 [bonding]
> ....
> --------------------------------------------------------
> Apr  4 18:51:32 exton kernel: swapper: page allocation failure. order:1,
> mode:0x20
> Apr  4 18:51:32 exton kernel: Pid: 0, comm: swapper Not tainted
> 2.6.32-279.19.1.el6.x86_64 #1
> Apr  4 18:51:32 exton kernel: Call Trace:
> Apr  4 18:51:32 exton kernel: <IRQ>  [<ffffffff811231ff>] ?
> __alloc_pages_nodemask+0x77f/0x940
> Apr  4 18:51:32 exton kernel: [<ffffffff8115d1a2>] ? kmem_getpages+0x62/0x170
> Apr  4 18:51:32 exton kernel: [<ffffffff8115ddba>] ? fallback_alloc+0x1ba/0x270
> Apr  4 18:51:32 exton kernel: [<ffffffff8115d80f>] ? cache_grow+0x2cf/0x320
> Apr  4 18:51:32 exton kernel: [<ffffffff8115db39>] ?
> ____cache_alloc_node+0x99/0x160
> Apr  4 18:51:32 exton kernel: [<ffffffff8115ed00>] ?
> kmem_cache_alloc_node_trace+0x90/0x200
> Apr  4 18:51:32 exton kernel: [<ffffffff8115ef1d>] ? __kmalloc_node+0x4d/0x60
> Apr  4 18:51:32 exton kernel: [<ffffffff8141ea1d>] ? __alloc_skb+0x6d/0x190
> Apr  4 18:51:32 exton kernel: [<ffffffff8141eb5d>] ? dev_alloc_skb+0x1d/0x40
> Apr  4 18:51:32 exton kernel: [<ffffffffa04f5f50>] ?
> ipoib_cm_alloc_rx_skb+0x30/0x430 [ib_ipoib]
> Apr  4 18:51:32 exton kernel: [<ffffffffa04f71ef>] ?
> ipoib_cm_handle_rx_wc+0x29f/0x770 [ib_ipoib]
> Apr  4 18:51:32 exton kernel: [<ffffffffa03c6a46>] ? mlx4_ib_poll_cq+0x2c6/0x7f0
> [mlx4_ib]
> ....
> ----


This one seems a real bug/problem in
drivers/infiniband/ulp/ipoib/ipoib_cm.c

It uses :

IPOIB_CM_HEAD_SIZE    = IPOIB_CM_BUF_SIZE % PAGE_SIZE, 
IPOIB_CM_RX_SG        = ALIGN(IPOIB_CM_BUF_SIZE, PAGE_SIZE) /
PAGE_SIZE, 

but then, ipoib_cm_alloc_rx_skb() does :

skb = dev_alloc_skb(IPOIB_CM_HEAD_SIZE + 12);

so really asking more than one page for the first frag (skb->head),
while the intent of the code was to use order-0 allocations.

 for (i = 0; i < frags; i++) {
    struct page *page = alloc_page(GFP_ATOMIC);
....

Ideally, IPOIB_CM_HEAD_SIZE  should be redefined to use

SKB_MAX_HEAD(NET_SKB_PAD + 12)

so that skb->head would use exactly oder-0 page, not order-1 one.


Do you know understand why we should not hide allocation errors ?




^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Patch v2] skbuff: Hide GFP_ATOMIC page allocation failures for dropped packets
  2013-05-28 16:29       ` Eric Dumazet
  2013-05-28 17:43         ` Rafael Aquini
@ 2013-05-28 18:19         ` Joe Perches
  2013-05-28 21:32         ` Rik van Riel
  2 siblings, 0 replies; 23+ messages in thread
From: Joe Perches @ 2013-05-28 18:19 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Rafael Aquini, Ben Greear, Francois Romieu, atomlin, netdev,
	davem, edumazet, pshelar, mst, alexander.h.duyck, riel,
	sergei.shtylyov, linux-kernel

On Tue, 2013-05-28 at 09:29 -0700, Eric Dumazet wrote:
> I would bump nopage_rs to somethin more reasonable, like one hour or one
> day.

Reasonable is harder to specify but perhaps it could
be made runtime configurable.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Patch v2] skbuff: Hide GFP_ATOMIC page allocation failures for dropped packets
  2013-05-27 22:41 ` Francois Romieu
  2013-05-28 16:00   ` Ben Greear
@ 2013-05-28 21:30   ` Rik van Riel
  1 sibling, 0 replies; 23+ messages in thread
From: Rik van Riel @ 2013-05-28 21:30 UTC (permalink / raw)
  To: Francois Romieu
  Cc: atomlin, netdev, davem, edumazet, pshelar, mst,
	alexander.h.duyck, aquini, sergei.shtylyov, linux-kernel

On 05/27/2013 06:41 PM, Francois Romieu wrote:
> atomlin@redhat.com <atomlin@redhat.com> :
> [...]
>> Failed GFP_ATOMIC allocations by the network stack result in dropped
>> packets, which will be received on a subsequent retransmit, and an
>> unnecessary, noisy warning with a kernel backtrace.
>>
>> These warnings are harmless, but they still cause users to panic and
>> file bug reports over dropped packets. It would be better to hide the
>> failed allocation warnings and backtraces, and let retransmits handle
>> dropped packets quietly.
>
> Linux VM may be perfect but device drivers do stupid things.
>
> Please don't paper over it just because some shit ends in your backyard.

It is impossible to free memory at the speed at which
10Gbit network packets can come in.

Dropped packets are a reality.

The network stack already has statistics counters to
keep track of dropped packets. There is absolutely
no reason to print out an entire kernel backtrace
for dropped network packets.

All that achieves is get people to file bug reports,
which nothing can be done about. Oh, and distract
them from whatever issue as causing their actual
problem, and delay them fixing what was going on.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Patch v2] skbuff: Hide GFP_ATOMIC page allocation failures for dropped packets
  2013-05-28 16:29       ` Eric Dumazet
  2013-05-28 17:43         ` Rafael Aquini
  2013-05-28 18:19         ` Joe Perches
@ 2013-05-28 21:32         ` Rik van Riel
  2 siblings, 0 replies; 23+ messages in thread
From: Rik van Riel @ 2013-05-28 21:32 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Rafael Aquini, Ben Greear, Francois Romieu, atomlin, netdev,
	davem, edumazet, pshelar, mst, alexander.h.duyck,
	sergei.shtylyov, linux-kernel

On 05/28/2013 12:29 PM, Eric Dumazet wrote:
> On Tue, 2013-05-28 at 13:15 -0300, Rafael Aquini wrote:
>
>> The real problem seems to be that more and more the network stack (drivers, perhaps)
>> is relying on chunks of contiguous page-blocks without a fallback mechanism to
>> order-0 page allocations. When memory gets fragmented, these alloc failures
>> start to pop up more often and they scare ordinary sysadmins out of their paints.
>>
>
> Where do you see that ?
>
> I see exactly the opposite trend.
>
> We have less and less buggy drivers, and we want to catch last
> offenders.

These backtraces would still get printed out for drivers
that DO do the right thing and fall back to smaller
allocations.

The initial failed large allocation would cause a backtrace.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Patch v2] skbuff: Hide GFP_ATOMIC page allocation failures for dropped packets
  2013-05-27 22:25   ` Eric Dumazet
  2013-05-28  4:31     ` Joe Perches
@ 2013-05-29  7:36     ` Rik van Riel
  1 sibling, 0 replies; 23+ messages in thread
From: Rik van Riel @ 2013-05-29  7:36 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: atomlin, netdev, davem, edumazet, pshelar, mst,
	alexander.h.duyck, aquini, sergei.shtylyov, linux-kernel

On 05/27/2013 06:25 PM, Eric Dumazet wrote:
> On Mon, 2013-05-27 at 13:39 -0400, Rik van Riel wrote:


>> Yes please. Getting memory management bug reports for
>> dropped network packets got old years ago.  Lets get
>> rid of those messages.
>
> I am only wondering why this path has anything needing special
> attention, over thousands of kmalloc() like call sites in the kernel.

There are a few special things about the network code:

1) network packets can arrive extremely fast, in
    large batches
2) the network code cannot wait for the VM to free
    memory (GFP_ATOMIC)

Other allocations tend to be done less at a time, and/or
allow the VM to free up memory before proceeding.

> If mm allocation warnings are useless, just make __GFP_NOWARN the
> default, and save us thousand of patches (adding the __GFP_NOWARN
> everywhere)
>
> Truth is : some network drivers don't deal very well with allocation
> errors. mlx4 for example absolutely wants order-2 pages in RX path, with
> no fallback to order-0 pages.

Network protocols and network applications tend to deal
with packet loss by retransmitting data, though.

Also, once all the data from one of those order-2 page
buffers has been delivered or forwarded, that buffer
becomes available to subsequent network packets.

Other allocations tend not to free & reuse their
memory as quickly as the network stack.

> So I am not against this patch, but I can not really acknowledge it,
> sorry.
>
>
>


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Patch v2] skbuff: Hide GFP_ATOMIC page allocation failures for dropped packets
  2013-05-28  4:31     ` Joe Perches
  2013-05-28  6:04       ` Eric Dumazet
@ 2013-05-28 17:15       ` Debabrata Banerjee
  1 sibling, 0 replies; 23+ messages in thread
From: Debabrata Banerjee @ 2013-05-28 17:15 UTC (permalink / raw)
  To: Joe Perches
  Cc: Eric Dumazet, Rik van Riel, atomlin, netdev, davem, edumazet,
	pshelar, mst, alexander.h.duyck, aquini, sergei.shtylyov,
	linux-kernel, Banerjee, Debabrata, Joshua Hunt

On Tue, May 28, 2013 at 12:31 AM, Joe Perches <joe@perches.com> wrote:
>
> I think the __alloc_skb alloc failure message is ok,
> but maybe there shouldn't be something "scary" like
> a dump_stack.
>
> Maybe this site should use a trivial debug error
> message like below instead.

The stack trace may or may not be important for debugging, however
what is important is that we know how often this is happening. That
implies that there should be a percpu counter of allocation failures
here, regardless of a dump_stack, rate limited or not. I would suppose
it's ok for no stack by default as long as there is a counter, for
this specific call only.

There are some scary ways this code is called, for example from
tcp_send_fin(). It does yield() in that loop, but otherwise it just
spins.

-Deb

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Patch v2] skbuff: Hide GFP_ATOMIC page allocation failures for dropped packets
  2013-05-28  6:04       ` Eric Dumazet
@ 2013-05-28  6:08         ` Joe Perches
  0 siblings, 0 replies; 23+ messages in thread
From: Joe Perches @ 2013-05-28  6:08 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Rik van Riel, atomlin, netdev, davem, edumazet, pshelar, mst,
	alexander.h.duyck, aquini, sergei.shtylyov, linux-kernel

On Mon, 2013-05-27 at 23:04 -0700, Eric Dumazet wrote:
> If dump_stack are scary, they are scary for every k[mz]alloc() users,
> not only __alloc_skb_alloc()

True, but perhaps they are relatively rarer though
for non net use cases.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Patch v2] skbuff: Hide GFP_ATOMIC page allocation failures for dropped packets
  2013-05-28  4:31     ` Joe Perches
@ 2013-05-28  6:04       ` Eric Dumazet
  2013-05-28  6:08         ` Joe Perches
  2013-05-28 17:15       ` Debabrata Banerjee
  1 sibling, 1 reply; 23+ messages in thread
From: Eric Dumazet @ 2013-05-28  6:04 UTC (permalink / raw)
  To: Joe Perches
  Cc: Rik van Riel, atomlin, netdev, davem, edumazet, pshelar, mst,
	alexander.h.duyck, aquini, sergei.shtylyov, linux-kernel

On Mon, 2013-05-27 at 21:31 -0700, Joe Perches wrote:

> I think the __alloc_skb alloc failure message is ok,
> but maybe there shouldn't be something "scary" like
> a dump_stack.
> 
> Maybe this site should use a trivial debug error
> message like below instead.
> ---

Oh well.

If dump_stack are scary, they are scary for every k[mz]alloc() users,
not only __alloc_skb_alloc()

I just said : Please do not add GFP_NOWARN to thousand of call sites,
and you suggest adding more code in network fast path. (???)

This is not a trivial code, we are speaking of a very sensitive one.

Let mm guys explain in what cases a full stack trace is nice to have,
and in what cases its useless. An heuristic should be defined in mm tree
for that, and not spread everywhere.

There must be a reason GFP_NOWARN is seldom used in the kernel, even if
most callers are able to recover properly from a failed memory
allocation.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Patch v2] skbuff: Hide GFP_ATOMIC page allocation failures for dropped packets
  2013-05-27 22:25   ` Eric Dumazet
@ 2013-05-28  4:31     ` Joe Perches
  2013-05-28  6:04       ` Eric Dumazet
  2013-05-28 17:15       ` Debabrata Banerjee
  2013-05-29  7:36     ` Rik van Riel
  1 sibling, 2 replies; 23+ messages in thread
From: Joe Perches @ 2013-05-28  4:31 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Rik van Riel, atomlin, netdev, davem, edumazet, pshelar, mst,
	alexander.h.duyck, aquini, sergei.shtylyov, linux-kernel

On Mon, 2013-05-27 at 15:25 -0700, Eric Dumazet wrote:
> On Mon, 2013-05-27 at 13:39 -0400, Rik van Riel wrote:
> > On 05/26/2013 04:19 PM, atomlin@redhat.com wrote:
> > > Failed GFP_ATOMIC allocations by the network stack result in dropped
> > > packets, which will be received on a subsequent retransmit, and an
> > > unnecessary, noisy warning with a kernel backtrace.
> 
> This claim is wrong, only some protocols deal with retransmits.
> 
> > > These warnings are harmless, but they still cause users to panic and
> > > file bug reports over dropped packets. It would be better to hide the
> > > failed allocation warnings and backtraces, and let retransmits handle
> > > dropped packets quietly.
> > 
> > Yes please. Getting memory management bug reports for
> > dropped network packets got old years ago.  Lets get
> > rid of those messages.
> 
> I am only wondering why this path has anything needing special
> attention, over thousands of kmalloc() like call sites in the kernel.

I don't think this site is particularly special.

> If mm allocation warnings are useless, just make __GFP_NOWARN the
> default, and save us thousand of patches (adding the __GFP_NOWARN
> everywhere)
> 
> Truth is : some network drivers don't deal very well with allocation
> errors. mlx4 for example absolutely wants order-2 pages in RX path, with
> no fallback to order-0 pages.
> 
> So I am not against this patch, but I can not really acknowledge it,
> sorry.

I think the __alloc_skb alloc failure message is ok,
but maybe there shouldn't be something "scary" like
a dump_stack.

Maybe this site should use a trivial debug error
message like below instead.
---
 net/core/skbuff.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index d629891..0154803 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -231,17 +231,24 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t gfp_mask,
 	struct sk_buff *skb;
 	u8 *data;
 	bool pfmemalloc;
+	bool warn_no_skb = !(gfp_mask & __GFP_NOWARN);
 
 	cache = (flags & SKB_ALLOC_FCLONE)
 		? skbuff_fclone_cache : skbuff_head_cache;
 
 	if (sk_memalloc_socks() && (flags & SKB_ALLOC_RX))
-		gfp_mask |= __GFP_MEMALLOC;
+		gfp_mask |= __GFP_MEMALLOC | __GFP_NOWARN;
 
 	/* Get the HEAD */
 	skb = kmem_cache_alloc_node(cache, gfp_mask & ~__GFP_DMA, node);
-	if (!skb)
+	if (!skb) {
+		if (warn_no_skb)
+			printk_ratelimited(KERN_DEBUG "%s: OOM from %pF via %pF\n",
+					   __func__,
+					   __builtin_return_address(0),
+					   __builtin_return_address(1));
 		goto out;
+	}
 	prefetchw(skb);
 
 	/* We do our best to align skb_shared_info on a separate cache



^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [Patch v2] skbuff: Hide GFP_ATOMIC page allocation failures for dropped packets
  2013-05-27 17:39 ` Rik van Riel
@ 2013-05-27 22:25   ` Eric Dumazet
  2013-05-28  4:31     ` Joe Perches
  2013-05-29  7:36     ` Rik van Riel
  0 siblings, 2 replies; 23+ messages in thread
From: Eric Dumazet @ 2013-05-27 22:25 UTC (permalink / raw)
  To: Rik van Riel
  Cc: atomlin, netdev, davem, edumazet, pshelar, mst,
	alexander.h.duyck, aquini, sergei.shtylyov, linux-kernel

On Mon, 2013-05-27 at 13:39 -0400, Rik van Riel wrote:
> On 05/26/2013 04:19 PM, atomlin@redhat.com wrote:
> > From: Aaron Tomlin <atomlin@redhat.com>
> >
> > Since v1:
> >   - Removed unnecessary parentheses
> >
> > ---8<---
> >
> > Failed GFP_ATOMIC allocations by the network stack result in dropped
> > packets, which will be received on a subsequent retransmit, and an
> > unnecessary, noisy warning with a kernel backtrace.

This claim is wrong, only some protocols deal with retransmits.

> >
> > These warnings are harmless, but they still cause users to panic and
> > file bug reports over dropped packets. It would be better to hide the
> > failed allocation warnings and backtraces, and let retransmits handle
> > dropped packets quietly.
> 
> Yes please. Getting memory management bug reports for
> dropped network packets got old years ago.  Lets get
> rid of those messages.

I am only wondering why this path has anything needing special
attention, over thousands of kmalloc() like call sites in the kernel.

If mm allocation warnings are useless, just make __GFP_NOWARN the
default, and save us thousand of patches (adding the __GFP_NOWARN
everywhere)

Truth is : some network drivers don't deal very well with allocation
errors. mlx4 for example absolutely wants order-2 pages in RX path, with
no fallback to order-0 pages.

So I am not against this patch, but I can not really acknowledge it,
sorry.




^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Patch v2] skbuff: Hide GFP_ATOMIC page allocation failures for dropped packets
  2013-05-26 20:19 atomlin
  2013-05-27 17:39 ` Rik van Riel
@ 2013-05-27 17:56 ` Sergei Shtylyov
  1 sibling, 0 replies; 23+ messages in thread
From: Sergei Shtylyov @ 2013-05-27 17:56 UTC (permalink / raw)
  To: atomlin
  Cc: netdev, davem, edumazet, pshelar, mst, alexander.h.duyck, riel,
	aquini, linux-kernel

Hello.

On 27-05-2013 0:19, atomlin@redhat.com wrote:

> From: Aaron Tomlin <atomlin@redhat.com>

> Since v1:
>   - Removed unnecessary parentheses

    The "changes since version X" section should typically go after --- 
tearline, else the mainatiner will have to edit your patch before applying.

> ---8<---

> Failed GFP_ATOMIC allocations by the network stack result in dropped
> packets, which will be received on a subsequent retransmit, and an
> unnecessary, noisy warning with a kernel backtrace.

> These warnings are harmless, but they still cause users to panic and
> file bug reports over dropped packets. It would be better to hide the
> failed allocation warnings and backtraces, and let retransmits handle
> dropped packets quietly.

> Signed-off-by: Aaron Tomlin <atomlin@redhat.com>

WBR, Sergei


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Patch v2] skbuff: Hide GFP_ATOMIC page allocation failures for dropped packets
  2013-05-26 20:19 atomlin
@ 2013-05-27 17:39 ` Rik van Riel
  2013-05-27 22:25   ` Eric Dumazet
  2013-05-27 17:56 ` Sergei Shtylyov
  1 sibling, 1 reply; 23+ messages in thread
From: Rik van Riel @ 2013-05-27 17:39 UTC (permalink / raw)
  To: atomlin
  Cc: netdev, davem, edumazet, pshelar, mst, alexander.h.duyck, aquini,
	sergei.shtylyov, linux-kernel

On 05/26/2013 04:19 PM, atomlin@redhat.com wrote:
> From: Aaron Tomlin <atomlin@redhat.com>
>
> Since v1:
>   - Removed unnecessary parentheses
>
> ---8<---
>
> Failed GFP_ATOMIC allocations by the network stack result in dropped
> packets, which will be received on a subsequent retransmit, and an
> unnecessary, noisy warning with a kernel backtrace.
>
> These warnings are harmless, but they still cause users to panic and
> file bug reports over dropped packets. It would be better to hide the
> failed allocation warnings and backtraces, and let retransmits handle
> dropped packets quietly.

Yes please. Getting memory management bug reports for
dropped network packets got old years ago.  Lets get
rid of those messages.

> Signed-off-by: Aaron Tomlin <atomlin@redhat.com>

Reviewed-by: Rik van Riel <riel@redhat.com>


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Patch v2] skbuff: Hide GFP_ATOMIC page allocation failures for dropped packets
@ 2013-05-26 20:19 atomlin
  2013-05-27 17:39 ` Rik van Riel
  2013-05-27 17:56 ` Sergei Shtylyov
  0 siblings, 2 replies; 23+ messages in thread
From: atomlin @ 2013-05-26 20:19 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, pshelar, mst, alexander.h.duyck, riel, aquini,
	sergei.shtylyov, linux-kernel, Aaron Tomlin

From: Aaron Tomlin <atomlin@redhat.com>

Since v1:
 - Removed unnecessary parentheses

---8<---

Failed GFP_ATOMIC allocations by the network stack result in dropped
packets, which will be received on a subsequent retransmit, and an
unnecessary, noisy warning with a kernel backtrace.

These warnings are harmless, but they still cause users to panic and
file bug reports over dropped packets. It would be better to hide the
failed allocation warnings and backtraces, and let retransmits handle
dropped packets quietly.

Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
---
 net/core/skbuff.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 02139d6..84aa870 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -236,7 +236,7 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t gfp_mask,
 		? skbuff_fclone_cache : skbuff_head_cache;
 
 	if (sk_memalloc_socks() && (flags & SKB_ALLOC_RX))
-		gfp_mask |= (__GFP_MEMALLOC|__GFP_NOWARN);
+		gfp_mask |= __GFP_MEMALLOC | __GFP_NOWARN;
 
 	/* Get the HEAD */
 	skb = kmem_cache_alloc_node(cache, gfp_mask & ~__GFP_DMA, node);
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2013-05-29  7:37 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-26 20:45 [Patch v2] skbuff: Hide GFP_ATOMIC page allocation failures for dropped packets atomlin
2013-05-27 18:18 ` Rafael Aquini
2013-05-27 22:41 ` Francois Romieu
2013-05-28 16:00   ` Ben Greear
2013-05-28 16:15     ` Rafael Aquini
2013-05-28 16:19       ` Ben Greear
2013-05-28 16:29       ` Eric Dumazet
2013-05-28 17:43         ` Rafael Aquini
2013-05-28 17:46           ` Eric Dumazet
2013-05-28 18:03           ` Eric Dumazet
2013-05-28 18:19         ` Joe Perches
2013-05-28 21:32         ` Rik van Riel
2013-05-28 17:29     ` Joe Perches
2013-05-28 21:30   ` Rik van Riel
  -- strict thread matches above, loose matches on Subject: below --
2013-05-26 20:19 atomlin
2013-05-27 17:39 ` Rik van Riel
2013-05-27 22:25   ` Eric Dumazet
2013-05-28  4:31     ` Joe Perches
2013-05-28  6:04       ` Eric Dumazet
2013-05-28  6:08         ` Joe Perches
2013-05-28 17:15       ` Debabrata Banerjee
2013-05-29  7:36     ` Rik van Riel
2013-05-27 17:56 ` Sergei Shtylyov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.