wrong usage of smp_processor_id()

PATCH In review Daniel Walker 2 participants 5 messages May 3, 2007

lore

Share & tools

lore

Daniel Walker

author

2007-05-03 20:10 UTC #0 lore raw

PATCH 1/1 wrong usage of smp_processor_id()

Found this while debugging with Steve .. trace_hardirqs_on() isn't always called with interrupt off.

 BUG: using smp_processor_id() in preemptible [00000000] code: init/1

caller is trace_hardirqs_on+0x16/0xe0

  [<c01054b1>] show_trace_log_lvl+0x21/0x50
  [<c0105c82>] show_trace+0x12/0x20
  [<c0105d56>] dump_stack+0x16/0x20
  [<c022c819>] debug_smp_processor_id+0xa9/0xb0
  [<c0152a76>] trace_hardirqs_on+0x16/0xe0
  [<c0103975>] do_notify_resume+0x115/0x7b0
  [<c01044f4>] work_notifysig+0x13/0x1b
  =======================
 ---------------------------
 | preempt count: 00000001 ]
 | 1-level deep critical section nesting:
 ----------------------------------------
 .. [<c022c7c3>] .... debug_smp_processor_id+0x53/0xb0
 .....[<c0152a76>] ..   ( <= trace_hardirqs_on+0x16/0xe0)

Signed-off-by Daniel Walker <[email protected]>

---
 kernel/latency_trace.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

kernel/latency_trace.c +1 -1

