From: Kenzo Iwami <k-iwami@cj.jp.nec.com>
To: Auke Kok <auke-jan.h.kok@intel.com>
Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>,
"Ronciak, John" <john.ronciak@intel.com>,
Shaw Vrana <shaw@vranix.com>,
netdev@vger.kernel.org
Subject: Re: watchdog timeout panic in e1000 driver
Date: Thu, 18 Jan 2007 18:22:20 +0900 [thread overview]
Message-ID: <45AF3C4C.1060606@cj.jp.nec.com> (raw)
In-Reply-To: <45AC8FEB.5090102@cj.jp.nec.com>
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 (k-iwami@cj.jp.nec.com)
Signed-off-by: Kenzo Iwami <k-iwami@cj.jp.nec.com>
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 {
next prev parent reply other threads:[~2007-01-18 9:22 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-11-16 17:20 watchdog timeout panic in e1000 driver Brandeburg, Jesse
2006-11-21 10:16 ` Kenzo Iwami
2006-12-04 9:14 ` Kenzo Iwami
2006-12-05 0:46 ` Auke Kok
2006-12-12 7:58 ` Kenzo Iwami
2006-12-19 0:13 ` Kenzo Iwami
2007-01-15 9:12 ` Kenzo Iwami
2007-01-15 16:14 ` Auke Kok
2007-01-16 8:42 ` Kenzo Iwami
2007-01-18 9:22 ` Kenzo Iwami [this message]
-- strict thread matches above, loose matches on Subject: below --
2006-10-19 10:19 Kenzo Iwami
2006-10-19 15:39 ` Auke Kok
[not found] ` <4538BFF2.2040207@cj.jp.nec.com>
2006-10-20 15:51 ` Auke Kok
2006-10-24 9:01 ` Kenzo Iwami
2006-10-24 16:15 ` Auke Kok
2006-10-25 13:41 ` Kenzo Iwami
2006-10-25 15:09 ` Auke Kok
2006-10-26 10:35 ` Kenzo Iwami
2006-10-26 14:34 ` Auke Kok
2006-10-30 11:36 ` Kenzo Iwami
2006-10-30 17:30 ` Auke Kok
2006-10-31 3:22 ` Shaw Vrana
2006-11-01 13:21 ` Kenzo Iwami
2006-11-15 10:33 ` Kenzo Iwami
2006-11-15 16:11 ` Auke Kok
2006-11-16 9:23 ` Kenzo Iwami
2007-02-20 9:26 ` Kenzo Iwami
2007-02-20 16:10 ` Auke Kok
2007-02-21 5:17 ` Kenzo Iwami
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=45AF3C4C.1060606@cj.jp.nec.com \
--to=k-iwami@cj.jp.nec.com \
--cc=auke-jan.h.kok@intel.com \
--cc=jesse.brandeburg@intel.com \
--cc=john.ronciak@intel.com \
--cc=netdev@vger.kernel.org \
--cc=shaw@vranix.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).