Hi Sebastian,
On Wed, 16 Feb 2022, Marek Szyprowski wrote:
On 12.02.2022 00:38, Sebastian Andrzej Siewior wrote:
Dave suggested a while ago (eleven years by now) "Let's make netif_rx()
work in all contexts and get rid of netif_rx_ni()". Eric agreed and
pointed out that modern devices should use netif_receive_skb() to avoid
the overhead.
In the meantime someone added another variant, netif_rx_any_context(),
which behaves as suggested.
netif_rx() must be invoked with disabled bottom halves to ensure that
pending softirqs, which were raised within the function, are handled.
netif_rx_ni() can be invoked only from process context (bottom halves
must be enabled) because the function handles pending softirqs without
checking if bottom halves were disabled or not.
netif_rx_any_context() invokes on the former functions by checking
in_interrupts().
netif_rx() could be taught to handle both cases (disabled and enabled
bottom halves) by simply disabling bottom halves while invoking
netif_rx_internal(). The local_bh_enable() invocation will then invoke
pending softirqs only if the BH-disable counter drops to zero.
Eric is concerned about the overhead of BH-disable+enable especially in
regard to the loopback driver. As critical as this driver is, it will
receive a shortcut to avoid the additional overhead which is not needed.
Add a local_bh_disable() section in netif_rx() to ensure softirqs are
handled if needed.
Provide __netif_rx() which does not disable BH and has a lockdep assert
to ensure that interrupts are disabled. Use this shortcut in the
loopback driver and in drivers/net/*.c.
Make netif_rx_ni() and netif_rx_any_context() invoke netif_rx() so they
can be removed once they are no more users left.
Link: https://lkml.kernel.org/r/20100415.020246.218622820.davem@xxxxxxxxxxxxx
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
Reviewed-by: Eric Dumazet <edumazet@xxxxxxxxxx>
Reviewed-by: Toke Høiland-Jørgensen <toke@xxxxxxxxxx>
This patch landed in linux-next 20220215 as commit baebdf48c360 ("net:
dev: Makes sure netif_rx() can be invoked in any context."). I found
that it triggers the following warning on my test systems with USB CDC
ethernet gadget:
------------[ cut here ]------------
WARNING: CPU: 0 PID: 876 at kernel/softirq.c:308
__local_bh_disable_ip+0xbc/0xc0
Similar on rbtx4927 (CONFIG_NE2000=y), where I'm getting a slightly
different warning:
Sending DHCP requests .
------------[ cut here ]------------
WARNING: CPU: 0 PID: 0 at kernel/softirq.c:362 __local_bh_enable_ip+0x4c/0xc0
Modules linked in:
CPU: 0 PID: 0 Comm: swapper Not tainted 5.17.0-rc5-rbtx4927-00770-ga8ca72253967 #300
Stack : 9800000000b80800 0000000000000008 0000000000000000 a5ba96d4be38c8b0
0000000000000000 9800000000813c10 ffffffff80468188 9800000000813a90
0000000000000001 9800000000813ab0 0000000000000000 20746f4e20726570
0000000000000010 ffffffff802c1400 ffffffff8054ce76 722d302e37312e35
0000000000000000 0000000000000000 0000000000000009 0000000000000000
98000000008bfd40 000000000000004c 0000000006020283 0000000006020287
0000000000000000 0000000000000000 0000000000000000 ffffffff80540000
ffffffff804b8000 9800000000813c10 9800000000b80800 ffffffff801238bc
0000000000000000 ffffffff80470000 0000000000000000 0000000000000009
0000000000000000 ffffffff80108738 0000000000000000 ffffffff801238bc
...
Call Trace:
[<ffffffff80108738>] show_stack+0x68/0xf4
[<ffffffff801238bc>] __warn+0xc0/0xf0
[<ffffffff80123964>] warn_slowpath_fmt+0x78/0x94
[<ffffffff80126408>] __local_bh_enable_ip+0x4c/0xc0
[<ffffffff80341754>] netif_rx+0x20/0x30
[<ffffffff8031d870>] ei_receive+0x2f0/0x36c
[<ffffffff8031e624>] eip_interrupt+0x2dc/0x36c
[<ffffffff8014f488>] __handle_irq_event_percpu+0x8c/0x134
[<ffffffff8014f548>] handle_irq_event_percpu+0x18/0x60
[<ffffffff8014f5c8>] handle_irq_event+0x38/0x60
[<ffffffff80152008>] handle_level_irq+0x80/0xbc
[<ffffffff8014eecc>] handle_irq_desc+0x24/0x3c
[<ffffffff804014b8>] do_IRQ+0x18/0x24
[<ffffffff801031b0>] handle_int+0x148/0x154
[<ffffffff80104e18>] arch_local_irq_enable+0x18/0x24
[<ffffffff8040148c>] default_idle_call+0x2c/0x3c
[<ffffffff801445d0>] do_idle+0xcc/0x104
[<ffffffff80144620>] cpu_startup_entry+0x18/0x20
[<ffffffff80508e34>] start_kernel+0x6f4/0x738
---[ end trace 0000000000000000 ]---
, OK
IP-Config: Got DHCP answer from a.b.c.d, my address is a.b.c.e
IP-Config: Complete:
Reverting baebdf48c3600807 ("net: dev: Makes sure netif_rx() can be
invoked in any context.") fixes the issue for me.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
