lkml.org 
[lkml]   [2007]   [Jul]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[PATCH] UP: smp_call_function_single() must warn on irqs_disabled()


On Wed, 18 Jul 2007, Al Viro wrote:

> On Wed, Jul 18, 2007 at 01:24:46AM +0200, Uwe Kleine-Konig wrote:
> > [...]
> > a52b1752c07 introduces usage of the WARN_ON macro in <linux/smp.h>, but
> > doesn't pull in <linux/kernel.h>. (<asm/bug.h> is not enough, at least
> > for arm, because WARN_ON uses printk there.)
> >
> > The obvious options are:
> >
> > 1) include <linux/kernel.h> in <linux/smp.h>, maybe conditioned by !SMP
> > 2) include <linux/kernel.h> in all includers of <linux/smp.h>
> > 3) remove the WARN_ONs introduced by a52b1752c07.
> > [...]
>
> 4) turn the sucker into macro


I think that warning is incorrect anyway ... and I'm to blame
for that. What happened is that I had earlier (month or two back,
_before_ the recent change in smp_call_function_single() semantics)
submitted a patch that put an unconditional WARN_ON(1) in this
function for UP -- in those days, both the smp_call_function*
variants were illegal on the current CPU itself.
( http://lkml.org/lkml/2007/6/7/262 )

Then recently, Andi / Avi proposed the new semantics for
smp_call_function_single() to allow it to just execute the given
function on current CPU as well -- smp_call_function() semantics
were left unchanged, so as not to break stuff like smp_send_stop().

I think I got confused and recommended the (cpuid != 0) warning,
but I don't really think it is necessary with new semantics --
sorry about this, Avi.

OTOH, the function _should_ include a WARN_ON, but for a different
condition -- irqs_disabled(), as explained in changelog below.

I'd be grateful if someone could apply this, Cc:'ing Andi too.

Satyam

---
[PATCH] UP: smp_call_function_single() must warn on irqs_disabled()

Because:
(1) smp_call_function_single() semantics dictate that.
(2) That makes UP behaviour similar to SMP case (implementations
of smp_call_function_single() in all the archs do this).
(3) We use the unconditional non-save/restore-flags versions of
local_irq_disable/enable just below this, so it's a bug to
call this function with IRQs disabled anyway.

Also remove the cpuid != 0 warning that I had erroneously
suggested to Avi earlier (sorry!)

Signed-off-by: Satyam Sharma <ssatyam@cse.iitk.ac.in>
Cc: Andi Kleen <ak@suse.de>
Cc: Avi Kivity <avi@qumranet.com>

---
include/linux/smp.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/linux/smp.h b/include/linux/smp.h
index 259a13c..016dab5 100644
--- a/include/linux/smp.h
+++ b/include/linux/smp.h
@@ -101,7 +101,7 @@ static inline void smp_send_reschedule(int cpu) { }
#define smp_prepare_boot_cpu() do {} while (0)
#define smp_call_function_single(cpuid, func, info, retry, wait) \
({ \
- WARN_ON(cpuid != 0); \
+ WARN_ON(irqs_disabled()); \
local_irq_disable(); \
(func)(info); \
local_irq_enable(); \
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
\
 
 \ /
  Last update: 2007-07-18 05:55    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site