public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3][RFC] tracing/rcu: Add _rcuidle() tracepoint to handle rcu_idle_exit() tracepoints
@ 2012-02-07 14:56 Steven Rostedt
  2012-02-07 14:56 ` [PATCH 1/3][RFC] tracing/rcu: Add trace_##name##__rcuidle() static tracepoint for inside rcu_idle_exit() sections Steven Rostedt
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Steven Rostedt @ 2012-02-07 14:56 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ingo Molnar, Andrew Morton, Paul E. McKenney, Rob Herring,
	Nicolas Pitre, Kevin Hilman, Josh Triplett, Mathieu Desnoyers

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(-)


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3][RFC] tracing/rcu: Add trace_##name##__rcuidle() static tracepoint for inside rcu_idle_exit() sections
  2012-02-07 14:56 [PATCH 0/3][RFC] tracing/rcu: Add _rcuidle() tracepoint to handle rcu_idle_exit() tracepoints Steven Rostedt
@ 2012-02-07 14:56 ` Steven Rostedt
  2012-02-07 14:56 ` [PATCH 2/3][RFC] x86/tracing: Denote the power and cpuidle tracepoints as _rcuidle() Steven Rostedt
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Steven Rostedt @ 2012-02-07 14:56 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ingo Molnar, Andrew Morton, Paul E. McKenney, Rob Herring,
	Nicolas Pitre, Kevin Hilman, Josh Triplett, Mathieu Desnoyers

[-- Attachment #1: 0001-tracing-rcu-Add-trace_-name-__rcuidle-static-tracepo.patch --]
[-- Type: text/plain, Size: 4119 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

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/1328563113.2200.39.camel@gandalf.stny.rr.com>

Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Reviewed-by: Josh Triplett <josh@joshtriplett.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/linux/tracepoint.h |   20 ++++++++++++++++----
 1 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index df0a779..00d073a 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -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						\
-- 
1.7.8.3



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/3][RFC] x86/tracing: Denote the power and cpuidle tracepoints as _rcuidle()
  2012-02-07 14:56 [PATCH 0/3][RFC] tracing/rcu: Add _rcuidle() tracepoint to handle rcu_idle_exit() tracepoints Steven Rostedt
  2012-02-07 14:56 ` [PATCH 1/3][RFC] tracing/rcu: Add trace_##name##__rcuidle() static tracepoint for inside rcu_idle_exit() sections Steven Rostedt
