tracing: A couple of fixes

Linus,

This includes two fixes.

The first is something that has come up a few times and has been worked out individually, but it's come up now enough that the problem should be generic. Tracepoints are protected by RCU sched. There are several tracepoints within core infrastructure like kfree().
If a tracepoint is called when the CPU is going down, or when it's coming up but has yet to be recognized by RCU, a RCU warning is triggered. This is a true bug as that tracepoint is not protected by RCU. Usually, this is taken care of by testing for cpu online as a tracepoint condition. But as this is happening more often, moving it from a individual tracepoint to a check in the tracepoint infrastructure is more robust.

Note, there is now a duplicate of a cpu online test, because this update does not remove the individual checks. But the overhead is small enough that the removal can be done in another release.

The second change is strange linker breakage due to the branch tracer's builtin_constant_p() check failing, and treating the condition as a variable instead of a constant. Arnd Bergmann found that this can be fixed by testing !!(cond) instead of just (cond).

Please pull the latest trace-fixes-v4.5-rc4 tree, which can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
trace-fixes-v4.5-rc4

Tag SHA1: abfeb62bfa1a219b42a0e984db59f6535a741b17 Head SHA1: b33c8ff4431a343561e2319f17c14286f2aa52e2

Arnd Bergmann (1):
tracing: Fix freak link error caused by branch tracer

Steven Rostedt (Red Hat) (1):
tracepoints: Do not trace when cpu is offline

----

 include/linux/compiler.h   | 2 +-
 include/linux/tracepoint.h | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

From: "Steven Rostedt (Red Hat)" <[email protected]>

The tracepoint infrastructure uses RCU sched protection to enable and disable tracepoints safely. There are some instances where tracepoints are used in infrastructure code (like kfree()) that get called after a CPU is going offline, and perhaps when it is coming back online but hasn't been registered yet.

This can probuce the following warning:

[ INFO: suspicious RCU usage. ]
4.4.0-00006-g0fe53e8-dirty #34 Tainted: G S
-------------------------------
include/trace/events/kmem.h:141 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/8/0.

 stack backtrace:
  CPU: 8 PID: 0 Comm: swapper/8 Tainted: G S              4.4.0-00006-g0fe53e8-dirty #34
  Call Trace:
  [c0000005b76c78d0] [c0000000008b9540] .dump_stack+0x98/0xd4 (unreliable)
  [c0000005b76c7950] [c00000000010c898] .lockdep_rcu_suspicious+0x108/0x170
  [c0000005b76c79e0] [c00000000029adc0] .kfree+0x390/0x440
  [c0000005b76c7a80] [c000000000055f74] .destroy_context+0x44/0x100
  [c0000005b76c7b00] [c0000000000934a0] .__mmdrop+0x60/0x150
  [c0000005b76c7b90] [c0000000000e3ff0] .idle_task_exit+0x130/0x140
  [c0000005b76c7c20] [c000000000075804] .pseries_mach_cpu_die+0x64/0x310
  [c0000005b76c7cd0] [c000000000043e7c] .cpu_die+0x3c/0x60
  [c0000005b76c7d40] [c0000000000188d8] .arch_cpu_idle_dead+0x28/0x40
  [c0000005b76c7db0] [c000000000101e6c] .cpu_startup_entry+0x50c/0x560
  [c0000005b76c7ed0] [c000000000043bd8] .start_secondary+0x328/0x360
  [c0000005b76c7f90] [c000000000008a6c] start_secondary_prolog+0x10/0x14

This warning is not a false positive either. RCU is not protecting code that is being executed while the CPU is offline.

Instead of playing "whack-a-mole(TM)" and adding conditional statements to the tracepoints we find that are used in this instance, simply add a cpu_online() test to the tracepoint code where the tracepoint will be ignored if the CPU is offline.

Use of raw_smp_processor_id() is fine, as there should never be a case where the tracepoint code goes from running on a CPU that is online and suddenly gets migrated to a CPU that is offline.

---
 include/linux/tracepoint.h | 5 +++++
 1 file changed, 5 insertions(+)

index acd522a91539..acfdbf353a0b 100644
@@ -14,8 +14,10 @@
  * See the file COPYING for more details.
  */
 
