GFP_ATOMIC page allocation failures.
Subsystems (2)
I found a few ways to cause pages and pages of spew to dmesg of the following form..
rhythmbox: page allocation failure. order:3, mode:0x4020
Pid: 4299, comm: rhythmbox Not tainted 2.6.25-0.172.rc7.git4.fc9.x86_64 #1
Call Trace: <IRQ> [<ffffffff810862dc>] __alloc_pages+0x3a3/0x3c3 [<ffffffff812a58df>] ? trace_hardirqs_on_thunk+0x35/0x3a [<ffffffff8109fd94>] alloc_pages_current+0x100/0x109 [<ffffffff810a6fd5>] new_slab+0x4a/0x249 [<ffffffff810a776a>] __slab_alloc+0x251/0x4e0 [<ffffffff8121c322>] ? __netdev_alloc_skb+0x31/0x4f [<ffffffff810a8736>] __kmalloc_node_track_caller+0x8a/0xe2 [<ffffffff8121c322>] ? __netdev_alloc_skb+0x31/0x4f [<ffffffff8121b5db>] __alloc_skb+0x6f/0x135 [<ffffffff8121c322>] __netdev_alloc_skb+0x31/0x4f [<ffffffff8814e5b4>] :e1000e:e1000_alloc_rx_buffers+0xb7/0x1dc [<ffffffff8814eada>] :e1000e:e1000_clean_rx_irq+0x271/0x307 [<ffffffff8814c71a>] :e1000e:e1000_clean+0x66/0x205 [<ffffffff8121eeb8>] net_rx_action+0xd9/0x20e [<ffffffff81038757>] __do_softirq+0x70/0xf1 [<ffffffff8100d25c>] call_softirq+0x1c/0x28 [<ffffffff8100e485>] do_softirq+0x39/0x8a [<ffffffff81038290>] irq_exit+0x4e/0x8f [<ffffffff8100e781>] do_IRQ+0x145/0x167 [<ffffffff8100c5e6>] ret_from_intr+0x0/0xf <EOI> [<ffffffff812a5ed8>] ? _spin_unlock_irqrestore+0x42/0x47 [<ffffffff8102a040>] ? __wake_up+0x43/0x50 [<ffffffff81056b7f>] ? wake_futex+0x47/0x53 [<ffffffff810584cf>] ? do_futex+0x697/0xc57 [<ffffffff8102fbc4>] ? hrtick_set+0xa1/0xfc [<ffffffff81058b84>] ? sys_futex+0xf5/0x113 [<ffffffff810133e7>] ? syscall_trace_enter+0xb5/0xb9 [<ffffffff8100c1d0>] ? tracesys+0xd5/0xda
Given that we seem to recover from these events without negative effects (ie, no apps get oom-killed), is there any value to actually flooding syslog with this stuff ?
iirc, SuSE have been patching out these traces for GFP_ATOMIC allocations for a long time. Should mainline do the same ?
Dave
Signature · 1 line
http://www.codemonkey.org.uk
Quoted 37 lines · Dave Jones
I found a few ways to cause pages and pages of spew to dmesg of the following form..
rhythmbox: page allocation failure. order:3, mode:0x4020
Pid: 4299, comm: rhythmbox Not tainted 2.6.25-0.172.rc7.git4.fc9.x86_64 #1Call Trace: <IRQ> [<ffffffff810862dc>] __alloc_pages+0x3a3/0x3c3 [<ffffffff812a58df>] ? trace_hardirqs_on_thunk+0x35/0x3a [<ffffffff8109fd94>] alloc_pages_current+0x100/0x109 [<ffffffff810a6fd5>] new_slab+0x4a/0x249 [<ffffffff810a776a>] __slab_alloc+0x251/0x4e0 [<ffffffff8121c322>] ? __netdev_alloc_skb+0x31/0x4f [<ffffffff810a8736>] __kmalloc_node_track_caller+0x8a/0xe2 [<ffffffff8121c322>] ? __netdev_alloc_skb+0x31/0x4f [<ffffffff8121b5db>] __alloc_skb+0x6f/0x135 [<ffffffff8121c322>] __netdev_alloc_skb+0x31/0x4f [<ffffffff8814e5b4>] :e1000e:e1000_alloc_rx_buffers+0xb7/0x1dc [<ffffffff8814eada>] :e1000e:e1000_clean_rx_irq+0x271/0x307 [<ffffffff8814c71a>] :e1000e:e1000_clean+0x66/0x205 [<ffffffff8121eeb8>] net_rx_action+0xd9/0x20e [<ffffffff81038757>] __do_softirq+0x70/0xf1 [<ffffffff8100d25c>] call_softirq+0x1c/0x28 [<ffffffff8100e485>] do_softirq+0x39/0x8a [<ffffffff81038290>] irq_exit+0x4e/0x8f [<ffffffff8100e781>] do_IRQ+0x145/0x167 [<ffffffff8100c5e6>] ret_from_intr+0x0/0xf <EOI> [<ffffffff812a5ed8>] ? _spin_unlock_irqrestore+0x42/0x47 [<ffffffff8102a040>] ? __wake_up+0x43/0x50 [<ffffffff81056b7f>] ? wake_futex+0x47/0x53 [<ffffffff810584cf>] ? do_futex+0x697/0xc57 [<ffffffff8102fbc4>] ? hrtick_set+0xa1/0xfc [<ffffffff81058b84>] ? sys_futex+0xf5/0x113 [<ffffffff810133e7>] ? syscall_trace_enter+0xb5/0xb9 [<ffffffff8100c1d0>] ? tracesys+0xd5/0xda
Quoted 3 lines
Given that we seem to recover from these events without negative effects (ie, no apps get oom-killed), is there any value to actually flooding syslog with this stuff ?
It's nice to have. Perhaps it could just be hardlimited to print say 10 times, and maybe we could have a vmstat counter to keep count after that.
Quoted 37 lines · Nick Piggin
Quoted 36 lines · Dave Jones
I found a few ways to cause pages and pages of spew to dmesg of the following form..
rhythmbox: page allocation failure. order:3, mode:0x4020
Pid: 4299, comm: rhythmbox Not tainted 2.6.25-0.172.rc7.git4.fc9.x86_64 #1Call Trace: <IRQ> [<ffffffff810862dc>] __alloc_pages+0x3a3/0x3c3 [<ffffffff812a58df>] ? trace_hardirqs_on_thunk+0x35/0x3a [<ffffffff8109fd94>] alloc_pages_current+0x100/0x109 [<ffffffff810a6fd5>] new_slab+0x4a/0x249 [<ffffffff810a776a>] __slab_alloc+0x251/0x4e0 [<ffffffff8121c322>] ? __netdev_alloc_skb+0x31/0x4f [<ffffffff810a8736>] __kmalloc_node_track_caller+0x8a/0xe2 [<ffffffff8121c322>] ? __netdev_alloc_skb+0x31/0x4f [<ffffffff8121b5db>] __alloc_skb+0x6f/0x135 [<ffffffff8121c322>] __netdev_alloc_skb+0x31/0x4f [<ffffffff8814e5b4>] :e1000e:e1000_alloc_rx_buffers+0xb7/0x1dc [<ffffffff8814eada>] :e1000e:e1000_clean_rx_irq+0x271/0x307 [<ffffffff8814c71a>] :e1000e:e1000_clean+0x66/0x205 [<ffffffff8121eeb8>] net_rx_action+0xd9/0x20e [<ffffffff81038757>] __do_softirq+0x70/0xf1 [<ffffffff8100d25c>] call_softirq+0x1c/0x28 [<ffffffff8100e485>] do_softirq+0x39/0x8a [<ffffffff81038290>] irq_exit+0x4e/0x8f [<ffffffff8100e781>] do_IRQ+0x145/0x167 [<ffffffff8100c5e6>] ret_from_intr+0x0/0xf <EOI> [<ffffffff812a5ed8>] ? _spin_unlock_irqrestore+0x42/0x47 [<ffffffff8102a040>] ? __wake_up+0x43/0x50 [<ffffffff81056b7f>] ? wake_futex+0x47/0x53 [<ffffffff810584cf>] ? do_futex+0x697/0xc57 [<ffffffff8102fbc4>] ? hrtick_set+0xa1/0xfc [<ffffffff81058b84>] ? sys_futex+0xf5/0x113 [<ffffffff810133e7>] ? syscall_trace_enter+0xb5/0xb9 [<ffffffff8100c1d0>] ? tracesys+0xd5/0xda
Quoted 7 lines
Quoted 3 lines
Given that we seem to recover from these events without negative effects (ie, no apps get oom-killed), is there any value to actually flooding syslog with this stuff ?
It's nice to have. Perhaps it could just be hardlimited to print say 10 times, and maybe we could have a vmstat counter to keep count after that.
As an end-user, that's still 10 times too many.
What is anyone expect to do with these traces ?
multi-page atomic allocations fail sometimes, we shouldn't be surprised by this. As long as the code that tries to do them is aware of this, is there a problem ?
Dave
Signature · 1 line
http://www.codemonkey.org.uk
Quoted 46 lines · Dave Jones
Quoted 37 lines · Nick Piggin
Quoted 36 lines · Dave Jones
I found a few ways to cause pages and pages of spew to dmesg of the following form..
rhythmbox: page allocation failure. order:3, mode:0x4020
Pid: 4299, comm: rhythmbox Not tainted 2.6.25-0.172.rc7.git4.fc9.x86_64 #1Call Trace: <IRQ> [<ffffffff810862dc>] __alloc_pages+0x3a3/0x3c3 [<ffffffff812a58df>] ? trace_hardirqs_on_thunk+0x35/0x3a [<ffffffff8109fd94>] alloc_pages_current+0x100/0x109 [<ffffffff810a6fd5>] new_slab+0x4a/0x249 [<ffffffff810a776a>] __slab_alloc+0x251/0x4e0 [<ffffffff8121c322>] ? __netdev_alloc_skb+0x31/0x4f [<ffffffff810a8736>] __kmalloc_node_track_caller+0x8a/0xe2 [<ffffffff8121c322>] ? __netdev_alloc_skb+0x31/0x4f [<ffffffff8121b5db>] __alloc_skb+0x6f/0x135 [<ffffffff8121c322>] __netdev_alloc_skb+0x31/0x4f [<ffffffff8814e5b4>] :e1000e:e1000_alloc_rx_buffers+0xb7/0x1dc [<ffffffff8814eada>] :e1000e:e1000_clean_rx_irq+0x271/0x307 [<ffffffff8814c71a>] :e1000e:e1000_clean+0x66/0x205 [<ffffffff8121eeb8>] net_rx_action+0xd9/0x20e [<ffffffff81038757>] __do_softirq+0x70/0xf1 [<ffffffff8100d25c>] call_softirq+0x1c/0x28 [<ffffffff8100e485>] do_softirq+0x39/0x8a [<ffffffff81038290>] irq_exit+0x4e/0x8f [<ffffffff8100e781>] do_IRQ+0x145/0x167 [<ffffffff8100c5e6>] ret_from_intr+0x0/0xf <EOI> [<ffffffff812a5ed8>] ? _spin_unlock_irqrestore+0x42/0x47 [<ffffffff8102a040>] ? __wake_up+0x43/0x50 [<ffffffff81056b7f>] ? wake_futex+0x47/0x53 [<ffffffff810584cf>] ? do_futex+0x697/0xc57 [<ffffffff8102fbc4>] ? hrtick_set+0xa1/0xfc [<ffffffff81058b84>] ? sys_futex+0xf5/0x113 [<ffffffff810133e7>] ? syscall_trace_enter+0xb5/0xb9 [<ffffffff8100c1d0>] ? tracesys+0xd5/0xdaQuoted 7 lines
Quoted 3 lines
Given that we seem to recover from these events without negative effects (ie, no apps get oom-killed), is there any value to actually flooding syslog with this stuff ?
It's nice to have. Perhaps it could just be hardlimited to print say 10 times, and maybe we could have a vmstat counter to keep count after that.
Quoted 9 lines
As an end-user, that's still 10 times too many.
What is anyone expect to do with these traces ?multi-page atomic allocations fail sometimes, we shouldn't be surprised by this. As long as the code that tries to do them is aware of this, is there a problem ?
Dave
I agree that this spew is quite excessive, but it's there for a reason.
Some code does *not* handle this failure gracefully, and may put the
machine in a state where it is subsequently unable to report/log errors from the calling code. If that happens, I'd like to see some sort of dying gasp.
Limiting this to once per boot should suffice for debugging purposes. Even if you manage to concoct a bug that always survives the first failure, you should be able to take the hint when you keep seeing this in dmesg.
-- Chris
Quoted 63 lines · On Wed, 02 Apr 2008 02:28:25 -0400 Chris Snook <[email protected]> wrote:
Quoted 46 lines · Dave Jones
Quoted 37 lines · Nick Piggin
Quoted 36 lines · Dave Jones
I found a few ways to cause pages and pages of spew to dmesg of the following form..
rhythmbox: page allocation failure. order:3, mode:0x4020
Pid: 4299, comm: rhythmbox Not tainted 2.6.25-0.172.rc7.git4.fc9.x86_64 #1Call Trace: <IRQ> [<ffffffff810862dc>] __alloc_pages+0x3a3/0x3c3 [<ffffffff812a58df>] ? trace_hardirqs_on_thunk+0x35/0x3a [<ffffffff8109fd94>] alloc_pages_current+0x100/0x109 [<ffffffff810a6fd5>] new_slab+0x4a/0x249 [<ffffffff810a776a>] __slab_alloc+0x251/0x4e0 [<ffffffff8121c322>] ? __netdev_alloc_skb+0x31/0x4f [<ffffffff810a8736>] __kmalloc_node_track_caller+0x8a/0xe2 [<ffffffff8121c322>] ? __netdev_alloc_skb+0x31/0x4f [<ffffffff8121b5db>] __alloc_skb+0x6f/0x135 [<ffffffff8121c322>] __netdev_alloc_skb+0x31/0x4f [<ffffffff8814e5b4>] :e1000e:e1000_alloc_rx_buffers+0xb7/0x1dc [<ffffffff8814eada>] :e1000e:e1000_clean_rx_irq+0x271/0x307 [<ffffffff8814c71a>] :e1000e:e1000_clean+0x66/0x205 [<ffffffff8121eeb8>] net_rx_action+0xd9/0x20e [<ffffffff81038757>] __do_softirq+0x70/0xf1 [<ffffffff8100d25c>] call_softirq+0x1c/0x28 [<ffffffff8100e485>] do_softirq+0x39/0x8a [<ffffffff81038290>] irq_exit+0x4e/0x8f [<ffffffff8100e781>] do_IRQ+0x145/0x167 [<ffffffff8100c5e6>] ret_from_intr+0x0/0xf <EOI> [<ffffffff812a5ed8>] ? _spin_unlock_irqrestore+0x42/0x47 [<ffffffff8102a040>] ? __wake_up+0x43/0x50 [<ffffffff81056b7f>] ? wake_futex+0x47/0x53 [<ffffffff810584cf>] ? do_futex+0x697/0xc57 [<ffffffff8102fbc4>] ? hrtick_set+0xa1/0xfc [<ffffffff81058b84>] ? sys_futex+0xf5/0x113 [<ffffffff810133e7>] ? syscall_trace_enter+0xb5/0xb9 [<ffffffff8100c1d0>] ? tracesys+0xd5/0xdaQuoted 7 lines
Quoted 3 lines
Given that we seem to recover from these events without negative effects (ie, no apps get oom-killed), is there any value to actually flooding syslog with this stuff ?
It's nice to have. Perhaps it could just be hardlimited to print say 10 times, and maybe we could have a vmstat counter to keep count after that.
Quoted 9 lines
As an end-user, that's still 10 times too many.
What is anyone expect to do with these traces ?multi-page atomic allocations fail sometimes, we shouldn't be surprised by this. As long as the code that tries to do them is aware of this, is there a problem ?
Dave
I agree that this spew is quite excessive, but it's there for a reason.
Some code does *not* handle this failure gracefully, and may put the
machine in a state where it is subsequently unable to report/log errors from the calling code. If that happens, I'd like to see some sort of dying gasp.
Quoted 4 lines
Limiting this to once per boot should suffice for debugging purposes. Even if you manage to concoct a bug that always survives the first failure, you should be able to take the hint when you keep seeing this in dmesg.
The appropriate thing to do here is to convert known-good drivers (such as e1000[e]) to use __GFP_NOWARN.
Unfortunately netdev_alloc_skb() went and assumed GFP_ATOMIC, but I guess we can dive below the covers and use __netdev_alloc_skb():
From: Andrew Morton <[email protected]>
We get rather a lot of reports of page allocation warnings coming out of e1000. But this driver is know to handle them properly so let's suppress them.
---
drivers/net/e1000/e1000.h | 4 ++++ drivers/net/e1000/e1000_main.c | 10 +++++----- 2 files changed, 9 insertions(+), 5 deletions(-)
drivers/net/e1000/e1000_main.c renamed from drivers/net/e1000/e1000_main.c~e1000-suppress-page-allocation-failure-warnings +5 -5
@@ -4296,7 +4296,7 @@ e1000_clean_rx_irq(struct e1000_adapter* of reassembly being done in the stack */ if (length < copybreak) {struct sk_buff *new_skb = -netdev_alloc_skb (netdev, length +NET_IP_ALIGN ); +e1000_alloc_skb (netdev, length +NET_IP_ALIGN );if (new_skb) {skb_reserve (new_skb,NET_IP_ALIGN ); skb_copy_to_linear_data_offset(new_skb,
@@ -4585,7 +4585,7 @@ e1000_alloc_rx_buffers(struct e1000_adap goto map_skb; } - skb =netdev_alloc_skb (netdev, bufsz); + skb =e1000_alloc_skb (netdev, bufsz);if (unlikely (!skb)) {/* Better luck next round */ adapter->alloc_rx_buff_failed++;
@@ -4598,7 +4598,7 @@ e1000_alloc_rx_buffers(struct e1000_adapDPRINTK (RX_ERR ,ERR ,"skb align check failed: %u bytes " "at %p\n" , bufsz, skb->data);/* Try again, without freeing the previous */ - skb =netdev_alloc_skb (netdev, bufsz); + skb =e1000_alloc_skb (netdev, bufsz);/* Failed allocation, critical failure */ if (!skb) {dev_kfree_skb (oldskb);
@@ -4720,8 +4720,8 @@ e1000_alloc_rx_buffers_ps(struct e1000_a rx_desc->read.buffer_addr[j+1 ] = ~cpu_to_le64 (0 ); } - skb =netdev_alloc_skb (netdev, - adapter->rx_ps_bsize0 +NET_IP_ALIGN ); + skb =e1000_alloc_skb (netdev, + adapter->rx_ps_bsize0 +NET_IP_ALIGN );if (unlikely (!skb)) { adapter->alloc_rx_buff_failed++;
drivers/net/e1000/e1000.h renamed from drivers/net/e1000/e1000.h~e1000-suppress-page-allocation-failure-warnings +4 -0
@@ -358,5 +358,9 @@ extern void e1000_power_up_phy(struct e1extern void e1000_set_ethtool_ops (struct net_device *netdev);extern void e1000_check_options (struct e1000_adapter *adapter); +static inline void *e1000_alloc_skb (struct net_device *dev,unsigned int length) +{ +return __netdev_alloc_skb (dev, length,GFP_ATOMIC |__GFP_NOWARN); +}#endif /* _E1000_H_ */
_
Hi
CC'ed netdev
Quoted 5 lines
The appropriate thing to do here is to convert known-good drivers (such as e1000[e]) to use
__GFP_NOWARN.Unfortunately
netdev_alloc_skb()went and assumedGFP_ATOMIC, but I guess we can dive below the covers and use__netdev_alloc_skb():
if network guys hope known-good driver should call __netdev_alloc_skb(dev, length, GFP_ATOMIC|__GFP_NOWARN) instead netdev_alloc_skb, I think we should make netdev_alloc_skb_nowarn.
static inline void *netdev_alloc_skb_nowarn(struct net_device *dev, unsigned int length)
{
return __netdev_alloc_skb(dev, length, GFP_ATOMIC|__GFP_NOWARN);
}if not, we make unnecessary various alloc_skb (e.g. e1000_alloc_skb, tg3_alloc_skb, 3com_alloc_skb etc..)
Quoted 71 lines
From: Andrew Morton <[email protected]>
We get rather a lot of reports of page allocation warnings coming out of e1000. But this driver is know to handle them properly so let's suppress them.
Cc Auke Kok <[email protected]>Cc Jesse Brandeburg <[email protected]>Cc Jeff Garzik <[email protected]>Signed-off-by Andrew Morton <[email protected]>---
drivers/net/e1000/e1000.h | 4 ++++ drivers/net/e1000/e1000_main.c | 10 +++++----- 2 files changed, 9 insertions(+), 5 deletions(-)diff -puN
drivers/net/e1000/e1000_main.c~e1000-suppress-page-allocation-failure-warningsdrivers/net/e1000/e1000_main.cdrivers/net/e1000/e1000_main.c renamed from drivers/net/e1000/e1000_main.c~e1000-suppress-page-allocation-failure-warnings +5
-5@@ -4296,7 +4296,7 @@ e1000_clean_rx_irq(struct e1000_adapter* of reassembly being done in the stack */ if (length < copybreak) {struct sk_buff *new_skb = -netdev_alloc_skb (netdev, length +NET_IP_ALIGN ); +e1000_alloc_skb (netdev, length +NET_IP_ALIGN );if (new_skb) {skb_reserve (new_skb,NET_IP_ALIGN ); skb_copy_to_linear_data_offset(new_skb,@@ -4585,7 +4585,7 @@ e1000_alloc_rx_buffers(struct e1000_adap goto map_skb; } - skb =netdev_alloc_skb (netdev, bufsz); + skb =e1000_alloc_skb (netdev, bufsz);if (unlikely (!skb)) {/* Better luck next round */ adapter->alloc_rx_buff_failed++;@@ -4598,7 +4598,7 @@ e1000_alloc_rx_buffers(struct e1000_adapDPRINTK (RX_ERR ,ERR ,"skb align check failed: %u bytes " "at %p\n" , bufsz, skb->data);/* Try again, without freeing the previous */ - skb =netdev_alloc_skb (netdev, bufsz); + skb =e1000_alloc_skb (netdev, bufsz);/* Failed allocation, critical failure */ if (!skb) {dev_kfree_skb (oldskb);@@ -4720,8 +4720,8 @@ e1000_alloc_rx_buffers_ps(struct e1000_a rx_desc->read.buffer_addr[j+1 ] = ~cpu_to_le64 (0 ); } - skb =netdev_alloc_skb (netdev, - adapter->rx_ps_bsize0 +NET_IP_ALIGN ); + skb =e1000_alloc_skb (netdev, + adapter->rx_ps_bsize0 +NET_IP_ALIGN );if (unlikely (!skb)) { adapter->alloc_rx_buff_failed++;diff -puN
drivers/net/e1000/e1000.h~e1000-suppress-page-allocation-failure-warningsdrivers/net/e1000/e1000.hdrivers/net/e1000/e1000.h renamed from drivers/net/e1000/e1000.h~e1000-suppress-page-allocation-failure-warnings +4
-0@@ -358,5 +358,9 @@ extern void e1000_power_up_phy(struct e1extern void e1000_set_ethtool_ops (struct net_device *netdev);extern void e1000_check_options (struct e1000_adapter *adapter); +static inline void *e1000_alloc_skb (struct net_device *dev,unsigned int length) +{ +return __netdev_alloc_skb (dev, length,GFP_ATOMIC |__GFP_NOWARN); +}#endif /* _E1000_H_ */ _
David Miller
maintainerFrom: KOSAKI Motohiro <[email protected]>
Date: Wed, 02 Apr 2008 17:17:50 +0900
Quoted 3 lines
if network guys hope known-good driver should call
__netdev_alloc_skb(dev, length, GFP_ATOMIC|__GFP_NOWARN)insteadnetdev_alloc_skb, I think we should makenetdev_alloc_skb_nowarn.
Giving it a proper name like this takes away the indication that this situation is very special.
Two leading underscores to an interface means "something special and unusual requiring more careful consideration than usual is occuring here." netdev_alloc_skb_nowarn() on the other hand, does not convey this meaning.
And we will have very few drivers that use this construct, thus it really is best to handle things the way Andrew has.
Quoted 10 lines · On Wed, 02 Apr 2008 01:24:06 -0700 (PDT) David Miller <[email protected]>…
From: KOSAKI Motohiro <[email protected]>
Date: Wed, 02 Apr 2008 17:17:50 +0900Quoted 3 lines
if network guys hope known-good driver should call
__netdev_alloc_skb(dev, length, GFP_ATOMIC|__GFP_NOWARN)insteadnetdev_alloc_skb, I think we should makenetdev_alloc_skb_nowarn.Giving it a proper name like this takes away the indication that this situation is very special.
Quoted 7 lines
Two leading underscores to an interface means "something special and unusual requiring more careful consideration than usual is occuring here."
netdev_alloc_skb_nowarn()on the other hand, does not convey this meaning.And we will have very few drivers that use this construct, thus it really is best to handle things the way Andrew has.
Oh. I just went and redid thing the other way. Picked the same name too.
Whatever.
Hi
Quoted 6 lines
Quoted 3 lines
if network guys hope known-good driver should call
__netdev_alloc_skb(dev, length, GFP_ATOMIC|__GFP_NOWARN)insteadnetdev_alloc_skb, I think we should makenetdev_alloc_skb_nowarn.Giving it a proper name like this takes away the indication that this situation is very special.
Hmmm,
at least, tg3 driver often output the same message on my test environment. (because I often run stress test ;)
I doubt almost driver has the same problem under heavy workload.
and I think tg3 is known-good driver too.
because its alloc failure doesn't cause any bad thing at all.
Do you favorite the following patch?
---------------------------------------------------------------------
We get rather a lot of reports of page allocation warnings coming out of tg3. But this driver is know to handle them properly so let's suppress them.
--- drivers/net/tg3.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
drivers/net/tg3.c 2008-04-02 18:54:15.000000000 +0900 renamed from drivers/net/tg3.c 2008-03-14 21:38:12.000000000 +0900 +8 -3
@@ -307,6 +307,11 @@ static const struct { { "interrupt test (offline)" }, }; +static inline void *tg3_alloc_skb(struct net_device *dev, unsigned int length) +{ + return __netdev_alloc_skb(dev, length, GFP_ATOMIC|__GFP_NOWARN); +} + static void tg3_write32(struct tg3 *tp, u32 off, u32 val) { writel(val, tp->regs + off);
@@ -3437,7 +3442,7 @@ static int tg3_alloc_rx_skb(struct tg3 * * Callers depend upon this behavior and assume that * we leave everything unchanged if we fail. */ - skb = netdev_alloc_skb(tp->dev, skb_size); + skb = tg3_alloc_skb(tp->dev, skb_size); if (skb == NULL) return -ENOMEM;
@@ -3609,7 +3614,7 @@ static int tg3_rx(struct tg3 *tp, int bu tg3_recycle_rx(tp, opaque_key, desc_idx, *post_ptr); - copy_skb = netdev_alloc_skb(tp->dev, len + 2); + copy_skb = tg3_alloc_skb(tp->dev, len + 2); if (copy_skb == NULL) goto drop_it_no_recycle;
@@ -9370,7 +9375,7 @@ static int tg3_run_loopback(struct tg3 * err = -EIO; tx_len = 1514; - skb = netdev_alloc_skb(tp->dev, tx_len); + skb = tg3_alloc_skb(tp->dev, tx_len); if (!skb) return -ENOMEM;
Quoted 5 lines
---------------------------------------------------------------------
We get rather a lot of reports of page allocation warnings coming out of tg3. But this driver is know to handle them properly so let's suppress them.
BTW:
other person's tg3 allocation failure bug report.
David Miller
maintainerFrom: KOSAKI Motohiro <[email protected]>
Date: Wed, 02 Apr 2008 19:56:05 +0900
Quoted 10 lines
Quoted 5 lines
---------------------------------------------------------------------
We get rather a lot of reports of page allocation warnings coming out of tg3. But this driver is know to handle them properly so let's suppress them.BTW:
other person's tg3 allocation failure bug report.
Yes, I know about it. It will occur for any device which must use linear RX buffers and supports
Quoted 1 line
PAGE_SIZEMTUs.
But modern chips are going to support segmentation of receive packets into multiple individual pages and therefore not have this order>0 page allocation requirement.
That is my point.
Quoted 8 lines · David Miller
From: KOSAKI Motohiro <[email protected]>
Date: Wed, 02 Apr 2008 19:56:05 +0900Quoted 4 lines
BTW:
other person's tg3 allocation failure bug report.
Quoted 8 lines
Yes, I know about it. It will occur for any device which must use linear RX buffers and supports
Quoted 1 line
PAGE_SIZEMTUs.But modern chips are going to support segmentation of receive packets into multiple individual pages and therefore not have this order>0 page allocation requirement.
tg3 hardware has a jumbo rx ring that can accommodate non-linear SKBs. Matt is working on some patches to add this feature.
David Miller
maintainerFrom: "Michael Chan" <[email protected]>
Date: Wed, 02 Apr 2008 12:12:06 -0800
Quoted 2 lines
tg3 hardware has a jumbo rx ring that can accommodate non-linear SKBs. Matt is working on some patches to add this feature.
Excellent.
Quoted 10 lines · David Miller
From: KOSAKI Motohiro <[email protected]>
Date: Wed, 02 Apr 2008 17:17:50 +0900Quoted 3 lines
if network guys hope known-good driver should call
__netdev_alloc_skb(dev, length, GFP_ATOMIC|__GFP_NOWARN)insteadnetdev_alloc_skb, I think we should makenetdev_alloc_skb_nowarn.Giving it a proper name like this takes away the indication that this situation is very special.
Quoted 7 lines
Two leading underscores to an interface means "something special and unusual requiring more careful consideration than usual is occuring here."
netdev_alloc_skb_nowarn()on the other hand, does not convey this meaning.And we will have very few drivers that use this construct, thus it really is best to handle things the way Andrew has.
Would we not hope that most net drivers can handle {,net}dev_alloc_skb() failing? Otherwise we have some serious trouble.
David Miller
maintainerFrom: Peter Zijlstra <[email protected]>
Date: Wed, 02 Apr 2008 13:04:59 +0200
Quoted 2 lines
Would we not hope that most net drivers can handle {,net}dev_alloc_skb() failing? Otherwise we have some serious trouble.
False presumption.
Most can but some legacy ones really do not handle this well.
Quoted 9 lines · David Miller
From: Peter Zijlstra <[email protected]>
Date: Wed, 02 Apr 2008 13:04:59 +0200Quoted 2 lines
Would we not hope that most net drivers can handle {,net}dev_alloc_skb() failing? Otherwise we have some serious trouble.
False presumption.
Most can but some legacy ones really do not handle this well.
Correct...
In addition, I remain worried about potential edge cases in drivers that do "burst refill" style allocations, rather than the [IMO safer] as-you-go style that tg3 employs.
I haven't done a serious audit, but I have nagging doubts about the behavior of burst-refill drivers when faced with a burst of failed allocations... do they stop DMA operation, or do they permit hardware to wander into part of the DMA ring where, in the previous "cycle" through the ring, valid DMA addresses were placed. But now with a string of allocation failures, you have a sequence of DMA ring descriptors that point to invalid memory. If the DMA engine is not stopped, or in some other way made to avoid those invalid DMA descriptors, then you can easily run into problems.
Sometimes, it's as easy as making sure to properly police the 'OWN' bit of each descriptor, something easily and commonly done as a matter of course. But modern hardware with multi-descriptor packets make things more complex, so it is actually getting more difficult to get these details right.
</ramble>
Jeff
Quoted 10 lines · Andrew Morton
Quoted 4 lines
Limiting this to once per boot should suffice for debugging purposes. Even if you manage to concoct a bug that always survives the first failure, you should be able to take the hint when you keep seeing this in dmesg.
The appropriate thing to do here is to convert known-good drivers (such as e1000[e]) to use
__GFP_NOWARN.Unfortunately
netdev_alloc_skb()went and assumedGFP_ATOMIC, but I guess we can dive below the covers and use__netdev_alloc_skb():
It's still actually nice to know how often it is happening even for these known good sites because too much can indicate a problem and that you could actually bring performance up by tuning some things.
So I think that the messages should stay, and they should print out some header to say that it is only a warning and if not happening too often then it is not a problem, and if it is continually happening then please try X or Y or post a message to lkml...
Quoted 78 lines
From: Andrew Morton <[email protected]>
We get rather a lot of reports of page allocation warnings coming out of e1000. But this driver is know to handle them properly so let's suppress them.
Cc Auke Kok <[email protected]>Cc Jesse Brandeburg <[email protected]>Cc Jeff Garzik <[email protected]>Signed-off-by Andrew Morton <[email protected]>---
drivers/net/e1000/e1000.h | 4 ++++ drivers/net/e1000/e1000_main.c | 10 +++++----- 2 files changed, 9 insertions(+), 5 deletions(-)diff -puN
drivers/net/e1000/e1000_main.c~e1000-suppress-page-allocation-failure-warningsdrivers/net/e1000/e1000_main.c--- a/drivers/net/e1000/e1000_main.c~e1000-suppress-page-allocation-failure-war nings +++a/drivers/net/e1000/e1000_main.cdrivers/net/e1000/e1000_main.c renamed from drivers/net/e1000/e1000_main.c~e1000-suppress-page-allocation-failure-warnings +5
-5@@ -4296,7 +4296,7 @@ e1000_clean_rx_irq(struct e1000_adapter* of reassembly being done in the stack */ if (length < copybreak) {struct sk_buff *new_skb = -netdev_alloc_skb (netdev, length +NET_IP_ALIGN ); +e1000_alloc_skb (netdev, length +NET_IP_ALIGN );if (new_skb) {skb_reserve (new_skb,NET_IP_ALIGN ); skb_copy_to_linear_data_offset(new_skb,@@ -4585,7 +4585,7 @@ e1000_alloc_rx_buffers(struct e1000_adap goto map_skb; } - skb =netdev_alloc_skb (netdev, bufsz); + skb =e1000_alloc_skb (netdev, bufsz);if (unlikely (!skb)) {/* Better luck next round */ adapter->alloc_rx_buff_failed++;@@ -4598,7 +4598,7 @@ e1000_alloc_rx_buffers(struct e1000_adapDPRINTK (RX_ERR ,ERR ,"skb align check failed: %u bytes " "at %p\n" , bufsz, skb->data);/* Try again, without freeing the previous */ - skb =netdev_alloc_skb (netdev, bufsz); + skb =e1000_alloc_skb (netdev, bufsz);/* Failed allocation, critical failure */ if (!skb) {dev_kfree_skb (oldskb);@@ -4720,8 +4720,8 @@ e1000_alloc_rx_buffers_ps(struct e1000_a rx_desc->read.buffer_addr[j+1 ] = ~cpu_to_le64 (0 ); } - skb =netdev_alloc_skb (netdev, - adapter->rx_ps_bsize0 +NET_IP_ALIGN ); + skb =e1000_alloc_skb (netdev, + adapter->rx_ps_bsize0 +NET_IP_ALIGN );if (unlikely (!skb)) { adapter->alloc_rx_buff_failed++;diff -puN
drivers/net/e1000/e1000.h~e1000-suppress-page-allocation-failure-warningsdrivers/net/e1000/e1000.h--- a/drivers/net/e1000/e1000.h~e1000-suppress-page-allocation-failure-warnings +++a/drivers/net/e1000/e1000.hdrivers/net/e1000/e1000.h renamed from drivers/net/e1000/e1000.h~e1000-suppress-page-allocation-failure-warnings +3
-0@@ -358,5 +358,9 @@ extern void e1000_power_up_phy(struct e1extern void e1000_set_ethtool_ops (struct net_device *netdev);extern void e1000_check_options (struct e1000_adapter *adapter); +static inline void *e1000_alloc_skb (struct net_device *dev,unsigned int length) +{ +return __netdev_alloc_skb (dev, length,GFP_ATOMIC |__GFP_NOWARN); +}#endif /* _E1000_H_ */ _
Quoted 10 lines · On Wed, 2 Apr 2008 20:12:58 +1100 Nick Piggin <[email protected]> wrote:
Quoted 8 lines · Andrew Morton
Quoted 4 lines
Limiting this to once per boot should suffice for debugging purposes. Even if you manage to concoct a bug that always survives the first failure, you should be able to take the hint when you keep seeing this in dmesg.
The appropriate thing to do here is to convert known-good drivers (such as e1000[e]) to use
__GFP_NOWARN.
Quoted 6 lines
Quoted 2 lines
Unfortunately
netdev_alloc_skb()went and assumedGFP_ATOMIC, but I guess we can dive below the covers and use__netdev_alloc_skb():It's still actually nice to know how often it is happening even for these known good sites because too much can indicate a problem and that you could actually bring performance up by tuning some things.
Yes, it's useful debugging. It tells us when we mucked up the page allocator.
It also tells us when we mucked up the net driver - I doubt if we (or at least, I) would have discovered that e1000 does a 32k allocation for a 5k(?) frame if this warning wasn't coming out.
Quoted 4 lines
So I think that the messages should stay, and they should print out some header to say that it is only a warning and if not happening too often then it is not a problem, and if it is continually happening then please try X or Y or post a message to lkml...
Yes, I suppose so.
hm, tricky.
Quoted 1 line · Andrew Morton
On Wed, 2 Apr 2008 20:12:58 +1100 Nick Piggin <[email protected]>
Quoted 12 lines · wrote:
Quoted 12 lines · Andrew Morton
Quoted 10 lines
Quoted 4 lines
Limiting this to once per boot should suffice for debugging purposes. Even if you manage to concoct a bug that always survives the first failure, you should be able to take the hint when you keep seeing this in dmesg.
The appropriate thing to do here is to convert known-good drivers (such as e1000[e]) to use
__GFP_NOWARN.Unfortunately
netdev_alloc_skb()went and assumedGFP_ATOMIC, but I guess we can dive below the covers and use__netdev_alloc_skb():
Quoted 6 lines
Quoted 3 lines
It's still actually nice to know how often it is happening even for these known good sites because too much can indicate a problem and that you could actually bring performance up by tuning some things.
Yes, it's useful debugging. It tells us when we mucked up the page allocator.
Right.
Quoted 3 lines
It also tells us when we mucked up the net driver - I doubt if we (or at least, I) would have discovered that e1000 does a 32k allocation for a 5k(?) frame if this warning wasn't coming out.
Is that right? If it is allocating for 9K MTU, then the slab allocator (slub in this case) will bump that up to the 16K kmalloc slab. If it is a 5K frame, then it would get the 8K kmalloc slab I think.
Oh, but SLUB's default MIN_OBJECTS is 4, so 4*8 is 32 indeed. So slub is probably deciding to round the kmalloc-8192 allocations up to order-3. I think. How did you know it was a 5k frame? :)
Quoted 11 lines · On Thu, 3 Apr 2008 16:22:26 +1100 Nick Piggin <[email protected]> wrote:
Quoted 3 lines
It also tells us when we mucked up the net driver - I doubt if we (or at least, I) would have discovered that e1000 does a 32k allocation for a 5k(?) frame if this warning wasn't coming out.
Is that right? If it is allocating for 9K MTU, then the slab allocator (slub in this case) will bump that up to the 16K kmalloc slab. If it is a 5K frame, then it would get the 8K kmalloc slab I think.
Oh, but SLUB's default
MIN_OBJECTSis 4, so 4*8 is 32 indeed. So slub is probably deciding to round the kmalloc-8192 allocations up to order-3. I think. How did you know it was a 5k frame? :)
urgh, it was a while ago, and I don't know if e1000e retains the behaviour.
iirc the issue was with some errant versions of the hardware needing exorbitant alignment and additional padding at the end because of occasional DMA overruns. Something like that.
Quoted 12 lines · Andrew Morton
Quoted 11 lines
Quoted 3 lines
It also tells us when we mucked up the net driver - I doubt if we (or at least, I) would have discovered that e1000 does a 32k allocation for a 5k(?) frame if this warning wasn't coming out.
Is that right? If it is allocating for 9K MTU, then the slab allocator (slub in this case) will bump that up to the 16K kmalloc slab. If it is a 5K frame, then it would get the 8K kmalloc slab I think.
Oh, but SLUB's default
MIN_OBJECTSis 4, so 4*8 is 32 indeed. So slub is probably deciding to round the kmalloc-8192 allocations up to order-3. I think. How did you know it was a 5k frame? :)
Quoted 5 lines
urgh, it was a while ago, and I don't know if e1000e retains the behaviour.
iirc the issue was with some errant versions of the hardware needing exorbitant alignment and additional padding at the end because of occasional DMA overruns. Something like that.
e1000 hardware does require power-of-two alignment, network stack adds additional structure at the end, so with e1000 it ends up with two rounds to the higher power of two.
5k ends up with 16k allocations, 9k - to 32k.
This problem is known for years already and number of fixes was proposed, but the really good one is to rewrite e1000 allocation path to use fragments, which I believe was done in the new e1000 driver.
And as a side note: shuting allocation failures is a very bad step, since it hides allocation problems for drivers. if people do care about it add __GFP_SMALL_WARN flag which will just print that allocation failed, its order and function where it happend.
Signature · 1 line
Evgeniy Polyakov
This thread seemed to die out with no resolution..
Quoted 19 lines · Evgeniy Polyakov
Quoted 12 lines · Andrew Morton
Quoted 11 lines
Quoted 3 lines
It also tells us when we mucked up the net driver - I doubt if we (or at least, I) would have discovered that e1000 does a 32k allocation for a 5k(?) frame if this warning wasn't coming out.
Is that right? If it is allocating for 9K MTU, then the slab allocator (slub in this case) will bump that up to the 16K kmalloc slab. If it is a 5K frame, then it would get the 8K kmalloc slab I think.
Oh, but SLUB's default
MIN_OBJECTSis 4, so 4*8 is 32 indeed. So slub is probably deciding to round the kmalloc-8192 allocations up to order-3. I think. How did you know it was a 5k frame? :)Quoted 5 lines
urgh, it was a while ago, and I don't know if e1000e retains the behaviour.
iirc the issue was with some errant versions of the hardware needing exorbitant alignment and additional padding at the end because of occasional DMA overruns. Something like that.
Quoted 8 lines
e1000 hardware does require power-of-two alignment, network stack adds additional structure at the end, so with e1000 it ends up with two rounds to the higher power of two.
5k ends up with 16k allocations, 9k - to 32k.This problem is known for years already and number of fixes was proposed, but the really good one is to rewrite e1000 allocation path to use fragments, which I believe was done in the new e1000 driver.
So this morning, we got a fresh report from this in 2.6.25.6's e1000 driver
https://bugzilla.redhat.com/show_bug.cgi?id=453010
Pages and pages of spew, which make users freak out.
This stuff might be 'nice to know', but if it isn't getting fixed, I can see why some distros have been shipping the 'silence GFP_ATOMIC failures' patches for some time.
Dave
Quoted 4 lines
And as a side note: shuting allocation failures is a very bad step, since it hides allocation problems for drivers. if people do care about it add
__GFP_SMALL_WARNflag which will just print that allocation failed, its order and function where it happend.
Signature · 1 line
http://www.codemonkey.org.uk
Quoted 31 lines · Dave Jones
This thread seemed to die out with no resolution..
Quoted 19 lines · Evgeniy Polyakov
Quoted 12 lines · Andrew Morton
Quoted 11 lines
Quoted 3 lines
It also tells us when we mucked up the net driver - I doubt if we (or at least, I) would have discovered that e1000 does a 32k allocation for a 5k(?) frame if this warning wasn't coming out.
Is that right? If it is allocating for 9K MTU, then the slab allocator (slub in this case) will bump that up to the 16K kmalloc slab. If it is a 5K frame, then it would get the 8K kmalloc slab I think.
Oh, but SLUB's default
MIN_OBJECTSis 4, so 4*8 is 32 indeed. So slub is probably deciding to round the kmalloc-8192 allocations up to order-3. I think. How did you know it was a 5k frame? :)Quoted 5 lines
urgh, it was a while ago, and I don't know if e1000e retains the behaviour.
iirc the issue was with some errant versions of the hardware needing exorbitant alignment and additional padding at the end because of occasional DMA overruns. Something like that.
Quoted 8 lines
e1000 hardware does require power-of-two alignment, network stack adds additional structure at the end, so with e1000 it ends up with two rounds to the higher power of two.
5k ends up with 16k allocations, 9k - to 32k.This problem is known for years already and number of fixes was proposed, but the really good one is to rewrite e1000 allocation path to use fragments, which I believe was done in the new e1000 driver.
Quoted 13 lines
So this morning, we got a fresh report from this in 2.6.25.6's e1000 driver
https://bugzilla.redhat.com/show_bug.cgi?id=453010
Pages and pages of spew, which make users freak out.
This stuff might be 'nice to know', but if it isn't getting fixed, I can see why some distros have been shipping the 'silenceGFP_ATOMICfailures' patches for some time.Dave
Quoted 4 lines
And as a side note: shuting allocation failures is a very bad step, since it hides allocation problems for drivers. if people do care about it add
__GFP_SMALL_WARNflag which will just print that allocation failed, its order and function where it happend.
Maybe we should make __GFP_SMALL_WARN the default behavior, and allow subsystems to silence or print the full warning if they see fit. That solves most of the spew, while still preserving the most important information for debugging, and still gives subsystem maintainers the power to do as they see fit.
-- Chris
Quoted 1 line · Dave Jones
This thread seemed to die out with no resolution..
Please ask to test e1000e driver instead, if it supports given PCI ids. It has completely rewritten input path which does not suffer from this kind of problems, so you do not need to care too much about e1000, which will die sooner or later.
Signature · 1 line
Evgeniy Polyakov
Quoted 8 lines · Andrew Morton
The appropriate thing to do here is to convert known-good drivers (such as e1000[e]) to use
__GFP_NOWARN.Unfortunately
netdev_alloc_skb()went and assumedGFP_ATOMIC, but I guess we can dive below the covers and use__netdev_alloc_skb():
Quoted 5 lines
From: Andrew Morton <[email protected]>
We get rather a lot of reports of page allocation warnings coming out of e1000. But this driver is know to handle them properly so let's suppress them.
Do you people hear what you're saying???
I respectfully but strongly disagree with this.
We do __not__ need a whitelist (__GFP_NOWARN) of drivers that handle allocation failures properly. That's a long list, a maintenance nightmare, and it is punishing good behavior.
It has been true for over a decade that allocations should be checked for NULL, and GFP_ATOMIC allocations MUST be checked for NULL.
Let's not crap all over good drivers, because a few bad apples don't have the proper checks.
Or at the very least, this TOTALLY BOGUS spew from working drivers should not be foisted upon users. Every time a working driver complains about this -- as in the examples here -- the value of the warning decreases to noise.
And the solution to noise is not _more noise_ (adding 'nowarn' to every damn driver in the kernel).
Jeff
Quoted 30 lines · On Wed, 02 Apr 2008 13:21:44 -0400 Jeff Garzik <[email protected]> wrote:
Quoted 8 lines · Andrew Morton
The appropriate thing to do here is to convert known-good drivers (such as e1000[e]) to use
__GFP_NOWARN.Unfortunately
netdev_alloc_skb()went and assumedGFP_ATOMIC, but I guess we can dive below the covers and use__netdev_alloc_skb():Quoted 5 lines
From: Andrew Morton <[email protected]>
We get rather a lot of reports of page allocation warnings coming out of e1000. But this driver is know to handle them properly so let's suppress them.
Do you people hear what you're saying???
I respectfully but strongly disagree with this.
We do
__not__need a whitelist (__GFP_NOWARN) of drivers that handle allocation failures properly. That's a long list, a maintenance nightmare, and it is punishing good behavior.It has been true for over a decade that allocations should be checked for NULL, and
GFP_ATOMICallocations MUST be checked for NULL.Let's not crap all over good drivers, because a few bad apples don't have the proper checks.
Quoted 8 lines
Or at the very least, this TOTALLY BOGUS spew from working drivers should not be foisted upon users. Every time a working driver complains about this -- as in the examples here -- the value of the warning decreases to noise.
And the solution to noise is not
_morenoise_(adding 'nowarn' to every damn driver in the kernel).
After you've read Nick's comments (which I pray you have not), and after you've convinced us and yourself of their wrongness, you might like to consider adding a __GFP_NOWARN to netdev_alloc_skb().
Quoted 3 lines · Andrew Morton
After you've read Nick's comments (which I pray you have not), and after you've convinced us and yourself of their wrongness, you might like to consider adding a
__GFP_NOWARNtonetdev_alloc_skb().
Already done so. Adding __GFP_NOWARN to netdev_alloc_skb() is wrong for several reasons.
It doesn't change the underlying conditions.
It doesn't fix the desire to stamp other drivers in this manner.
And most importantly, it is not even correct: the handling of the allocation failure remains delegated to the netdev_alloc_skb() users, which may or may not be properly handling allocation failures.
Put simply, you don't know if the caller is stupid or smart. And there are a _lot_ of callers, do you really want to flag all of them?
Many modern net drivers are smart, and quite gracefully handle allocation failure without skipping a beat.
But some are really dumb, and leave big holes in their DMA rings when allocations fail.
The warnings are valid _sometimes_, but not for others. So adding __GFP_NOWARN to netdev_alloc_skb() unconditionally makes no sense, except as an admission that the "spew when there is memory pressure" idea was silly.
Turning to Nick's comment,
Quoted 3 lines
It's still actually nice to know how often it is happening even for these known good sites because too much can indicate a problem and that you could actually bring performance up by tuning some things.
then create a counter or acculuation buffer somewhere.
We don't need spew every time there is memory pressure of this magnitude.
IMO there are much better ways than printk(), to inform tasks, and humans, of allocation failures.
Jeff
Quoted 37 lines · Jeff Garzik
Quoted 3 lines · Andrew Morton
After you've read Nick's comments (which I pray you have not), and after you've convinced us and yourself of their wrongness, you might like to consider adding a
__GFP_NOWARNtonetdev_alloc_skb().Already done so. Adding
__GFP_NOWARNtonetdev_alloc_skb()is wrong
for several reasons.It doesn't change the underlying conditions.
It doesn't fix the desire to stamp other drivers in this manner.And most importantly, it is not even correct: the handling of the allocation failure remains delegated to the
netdev_alloc_skb()users, which may or may not be properly handling allocation failures.Put simply, you don't know if the caller is stupid or smart. And there are a _lot_ of callers, do you really want to flag all of them?
Many modern net drivers are smart, and quite gracefully handle allocation failure without skipping a beat.
But some are really dumb, and leave big holes in their DMA rings when allocations fail.
The warnings are valid _sometimes_, but not for others. So adding
__GFP_NOWARNtonetdev_alloc_skb()unconditionally makes no sense, except as an admission that the "spew when there is memory pressure" idea was silly.Turning to Nick's comment,
Quoted 3 lines
It's still actually nice to know how often it is happening even for these known good sites because too much can indicate a problem and that you could actually bring performance up by tuning some things.
Quoted 6 lines
then create a counter or acculuation buffer somewhere.
We don't need spew every time there is memory pressure of this magnitude.
IMO there are much better ways than
printk(), to inform tasks, and humans, of allocation failures.
FYI e1000 and family already count various levels of alloc failures resulting from this:
alloc_rx_buff_failed - page alloc failure (might be harmless) rx_no_buffer_count - no buffer available for HW to use (harmless, hw will retry) rx_missed_errors - hw dropped a packet because of above failures
still I personally think the page alloc warnings are a good thing and we've had several issues resolve quickly because of them.
shutting them up completely moves the focus to our driver which ends up being a victim of suspicion, and we have to circle around hard to convince the user otherwise.
Auke
Quoted 9 lines · Jeff Garzik
Turning to Nick's comment,
Quoted 3 lines
It's still actually nice to know how often it is happening even for these known good sites because too much can indicate a problem and that you could actually bring performance up by tuning some things.
then create a counter or acculuation buffer somewhere.
We don't need spew every time there is memory pressure of this magnitude.
Not a complete solution. Counter would be nice, but you need backtraces and want a way to more proactively warn the user/tester/developer.
I agree that I don't exactly like adding nowarns around, and I don't think places like driver writers should have to know about this stuff.
Quoted 2 lines
IMO there are much better ways than
printk(), to inform tasks, and humans, of allocation failures.
I think with a tweaked warning message, a ratelimited printk is OK.
Quoted 14 lines · Nick Piggin
Quoted 8 lines · Jeff Garzik
Turning to Nick's comment,
Quoted 3 lines
It's still actually nice to know how often it is happening even for these known good sites because too much can indicate a problem and that you could actually bring performance up by tuning some things.
then create a counter or acculuation buffer somewhere.
We don't need spew every time there is memory pressure of this magnitude.
Not a complete solution. Counter would be nice, but you need backtraces and want a way to more proactively warn the user/tester/developer.
Quoted 8 lines
I agree that I don't exactly like adding nowarns around, and I don't think places like driver writers should have to know about this stuff.
Quoted 2 lines
IMO there are much better ways than
printk(), to inform tasks, and humans, of allocation failures.I think with a tweaked warning message, a ratelimited printk is OK.
No objections here, and agreed on all points.
Though IMO adding __GFP_NOWARN to netdev_alloc_skb() falls into that category (should not generally be in a driver or driver API).
Jeff