[patch 2.6.7] bug_smp_call_function

[patch 2.6.7] bug_smp_call_function

[Keith Owens]

sg.c has been fixed to no longer call vfree() with interrupts disabled.
Change smp_call_function() from WARN_ON to BUG_ON when interrupts are
disabled.  It was only set to WARN_ON because of sg.c.

Signed-off-by: Keith Owens <kaos@sgi.com>

Index: 2.6.7/arch/alpha/kernel/smp.c
===================================================================
--- 2.6.7.orig/arch/alpha/kernel/smp.c	2004-06-16 20:53:04.000000000 +1000
+++ 2.6.7/arch/alpha/kernel/smp.c	2004-06-19 19:25:23.000000000 +1000
@@ -821,7 +821,7 @@ smp_call_function_on_cpu (void (*func) (
 	int num_cpus_to_call;
 	
 	/* Can deadlock when called with interrupts disabled */
-	WARN_ON(irqs_disabled());
+	BUG_ON(irqs_disabled());
 
 	data.func = func;
 	data.info = info;
Index: 2.6.7/arch/i386/kernel/smp.c
===================================================================
--- 2.6.7.orig/arch/i386/kernel/smp.c	2004-06-16 20:53:07.000000000 +1000
+++ 2.6.7/arch/i386/kernel/smp.c	2004-06-19 19:25:25.000000000 +1000
@@ -520,7 +520,7 @@ int smp_call_function (void (*func) (voi
 		return 0;
 
 	/* Can deadlock when called with interrupts disabled */
-	WARN_ON(irqs_disabled());
+	BUG_ON(irqs_disabled());
 
 	data.func = func;
 	data.info = info;
Index: 2.6.7/arch/ia64/kernel/smp.c
===================================================================
--- 2.6.7.orig/arch/ia64/kernel/smp.c	2004-06-16 20:53:09.000000000 +1000
+++ 2.6.7/arch/ia64/kernel/smp.c	2004-06-19 19:25:28.000000000 +1000
@@ -332,7 +332,7 @@ smp_call_function (void (*func) (void *i
 		return 0;
 
 	/* Can deadlock when called with interrupts disabled */
-	WARN_ON(irqs_disabled());
+	BUG_ON(irqs_disabled());
 
 	data.func = func;
 	data.info = info;
Index: 2.6.7/arch/mips/kernel/smp.c
===================================================================
--- 2.6.7.orig/arch/mips/kernel/smp.c	2004-06-16 20:53:13.000000000 +1000
+++ 2.6.7/arch/mips/kernel/smp.c	2004-06-19 19:25:30.000000000 +1000
@@ -152,7 +152,7 @@ int smp_call_function (void (*func) (voi
 		return 0;
 
 	/* Can deadlock when called with interrupts disabled */
-	WARN_ON(irqs_disabled());
+	BUG_ON(irqs_disabled());
 
 	data.func = func;
 	data.info = info;
Index: 2.6.7/arch/parisc/kernel/smp.c
===================================================================
--- 2.6.7.orig/arch/parisc/kernel/smp.c	2004-06-16 20:53:13.000000000 +1000
+++ 2.6.7/arch/parisc/kernel/smp.c	2004-06-19 19:25:32.000000000 +1000
@@ -326,7 +326,7 @@ smp_call_function (void (*func) (void *i
 	static spinlock_t lock = SPIN_LOCK_UNLOCKED;
 
 	/* Can deadlock when called with interrupts disabled */
-	WARN_ON(irqs_disabled());
+	BUG_ON(irqs_disabled());
 	
 	data.func = func;
 	data.info = info;
Index: 2.6.7/arch/ppc/kernel/smp.c
===================================================================
--- 2.6.7.orig/arch/ppc/kernel/smp.c	2004-06-16 20:53:15.000000000 +1000
+++ 2.6.7/arch/ppc/kernel/smp.c	2004-06-19 19:25:42.000000000 +1000
@@ -212,7 +212,7 @@ int smp_call_function(void (*func) (void
 	if (num_online_cpus() <= 1)
 		return 0;
 	/* Can deadlock when called with interrupts disabled */
-	WARN_ON(irqs_disabled());
+	BUG_ON(irqs_disabled());
 	return __smp_call_function(func, info, wait, MSG_ALL_BUT_SELF);
 }
 
Index: 2.6.7/arch/ppc64/kernel/smp.c
===================================================================
--- 2.6.7.orig/arch/ppc64/kernel/smp.c	2004-06-16 20:53:16.000000000 +1000
+++ 2.6.7/arch/ppc64/kernel/smp.c	2004-06-19 19:25:35.000000000 +1000
@@ -692,7 +692,7 @@ int smp_call_function (void (*func) (voi
 	unsigned long timeout;
 
 	/* Can deadlock when called with interrupts disabled */
-	WARN_ON(irqs_disabled());
+	BUG_ON(irqs_disabled());
 
 	data.func = func;
 	data.info = info;
Index: 2.6.7/arch/s390/kernel/smp.c
===================================================================
--- 2.6.7.orig/arch/s390/kernel/smp.c	2004-06-16 20:53:17.000000000 +1000
+++ 2.6.7/arch/s390/kernel/smp.c	2004-06-19 19:25:47.000000000 +1000
@@ -129,7 +129,7 @@ int smp_call_function (void (*func) (voi
 		return 0;
 
 	/* Can deadlock when called with interrupts disabled */
-	WARN_ON(irqs_disabled());
+	BUG_ON(irqs_disabled());
 
 	data.func = func;
 	data.info = info;
Index: 2.6.7/arch/sh/kernel/smp.c
===================================================================
--- 2.6.7.orig/arch/sh/kernel/smp.c	2004-06-16 20:53:17.000000000 +1000
+++ 2.6.7/arch/sh/kernel/smp.c	2004-06-19 19:25:50.000000000 +1000
@@ -182,7 +182,7 @@ int smp_call_function(void (*func)(void 
 		return 0;
 
 	/* Can deadlock when called with interrupts disabled */
-	WARN_ON(irqs_disabled());
+	BUG_ON(irqs_disabled());
 
 	spin_lock(&smp_fn_call.lock);
 
Index: 2.6.7/arch/sparc64/kernel/smp.c
===================================================================
--- 2.6.7.orig/arch/sparc64/kernel/smp.c	2004-06-16 20:53:18.000000000 +1000
+++ 2.6.7/arch/sparc64/kernel/smp.c	2004-06-19 19:25:53.000000000 +1000
@@ -602,7 +602,7 @@ int smp_call_function(void (*func)(void 
 		return 0;
 
 	/* Can deadlock when called with interrupts disabled */
-	WARN_ON(irqs_disabled());
+	BUG_ON(irqs_disabled());
 
 	data.func = func;
 	data.info = info;
Index: 2.6.7/arch/um/kernel/smp.c
===================================================================
--- 2.6.7.orig/arch/um/kernel/smp.c	2004-06-16 20:53:19.000000000 +1000
+++ 2.6.7/arch/um/kernel/smp.c	2004-06-19 19:26:31.000000000 +1000
@@ -267,7 +267,7 @@ int smp_call_function(void (*_func)(void
 		return 0;
 
 	/* Can deadlock when called with interrupts disabled */
-	WARN_ON(irqs_disabled());
+	BUG_ON(irqs_disabled());
 
 	spin_lock_bh(&call_lock);
 	atomic_set(&scf_started, 0);
Index: 2.6.7/arch/x86_64/kernel/smp.c
===================================================================
--- 2.6.7.orig/arch/x86_64/kernel/smp.c	2004-06-16 20:53:19.000000000 +1000
+++ 2.6.7/arch/x86_64/kernel/smp.c	2004-06-19 19:26:35.000000000 +1000
@@ -405,7 +405,7 @@ int smp_call_function (void (*func) (voi
 		return 0;
 
 	/* Can deadlock when called with interrupts disabled */
-	WARN_ON(irqs_disabled());
+	BUG_ON(irqs_disabled());
 
 	data.func = func;
 	data.info = info;

Re: [patch 2.6.7] bug_smp_call_function

[Andrew Morton]

Keith Owens <kaos@sgi.com> wrote:
>
>  sg.c has been fixed to no longer call vfree() with interrupts disabled.
>  Change smp_call_function() from WARN_ON to BUG_ON when interrupts are
>  disabled.  It was only set to WARN_ON because of sg.c.

I prefer the WARN_ON.  It is exceedingly unlikely that the bug will cause
lockups or memory/data corruption or anything else, so why nuke the user's
box when we can trivially continue?

We'll be sent the bug report either way.

Re: [patch 2.6.7] bug_smp_call_function

[Keith Owens]

On Sat, 19 Jun 2004 02:44:16 -0700, 
Andrew Morton <akpm@osdl.org> wrote:
>Keith Owens <kaos@sgi.com> wrote:
>>
>>  sg.c has been fixed to no longer call vfree() with interrupts disabled.
>>  Change smp_call_function() from WARN_ON to BUG_ON when interrupts are
>>  disabled.  It was only set to WARN_ON because of sg.c.
>
>I prefer the WARN_ON.  It is exceedingly unlikely that the bug will cause
>lockups or memory/data corruption or anything else, so why nuke the user's
>box when we can trivially continue?
>
>We'll be sent the bug report either way.

I prefer to catch this bug every time instead of assuming that somebody
will see the report in the syslog.  Once the kernel code is clean
(which it should be now), BUG_ON() will prevent any new code
reintroducing this error.

Re: [patch 2.6.7] bug_smp_call_function

[William Lee Irwin III]

Keith Owens <kaos@sgi.com> wrote:
>>  sg.c has been fixed to no longer call vfree() with interrupts disabled.
>>  Change smp_call_function() from WARN_ON to BUG_ON when interrupts are
>>  disabled.  It was only set to WARN_ON because of sg.c.

On Sat, Jun 19, 2004 at 02:44:16AM -0700, Andrew Morton wrote:
> I prefer the WARN_ON.  It is exceedingly unlikely that the bug will cause
> lockups or memory/data corruption or anything else, so why nuke the user's
> box when we can trivially continue?
> We'll be sent the bug report either way.

Calls to smp_call_function() with interrupts off or spinlocks held
typically causes deadlocks on SMP systems. ISTR debugging such an
issue in the scheduler a while back, i.e. mmdrop() under rq->lock
doing vfree() of an LDT. Basically smp_call_function() will spin
waiting for the other cpus to answer the interrupt on multiple cpus.
It also doesn't need to be the same function doing smp_call_function();
generally TLB flushing deadlocks against anything doing this.


-- wli

Re: [patch 2.6.7] bug_smp_call_function

[Keith Owens]

On Sat, 19 Jun 2004 02:59:10 -0700, 
William Lee Irwin III <wli@holomorphy.com> wrote:
>Keith Owens <kaos@sgi.com> wrote:
>>>  sg.c has been fixed to no longer call vfree() with interrupts disabled.
>>>  Change smp_call_function() from WARN_ON to BUG_ON when interrupts are
>>>  disabled.  It was only set to WARN_ON because of sg.c.
>
>On Sat, Jun 19, 2004 at 02:44:16AM -0700, Andrew Morton wrote:
>> I prefer the WARN_ON.  It is exceedingly unlikely that the bug will cause
>> lockups or memory/data corruption or anything else, so why nuke the user's
>> box when we can trivially continue?
>> We'll be sent the bug report either way.
>
>Calls to smp_call_function() with interrupts off or spinlocks held
>typically causes deadlocks on SMP systems. ISTR debugging such an
>issue in the scheduler a while back, i.e. mmdrop() under rq->lock
>doing vfree() of an LDT. Basically smp_call_function() will spin
>waiting for the other cpus to answer the interrupt on multiple cpus.
>It also doesn't need to be the same function doing smp_call_function();
>generally TLB flushing deadlocks against anything doing this.

Agreed, that is exactly the class of problems that I spent days
debugging.  WARN_ON() lets developers add code that breaks the rules
and assumes that we will have to fix the bad code later.  BUG_ON()
prevents any bad code being added because it catches the developer as
soon as they add it.

Re: [patch 2.6.7] bug_smp_call_function

[Andrew Morton]

William Lee Irwin III <wli@holomorphy.com> wrote:
>
> Keith Owens <kaos@sgi.com> wrote:
> >>  sg.c has been fixed to no longer call vfree() with interrupts disabled.
> >>  Change smp_call_function() from WARN_ON to BUG_ON when interrupts are
> >>  disabled.  It was only set to WARN_ON because of sg.c.
> 
> On Sat, Jun 19, 2004 at 02:44:16AM -0700, Andrew Morton wrote:
> > I prefer the WARN_ON.  It is exceedingly unlikely that the bug will cause
> > lockups or memory/data corruption or anything else, so why nuke the user's
> > box when we can trivially continue?
> > We'll be sent the bug report either way.
> 
> Calls to smp_call_function() with interrupts off or spinlocks held
> typically causes deadlocks on SMP systems.

No, this doesn't "typically" deadlock.  It will deadlock on every ten
millionth call.  The preceding 9,999,999 warnings should have imparted
sufficient clue?

Re: [patch 2.6.7] bug_smp_call_function

[William Lee Irwin III]

William Lee Irwin III <wli@holomorphy.com> wrote:
>> Calls to smp_call_function() with interrupts off or spinlocks held
>> typically causes deadlocks on SMP systems.

On Sat, Jun 19, 2004 at 03:08:34AM -0700, Andrew Morton wrote:
> No, this doesn't "typically" deadlock.  It will deadlock on every ten
> millionth call.  The preceding 9,999,999 warnings should have imparted
> sufficient clue?

I wasn't counting function calls. I suppose it won't deadlock on every
call.


-- wli