diff mbox

[-next] tracepoint: ipi: don't trace IPI on offline CPUs

Message ID 1457526142-31923-1-git-send-email-sudeep.holla@arm.com
State New
Headers show

Commit Message

Sudeep Holla March 9, 2016, 12:22 p.m. UTC 
Tracepoints use RCU for protection and they must not be called on
offline CPUS. So make this tracepoint conditional. Otherwise it can
produce the following warning:

--
1.9.1

Comments

Sudeep Holla March 9, 2016, 4:40 p.m. UTC | #1 
On 09/03/16 16:05, Steven Rostedt wrote:
> On Wed,  9 Mar 2016 12:22:22 +0000

> Sudeep Holla <sudeep.holla@arm.com> wrote:

>

>

>> Hi Steven,

>>

>> I observed that in "include/linux/tracepoint.h", we have

>> #define __DO_TRACE(tp, proto, args, cond, prercu, postrcu)

>> ...

>>                   if (!cpu_online(raw_smp_processor_id()))

>>                           return;

>>

>>                   if (!(cond))

>>                           return;

>> ...

>>

>> where !cond check seems reduntant if it's cpu_online check.

>> So, does this patch handle the warning correctly or is there any better

>> way ? I did see few traces with same condition, just thought of checking

>> with you.

>>

>

> Bah, I forgot that we have lockdep checks for when the event isn't

> enabled.


Yes I was about to ask you the same. I did further digging to check if I
was missing something after seeing your series[1] especially patch 2/12
(tracing: Remove duplicate checks for online CPUs)

> Can you try this patch:


It works. Thanks for the quick fix.

Tested-by: Sudeep Holla <sudeep.holla@arm.com>


--
Regards,
Sudeep

[1] http://www.spinics.net/lists/kernel/msg2208604.html
diff mbox

Patch

===============================
[ INFO: suspicious RCU usage. ]
4.5.0-rc7-next-20160308 #44 Not tainted
-------------------------------
include/trace/events/ipi.h:35 suspicious rcu_dereference_check() usage!

other info that might help us debug this:

RCU used illegally from offline CPU!
rcu_scheduler_active = 1, debug_locks = 1
no locks held by swapper/3/0.

stack backtrace:
CPU: 3 PID: 0 Comm: swapper/3 Not tainted 4.5.0-rc7-next-20160308 #44
Hardware name: ARM Juno development board (r2) (DT)
Call trace:
 dump_backtrace+0x0/0x218
 show_stack+0x24/0x30
 dump_stack+0xb4/0xf0
 lockdep_rcu_suspicious+0xd0/0x118
 smp_cross_call+0x1a8/0x1b0
 arch_send_call_function_single_ipi+0x3c/0x48
 generic_exec_single+0xc0/0x178
 smp_call_function_single+0xec/0x1b8
 cpuhp_report_idle_dead+0x70/0x88
 cpu_startup_entry+0x358/0x3c0
 secondary_start_kernel+0x190/0x1e8

Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Ingo Molnar <mingo@redhat.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 include/trace/events/ipi.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Hi Steven,

I observed that in "include/linux/tracepoint.h", we have
#define __DO_TRACE(tp, proto, args, cond, prercu, postrcu)
...
                 if (!cpu_online(raw_smp_processor_id()))
                         return;

                 if (!(cond))
                         return;
...

where !cond check seems reduntant if it's cpu_online check.
So, does this patch handle the warning correctly or is there any better
way ? I did see few traces with same condition, just thought of checking
with you.

Regards,
Sudeep

diff --git a/include/trace/events/ipi.h b/include/trace/events/ipi.h
index 834a7362a610..d8ca2f2e24ac 100644
--- a/include/trace/events/ipi.h
+++ b/include/trace/events/ipi.h
@@ -15,12 +15,14 @@ 
  * It is necessary for @reason to be a static string declared with
  * __tracepoint_string.
  */
-TRACE_EVENT(ipi_raise,
+TRACE_EVENT_CONDITION(ipi_raise,

 	TP_PROTO(const struct cpumask *mask, const char *reason),

 	TP_ARGS(mask, reason),

+	TP_CONDITION(cpu_online(raw_smp_processor_id())),
+
 	TP_STRUCT__entry(
 		__bitmask(target_cpus, nr_cpumask_bits)
 		__field(const char *, reason)