Linux kernel -stable discussions
 help / color / mirror / Atom feed
* Patch "tracing: Fix check for cpu online when event is disabled" has been added to the 3.10-stable tree
@ 2016-03-12  7:04 gregkh
  2016-03-14 17:18 ` Luis Henriques
  0 siblings, 1 reply; 6+ messages in thread
From: gregkh @ 2016-03-12  7:04 UTC (permalink / raw)
  To: rostedt, gregkh, sudeep.holla; +Cc: stable, stable-commits


This is a note to let you know that I've just added the patch titled

    tracing: Fix check for cpu online when event is disabled

to the 3.10-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     tracing-fix-check-for-cpu-online-when-event-is-disabled.patch
and it can be found in the queue-3.10 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.


>From dc17147de328a74bbdee67c1bf37d2f1992de756 Mon Sep 17 00:00:00 2001
From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
Date: Wed, 9 Mar 2016 11:58:41 -0500
Subject: tracing: Fix check for cpu online when event is disabled

From: Steven Rostedt (Red Hat) <rostedt@goodmis.org>

commit dc17147de328a74bbdee67c1bf37d2f1992de756 upstream.

Commit f37755490fe9b ("tracepoints: Do not trace when cpu is offline") added
a check to make sure that tracepoints only get called when the cpu is
online, as it uses rcu_read_lock_sched() for protection.

Commit 3a630178fd5f3 ("tracing: generate RCU warnings even when tracepoints
are disabled") added lockdep checks (including rcu checks) for events that
are not enabled to catch possible RCU issues that would only be triggered if
a trace event was enabled. Commit f37755490fe9b only stopped the warnings
when the trace event was enabled but did not prevent warnings if the trace
event was called when disabled.

To fix this, the cpu online check is moved to where the condition is added
to the trace event. This will place the cpu online check in all places that
it may be used now and in the future.

Fixes: f37755490fe9b ("tracepoints: Do not trace when cpu is offline")
Fixes: 3a630178fd5f3 ("tracing: generate RCU warnings even when tracepoints are disabled")
Reported-by: Sudeep Holla <sudeep.holla@arm.com>
Tested-by: Sudeep Holla <sudeep.holla@arm.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 include/linux/tracepoint.h |   17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -129,9 +129,6 @@ static inline void tracepoint_synchroniz
 		void *it_func;						\
 		void *__data;						\
 									\
-		if (!cpu_online(raw_smp_processor_id()))		\
-			return;						\
-									\
 		if (!(cond))						\
 			return;						\
 		prercu;							\
@@ -265,15 +262,19 @@ static inline void tracepoint_synchroniz
  * "void *__data, proto" as the callback prototype.
  */
 #define DECLARE_TRACE_NOARGS(name)					\
-		__DECLARE_TRACE(name, void, , 1, void *__data, __data)
+	__DECLARE_TRACE(name, void, ,					\
+			cpu_online(raw_smp_processor_id()),		\
+			void *__data, __data)
 
 #define DECLARE_TRACE(name, proto, args)				\
-		__DECLARE_TRACE(name, PARAMS(proto), PARAMS(args), 1,	\
-				PARAMS(void *__data, proto),		\
-				PARAMS(__data, args))
+	__DECLARE_TRACE(name, PARAMS(proto), PARAMS(args),		\
+			cpu_online(raw_smp_processor_id()),		\
+			PARAMS(void *__data, proto),			\
+			PARAMS(__data, args))
 
 #define DECLARE_TRACE_CONDITION(name, proto, args, cond)		\
-	__DECLARE_TRACE(name, PARAMS(proto), PARAMS(args), PARAMS(cond), \
+	__DECLARE_TRACE(name, PARAMS(proto), PARAMS(args),		\
+			cpu_online(raw_smp_processor_id()) && (PARAMS(cond)), \
 			PARAMS(void *__data, proto),			\
 			PARAMS(__data, args))
 


Patches currently in stable-queue which might be from rostedt@goodmis.org are

queue-3.10/tracing-fix-check-for-cpu-online-when-event-is-disabled.patch

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

* Re: Patch "tracing: Fix check for cpu online when event is disabled" has been added to the 3.10-stable tree
  2016-03-12  7:04 Patch "tracing: Fix check for cpu online when event is disabled" has been added to the 3.10-stable tree gregkh
@ 2016-03-14 17:18 ` Luis Henriques
  2016-03-14 17:25   ` Greg KH
  2016-03-14 17:25   ` Steven Rostedt
  0 siblings, 2 replies; 6+ messages in thread
