Subject: [patch] ACPI, i686, x86_64: fix laptop bootup hang in init_acpi()
From: Ingo Molnar <[email protected]>
during kernel bootup, a new T60 laptop (CoreDuo, 32-bit) hangs about
10%-20% of the time in acpi_init():
Calling initcall 0xc055ce1a: topology_init+0x0/0x2f()
Calling initcall 0xc055d75e: mtrr_init_finialize+0x0/0x2c()
Calling initcall 0xc05664f3: param_sysfs_init+0x0/0x175()
Calling initcall 0xc014cb65: pm_sysrq_init+0x0/0x17()
Calling initcall 0xc0569f99: init_bio+0x0/0xf4()
Calling initcall 0xc056b865: genhd_device_init+0x0/0x50()
Calling initcall 0xc056c4bd: fbmem_init+0x0/0x87()
Calling initcall 0xc056dd74: acpi_init+0x0/0x1ee()
it's a hard hang that not even an NMI could punch through!
Frustratingly, adding printks or function tracing to the ACPI code made
the hangs go away ...
after some time an additional detail emerged: disabling the NMI watchdog
made these occasional hangs go away.
So i spent the better part of today trying to debug this and trying out
various theories when i finally found the likely reason for the hang: if
acpi_ns_initialize_devices() executes an _INI AML method and an NMI
happens to hit that AML execution in the wrong moment, the machine would
hang. (my theory is that this must be some sort of chipset setup method
doing stores to chipset mmio registers?)
Unfortunately given the characteristics of the hang it was sheer
impossible to figure out which of the numerous AML methods is impacted
by this problem.
as a workaround i wrote an interface to disable chipset-based NMIs while
executing _INI sections - and indeed this fixed the hang. I did a
boot-loop of 100 separate reboots and none hung - while without the
patch it would hang every 5-10 attempts. Out of caution i did not touch
the nmi_watchdog=2 case (it's not related to the chipset anyway and
didnt hang).
I implemented this for both x86_64 and i686, tested the i686 laptop both
with nmi_watchdog=1 [which triggered the hangs] and nmi_watchdog=2, and
tested an Athlon64 box with the 64-bit kernel as well. Everything builds
and works with the patch applied.
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/i386/kernel/nmi.c | 28 ++++++++++++++++++++++++++++
arch/x86_64/kernel/nmi.c | 27 +++++++++++++++++++++++++++
drivers/acpi/namespace/nsinit.c | 9 +++++++++
include/linux/nmi.h | 9 ++++++++-
4 files changed, 72 insertions(+), 1 deletion(-)
Index: linux/arch/i386/kernel/nmi.c
===================================================================
--- linux.orig/arch/i386/kernel/nmi.c
+++ linux/arch/i386/kernel/nmi.c
@@ -376,6 +376,34 @@ void enable_timer_nmi_watchdog(void)
}
}
+static void __acpi_nmi_disable(void *__unused)
+{
+ apic_write_around(APIC_LVT0, APIC_DM_NMI | APIC_LVT_MASKED);
+}
+
+/*
+ * Disable timer based NMIs on all CPUs:
+ */
+void acpi_nmi_disable(void)
+{
+ if (atomic_read(&nmi_active) && nmi_watchdog == NMI_IO_APIC)
+ on_each_cpu(__acpi_nmi_disable, NULL, 0, 1);
+}
+
+static void __acpi_nmi_enable(void *__unused)
+{
+ apic_write_around(APIC_LVT0, APIC_DM_NMI);
+}
+
+/*
+ * Enable timer based NMIs on all CPUs:
+ */
+void acpi_nmi_enable(void)
+{
+ if (atomic_read(&nmi_active) && nmi_watchdog == NMI_IO_APIC)
+ on_each_cpu(__acpi_nmi_enable, NULL, 0, 1);
+}
+
#ifdef CONFIG_PM
static int nmi_pm_active; /* nmi_active before suspend */
Index: linux/arch/x86_64/kernel/nmi.c
===================================================================
--- linux.orig/arch/x86_64/kernel/nmi.c
+++ linux/arch/x86_64/kernel/nmi.c
@@ -360,6 +360,33 @@ void enable_timer_nmi_watchdog(void)
}
}
+static void __acpi_nmi_disable(void *__unused)
+{
+ apic_write(APIC_LVT0, APIC_DM_NMI | APIC_LVT_MASKED);
+}
+
+/*
+ * Disable timer based NMIs on all CPUs:
+ */
+void acpi_nmi_disable(void)
+{
+ if (atomic_read(&nmi_active) && nmi_watchdog == NMI_IO_APIC)
+ on_each_cpu(__acpi_nmi_disable, NULL, 0, 1);
+}
+
+static void __acpi_nmi_enable(void *__unused)
+{
+ apic_write(APIC_LVT0, APIC_DM_NMI);
+}
+
+/*
+ * Enable timer based NMIs on all CPUs:
+ */
+void acpi_nmi_enable(void)
+{
+ if (atomic_read(&nmi_active) && nmi_watchdog == NMI_IO_APIC)
+ on_each_cpu(__acpi_nmi_enable, NULL, 0, 1);
+}
#ifdef CONFIG_PM
static int nmi_pm_active; /* nmi_active before suspend */
Index: linux/drivers/acpi/namespace/nsinit.c
===================================================================
--- linux.orig/drivers/acpi/namespace/nsinit.c
+++ linux/drivers/acpi/namespace/nsinit.c
@@ -45,6 +45,7 @@
#include <acpi/acnamesp.h>
#include <acpi/acdispat.h>
#include <acpi/acinterp.h>
+#include <linux/nmi.h>
#define _COMPONENT ACPI_NAMESPACE
ACPI_MODULE_NAME("nsinit")
@@ -537,7 +538,15 @@ acpi_ns_init_one_device(acpi_handle obj_
info->parameter_type = ACPI_PARAM_ARGS;
info->flags = ACPI_IGNORE_RETURN_VALUE;
+ /*
+ * Some hardware relies on this being executed as atomically
+ * as possible (without an NMI being received in the middle of
+ * this) - so disable NMIs and initialize the device:
+ */
+ acpi_nmi_disable();
status = acpi_ns_evaluate(info);
+ acpi_nmi_enable();
+
if (ACPI_SUCCESS(status)) {
walk_info->num_INI++;
Index: linux/include/linux/nmi.h
===================================================================
--- linux.orig/include/linux/nmi.h
+++ linux/include/linux/nmi.h
@@ -16,8 +16,15 @@
*/
#ifdef ARCH_HAS_NMI_WATCHDOG
extern void touch_nmi_watchdog(void);
+extern void acpi_nmi_disable(void);
+extern void acpi_nmi_enable(void);
#else
-# define touch_nmi_watchdog() touch_softlockup_watchdog()
+static inline void touch_nmi_watchdog(void)
+{
+ touch_softlockup_watchdog();
+}
+static inline void acpi_nmi_disable(void) { }
+static inline void acpi_nmi_enable(void) { }
#endif
#endif
On Wednesday 06 December 2006 17:30, Ingo Molnar wrote:
> Subject: [patch] ACPI, i686, x86_64: fix laptop bootup hang in init_acpi()
> From: Ingo Molnar <[email protected]>
>
> during kernel bootup, a new T60 laptop (CoreDuo, 32-bit) hangs about
> 10%-20% of the time in acpi_init():
>
> Calling initcall 0xc055ce1a: topology_init+0x0/0x2f()
> Calling initcall 0xc055d75e: mtrr_init_finialize+0x0/0x2c()
> Calling initcall 0xc05664f3: param_sysfs_init+0x0/0x175()
> Calling initcall 0xc014cb65: pm_sysrq_init+0x0/0x17()
> Calling initcall 0xc0569f99: init_bio+0x0/0xf4()
> Calling initcall 0xc056b865: genhd_device_init+0x0/0x50()
> Calling initcall 0xc056c4bd: fbmem_init+0x0/0x87()
> Calling initcall 0xc056dd74: acpi_init+0x0/0x1ee()
>
> it's a hard hang that not even an NMI could punch through!
> Frustratingly, adding printks or function tracing to the ACPI code made
> the hangs go away ...
>
> after some time an additional detail emerged: disabling the NMI watchdog
> made these occasional hangs go away.
>
> So i spent the better part of today trying to debug this and trying out
> various theories when i finally found the likely reason for the hang: if
> acpi_ns_initialize_devices() executes an _INI AML method and an NMI
> happens to hit that AML execution in the wrong moment, the machine would
> hang. (my theory is that this must be some sort of chipset setup method
> doing stores to chipset mmio registers?)
>
> Unfortunately given the characteristics of the hang it was sheer
> impossible to figure out which of the numerous AML methods is impacted
> by this problem.
>
> as a workaround i wrote an interface to disable chipset-based NMIs while
> executing _INI sections - and indeed this fixed the hang. I did a
> boot-loop of 100 separate reboots and none hung - while without the
> patch it would hang every 5-10 attempts. Out of caution i did not touch
> the nmi_watchdog=2 case (it's not related to the chipset anyway and
> didnt hang).
>
> I implemented this for both x86_64 and i686, tested the i686 laptop both
> with nmi_watchdog=1 [which triggered the hangs] and nmi_watchdog=2, and
> tested an Athlon64 box with the 64-bit kernel as well. Everything builds
> and works with the patch applied.
Good work root-causing this failure!
Personally I have never been a big fan of having the NMI watchdog
running by default on all systems -- but Andi insists that it helps him
debug failures, so tick it does...
Clearly this laptop was validated with Windows, and Windows doesn't
have this problem -- suggesting that we may be somewhat out on a limb...
Some alternatives to consider...
a. don't enable NMI watchdog by default
b. enable NMI watchdog, but later during kernel boot
(assumption here that the issue is only during _INI)
c. disable the NMI whenever the ACPI interpeter is running
(who knows, maybe this isn't limited to the _INI case, but
could cause a hang at some other time -- only the
BIOS AML writers knows....)
thoughts?
-Len
> Signed-off-by: Ingo Molnar <[email protected]>
> ---
> arch/i386/kernel/nmi.c | 28 ++++++++++++++++++++++++++++
> arch/x86_64/kernel/nmi.c | 27 +++++++++++++++++++++++++++
> drivers/acpi/namespace/nsinit.c | 9 +++++++++
> include/linux/nmi.h | 9 ++++++++-
> 4 files changed, 72 insertions(+), 1 deletion(-)
>
> Index: linux/arch/i386/kernel/nmi.c
> ===================================================================
> --- linux.orig/arch/i386/kernel/nmi.c
> +++ linux/arch/i386/kernel/nmi.c
> @@ -376,6 +376,34 @@ void enable_timer_nmi_watchdog(void)
> }
> }
>
> +static void __acpi_nmi_disable(void *__unused)
> +{
> + apic_write_around(APIC_LVT0, APIC_DM_NMI | APIC_LVT_MASKED);
> +}
> +
> +/*
> + * Disable timer based NMIs on all CPUs:
> + */
> +void acpi_nmi_disable(void)
> +{
> + if (atomic_read(&nmi_active) && nmi_watchdog == NMI_IO_APIC)
> + on_each_cpu(__acpi_nmi_disable, NULL, 0, 1);
> +}
> +
> +static void __acpi_nmi_enable(void *__unused)
> +{
> + apic_write_around(APIC_LVT0, APIC_DM_NMI);
> +}
> +
> +/*
> + * Enable timer based NMIs on all CPUs:
> + */
> +void acpi_nmi_enable(void)
> +{
> + if (atomic_read(&nmi_active) && nmi_watchdog == NMI_IO_APIC)
> + on_each_cpu(__acpi_nmi_enable, NULL, 0, 1);
> +}
> +
> #ifdef CONFIG_PM
>
> static int nmi_pm_active; /* nmi_active before suspend */
> Index: linux/arch/x86_64/kernel/nmi.c
> ===================================================================
> --- linux.orig/arch/x86_64/kernel/nmi.c
> +++ linux/arch/x86_64/kernel/nmi.c
> @@ -360,6 +360,33 @@ void enable_timer_nmi_watchdog(void)
> }
> }
>
> +static void __acpi_nmi_disable(void *__unused)
> +{
> + apic_write(APIC_LVT0, APIC_DM_NMI | APIC_LVT_MASKED);
> +}
> +
> +/*
> + * Disable timer based NMIs on all CPUs:
> + */
> +void acpi_nmi_disable(void)
> +{
> + if (atomic_read(&nmi_active) && nmi_watchdog == NMI_IO_APIC)
> + on_each_cpu(__acpi_nmi_disable, NULL, 0, 1);
> +}
> +
> +static void __acpi_nmi_enable(void *__unused)
> +{
> + apic_write(APIC_LVT0, APIC_DM_NMI);
> +}
> +
> +/*
> + * Enable timer based NMIs on all CPUs:
> + */
> +void acpi_nmi_enable(void)
> +{
> + if (atomic_read(&nmi_active) && nmi_watchdog == NMI_IO_APIC)
> + on_each_cpu(__acpi_nmi_enable, NULL, 0, 1);
> +}
> #ifdef CONFIG_PM
>
> static int nmi_pm_active; /* nmi_active before suspend */
> Index: linux/drivers/acpi/namespace/nsinit.c
> ===================================================================
> --- linux.orig/drivers/acpi/namespace/nsinit.c
> +++ linux/drivers/acpi/namespace/nsinit.c
> @@ -45,6 +45,7 @@
> #include <acpi/acnamesp.h>
> #include <acpi/acdispat.h>
> #include <acpi/acinterp.h>
> +#include <linux/nmi.h>
>
> #define _COMPONENT ACPI_NAMESPACE
> ACPI_MODULE_NAME("nsinit")
> @@ -537,7 +538,15 @@ acpi_ns_init_one_device(acpi_handle obj_
> info->parameter_type = ACPI_PARAM_ARGS;
> info->flags = ACPI_IGNORE_RETURN_VALUE;
>
> + /*
> + * Some hardware relies on this being executed as atomically
> + * as possible (without an NMI being received in the middle of
> + * this) - so disable NMIs and initialize the device:
> + */
> + acpi_nmi_disable();
> status = acpi_ns_evaluate(info);
> + acpi_nmi_enable();
> +
> if (ACPI_SUCCESS(status)) {
> walk_info->num_INI++;
>
> Index: linux/include/linux/nmi.h
> ===================================================================
> --- linux.orig/include/linux/nmi.h
> +++ linux/include/linux/nmi.h
> @@ -16,8 +16,15 @@
> */
> #ifdef ARCH_HAS_NMI_WATCHDOG
> extern void touch_nmi_watchdog(void);
> +extern void acpi_nmi_disable(void);
> +extern void acpi_nmi_enable(void);
> #else
> -# define touch_nmi_watchdog() touch_softlockup_watchdog()
> +static inline void touch_nmi_watchdog(void)
> +{
> + touch_softlockup_watchdog();
> +}
> +static inline void acpi_nmi_disable(void) { }
> +static inline void acpi_nmi_enable(void) { }
> #endif
>
> #endif
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
On Wed, 2006-12-06 at 23:30 +0100, Ingo Molnar wrote:
> Subject: [patch] ACPI, i686, x86_64: fix laptop bootup hang in init_acpi()
> From: Ingo Molnar <[email protected]>
Hi Ingo,
Just curiosity ,have we this patch on 2.6.19-1.rt6 ?
Thanks,
--
S?rgio M.B.
Am Donnerstag, 7. Dezember 2006 03:28 schrieb Sergio Monteiro Basto:
> On Wed, 2006-12-06 at 23:30 +0100, Ingo Molnar wrote:
> > Subject: [patch] ACPI, i686, x86_64: fix laptop bootup hang in init_acpi()
> > From: Ingo Molnar <[email protected]>
> Hi Ingo,
> Just curiosity ,have we this patch on 2.6.19-1.rt6 ?
No, its in rt7.
Karsten
* Len Brown <[email protected]> wrote:
> c. disable the NMI whenever the ACPI interpeter is running
> (who knows, maybe this isn't limited to the _INI case, but
> could cause a hang at some other time -- only the
> BIOS AML writers knows....)
i have tested this by forcing the NMI frequency to 10,000 per second,
and never saw any other problem. So at least this particular laptop
should be OK.
So i /think/ this should be enough - the _INI case should be limited to
bootup - or can it trigger during module load too? The IO-APIC based NMI
watchdog should really only involve the southbridge (whose
initialization package has this problem, in my guesstimation - do you
agree?) and not random other devices - so once we have booted up we
should be fine from this particular issue. acpi_nmi_disable()/enable()
does a cross-IPI to all CPUs, so it can be quite heavy-handed - i'm not
sure we want it for every interpreter invocation.
Ingo
* Karsten Wiese <[email protected]> wrote:
> Am Donnerstag, 7. Dezember 2006 03:28 schrieb Sergio Monteiro Basto:
> > On Wed, 2006-12-06 at 23:30 +0100, Ingo Molnar wrote:
> > > Subject: [patch] ACPI, i686, x86_64: fix laptop bootup hang in init_acpi()
> > > From: Ingo Molnar <[email protected]>
> > Hi Ingo,
> > Just curiosity ,have we this patch on 2.6.19-1.rt6 ?
>
> No, its in rt7.
yeah, it's in -rt7. [ -rt6 is more than 1 day old now, so it's truly
ancient stuff ;-) ]
Ingo
* Ingo Molnar <[email protected]> wrote:
> * Karsten Wiese <[email protected]> wrote:
>
> > Am Donnerstag, 7. Dezember 2006 03:28 schrieb Sergio Monteiro Basto:
> > > On Wed, 2006-12-06 at 23:30 +0100, Ingo Molnar wrote:
> > > > Subject: [patch] ACPI, i686, x86_64: fix laptop bootup hang in init_acpi()
> > > > From: Ingo Molnar <[email protected]>
> > > Hi Ingo,
> > > Just curiosity ,have we this patch on 2.6.19-1.rt6 ?
> >
> > No, its in rt7.
>
> yeah, it's in -rt7. [ -rt6 is more than 1 day old now, so it's truly
> ancient stuff ;-) ]
did i really say -rt7? The ancient -rt7 stuff is 12+ hours old already,
and there's the 1 minute old -rt8 available (patch and yum rpms too).
Sheesh ... ;-)
Ingo
* Len Brown <[email protected]> wrote:
> Personally I have never been a big fan of having the NMI watchdog
> running by default on all systems -- but Andi insists that it helps
> him debug failures, so tick it does...
enabling it by default was IMO a really bad decision and it needs to be
undone via the patch attached further below.
If Andi wants to debug stuff via the NMI wachdog, he should use the
nmi_watchdog=2 boot option: next to the tty=ttyS0 serial console
options, initcall_debug, apic=debug, earlyprintk and myriads of other
kernel-hackers-only boot options that we all use to 'help debug
failures' ...
also, lock debugging facilities catch lockup possibilities (and actual
lockups) alot more efficiently, i cannot remember the last time the NMI
watchdog caught anything on my boxes without some other facility not
triggering first. (and i have it enabled everywhere)
I have run a quick analysis over all locking related crashes i triggered
on one particular testbox over the past 2 weeks. Out of 26 separate lock
related incidents spread out equally over that timeframe (out of 497
bootups on this box), this was the distribution of debugging facilities
that caught the bugs:
1 BUG: spinlock lockup on
1 [ INFO: inconsistent lock state ]
2 BUG: scheduling while atomic
2 [ BUG: bad unlock balance detected! ]
2 BUG: sleeping function called from invalid context
6 BUG: scheduling with irqs disabled
5 [ INFO: hard-safe -> hard-unsafe lock order detected ]
7 BUG: using smp_processor_id() in preemptible [] code
8 were caught by lockdep, 8 by atomicity checks in the scheduler, 7 by
DEBUG_PREEMPT and 1 by DEBUG_SPINLOCK.
Note: zero were caught by the NMI watchdog, and i run the NMI watchdog
enabled by default on all architectures, and i have serial logging of
everything.
but even for the typical distro kernel and for the typical user, the NMI
watchdog is normally useless, because NMI lockups rarely make it into
the syslog and X just locks up without showing anything on the screen.
Ingo
----------------------->
Subject: [patch] x86_64: do not enable the NMI watchdog by default
From: Ingo Molnar <[email protected]>
do not enable the NMI watchdog by default. Now that we have lockdep i
cannot remember the last time it caught a real bug, but the NMI watchdog
can /cause/ problems. Furthermore, to the typical user, an NMI watchdog
assert results in a total lockup anyway (if under X). In that sense, all
that the NMI watchdog does is that it makes the system /less/ stable and
/less/ debuggable.
people can still enable it either after bootup via:
echo 1 > /proc/sys/kernel/nmi
or via the nmi_watchdog=1 or nmi_watchdog=2 boot options.
build and boot tested on an Athlon64 box.
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86_64/kernel/apic.c | 1 -
arch/x86_64/kernel/io_apic.c | 5 +----
arch/x86_64/kernel/nmi.c | 2 +-
arch/x86_64/kernel/smpboot.c | 1 -
include/asm-x86_64/nmi.h | 1 -
5 files changed, 2 insertions(+), 8 deletions(-)
Index: linux/arch/x86_64/kernel/apic.c
===================================================================
--- linux.orig/arch/x86_64/kernel/apic.c
+++ linux/arch/x86_64/kernel/apic.c
@@ -443,7 +443,6 @@ void __cpuinit setup_local_APIC (void)
oldvalue, value);
}
- nmi_watchdog_default();
setup_apic_nmi_watchdog(NULL);
apic_pm_activate();
}
Index: linux/arch/x86_64/kernel/io_apic.c
===================================================================
--- linux.orig/arch/x86_64/kernel/io_apic.c
+++ linux/arch/x86_64/kernel/io_apic.c
@@ -1604,7 +1604,6 @@ static inline void check_timer(void)
*/
unmask_IO_APIC_irq(0);
if (!no_timer_check && timer_irq_works()) {
- nmi_watchdog_default();
if (nmi_watchdog == NMI_IO_APIC) {
disable_8259A_irq(0);
setup_nmi();
@@ -1630,10 +1629,8 @@ static inline void check_timer(void)
setup_ExtINT_IRQ0_pin(apic2, pin2, vector);
if (timer_irq_works()) {
apic_printk(APIC_VERBOSE," works.\n");
- nmi_watchdog_default();
- if (nmi_watchdog == NMI_IO_APIC) {
+ if (nmi_watchdog == NMI_IO_APIC)
setup_nmi();
- }
return;
}
/*
Index: linux/arch/x86_64/kernel/nmi.c
===================================================================
--- linux.orig/arch/x86_64/kernel/nmi.c
+++ linux/arch/x86_64/kernel/nmi.c
@@ -181,7 +181,7 @@ static __cpuinit inline int nmi_known_cp
}
/* Run after command line and cpu_init init, but before all other checks */
-void nmi_watchdog_default(void)
+static inline void nmi_watchdog_default(void)
{
if (nmi_watchdog != NMI_DEFAULT)
return;
Index: linux/arch/x86_64/kernel/smpboot.c
===================================================================
--- linux.orig/arch/x86_64/kernel/smpboot.c
+++ linux/arch/x86_64/kernel/smpboot.c
@@ -866,7 +866,6 @@ static int __init smp_sanity_check(unsig
*/
void __init smp_prepare_cpus(unsigned int max_cpus)
{
- nmi_watchdog_default();
current_cpu_data = boot_cpu_data;
current_thread_info()->cpu = 0; /* needed? */
set_cpu_sibling_map(0);
Index: linux/include/asm-x86_64/nmi.h
===================================================================
--- linux.orig/include/asm-x86_64/nmi.h
+++ linux/include/asm-x86_64/nmi.h
@@ -59,7 +59,6 @@ extern void disable_timer_nmi_watchdog(v
extern void enable_timer_nmi_watchdog(void);
extern int nmi_watchdog_tick (struct pt_regs * regs, unsigned reason);
-extern void nmi_watchdog_default(void);
extern int setup_nmi_watchdog(char *);
extern atomic_t nmi_active;
On Thu, 7 Dec 2006 13:11:35 +0100
Ingo Molnar <[email protected]> wrote:
> or via the nmi_watchdog=1 or nmi_watchdog=2 boot options.
>
> build and boot tested on an Athlon64 box.
>
> Signed-off-by: Ingo Molnar <[email protected]>
Acked-by: Alan Cox <[email protected]>
On Thu, Dec 07, 2006 at 09:55:22PM +0100, Ingo Molnar wrote:
>
> fallout of the recent big networking merge i guess. Tested fix below.
> David, Herbert, do you agree with it, or is it a false positive?
I agree that this is a bug, but the fix is in the wrong spot. The
dev_watchdog function already runs in softirq context so it doesn't
need to disable BH.
You can almost be guaranteed that if netpoll is involved in a bug
then it should be fixed :)
In this case, it's taking the tx lock in process context which is
not allowed. So it should disable BH before taking the tx lock.
Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
* Ingo Molnar <[email protected]> wrote:
> > (the patch doesn't vaguely apply btw).
>
> patch below should apply to tail of current-ish -mm. Build and boot
> tested on x86_64.
btw., lockdep noticed a locking breakage in netconsole, see the log
below. My guess: dev_watchdog shouldnt be taking the lock without _bh.
Ingo
--------------->
Calling initcall 0xffffffff8098f2e0: rtl8139_init_module+0x0/0x14()
8139too Fast Ethernet driver 0.9.28
ACPI: PCI Interrupt Link [APC2] enabled at IRQ 17
IOAPIC[0]: Set PCI routing entry (2-17 -> 0x59 -> IRQ 17 Mode:1 Active:1)
ACPI: PCI Interrupt 0000:05:07.0[A] -> Link [APC2] -> GSI 17 (level, low) -> IRQ 17
eth0: RealTek RTL8139 at 0xa000, 00:c0:df:03:68:5d, IRQ 17
eth0: Identified 8139 chip type 'RTL-8139B'
Calling initcall 0xffffffff803ce560: init_netconsole+0x0/0x68()
netconsole: device eth0 not up yet, forcing it
eth0: link up, 100Mbps, full-duplex, lpa 0xC5E1
netconsole: carrier detect appears untrustworthy, waiting 4 seconds
=================================
[ INFO: inconsistent lock state ]
2.6.19-mm1 #4
---------------------------------
inconsistent {softirq-on-W} -> {in-softirq-W} usage.
swapper/0 [HC0[0]:SC1[1]:HE1:SE0] takes:
(&dev->_xmit_lock){-+..}, at: [<ffffffff80453151>] dev_watchdog+0x15/0xe0
{softirq-on-W} state was registered at:
[<ffffffff80251078>] mark_lock+0x78/0x3cf
[<ffffffff80251422>] mark_held_locks+0x53/0x71
[<ffffffff802515ed>] trace_hardirqs_on+0x113/0x137
[<ffffffff803cda5f>] rtl8139_poll+0x3c9/0x3ee
[<ffffffff8044f03d>] netpoll_poll+0xa1/0x32f
[<ffffffff8044ef44>] netpoll_send_skb+0xdf/0x137
[<ffffffff8044f5b4>] netpoll_send_udp+0x263/0x270
[<ffffffff803ce632>] write_msg+0x4c/0x7e
[<ffffffff8023671b>] __call_console_drivers+0x5f/0x70
[<ffffffff80236790>] _call_console_drivers+0x64/0x68
[<ffffffff80236e6c>] release_console_sem+0x148/0x207
[<ffffffff80237165>] register_console+0x1b1/0x1ba
[<ffffffff803ce5b4>] init_netconsole+0x54/0x68
[<ffffffff802071d9>] init+0x178/0x347
[<ffffffff8020ab98>] child_rip+0xa/0x12
[<ffffffffffffffff>] 0xffffffffffffffff
irq event stamp: 23912
hardirqs last enabled at (23912): [<ffffffff804aedc5>] _spin_unlock_irq+0x28/0x52
hardirqs last disabled at (23911): [<ffffffff804aecec>] _spin_lock_irq+0xf/0x3e
softirqs last enabled at (23896): [<ffffffff8023befd>] __do_softirq+0xdb/0xe4
softirqs last disabled at (23909): [<ffffffff8020af0c>] call_softirq+0x1c/0x30
other info that might help us debug this:
no locks held by swapper/0.
stack backtrace:
Call Trace:
[<ffffffff8020b304>] dump_trace+0xc1/0x3eb
[<ffffffff8020b667>] show_trace+0x39/0x57
[<ffffffff8020b89c>] dump_stack+0x13/0x15
[<ffffffff80250cff>] print_usage_bug+0x26b/0x27a
[<ffffffff8025112b>] mark_lock+0x12b/0x3cf
[<ffffffff80251b0b>] __lock_acquire+0x3c0/0xa0f
[<ffffffff80252426>] lock_acquire+0x4d/0x67
[<ffffffff804ae747>] _spin_lock+0x2c/0x38
[<ffffffff80453151>] dev_watchdog+0x15/0xe0
[<ffffffff802401d9>] run_timer_softirq+0x167/0x1db
[<ffffffff8023be84>] __do_softirq+0x62/0xe4
[<ffffffff8020af0c>] call_softirq+0x1c/0x30
[<ffffffff8020c6a2>] do_softirq+0x36/0x9c
[<ffffffff8023bb47>] irq_exit+0x45/0x51
[<ffffffff80219d79>] smp_apic_timer_interrupt+0x49/0x5c
[<ffffffff8020a9bb>] apic_timer_interrupt+0x6b/0x70
[<ffffffff80208823>] default_idle+0x36/0x50
[<ffffffff802088d8>] cpu_idle+0x9b/0xd4
[<ffffffff802193f9>] start_secondary+0x498/0x4a7
netconsole: network logging started
Calling initcall 0xffffffff803ce8f4: aec62xx_ide_init+0x0/0x14()
Calling initcall 0xffffffff803cf093: ali15x3_ide_init+0x0/0x14()
Calling initcall 0xffffffff803d086b: amd74xx_ide_init+0x0/0x14()
On Thu, 7 Dec 2006 12:30:11 +0000
Alan <[email protected]> wrote:
> On Thu, 7 Dec 2006 13:11:35 +0100
> Ingo Molnar <[email protected]> wrote:
>
> > or via the nmi_watchdog=1 or nmi_watchdog=2 boot options.
> >
> > build and boot tested on an Athlon64 box.
> >
> > Signed-off-by: Ingo Molnar <[email protected]>
>
> Acked-by: Alan Cox <[email protected]>
metoo. I'm really struggling to recall an occasion on which the NMI
watchdog helped diagnose or fix a bug. The usual scenario nowadays is that
I ask a reporter to enable NMI watchdog and for various reasons (mostly
mysterious) no useful information comes of it.
If it's causing machines to go down then the current tradeoff doesn't seem
right.
But _is_ it causing machines to go down, after the ACPI fix?
(the patch doesn't vaguely apply btw).
> and it needs to be
> undone via the patch attached further below.
I disagree. And it has often saved my ass on 64bit. I
On 32bit it might be reevaluated -- i didn't expect that amount
of laptop firmware bugs triggered by it, but I'm not quite
ready to give up on that yet.
> If Andi wants to debug stuff via the NMI wachdog, he should use the
> nmi_watchdog=2 boot option:
This means for most lockups which are hard to reproduce we don't
get any backtrace.
And nmi_watchdog=2 is bad because it runs at HZ frequency
and has quite high overhead.
> also, lock debugging facilities catch lockup possibilities (and actual
> lockups) alot more efficiently,
Production kernels don't have lock debugging enabled because it
has far too much overhead.
> 8 were caught by lockdep, 8 by atomicity checks in the scheduler, 7 by
> DEBUG_PREEMPT and 1 by DEBUG_SPINLOCK.
None of which is enabled on non debug kernels.
> Note: zero were caught by the NMI watchdog, and i run the NMI watchdog
> enabled by default on all architectures, and i have serial logging of
> everything.
Sure lock debugging will probably catch most of this earlier,
but we don't have it usually.
-Andi
* Andrew Morton <[email protected]> wrote:
> (the patch doesn't vaguely apply btw).
patch below should apply to tail of current-ish -mm. Build and boot
tested on x86_64.
Ingo
---------------------->
Subject: [patch] x86_64: do not enable the NMI watchdog by default
From: Ingo Molnar <[email protected]>
do not enable the NMI watchdog by default. Now that we have
lockdep i cannot remember the last time it caught a real bug,
but the NMI watchdog can /cause/ problems. Furthermore, to the
typical user, an NMI watchdog assert results in a total lockup
anyway (if under X). In that sense, all that the NMI watchdog
does is that it makes the system /less/ stable and /less/
debuggable.
people can still enable it either after bootup via:
echo 1 > /proc/sys/kernel/nmi
or via the nmi_watchdog=1 or nmi_watchdog=2 boot options.
build and boot tested on an Athlon64 box.
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86_64/kernel/apic.c | 1 -
arch/x86_64/kernel/io_apic.c | 1 -
arch/x86_64/kernel/nmi.c | 2 +-
arch/x86_64/kernel/smpboot.c | 1 -
include/asm-x86_64/nmi.h | 1 -
5 files changed, 1 insertion(+), 5 deletions(-)
Index: linux-mm-genapic.q/arch/x86_64/kernel/apic.c
===================================================================
--- linux-mm-genapic.q.orig/arch/x86_64/kernel/apic.c
+++ linux-mm-genapic.q/arch/x86_64/kernel/apic.c
@@ -427,7 +427,6 @@ void __cpuinit setup_local_APIC (void)
oldvalue, value);
}
- nmi_watchdog_default();
setup_apic_nmi_watchdog(NULL);
apic_pm_activate();
}
Index: linux-mm-genapic.q/arch/x86_64/kernel/io_apic.c
===================================================================
--- linux-mm-genapic.q.orig/arch/x86_64/kernel/io_apic.c
+++ linux-mm-genapic.q/arch/x86_64/kernel/io_apic.c
@@ -1580,7 +1580,6 @@ static int try_apic_pin(int apic, int pi
* Ok, does IRQ0 through the IOAPIC work?
*/
if (!no_timer_check && timer_irq_works()) {
- nmi_watchdog_default();
if (nmi_watchdog == NMI_IO_APIC) {
disable_8259A_irq(0);
setup_nmi();
Index: linux-mm-genapic.q/arch/x86_64/kernel/nmi.c
===================================================================
--- linux-mm-genapic.q.orig/arch/x86_64/kernel/nmi.c
+++ linux-mm-genapic.q/arch/x86_64/kernel/nmi.c
@@ -183,7 +183,7 @@ static __cpuinit inline int nmi_known_cp
}
/* Run after command line and cpu_init init, but before all other checks */
-void nmi_watchdog_default(void)
+static inline void nmi_watchdog_default(void)
{
if (nmi_watchdog != NMI_DEFAULT)
return;
Index: linux-mm-genapic.q/arch/x86_64/kernel/smpboot.c
===================================================================
--- linux-mm-genapic.q.orig/arch/x86_64/kernel/smpboot.c
+++ linux-mm-genapic.q/arch/x86_64/kernel/smpboot.c
@@ -1080,7 +1080,6 @@ static int __init smp_sanity_check(unsig
*/
void __init smp_prepare_cpus(unsigned int max_cpus)
{
- nmi_watchdog_default();
current_cpu_data = boot_cpu_data;
current_thread_info()->cpu = 0; /* needed? */
set_cpu_sibling_map(0);
Index: linux-mm-genapic.q/include/asm-x86_64/nmi.h
===================================================================
--- linux-mm-genapic.q.orig/include/asm-x86_64/nmi.h
+++ linux-mm-genapic.q/include/asm-x86_64/nmi.h
@@ -59,7 +59,6 @@ extern void disable_timer_nmi_watchdog(v
extern void enable_timer_nmi_watchdog(void);
extern int nmi_watchdog_tick (struct pt_regs * regs, unsigned reason);
-extern void nmi_watchdog_default(void);
extern int setup_nmi_watchdog(char *);
extern atomic_t nmi_active;
* Ingo Molnar <[email protected]> wrote:
> > patch below should apply to tail of current-ish -mm. Build and boot
> > tested on x86_64.
>
> btw., lockdep noticed a locking breakage in netconsole, see the log
> below. My guess: dev_watchdog shouldnt be taking the lock without _bh.
fallout of the recent big networking merge i guess. Tested fix below.
David, Herbert, do you agree with it, or is it a false positive?
Ingo
------------->
Subject: [patch] net: dev_watchdog() locking fix
From: Ingo Molnar <[email protected]>
lockdep noticed the following bug:
=================================
[ INFO: inconsistent lock state ]
2.6.19-mm1 #4
---------------------------------
inconsistent {softirq-on-W} -> {in-softirq-W} usage.
swapper/0 [HC0[0]:SC1[1]:HE1:SE0] takes:
(&dev->_xmit_lock){-+..}, at: [<ffffffff80453151>] dev_watchdog+0x15/0xe0
{softirq-on-W} state was registered at:
[<ffffffff80251078>] mark_lock+0x78/0x3cf
[<ffffffff80251422>] mark_held_locks+0x53/0x71
[<ffffffff802515ed>] trace_hardirqs_on+0x113/0x137
[<ffffffff803cda5f>] rtl8139_poll+0x3c9/0x3ee
[<ffffffff8044f03d>] netpoll_poll+0xa1/0x32f
[<ffffffff8044ef44>] netpoll_send_skb+0xdf/0x137
[<ffffffff8044f5b4>] netpoll_send_udp+0x263/0x270
[<ffffffff803ce632>] write_msg+0x4c/0x7e
[<ffffffff8023671b>] __call_console_drivers+0x5f/0x70
[<ffffffff80236790>] _call_console_drivers+0x64/0x68
[<ffffffff80236e6c>] release_console_sem+0x148/0x207
[<ffffffff80237165>] register_console+0x1b1/0x1ba
[<ffffffff803ce5b4>] init_netconsole+0x54/0x68
[<ffffffff802071d9>] init+0x178/0x347
[<ffffffff8020ab98>] child_rip+0xa/0x12
[<ffffffffffffffff>] 0xffffffffffffffff
irq event stamp: 23912
hardirqs last enabled at (23912): [<ffffffff804aedc5>] _spin_unlock_irq+0x28/0x52
hardirqs last disabled at (23911): [<ffffffff804aecec>] _spin_lock_irq+0xf/0x3e
softirqs last enabled at (23896): [<ffffffff8023befd>] __do_softirq+0xdb/0xe4
softirqs last disabled at (23909): [<ffffffff8020af0c>] call_softirq+0x1c/0x30
other info that might help us debug this:
no locks held by swapper/0.
stack backtrace:
Call Trace:
[<ffffffff8020b304>] dump_trace+0xc1/0x3eb
[<ffffffff8020b667>] show_trace+0x39/0x57
[<ffffffff8020b89c>] dump_stack+0x13/0x15
[<ffffffff80250cff>] print_usage_bug+0x26b/0x27a
[<ffffffff8025112b>] mark_lock+0x12b/0x3cf
[<ffffffff80251b0b>] __lock_acquire+0x3c0/0xa0f
[<ffffffff80252426>] lock_acquire+0x4d/0x67
[<ffffffff804ae747>] _spin_lock+0x2c/0x38
[<ffffffff80453151>] dev_watchdog+0x15/0xe0
[<ffffffff802401d9>] run_timer_softirq+0x167/0x1db
[<ffffffff8023be84>] __do_softirq+0x62/0xe4
[<ffffffff8020af0c>] call_softirq+0x1c/0x30
[<ffffffff8020c6a2>] do_softirq+0x36/0x9c
[<ffffffff8023bb47>] irq_exit+0x45/0x51
[<ffffffff80219d79>] smp_apic_timer_interrupt+0x49/0x5c
[<ffffffff8020a9bb>] apic_timer_interrupt+0x6b/0x70
[<ffffffff80208823>] default_idle+0x36/0x50
[<ffffffff802088d8>] cpu_idle+0x9b/0xd4
[<ffffffff802193f9>] start_secondary+0x498/0x4a7
taking the lock _bh safe fixes it for me.
Signed-off-by: Ingo Molnar <[email protected]>
---
net/sched/sch_generic.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Index: linux-mm-genapic.q/net/sched/sch_generic.c
===================================================================
--- linux-mm-genapic.q.orig/net/sched/sch_generic.c
+++ linux-mm-genapic.q/net/sched/sch_generic.c
@@ -197,7 +197,7 @@ static void dev_watchdog(unsigned long a
{
struct net_device *dev = (struct net_device *)arg;
- netif_tx_lock(dev);
+ netif_tx_lock_bh(dev);
if (dev->qdisc != &noop_qdisc) {
if (netif_device_present(dev) &&
netif_running(dev) &&
@@ -213,7 +213,7 @@ static void dev_watchdog(unsigned long a
dev_hold(dev);
}
}
- netif_tx_unlock(dev);
+ netif_tx_unlock_bh(dev);
dev_put(dev);
}
On Fri, 8 Dec 2006 08:06:57 +1100
Herbert Xu <[email protected]> wrote:
> On Thu, Dec 07, 2006 at 09:55:22PM +0100, Ingo Molnar wrote:
> >
> > fallout of the recent big networking merge i guess. Tested fix below.
> > David, Herbert, do you agree with it, or is it a false positive?
>
> I agree that this is a bug, but the fix is in the wrong spot. The
> dev_watchdog function already runs in softirq context so it doesn't
> need to disable BH.
>
> You can almost be guaranteed that if netpoll is involved in a bug
> then it should be fixed :)
>
> In this case, it's taking the tx lock in process context which is
> not allowed. So it should disable BH before taking the tx lock.
>
Like this?
/* don't get messages out of order, and no recursion */
if (skb_queue_len(&npinfo->txq) == 0 &&
npinfo->poll_owner != smp_processor_id()) {
local_bh_disable(); /* Where's netif_tx_trylock_bh()? */
if (netif_tx_trylock(dev)) {
/* try until next clock tick */
for (tries = jiffies_to_usecs(1)/USEC_PER_POLL;
tries > 0; --tries) {
if (!netif_queue_stopped(dev))
status = dev->hard_start_xmit(skb, dev);
if (status == NETDEV_TX_OK)
break;
/* tickle device maybe there is some cleanup */
netpoll_poll(np);
udelay(USEC_PER_POLL);
}
netif_tx_unlock(dev);
}
local_bh_enable();
}
--- a/net/core/netpoll.c~netpoll-locking-fix
+++ a/net/core/netpoll.c
@@ -242,22 +242,26 @@ static void netpoll_send_skb(struct netp
/* don't get messages out of order, and no recursion */
if (skb_queue_len(&npinfo->txq) == 0 &&
- npinfo->poll_owner != smp_processor_id() &&
- netif_tx_trylock(dev)) {
- /* try until next clock tick */
- for (tries = jiffies_to_usecs(1)/USEC_PER_POLL; tries > 0; --tries) {
- if (!netif_queue_stopped(dev))
- status = dev->hard_start_xmit(skb, dev);
+ npinfo->poll_owner != smp_processor_id()) {
+ local_bh_disable(); /* Where's netif_tx_trylock_bh()? */
+ if (netif_tx_trylock(dev)) {
+ /* try until next clock tick */
+ for (tries = jiffies_to_usecs(1)/USEC_PER_POLL;
+ tries > 0; --tries) {
+ if (!netif_queue_stopped(dev))
+ status = dev->hard_start_xmit(skb, dev);
- if (status == NETDEV_TX_OK)
- break;
+ if (status == NETDEV_TX_OK)
+ break;
- /* tickle device maybe there is some cleanup */
- netpoll_poll(np);
+ /* tickle device maybe there is some cleanup */
+ netpoll_poll(np);
- udelay(USEC_PER_POLL);
+ udelay(USEC_PER_POLL);
+ }
+ netif_tx_unlock(dev);
}
- netif_tx_unlock(dev);
+ local_bh_enable();
}
if (status != NETDEV_TX_OK) {
_
On Fri, Dec 08, 2006 at 03:19:02PM -0800, Andrew Morton wrote:
>
> Like this?
>
> /* don't get messages out of order, and no recursion */
> if (skb_queue_len(&npinfo->txq) == 0 &&
> npinfo->poll_owner != smp_processor_id()) {
> local_bh_disable(); /* Where's netif_tx_trylock_bh()? */
> if (netif_tx_trylock(dev)) {
> /* try until next clock tick */
> for (tries = jiffies_to_usecs(1)/USEC_PER_POLL;
> tries > 0; --tries) {
> if (!netif_queue_stopped(dev))
> status = dev->hard_start_xmit(skb, dev);
>
> if (status == NETDEV_TX_OK)
> break;
>
> /* tickle device maybe there is some cleanup */
> netpoll_poll(np);
>
> udelay(USEC_PER_POLL);
> }
> netif_tx_unlock(dev);
> }
> local_bh_enable();
> }
Looks good to me. Thanks Andrew!
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
From: Herbert Xu <[email protected]>
Date: Sat, 9 Dec 2006 10:59:52 +1100
> On Fri, Dec 08, 2006 at 03:19:02PM -0800, Andrew Morton wrote:
> >
> > Like this?
> >
> > /* don't get messages out of order, and no recursion */
> > if (skb_queue_len(&npinfo->txq) == 0 &&
> > npinfo->poll_owner != smp_processor_id()) {
> > local_bh_disable(); /* Where's netif_tx_trylock_bh()? */
> > if (netif_tx_trylock(dev)) {
> > /* try until next clock tick */
> > for (tries = jiffies_to_usecs(1)/USEC_PER_POLL;
> > tries > 0; --tries) {
> > if (!netif_queue_stopped(dev))
> > status = dev->hard_start_xmit(skb, dev);
> >
> > if (status == NETDEV_TX_OK)
> > break;
> >
> > /* tickle device maybe there is some cleanup */
> > netpoll_poll(np);
> >
> > udelay(USEC_PER_POLL);
> > }
> > netif_tx_unlock(dev);
> > }
> > local_bh_enable();
> > }
>
> Looks good to me. Thanks Andrew!
I've applied this patch, thanks a lot.
On Sat, 09 Dec 2006 14:02:05 -0800 (PST)
David Miller <[email protected]> wrote:
> From: Herbert Xu <[email protected]>
> Date: Sat, 9 Dec 2006 10:59:52 +1100
>
> > On Fri, Dec 08, 2006 at 03:19:02PM -0800, Andrew Morton wrote:
> > >
> > > Like this?
> > >
> > > /* don't get messages out of order, and no recursion */
> > > if (skb_queue_len(&npinfo->txq) == 0 &&
> > > npinfo->poll_owner != smp_processor_id()) {
> > > local_bh_disable(); /* Where's netif_tx_trylock_bh()? */
> > > if (netif_tx_trylock(dev)) {
> > > /* try until next clock tick */
> > > for (tries = jiffies_to_usecs(1)/USEC_PER_POLL;
> > > tries > 0; --tries) {
> > > if (!netif_queue_stopped(dev))
> > > status = dev->hard_start_xmit(skb, dev);
> > >
> > > if (status == NETDEV_TX_OK)
> > > break;
> > >
> > > /* tickle device maybe there is some cleanup */
> > > netpoll_poll(np);
> > >
> > > udelay(USEC_PER_POLL);
> > > }
> > > netif_tx_unlock(dev);
> > > }
> > > local_bh_enable();
> > > }
> >
> > Looks good to me. Thanks Andrew!
>
> I've applied this patch, thanks a lot.
It spits a nasty during bringup
e1000: eth0: e1000_probe: Intel(R) PRO/1000 Network Connection
forcedeth.c: Reverse Engineered nForce ethernet driver. Version 0.59.
netconsole: device eth0 not up yet, forcing it
e1000: eth0: e1000_watchdog: NIC Link is Up 100 Mbps Full Duplex
WARNING (!__warned) at kernel/softirq.c:137 local_bh_enable()
Call Trace:
[<ffffffff80235baf>] local_bh_enable+0x41/0xa3
[<ffffffff8045ab8e>] netpoll_send_skb+0x116/0x144
[<ffffffff8045b1ee>] netpoll_send_udp+0x263/0x271
[<ffffffff803d41ec>] write_msg+0x42/0x5e
[<ffffffff80230c9b>] __call_console_drivers+0x5f/0x70
[<ffffffff80230d19>] _call_console_drivers+0x6d/0x71
[<ffffffff802313f0>] release_console_sem+0x148/0x1ec
[<ffffffff802316ce>] register_console+0x1b1/0x1ba
[<ffffffff803d4178>] init_netconsole+0x54/0x68
[<ffffffff802071ae>] init+0x152/0x308
[<ffffffff804dac8b>] _spin_unlock_irq+0x14/0x30
[<ffffffff8022c15e>] schedule_tail+0x43/0x9f
[<ffffffff8020a758>] child_rip+0xa/0x12
[<ffffffff8020705c>] init+0x0/0x308
[<ffffffff8020a74e>] child_rip+0x0/0x12
because local irqs are disabled.
On Sun, Dec 10, 2006 at 11:45:08PM -0800, Andrew Morton wrote:
>
> It spits a nasty during bringup
>
> e1000: eth0: e1000_probe: Intel(R) PRO/1000 Network Connection
> forcedeth.c: Reverse Engineered nForce ethernet driver. Version 0.59.
> netconsole: device eth0 not up yet, forcing it
> e1000: eth0: e1000_watchdog: NIC Link is Up 100 Mbps Full Duplex
> WARNING (!__warned) at kernel/softirq.c:137 local_bh_enable()
Normally networking isn't invoked with interrupts turned off, but
I suppose we don't have a choice here. This is unique being a
place where you can get called with BH on, off, or IRQs off.
Given that this is only used for printk, the easiest solution is
probably just to disable local IRQs instead of BH.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
* Herbert Xu <[email protected]> wrote:
> > It spits a nasty during bringup
> >
> > e1000: eth0: e1000_probe: Intel(R) PRO/1000 Network Connection
> > forcedeth.c: Reverse Engineered nForce ethernet driver. Version 0.59.
> > netconsole: device eth0 not up yet, forcing it
> > e1000: eth0: e1000_watchdog: NIC Link is Up 100 Mbps Full Duplex
> > WARNING (!__warned) at kernel/softirq.c:137 local_bh_enable()
>
> Normally networking isn't invoked with interrupts turned off, but I
> suppose we don't have a choice here. This is unique being a place
> where you can get called with BH on, off, or IRQs off.
>
> Given that this is only used for printk, the easiest solution is
> probably just to disable local IRQs instead of BH.
yeah. local_bh_enable() can execute pending softirqs and that warning
protects us against doing that from within irqs-off sections.
Ingo
On Mon, 11 Dec 2006 18:51:11 +1100
Herbert Xu <[email protected]> wrote:
> On Sun, Dec 10, 2006 at 11:45:08PM -0800, Andrew Morton wrote:
> >
> > It spits a nasty during bringup
> >
> > e1000: eth0: e1000_probe: Intel(R) PRO/1000 Network Connection
> > forcedeth.c: Reverse Engineered nForce ethernet driver. Version 0.59.
> > netconsole: device eth0 not up yet, forcing it
> > e1000: eth0: e1000_watchdog: NIC Link is Up 100 Mbps Full Duplex
> > WARNING (!__warned) at kernel/softirq.c:137 local_bh_enable()
>
> Normally networking isn't invoked with interrupts turned off, but
> I suppose we don't have a choice here. This is unique being a
> place where you can get called with BH on, off, or IRQs off.
>
> Given that this is only used for printk, the easiest solution is
> probably just to disable local IRQs instead of BH.
>
I'll try that. I wonder what will explode now..