Index: linux-2.6.21/kernel/latency_trace.c
===================================================================
@@ -105,7 +105,7 @@ static inline int DEBUG_WARN_ON(int cond
 #ifdef CONFIG_CRITICAL_IRQSOFF_TIMING
 # ifdef CONFIG_CRITICAL_PREEMPT_TIMING
    static DEFINE_PER_CPU(int, trace_cpu_idle);
-#  define irqs_off_preempt_count() (!__get_cpu_var(trace_cpu_idle) && preempt_count())
+#  define irqs_off_preempt_count() (preempt_count() && !__get_cpu_var(trace_cpu_idle))
 # else
 #  define irqs_off_preempt_count() 0
 # endif

Signature · 1 line

--

Steven Rostedt

Acked-by on v1 1/1

2007-05-03 20:14 UTC #1 lore raw

Quoted 9 lines · Daniel Walker

---
 kernel/latency_trace.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-2.6.21/kernel/latency_trace.c
===================================================================

kernel/latency_trace.c static inline int DEBUG_WARN_ON(int cond · line 111

Quoted 8 lines

kernel/latency_trace.c +1 -1

@@ -105,7 +105,7 @@ static inline int DEBUG_WARN_ON(int cond
 #ifdef CONFIG_CRITICAL_IRQSOFF_TIMING
 # ifdef CONFIG_CRITICAL_PREEMPT_TIMING
    static DEFINE_PER_CPU(int, trace_cpu_idle);
-#  define irqs_off_preempt_count() (!__get_cpu_var(trace_cpu_idle) && preempt_count())
+#  define irqs_off_preempt_count() (preempt_count() && !__get_cpu_var(trace_cpu_idle))
 # else
 #  define irqs_off_preempt_count() 0
 # endif

Ingo,

This is on top of one of my patches that I sent you (the cpu idle one). So it's not for the 2.6.21-rt1, but if you apply my patch, this needs to be added too.

Acked-by Steven Rostedt <[email protected]>

-- Steve

Daniel Walker

author

2007-05-03 20:23 UTC #2 lore raw

Quoted 22 lines · Steven Rostedt

Quoted 9 lines · Daniel Walker
---
 kernel/latency_trace.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-2.6.21/kernel/latency_trace.c
===================================================================
kernel/latency_trace.c static inline int DEBUG_WARN_ON(int cond · line 111
Quoted 8 lines
kernel/latency_trace.c +1 -1
@@ -105,7 +105,7 @@ static inline int DEBUG_WARN_ON(int cond
 #ifdef CONFIG_CRITICAL_IRQSOFF_TIMING
 # ifdef CONFIG_CRITICAL_PREEMPT_TIMING
    static DEFINE_PER_CPU(int, trace_cpu_idle);
-#  define irqs_off_preempt_count() (!__get_cpu_var(trace_cpu_idle) && preempt_count())
+#  define irqs_off_preempt_count() (preempt_count() && !__get_cpu_var(trace_cpu_idle))
 # else
 #  define irqs_off_preempt_count() 0
 # endif
Ingo,

Quoted 3 lines

This is on top of one of my patches that I sent you (the cpu idle one). So it's not for the 2.6.21-rt1, but if you apply my patch, this needs to be added too.

Isn't it a generic problem? It doesn't look specific to your change unless you made some changes to entry.S .

Daniel

Steven Rostedt

2007-05-03 20:32 UTC #3 lore raw

Quoted 9 lines · Daniel Walker

Quoted 9 lines
Quoted 9 lines
Index: linux-2.6.21/kernel/latency_trace.c
===================================================================
kernel/latency_trace.c +0 -1
@@ -105,7 +105,7 @@ static inline int DEBUG_WARN_ON(int cond
 #ifdef CONFIG_CRITICAL_IRQSOFF_TIMING
 # ifdef CONFIG_CRITICAL_PREEMPT_TIMING
    static DEFINE_PER_CPU(int, trace_cpu_idle);
-#  define irqs_off_preempt_count() (!__get_cpu_var(trace_cpu_idle) && preempt_count())

Quoted 9 lines

Quoted 4 lines
kernel/latency_trace.c static inline int DEBUG_WARN_ON(int cond · line 111
Quoted 4 lines
kernel/latency_trace.c +1 -0
+#  define irqs_off_preempt_count() (preempt_count() && !__get_cpu_var(trace_cpu_idle))
 # else
 #  define irqs_off_preempt_count() 0
 # endif
Isn't it a generic problem? It doesn't look specific to your change unless you made some changes to entry.S .

See: http://lkml.org/lkml/2007/4/23/183

Specifically:

#ifdef CONFIG_CRITICAL_IRQSOFF_TIMING

 # ifdef CONFIG_CRITICAL_PREEMPT_TIMING
-#  define irqs_off_preempt_count() preempt_count()
+   static DEFINE_PER_CPU(int, trace_cpu_idle);
+#  define irqs_off_preempt_count() (!__get_cpu_var(trace_cpu_idle) && preempt_count())
 # else
 #  define irqs_off_preempt_count() 0
 # endif
@@ -2153,6 +2154,20 @@ void notrace unmask_preempt_count(unsign
 }
 EXPORT_SYMBOL(unmask_preempt_count);

-- Steve

Daniel Walker

author

2007-05-03 20:34 UTC #4 lore raw

Quoted 21 lines · Steven Rostedt

Quoted 9 lines · Daniel Walker
Quoted 9 lines
Quoted 9 lines
Index: linux-2.6.21/kernel/latency_trace.c
===================================================================
kernel/latency_trace.c +0 -1
@@ -105,7 +105,7 @@ static inline int DEBUG_WARN_ON(int cond
 #ifdef CONFIG_CRITICAL_IRQSOFF_TIMING
 # ifdef CONFIG_CRITICAL_PREEMPT_TIMING
    static DEFINE_PER_CPU(int, trace_cpu_idle);
-#  define irqs_off_preempt_count() (!__get_cpu_var(trace_cpu_idle) && preempt_count())
Quoted 9 lines
Quoted 4 lines
kernel/latency_trace.c static inline int DEBUG_WARN_ON(int cond · line 111
Quoted 4 lines
kernel/latency_trace.c +1 -0
+#  define irqs_off_preempt_count() (preempt_count() && !__get_cpu_var(trace_cpu_idle))
 # else
 #  define irqs_off_preempt_count() 0
 # endif
Isn't it a generic problem? It doesn't look specific to your change unless you made some changes to entry.S .

Quoted 2 lines

See: http://lkml.org/lkml/2007/4/23/183

Ahh, well safe bet I think.

Daniel