Subject: [PATCH] linux-acpi: smp_alternatives sleeping in spinlock

Previous thread: Stable regression: usb-storage is stuck in 2.6.26.5 by Michael Tokarev on Saturday, September 13, 2008 - 5:16 am. (8 messages)

Next thread: [PATCH 1/4] ext3: Fix ext3_dx_readdir hash collision handling by Theodore Ts'o on Saturday, September 13, 2008 - 8:32 am. (13 messages)
From: Raz
Date: Saturday, September 13, 2008 - 6:52 am

From: Raz Ben Yehuda <raziebe@gmail.com>

When booting a kernel with PREEMPT_ENABLE and SLAB_DEBUG, unplugging a processor
results in BUG in slab.
Signed-off-by: Raz Ben Yehuda <raziebe@gmail.com>
---
arch/x86/kernel/alternative.c |   17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
this bug fix refers to bug numner 11562 in bugzilla.kernel.org

diff -urpN -X dontdiff
linux-2.6.26-5-vanilla/arch/x86/kernel/alternative.c
linux-2.6.26-5-dbg/arch/x86/kernel/alternative.c
--- linux-2.6.26-5-vanilla/arch/x86/kernel/alternative.c	2008-07-13
23:51:29.000000000 +0200
+++ linux-2.6.26-5-dbg/arch/x86/kernel/alternative.c	2008-09-13
15:31:52.000000000 +0200
@@ -13,6 +13,7 @@
 #include <asm/vsyscall.h>
 #include <asm/cacheflush.h>
 #include <asm/io.h>
+#include <linux/semaphore.h>

 #define MAX_PATCH_LEN (255-1)

@@ -279,7 +280,7 @@ struct smp_alt_module {
 	struct list_head next;
 };
 static LIST_HEAD(smp_alt_modules);
-static DEFINE_SPINLOCK(smp_alt);
+static __DECLARE_SEMAPHORE_GENERIC(smp_alt_lock, 1);
 static int smp_mode = 1;	/* protected by smp_alt */

 void alternatives_smp_module_add(struct module *mod, char *name,
@@ -312,12 +313,12 @@ void alternatives_smp_module_add(struct
 		__func__, smp->locks, smp->locks_end,
 		smp->text, smp->text_end, smp->name);

-	spin_lock(&smp_alt);
+	down(&smp_alt_lock);
 	list_add_tail(&smp->next, &smp_alt_modules);
 	if (boot_cpu_has(X86_FEATURE_UP))
 		alternatives_smp_unlock(smp->locks, smp->locks_end,
 					smp->text, smp->text_end);
-	spin_unlock(&smp_alt);
+	up(&smp_alt_lock);
 }

 void alternatives_smp_module_del(struct module *mod)
