[RFC PATCH 00/12] e1000e debug and protection patches
Jesse Brandeburg
validating, but we wanted to post them sooner than later. this series of
patches implements:
1-2) base kernel code fixes that might be helpful for debugging
3-6) a series of patches to fix some (possible) buglets in eeprom access code
7) a patch that triggered the bugs that were fixed in 3-6, from Thomas
8-9) (what should be) debug helper patches for testers to log to syslog the
first bytes of the eeprom, that are checksummed at least.
10) protect iomapped memory from write by kernel processes
11) write protect the actual e1000e flash space, add module param to disable[1]
12) update the version so we can tell we're running with this test driver.
# This series applies on GIT commit eaf9e45100b56c18a2236464cadd92d9a3d399ea
Tomorrow we should be able to test and post the eeprom restore patches for
the driver.
[1] we will follow up tomorrow with a patch that locks the actual flash AND
the bits used by 11) that requires a full chip reset to unlock, as
we see a possible way that the iomapped registers could still be poked
in such a way to unlock the flash space.
I will probably update these again but they seem to work for now.
---
Bruce Allan (3):
e1000e: write protect ICHx NVM to prevent malicious write/erase
e1000e: Use set_memory_ro()/set_memory_rw() to protect flash memory
x86: export set_memory_ro and set_memory_rw
Jesse Barnes (1):
pci sysfs warning
Jesse Brandeburg (7):
update version
e1000e: dump eeprom to dmesg for ich8/9
e1000e: allow bad checksum
e1000e: drop stats lock
e1000e: fix lockdep issues
e1000e: do not ever sleep in interrupt context
e1000e: reset swflag after resetting hardware
Thomas Gleixner (1):
e1000e: debug contention on NVM SWFLAG
arch/x86/mm/pageattr.c | 2 +
drivers/net/e1000e/e1000.h | 6 +-
drivers/net/e1000e/ethtool.c | 9 ++
drivers/net/e1000e/hw.h | 1
drivers/net/e1000e/ich8lan.c | 82 +++++++++++++++++++++
drivers/net/e1000e/netdev.c | 167 ++++++++++++++++++++++++++++--------------
drivers/net/e1000e/param.c | 30 ++++++++
drivers/pci/pci-sysfs.c | 14 ++++
8 files changed, 255 insertions(+), 56 deletions(-)
--
Signature
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Jesse Brandeburg
Set the hardware to ignore all write/erase cycles to the GbE region in
the ICHx NVM. This feature can be disabled by the WriteProtectNVM module
parameter (enabled by default) though that is not recommended.
Signed-off-by: Bruce Allan <bruce....@intel.com>
Signed-off-by: Jesse Brandeburg <jesse.br...@intel.com>
---
drivers/net/e1000e/e1000.h | 2 ++
drivers/net/e1000e/ethtool.c | 3 +++
drivers/net/e1000e/ich8lan.c | 46 ++++++++++++++++++++++++++++++++++++++++++
drivers/net/e1000e/netdev.c | 8 +++++--
drivers/net/e1000e/param.c | 30 +++++++++++++++++++++++++++
5 files changed, 87 insertions(+), 2 deletions(-)
diff --git a/drivers/net/e1000e/e1000.h b/drivers/net/e1000e/e1000.h
index 2a3a311..6b0eb73 100644
--- a/drivers/net/e1000e/e1000.h
+++ b/drivers/net/e1000e/e1000.h
@@ -307,6 +307,7 @@ struct e1000_info {
#define FLAG_HAS_CTRLEXT_ON_LOAD (1 << 5)
#define FLAG_HAS_SWSM_ON_LOAD (1 << 6)
#define FLAG_HAS_JUMBO_FRAMES (1 << 7)
+#define FLAG_READ_ONLY_NVM (1 << 8)
#define FLAG_IS_ICH (1 << 9)
#define FLAG_HAS_SMART_POWER_DOWN (1 << 11)
#define FLAG_IS_QUAD_PORT_A (1 << 12)
@@ -387,6 +388,7 @@ extern bool e1000e_enable_mng_pass_thru(struct e1000_hw *hw);
extern bool e1000e_get_laa_state_82571(struct e1000_hw *hw);
extern void e1000e_set_laa_state_82571(struct e1000_hw *hw, bool state);
+extern void e1000e_write_protect_nvm_ich8lan(struct e1000_hw *hw, bool enable);
extern void e1000e_set_kmrn_lock_loss_workaround_ich8lan(struct e1000_hw *hw,
bool state);
extern void e1000e_igp3_phy_powerdown_workaround_ich8lan(struct e1000_hw *hw);
diff --git a/drivers/net/e1000e/ethtool.c b/drivers/net/e1000e/ethtool.c
index f3b49f6..33a3ff1 100644
--- a/drivers/net/e1000e/ethtool.c
+++ b/drivers/net/e1000e/ethtool.c
@@ -533,6 +533,9 @@ static int e1000_set_eeprom(struct net_device *netdev,
if (eeprom->magic != (adapter->pdev->vendor | (adapter->pdev->device << 16)))
return -EFAULT;
+ if (adapter->flags & FLAG_READ_ONLY_NVM)
+ return -EINVAL;
+
max_len = hw->nvm.word_size * 2;
first_word = eeprom->offset >> 1;
diff --git a/drivers/net/e1000e/ich8lan.c b/drivers/net/e1000e/ich8lan.c
index 2b1aa2a..5e3dac9 100644
--- a/drivers/net/e1000e/ich8lan.c
+++ b/drivers/net/e1000e/ich8lan.c
@@ -58,6 +58,7 @@
#define ICH_FLASH_HSFCTL 0x0006
#define ICH_FLASH_FADDR 0x0008
#define ICH_FLASH_FDATA0 0x0010
+#define ICH_FLASH_PR0 0x0074
#define ICH_FLASH_READ_COMMAND_TIMEOUT 500
#define ICH_FLASH_WRITE_COMMAND_TIMEOUT 500
@@ -150,6 +151,19 @@ union ich8_hws_flash_regacc {
u16 regval;
};
+/* ICH Flash Protected Region */
+union ich8_flash_protected_range {
+ struct ich8_pr {
+ u32 base:13; /* 0:12 Protected Range Base */
+ u32 reserved1:2; /* 13:14 Reserved */
+ u32 rpe:1; /* 15 Read Protection Enable */
+ u32 limit:13; /* 16:28 Protected Range Limit */
+ u32 reserved2:2; /* 29:30 Reserved */
+ u32 wpe:1; /* 31 Write Protection Enable */
+ } range;
+ u32 regval;
+};
+
static s32 e1000_setup_link_ich8lan(struct e1000_hw *hw);
static void e1000_clear_hw_cntrs_ich8lan(struct e1000_hw *hw);
static void e1000_initialize_hw_bits_ich8lan(struct e1000_hw *hw);
@@ -1317,6 +1331,7 @@ static s32 e1000_update_nvm_checksum_ich8lan(struct e1000_hw *hw)
* programming failed.
*/
if (ret_val) {
+ /* Possibly read-only, see e1000e_write_protect_nvm_ich8lan() */
hw_dbg(hw, "Flash commit failed.\n");
e1000_release_swflag_ich8lan(hw);
return ret_val;
@@ -1407,6 +1422,37 @@ static s32 e1000_validate_nvm_checksum_ich8lan(struct e1000_hw *hw)
}
/**
+ * e1000e_write_protect_nvm_ich8lan - Make the NVM read-only
+ * @hw: pointer to the HW structure
+ * @enable: pointer to the HW structure
+ * @enable: TRUE to enable write protection, FALSE to disable write protection
+ *
+ * To prevent malicious write/erase of the NVM, set it to be read-only
+ * so that the hardware ignores all write/erase cycles of the NVM via
+ * the flash control registers. The shadow-ram copy of the NVM will
+ * still be updated, however any updates to this copy will not stick
+ * across driver reloads.
+ **/
+void e1000e_write_protect_nvm_ich8lan(struct e1000_hw *hw, bool enable)
+{
+ union ich8_flash_protected_range pr0;
+ u32 gfpreg;
+
+ if (hw->nvm.ops.acquire_nvm(hw))
+ return;
+
+ gfpreg = er32flash(ICH_FLASH_GFPREG);
+
+ pr0.regval = er32flash(ICH_FLASH_PR0);
+ pr0.range.base = gfpreg & FLASH_GFPREG_BASE_MASK;
+ pr0.range.limit = ((gfpreg >> 16) & FLASH_GFPREG_BASE_MASK);
+ pr0.range.wpe = enable;
+ ew32flash(ICH_FLASH_PR0, pr0.regval);
+
+ hw->nvm.ops.release_nvm(hw);
+}
+
+/**
* e1000_write_flash_data_ich8lan - Writes bytes to the NVM
* @hw: pointer to the HW structure
* @offset: The offset (in bytes) of the byte/word to read.
diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index f04de5a..2626c42 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -4508,6 +4508,8 @@ static int __devinit e1000_probe(struct pci_dev *pdev,
adapter->bd_number = cards_found++;
+ e1000e_check_options(adapter);
+
/* setup adapter struct */
err = e1000_sw_init(adapter);
if (err)
@@ -4523,6 +4525,10 @@ static int __devinit e1000_probe(struct pci_dev *pdev,
if (err)
goto err_hw_init;
+ if (adapter->flags & FLAG_IS_ICH)
+ e1000e_write_protect_nvm_ich8lan(&adapter->hw,
+ !!(adapter->flags & FLAG_READ_ONLY_NVM));
+
hw->mac.ops.get_bus_info(&adapter->hw);
adapter->hw.phy.autoneg_wait_to_complete = 0;
@@ -4629,8 +4635,6 @@ static int __devinit e1000_probe(struct pci_dev *pdev,
INIT_WORK(&adapter->downshift_task, e1000e_downshift_workaround);
INIT_WORK(&adapter->update_phy_task, e1000e_update_phy_task);
- e1000e_check_options(adapter);
-
/* Initialize link parameters. User can change them with ethtool */
adapter->hw.mac.autoneg = 1;
adapter->fc_autoneg = 1;
diff --git a/drivers/net/e1000e/param.c b/drivers/net/e1000e/param.c
index ed912e0..d91dbf7 100644
--- a/drivers/net/e1000e/param.c
+++ b/drivers/net/e1000e/param.c
@@ -133,6 +133,15 @@ E1000_PARAM(SmartPowerDownEnable, "Enable PHY smart power down");
*/
E1000_PARAM(KumeranLockLoss, "Enable Kumeran lock loss workaround");
+/*
+ * Write Protect NVM
+ *
+ * Valid Range: 0, 1
+ *
+ * Default Value: 1 (enabled)
+ */
+E1000_PARAM(WriteProtectNVM, "Write-protect NVM [WARNING: disabling this can lead to corrupted NVM]");
+
struct e1000_option {
enum { enable_option, range_option, list_option } type;
const char *name;
@@ -388,4 +397,25 @@ void __devinit e1000e_check_options(struct e1000_adapter *adapter)
opt.def);
}
}
+ { /* Write-protect NVM */
+ const struct e1000_option opt = {
+ .type = enable_option,
+ .name = "Write-protect NVM",
+ .err = "defaulting to Enabled",
+ .def = OPTION_ENABLED
+ };
+
+ if (adapter->flags & FLAG_IS_ICH) {
+ if (num_WriteProtectNVM > bd) {
+ unsigned int write_protect_nvm = WriteProtectNVM[bd];
+ e1000_validate_option(&write_protect_nvm, &opt,
+ adapter);
+ if (write_protect_nvm)
+ adapter->flags |= FLAG_READ_ONLY_NVM;
+ } else {
+ if (opt.def)
+ adapter->flags |= FLAG_READ_ONLY_NVM;
+ }
+ }
+ }
Jesse Brandeburg
The good news is that this patch fixes all the ones I
could find, without damaging any functionality.
CC: Thomas Gleixner <tg...@linutronix.de>
Signed-off-by: Jesse Brandeburg <jesse.br...@intel.com>
---
drivers/net/e1000e/ethtool.c | 6 +++++-
drivers/net/e1000e/netdev.c | 13 -------------
2 files changed, 5 insertions(+), 14 deletions(-)
diff --git a/drivers/net/e1000e/ethtool.c b/drivers/net/e1000e/ethtool.c
index e21c9e0..f3b49f6 100644
--- a/drivers/net/e1000e/ethtool.c
+++ b/drivers/net/e1000e/ethtool.c
@@ -432,6 +432,10 @@ static void e1000_get_regs(struct net_device *netdev,
regs_buff[11] = er32(TIDV);
regs_buff[12] = adapter->hw.phy.type; /* PHY type (IGP=1, M88=0) */
+
+ /* ethtool doesn't use anything past this point, so all this
+ * code is likely legacy junk for apps that may or may not
+ * exist */
if (hw->phy.type == e1000_phy_m88) {
e1e_rphy(hw, M88E1000_PHY_SPEC_STATUS, &phy_data);
regs_buff[13] = (u32)phy_data; /* cable length */
@@ -447,7 +451,7 @@ static void e1000_get_regs(struct net_device *netdev,
regs_buff[22] = adapter->phy_stats.receive_errors;
regs_buff[23] = regs_buff[13]; /* mdix mode */
}
- regs_buff[21] = adapter->phy_stats.idle_errors; /* phy idle errors */
+ regs_buff[21] = 0; /* was idle_errors */
e1e_rphy(hw, PHY_1000T_STATUS, &phy_data);
regs_buff[24] = (u32)phy_data; /* phy local receiver status */
regs_buff[25] = regs_buff[24]; /* phy remote receiver status */
diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index f8c6c32..44ce120 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -2954,9 +2954,6 @@ void e1000e_update_stats(struct e1000_adapter *adapter)
struct e1000_hw *hw = &adapter->hw;
struct pci_dev *pdev = adapter->pdev;
unsigned long irq_flags;
- u16 phy_tmp;
-
-#define PHY_IDLE_ERROR_COUNT_MASK 0x00FF
/*
* Prevent stats update while adapter is being reset, or if the pci
@@ -3045,15 +3042,6 @@ void e1000e_update_stats(struct e1000_adapter *adapter)
/* Tx Dropped needs to be maintained elsewhere */
- /* Phy Stats */
- if (hw->phy.media_type == e1000_media_type_copper) {
- if ((adapter->link_speed == SPEED_1000) &&
- (!e1e_rphy(hw, PHY_1000T_STATUS, &phy_tmp))) {
- phy_tmp &= PHY_IDLE_ERROR_COUNT_MASK;
- adapter->phy_stats.idle_errors += phy_tmp;
- }
- }
-
/* Management Stats */
adapter->stats.mgptc += er32(MGTPTC);
adapter->stats.mgprc += er32(MGTPRC);
@@ -3073,7 +3061,6 @@ static void e1000_phy_read_status(struct e1000_adapter *adapter)
int ret_val;
unsigned long irq_flags;
-
spin_lock_irqsave(&adapter->stats_lock, irq_flags);
if ((er32(STATUS) & E1000_STATUS_LU) &&
Jesse Brandeburg
has the adjust tbi stats function that required the addition
of the stats lock to begin with.
adding a mutex to acquire_swflag helped catch this one too.
Signed-off-by: Jesse Brandeburg <jesse.br...@intel.com>
CC: Thomas Gleixner <tg...@linutronix.de>
---
drivers/net/e1000e/e1000.h | 1 -
drivers/net/e1000e/netdev.c | 18 ------------------
2 files changed, 0 insertions(+), 19 deletions(-)
diff --git a/drivers/net/e1000e/e1000.h b/drivers/net/e1000e/e1000.h
index ed9d974..f80e43a 100644
--- a/drivers/net/e1000e/e1000.h
+++ b/drivers/net/e1000e/e1000.h
@@ -257,7 +257,6 @@ struct e1000_adapter {
struct net_device *netdev;
struct pci_dev *pdev;
struct net_device_stats net_stats;
- spinlock_t stats_lock; /* prevent concurrent stats updates */
/* structs defined in e1000_hw.h */
struct e1000_hw hw;
diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 44ce120..9aa3c79 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -2600,8 +2600,6 @@ static int __devinit e1000_sw_init(struct e1000_adapter *adapter)
/* Explicitly disable IRQ since the NIC can be in any state. */
e1000_irq_disable(adapter);
- spin_lock_init(&adapter->stats_lock);
-
set_bit(__E1000_DOWN, &adapter->state);
return 0;
@@ -2953,7 +2951,6 @@ void e1000e_update_stats(struct e1000_adapter *adapter)
{
struct e1000_hw *hw = &adapter->hw;
struct pci_dev *pdev = adapter->pdev;
- unsigned long irq_flags;
/*
* Prevent stats update while adapter is being reset, or if the pci
@@ -2964,14 +2961,6 @@ void e1000e_update_stats(struct e1000_adapter *adapter)
if (pci_channel_offline(pdev))
return;
- spin_lock_irqsave(&adapter->stats_lock, irq_flags);
-
- /*
- * these counters are modified from e1000_adjust_tbi_stats,
- * called from the interrupt context, so they must only
- * be written while holding adapter->stats_lock
- */
-
adapter->stats.crcerrs += er32(CRCERRS);
adapter->stats.gprc += er32(GPRC);
adapter->stats.gorc += er32(GORCL);
@@ -3046,8 +3035,6 @@ void e1000e_update_stats(struct e1000_adapter *adapter)
adapter->stats.mgptc += er32(MGTPTC);
adapter->stats.mgprc += er32(MGTPRC);
adapter->stats.mgpdc += er32(MGTPDC);
-
- spin_unlock_irqrestore(&adapter->stats_lock, irq_flags);
}
/**
@@ -3059,9 +3046,6 @@ static void e1000_phy_read_status(struct e1000_adapter *adapter)
struct e1000_hw *hw = &adapter->hw;
struct e1000_phy_regs *phy = &adapter->phy_regs;
int ret_val;
- unsigned long irq_flags;
-
- spin_lock_irqsave(&adapter->stats_lock, irq_flags);
if ((er32(STATUS) & E1000_STATUS_LU) &&
(adapter->hw.phy.media_type == e1000_media_type_copper)) {
@@ -3092,8 +3076,6 @@ static void e1000_phy_read_status(struct e1000_adapter *adapter)
phy->stat1000 = 0;
phy->estatus = (ESTATUS_1000_TFULL | ESTATUS_1000_THALF);
}
-
- spin_unlock_irqrestore(&adapter->stats_lock, irq_flags);
}
static void e1000_print_link_info(struct e1000_adapter *adapter)
Jesse Brandeburg
A number of users have reported NVM corruption on various ICHx platform
LOMs. One possible reasons for this could be unexpected and/or malicious
writes to the flash memory area mapped into kernel memory. Once the
interface is up, there should be very few reads/writes of the mapped flash
memory. This patch makes use of the x86 set_memory_*() functions to set
the mapped memory read-only and temporarily set it writable only when the
driver needs to write to it. With the memory set read-only, any unexpected
write will be logged with a stack dump indicating the offending code.
Since these LOMs are only on x86 ICHx platforms, it does not matter that
this API is not yet available on other architectures, however it is
dependent on a previous patch that exports these function name symbols.
Signed-off-by: Bruce Allan <bruce....@intel.com>
Signed-off-by: Jesse Brandeburg <jesse.br...@intel.com>
---
drivers/net/e1000e/e1000.h | 1 +
drivers/net/e1000e/hw.h | 1 +
drivers/net/e1000e/ich8lan.c | 16 ++++++++++++++++
drivers/net/e1000e/netdev.c | 11 +++++++----
4 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/drivers/net/e1000e/e1000.h b/drivers/net/e1000e/e1000.h
index f80e43a..2a3a311 100644
--- a/drivers/net/e1000e/e1000.h
+++ b/drivers/net/e1000e/e1000.h
@@ -36,6 +36,7 @@
#include <linux/workqueue.h>
#include <linux/io.h>
#include <linux/netdevice.h>
+#include <asm/cacheflush.h>
#include "hw.h"
diff --git a/drivers/net/e1000e/hw.h b/drivers/net/e1000e/hw.h
index 74f263a..dd25009 100644
--- a/drivers/net/e1000e/hw.h
+++ b/drivers/net/e1000e/hw.h
@@ -863,6 +863,7 @@ struct e1000_hw {
u8 __iomem *hw_addr;
u8 __iomem *flash_address;
+ resource_size_t flash_len;
struct e1000_mac_info mac;
struct e1000_fc_info fc;
diff --git a/drivers/net/e1000e/ich8lan.c b/drivers/net/e1000e/ich8lan.c
index 57c6d2f..2b1aa2a 100644
--- a/drivers/net/e1000e/ich8lan.c
+++ b/drivers/net/e1000e/ich8lan.c
@@ -176,12 +176,28 @@ static inline u32 __er32flash(struct e1000_hw *hw, unsigned long reg)
static inline void __ew16flash(struct e1000_hw *hw, unsigned long reg, u16 val)
{
+#ifdef _ASM_X86_CACHEFLUSH_H
+ set_memory_rw((unsigned long)hw->flash_address,
+ hw->flash_len >> PAGE_SHIFT);
+#endif
writew(val, hw->flash_address + reg);
+#ifdef _ASM_X86_CACHEFLUSH_H
+ set_memory_ro((unsigned long)hw->flash_address,
+ hw->flash_len >> PAGE_SHIFT);
+#endif
}
static inline void __ew32flash(struct e1000_hw *hw, unsigned long reg, u32 val)
{
+#ifdef _ASM_X86_CACHEFLUSH_H
+ set_memory_rw((unsigned long)hw->flash_address,
+ hw->flash_len >> PAGE_SHIFT);
+#endif
writel(val, hw->flash_address + reg);
+#ifdef _ASM_X86_CACHEFLUSH_H
+ set_memory_ro((unsigned long)hw->flash_address,
+ hw->flash_len >> PAGE_SHIFT);
+#endif
}
#define er16flash(reg) __er16flash(hw, (reg))
diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 57cead3..f04de5a 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -4402,7 +4402,6 @@ static int __devinit e1000_probe(struct pci_dev *pdev,
struct e1000_hw *hw;
const struct e1000_info *ei = e1000_info_tbl[ent->driver_data];
resource_size_t mmio_start, mmio_len;
- resource_size_t flash_start, flash_len;
static int cards_found;
int i, err, pci_using_dac;
@@ -4472,11 +4471,15 @@ static int __devinit e1000_probe(struct pci_dev *pdev,
if ((adapter->flags & FLAG_HAS_FLASH) &&
(pci_resource_flags(pdev, 1) & IORESOURCE_MEM)) {
- flash_start = pci_resource_start(pdev, 1);
- flash_len = pci_resource_len(pdev, 1);
- adapter->hw.flash_address = ioremap(flash_start, flash_len);
+ adapter->hw.flash_len = pci_resource_len(pdev, 1);
+ adapter->hw.flash_address = ioremap(pci_resource_start(pdev, 1),
+ adapter->hw.flash_len);
if (!adapter->hw.flash_address)
goto err_flashmap;
+#ifdef _ASM_X86_CACHEFLUSH_H
+ set_memory_ro((unsigned long)adapter->hw.flash_address,
+ adapter->hw.flash_len >> PAGE_SHIFT);
+#endif
}
/* construct the net_device struct */
Jesse Brandeburg
by the driver after reset, and the swflag is probably not reset unless
management firmware clears it after 100ms.
Signed-off-by: Jesse Brandeburg <jesse.br...@intel.com>
---
drivers/net/e1000e/ich8lan.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/net/e1000e/ich8lan.c b/drivers/net/e1000e/ich8lan.c
index 9e38452..a076079 100644
--- a/drivers/net/e1000e/ich8lan.c
+++ b/drivers/net/e1000e/ich8lan.c
@@ -1720,6 +1720,9 @@ static s32 e1000_reset_hw_ich8lan(struct e1000_hw *hw)
ew32(CTRL, (ctrl | E1000_CTRL_RST));
msleep(20);
+ /* release the swflag because it is not reset by hardware reset */
+ e1000_release_swflag_ich8lan(hw);
+
ret_val = e1000e_get_auto_rd_done(hw);
if (ret_val) {
/*
Jesse Brandeburg
---
drivers/net/e1000e/netdev.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 2626c42..df547b6 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -47,7 +47,7 @@
#include "e1000.h"
-#define DRV_VERSION "0.3.3.3-k2"
+#define DRV_VERSION "0.3.3.3-kt"
char e1000e_driver_name[] = "e1000e";
const char e1000e_driver_version[] = DRV_VERSION;
Jesse Brandeburg
load. This patch allows the driver load process to continue with
an invalid mac address and could allow the user to use ethtool or
another app to fix the eeprom.
copied from implementation in e1000
Signed-off-by: Jesse Brandeburg <jesse.br...@intel.com>
---
drivers/net/e1000e/netdev.c | 81 ++++++++++++++++++++++++++++++++++++-------
1 files changed, 67 insertions(+), 14 deletions(-)
diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 9aa3c79..48b0ded 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -4338,6 +4338,52 @@ static void e1000_eeprom_checks(struct e1000_adapter *adapter)
}
/**
+ * e1000e_dump_eeprom - write the eeprom to kernel log
+ * @adapter: our adapter struct
+ *
+ * Dump the eeprom for users having checksum issues
+ **/
+static void e1000e_dump_eeprom(struct e1000_adapter *adapter)
+{
+ struct net_device *netdev = adapter->netdev;
+ struct ethtool_eeprom eeprom;
+ const struct ethtool_ops *ops = netdev->ethtool_ops;
+ u8 *data;
+ int i;
+ u16 csum_old, csum_new = 0;
+
+ eeprom.len = ops->get_eeprom_len(netdev);
+ eeprom.offset = 0;
+
+ data = kzalloc(eeprom.len, GFP_KERNEL);
+ if (!data) {
+ printk(KERN_ERR "Unable to allocate memory to dump EEPROM"
+ " data\n");
+ return;
+ }
+
+ ops->get_eeprom(netdev, &eeprom, data);
+
+ csum_old = (data[NVM_CHECKSUM_REG * 2]) +
+ (data[NVM_CHECKSUM_REG * 2 + 1] << 8);
+ for (i = 0; i < NVM_CHECKSUM_REG * 2; i += 2)
+ csum_new += data[i] + (data[i + 1] << 8);
+ csum_new = NVM_SUM - csum_new;
+
+ printk(KERN_ERR "/*********************/\n");
+ printk(KERN_ERR "Current EEPROM Checksum : 0x%04x\n", csum_old);
+ printk(KERN_ERR "Calculated : 0x%04x\n", csum_new);
+
+ printk(KERN_ERR "Offset Values\n");
+ printk(KERN_ERR "======== ======\n");
+ print_hex_dump(KERN_ERR, "", DUMP_PREFIX_OFFSET, 16, 1, data, 128, 0);
+
+ printk(KERN_ERR "/*********************/\n");
+
+ kfree(data);
+}
+
+/**
* e1000_probe - Device Initialization Routine
* @pdev: PCI device information struct
* @ent: entry in e1000_pci_tbl
@@ -4527,31 +4573,39 @@ static int __devinit e1000_probe(struct pci_dev *pdev,
* attempt. Let's give it a few tries
*/
for (i = 0;; i++) {
- if (e1000_validate_nvm_checksum(&adapter->hw) >= 0)
+ if (e1000_validate_nvm_checksum(hw) >= 0) {
+ /* copy the MAC address out of the NVM */
+ if (e1000e_read_mac_addr(&adapter->hw))
+ e_err("NVM Read Error reading MAC address\n");
break;
+ }
if (i == 2) {
e_err("The NVM Checksum Is Not Valid\n");
- err = -EIO;
- goto err_eeprom;
+ e1000e_dump_eeprom(adapter);
+ /*
+ * set MAC address to all zeroes to invalidate and
+ * temporary disable this device for the user. This
+ * blocks regular traffic while still permitting
+ * ethtool ioctls from reaching the hardware as well as
+ * allowing the user to run the interface after
+ * manually setting a hw addr using
+ * `ip link set address`
+ */
+ memset(hw->mac.addr, 0, netdev->addr_len);
+ break;
}
}
e1000_eeprom_checks(adapter);
- /* copy the MAC address out of the NVM */
- if (e1000e_read_mac_addr(&adapter->hw))
- e_err("NVM Read Error while reading MAC address\n");
-
+ /* don't block initalization here due to bad MAC address */
memcpy(netdev->dev_addr, adapter->hw.mac.addr, netdev->addr_len);
memcpy(netdev->perm_addr, adapter->hw.mac.addr, netdev->addr_len);
if (!is_valid_ether_addr(netdev->perm_addr)) {
- e_err("Invalid MAC Address: %02x:%02x:%02x:%02x:%02x:%02x\n",
- netdev->perm_addr[0], netdev->perm_addr[1],
- netdev->perm_addr[2], netdev->perm_addr[3],
- netdev->perm_addr[4], netdev->perm_addr[5]);
- err = -EIO;
- goto err_eeprom;
+ DECLARE_MAC_BUF(mac);
+ e_err("Invalid MAC Address: %s\n",
+ print_mac(mac, netdev->perm_addr));
}
init_timer(&adapter->watchdog_timer);
@@ -4640,7 +4694,6 @@ static int __devinit e1000_probe(struct pci_dev *pdev,
err_register:
if (!(adapter->flags & FLAG_HAS_AMT))
e1000_release_hw_control(adapter);
-err_eeprom:
if (!e1000_check_reset_block(&adapter->hw))
e1000_phy_hw_reset(&adapter->hw);
err_hw_init:
Jesse Brandeburg
This patch adds a mutex to the e1000e driver that would help
catch any collisions of two e1000e threads accessing hardware
at the same time.
description and patch updated by Jesse
Signed-off-by: Thomas Gleixner <tg...@linutronix.de>
Signed-off-by: Jesse Brandeburg <jesse.br...@intel.com>
---
drivers/net/e1000e/ich8lan.c | 17 +++++++++++++++++
1 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/drivers/net/e1000e/ich8lan.c b/drivers/net/e1000e/ich8lan.c
index a076079..57c6d2f 100644
--- a/drivers/net/e1000e/ich8lan.c
+++ b/drivers/net/e1000e/ich8lan.c
@@ -366,6 +366,9 @@ static s32 e1000_get_variants_ich8lan(struct e1000_adapter *adapter)
return 0;
}
+static DEFINE_MUTEX(nvm_mutex);
+static pid_t nvm_owner = -1;
+
/**
* e1000_acquire_swflag_ich8lan - Acquire software control flag
* @hw: pointer to the HW structure
@@ -379,6 +382,15 @@ static s32 e1000_acquire_swflag_ich8lan(struct e1000_hw *hw)
u32 extcnf_ctrl;
u32 timeout = PHY_CFG_TIMEOUT;
+ WARN_ON(preempt_count());
+
+ if (!mutex_trylock(&nvm_mutex)) {
+ WARN(1, KERN_ERR "e1000e mutex contention. Owned by pid %d\n",
+ nvm_owner);
+ mutex_lock(&nvm_mutex);
+ }
+ nvm_owner = current->pid;
+
while (timeout) {
extcnf_ctrl = er32(EXTCNF_CTRL);
extcnf_ctrl |= E1000_EXTCNF_CTRL_SWFLAG;
@@ -393,6 +405,8 @@ static s32 e1000_acquire_swflag_ich8lan(struct e1000_hw *hw)
if (!timeout) {
hw_dbg(hw, "FW or HW has locked the resource for too long.\n");
+ nvm_owner = -1;
+ mutex_unlock(&nvm_mutex);
return -E1000_ERR_CONFIG;
}
@@ -414,6 +428,9 @@ static void e1000_release_swflag_ich8lan(struct e1000_hw *hw)
extcnf_ctrl = er32(EXTCNF_CTRL);
extcnf_ctrl &= ~E1000_EXTCNF_CTRL_SWFLAG;
ew32(EXTCNF_CTRL, extcnf_ctrl);
+
+ nvm_owner = -1;
+ mutex_unlock(&nvm_mutex);
}
/**
Jesse Brandeburg
hack, but might be useful when we want to restore it.
if syslogd (or something like) isn't running it won't be kept
however.
Signed-off-by: Jesse Brandeburg <jesse.br...@intel.com>
---
drivers/net/e1000e/netdev.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 48b0ded..57cead3 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -4598,6 +4598,11 @@ static int __devinit e1000_probe(struct pci_dev *pdev,
e1000_eeprom_checks(adapter);
+ /* debug code ... dump the first bytes of the eeprom for
+ * ich parts that might get a corruption */
+ if (adapter->flags & FLAG_IS_ICH)
+ e1000e_dump_eeprom(adapter);
+
/* don't block initalization here due to bad MAC address */
memcpy(netdev->dev_addr, adapter->hw.mac.addr, netdev->addr_len);
memcpy(netdev->perm_addr, adapter->hw.mac.addr, netdev->addr_len);
--
Jiri Kosina
BTW wouldn't something like
if (e1000_validate_nvm_checksum(hw) >= 0 ||
e1000_validate_nvm_checksum(hw) >= 0) {
/* copy the MAC address out of the NVM */
if (e1000e_read_mac_addr(&adapter->hw))
e_err("NVM Read Error reading MAC address\n");
} else {
e_err("The NVM Checksum Is Not Valid\n");
e1000e_dump_eeprom(adapter);
/*
* set MAC address to all zeroes to invalidate and
* temporary disable this device for the user. This
* blocks regular traffic while still permitting
* ethtool ioctls from reaching the hardware as well as
* allowing the user to run the interface after
* manually setting a hw addr using
* `ip link set address`
*/
memset(hw->mac.addr, 0, netdev->addr_len);
}
just be much more readable? Having for(;;) loop which always performs
three iterations, and having "if" inside that distinguishes two iterations
from each other just looks peculiar to my eyes :)
Thanks,
--
Jiri Kosina
SUSE Labs
Jiri Kosina
> Set the hardware to ignore all write/erase cycles to the GbE region in
> the ICHx NVM. This feature can be disabled by the WriteProtectNVM module
> parameter (enabled by default) though that is not recommended.
>
> Signed-off-by: Bruce Allan <bruce....@intel.com>
> Signed-off-by: Jesse Brandeburg <jesse.br...@intel.com>
I guess there is no chance to have kernel somehow notified when
write/erase cycle is unsuccessfully tried, is it? This way, it would also
make chasing the root cause easier.
Thanks,
--
Jiri Kosina
SUSE Labs
--
Allan, Bruce W
Jiri Kosina
> Yeah, we can do that. I need to amend the patch a bit to prevent the
> protected range lock from being lifted unintentionally and will add some
> debug statements if/when any write/erase cycles fail.
Olaf raised a rather interesting question -- would iAMT be able to access
NVM contents directly, even if the lock bit would be set on the device?
I.e. is iAMT allowed direct access to the EEPROM contents, bypassing
shadow ram mappings?
Allan, Bruce W
>
>Olaf raised a rather interesting question -- would iAMT be
>able to access
>NVM contents directly, even if the lock bit would be set on the device?
>I.e. is iAMT allowed direct access to the EEPROM contents, bypassing
>shadow ram mappings?
>
>Thanks,
>
>--
>Jiri Kosina
>SUSE Labs
>
Only write/erase accesses are blocked by hardware after the protected range and lockdown bits are set in this patch; reads are still allowed. I just received confirmation that iAMT does not write to the GbE region of the NVM.
Jiri Kosina
A few minutes ago, I have actually just hit this, while debugging the
issue on a kernel that had this patch included.
I was not successful reproducing it yet though, but still it might be a
pointer into direction where the real bug is.
15:49:07 linux-pr0e dhclient: Listening on LPF/eth1/00:15:58:c6:4a:ff
15:49:07 linux-pr0e dhclient: Sending on LPF/eth1/00:15:58:c6:4a:ff
15:49:07 linux-pr0e dhclient: Sending on Socket/fallback
15:49:07 linux-pr0e dhclient: DHCPDISCOVER on eth1 to 255.255.255.255 port 67 interval 3
15:49:10 linux-pr0e dhclient: DHCPDISCOVER on eth1 to 255.255.255.255 port 67 interval 8
15:49:18 linux-pr0e dhclient: DHCPDISCOVER on eth1 to 255.255.255.255 port 67 interval 9
15:49:27 linux-pr0e dhclient: DHCPDISCOVER on eth1 to 255.255.255.255 port 67 interval 9
15:49:36 linux-pr0e dhclient: DHCPDISCOVER on eth1 to 255.255.255.255 port 67 interval 17
15:49:53 linux-pr0e dhclient: DHCPDISCOVER on eth1 to 255.255.255.255 port 67 interval 12
15:50:05 linux-pr0e dhclient: DHCPDISCOVER on eth1 to 255.255.255.255 port 67 interval 3
15:50:08 linux-pr0e dhclient: No DHCPOFFERS received.
15:50:08 linux-pr0e dhclient: No working leases in persistent database - sleeping.
15:50:52 linux-pr0e kernel: ------------[ cut here ]------------
15:50:52 linux-pr0e kernel: WARNING: at drivers/net/e1000e/ich8lan.c:424 e1000_acquire_swflag_ich8lan+0x5a/0xdc [e1000e]()
15:50:52 linux-pr0e kernel: e1000e mutex contention. Owned by pid 4162
15:50:52 linux-pr0e kernel: Modules linked in: af_packet i915 drm ipv6 snd_pcm_oss snd_mixer_oss snd_seq snd_seq_device cpufreq_conservative cpufreq_userspace cpufreq_powersave acpi_cpufreq microcode fuse loop dm_mod tulip arc4 ecb snd_hda_intel snd_pcm crypto_blkcipher rtc_cmos snd_timer ppdev iwl3945 thinkpad_acpi pcmcia uvcvideo parport_pc rtc_core snd_page_alloc video rfkill i2c_i801 mac80211 iTCO_wdt compat_ioctl32 rtc_lib yenta_socket pcspkr joydev ohci1394 snd_hwdep rsrc_nonstatic output i2c_core btusb parport battery led_class videodev ac ieee1394 v4l1_compat e1000e wmi iTCO_vendor_support pcmcia_core button snd soundcore intel_agp cfg80211 bluetooth sg sr_mod cdrom sd_mod crc_t10dif ehci_hcd uhci_hcd usbcore edd ext3 mbcache jbd fan ide_pci_generic ide_core ata_generic ata_piix ahci pata_acpi libata scsi_mod dock thermal processor
15:50:52 linux-pr0e kernel: Pid: 7, comm: events/0 Tainted: G 2.6.27-rc7-7.10-default #1
15:50:52 linux-pr0e kernel:
15:50:52 linux-pr0e kernel: Call Trace:
15:50:52 linux-pr0e kernel: [<ffffffff8020e41e>] show_trace_log_lvl+0x41/0x58
15:50:52 linux-pr0e kernel: [<ffffffff80493716>] dump_stack+0x69/0x6f
15:50:52 linux-pr0e kernel: [<ffffffff8023ee54>] warn_slowpath+0xb4/0xdc
15:50:52 linux-pr0e kernel: [<ffffffffa022ce2e>] e1000_acquire_swflag_ich8lan+0x5a/0xdc [e1000e]
15:50:52 linux-pr0e kernel: [<ffffffffa02317ba>] e1000e_read_phy_reg_igp+0x19/0x64 [e1000e]
15:50:52 linux-pr0e kernel: [<ffffffffa02319f8>] e1000e_phy_has_link_generic+0x50/0xcc [e1000e]
15:50:52 linux-pr0e kernel: [<ffffffffa02306f9>] e1000e_check_for_copper_link+0x24/0x86 [e1000e]
15:50:52 linux-pr0e kernel: [<ffffffffa0236982>] e1000_watchdog_task+0x5c/0x5eb [e1000e]
15:50:52 linux-pr0e kernel: [<ffffffff8024ecdb>] run_workqueue+0xa4/0x14c
15:50:52 linux-pr0e kernel: [<ffffffff8024ee5b>] worker_thread+0xd8/0xe7
15:50:52 linux-pr0e kernel: [<ffffffff80251fe5>] kthread+0x47/0x73
15:50:52 linux-pr0e kernel: [<ffffffff8020d7a9>] child_rip+0xa/0x11
15:50:52 linux-pr0e kernel:
15:50:52 linux-pr0e kernel: ---[ end trace 6f68a3c748ede326 ]---
15:51:25 linux-pr0e dhclient: DHCPDISCOVER on eth1 to 255.255.255.255 port 67 interval 3
15:51:28 linux-pr0e dhclient: DHCPDISCOVER on eth1 to 255.255.255.255 port 67 interval 8
15:51:36 linux-pr0e dhclient: DHCPDISCOVER on eth1 to 255.255.255.255 port 67 interval 13
15:51:49 linux-pr0e dhclient: DHCPDISCOVER on eth1 to 255.255.255.255 port 67 interval 13
15:52:02 linux-pr0e dhclient: DHCPDISCOVER on eth1 to 255.255.255.255 port 67 interval 18
15:52:15 linux-pr0e kernel: Machine check events logged
--
Jiri Kosina
SUSE Labs
--
Olaf Kirch
>
> 15:50:52 linux-pr0e kernel: WARNING: at drivers/net/e1000e/ich8lan.c:424 e1000_acquire_swflag_ich8lan+0x5a/0xdc [e1000e]()
> 15:50:52 linux-pr0e kernel: e1000e mutex contention. Owned by pid 4162
> 15:50:52 linux-pr0e kernel: [<ffffffff8020e41e>] show_trace_log_lvl+0x41/0x58
> 15:50:52 linux-pr0e kernel: [<ffffffff80493716>] dump_stack+0x69/0x6f
> 15:50:52 linux-pr0e kernel: [<ffffffff8023ee54>] warn_slowpath+0xb4/0xdc
> 15:50:52 linux-pr0e kernel: [<ffffffffa022ce2e>] e1000_acquire_swflag_ich8lan+0x5a/0xdc [e1000e]
> 15:50:52 linux-pr0e kernel: [<ffffffffa02317ba>] e1000e_read_phy_reg_igp+0x19/0x64 [e1000e]
> 15:50:52 linux-pr0e kernel: [<ffffffffa02319f8>] e1000e_phy_has_link_generic+0x50/0xcc [e1000e]
> 15:50:52 linux-pr0e kernel: [<ffffffffa02306f9>] e1000e_check_for_copper_link+0x24/0x86 [e1000e]
> 15:50:52 linux-pr0e kernel: [<ffffffffa0236982>] e1000_watchdog_task+0x5c/0x5eb [e1000e]
> 15:50:52 linux-pr0e kernel: [<ffffffff8024ecdb>] run_workqueue+0xa4/0x14c
> 15:50:52 linux-pr0e kernel: [<ffffffff8024ee5b>] worker_thread+0xd8/0xe7
> 15:50:52 linux-pr0e kernel: [<ffffffff80251fe5>] kthread+0x47/0x73
> 15:50:52 linux-pr0e kernel: [<ffffffff8020d7a9>] child_rip+0xa/0x11
Looks like the e1000 watchdog racing with some dhclient activity (upping the interface).
I just noticed that the driver actually uses register pages. So it looks like it's
possible to have something like this without the mutex:
process A selects page A
process B selects page B
process A writes to register at offset A'
So we may end up writing to the wrong register. I think I heard Vojtech mention
that the e1000e also has a register based interface to erase/rewrite the NVM
programmatically. Do we know at which offsets these registers live?
Olaf
--
Neo didn't bring down the Matrix. SOA did.
--soafacts.com
Brandeburg, Jesse
> Looks like the e1000 watchdog racing with some dhclient activity
> (upping the interface).
> I just noticed that the driver actually uses register pages. So it
> looks like it's possible to have something like this without the
> mutex:
>
> process A selects page A
> process B selects page B
> process A writes to register at offset A'
I think that is possible, which is why the mutex patch would be good for
the future. However we have not shown that to be happening as a root
cause, but I don't rule it out.
so, why now? Drivers since before the e1000/e1000e split had this same
code, with no reports of problems. This code has been heavily tested,
and one of the platforms easily reproducing this has been available for
3 years now (ich8), with code that is basically unchanged in the driver.
> So we may end up writing to the wrong register. I think I heard
> Vojtech mention
> that the e1000e also has a register based interface to erase/rewrite
> the NVM programmatically. Do we know at which offsets these registers
> live?
The flash control registers are documented in the ICH documentation, and
are located at physical memory location indicated by BAR1 in the config
space of device 0:19.0
I wonder if we couldn't put a check in to see if the value we end up
reading from the register controlling the operation matches the
operation we were expecting (read vs write vs block erase)
Olaf Kirch
> so, why now? Drivers since before the e1000/e1000e split had this same
> code, with no reports of problems. This code has been heavily tested,
> and one of the platforms easily reproducing this has been available for
> 3 years now (ich8), with code that is basically unchanged in the driver.
Possibly the dhcp client is doing something differently, or at a much higher
frequency. At any rate, it seems we're seeing this now even when we just
use init level 3, without X involvement. Karsten reports NVM corruption
after 34 reboots into init level 3.
> The flash control registers are documented in the ICH documentation, and
> are located at physical memory location indicated by BAR1 in the config
> space of device 0:19.0
>
> I wonder if we couldn't put a check in to see if the value we end up
> reading from the register controlling the operation matches the
> operation we were expecting (read vs write vs block erase)
That may help, yes.
Olaf
--
Neo didn't bring down the Matrix. SOA did.
--soafacts.com
--
Thomas Gleixner
> Olaf Kirch wrote:
> > Looks like the e1000 watchdog racing with some dhclient activity
> > (upping the interface).
>
> > I just noticed that the driver actually uses register pages. So it
> > looks like it's possible to have something like this without the
> > mutex:
> >
> > process A selects page A
> > process B selects page B
> > process A writes to register at offset A'
>
> I think that is possible, which is why the mutex patch would be good for
> the future. However we have not shown that to be happening as a root
> cause, but I don't rule it out.
Nevertheless I vote strongly for putting that check in _NOW_. It has
proven that there is concurrent access and that's definitely a bug by
all means.
> so, why now? Drivers since before the e1000/e1000e split had this same
> code, with no reports of problems. This code has been heavily tested,
> and one of the platforms easily reproducing this has been available for
> 3 years now (ich8), with code that is basically unchanged in the driver.
Well, timing of events changes slightly over time and we definitely
had some major changes in the last three years which influence timing
(high res timers, dynticks, NAPI ....)
Thanks,
tglx
Thomas Gleixner
> On Thursday 02 October 2008 18:27:12 Brandeburg, Jesse wrote:
> > so, why now? Drivers since before the e1000/e1000e split had this same
> > code, with no reports of problems. This code has been heavily tested,
> > and one of the platforms easily reproducing this has been available for
> > 3 years now (ich8), with code that is basically unchanged in the driver.
>
> Possibly the dhcp client is doing something differently, or at a much higher
> frequency. At any rate, it seems we're seeing this now even when we just
> use init level 3, without X involvement. Karsten reports NVM corruption
> after 34 reboots into init level 3.
Had Karsten the mutex patch applied or not ?
tglx
Olaf Kirch
> That was openSuse 11.1 without any patches
I should have said 11.1 beta1.
Olaf
--
Neo didn't bring down the Matrix. SOA did.
--soafacts.com
--
Olaf Kirch
> On Thu, 2 Oct 2008, Olaf Kirch wrote:
>
> > On Thursday 02 October 2008 18:27:12 Brandeburg, Jesse wrote:
> > > so, why now? Drivers since before the e1000/e1000e split had this same
> > > code, with no reports of problems. This code has been heavily tested,
> > > and one of the platforms easily reproducing this has been available for
> > > 3 years now (ich8), with code that is basically unchanged in the driver.
> >
> > Possibly the dhcp client is doing something differently, or at a much higher
> > frequency. At any rate, it seems we're seeing this now even when we just
> > use init level 3, without X involvement. Karsten reports NVM corruption
> > after 34 reboots into init level 3.
>
> Had Karsten the mutex patch applied or not ?
That was openSuse 11.1 without any patches
Olaf
--
Neo didn't bring down the Matrix. SOA did.
--soafacts.com
--
Jesse Brandeburg
by the driver after reset, and the swflag is probably not reset unless
management firmware clears it after 100ms.
Signed-off-by: Jesse Brandeburg <jesse.br...@intel.com>
---
drivers/net/e1000e/ich8lan.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/net/e1000e/ich8lan.c b/drivers/net/e1000e/ich8lan.c
index d8efba8..f4b6744 100644
--- a/drivers/net/e1000e/ich8lan.c
+++ b/drivers/net/e1000e/ich8lan.c
@@ -1778,6 +1778,9 @@ static s32 e1000_reset_hw_ich8lan(struct e1000_hw *hw)
ew32(CTRL, (ctrl | E1000_CTRL_RST));
msleep(20);
+ /* release the swflag because it is not reset by hardware reset */
+ e1000_release_swflag_ich8lan(hw);
+
ret_val = e1000e_get_auto_rd_done(hw);
if (ret_val) {
/*
--
Jesse Brandeburg
the SWFLAG bit for exclusive (to hardware and firmware) access to
the PHY and NVM (aka eeprom). These accesses could possibly call
msleep to wait for the resource which is not allowed from interrupt
context.
Signed-off-by: Jesse Brandeburg <jesse.br...@intel.com>
CC: Thomas Gleixner <tg...@linutronix.de>
---
drivers/net/e1000e/e1000.h | 2 ++
drivers/net/e1000e/netdev.c | 31 ++++++++++++++++++++++++++++---
2 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/drivers/net/e1000e/e1000.h b/drivers/net/e1000e/e1000.h
index f0c48a2..8087bda 100644
--- a/drivers/net/e1000e/e1000.h
+++ b/drivers/net/e1000e/e1000.h
@@ -284,6 +284,8 @@ struct e1000_adapter {
unsigned long led_status;
unsigned int flags;
+ struct work_struct downshift_task;
+ struct work_struct update_phy_task;
};
struct e1000_info {
diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 1f767fe..803545b 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -1115,6 +1115,14 @@ static void e1000_clean_rx_ring(struct e1000_adapter *adapter)
writel(0, adapter->hw.hw_addr + rx_ring->tail);
}
+static void e1000e_downshift_workaround(struct work_struct *work)
+{
+ struct e1000_adapter *adapter = container_of(work,
+ struct e1000_adapter, downshift_task);
+
+ e1000e_gig_downshift_workaround_ich8lan(&adapter->hw);
+}
+
/**
* e1000_intr_msi - Interrupt Handler
* @irq: interrupt number
@@ -1139,7 +1147,7 @@ static irqreturn_t e1000_intr_msi(int irq, void *data)
*/
if ((adapter->flags & FLAG_LSC_GIG_SPEED_DROP) &&
(!(er32(STATUS) & E1000_STATUS_LU)))
- e1000e_gig_downshift_workaround_ich8lan(hw);
+ schedule_work(&adapter->downshift_task);
/*
* 80003ES2LAN workaround-- For packet buffer work-around on
@@ -1205,7 +1213,7 @@ static irqreturn_t e1000_intr(int irq, void *data)
*/
if ((adapter->flags & FLAG_LSC_GIG_SPEED_DROP) &&
(!(er32(STATUS) & E1000_STATUS_LU)))
- e1000e_gig_downshift_workaround_ich8lan(hw);
+ schedule_work(&adapter->downshift_task);
/*
* 80003ES2LAN workaround--
@@ -2912,6 +2920,21 @@ static int e1000_set_mac(struct net_device *netdev, void *p)
return 0;
}
+/**
+ * e1000e_update_phy_task - work thread to update phy
+ * @work: pointer to our work struct
+ *
+ * this worker thread exists because we must acquire a
+ * semaphore to read the phy, which we could msleep while
+ * waiting for it, and we can't msleep in a timer.
+ **/
+static void e1000e_update_phy_task(struct work_struct *work)
+{
+ struct e1000_adapter *adapter = container_of(work,
+ struct e1000_adapter, update_phy_task);
+ e1000_get_phy_info(&adapter->hw);
+}
+
/*
* Need to wait a few seconds after link up to get diagnostic information from
* the phy
@@ -2919,7 +2942,7 @@ static int e1000_set_mac(struct net_device *netdev, void *p)
static void e1000_update_phy_info(unsigned long data)
{
struct e1000_adapter *adapter = (struct e1000_adapter *) data;
- e1000_get_phy_info(&adapter->hw);
+ schedule_work(&adapter->update_phy_task);
}
/**
@@ -4578,6 +4601,8 @@ static int __devinit e1000_probe(struct pci_dev *pdev,
INIT_WORK(&adapter->reset_task, e1000_reset_task);
INIT_WORK(&adapter->watchdog_task, e1000_watchdog_task);
+ INIT_WORK(&adapter->downshift_task, e1000e_downshift_workaround);
+ INIT_WORK(&adapter->update_phy_task, e1000e_update_phy_task);
/* Initialize link parameters. User can change them with ethtool */
adapter->hw.mac.autoneg = 1;
--
Jesse Brandeburg
This patch adds a mutex to the e1000e driver that would help
catch any collisions of two e1000e threads accessing hardware
at the same time.
description and patch updated by Jesse
Signed-off-by: Thomas Gleixner <tg...@linutronix.de>
Signed-off-by: Jesse Brandeburg <jesse.br...@intel.com>
---
drivers/net/e1000e/ich8lan.c | 17 +++++++++++++++++
1 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/drivers/net/e1000e/ich8lan.c b/drivers/net/e1000e/ich8lan.c
index f4b6744..0b6095b 100644
--- a/drivers/net/e1000e/ich8lan.c
+++ b/drivers/net/e1000e/ich8lan.c
@@ -380,6 +380,9 @@ static s32 e1000_get_variants_ich8lan(struct e1000_adapter *adapter)
return 0;
}
+static DEFINE_MUTEX(nvm_mutex);
+static pid_t nvm_owner = -1;
+
/**
* e1000_acquire_swflag_ich8lan - Acquire software control flag
* @hw: pointer to the HW structure
@@ -393,6 +396,15 @@ static s32 e1000_acquire_swflag_ich8lan(struct e1000_hw *hw)
u32 extcnf_ctrl;
u32 timeout = PHY_CFG_TIMEOUT;
+ WARN_ON(preempt_count());
+
+ if (!mutex_trylock(&nvm_mutex)) {
+ WARN(1, KERN_ERR "e1000e mutex contention. Owned by pid %d\n",
+ nvm_owner);
+ mutex_lock(&nvm_mutex);
+ }
+ nvm_owner = current->pid;
+
while (timeout) {
extcnf_ctrl = er32(EXTCNF_CTRL);
extcnf_ctrl |= E1000_EXTCNF_CTRL_SWFLAG;
@@ -407,6 +419,8 @@ static s32 e1000_acquire_swflag_ich8lan(struct e1000_hw *hw)
if (!timeout) {
hw_dbg(hw, "FW or HW has locked the resource for too long.\n");
+ nvm_owner = -1;
+ mutex_unlock(&nvm_mutex);
return -E1000_ERR_CONFIG;
}
@@ -428,6 +442,9 @@ static void e1000_release_swflag_ich8lan(struct e1000_hw *hw)
extcnf_ctrl = er32(EXTCNF_CTRL);
extcnf_ctrl &= ~E1000_EXTCNF_CTRL_SWFLAG;
ew32(EXTCNF_CTRL, extcnf_ctrl);
+
+ nvm_owner = -1;
+ mutex_unlock(&nvm_mutex);
}
/**
Jesse Brandeburg
---
drivers/net/e1000e/netdev.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 01e9558..b81c423 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -47,7 +47,7 @@
#include "e1000.h"
-#define DRV_VERSION "0.3.3.3-k4"
+#define DRV_VERSION "0.3.3.3-k6"
char e1000e_driver_name[] = "e1000e";
const char e1000e_driver_version[] = DRV_VERSION;
Jesse Brandeburg
has the adjust tbi stats function that required the addition
of the stats lock to begin with.
adding a mutex to acquire_swflag helped catch this one too.
Signed-off-by: Jesse Brandeburg <jesse.br...@intel.com>
CC: Thomas Gleixner <tg...@linutronix.de>
---
drivers/net/e1000e/e1000.h | 1 -
drivers/net/e1000e/netdev.c | 18 ------------------
2 files changed, 0 insertions(+), 19 deletions(-)
diff --git a/drivers/net/e1000e/e1000.h b/drivers/net/e1000e/e1000.h
index 8087bda..5ea6b60 100644
--- a/drivers/net/e1000e/e1000.h
+++ b/drivers/net/e1000e/e1000.h
@@ -257,7 +257,6 @@ struct e1000_adapter {
struct net_device *netdev;
struct pci_dev *pdev;
struct net_device_stats net_stats;
- spinlock_t stats_lock; /* prevent concurrent stats updates */
/* structs defined in e1000_hw.h */
struct e1000_hw hw;
diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 835b692..01e9558 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -2600,8 +2600,6 @@ static int __devinit e1000_sw_init(struct e1000_adapter *adapter)
/* Explicitly disable IRQ since the NIC can be in any state. */
e1000_irq_disable(adapter);
- spin_lock_init(&adapter->stats_lock);
-
set_bit(__E1000_DOWN, &adapter->state);
return 0;
@@ -2953,7 +2951,6 @@ void e1000e_update_stats(struct e1000_adapter *adapter)
{
struct e1000_hw *hw = &adapter->hw;
struct pci_dev *pdev = adapter->pdev;
- unsigned long irq_flags;
/*
* Prevent stats update while adapter is being reset, or if the pci
@@ -2964,14 +2961,6 @@ void e1000e_update_stats(struct e1000_adapter *adapter)
if (pci_channel_offline(pdev))
return;
- spin_lock_irqsave(&adapter->stats_lock, irq_flags);
-
--
Jesse Brandeburg
this patch removes the phy read from inside a spinlock, paving
the way for removing the spinlock completely. The phy read was
only feeding a statistic that wasn't used.
Signed-off-by: Jesse Brandeburg <jesse.br...@intel.com>
CC: Thomas Gleixner <tg...@linutronix.de>
---
drivers/net/e1000e/ethtool.c | 6 +++++-
drivers/net/e1000e/netdev.c | 13 -------------
2 files changed, 5 insertions(+), 14 deletions(-)
diff --git a/drivers/net/e1000e/ethtool.c b/drivers/net/e1000e/ethtool.c
index 5ed8e13..33a3ff1 100644
--- a/drivers/net/e1000e/ethtool.c
+++ b/drivers/net/e1000e/ethtool.c
@@ -432,6 +432,10 @@ static void e1000_get_regs(struct net_device *netdev,
regs_buff[11] = er32(TIDV);
regs_buff[12] = adapter->hw.phy.type; /* PHY type (IGP=1, M88=0) */
+
+ /* ethtool doesn't use anything past this point, so all this
+ * code is likely legacy junk for apps that may or may not
+ * exist */
if (hw->phy.type == e1000_phy_m88) {
e1e_rphy(hw, M88E1000_PHY_SPEC_STATUS, &phy_data);
regs_buff[13] = (u32)phy_data; /* cable length */
@@ -447,7 +451,7 @@ static void e1000_get_regs(struct net_device *netdev,
regs_buff[22] = adapter->phy_stats.receive_errors;
regs_buff[23] = regs_buff[13]; /* mdix mode */
}
- regs_buff[21] = adapter->phy_stats.idle_errors; /* phy idle errors */
+ regs_buff[21] = 0; /* was idle_errors */
e1e_rphy(hw, PHY_1000T_STATUS, &phy_data);
regs_buff[24] = (u32)phy_data; /* phy local receiver status */
regs_buff[25] = regs_buff[24]; /* phy remote receiver status */
diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 803545b..835b692 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -2954,9 +2954,6 @@ void e1000e_update_stats(struct e1000_adapter *adapter)
struct e1000_hw *hw = &adapter->hw;
struct pci_dev *pdev = adapter->pdev;
unsigned long irq_flags;
- u16 phy_tmp;
-
-#define PHY_IDLE_ERROR_COUNT_MASK 0x00FF
/*
* Prevent stats update while adapter is being reset, or if the pci
@@ -3045,15 +3042,6 @@ void e1000e_update_stats(struct e1000_adapter *adapter)
/* Tx Dropped needs to be maintained elsewhere */
- /* Phy Stats */
- if (hw->phy.media_type == e1000_media_type_copper) {
- if ((adapter->link_speed == SPEED_1000) &&
- (!e1e_rphy(hw, PHY_1000T_STATUS, &phy_tmp))) {
- phy_tmp &= PHY_IDLE_ERROR_COUNT_MASK;
- adapter->phy_stats.idle_errors += phy_tmp;
- }
- }
-
/* Management Stats */
adapter->stats.mgptc += er32(MGTPTC);
adapter->stats.mgprc += er32(MGTPRC);
@@ -3073,7 +3061,6 @@ static void e1000_phy_read_status(struct e1000_adapter *adapter)
int ret_val;
unsigned long irq_flags;
-
spin_lock_irqsave(&adapter->stats_lock, irq_flags);
if ((er32(STATUS) & E1000_STATUS_LU) &&
--
Thomas Gleixner
Linus,
can you please apply the patch below which prevents the concurrent
access to the NVRAM. It triggered the trace which Olaf reported above.
I worked out that patch on Sept 24th and it triggered a couple of
problems in the e1000e code right away. These have been addressed by
Jesse and are part of the patch series he posted in the last days.
Still, all we have in mainline is some "hopefully bug preventing"
patch which does not address the root cause at all.
The confirmed bugs where the nvram acquire code was called
concurrently are still in your tree and the prevention patch along
with the resulting bugfixes are stuck in some obscure intel QA
process.
Please apply at least the bug prevention patch below.
Thanks,
tglx
---
Subject: e1000e prevent concurrent access and debug contention on NVM SWFALG
Date: Wed, 24 Sep 2008 00:45:08 -0700
From: Thomas Gleixner <tg...@linutronix.de>
Signed-off-by: Thomas Gleixner <tg...@linutronix.de>
Cc: jesse.br...@intel.com
---
drivers/net/e1000e/ich8lan.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
Index: linux-2.6/drivers/net/e1000e/ich8lan.c
===================================================================
--- linux-2.6.orig/drivers/net/e1000e/ich8lan.c
+++ linux-2.6/drivers/net/e1000e/ich8lan.c
@@ -382,6 +382,9 @@ static s32 e1000_get_variants_ich8lan(st
return 0;
}
+static DEFINE_MUTEX(nvm_mutex);
+static pid_t nvm_owner = -1;
+
/**
* e1000_acquire_swflag_ich8lan - Acquire software control flag
* @hw: pointer to the HW structure
@@ -395,6 +398,15 @@ static s32 e1000_acquire_swflag_ich8lan(
u32 extcnf_ctrl;
u32 timeout = PHY_CFG_TIMEOUT;
+ WARN_ON(preempt_count());
+
+ if (!mutex_trylock(&nvm_mutex)) {
+ WARN(1, KERN_ERR "e1000e mutex contention. Owned by pid %d\n",
+ nvm_owner);
+ mutex_lock(&nvm_mutex);
+ }
+ nvm_owner = current->pid;
+
while (timeout) {
extcnf_ctrl = er32(EXTCNF_CTRL);
extcnf_ctrl |= E1000_EXTCNF_CTRL_SWFLAG;
@@ -409,6 +421,8 @@ static s32 e1000_acquire_swflag_ich8lan(
if (!timeout) {
hw_dbg(hw, "FW or HW has locked the resource for too long.\n");
+ nvm_owner = -1;
+ mutex_unlock(&nvm_mutex);
return -E1000_ERR_CONFIG;
}
@@ -430,6 +444,9 @@ static void e1000_release_swflag_ich8lan
extcnf_ctrl = er32(EXTCNF_CTRL);
extcnf_ctrl &= ~E1000_EXTCNF_CTRL_SWFLAG;
ew32(EXTCNF_CTRL, extcnf_ctrl);
+
+ nvm_owner = -1;
+ mutex_unlock(&nvm_mutex);
}
/**
Jesse Brandeburg
> The confirmed bugs where the nvram acquire code was called
> concurrently are still in your tree and the prevention patch along
> with the resulting bugfixes are stuck in some obscure intel QA
> process.
>
> Please apply at least the bug prevention patch below.
This is the same patch I posted 7 minutes ago, except that this patch
without the e1000e changes applied before it will cause all sorts of
WARN's to be printed during normal operation. If at all possible I
think they should stay together as a group to prevent un-necessary
noise in the logs.
Thomas Gleixner
> On Thu, Oct 2, 2008 at 4:42 PM, Thomas Gleixner <tg...@linutronix.de> wrote:
> > The confirmed bugs where the nvram acquire code was called
> > concurrently are still in your tree and the prevention patch along
> > with the resulting bugfixes are stuck in some obscure intel QA
> > process.
> >
> > Please apply at least the bug prevention patch below.
>
> This is the same patch I posted 7 minutes ago, except that this patch
> without the e1000e changes applied before it will cause all sorts of
> WARN's to be printed during normal operation. If at all possible I
> think they should stay together as a group to prevent un-necessary
> noise in the logs.
Sure, I'm all for the combo of those. I just wanted to get some motion
into this stale process.
tglx
Thomas Gleixner
> thanks to tglx, we're finding some interesting reentrancy issues.
> this patch removes the phy read from inside a spinlock, paving
> the way for removing the spinlock completely. The phy read was
> only feeding a statistic that wasn't used.
>
> Signed-off-by: Jesse Brandeburg <jesse.br...@intel.com>
> CC: Thomas Gleixner <tg...@linutronix.de>
Acked-by: Thomas Gleixner <tg...@linutronix.de>
Thomas Gleixner
On Thu, 2 Oct 2008, Jesse Brandeburg wrote:
> e1000e was apparently calling two functions that attempted to reserve
> the SWFLAG bit for exclusive (to hardware and firmware) access to
> the PHY and NVM (aka eeprom). These accesses could possibly call
> msleep to wait for the resource which is not allowed from interrupt
> context.
>
> Signed-off-by: Jesse Brandeburg <jesse.br...@intel.com>
> CC: Thomas Gleixner <tg...@linutronix.de>
Acked-by: Thomas Gleixner <tg...@linutronix.de>
Tested-by: Thomas Gleixner <tg...@linutronix.de>
Thomas Gleixner
On Thu, 2 Oct 2008, Jesse Brandeburg wrote:
> the stats lock is left over from e1000, e1000e no longer
> has the adjust tbi stats function that required the addition
> of the stats lock to begin with.
>
> adding a mutex to acquire_swflag helped catch this one too.
>
> Signed-off-by: Jesse Brandeburg <jesse.br...@intel.com>
> CC: Thomas Gleixner <tg...@linutronix.de>
Acked-by: Thomas Gleixner <tg...@linutronix.de>
Jiri Kosina
> From: Thomas Gleixner <tg...@linutronix.de>
>
> This patch adds a mutex to the e1000e driver that would help
> catch any collisions of two e1000e threads accessing hardware
> at the same time.
Apparently this has some bug wrt suspend, see
https://bugzilla.novell.com/show_bug.cgi?id=431914
WARNING: at drivers/net/e1000e/ich8lan.c:424
e1000_acquire_swflag_ich8lan+0x56/0xcb [e1000e]()
e1000e mutex contention. Owned by pid -1
Modules linked in: xt_tcpudp xt_pkttype tun ipt_ULOG xt_limit aes_i586
aes_generic i915 drm af_packet snd_pcm_oss snd_mixer_oss snd_seq
snd_seq_device ipt_REJECT xt_state cpufreq_conservative iptable_mangle
cpufreq_userspace iptable_nat cpufreq_powersave nf_nat acpi_cpufreq
iptable_filter speedstep_lib nf_conntrack_ipv4 nf_conntrack ip_tables
ip6_tables x_tables microcode loop arc4 ecb crypto_blkcipher snd_hda_intel
iwl3945 pcmcia thinkpad_acpi snd_pcm snd_timer sdhci_pci snd_page_alloc
rfkill sdhci snd_hwdep mac80211 yenta_socket rsrc_nonstatic video rtc_cmos
led_class ohci1394 snd ieee1394 output mmc_core i2c_i801 ac battery
pcmcia_core iTCO_wdt button intel_agp rtc_core cfg80211 nvram soundcore
iTCO_vendor_support agpgart e1000e rtc_lib i2c_core pcspkr uinput sg
sd_mod ehci_hcd uhci_hcd crc_t10dif usbcore edd ext3 mbcache jbd fan
ata_piix ahci libata scsi_mod dock thermal processor
Pid: 23153, comm: events/1 Tainted: G W
2.6.27-rc8-HEAD_20081001123454-pae #1
[<c0105590>] dump_trace+0x6b/0x249
[<c01060c5>] show_trace+0x20/0x39
[<c033b52d>] dump_stack+0x71/0x76
[<c012ba12>] warn_slowpath+0x6f/0x90
[<f91cfb9b>] e1000_acquire_swflag_ich8lan+0x56/0xcb [e1000e]
[<f91d3db4>] e1000e_read_phy_reg_igp+0x10/0x4f [e1000e]
[<f91d3f83>] e1000e_phy_has_link_generic+0x32/0x99 [e1000e]
[<f91d2e35>] e1000e_check_for_copper_link+0x26/0x80 [e1000e]
[<f91d8f3a>] e1000_watchdog_task+0x5b/0x5eb [e1000e]
[<c013a1b4>] run_workqueue+0x9f/0x13e
[<c013a309>] worker_thread+0xb6/0xc2
[<c013cdff>] kthread+0x38/0x5d
[<c01050e7>] kernel_thread_helper+0x7/0x10
The debugging message is racy anyway with respect to accessing nvm_owner,
right? It should be done after the mutex has been succesfully acquired.
--
Jiri Kosina
SUSE Labs
--
Linus Torvalds
On Fri, 3 Oct 2008, Jiri Kosina wrote:
>
> The debugging message is racy anyway with respect to accessing nvm_owner,
> right? It should be done after the mutex has been succesfully acquired.
It's done that way on purpose - to see who _could_ be racing.
After the mutex, it could never trigger, because the mutex is the thing
that guarantees non-racy-ness.
IOW, it's a debugging message just to see that the old bug (the "before
the fix") really did happen. We can/will remove it, but I think people are
still looking at the e1000e driver and probably want to see the paths that
can cause problems.
Of course, it's entirely possible that we should remove it in mainline
already, and just let the people inside intel/suse/xyzzy who are trying to
reproduce it have it.
Jesse? Thomas?
Linus
Jiri Kosina
> > The debugging message is racy anyway with respect to accessing
> > nvm_owner, right? It should be done after the mutex has been
> > succesfully acquired.
> It's done that way on purpose - to see who _could_ be racing.
I know. I just wanted to point out that we probably don't want the patch
in 2.6.27 in this form, users wouldn't like to have warning in their logs
every time mutex is not acquired on a first attempt :)
We are currently running reproduction tests on affected machines to verify
that this patch really fixes the root cause of this whole e1000e saga.
--
Jiri Kosina
SUSE Labs
Thomas Gleixner
> On Fri, 3 Oct 2008, Linus Torvalds wrote:
>
> > > The debugging message is racy anyway with respect to accessing
> > > nvm_owner, right? It should be done after the mutex has been
> > > succesfully acquired.
> > It's done that way on purpose - to see who _could_ be racing.
>
> I know. I just wanted to point out that we probably don't want the patch
> in 2.6.27 in this form, users wouldn't like to have warning in their logs
> every time mutex is not acquired on a first attempt :)
Yeah, it should go before .27 final. It now just gathers data as
people actually care about warn_ons or they are even caught
automatically via kernel oops.
Thanks,
tglx