Re: linux-next: Tree for Feb 4

[Steven Rostedt <rostedt@goodmis.org>]

On Thu, 5 Feb 2015 23:16:21 +0100
Sedat Dilek <sedat.dilek@gmail.com> wrote:

> On Thu, Feb 5, 2015 at 11:09 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> > On Thu, 5 Feb 2015 22:45:59 +0100
> > Sedat Dilek <sedat.dilek@gmail.com> wrote:
> >
> >> Steve, this was a typo it's called tlb_flush not tlb_flush*ed*:
> >
> > Heh, yeah, I typed that entire line in by hand. Just be lucky that was
> > the only typo ;-)
> >
> >>
> >> # cat /sys/kernel/debug/tracing/events/tlb/tlb_flush/enable
> >> 1
> >>
> >> [  391.090381] intel_pstate CPU 1 exiting
> >> [  391.104491] smpboot: CPU 1 is now offline
> >>
> >
> > Now, if you disable that (echo 0 to that file), do you still get the
> > rcu lockdep splat if you suspend and resume?
> >
> 
> YES, I get the call-trace again!
> 

Bah! I see where the warning comes from. In include/linux/tracepoint.h
we have:

#define __DECLARE_TRACE(name, proto, args, cond, data_proto, data_args) \
	extern struct tracepoint __tracepoint_##name;			\
	static inline void trace_##name(proto)				\
	{								\
		if (static_key_false(&__tracepoint_##name.key))		\
			__DO_TRACE(&__tracepoint_##name,		\
				TP_PROTO(data_proto),			\
				TP_ARGS(data_args),			\
				TP_CONDITION(cond),,);			\
		if (IS_ENABLED(CONFIG_LOCKDEP)) {			\
			rcu_read_lock_sched_notrace();			\
			rcu_dereference_sched(__tracepoint_##name.funcs);\
			rcu_read_unlock_sched_notrace();		\
		}							\
	}								\

See that if (IS_ENABLED(CONFIG_LOCKDEP))?

I'm recalling this. Because tracepoints require RCU, and RCU lockdep
doesn't trigger if a tracepoint isn't enabled (because the rcu calls
are hidden in the __DO_TRACE() behind that static_key_false), we would
be missing lots of rcu problem tracepoints because tests were run
without them enabled.

The answer was to add this rcu check when LOCKDEP was enabled. So no,
adding that conditional isn't going to help, because lockdep will
trigger here, even if it were safe because of the conditional :-/.

That said, let's add this (on top of the old patch):

(again, not tested)

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
-------
diff --git a/arch/x86/include/asm/mmu_context.h b/arch/x86/include/asm/mmu_context.h
index 4b75d591eb5e..401b5bfbcdbd 100644
--- a/arch/x86/include/asm/mmu_context.h
+++ b/arch/x86/include/asm/mmu_context.h
@@ -47,7 +47,12 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
 
 		/* Re-load page tables */
 		load_cr3(next->pgd);
-		trace_tlb_flush(TLB_FLUSH_ON_TASK_SWITCH, TLB_FLUSH_ALL);
+		/*
+		 * Do not check rcu when tracing is not enabled. The
+		 * tracepoint has a condition to not trace if the CPU is
+		 * offline, and rcu check will complain if it is.
+		 */
+		trace_tlb_flush_rcu_nocheck(TLB_FLUSH_ON_TASK_SWITCH, TLB_FLUSH_ALL);
 
 		/* Stop flush ipis for the previous mm */
 		cpumask_clear_cpu(cpu, mm_cpumask(prev));
@@ -84,7 +89,13 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
 			 * to make sure to use no freed page tables.
 			 */
 			load_cr3(next->pgd);
-			trace_tlb_flush(TLB_FLUSH_ON_TASK_SWITCH, TLB_FLUSH_ALL);
+			/*
+			 * Do not check rcu when tracing is not enabled. The
+			 * tracepoint has a condition to not trace if the CPU is
+			 * offline, and rcu check will complain if it is.
+			 */
+			trace_tlb_flush_rcu_nocheck(TLB_FLUSH_ON_TASK_SWITCH,
+						    TLB_FLUSH_ALL);
 			load_LDT_nolock(&next->context);
 		}
 	}
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index e08e21e5f601..747a05aceb60 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -179,6 +179,14 @@ extern void syscall_unregfunc(void);
 			rcu_read_unlock_sched_notrace();		\
 		}							\
 	}								\
+	static inline void trace_##name##_rcu_nocheck(proto)		\
+	{								\
+		if (static_key_false(&__tracepoint_##name.key))		\
+			__DO_TRACE(&__tracepoint_##name,		\
+				TP_PROTO(data_proto),			\
+				TP_ARGS(data_args),			\
+				TP_CONDITION(cond),,);			\
+	}								\
 	__DECLARE_TRACE_RCU(name, PARAMS(proto), PARAMS(args),		\
 		PARAMS(cond), PARAMS(data_proto), PARAMS(data_args))	\
 	static inline int						\
@@ -230,6 +238,8 @@ extern void syscall_unregfunc(void);
 #define __DECLARE_TRACE(name, proto, args, cond, data_proto, data_args) \
 	static inline void trace_##name(proto)				\
 	{ }								\
+	static inline void trace_##name##_rcu_nocheck(proto)		\
+	{ }								\
 	static inline void trace_##name##_rcuidle(proto)		\
 	{ }								\
 	static inline int						\