Commit 5f4a780d authored by Bruce Allan's avatar Bruce Allan Committed by Jeff Kirsher

e1000e: hitting BUG_ON() from napi_enable

Based on a patch from Mike McElroy created against the out-of-tree e1000e
driver:

Hitting the BUG_ON in napi_enable(). Code inspection shows that this can
only be triggered by calling napi_enable() twice without an intervening
napi_disable().

I saw the following sequence of events in the stack trace:

1) We simulated a cable pull using an Extreme switch.
2) e1000_tx_timeout() was entered.
3) e1000_reset_task() was called. Saw the message from e_err() in the
console log.
4) e1000_reinit_locked was called. This function calls e1000_down() and
e1000_up(). These functions call napi_disable() and napi_enable()
respectively.
5) Then on another thread, a monitor task saw carrier was down and executed
'ip set link down' and 'ip set link up' commands.
6) Saw the '_E1000_RESETTING'warning fron the e1000_close function.
7) Either the e1000_open() executed between the e1000_down() and e1000_up()
calls in step 4 or the e1000_open() call executed after the e1000_up()
call.  In either case, napi_enable() is called twice which triggers the
BUG_ON.
Signed-off-by: default avatarBruce Allan <bruce.w.allan@intel.com>
Cc: Mike McElroy <mike.mcelroy@stratus.com>
Tested-by: default avatarAaron Brown <aaron.f.brown@intel.com>
Signed-off-by: default avatarJeff Kirsher <jeffrey.t.kirsher@intel.com>
parent 09357b00
...@@ -3516,7 +3516,6 @@ int e1000e_up(struct e1000_adapter *adapter) ...@@ -3516,7 +3516,6 @@ int e1000e_up(struct e1000_adapter *adapter)
clear_bit(__E1000_DOWN, &adapter->state); clear_bit(__E1000_DOWN, &adapter->state);
napi_enable(&adapter->napi);
if (adapter->msix_entries) if (adapter->msix_entries)
e1000_configure_msix(adapter); e1000_configure_msix(adapter);
e1000_irq_enable(adapter); e1000_irq_enable(adapter);
...@@ -3578,7 +3577,6 @@ void e1000e_down(struct e1000_adapter *adapter) ...@@ -3578,7 +3577,6 @@ void e1000e_down(struct e1000_adapter *adapter)
e1e_flush(); e1e_flush();
usleep_range(10000, 20000); usleep_range(10000, 20000);
napi_disable(&adapter->napi);
e1000_irq_disable(adapter); e1000_irq_disable(adapter);
del_timer_sync(&adapter->watchdog_timer); del_timer_sync(&adapter->watchdog_timer);
...@@ -3901,6 +3899,8 @@ static int e1000_close(struct net_device *netdev) ...@@ -3901,6 +3899,8 @@ static int e1000_close(struct net_device *netdev)
pm_runtime_get_sync(&pdev->dev); pm_runtime_get_sync(&pdev->dev);
napi_disable(&adapter->napi);
if (!test_bit(__E1000_DOWN, &adapter->state)) { if (!test_bit(__E1000_DOWN, &adapter->state)) {
e1000e_down(adapter); e1000e_down(adapter);
e1000_free_irq(adapter); e1000_free_irq(adapter);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment