e1000: Avoid that e1000_netpoll() triggers a kernel warning
Subsystems (3)
PATCH 1/1 e1000: Avoid that e1000_netpoll() triggers a kernel warning
console_cont_flush(), which is called by console_unlock(), calls call_console_drivers() and hence also the netconsole function write_msg() with local interrupts disabled. This means that it is not allowed to call disable_irq() from inside a netpoll callback function. Hence eliminate the disable_irq() / enable_irq() pair from the e1000 netpoll function. This patch avoids that the e1000 networking driver triggers the following complaint:
BUG: sleeping function called from invalid context at kernel/irq/manage.c:104 Call Trace: [<ffffffff814d1ec5>] dump_stack+0x4c/0x65 [<ffffffff8107bcc5>] ___might_sleep+0x175/0x230 [<ffffffff8107bdba>] __might_sleep+0x3a/0xa0 [<ffffffff810a78c8>] synchronize_irq+0x38/0xa0 [<ffffffff810a7a20>] disable_irq+0x20/0x30 [<ffffffffa04b4442>] e1000_netpoll+0x102/0x130 [e1000e] [<ffffffff813ffff2>] netpoll_poll_dev+0x72/0x350 [<ffffffff81400489>] netpoll_send_skb_on_dev+0x1b9/0x2b0 [<ffffffff81400842>] netpoll_send_udp+0x2c2/0x430 [<ffffffffa058187f>] write_msg+0xcf/0x120 [netconsole] [<ffffffff810a4682>] call_console_drivers.constprop.25+0xc2/0x250 [<ffffffff810a5588>] console_unlock+0x328/0x4c0 [<ffffffff810a59f0>] vprintk_emit+0x2d0/0x570 [<ffffffff810a5def>] vprintk_default+0x1f/0x30 [<ffffffff814cf680>] printk+0x46/0x48
See also "[RFC PATCH net-next 00/11] net: remove disable_irq() from ->ndo_poll_controller" (http://thread.gmane.org/gmane.linux.network/342096).
See also patch "sched/wait: Add might_sleep() checks" (kernel v3.19-rc1; commit e22b886a8a43).
--- drivers/net/ethernet/intel/e1000/e1000.h | 5 +++++ drivers/net/ethernet/intel/e1000/e1000_main.c | 27 ++++++++++++++++++++++----- 2 files changed, 27 insertions(+), 5 deletions(-)
drivers/net/ethernet/intel/e1000/e1000.h +5 -0
@@ -323,6 +323,11 @@ struct e1000_adapter {struct delayed_work watchdog_task;struct delayed_work fifo_stall_task;struct delayed_work phy_info_task; + +#ifdef CONFIG_NET_POLL_CONTROLLER +/* Used to serialize e1000 interrupts and the e1000 netpoll callback. */ +spinlock_t netpoll_lock; +#endif };enum e1000_state_t {
drivers/net/ethernet/intel/e1000/e1000_main.c +22 -5
@@ -1313,6 +1313,9 @@ static int e1000_sw_init(struct e1000_adapter *adapter)e1000_irq_disable (adapter);spin_lock_init (&adapter->stats_lock); +#ifdef CONFIG_NET_POLL_CONTROLLER +spin_lock_init (&adapter->netpoll_lock); +#endif set_bit (__E1000_DOWN, &adapter->flags);
@@ -3751,10 +3754,8 @@ void e1000_update_stats(struct e1000_adapter *adapter)* @irq: interrupt number * @data: pointer to a network interface device structure **/ -static irqreturn_t e1000_intr (int irq,void *data) +static irqreturn_t __e1000_intr (int irq,struct e1000_adapter *adapter) { -struct net_device *netdev = data; -struct e1000_adapter *adapter =netdev_priv (netdev);struct e1000_hw *hw = &adapter->hw;u32 icr =er32 (ICR );
@@ -3796,6 +3797,24 @@ static irqreturn_t e1000_intr(int irq, void *data)return IRQ_HANDLED ; } +static irqreturn_t e1000_intr (int irq,void *data) +{ +struct net_device *netdev = data; +struct e1000_adapter *adapter =netdev_priv (netdev); +irqreturn_t ret; +#ifdef CONFIG_NET_POLL_CONTROLLER +unsigned long flags; + +spin_lock_irqsave (&adapter->netpoll_lock, flags); + ret =__e1000_intr (irq, adapter); +spin_unlock_irqrestore (&adapter->netpoll_lock, flags); +#else + ret =__e1000_intr (irq, adapter); +#endif + +return ret; +} +/** * e1000_clean - NAPI Rx polling callback * @adapter: board private structure
@@ -5220,9 +5239,7 @@ static void e1000_netpoll(struct net_device *netdev) {struct e1000_adapter *adapter =netdev_priv (netdev); -disable_irq (adapter->pdev->irq);e1000_intr (adapter->pdev->irq, netdev); -enable_irq (adapter->pdev->irq); }#endif
Signature · 1 line
2.1.4
Quoted 39 lines · Bart Van Assche
console_cont_flush(), which is called byconsole_unlock(), callscall_console_drivers()and hence also the netconsole functionwrite_msg()with local interrupts disabled. This means that it is not allowed to calldisable_irq()from inside a netpoll callback function. Hence eliminate thedisable_irq()/enable_irq()pair from the e1000 netpoll function. This patch avoids that the e1000 networking driver triggers the following complaint:BUG: sleeping function called from invalid context at kernel/irq/manage.c:104 Call Trace: [<ffffffff814d1ec5>] dump_stack+0x4c/0x65 [<ffffffff8107bcc5>] ___might_sleep+0x175/0x230 [<ffffffff8107bdba>] __might_sleep+0x3a/0xa0 [<ffffffff810a78c8>] synchronize_irq+0x38/0xa0 [<ffffffff810a7a20>] disable_irq+0x20/0x30 [<ffffffffa04b4442>] e1000_netpoll+0x102/0x130 [e1000e] [<ffffffff813ffff2>] netpoll_poll_dev+0x72/0x350 [<ffffffff81400489>] netpoll_send_skb_on_dev+0x1b9/0x2b0 [<ffffffff81400842>] netpoll_send_udp+0x2c2/0x430 [<ffffffffa058187f>] write_msg+0xcf/0x120 [netconsole] [<ffffffff810a4682>] call_console_drivers.constprop.25+0xc2/0x250 [<ffffffff810a5588>] console_unlock+0x328/0x4c0 [<ffffffff810a59f0>] vprintk_emit+0x2d0/0x570 [<ffffffff810a5def>] vprintk_default+0x1f/0x30 [<ffffffff814cf680>] printk+0x46/0x48See also "[RFC PATCH net-next 00/11] net: remove
disable_irq()from ->ndo_poll_controller" (http://thread.gmane.org/gmane.linux.network/342096).See also patch "sched/wait: Add
might_sleep()checks" (kernel v3.19-rc1; commite22b886a8a43).Reported-by Sabrina Dubroca <[email protected]>Signed-off-by Bart Van Assche <[email protected]>Cc Thomas Gleixner <[email protected]>Cc David S. Miller <[email protected]>Cc Peter Zijlstra <[email protected]>Cc <[email protected]>
Quoted 5 lines
--- drivers/net/ethernet/intel/e1000/e1000.h | 5 +++++ drivers/net/ethernet/intel/e1000/e1000_main.c | 27 ++++++++++++++++++++++----- 2 files changed, 27 insertions(+), 5 deletions(-)
Instead of adding all of this complexity why not just simplify things and replace the call to e1000_intr with just the bits that matter? You should be able to just copy the napi_schedule_prep, stats reset, and the call to __napi_schedule and you should then be all set to do this without needing to add all the extra locking.
- Alex
Quoted 27 lines · Bart Van Assche
console_cont_flush(), which is called byconsole_unlock(), callscall_console_drivers()and hence also the netconsole functionwrite_msg()with local interrupts disabled. This means that it is not allowed to calldisable_irq()from inside a netpoll callback function. Hence eliminate thedisable_irq()/enable_irq()pair from the e1000 netpoll function. This patch avoids that the e1000 networking driver triggers the following complaint:BUG: sleeping function called from invalid context at kernel/irq/manage.c:104 Call Trace: [<ffffffff814d1ec5>] dump_stack+0x4c/0x65 [<ffffffff8107bcc5>] ___might_sleep+0x175/0x230 [<ffffffff8107bdba>] __might_sleep+0x3a/0xa0 [<ffffffff810a78c8>] synchronize_irq+0x38/0xa0 [<ffffffff810a7a20>] disable_irq+0x20/0x30 [<ffffffffa04b4442>] e1000_netpoll+0x102/0x130 [e1000e] [<ffffffff813ffff2>] netpoll_poll_dev+0x72/0x350 [<ffffffff81400489>] netpoll_send_skb_on_dev+0x1b9/0x2b0 [<ffffffff81400842>] netpoll_send_udp+0x2c2/0x430 [<ffffffffa058187f>] write_msg+0xcf/0x120 [netconsole] [<ffffffff810a4682>] call_console_drivers.constprop.25+0xc2/0x250 [<ffffffff810a5588>] console_unlock+0x328/0x4c0 [<ffffffff810a59f0>] vprintk_emit+0x2d0/0x570 [<ffffffff810a5def>] vprintk_default+0x1f/0x30 [<ffffffff814cf680>] printk+0x46/0x48
Quoted 2 lines
See also "[RFC PATCH net-next 00/11] net: remove
disable_irq()from ->ndo_poll_controller" (http://thread.gmane.org/gmane.linux.network/342096).
That approach was NACKed by David Miller is this exact thread.
Peter Zijlstra added disable_hardirq in commit 02cea3958664 ("genirq: Provide disable_hardirq()")
and I have prepared patches that use this function in drivers/net, but this commit is not in David Miller's trees yet.
Or use Alex's suggestion.
Signature · 1 line
Sabrina
Quoted 39 lines · Bart Van Assche
console_cont_flush(), which is called byconsole_unlock(), callscall_console_drivers()and hence also the netconsole functionwrite_msg()with local interrupts disabled. This means that it is not allowed to calldisable_irq()from inside a netpoll callback function. Hence eliminate thedisable_irq()/enable_irq()pair from the e1000 netpoll function. This patch avoids that the e1000 networking driver triggers the following complaint:BUG: sleeping function called from invalid context at kernel/irq/manage.c:104 Call Trace: [<ffffffff814d1ec5>] dump_stack+0x4c/0x65 [<ffffffff8107bcc5>] ___might_sleep+0x175/0x230 [<ffffffff8107bdba>] __might_sleep+0x3a/0xa0 [<ffffffff810a78c8>] synchronize_irq+0x38/0xa0 [<ffffffff810a7a20>] disable_irq+0x20/0x30 [<ffffffffa04b4442>] e1000_netpoll+0x102/0x130 [e1000e] [<ffffffff813ffff2>] netpoll_poll_dev+0x72/0x350 [<ffffffff81400489>] netpoll_send_skb_on_dev+0x1b9/0x2b0 [<ffffffff81400842>] netpoll_send_udp+0x2c2/0x430 [<ffffffffa058187f>] write_msg+0xcf/0x120 [netconsole] [<ffffffff810a4682>] call_console_drivers.constprop.25+0xc2/0x250 [<ffffffff810a5588>] console_unlock+0x328/0x4c0 [<ffffffff810a59f0>] vprintk_emit+0x2d0/0x570 [<ffffffff810a5def>] vprintk_default+0x1f/0x30 [<ffffffff814cf680>] printk+0x46/0x48See also "[RFC PATCH net-next 00/11] net: remove
disable_irq()from ->ndo_poll_controller" (http://thread.gmane.org/gmane.linux.network/342096).See also patch "sched/wait: Add
might_sleep()checks" (kernel v3.19-rc1; commite22b886a8a43).Reported-by Sabrina Dubroca <[email protected]>Signed-off-by Bart Van Assche <[email protected]>Cc Thomas Gleixner <[email protected]>Cc David S. Miller <[email protected]>Cc Peter Zijlstra <[email protected]>Cc <[email protected]>
Quoted 4 lines
--- drivers/net/ethernet/intel/e1000/e1000.h | 5 +++++ drivers/net/ethernet/intel/e1000/e1000_main.c | 27 ++++++++++++++++++++++----- 2 files changed, 27 insertions(+), 5 deletions(-)
Any particular reason you did not CC the e1000 maintainer (me)?