tracing/rcu: Add _rcuidle() tracepoint to handle rcu_idle_exit() tracepoints

RFC PATCH In review Steven Rostedt 3 participants 10 messages Feb 7, 2012

Subsystems (3)

x86 Tracing CPU idle time management framework

lore

Share & tools

lore

Steven Rostedt

author maintainer

2012-02-07 15:10 UTC #0 lore raw

cover tracing/rcu: Add _rcuidle() tracepoint to handle rcu_idle_exit() tracepoints

This is an RFC patch set for what I plan on pushing out to handle the tracepoints that are called from within rcu_idle_exit() sections.

The first patch is the tracepoint infrastructure,
the second is the changes in x86
the third is the changes in the cpuidle driver.

I see there's use cases in the arm arch, but as that is in flux, I'll let the arm folks deal with those.

-- Steve

Steven Rostedt (3):
tracing/rcu: Add trace_##name##__rcuidle() static tracepoint for inside rcu_idle_exit() sections x86/tracing: Denote the power and cpuidle tracepoints as _rcuidle() cpuidle/tracing: Denote the tracepoints as being in rcu_idle_exit() section

----

 arch/x86/kernel/process.c  |   24 ++++++++++++------------
 drivers/cpuidle/cpuidle.c  |    8 ++++----
 include/linux/tracepoint.h |   20 ++++++++++++++++----
 3 files changed, 32 insertions(+), 20 deletions(-)

Steven Rostedt

author maintainer

2012-02-07 15:11 UTC #1 lore raw

RFC PATCH 1/3 tracing/rcu: Add trace_##name##__rcuidle() static tracepoint for inside rcu_idle_exit() sections

From: Steven Rostedt <[email protected]>

Added is a new static inline function that lets *any* tracepoint be used inside a rcu_idle_exit() section. And this also solves the problem where the same tracepoint may be used inside a rcu_idle_exit() section as well as outside of one.

I added a new tracepoint function with a "_rcuidle" extension. All tracepoints can be used with either the normal "trace_foobar()" function, or the "trace_foobar_rcuidle()" function when inside a rcu_idle_exit() section.

All tracepoints defined by TRACE_EVENT() or any of the derivatives will have a "_rcuidle()" function also defined. When a tracepoint is used within an rcu_idle_exit() section, the "_rcuidle()" version must be used. This denotes that the tracepoint is within rcu_idle_exit() and it allows the rcu read locks within the tracepoint to still be valid, as this version takes us out of rcu_idle_exit().

Another nice aspect about this patch is that "static inline"s are not compiled into text when not used. So only the tracepoints that actually use the _rcuidle() version will have them defined in the actual text that is booted.

Link http://lkml.kernel.org/r/[email protected]> open here

Acked-by Paul E. McKenney <[email protected]>

Reviewed-by Josh Triplett <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 include/linux/tracepoint.h |   20 ++++++++++++++++----
 1 files changed, 16 insertions(+), 4 deletions(-)

include/linux/tracepoint.h +16 -4

index df0a779..00d073a 100644
@@ -114,7 +114,7 @@ static inline void tracepoint_synchronize_unregister(void)
  * as "(void *, void)". The DECLARE_TRACE_NOARGS() will pass in just
  * "void *data", where as the DECLARE_TRACE() will pass in "void *data, proto".
  */
-#define __DO_TRACE(tp, proto, args, cond)				\
+#define __DO_TRACE(tp, proto, args, cond, prercu, postrcu)		\
 	do {								\
 		struct tracepoint_func *it_func_ptr;			\
 		void *it_func;						\

@@ -122,6 +122,7 @@ static inline void tracepoint_synchronize_unregister(void)
 									\
 		if (!(cond))						\
 			return;						\
+		prercu;							\
 		rcu_read_lock_sched_notrace();				\
 		it_func_ptr = rcu_dereference_sched((tp)->funcs);	\
 		if (it_func_ptr) {					\

@@ -132,6 +133,7 @@ static inline void tracepoint_synchronize_unregister(void)
 			} while ((++it_func_ptr)->func);		\
 		}							\
 		rcu_read_unlock_sched_notrace();			\
+		postrcu;						\
 	} while (0)
 
 /*

@@ -139,7 +141,7 @@ static inline void tracepoint_synchronize_unregister(void)
  * not add unwanted padding between the beginning of the section and the
  * structure. Force alignment to the same alignment as the section start.
  */
-#define __DECLARE_TRACE(name, proto, args, cond, data_proto, data_args)	\
+#define __DECLARE_TRACE(name, proto, args, cond, data_proto, data_args) \
 	extern struct tracepoint __tracepoint_##name;			\
 	static inline void trace_##name(proto)				\
 	{								\

@@ -147,7 +149,17 @@ static inline void tracepoint_synchronize_unregister(void)
 			__DO_TRACE(&__tracepoint_##name,		\
 				TP_PROTO(data_proto),			\
 				TP_ARGS(data_args),			\
-				TP_CONDITION(cond));			\
+				TP_CONDITION(cond),,);			\
+	}								\
+	static inline void trace_##name##_rcuidle(proto)		\
+	{								\
+		if (static_branch(&__tracepoint_##name.key))		\
+			__DO_TRACE(&__tracepoint_##name,		\
+				TP_PROTO(data_proto),			\
+				TP_ARGS(data_args),			\
+				TP_CONDITION(cond),			\
+				rcu_idle_exit(),			\
+				rcu_idle_enter());			\
 	}								\
 	static inline int						\
 	register_trace_##name(void (*probe)(data_proto), void *data)	\

@@ -190,7 +202,7 @@ static inline void tracepoint_synchronize_unregister(void)
 	EXPORT_SYMBOL(__tracepoint_##name)
 
 #else /* !CONFIG_TRACEPOINTS */
-#define __DECLARE_TRACE(name, proto, args, cond, data_proto, data_args)	\
+#define __DECLARE_TRACE(name, proto, args, cond, data_proto, data_args) \
 	static inline void trace_##name(proto)				\
 	{ }								\
 	static inline int						\

Signature · 1 line

1.7.8.3

Steven Rostedt

author maintainer

2012-02-07 15:10 UTC #2 lore raw

RFC PATCH 2/3 x86/tracing: Denote the power and cpuidle tracepoints as _rcuidle()

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

The power and cpuidle tracepoints are called within a rcu_idle_exit() section, and must be denoted with the _rcuidle() version of the tracepoint.

Cc Paul E. McKenney <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 arch/x86/kernel/process.c |   24 ++++++++++++------------
 1 files changed, 12 insertions(+), 12 deletions(-)

arch/x86/kernel/process.c +12 ~~-12~~

index 15763af..44eefde 100644
@@ -377,8 +377,8 @@ static inline int hlt_use_halt(void)
 void default_idle(void)
 {
 	if (hlt_use_halt()) {
-		trace_power_start(POWER_CSTATE, 1, smp_processor_id());
-		trace_cpu_idle(1, smp_processor_id());
+		trace_power_start_rcuidle(POWER_CSTATE, 1, smp_processor_id());
+		trace_cpu_idle_rcuidle(1, smp_processor_id());
 		current_thread_info()->status &= ~TS_POLLING;
 		/*
 		 * TS_POLLING-cleared state must be visible before we

@@ -391,8 +391,8 @@ void default_idle(void)
 		else
 			local_irq_enable();
 		current_thread_info()->status |= TS_POLLING;
-		trace_power_end(smp_processor_id());
-		trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
+		trace_power_end_rcuidle(smp_processor_id());
+		trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, smp_processor_id());
 	} else {
 		local_irq_enable();
 		/* loop is done by the caller */

@@ -450,8 +450,8 @@ EXPORT_SYMBOL_GPL(cpu_idle_wait);
 static void mwait_idle(void)
 {
 	if (!need_resched()) {
-		trace_power_start(POWER_CSTATE, 1, smp_processor_id());
-		trace_cpu_idle(1, smp_processor_id());
+		trace_power_start_rcuidle(POWER_CSTATE, 1, smp_processor_id());
+		trace_cpu_idle_rcuidle(1, smp_processor_id());
 		if (this_cpu_has(X86_FEATURE_CLFLUSH_MONITOR))
 			clflush((void *)&current_thread_info()->flags);

@@ -461,8 +461,8 @@ static void mwait_idle(void)
 			__sti_mwait(0, 0);
 		else
 			local_irq_enable();
-		trace_power_end(smp_processor_id());
-		trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
+		trace_power_end_rcuidle(smp_processor_id());
+		trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, smp_processor_id());
 	} else
 		local_irq_enable();
 }

@@ -474,13 +474,13 @@ static void mwait_idle(void)
  */
 static void poll_idle(void)
 {
-	trace_power_start(POWER_CSTATE, 0, smp_processor_id());
-	trace_cpu_idle(0, smp_processor_id());
+	trace_power_start_rcuidle(POWER_CSTATE, 0, smp_processor_id());
+	trace_cpu_idle_rcuidle(0, smp_processor_id());
 	local_irq_enable();
 	while (!need_resched())
 		cpu_relax();
-	trace_power_end(smp_processor_id());
-	trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
+	trace_power_end_rcuidle(smp_processor_id());
+	trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, smp_processor_id());
 }
 
 /*

Signature · 1 line

1.7.8.3

Paul E. McKenney

Acked-by on v1 2/3

2012-02-07 22:54 UTC #3 lore raw

Re: [PATCH 2/3][RFC] x86/tracing: Denote the power and cpuidle tracepoints as _rcuidle()

Quoted 7 lines · Steven Rostedt

From: Steven Rostedt <[email protected]>
The power and cpuidle tracepoints are called within a rcu_idle_exit() section, and must be denoted with the _rcuidle() version of the tracepoint.
Cc Paul E. McKenney <[email protected]>
Signed-off-by Steven Rostedt <[email protected]>

(Sorry, fat-fingered this one the first time around.)

Acked-by Paul E. McKenney <[email protected]>

Quoted 80 lines

---
 arch/x86/kernel/process.c |   24 ++++++++++++------------
 1 files changed, 12 insertions(+), 12 deletions(-)

arch/x86/kernel/process.c +12 ~~-12~~

index 15763af..44eefde 100644
@@ -377,8 +377,8 @@ static inline int hlt_use_halt(void)
 void default_idle(void)
 {
 	if (hlt_use_halt()) {
-		trace_power_start(POWER_CSTATE, 1, smp_processor_id());
-		trace_cpu_idle(1, smp_processor_id());
+		trace_power_start_rcuidle(POWER_CSTATE, 1, smp_processor_id());
+		trace_cpu_idle_rcuidle(1, smp_processor_id());
 		current_thread_info()->status &= ~TS_POLLING;
 		/*
 		 * TS_POLLING-cleared state must be visible before we

@@ -391,8 +391,8 @@ void default_idle(void)
 		else
 			local_irq_enable();
 		current_thread_info()->status |= TS_POLLING;
-		trace_power_end(smp_processor_id());
-		trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
+		trace_power_end_rcuidle(smp_processor_id());
+		trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, smp_processor_id());
 	} else {
 		local_irq_enable();
 		/* loop is done by the caller */

@@ -450,8 +450,8 @@ EXPORT_SYMBOL_GPL(cpu_idle_wait);
 static void mwait_idle(void)
 {
 	if (!need_resched()) {
-		trace_power_start(POWER_CSTATE, 1, smp_processor_id());
-		trace_cpu_idle(1, smp_processor_id());
+		trace_power_start_rcuidle(POWER_CSTATE, 1, smp_processor_id());
+		trace_cpu_idle_rcuidle(1, smp_processor_id());
 		if (this_cpu_has(X86_FEATURE_CLFLUSH_MONITOR))
 			clflush((void *)&current_thread_info()->flags);

@@ -461,8 +461,8 @@ static void mwait_idle(void)
 			__sti_mwait(0, 0);
 		else
 			local_irq_enable();
-		trace_power_end(smp_processor_id());
-		trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
+		trace_power_end_rcuidle(smp_processor_id());
+		trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, smp_processor_id());
 	} else
 		local_irq_enable();
 }

@@ -474,13 +474,13 @@ static void mwait_idle(void)
  */
 static void poll_idle(void)
 {
-	trace_power_start(POWER_CSTATE, 0, smp_processor_id());
-	trace_cpu_idle(0, smp_processor_id());
+	trace_power_start_rcuidle(POWER_CSTATE, 0, smp_processor_id());
+	trace_cpu_idle_rcuidle(0, smp_processor_id());
 	local_irq_enable();
 	while (!need_resched())
 		cpu_relax();
-	trace_power_end(smp_processor_id());
-	trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
+	trace_power_end_rcuidle(smp_processor_id());
+	trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, smp_processor_id());
 }
 
 /*

Signature · 8 lines

1.7.8.3


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Steven Rostedt

author maintainer

2012-02-07 15:11 UTC #4 lore raw

RFC PATCH 3/3 cpuidle/tracing: Denote the tracepoints as being in rcu_idle_exit() section

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

As the tracepoints in the cpuidle code are called when rcu_idle_exit() is in effect, the _rcuidle() version must be used, otherwise the rcu_read_lock()s that protect the tracepoint will not be honored.

Cc Len Brown <[email protected]>

Cc Paul E. McKenney <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 drivers/cpuidle/cpuidle.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

drivers/cpuidle/cpuidle.c +4 -4

index 59f4261..6588f43 100644
@@ -94,13 +94,13 @@ int cpuidle_idle_call(void)
 
 	target_state = &drv->states[next_state];
 
-	trace_power_start(POWER_CSTATE, next_state, dev->cpu);
-	trace_cpu_idle(next_state, dev->cpu);
+	trace_power_start_rcuidle(POWER_CSTATE, next_state, dev->cpu);
+	trace_cpu_idle_rcuidle(next_state, dev->cpu);
 
 	entered_state = target_state->enter(dev, drv, next_state);
 
-	trace_power_end(dev->cpu);
-	trace_cpu_idle(PWR_EVENT_EXIT, dev->cpu);
+	trace_power_end_rcuidle(dev->cpu);
+	trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, dev->cpu);
 
 	if (entered_state >= 0) {
 		/* Update cpuidle counters */

Signature · 1 line

1.7.8.3

Paul E. McKenney

Reviewed-by on v1 3/3

2012-02-07 20:08 UTC #5 lore raw

Re: [PATCH 3/3][RFC] cpuidle/tracing: Denote the tracepoints as being in rcu_idle_exit() section

Quoted 9 lines · Steven Rostedt

From: Steven Rostedt <[email protected]>
As the tracepoints in the cpuidle code are called when rcu_idle_exit() is in effect, the _rcuidle() version must be used, otherwise the rcu_read_lock()s that protect the tracepoint will not be honored.
Cc Len Brown <[email protected]>
Cc Paul E. McKenney <[email protected]>
Signed-off-by Steven Rostedt <[email protected]>

Looks good to me!

Reviewed-by Paul E. McKenney <[email protected]>

Quoted 30 lines

---
 drivers/cpuidle/cpuidle.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

drivers/cpuidle/cpuidle.c +4 -4

index 59f4261..6588f43 100644
@@ -94,13 +94,13 @@ int cpuidle_idle_call(void)
 
 	target_state = &drv->states[next_state];
 
-	trace_power_start(POWER_CSTATE, next_state, dev->cpu);
-	trace_cpu_idle(next_state, dev->cpu);
+	trace_power_start_rcuidle(POWER_CSTATE, next_state, dev->cpu);
+	trace_cpu_idle_rcuidle(next_state, dev->cpu);
 
 	entered_state = target_state->enter(dev, drv, next_state);
 
-	trace_power_end(dev->cpu);
-	trace_cpu_idle(PWR_EVENT_EXIT, dev->cpu);
+	trace_power_end_rcuidle(dev->cpu);
+	trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, dev->cpu);
 
 	if (entered_state >= 0) {
 		/* Update cpuidle counters */

Signature · 1 line

1.7.8.3

Steven Rostedt

author maintainer

2012-02-07 20:49 UTC #6 lore raw

Re: [PATCH 3/3][RFC] cpuidle/tracing: Denote the tracepoints as being in rcu_idle_exit() section

Quoted 7 lines · Paul E. McKenney

Quoted 6 lines · Steven Rostedt
From: Steven Rostedt <[email protected]>
As the tracepoints in the cpuidle code are called when rcu_idle_exit() is in effect, the _rcuidle() version must be used, otherwise the rcu_read_lock()s that protect the tracepoint will not be honored.

Quoted 7 lines

Quoted 3 lines
Cc Len Brown <[email protected]>
Cc Paul E. McKenney <[email protected]>
Signed-off-by Steven Rostedt <[email protected]>
Looks good to me!
Reviewed-by Paul E. McKenney <[email protected]>

Thanks, but does this include patch 2/3 as well?

-- Steve

Paul E. McKenney

2012-02-07 15:39 UTC #7 lore raw

In reply to Steven Rostedt

Quoted 9 lines · Steven Rostedt

This is an RFC patch set for what I plan on pushing out to handle the tracepoints that are called from within rcu_idle_exit() sections.
The first patch is the tracepoint infrastructure,
the second is the changes in x86
the third is the changes in the cpuidle driver.
I see there's use cases in the arm arch, but as that is in flux, I'll let the arm folks deal with those.

Please note that the __rcudle() tracepoints depend on patch #45 from my series (https://lkml.org/lkml/2012/2/3/471), though only for code containing tracepoints that are called both from idle and non-idle. I believe that the tracepoints in this patch series are only invoked from idle, so should be no problem.

Thanx, Paul

Quoted 20 lines

-- Steve
Steven Rostedt (3):
tracing/rcu: Add trace_##name##__rcuidle() static tracepoint for inside rcu_idle_exit() sections x86/tracing: Denote the power and cpuidle tracepoints as _rcuidle() cpuidle/tracing: Denote the tracepoints as being in rcu_idle_exit() section
----
 arch/x86/kernel/process.c  |   24 ++++++++++++------------
 drivers/cpuidle/cpuidle.c  |    8 ++++----
 include/linux/tracepoint.h |   20 ++++++++++++++++----
 3 files changed, 32 insertions(+), 20 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Steven Rostedt

author maintainer

2012-02-07 16:09 UTC #8 lore raw

Quoted 5 lines · Paul E. McKenney

Please note that the __rcudle() tracepoints depend on patch #45 from my series (https://lkml.org/lkml/2012/2/3/471), though only for code containing tracepoints that are called both from idle and non-idle. I believe that the tracepoints in this patch series are only invoked from idle, so should be no problem.

Right, as I tested these without your patch series.

-- Steve

Josh Triplett

Reviewed-by on the v1 series

2012-02-07 20:18 UTC #9 lore raw

In reply to Steven Rostedt

Quoted 6 lines · Steven Rostedt

This is an RFC patch set for what I plan on pushing out to handle the tracepoints that are called from within rcu_idle_exit() sections.
The first patch is the tracepoint infrastructure,
the second is the changes in x86
the third is the changes in the cpuidle driver.

Patches 2 and 3 look sensible to me as well.

Reviewed-by Josh Triplett <[email protected]>