@ 2012-02-07 14:56 ` Steven Rostedt
  2012-02-07 22:51   ` Paul E. McKenney
  2012-02-07 14:56 ` [PATCH 3/3][RFC] cpuidle/tracing: Denote the tracepoints as being in rcu_idle_exit() section Steven Rostedt
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Steven Rostedt @ 2012-02-07 14:56 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ingo Molnar, Andrew Morton, Paul E. McKenney, Rob Herring,
	Nicolas Pitre, Kevin Hilman, Josh Triplett, Mathieu Desnoyers

[-- Attachment #1: 0002-x86-tracing-Denote-the-power-and-cpuidle-tracepoints.patch --]
[-- Type: text/plain, Size: 2779 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

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 <paulmck@linux.vnet.ibm.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 arch/x86/kernel/process.c |   24 ++++++++++++------------
 1 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 15763af..44eefde 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -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());
 }
 
 /*
-- 
1.7.8.3



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/3][RFC] cpuidle/tracing: Denote the tracepoints as being in rcu_idle_exit() section
  2012-02-07 14:56 [PATCH 0/3][RFC] tracing/rcu: Add _rcuidle() tracepoint to handle rcu_idle_exit() tracepoints Steven Rostedt
  2012-02-07 14:56 ` [PATCH 1/3][RFC] tracing/rcu: Add trace_##name##__rcuidle() static tracepoint for inside rcu_idle_exit() sections Steven Rostedt
  2012-02-07 14:56 ` [PATCH 2/3][RFC] x86/tracing: Denote the power and cpuidle tracepoints as _rcuidle() Steven Rostedt
@ 2012-02-07 14:56 ` Steven Rostedt
  2012-02-07 15:40   ` Paul E. McKenney
  2012-02-07 15:37 ` [PATCH 0/3][RFC] tracing/rcu: Add _rcuidle() tracepoint to handle rcu_idle_exit() tracepoints Paul E. McKenney
  2012-02-07 20:17 ` Josh Triplett
  4 siblings, 1 reply; 12+ messages in thread
From: Steven Rostedt @ 2012-02-07 14:56 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ingo Molnar, Andrew Morton, Paul E. McKenney, Rob Herring,
	Nicolas Pitre, Kevin Hilman, Josh Triplett, Mathieu Desnoyers,
	Len Brown

[-- Attachment #1: 0003-cpuidle-tracing-Denote-the-tracepoints-as-being-in-r.patch --]
[-- Type: text/plain, Size: 1253 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

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 <len.brown@intel.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 drivers/cpuidle/cpuidle.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index 59f4261..6588f43 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -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 */
-- 
1.7.8.3



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3][RFC] tracing/rcu: Add _rcuidle() tracepoint to handle rcu_idle_exit() tracepoints
  2012-02-07 14:56 [PATCH 0/3][RFC] tracing/rcu: Add _rcuidle() tracepoint to handle rcu_idle_exit() tracepoints Steven Rostedt
                   ` (2 preceding siblings ...)
  2012-02-07 14:56 ` [PATCH 3/3][RFC] cpuidle/tracing: Denote the tracepoints as being in rcu_idle_exit() section Steven Rostedt
@ 2012-02-07 15:37 ` Paul E. McKenney
  2012-02-07 16:09   ` Steven Rostedt
  2012-02-07 20:17 ` Josh Triplett
  4 siblings, 1 reply; 12+ messages in thread
From: Paul E. McKenney @ 2012-02-07 15:37 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, Ingo Molnar, Andrew Morton, Rob Herring,
	Nicolas Pitre, Kevin Hilman, Josh Triplett, Mathieu Desnoyers

On Tue, Feb 07, 2012 at 09:56:53AM -0500, Steven Rostedt wrote:
> 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

> -- 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3][RFC] cpuidle/tracing: Denote the tracepoints as being in rcu_idle_exit() section
  2012-02-07 14:56 ` [PATCH 3/3][RFC] cpuidle/tracing: Denote the tracepoints as being in rcu_idle_exit() section Steven Rostedt
@ 2012-02-07 15:40   ` Paul E. McKenney
  2012-02-07 20:49     ` Steven Rostedt
  0 siblings, 1 reply; 12+ messages in thread
From: Paul E. McKenney @ 2012-02-07 15:40 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, Ingo Molnar, Andrew Morton, Rob Herring,
	Nicolas Pitre, Kevin Hilman, Josh Triplett, Mathieu Desnoyers,
	Len Brown

On Tue, Feb 07, 2012 at 09:56:56AM -0500, Steven Rostedt wrote:
> From: Steven Rostedt <srostedt@redhat.com>
> 
> 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 <len.brown@intel.com>
> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

Looks good to me!

Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

> ---
>  drivers/cpuidle/cpuidle.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 59f4261..6588f43 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -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 */
> -- 
> 1.7.8.3
> 
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3][RFC] tracing/rcu: Add _rcuidle() tracepoint to handle rcu_idle_exit() tracepoints
  2012-02-07 15:37 ` [PATCH 0/3][RFC] tracing/rcu: Add _rcuidle() tracepoint to handle rcu_idle_exit() tracepoints Paul E. McKenney
@ 2012-02-07 16:09   ` Steven Rostedt
  0 siblings, 0 replies; 12+ messages in thread
From: Steven Rostedt @ 2012-02-07 16:09 UTC (permalink / raw)
  To: paulmck
  Cc: linux-kernel, Ingo Molnar, Andrew Morton, Rob Herring,
	Nicolas Pitre, Kevin Hilman, Josh Triplett, Mathieu Desnoyers

On Tue, 2012-02-07 at 07:37 -0800, Paul E. McKenney wrote:

> 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



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3][RFC] tracing/rcu: Add _rcuidle() tracepoint to handle rcu_idle_exit() tracepoints
  2012-02-07 14:56 [PATCH 0/3][RFC] tracing/rcu: Add _rcuidle() tracepoint to handle rcu_idle_exit() tracepoints Steven Rostedt
                   ` (3 preceding siblings ...)
  2012-02-07 15:37 ` [PATCH 0/3][RFC] tracing/rcu: Add _rcuidle() tracepoint to handle rcu_idle_exit() tracepoints Paul E. McKenney
@ 2012-02-07 20:17 ` Josh Triplett
  4 siblings, 0 replies; 12+ messages in thread
