Skip to content

Commit

Permalink
e1000: rework driver hardware reset locking
Browse files Browse the repository at this point in the history
After studying the driver mac reset code it was found that there
were multiple race conditions possible to reset the unit twice or
bring it e1000_up() double. This fixes all occurences where the
driver needs to reset the mac.

We also remove irq requesting/releasing into _open and _close so
that while the device is _up we will never touch the irq's. This fixes
the double free irq bug that people saw.

To make sure that the watchdog task doesn't cause another race we let
it run as a non-scheduled task.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
  • Loading branch information
Auke Kok authored and Auke Kok committed Jun 27, 2006
1 parent acfbc9f commit 2db10a0
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 75 deletions.
8 changes: 6 additions & 2 deletions drivers/net/e1000/e1000.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@
#ifdef NETIF_F_TSO
#include <net/checksum.h>
#endif
#include <linux/workqueue.h>
#include <linux/mii.h>
#include <linux/ethtool.h>
#include <linux/if_vlan.h>
Expand Down Expand Up @@ -255,7 +254,6 @@ struct e1000_adapter {
spinlock_t tx_queue_lock;
#endif
atomic_t irq_sem;
struct work_struct watchdog_task;
struct work_struct reset_task;
uint8_t fc_autoneg;

Expand Down Expand Up @@ -340,15 +338,21 @@ struct e1000_adapter {
#ifdef NETIF_F_TSO
boolean_t tso_force;
#endif
unsigned long flags;
};

enum e1000_state_t {
__E1000_DRIVER_TESTING,
__E1000_RESETTING,
};

/* e1000_main.c */
extern char e1000_driver_name[];
extern char e1000_driver_version[];
int e1000_up(struct e1000_adapter *adapter);
void e1000_down(struct e1000_adapter *adapter);
void e1000_reset(struct e1000_adapter *adapter);
void e1000_reinit_locked(struct e1000_adapter *adapter);
int e1000_setup_all_tx_resources(struct e1000_adapter *adapter);
void e1000_free_all_tx_resources(struct e1000_adapter *adapter);
int e1000_setup_all_rx_resources(struct e1000_adapter *adapter);
Expand Down
46 changes: 26 additions & 20 deletions drivers/net/e1000/e1000_ethtool.c
Original file line number Diff line number Diff line change
Expand Up @@ -203,11 +203,9 @@ e1000_set_settings(struct net_device *netdev, struct ethtool_cmd *ecmd)

/* reset the link */

if (netif_running(adapter->netdev)) {
e1000_down(adapter);
e1000_reset(adapter);
e1000_up(adapter);
} else
if (netif_running(adapter->netdev))
e1000_reinit_locked(adapter);
else
e1000_reset(adapter);

return 0;
Expand Down Expand Up @@ -254,10 +252,9 @@ e1000_set_pauseparam(struct net_device *netdev,
hw->original_fc = hw->fc;

if (adapter->fc_autoneg == AUTONEG_ENABLE) {
if (netif_running(adapter->netdev)) {
e1000_down(adapter);
e1000_up(adapter);
} else
if (netif_running(adapter->netdev))
e1000_reinit_locked(adapter);
else
e1000_reset(adapter);
} else
return ((hw->media_type == e1000_media_type_fiber) ?
Expand All @@ -279,10 +276,9 @@ e1000_set_rx_csum(struct net_device *netdev, uint32_t data)
struct e1000_adapter *adapter = netdev_priv(netdev);
adapter->rx_csum = data;

if (netif_running(netdev)) {
e1000_down(adapter);
e1000_up(adapter);
} else
if (netif_running(netdev))
e1000_reinit_locked(adapter);
else
e1000_reset(adapter);
return 0;
}
Expand Down Expand Up @@ -631,6 +627,9 @@ e1000_set_ringparam(struct net_device *netdev,
tx_ring_size = sizeof(struct e1000_tx_ring) * adapter->num_tx_queues;
rx_ring_size = sizeof(struct e1000_rx_ring) * adapter->num_rx_queues;

while (test_and_set_bit(__E1000_RESETTING, &adapter->flags))
msleep(1);

if (netif_running(adapter->netdev))
e1000_down(adapter);

Expand Down Expand Up @@ -691,16 +690,20 @@ e1000_set_ringparam(struct net_device *netdev,
adapter->rx_ring = rx_new;
adapter->tx_ring = tx_new;
if ((err = e1000_up(adapter)))
return err;
goto err_setup;
}

clear_bit(__E1000_RESETTING, &adapter->flags);

return 0;
err_setup_tx:
e1000_free_all_rx_resources(adapter);
err_setup_rx:
adapter->rx_ring = rx_old;
adapter->tx_ring = tx_old;
e1000_up(adapter);
err_setup:
clear_bit(__E1000_RESETTING, &adapter->flags);
return err;
}

Expand Down Expand Up @@ -1568,6 +1571,7 @@ e1000_diag_test(struct net_device *netdev,
struct e1000_adapter *adapter = netdev_priv(netdev);
boolean_t if_running = netif_running(netdev);

set_bit(__E1000_DRIVER_TESTING, &adapter->flags);
if (eth_test->flags == ETH_TEST_FL_OFFLINE) {
/* Offline tests */

Expand All @@ -1582,7 +1586,8 @@ e1000_diag_test(struct net_device *netdev,
eth_test->flags |= ETH_TEST_FL_FAILED;

if (if_running)
e1000_down(adapter);
/* indicate we're in test mode */
dev_close(netdev);
else
e1000_reset(adapter);

Expand All @@ -1607,8 +1612,9 @@ e1000_diag_test(struct net_device *netdev,
adapter->hw.autoneg = autoneg;

e1000_reset(adapter);
clear_bit(__E1000_DRIVER_TESTING, &adapter->flags);
if (if_running)
e1000_up(adapter);
dev_open(netdev);
} else {
/* Online tests */
if (e1000_link_test(adapter, &data[4]))
Expand All @@ -1619,6 +1625,8 @@ e1000_diag_test(struct net_device *netdev,
data[1] = 0;
data[2] = 0;
data[3] = 0;

clear_bit(__E1000_DRIVER_TESTING, &adapter->flags);
}
msleep_interruptible(4 * 1000);
}
Expand Down Expand Up @@ -1807,10 +1815,8 @@ static int
e1000_nway_reset(struct net_device *netdev)
{
struct e1000_adapter *adapter = netdev_priv(netdev);
if (netif_running(netdev)) {
e1000_down(adapter);
e1000_up(adapter);
}
if (netif_running(netdev))
e1000_reinit_locked(adapter);
return 0;
}

Expand Down
Loading

0 comments on commit 2db10a0

Please sign in to comment.