From: Luis Henriques @ 2016-03-14 17:18 UTC (permalink / raw)
  To: gregkh; +Cc: rostedt, sudeep.holla, stable, stable-commits

On Fri, Mar 11, 2016 at 11:04:00PM -0800, Greg Kroah-Hartman wrote:
> 
> This is a note to let you know that I've just added the patch titled
> 
>     tracing: Fix check for cpu online when event is disabled
> 
> to the 3.10-stable tree which can be found at:
>     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> 
> The filename of the patch is:
>      tracing-fix-check-for-cpu-online-when-event-is-disabled.patch
> and it can be found in the queue-3.10 subdirectory.
> 
> If you, or anyone else, feels it should not be added to the stable tree,
> please let <stable@vger.kernel.org> know about it.
>

Please note that this patch was tagged for stable 3.18+.

Cheers,
--
Lu�s


> 
> From dc17147de328a74bbdee67c1bf37d2f1992de756 Mon Sep 17 00:00:00 2001
> From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> Date: Wed, 9 Mar 2016 11:58:41 -0500
> Subject: tracing: Fix check for cpu online when event is disabled
> 
> From: Steven Rostedt (Red Hat) <rostedt@goodmis.org>
> 
> commit dc17147de328a74bbdee67c1bf37d2f1992de756 upstream.
> 
> Commit f37755490fe9b ("tracepoints: Do not trace when cpu is offline") added
> a check to make sure that tracepoints only get called when the cpu is
> online, as it uses rcu_read_lock_sched() for protection.
> 
> Commit 3a630178fd5f3 ("tracing: generate RCU warnings even when tracepoints
> are disabled") added lockdep checks (including rcu checks) for events that
> are not enabled to catch possible RCU issues that would only be triggered if
> a trace event was enabled. Commit f37755490fe9b only stopped the warnings
> when the trace event was enabled but did not prevent warnings if the trace
> event was called when disabled.
> 
> To fix this, the cpu online check is moved to where the condition is added
> to the trace event. This will place the cpu online check in all places that
> it may be used now and in the future.
> 
> Fixes: f37755490fe9b ("tracepoints: Do not trace when cpu is offline")
> Fixes: 3a630178fd5f3 ("tracing: generate RCU warnings even when tracepoints are disabled")
> Reported-by: Sudeep Holla <sudeep.holla@arm.com>
> Tested-by: Sudeep Holla <sudeep.holla@arm.com>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> ---
>  include/linux/tracepoint.h |   17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> --- a/include/linux/tracepoint.h
> +++ b/include/linux/tracepoint.h
> @@ -129,9 +129,6 @@ static inline void tracepoint_synchroniz
>  		void *it_func;						\
>  		void *__data;						\
>  									\
> -		if (!cpu_online(raw_smp_processor_id()))		\
> -			return;						\
> -									\
>  		if (!(cond))						\
>  			return;						\
>  		prercu;							\
> @@ -265,15 +262,19 @@ static inline void tracepoint_synchroniz
>   * "void *__data, proto" as the callback prototype.
>   */
>  #define DECLARE_TRACE_NOARGS(name)					\
> -		__DECLARE_TRACE(name, void, , 1, void *__data, __data)
> +	__DECLARE_TRACE(name, void, ,					\
> +			cpu_online(raw_smp_processor_id()),		\
> +			void *__data, __data)
>  
>  #define DECLARE_TRACE(name, proto, args)				\
> -		__DECLARE_TRACE(name, PARAMS(proto), PARAMS(args), 1,	\
> -				PARAMS(void *__data, proto),		\
> -				PARAMS(__data, args))
> +	__DECLARE_TRACE(name, PARAMS(proto), PARAMS(args),		\
> +			cpu_online(raw_smp_processor_id()),		\
> +			PARAMS(void *__data, proto),			\
> +			PARAMS(__data, args))
>  
>  #define DECLARE_TRACE_CONDITION(name, proto, args, cond)		\
> -	__DECLARE_TRACE(name, PARAMS(proto), PARAMS(args), PARAMS(cond), \
> +	__DECLARE_TRACE(name, PARAMS(proto), PARAMS(args),		\
> +			cpu_online(raw_smp_processor_id()) && (PARAMS(cond)), \
>  			PARAMS(void *__data, proto),			\
>  			PARAMS(__data, args))
>  
> 
> 
> Patches currently in stable-queue which might be from rostedt@goodmis.org are
> 
> queue-3.10/tracing-fix-check-for-cpu-online-when-event-is-disabled.patch
> --
> To unsubscribe from this list: send the line "unsubscribe stable" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: Patch "tracing: Fix check for cpu online when event is disabled" has been added to the 3.10-stable tree
  2016-03-14 17:18 ` Luis Henriques
@ 2016-03-14 17:25   ` Greg KH
  2016-03-14 17:25   ` Steven Rostedt
  1 sibling, 0 replies; 6+ messages in thread
From: Greg KH @ 2016-03-14 17:25 UTC (permalink / raw)
  To: Luis Henriques; +Cc: rostedt, sudeep.holla, stable, stable-commits

On Mon, Mar 14, 2016 at 05:18:52PM +0000, Luis Henriques wrote:
> On Fri, Mar 11, 2016 at 11:04:00PM -0800, Greg Kroah-Hartman wrote:
> > 
> > This is a note to let you know that I've just added the patch titled
> > 
> >     tracing: Fix check for cpu online when event is disabled
> > 
> > to the 3.10-stable tree which can be found at:
> >     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> > 
> > The filename of the patch is:
> >      tracing-fix-check-for-cpu-online-when-event-is-disabled.patch
> > and it can be found in the queue-3.10 subdirectory.
> > 
> > If you, or anyone else, feels it should not be added to the stable tree,
> > please let <stable@vger.kernel.org> know about it.
> >
> 
> Please note that this patch was tagged for stable 3.18+.

Please fix your scripts, and look deeper down the chain... :)


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

* Re: Patch "tracing: Fix check for cpu online when event is disabled" has been added to the 3.10-stable tree
  2016-03-14 17:18 ` Luis Henriques
  2016-03-14 17:25   ` Greg KH
@ 2016-03-14 17:25   ` Steven Rostedt
  2016-03-14 18:14     ` Luis Henriques
  1 sibling, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2016-03-14 17:25 UTC (permalink / raw)
  To: Luis Henriques; +Cc: gregkh, sudeep.holla, stable, stable-commits

On Mon, 14 Mar 2016 17:18:52 +0000
Luis Henriques <luis.henriques@canonical.com> wrote:

> On Fri, Mar 11, 2016 at 11:04:00PM -0800, Greg Kroah-Hartman wrote:
> > 
> > This is a note to let you know that I've just added the patch titled
> > 
> >     tracing: Fix check for cpu online when event is disabled
> > 
> > to the 3.10-stable tree which can be found at:
> >     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> > 
> > The filename of the patch is:
> >      tracing-fix-check-for-cpu-online-when-event-is-disabled.patch
> > and it can be found in the queue-3.10 subdirectory.
> > 
> > If you, or anyone else, feels it should not be added to the stable tree,
> > please let <stable@vger.kernel.org> know about it.
> >  
> 
> Please note that this patch was tagged for stable 3.18+.
> 

Correct. That's because:

> > 
> > Commit f37755490fe9b ("tracepoints: Do not trace when cpu is offline") added
> > a check to make sure that tracepoints only get called when the cpu is
> > online, as it uses rcu_read_lock_sched() for protection.

This was added much earlier and fixes a real bug in various places.

> > 
> > Commit 3a630178fd5f3 ("tracing: generate RCU warnings even when tracepoints
> > are disabled") added lockdep checks (including rcu checks) for events that
> > are not enabled to catch possible RCU issues that would only be triggered if
> > a trace event was enabled. Commit f37755490fe9b only stopped the warnings
> > when the trace event was enabled but did not prevent warnings if the trace
> > event was called when disabled.

The above wasn't added till 3.18, which warns when the trace event is
not enabled. With the backported patch, it now triggers false warnings.
But the false warnings wont exist before 3.18.

It doesn't hurt to backport this patch further, it's just not
necessary, unless 3a630178fd5f3 was backported too. Was it?

-- Steve


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

* Re: Patch "tracing: Fix check for cpu online when event is disabled" has been added to the 3.10-stable tree
  2016-03-14 17:25   ` Steven Rostedt
@ 2016-03-14 18:14     ` Luis Henriques
  2016-03-14 18:39       ` Steven Rostedt
  0 siblings, 1 reply; 6+ messages in thread
From: Luis Henriques @ 2016-03-14 18:14 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: gregkh, sudeep.holla, stable, stable-commits

On Mon, Mar 14, 2016 at 01:25:27PM -0400, Steven Rostedt wrote:
> On Mon, 14 Mar 2016 17:18:52 +0000
> Luis Henriques <luis.henriques@canonical.com> wrote:
> 
> > On Fri, Mar 11, 2016 at 11:04:00PM -0800, Greg Kroah-Hartman wrote:
> > > 
> > > This is a note to let you know that I've just added the patch titled
> > > 
> > >     tracing: Fix check for cpu online when event is disabled
> > > 
> > > to the 3.10-stable tree which can be found at:
> > >     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> > > 
> > > The filename of the patch is:
> > >      tracing-fix-check-for-cpu-online-when-event-is-disabled.patch
> > > and it can be found in the queue-3.10 subdirectory.
> > > 
> > > If you, or anyone else, feels it should not be added to the stable tree,
> > > please let <stable@vger.kernel.org> know about it.
> > >  
> > 
> > Please note that this patch was tagged for stable 3.18+.
> > 
> 
> Correct. That's because:
> 
> > > 
> > > Commit f37755490fe9b ("tracepoints: Do not trace when cpu is offline") added
> > > a check to make sure that tracepoints only get called when the cpu is
> > > online, as it uses rcu_read_lock_sched() for protection.
> 
> This was added much earlier and fixes a real bug in various places.
> 
> > > 
> > > Commit 3a630178fd5f3 ("tracing: generate RCU warnings even when tracepoints
> > > are disabled") added lockdep checks (including rcu checks) for events that
> > > are not enabled to catch possible RCU issues that would only be triggered if
> > > a trace event was enabled. Commit f37755490fe9b only stopped the warnings
> > > when the trace event was enabled but did not prevent warnings if the trace
> > > event was called when disabled.
> 
> The above wasn't added till 3.18, which warns when the trace event is
> not enabled. With the backported patch, it now triggers false warnings.
> But the false warnings wont exist before 3.18.
> 
> It doesn't hurt to backport this patch further, it's just not
> necessary, unless 3a630178fd5f3 was backported too. Was it?
> 
> -- Steve
> 

Thanks for clarifying, Steve.  I don't think commit 3a630178fd5f3 has
been backport to stable kernels, so if you're saying it doesn't hurt
to backport dc17147de328 I guess that's up to the stable maintainers
to include it or not.  Thanks!

Cheers,
--
Lu�s

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

* Re: Patch "tracing: Fix check for cpu online when event is disabled" has been added to the 3.10-stable tree
  2016-03-14 18:14     ` Luis Henriques
@ 2016-03-14 18:39       ` Steven Rostedt
  0 siblings, 0 replies; 6+ messages in thread
From: Steven Rostedt @ 2016-03-14 18:39 UTC (permalink / raw)
  To: Luis Henriques; +Cc: gregkh, sudeep.holla, stable, stable-commits

On Mon, 14 Mar 2016 18:14:18 +0000
Luis Henriques <luis.henriques@canonical.com> wrote:


> Thanks for clarifying, Steve.  I don't think commit 3a630178fd5f3 has
> been backport to stable kernels, so if you're saying it doesn't hurt
> to backport dc17147de328 I guess that's up to the stable maintainers
> to include it or not.  Thanks!

Right, it doesn't hurt to backport the patch as it's just a more robust
way to do the same change as it was fixing. I only added the 3.18 tag
in case the change failed to apply to stable trees earlier than 3.18.
Then it wouldn't need to be backported unless 3a630178fd5f3 was.

-- Steve

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

end of thread, other threads:[~2016-03-14 18:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-12  7:04 Patch "tracing: Fix check for cpu online when event is disabled" has been added to the 3.10-stable tree gregkh
2016-03-14 17:18 ` Luis Henriques
2016-03-14 17:25   ` Greg KH
2016-03-14 17:25   ` Steven Rostedt
2016-03-14 18:14     ` Luis Henriques
2016-03-14 18:39       ` Steven Rostedt

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