Hi, > My patch may seem like a huge change, but in essence the change is > pretty simple. > > In my patch, the interrupt handler code will check whether the interrupted > code is holding the swfw semaphore. If it is held, the watchdog function > is deferred until swfw semaphore is released. > The modification is for the interrupted code which is holding the > semaphore, and the interrupt handler, so they are both directly related > to this problem. > > I will try to add some comments to my code to make it more readable.
I have rebased this patch for 2.6.20-rc5 and added some comments to this patch to make it more readable. Does this patch have problems? Or do you have any other ideas? -- Kenzo Iwami ([EMAIL PROTECTED]) Signed-off-by: Kenzo Iwami <[EMAIL PROTECTED]> diff -urpN linux-2.6.20-rc5_org/drivers/net/e1000/e1000_hw.c linux-2.6.20-rc5_fix/drivers/net/e1000/e1000_hw.c --- linux-2.6.20-rc5_org/drivers/net/e1000/e1000_hw.c 2007-01-13 03:54:26.000000000 +0900 +++ linux-2.6.20-rc5_fix/drivers/net/e1000/e1000_hw.c 2007-01-18 18:04:04.000000000 +0900 @@ -35,6 +35,7 @@ static int32_t e1000_swfw_sync_acquire(struct e1000_hw *hw, uint16_t mask); static void e1000_swfw_sync_release(struct e1000_hw *hw, uint16_t mask); +static void e1000_check_watchdog_deferred(struct e1000_hw *hw); static int32_t e1000_read_kmrn_reg(struct e1000_hw *hw, uint32_t reg_addr, uint16_t *data); static int32_t e1000_write_kmrn_reg(struct e1000_hw *hw, uint32_t reg_addr, uint16_t data); static int32_t e1000_get_software_semaphore(struct e1000_hw *hw); @@ -3396,6 +3397,29 @@ e1000_shift_in_mdi_bits(struct e1000_hw return data; } +static void +e1000_check_watchdog_deferred(struct e1000_hw *hw) +{ + /* If watchdog interrupt was deferred while this process was holding + * the swfw semaphore, process the watchdog now. + */ + if (atomic_xchg(&hw->watchdog_deferred, 0)) { +retry: + e1000_do_watchdog(hw); + } + atomic_dec(&hw->swfw_sem_count); + + /* Check watchdog_deferred once more just in case watchdog interrupt + * occurred between atomic_xchg and atomic_dec. + * By nature of the watchdog interrupt, we shouldn't go through this + * more than once. + */ + if (atomic_xchg(&hw->watchdog_deferred, 0)) { + atomic_inc(&hw->swfw_sem_count); + goto retry; + } +} + static int32_t e1000_swfw_sync_acquire(struct e1000_hw *hw, uint16_t mask) { @@ -3413,8 +3437,11 @@ e1000_swfw_sync_acquire(struct e1000_hw return e1000_get_hw_eeprom_semaphore(hw); while (timeout) { - if (e1000_get_hw_eeprom_semaphore(hw)) + atomic_inc(&hw->swfw_sem_count); + if (e1000_get_hw_eeprom_semaphore(hw)) { + e1000_check_watchdog_deferred(hw); return -E1000_ERR_SWFW_SYNC; + } swfw_sync = E1000_READ_REG(hw, SW_FW_SYNC); if (!(swfw_sync & (fwmask | swmask))) { @@ -3424,6 +3451,7 @@ e1000_swfw_sync_acquire(struct e1000_hw /* firmware currently using resource (fwmask) */ /* or other software thread currently using resource (swmask) */ e1000_put_hw_eeprom_semaphore(hw); + e1000_check_watchdog_deferred(hw); mdelay(5); timeout--; } @@ -3437,6 +3465,8 @@ e1000_swfw_sync_acquire(struct e1000_hw E1000_WRITE_REG(hw, SW_FW_SYNC, swfw_sync); e1000_put_hw_eeprom_semaphore(hw); + e1000_check_watchdog_deferred(hw); + return E1000_SUCCESS; } @@ -3458,6 +3488,7 @@ e1000_swfw_sync_release(struct e1000_hw return; } + atomic_inc(&hw->swfw_sem_count); /* if (e1000_get_hw_eeprom_semaphore(hw)) * return -E1000_ERR_SWFW_SYNC; */ while (e1000_get_hw_eeprom_semaphore(hw) != E1000_SUCCESS); @@ -3468,6 +3499,7 @@ e1000_swfw_sync_release(struct e1000_hw E1000_WRITE_REG(hw, SW_FW_SYNC, swfw_sync); e1000_put_hw_eeprom_semaphore(hw); + e1000_check_watchdog_deferred(hw); } /***************************************************************************** diff -urpN linux-2.6.20-rc5_org/drivers/net/e1000/e1000_hw.h linux-2.6.20-rc5_fix/drivers/net/e1000/e1000_hw.h --- linux-2.6.20-rc5_org/drivers/net/e1000/e1000_hw.h 2007-01-13 03:54:26.000000000 +0900 +++ linux-2.6.20-rc5_fix/drivers/net/e1000/e1000_hw.h 2007-01-18 18:04:42.000000000 +0900 @@ -306,6 +306,7 @@ typedef enum { #define E1000_BYTE_SWAP_WORD(_value) ((((_value) & 0x00ff) << 8) | \ (((_value) & 0xff00) >> 8)) +extern void e1000_do_watchdog(struct e1000_hw *hw); /* Function prototypes */ /* Initialization */ int32_t e1000_reset_hw(struct e1000_hw *hw); @@ -1465,6 +1466,8 @@ struct e1000_hw { boolean_t has_manc2h; boolean_t rx_needs_kicking; boolean_t has_smbus; + atomic_t swfw_sem_count; /* >0 if swfw_sem held on ESB2 */ + atomic_t watchdog_deferred; /* watchdog deferred on ESB2 */ }; diff -urpN linux-2.6.20-rc5_org/drivers/net/e1000/e1000_main.c linux-2.6.20-rc5_fix/drivers/net/e1000/e1000_main.c --- linux-2.6.20-rc5_org/drivers/net/e1000/e1000_main.c 2007-01-13 03:54:26.000000000 +0900 +++ linux-2.6.20-rc5_fix/drivers/net/e1000/e1000_main.c 2007-01-18 18:05:54.000000000 +0900 @@ -152,6 +152,7 @@ static void e1000_clean_rx_ring(struct e static void e1000_set_multi(struct net_device *netdev); static void e1000_update_phy_info(unsigned long data); static void e1000_watchdog(unsigned long data); +void e1000_do_watchdog(struct e1000_hw *hw); static void e1000_82547_tx_fifo_stall(unsigned long data); static int e1000_xmit_frame(struct sk_buff *skb, struct net_device *netdev); static struct net_device_stats * e1000_get_stats(struct net_device *netdev); @@ -1313,6 +1314,9 @@ e1000_sw_init(struct e1000_adapter *adap hw->tbi_compatibility_en = TRUE; hw->adaptive_ifs = TRUE; + atomic_set(&hw->swfw_sem_count, 0); + atomic_set(&hw->watchdog_deferred, 0); + /* Copper options */ if (hw->media_type == e1000_media_type_copper) { @@ -2555,6 +2559,29 @@ static void e1000_watchdog(unsigned long data) { struct e1000_adapter *adapter = (struct e1000_adapter *) data; + struct e1000_hw *hw = &adapter->hw; + + if (hw->swfw_sync_present) { + /* Check whether the interrupted code is holding the swfw + * semaphore. If it is held, defer processing the watchdog + * until swfw semaphore is released. + */ + if (atomic_read(&hw->swfw_sem_count)) + atomic_set(&hw->watchdog_deferred, 1); + else + e1000_do_watchdog(hw); + } else { + e1000_do_watchdog(hw); + } + + /* Reset the timer */ + mod_timer(&adapter->watchdog_timer, jiffies + 2 * HZ); +} + +void +e1000_do_watchdog(struct e1000_hw *hw) +{ + struct e1000_adapter *adapter = hw->back; struct net_device *netdev = adapter->netdev; struct e1000_tx_ring *txdr = adapter->tx_ring; uint32_t link, tctl; @@ -2722,9 +2749,6 @@ e1000_watchdog(unsigned long data) * reset from the other port. Set the appropriate LAA in RAR[0] */ if (adapter->hw.mac_type == e1000_82571 && adapter->hw.laa_is_present) e1000_rar_set(&adapter->hw, adapter->hw.mac_addr, 0); - - /* Reset the timer */ - mod_timer(&adapter->watchdog_timer, jiffies + 2 * HZ); } enum latency_range { - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html