Home » Mailing list archives » linux-kernel » 2008 » February » 12

Re: [PATCH 1/2] acer-wmi - Fail gracefully if ACPI is disabled

Previous thread: [PATCH] ide-cd: replace ntohs with generic byteorder macro be16_to_cpu by Borislav Petkov on Friday, February 8, 2008 - 2:29 am. (2 messages)

Next thread: [PATCH] mxser: convert large macros to functions by Christoph Hellwig on Friday, February 8, 2008 - 2:51 am. (1 message)
[view original message]
From: Len Brown <lenb@...>
To: Linus Torvalds <torvalds@...>
Cc: Andrew Morton <akpm@...>, <linux-acpi@...>, <linux-kernel@...>, <linux-pm@...>
Subject: [GIT PATCH] ACPI patches for 2.6.25-rc0 (#2)
Date: Friday, February 8, 2008 - 2:45 am

Hi Linus,


please pull from: 


git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux-acpi-2.6.git release


Nothing exciting here, generally just tweaks to the previous batch.


This will update the files shown below.


thanks!


-Len


ps. individual patches are available on linux-acpi@vger.kernel.org

and a consolidated plain patch is available here:

ftp://ftp.kernel.org/pub/linux/kernel/people/lenb/acpi/patches/release/2...


 Documentation/thermal/sysfs-api.txt |   23 +++++------

 drivers/acpi/Kconfig                |   20 +++++++--

 drivers/acpi/blacklist.c            |   20 +++++++++

 drivers/acpi/osl.c                  |    2

 drivers/acpi/processor_perflib.c    |    7 +++

 drivers/misc/Kconfig                |    5 +-

 drivers/thermal/Kconfig             |    4 -

 drivers/thermal/thermal.c           |   49 +++++++++++++-----------

 8 files changed, 88 insertions(+), 42 deletions(-)


through these commits:


Carlos Corbacho (1):

      ACPI: WMI: Improve Kconfig description


Len Brown (6):

      ACPI: add newline to printk

      ACPI: build WMI on X86 only

      intel_menlo: build on X86 only

      ACPI: thermal: syntax, spelling, kernel-doc

      ACPI: DMI: add Panasonic CF-52 and Thinpad X61

      acer-wmi, tc1100-wmi: select ACPI_WMI


Thomas Renninger (1):

      ACPI: cpufreq: Print _PPC changes via cpufreq debug layer


with this log:


commit 2e6c4e5101633a54aeee1f2e83020ee77fcb70d2

Merge: 446b1df... 4a507d9...

Author: Len Brown <len.brown@intel.com>

Date:   Fri Feb 8 01:22:26 2008 -0500


    Merge branches 'release', 'dmi' and 'misc' into release


commit 4a507d93fac78ecd37d18343c57c564f6a126f01

Author: Len Brown <len.brown@intel.com>

Date:   Fri Feb 8 00:37:16 2008 -0500


    acer-wmi, tc1100-wmi: select ACPI_WMI


    It is safe for these Kconfig entries to use select because

    they select ACPI_WMI, which already has its dependencies

    satisfied.  This makes Kconfig more user friendly, sin...
[ message continues ]
[view original message]
From: Ingo Molnar <mingo@...>
To: Len Brown <lenb@...>
Cc: Linus Torvalds <torvalds@...>, Andrew Morton <akpm@...>, <linux-acpi@...>, <linux-kernel@...>, <linux-pm@...>
Subject: Re: [GIT PATCH] ACPI patches for 2.6.25-rc0 (#2)
Date: Saturday, February 9, 2008 - 2:17 am

hm, this new WMI code caused a bootup crash in the overnight x86.git

tests:


initcall 0x40cffbd0 ran for 0 msecs: msi_init+0x0/0x150()

Calling initcall 0x40cffd20: tc1100_init+0x0/0x70()

BUG: unable to handle kernel NULL pointer dereference at 00000000

IP: [<404b96a2>] find_guid+0x132/0x150

*pde = 00000000

Oops: 0000 [#1] SMP 


Pid: 1, comm: swapper Not tainted (2.6.24 #34)

EIP: 0060:[<404b96a2>] EFLAGS: 00010247 CPU: 1

EIP is at find_guid+0x132/0x150

EAX: 05a572d4 EBX: 5f81af54 ECX: 00000000 EDX: ffffffff

ESI: 00000000 EDI: 5f81af54 EBP: 5f81af80 ESP: 5f81af48

 DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068

Process swapper (pid: 1, ti=5f81a000 task=5f815410 task.ti=5f81a000)

Stack: 00000000 5f81af74 00000005 c364ac71 495a36db 39b49484 05a572d4 71ac64c3

       5a49db36 39b49484 05a572d4 00000000 00000000 ffffffed 5f81af88 404b96cf

       5f81af94 40cffd33 00000000 5f81afe0 40cdb5d3 2c7b1366 00000001 00000000

Call Trace:

 [<404b96cf>] ? wmi_has_guid+0xf/0x20

 [<40cffd33>] ? tc1100_init+0x13/0x70

 [<40cdb5d3>] ? kernel_init+0x123/0x2f0

 [<40cdb4b0>] ? kernel_init+0x0/0x2f0

 [<401053d7>] ? kernel_thread_helper+0x7/0x10

 =======================

Code: 0c 41 8d 7d d4 eb 24 8d 46 08 b9 10 00 00 00 89 fa e8 e3 88 f6 ff 85 c0 75 0f b0 01 83 7d c8 00 74 19 8b 55 c8 89 32 eb 12 89 de <8b> 1e 0f 18 03 90 81 fe 90 c1 0c 41 75 ce 31 c0 83 c4 2c 5b 5e

EIP: [<404b96a2>] find_guid+0x132/0x150 SS:ESP 0068:5f81af48

---[ end trace 5d95b7a12e9c31b7 ]---

Kernel panic - not syncing: Attempted to kill init!

Press any key to enter the menu


config and crashlog attached.


I double-checked a second bootup [with a slightly different but

essentially same kernel], but this time the crash did not happen:


 Calling initcall 0x40cb8b80: acpi_wmi_init+0x0/0x50()

 ACPI: WMI: Mapper loaded

 initcall 0x40cb8b80: acpi_wmi_init+0x0/0x50() returned 0.

 initcall 0x40cb8b80 ran for 0 msecs: acpi_wmi_init+0x0/0x50()


 Calling initcall 0x40cbd770: tc11...
[ message continues ]
[view original message]
From: Ingo Molnar <mingo@...>
To: Len Brown <lenb@...>
Cc: Linus Torvalds <torvalds@...>, Andrew Morton <akpm@...>, <linux-acpi@...>, <linux-kernel@...>, <linux-pm@...>, Carlos Corbacho <carlos@...>
Subject: Re: [GIT PATCH] ACPI patches for 2.6.25-rc0 (#2)
Date: Saturday, February 9, 2008 - 2:19 am

(Carlos Cc:-ed too)


--
[view original message]
From: Carlos Corbacho <carlos@...>
To: Ingo Molnar <mingo@...>
Cc: Len Brown <lenb@...>, Linus Torvalds <torvalds@...>, Andrew Morton <akpm@...>, <linux-acpi@...>, <linux-kernel@...>, <linux-pm@...>
Subject: Re: [GIT PATCH] ACPI patches for 2.6.25-rc0 (#2)
Date: Saturday, February 9, 2008 - 7:42 am

I suspect this is a problem on systems without ACPI-WMI in the DSDT - though I

can't reproduce this here on my other box that lacks ACPI-WMI.


Ingo, can you try the patch below and see if this cures it?

---

ACPI: WMI: Fix crash on WMI free systems


From: Carlos Corbacho <carlos@strangeworlds.co.uk>


We have two pointers in the find_guid() code that aren't checked before

we use them. On ACPI-WMI free systems, this can sometimes cause a crash if

another driver tries to use any of the methods provided by WMI.


Also throw in an extra early check to see if the GUID list is empty, so we

can try to bail out before reaching this code as well.


Spotted by Ingo Molnar.


Signed-off-by: Carlos Corbacho <carlos@strangeworlds.co.uk>

CC: Ingo Molnar <mingo@elte.hu>

CC: Linus Torvalds <torvalds@linux-foundation.org>

CC: Andrew Morton <akpm@linux-foundation.org>

CC: Len Brown <lenb@kernel.org>

---


 drivers/acpi/wmi.c |    8 ++++++++

 1 files changed, 8 insertions(+), 0 deletions(-)


diff --git a/drivers/acpi/wmi.c b/drivers/acpi/wmi.c

index 36b84ab..b54e734 100644

--- a/drivers/acpi/wmi.c

+++ b/drivers/acpi/wmi.c

@@ -201,12 +201,20 @@ static bool find_guid(const char *guid_string, struct wmi_block **out)

 	struct guid_block *block;

 	struct list_head *p;


+	if (list_empty(&wmi_blocks.list))

+		return 0;

+

 	wmi_parse_guid(guid_string, tmp);

 	wmi_swap_bytes(tmp, guid_input);


 	list_for_each(p, &wmi_blocks.list) {

 		wblock = list_entry(p, struct wmi_block, list);

+		if (!wblock)

+			return 0;

+

 		block = &wblock->gblock;

+		if (!block)

+			return 0;


 		if (memcmp(block->guid, guid_input, 16) == 0) {

 			if (out)

--

E-Mail: carlos@strangeworlds.co.uk

Web: strangeworlds.co.uk

GPG Key ID: 0x23EE722D

--
[view original message]
From: Ingo Molnar <mingo@...>
To: Carlos Corbacho <carlos@...>
Cc: Len Brown <lenb@...>, Linus Torvalds <torvalds@...>, Andrew Morton <akpm@...>, <linux-acpi@...>, <linux-kernel@...>, <linux-pm@...>
Subject: Re: [GIT PATCH] ACPI patches for 2.6.25-rc0 (#2)
Date: Monday, February 11, 2008 - 5:17 am

no, it does not help - see the attached .config and the crash.log. 


(triggered this crash after about 80 randconfig bootups, 7 of which had

WMI enabled - so the chance for the crash is about 1:10 on a WMI enabled

kernel.)


	Ingo
[view original message]
From: Carlos Corbacho <carlos@...>
To: Ingo Molnar <mingo@...>
Cc: Len Brown <lenb@...>, Linus Torvalds <torvalds@...>, Andrew Morton <akpm@...>, <linux-acpi@...>, <linux-kernel@...>, <linux-pm@...>
Subject: Re: [GIT PATCH] ACPI patches for 2.6.25-rc0 (#2)
Date: Monday, February 11, 2008 - 3:23 pm

Thanks Ingo - the cause of the crash is ACPI being disabled on your system for

some reason. I can reproduce your crash every time here with acpi=off.


The two WMI based drivers (acer-wmi, tc1100-wmi) in the kernel both need a

simple patch each to fix this - patches to follow.


-Carlos

--

E-Mail: carlos@strangeworlds.co.uk

Web: strangeworlds.co.uk

GPG Key ID: 0x23EE722D

--
[view original message]
From: Carlos Corbacho <carlos@...>
To: Ingo Molnar <mingo@...>
Cc: Len Brown <lenb@...>, Linus Torvalds <torvalds@...>, Andrew Morton <akpm@...>, <linux-acpi@...>, <linux-kernel@...>, <linux-pm@...>
Subject: [PATCH 2/2] tc1100-wmi - Fail gracefully if ACPI is disabled
Date: Monday, February 11, 2008 - 3:55 pm

tc1100-wmi - Fail gracefully if ACPI is disabled


From: Carlos Corbacho <carlos@strangeworlds.co.uk>


WMI drivers, like their ACPI counterparts, should also check if ACPI is

disabled or not, and bail out if so, otherwise we cause a crash.


Spotted by Ingo Molnar.


Signed-off-by: Carlos Corbacho <carlos@strangeworlds.co.uk>

CC: Ingo Molnar <mingo@elte.hu>

CC: Linus Torvalds <torvalds@linux-foundation.org>

CC: Andrew Morton <akpm@linux-foundation.org>

CC: Len Brown <lenb@kernel.org>

---


 drivers/misc/tc1100-wmi.c |    3 +++

 1 files changed, 3 insertions(+), 0 deletions(-)


diff --git a/drivers/misc/tc1100-wmi.c b/drivers/misc/tc1100-wmi.c

index f25e4c9..cb8f79f 100644

--- a/drivers/misc/tc1100-wmi.c

+++ b/drivers/misc/tc1100-wmi.c

@@ -263,6 +263,9 @@ static int __init tc1100_init(void)

 {

 	int result = 0;


+	if (acpi_disabled)

+		return -ENODEV;

+

 	if (!wmi_has_guid(GUID))

 		return -ENODEV;


--
[view original message]
From: Len Brown <lenb@...>
To: Carlos Corbacho <carlos@...>
Cc: Ingo Molnar <mingo@...>, Linus Torvalds <torvalds@...>, Andrew Morton <akpm@...>, <linux-acpi@...>, <linux-kernel@...>, <linux-pm@...>
Subject: Re: [PATCH 2/2] tc1100-wmi - Fail gracefully if ACPI is disabled
Date: Thursday, February 14, 2008 - 2:11 am

applied.


thanks,

-len


--
[view original message]
From: Len Brown <lenb@...>
To: Carlos Corbacho <carlos@...>
Cc: Ingo Molnar <mingo@...>, Linus Torvalds <torvalds@...>, Andrew Morton <akpm@...>, <linux-acpi@...>, <linux-kernel@...>, <linux-pm@...>
Subject: Re: [PATCH 2/2] tc1100-wmi - Fail gracefully if ACPI is disabled
Date: Thursday, February 14, 2008 - 2:15 am

never mind -- linus fixed this in a more elegant way.


-len


--
[view original message]
From: Carlos Corbacho <carlos@...>
To: Ingo Molnar <mingo@...>
Cc: Len Brown <lenb@...>, Linus Torvalds <torvalds@...>, Andrew Morton <akpm@...>, <linux-acpi@...>, <linux-kernel@...>, <linux-pm@...>
Subject: [PATCH 1/2] acer-wmi - Fail gracefully if ACPI is disabled
Date: Monday, February 11, 2008 - 3:55 pm

acer-wmi - Fail gracefully if ACPI is disabled


From: Carlos Corbacho <carlos@strangeworlds.co.uk>


WMI drivers, like their ACPI counterparts, should also check if ACPI is

disabled or not, and bail out if so, otherwise we cause a crash.


Spotted by Ingo Molnar.


Signed-off-by: Carlos Corbacho <carlos@strangeworlds.co.uk>

CC: Ingo Molnar <mingo@elte.hu>

CC: Linus Torvalds <torvalds@linux-foundation.org>

CC: Andrew Morton <akpm@linux-foundation.org>

CC: Len Brown <lenb@kernel.org>

---


 drivers/misc/acer-wmi.c |    3 +++

 1 files changed, 3 insertions(+), 0 deletions(-)


diff --git a/drivers/misc/acer-wmi.c b/drivers/misc/acer-wmi.c

index d7aea93..6e81027 100644

--- a/drivers/misc/acer-wmi.c

+++ b/drivers/misc/acer-wmi.c

@@ -1029,6 +1029,9 @@ static int __init acer_wmi_init(void)

 {

 	int err;


+	if (acpi_disabled)

+		return -ENODEV;

+

 	printk(ACER_INFO "Acer Laptop ACPI-WMI Extras version %s\n",

 			ACER_WMI_VERSION);


--
[view original message]
From: Linus Torvalds <torvalds@...>
To: Carlos Corbacho <carlos@...>
Cc: Ingo Molnar <mingo@...>, Len Brown <lenb@...>, Andrew Morton <akpm@...>, <linux-acpi@...>, <linux-kernel@...>, <linux-pm@...>
Subject: Re: [PATCH 1/2] acer-wmi - Fail gracefully if ACPI is disabled
Date: Tuesday, February 12, 2008 - 12:41 am

Shouldn't "wmi_has_guid()" just return false if ACPI isn't enabled, and

the drivers should just then always give up?


The proper way to get there would seem to be to just do this instead..


We should *not* add some random ACPI workarounds to individual drivers, we

should just make the wmi subsystem so robust that nobody *cares* if acpi

exists or is enabled on that machine.


		Linus


---

 drivers/acpi/wmi.c |    4 ++--

 1 files changed, 2 insertions(+), 2 deletions(-)


diff --git a/drivers/acpi/wmi.c b/drivers/acpi/wmi.c

index 36b84ab..457ed3d 100644

--- a/drivers/acpi/wmi.c

+++ b/drivers/acpi/wmi.c

@@ -673,11 +673,11 @@ static int __init acpi_wmi_init(void)

 {

 	acpi_status result;


+	INIT_LIST_HEAD(&wmi_blocks.list);

+

 	if (acpi_disabled)

 		return -ENODEV;


-	INIT_LIST_HEAD(&wmi_blocks.list);

-

 	result = acpi_bus_register_driver(&acpi_wmi_driver);


 	if (result < 0) {

--
[view original message]
From: Carlos Corbacho <carlos@...>
To: Linus Torvalds <torvalds@...>
Cc: Ingo Molnar <mingo@...>, Len Brown <lenb@...>, Andrew Morton <akpm@...>, <linux-acpi@...>, <linux-kernel@...>, <linux-pm@...>
Subject: Re: [PATCH 1/2] acer-wmi - Fail gracefully if ACPI is disabled
Date: Tuesday, February 12, 2008 - 3:28 pm

Yes - that's a better fix.


-Carlos

--

E-Mail: carlos@strangeworlds.co.uk

Web: strangeworlds.co.uk

GPG Key ID: 0x23EE722D

--
[view original message]
From: Ingo Molnar <mingo@...>
To: Linus Torvalds <torvalds@...>
Cc: Carlos Corbacho <carlos@...>, Len Brown <lenb@...>, Andrew Morton <akpm@...>, <linux-acpi@...>, <linux-kernel@...>, <linux-pm@...>
Subject: Re: [PATCH 1/2] acer-wmi - Fail gracefully if ACPI is disabled
Date: Tuesday, February 12, 2008 - 11:12 am

test results: i've turned the WMI options back on earlier today on

latest -git which has your patch above, and those crashes i reported

disappeared. So the problem is fixed.


	Ingo

--
Previous thread: [PATCH] ide-cd: replace ntohs with generic byteorder macro be16_to_cpu by Borislav Petkov on Friday, February 8, 2008 - 2:29 am. (2 messages)

Next thread: [PATCH] mxser: convert large macros to functions by Christoph Hellwig on Friday, February 8, 2008 - 2:51 am. (1 message)