Re: [PATCH] ACPI: don't walk tables if ACPI was disabled


* Vegard Nossum <vegard.nossum@xxxxxxxxx> wrote:

On Fri, Jun 20, 2008 at 11:27 PM, Vegard Nossum <vegard.nossum@xxxxxxxxx> wrote:
So I guess this function, pnpbios_init() needs the check as well. In
fact, it has this:

#ifdef CONFIG_PNPACPI
if (!acpi_disabled && !pnpacpi_disabled) {
pnpbios_disabled = 1;
printk(KERN_INFO "PnPBIOS: Disabled by ACPI PNP\n");
return -ENODEV;
}
#endif /* CONFIG_ACPI */

...I guess that should be changed to say if (acpi_disabled ||
pnpacpi_disabled)? Or... I don't understand the purpose of the
original test. But it seems to be there since the beginning of time
(or, well, v2.6.12-rc2).

Nope. I found the introduction of the change in the historical git repository:

commit 4723ebe898a32262ed49fe677897ccea47e72ff4
Author: Adam Belay <ambx1@xxxxxxxxxx>
Date: Sun Oct 24 15:07:32 2004 -0400

[PNPBIOS] disable if ACPI is active

As further ACPI pnp functionaility is implemented it is no longer
safe to run ACPI and PNPBIOS concurrently.

We therefore take the following approach:
- attempt to enable ACPI support
- if ACPI fails (blacklist etc.) enable pnpbios support
- if ACPI support is not compiled in the kernel enable pnpbios support

Signed-off-by: Adam Belay <ambx1@xxxxxxxxxx>

and now I understand the purpose of the check; pnpbios does not depend
on ACPI; ACPI/pnpacpi is incompatible with pnpbios.

wow, rather old bug - i guess lockdep made it more visible.

Yet it remains a fact that pnpbios will discover devices which then
ACPI code uses erroneously. Which means that my original fix for Ingo
probably is the right one after all. Should I submit another patch
which does the right thing for everything under drivers/acpi/, or can
you do it on your own? :-)

i havent seen the warning reappear with your fix after thousands of
bootups - so i guess we can consider it fixed.

Len, please consider the patch below. (it's in tip/out-of-tree)

Ingo

----------------->
commit acc85833791a5d8f84b8df601afc1cc44568dd18
Author: Vegard Nossum <vegard.nossum@xxxxxxxxx>
Date: Fri Jun 20 15:56:40 2008 +0200

ACPI: don't walk tables if ACPI was disabled

Ingo Molnar wrote:
> -tip auto-testing started triggering this spinlock corruption message
> yesterday:
>
> [ 3.976213] calling acpi_rtc_init+0x0/0xd3
> [ 3.980213] ACPI Exception (utmutex-0263): AE_BAD_PARAMETER, Thread F7C50000 could not acquire Mutex [3] [20080321]
> [ 3.992213] BUG: spinlock bad magic on CPU#0, swapper/1
> [ 3.992213] lock: c2508dc4, .magic: 00000000, .owner: swapper/1, .owner_cpu: 0

This is apparently because some parts of ACPI, including mutexes, are not
initialized when acpi=off is passed to the kernel.

Reported-by: Ingo Molnar <mingo@xxxxxxx>
Signed-off-by: Vegard Nossum <vegard.nossum@xxxxxxxxx>
Cc: Len Brown <lenb@xxxxxxxxxx>
Cc: Zhao Yakui <yakui.zhao@xxxxxxxxx>
Cc: "Rafael J. Wysocki" <rjw@xxxxxxx>
Cc: Alexey Starikovskiy <astarikovskiy@xxxxxxx>
Cc: Yinghai Lu <yhlu.kernel@xxxxxxxxx>
Signed-off-by: Ingo Molnar <mingo@xxxxxxx>

diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
index b4d4ce0..c3e1eeb 100644
--- a/drivers/acpi/glue.c
+++ b/drivers/acpi/glue.c
@@ -334,6 +334,9 @@ static int __init acpi_rtc_init(void)
{
struct device *dev = get_rtc_dev();

+ if (acpi_disabled)
+ return 0;
+
if (dev) {
rtc_wake_setup();
rtc_info.wake_on = rtc_wake_on;
diff --git a/drivers/acpi/namespace/nsxfeval.c b/drivers/acpi/namespace/nsxfeval.c
index a8d5491..c274d1d 100644
--- a/drivers/acpi/namespace/nsxfeval.c
+++ b/drivers/acpi/namespace/nsxfeval.c
@@ -391,6 +391,9 @@ acpi_walk_namespace(acpi_object_type type,

ACPI_FUNCTION_TRACE(acpi_walk_namespace);

+ if (acpi_disabled)
+ return_ACPI_STATUS(AE_NO_NAMESPACE);
+
/* Parameter validation */

if ((type > ACPI_TYPE_LOCAL_MAX) || (!max_depth) || (!user_function)) {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Relevant Pages

  • Re: PCI memory allocation bug with CONFIG_HIGHMEM
    ... There probably isn't anything wrong with the way we're calling the PnPBIOS. ... the PnPBIOS driver found in -mm. ... ACPI or systems with blacklisted ACPI support, ... fully supported relative to resource management in the Linux PnPBIOS driver. ...
    (Linux-Kernel)
  • Re: [PATCH] ACPI: dont walk tables if ACPI was disabled
    ... As further ACPI pnp functionaility is implemented it is no longer ... safe to run ACPI and PNPBIOS concurrently. ... if ACPI support is not compiled in the kernel enable pnpbios support ... No, that commit was not a bug, it was correct, and still is, ...
    (Linux-Kernel)
  • Re: [PATCH] ACPI: dont walk tables if ACPI was disabled
    ... original test. ... As further ACPI pnp functionaility is implemented it is no longer ... safe to run ACPI and PNPBIOS concurrently. ... if ACPI support is not compiled in the kernel enable pnpbios support ...
    (Linux-Kernel)
  • Re: How should I handle device with two PNP-BIOS ids?
    ... For PNPBIOS, that would be the "device product identifier." ... For ACPI, ... Systems with the new chip should report both PNP IDs. ...
    (Linux-Kernel)
  • [PATCH] PnP Fixes for 2.6.10-rc1
    ... # As further ACPI pnp functionaility is implemented it is no longer ... # safe to run ACPI and PNPBIOS concurrently. ... # - if ACPI support is not compiled in the kernel enable pnpbios support ... send the line "unsubscribe linux-kernel" in ...
    (Linux-Kernel)