From: Josh Triplett @ 2012-02-07 20:17 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, Ingo Molnar, Andrew Morton, Paul E. McKenney,
	Rob Herring, Nicolas Pitre, Kevin Hilman, Mathieu Desnoyers

On Tue, Feb 07, 2012 at 09:56:53AM -0500, Steven Rostedt wrote:
> 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 <josh@joshtriplett.org>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3][RFC] cpuidle/tracing: Denote the tracepoints as being in rcu_idle_exit() section
  2012-02-07 15:40   ` Paul E. McKenney
@ 2012-02-07 20:49     ` Steven Rostedt
  0 siblings, 0 replies; 12+ messages in thread
From: Steven Rostedt @ 2012-02-07 20:49 UTC (permalink / raw)
  To: paulmck
  Cc: linux-kernel, Ingo Molnar, Andrew Morton, Rob Herring,
	Nicolas Pitre, Kevin Hilman, Josh Triplett, Mathieu Desnoyers,
	Len Brown

On Tue, 2012-02-07 at 07:40 -0800, Paul E. McKenney wrote:
> On Tue, Feb 07, 2012 at 09:56:56AM -0500, Steven Rostedt wrote:
> > From: Steven Rostedt <srostedt@redhat.com>
> > 
> > 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 <len.brown@intel.com>
> > Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> 
> Looks good to me!
> 
> Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

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

-- Steve



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3][RFC] x86/tracing: Denote the power and cpuidle tracepoints as _rcuidle()
  2012-02-07 14:56 ` [PATCH 2/3][RFC] x86/tracing: Denote the power and cpuidle tracepoints as _rcuidle() Steven Rostedt
@ 2012-02-07 22:51   ` Paul E. McKenney
  0 siblings, 0 replies; 12+ messages in thread
From: Paul E. McKenney @ 2012-02-07 22:51 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, Ingo Molnar, Andrew Morton, Rob Herring,
	Nicolas Pitre, Kevin Hilman, Josh Triplett, Mathieu Desnoyers

On Tue, Feb 07, 2012 at 09:56:55AM -0500, Steven Rostedt wrote:
> From: Steven Rostedt <srostedt@redhat.com>
> 
> 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 <paulmck@linux.vnet.ibm.com>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

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

Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