@@ -327,17 +328,17 @@ void alternatives_smp_module_del(struct
 	if (smp_alt_once || noreplace_smp)
 		return;

-	spin_lock(&smp_alt);
+	down(&smp_alt_lock);
 	list_for_each_entry(item, &smp_alt_modules, next) {
 		if (mod != item->mod)
 			continue;
 		list_del(&item->next);
-		spin_unlock(&smp_alt);
+		up(&smp_alt_lock);
 		DPRINTK("%s: %s\n", __func__, ...
From: Ingo Molnar
Date: Sunday, September 14, 2008 - 6:18 am

could you please post that BUG? (and which version of the kernel you 
have tried, and exactly what you did to trigger this bug)


is already done correctly in the latest upstream kernel, see this 
commit:

 # 2f1dafe: x86: fix SMP alternatives: use mutex instead of spinlock

the better solution is to use a mutex, not a semaphore. This fix is part 
of the v2.6.26 kernel.

	Ingo
--

From: Raz
Date: Sunday, September 14, 2008 - 6:37 am

Latest working kernel version: unknown
Earliest failing kernel version: 2.6.26.5  <-- this is the kernel I tested.
Distribution:centos 5.0 64bit
Hardware Environment: Lenovo T61 Dual core.
Software Environment:
Problem Description:
1. configure kernel as describe in Documentation/submitCheckList section 12.
2. boot this kernel
3. run . echo 0 >  /sys/devices/system/node/node0/cpu1/online

result:

SMP alternatives: switching to UP code
BUG: sleeping function called from invalid context at mm/slab.c:3052
in_atomic():1, irqs_disabled():0
INFO: lockdep is turned off.
Pid: 2463, comm: bash Not tainted 2.6.26.5 #28

Call Trace:
 [<ffffffff80251c61>] ? __debug_show_held_locks+0x1b/0x24
 [<ffffffff8022aae6>] __might_sleep+0x108/0x10a
 [<ffffffff802962fd>] kmem_cache_alloc_node+0x39/0x1a3
 [<ffffffff80289432>] ? __get_vm_area_node+0xa4/0x1d7
 [<ffffffff80289432>] __get_vm_area_node+0xa4/0x1d7
 [<ffffffff8020a3d7>] ? disable_TSC+0x17/0x53
 [<ffffffff802895c5>] get_vm_area_caller+0x2f/0x31
 [<ffffffff804b2653>] ? text_poke+0x11d/0x19a
 [<ffffffff80289d40>] vmap+0x31/0x63
 [<ffffffff804b7b62>] ? _etext+0x0/0xe
 [<ffffffff804b2653>] text_poke+0x11d/0x19a
 [<ffffffff804b7b62>] ? _etext+0x0/0xe
 [<ffffffff802118d4>] alternatives_smp_unlock+0x4f/0x63
 [<ffffffff80211b77>] alternatives_smp_switch+0x161/0x19e
 [<ffffffff8021b80c>] __cpu_die+0x5c/0x86
 [<ffffffff8049d047>] _cpu_down+0x1b5/0x28d
 [<ffffffff8049d145>] cpu_down+0x26/0x36
 [<ffffffff8049e306>] store_online+0x32/0x75
 [<ffffffff8037402e>] sysdev_store+0x24/0x26
 [<ffffffff802e3134>] sysfs_write_file+0xe5/0x121
 [<ffffffff8029dddc>] vfs_write+0xae/0x124
 [<ffffffff8029e320>] sys_write+0x47/0x70
 [<ffffffff8020bffb>] system_call_after_swapgs+0x7b/0x80

yes. I understand.
--

From: Ingo Molnar
Date: Sunday, September 14, 2008 - 6:42 am

ah, sorry - it's actually part of .27-rc, so not yet part of the stable 
kernel.

I've Cc:-ed stable@kernel.org. Stable folks, please apply the commit 
below to -stable. I've checked that it cherry-picks cleanly on v2.6.26.

	Ingo

-------------->
From 2f1dafe50cc4e58a239fd81bd47f87f32042a1ee Mon Sep 17 00:00:00 2001
From: Pekka Paalanen <pq@iki.fi>
Date: Mon, 12 May 2008 21:21:01 +0200
Subject: [PATCH] x86: fix SMP alternatives: use mutex instead of spinlock, text_poke is sleepable

text_poke is sleepable.
The original fix by Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>.

Signed-off-by: Pekka Paalanen <pq@iki.fi>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/kernel/alternative.c |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index de240ba..2763cb3 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -1,6 +1,6 @@
 #include <linux/module.h>
 #include <linux/sched.h>
-#include <linux/spinlock.h>
+#include <linux/mutex.h>
 #include <linux/list.h>
 #include <linux/kprobes.h>
 #include <linux/mm.h>
@@ -279,7 +279,7 @@ struct smp_alt_module {
 	struct list_head next;
 };
 static LIST_HEAD(smp_alt_modules);
-static DEFINE_SPINLOCK(smp_alt);
+static DEFINE_MUTEX(smp_alt);
 static int smp_mode = 1;	/* protected by smp_alt */
 
 void alternatives_smp_module_add(struct module *mod, char *name,
@@ -312,12 +312,12 @@ void alternatives_smp_module_add(struct module *mod, char *name,
 		__func__, smp->locks, smp->locks_end,
 		smp->text, smp->text_end, smp->name);
 
-	spin_lock(&smp_alt);
+	mutex_lock(&smp_alt);
 	list_add_tail(&smp->next, &smp_alt_modules);
 	if (boot_cpu_has(X86_FEATURE_UP))
 		alternatives_smp_unlock(smp->locks, smp->locks_end,
 					smp->text, smp->text_end);
-	spin_unlock(&smp_alt);
+	mutex_unlock(&smp_alt);
 }
 
 void ...
Previous thread: Stable regression: usb-storage is stuck in 2.6.26.5 by Michael Tokarev on Saturday, September 13, 2008 - 5:16 am. (8 messages)

Next thread: [PATCH 1/4] ext3: Fix ext3_dx_readdir hash collision handling by Theodore Ts'o on Saturday, September 13, 2008 - 8:32 am. (13 messages)