+#include <linux/smp.h>
 #include <linux/errno.h>
 #include <linux/types.h>
+#include <linux/cpumask.h>
 #include <linux/rcupdate.h>
 #include <linux/tracepoint-defs.h>
 

@@ -132,6 +134,9 @@ extern void syscall_unregfunc(void);
 		void *it_func;						\
 		void *__data;						\
 									\
+		if (!cpu_online(raw_smp_processor_id()))		\
+			return;						\
+									\
 		if (!(cond))						\
 			return;						\
 		prercu;							\

2.6.4

If I get this right, you are proposing to "hide" events happening during CPU hot-unplug on dying CPUs from the tracers to fix an issue caused by interaction of RCU-sched (used for Tracepoint synchronization) wrt CPU hotplug.

Removing tracing visibility of hot-unplug events seems to be an unwelcome side-effect. I don't know how far Thomas Gleixner got in his overhaul of CPU hotplug, but he might have something to say about this, as I believe he would be the first user concerned.

Thoughts ?

Thanks,

Mathieu

Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

Well, trace_printk() still works. But right now you *can't* have a tracepoint executed on a CPU that is offline, because it is a bug. Period. That's because we use RCU sched to protect tracepoints. When the CPU is offline, there is no protection. It is possible that the tracepoint structures may get corrupted, or worse, crash the system. Granted, the race is quite small but it is a bug never the less.

Now, if you want tracepoints to be visible for CPUs that are offline, then we need something else to protect it. But until then, this fixes the issue.

And before this patch, we've been adding conditional tracepoints to check "if (cpu_online(raw_smp_processor_id()))" when a warning appeared. This patch gets rid of the need to keep adding these whack-a-mole patches.

-- Steve

From: Arnd Bergmann <[email protected]>

In my randconfig tests, I came across a bug that involves several components:

* gcc-4.9 through at least 5.3
* CONFIG_GCOV_PROFILE_ALL enabling -fprofile-arcs for all files
* CONFIG_PROFILE_ALL_BRANCHES overriding every if()
* The optimized implementation of do_div() that tries to replace a library call with an division by multiplication
* code in drivers/media/dvb-frontends/zl10353.c doing

        u32 adc_clock = 450560; /* 45.056 MHz */
        if (state->config.adc_clock)
                adc_clock = state->config.adc_clock;
        do_div(value, adc_clock);

In this case, gcc fails to determine whether the divisor in do_div() is __builtin_constant_p(). In particular, it concludes that __builtin_constant_p(adc_clock) is false, while __builtin_constant_p(!!adc_clock) is true.

That in turn throws off the logic in do_div() that also uses __builtin_constant_p(), and instead of picking either the constant- optimized division, and the code in ilog2() that uses __builtin_constant_p() to figure out whether it knows the answer at compile time. The result is a link error from failing to find multiple symbols that should never have been called based on the __builtin_constant_p():

dvb-frontends/zl10353.c:138: undefined reference to ____ilog2_NaN' dvb-frontends/zl10353.c:138: undefined reference to __aeabi_uldivmod'
ERROR: "____ilog2_NaN" [drivers/media/dvb-frontends/zl10353.ko] undefined!
ERROR: "__aeabi_uldivmod" [drivers/media/dvb-frontends/zl10353.ko] undefined!

This patch avoids the problem by changing __trace_if() to check whether the condition is known at compile-time to be nonzero, rather than checking whether it is actually a constant.

I see this one link error in roughly one out of 1600 randconfig builds on ARM, and the patch fixes all known instances.

---
 include/linux/compiler.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 00b042c49ccd..48f5aab117ae 100644
@@ -144,7 +144,7 @@ void ftrace_likely_update(struct ftrace_branch_data *f, int val, int expect);
  */
 #define if(cond, ...) __trace_if( (cond , ## __VA_ARGS__) )
 #define __trace_if(cond) \
-	if (__builtin_constant_p((cond)) ? !!(cond) :			\
+	if (__builtin_constant_p(!!(cond)) ? !!(cond) :			\
 	({								\
 		int ______r;						\
 		static struct ftrace_branch_data			\

2.6.4