> ---
>  arch/x86/kernel/process.c |   24 ++++++++++++------------
>  1 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
> index 15763af..44eefde 100644
> --- a/arch/x86/kernel/process.c
> +++ b/arch/x86/kernel/process.c
> @@ -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());
>  }
> 
>  /*
> -- 
> 1.7.8.3
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3][RFC] tracing/rcu: Add _rcuidle() tracepoint to handle rcu_idle_exit() tracepoints
@ 2012-02-14 18:09 Witold Baryluk
  2012-02-15 19:16 ` Steven Rostedt
  0 siblings, 1 reply; 12+ messages in thread
From: Witold Baryluk @ 2012-02-14 18:09 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Paul E. McKenney, Rob Herring, Nicolas Pitre, Kevin Hilman,
	linux-kernel, Ingo Molnar, Andrew Morton

Hi,

I have some problem with cpuidle tracing:

https://bugzilla.kernel.org/show_bug.cgi?id=42742

so I applied 3 patches from https://lkml.org/lkml/headers/2012/2/7/231

> 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

All 3 patches applied cleanly but for unknown reasons to me compilation fails:

CC      arch/x86/kernel/process.o
  arch/x86/kernel/process.c: In function ‘default_idle’:
  arch/x86/kernel/process.c:380:3: error: implicit declaration of function ‘trace_power_start_rcuidle’ [-Werror=implicit-function-declaration]
  arch/x86/kernel/process.c:394:3: error: implicit declaration of function ‘trace_power_end_rcuidle’ [-Werror=implicit-function-declaration]
  cc1: some warnings being treated as errors

I think this is because

        static inline void trace_##name##_rcuidle(proto)                \

should have only 1 argument passed.

but more are passed

        trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, smp_processor_id());
        trace_power_start_rcuidle(POWER_CSTATE, next_state, dev->cpu);

However this explanation fails because

        trace_power_end_rcuidle(smp_processor_id());

have single argument, and because without patches
arch/x86/kernel/process.c compiles without a problem. and there is
nothing in my 'git diff' which should justify such change.

It is mainline Linus kernel with just this 3 patches added.

Any idea?

gcc version 4.6.2 (Debian 4.6.2-14) 
Debian GNU/Linux, wheezy i386. (IA-32)
Intel Pentium M processor.

-- 
Witold Baryluk

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3][RFC] tracing/rcu: Add _rcuidle() tracepoint to handle rcu_idle_exit() tracepoints
  2012-02-14 18:09 Witold Baryluk
@ 2012-02-15 19:16 ` Steven Rostedt
  0 siblings, 0 replies; 12+ messages in thread
From: Steven Rostedt @ 2012-02-15 19:16 UTC (permalink / raw)
  To: Witold Baryluk
  Cc: Paul E. McKenney, Rob Herring, Nicolas Pitre, Kevin Hilman,
	linux-kernel, Ingo Molnar, Andrew Morton

On Tue, 2012-02-14 at 19:09 +0100, Witold Baryluk wrote:
> Hi,
> 
> I have some problem with cpuidle tracing:
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=42742
> 
> so I applied 3 patches from https://lkml.org/lkml/headers/2012/2/7/231
> 
> > 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
> 
> All 3 patches applied cleanly but for unknown reasons to me compilation fails:

Yeah, my testing hit this issue and I fixed it up. I pushed out my
patches to Ingo that have passed all my tests. You can get them here:

https://lkml.org/lkml/2012/2/13/528

You only need to apply patches 3,4 and 5.

-- Steve

> 
> CC      arch/x86/kernel/process.o
>   arch/x86/kernel/process.c: In function ‘default_idle’:
>   arch/x86/kernel/process.c:380:3: error: implicit declaration of function ‘trace_power_start_rcuidle’ [-Werror=implicit-function-declaration]
>   arch/x86/kernel/process.c:394:3: error: implicit declaration of function ‘trace_power_end_rcuidle’ [-Werror=implicit-function-declaration]
>   cc1: some warnings being treated as errors
> 



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2012-02-15 19:16 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-07 14:56 [PATCH 0/3][RFC] tracing/rcu: Add _rcuidle() tracepoint to handle rcu_idle_exit() tracepoints Steven Rostedt
2012-02-07 14:56 ` [PATCH 1/3][RFC] tracing/rcu: Add trace_##name##__rcuidle() static tracepoint for inside rcu_idle_exit() sections Steven Rostedt
2012-02-07 14:56 ` [PATCH 2/3][RFC] x86/tracing: Denote the power and cpuidle tracepoints as _rcuidle() Steven Rostedt
2012-02-07 22:51   ` Paul E. McKenney
2012-02-07 14:56 ` [PATCH 3/3][RFC] cpuidle/tracing: Denote the tracepoints as being in rcu_idle_exit() section Steven Rostedt
2012-02-07 15:40   ` Paul E. McKenney
2012-02-07 20:49     ` Steven Rostedt
2012-02-07 15:37 ` [PATCH 0/3][RFC] tracing/rcu: Add _rcuidle() tracepoint to handle rcu_idle_exit() tracepoints Paul E. McKenney
2012-02-07 16:09   ` Steven Rostedt
2012-02-07 20:17 ` Josh Triplett
  -- strict thread matches above, loose matches on Subject: below --
2012-02-14 18:09 Witold Baryluk
2012-02-15 19:16 ` Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox