Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1618823 > unrolled thread
| Started by | Steven Rostedt <rostedt@goodmis.org> |
|---|---|
| First post | 2017-04-07 16:10 +0200 |
| Last post | 2017-04-07 19:50 +0200 |
| Articles | 8 on this page of 28 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH 0/5 v2] tracing: Add usecase of synchronize_rcu_tasks() and stack_tracer_disable() Steven Rostedt <rostedt@goodmis.org> - 2017-04-07 16:10 +0200
[PATCH 1/5 v2] ftrace: Add use of synchronize_rcu_tasks() with dynamic trampolines Steven Rostedt <rostedt@goodmis.org> - 2017-04-07 16:10 +0200
[PATCH 3/5 v2] tracing: Add stack_tracer_disable/enable() functions Steven Rostedt <rostedt@goodmis.org> - 2017-04-07 16:10 +0200
[PATCH 3/5 v2.1] tracing: Add stack_tracer_disable/enable() functions Steven Rostedt <rostedt@goodmis.org> - 2017-04-07 16:30 +0200
Re: [PATCH 3/5 v2] tracing: Add stack_tracer_disable/enable() functions Steven Rostedt <rostedt@goodmis.org> - 2017-04-07 16:30 +0200
Re: [PATCH 0/5 v2] tracing: Add usecase of synchronize_rcu_tasks() and stack_tracer_disable() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-04-07 16:50 +0200
Re: [PATCH 0/5 v2] tracing: Add usecase of synchronize_rcu_tasks() and stack_tracer_disable() Steven Rostedt <rostedt@goodmis.org> - 2017-04-07 17:10 +0200
Re: [PATCH 0/5 v2] tracing: Add usecase of synchronize_rcu_tasks() and stack_tracer_disable() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-04-07 17:20 +0200
Re: [PATCH 0/5 v2] tracing: Add usecase of synchronize_rcu_tasks() and stack_tracer_disable() Steven Rostedt <rostedt@goodmis.org> - 2017-04-07 17:30 +0200
[PATCH 6/5]rcu/tracing: Add rcu_disabled to denote when rcu_irq_enter() will not work Steven Rostedt <rostedt@goodmis.org> - 2017-04-07 18:40 +0200
Re: [PATCH 6/5]rcu/tracing: Add rcu_disabled to denote when rcu_irq_enter() will not work "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-04-07 18:50 +0200
Re: [PATCH 6/5]rcu/tracing: Add rcu_disabled to denote when rcu_irq_enter() will not work Steven Rostedt <rostedt@goodmis.org> - 2017-04-07 18:50 +0200
Re: [PATCH 6/5]rcu/tracing: Add rcu_disabled to denote when rcu_irq_enter() will not work "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-04-07 19:00 +0200
[PATCH 6/5 v2] rcu/tracing: Add rcu_disabled to denote when rcu_irq_enter() will not work Steven Rostedt <rostedt@goodmis.org> - 2017-04-07 19:10 +0200
Re: [PATCH 6/5 v2] rcu/tracing: Add rcu_disabled to denote when rcu_irq_enter() will not work "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-04-07 19:20 +0200
[PATCH 7/5] tracing: Make sure rcu_irq_enter() can work for trace_*_rcuidle() trace events Steven Rostedt <rostedt@goodmis.org> - 2017-04-07 19:10 +0200
Re: [PATCH 7/5] tracing: Make sure rcu_irq_enter() can work for trace_*_rcuidle() trace events "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-04-07 19:20 +0200
Re: [PATCH 7/5] tracing: Make sure rcu_irq_enter() can work for trace_*_rcuidle() trace events Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2017-04-07 19:20 +0200
Re: [PATCH 7/5] tracing: Make sure rcu_irq_enter() can work for trace_*_rcuidle() trace events Steven Rostedt <rostedt@goodmis.org> - 2017-04-07 19:30 +0200
Re: [PATCH 7/5] tracing: Make sure rcu_irq_enter() can work for trace_*_rcuidle() trace events Steven Rostedt <rostedt@goodmis.org> - 2017-04-07 19:40 +0200
Re: [PATCH 7/5] tracing: Make sure rcu_irq_enter() can work for trace_*_rcuidle() trace events Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2017-04-07 19:50 +0200
Re: [PATCH 7/5] tracing: Make sure rcu_irq_enter() can work for trace_*_rcuidle() trace events Steven Rostedt <rostedt@goodmis.org> - 2017-04-07 20:00 +0200
[PATCH 7/5 v3] tracing: Make sure rcu_irq_enter() can work for trace_*_rcuidle() trace events Steven Rostedt <rostedt@goodmis.org> - 2017-04-07 20:20 +0200
Re: [PATCH 7/5 v3] tracing: Make sure rcu_irq_enter() can work for trace_*_rcuidle() trace events Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2017-04-07 20:20 +0200
Re: [PATCH 7/5 v4] tracing: Make sure rcu_irq_enter() can work for trace_*_rcuidle() trace events Steven Rostedt <rostedt@goodmis.org> - 2017-04-07 21:50 +0200
[PATCH 7/5 v4] tracing: Make sure rcu_irq_enter() can work for trace_*_rcuidle() trace events Steven Rostedt <rostedt@goodmis.org> - 2017-04-07 21:50 +0200
Re: [PATCH 7/5] tracing: Make sure rcu_irq_enter() can work for trace_*_rcuidle() trace events Steven Rostedt <rostedt@goodmis.org> - 2017-04-07 19:30 +0200
[PATCH 7/5 v2] tracing: Make sure rcu_irq_enter() can work for trace_*_rcuidle() trace events Steven Rostedt <rostedt@goodmis.org> - 2017-04-07 19:50 +0200
Page 2 of 2 — ← Prev page 1 [2]
| From | Mathieu Desnoyers <mathieu.desnoyers@efficios.com> |
|---|---|
| Date | 2017-04-07 19:50 +0200 |
| Subject | Re: [PATCH 7/5] tracing: Make sure rcu_irq_enter() can work for trace_*_rcuidle() trace events |
| Message-ID | <ttGbg-21h-19@gated-at.bofh.it> |
| In reply to | #1618987 |
----- On Apr 7, 2017, at 1:26 PM, rostedt rostedt@goodmis.org wrote:
> On Fri, 7 Apr 2017 17:19:05 +0000 (UTC)
> Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
>
[...]
>> > ---
>> > include/linux/tracepoint.h | 2 ++
>> > 1 file changed, 2 insertions(+)
>> >
>> > diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
>> > index f72fcfe..8baef96 100644
>> > --- a/include/linux/tracepoint.h
>> > +++ b/include/linux/tracepoint.h
>> > @@ -159,6 +159,8 @@ extern void syscall_unregfunc(void);
>> > TP_PROTO(data_proto), \
>> > TP_ARGS(data_args), \
>> > TP_CONDITION(cond), \
>> > + if (WARN_ON_ONCE(rcu_irq_enter_disabled())) \
>> > + return; \
>>
>> I must admit that it's a bit odd to have:
>>
>> if (WARN_ON_ONCE(rcu_irq_enter_disabled()))
>> return;
>> rcu_irq_enter_irqson()
>
> Welcome to MACRO MAGIC!
>
>>
>> as one argument to the __DO_TRACE() macro. To me it's a bit unexpected
>> coding-style wise. Am I the only one not comfortable with the proposed
>> syntax ?
>
> The entire TRACE_EVENT()/__DO_TRACE() is special.
>
> I thought about add yet another parameter, but as it doesn't change
> much, I figured this was good enough. We could beak it up if you like:
>
> #define RCU_IRQ_ENTER_CHECK \
> if (WARN_ON_ONCE(rcu_irq_enter_disabled()) \
> return; \
> rcu_irq_enter_irqson();
>
> [..]
> __DO_TRACE(&__tracepoint_##name, \
> TP_PROTO(data_proto), \
> TP_ARGS(data_args), \
> TP_CONDITION(cond), \
> PARAMS(RCU_IRQ_ENTER_CHECK), \
> rcu_irq_exit_irqson()); \
>
>
> Would that make you feel more comfortable?
No, it's almost worse and adds still adds a return that apply within __DO_TRACE(),
but which is passed as an argument (code as macro argument), which I find really
unsettling.
I would prefer to add a new argument to __DO_TRACE, which we can call
"checkrcu", e.g.:
#define __DO_TRACE(tp, proto, args, cond, checkrcu, prercu, postrcu) \
do { \
struct tracepoint_func *it_func_ptr; \
void *it_func; \
void *__data; \
\
if (!((cond) && (checkrcu))) \
return; \
prercu; \
rcu_read_lock_sched_notrace(); \
it_func_ptr = rcu_dereference_sched((tp)->funcs); \
if (it_func_ptr) { \
do { \
it_func = (it_func_ptr)->func; \
__data = (it_func_ptr)->data; \
((void(*)(proto))(it_func))(args); \
} while ((++it_func_ptr)->func); \
} \
rcu_read_unlock_sched_notrace(); \
postrcu; \
} while (0)
And use it like this:
#define __DECLARE_TRACE_RCU(name, proto, args, cond, data_proto, data_args) \
static inline void trace_##name##_rcuidle(proto) \
{ \
if (static_key_false(&__tracepoint_##name.key)) \
__DO_TRACE(&__tracepoint_##name, \
TP_PROTO(data_proto), \
TP_ARGS(data_args), \
TP_CONDITION(cond), \
!WARN_ON_ONCE(rcu_irq_enter_disabled()),\
rcu_irq_enter_irqson(), \
rcu_irq_exit_irqson()); \
}
This way we only pass evaluated expression (not code with "return" that
changes the flow) as arguments to __DO_TRACE, which makes it behave more
like a "sub-function", which is what we usually expect.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
[toc] | [prev] | [next] | [standalone]
| From | Steven Rostedt <rostedt@goodmis.org> |
|---|---|
| Date | 2017-04-07 20:00 +0200 |
| Subject | Re: [PATCH 7/5] tracing: Make sure rcu_irq_enter() can work for trace_*_rcuidle() trace events |
| Message-ID | <ttGkW-28n-13@gated-at.bofh.it> |
| In reply to | #1619005 |
On Fri, 7 Apr 2017 17:49:17 +0000 (UTC)
Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
> > Welcome to MACRO MAGIC!
Somebody is not wizardly happy.
> >
> >>
> >> as one argument to the __DO_TRACE() macro. To me it's a bit unexpected
> >> coding-style wise. Am I the only one not comfortable with the proposed
> >> syntax ?
> >
> > The entire TRACE_EVENT()/__DO_TRACE() is special.
> >
> > I thought about add yet another parameter, but as it doesn't change
> > much, I figured this was good enough. We could beak it up if you like:
> >
> > #define RCU_IRQ_ENTER_CHECK \
> > if (WARN_ON_ONCE(rcu_irq_enter_disabled()) \
> > return; \
> > rcu_irq_enter_irqson();
> >
> > [..]
> > __DO_TRACE(&__tracepoint_##name, \
> > TP_PROTO(data_proto), \
> > TP_ARGS(data_args), \
> > TP_CONDITION(cond), \
> > PARAMS(RCU_IRQ_ENTER_CHECK), \
> > rcu_irq_exit_irqson()); \
> >
> >
> > Would that make you feel more comfortable?
>
> No, it's almost worse and adds still adds a return that apply within __DO_TRACE(),
> but which is passed as an argument (code as macro argument), which I find really
> unsettling.
/me finds it strangely enjoyable to make Mathieu unsettled.
>
> I would prefer to add a new argument to __DO_TRACE, which we can call
> "checkrcu", e.g.:
>
> #define __DO_TRACE(tp, proto, args, cond, checkrcu, prercu, postrcu) \
Grumble. I was trying to avoid making the patch more intrusive. But I
do understand your concern.
> do { \
> struct tracepoint_func *it_func_ptr; \
> void *it_func; \
> void *__data; \
> \
> if (!((cond) && (checkrcu))) \
> return; \
> prercu; \
> rcu_read_lock_sched_notrace(); \
> it_func_ptr = rcu_dereference_sched((tp)->funcs); \
> if (it_func_ptr) { \
> do { \
> it_func = (it_func_ptr)->func; \
> __data = (it_func_ptr)->data; \
> ((void(*)(proto))(it_func))(args); \
> } while ((++it_func_ptr)->func); \
> } \
> rcu_read_unlock_sched_notrace(); \
> postrcu; \
> } while (0)
>
> And use it like this:
>
> #define __DECLARE_TRACE_RCU(name, proto, args, cond, data_proto, data_args) \
> static inline void trace_##name##_rcuidle(proto) \
> { \
> if (static_key_false(&__tracepoint_##name.key)) \
> __DO_TRACE(&__tracepoint_##name, \
> TP_PROTO(data_proto), \
> TP_ARGS(data_args), \
> TP_CONDITION(cond), \
> !WARN_ON_ONCE(rcu_irq_enter_disabled()),\
> rcu_irq_enter_irqson(), \
> rcu_irq_exit_irqson()); \
> }
>
> This way we only pass evaluated expression (not code with "return" that
> changes the flow) as arguments to __DO_TRACE, which makes it behave more
> like a "sub-function", which is what we usually expect.
I understand what you are getting at, and I will concede your point.
OK, I'll do it your way, but I still think you take all the fun out of
it. ;-)
-- Steve
[toc] | [prev] | [next] | [standalone]
| From | Steven Rostedt <rostedt@goodmis.org> |
|---|---|
| Date | 2017-04-07 20:20 +0200 |
| Subject | [PATCH 7/5 v3] tracing: Make sure rcu_irq_enter() can work for trace_*_rcuidle() trace events |
| Message-ID | <ttGEh-2A6-1@gated-at.bofh.it> |
| In reply to | #1619005 |
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Stack tracing discovered that there's a small location inside the RCU
infrastructure where calling rcu_irq_enter() does not work. As trace events
use rcu_irq_enter() it must make sure that it is functionable. A check
against rcu_irq_enter_disabled() is added with a WARN_ON_ONCE() as no trace
event should ever be used in that part of RCU. If the warning is triggered,
then the trace event is ignored.
Link: http://lkml.kernel.org/r/20170405093207.404f8deb@gandalf.local.home
Cc: Mathieu (making code boring) Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
Mathieu,
There! Are you now satisfied?
include/linux/tracepoint.h | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index f72fcfe..7f98d50 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -128,7 +128,7 @@ extern void syscall_unregfunc(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, prercu, postrcu) \
+#define __DO_TRACE(tp, proto, args, cond, rcucheck, prercu, postrcu) \
do { \
struct tracepoint_func *it_func_ptr; \
void *it_func; \
@@ -136,6 +136,8 @@ extern void syscall_unregfunc(void);
\
if (!(cond)) \
return; \
+ if (rcucheck) \
+ return; \
prercu; \
rcu_read_lock_sched_notrace(); \
it_func_ptr = rcu_dereference_sched((tp)->funcs); \
@@ -151,7 +153,7 @@ extern void syscall_unregfunc(void);
} while (0)
#ifndef MODULE
-#define __DECLARE_TRACE_RCU(name, proto, args, cond, data_proto, data_args) \
+#define __DECLARE_TRACE_RCU(name, proto, args, cond, data_proto, data_args) \
static inline void trace_##name##_rcuidle(proto) \
{ \
if (static_key_false(&__tracepoint_##name.key)) \
@@ -159,6 +161,7 @@ extern void syscall_unregfunc(void);
TP_PROTO(data_proto), \
TP_ARGS(data_args), \
TP_CONDITION(cond), \
+ WARN_ON_ONCE(rcu_irq_enter_disabled()),\
rcu_irq_enter_irqson(), \
rcu_irq_exit_irqson()); \
}
@@ -186,7 +189,7 @@ extern void syscall_unregfunc(void);
__DO_TRACE(&__tracepoint_##name, \
TP_PROTO(data_proto), \
TP_ARGS(data_args), \
- TP_CONDITION(cond),,); \
+ TP_CONDITION(cond),0,,); \
if (IS_ENABLED(CONFIG_LOCKDEP) && (cond)) { \
rcu_read_lock_sched_notrace(); \
rcu_dereference_sched(__tracepoint_##name.funcs);\
--
2.9.3
[toc] | [prev] | [next] | [standalone]
| From | Mathieu Desnoyers <mathieu.desnoyers@efficios.com> |
|---|---|
| Date | 2017-04-07 20:20 +0200 |
| Subject | Re: [PATCH 7/5 v3] tracing: Make sure rcu_irq_enter() can work for trace_*_rcuidle() trace events |
| Message-ID | <ttGEi-2A6-5@gated-at.bofh.it> |
| In reply to | #1619018 |
----- On Apr 7, 2017, at 2:10 PM, rostedt rostedt@goodmis.org wrote:
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
>
> Stack tracing discovered that there's a small location inside the RCU
> infrastructure where calling rcu_irq_enter() does not work. As trace events
> use rcu_irq_enter() it must make sure that it is functionable. A check
> against rcu_irq_enter_disabled() is added with a WARN_ON_ONCE() as no trace
> event should ever be used in that part of RCU. If the warning is triggered,
> then the trace event is ignored.
>
> Link: http://lkml.kernel.org/r/20170405093207.404f8deb@gandalf.local.home
>
> Cc: Mathieu (making code boring) Desnoyers <mathieu.desnoyers@efficios.com>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
>
> Mathieu,
>
> There! Are you now satisfied?
Since you reversed the boolean logic from my proposal, perhaps
rename the "rcucheck" argument to "rcudisabled" to reflect the
logic swap ?
Other than that:
Acked-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Thanks,
Mathieu
>
>
>
> include/linux/tracepoint.h | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
> index f72fcfe..7f98d50 100644
> --- a/include/linux/tracepoint.h
> +++ b/include/linux/tracepoint.h
> @@ -128,7 +128,7 @@ extern void syscall_unregfunc(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, prercu, postrcu) \
> +#define __DO_TRACE(tp, proto, args, cond, rcucheck, prercu, postrcu) \
> do { \
> struct tracepoint_func *it_func_ptr; \
> void *it_func; \
> @@ -136,6 +136,8 @@ extern void syscall_unregfunc(void);
> \
> if (!(cond)) \
> return; \
> + if (rcucheck) \
> + return; \
> prercu; \
> rcu_read_lock_sched_notrace(); \
> it_func_ptr = rcu_dereference_sched((tp)->funcs); \
> @@ -151,7 +153,7 @@ extern void syscall_unregfunc(void);
> } while (0)
>
> #ifndef MODULE
> -#define __DECLARE_TRACE_RCU(name, proto, args, cond, data_proto, data_args) \
> +#define __DECLARE_TRACE_RCU(name, proto, args, cond, data_proto, data_args) \
> static inline void trace_##name##_rcuidle(proto) \
> { \
> if (static_key_false(&__tracepoint_##name.key)) \
> @@ -159,6 +161,7 @@ extern void syscall_unregfunc(void);
> TP_PROTO(data_proto), \
> TP_ARGS(data_args), \
> TP_CONDITION(cond), \
> + WARN_ON_ONCE(rcu_irq_enter_disabled()),\
> rcu_irq_enter_irqson(), \
> rcu_irq_exit_irqson()); \
> }
> @@ -186,7 +189,7 @@ extern void syscall_unregfunc(void);
> __DO_TRACE(&__tracepoint_##name, \
> TP_PROTO(data_proto), \
> TP_ARGS(data_args), \
> - TP_CONDITION(cond),,); \
> + TP_CONDITION(cond),0,,); \
> if (IS_ENABLED(CONFIG_LOCKDEP) && (cond)) { \
> rcu_read_lock_sched_notrace(); \
> rcu_dereference_sched(__tracepoint_##name.funcs);\
> --
> 2.9.3
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
[toc] | [prev] | [next] | [standalone]
| From | Steven Rostedt <rostedt@goodmis.org> |
|---|---|
| Date | 2017-04-07 21:50 +0200 |
| Subject | Re: [PATCH 7/5 v4] tracing: Make sure rcu_irq_enter() can work for trace_*_rcuidle() trace events |
| Message-ID | <ttI3n-3uy-11@gated-at.bofh.it> |
| In reply to | #1619019 |
On Fri, 7 Apr 2017 15:41:17 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> #ifndef MODULE
> -#define __DECLARE_TRACE_RCU(name, proto, args, cond, data_proto, data_args) \
> +#define __DECLARE_TRACE_RCU(name, proto, args, cond, data_proto, data_args) \
> static inline void trace_##name##_rcuidle(proto) \
> { \
> if (static_key_false(&__tracepoint_##name.key)) \
> __DO_TRACE(&__tracepoint_##name, \
> TP_PROTO(data_proto), \
> TP_ARGS(data_args), \
> - TP_CONDITION(cond), \
> - rcu_irq_enter_irqson(), \
> - rcu_irq_exit_irqson()); \
> + TP_CONDITION(cond),1); \
I'm going to update this patch to add a space before the 1.
> }
> #else
> #define __DECLARE_TRACE_RCU(name, proto, args, cond, data_proto, data_args)
> @@ -186,7 +189,7 @@ extern void syscall_unregfunc(void);
> __DO_TRACE(&__tracepoint_##name, \
> TP_PROTO(data_proto), \
> TP_ARGS(data_args), \
> - TP_CONDITION(cond),,); \
> + TP_CONDITION(cond),0); \
And before the 0.
-- Steve
> if (IS_ENABLED(CONFIG_LOCKDEP) && (cond)) { \
> rcu_read_lock_sched_notrace(); \
> rcu_dereference_sched(__tracepoint_##name.funcs);\
[toc] | [prev] | [next] | [standalone]
| From | Steven Rostedt <rostedt@goodmis.org> |
|---|---|
| Date | 2017-04-07 21:50 +0200 |
| Subject | [PATCH 7/5 v4] tracing: Make sure rcu_irq_enter() can work for trace_*_rcuidle() trace events |
| Message-ID | <ttI3n-3uy-13@gated-at.bofh.it> |
| In reply to | #1619019 |
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Stack tracing discovered that there's a small location inside the RCU
infrastructure where calling rcu_irq_enter() does not work. As trace events
use rcu_irq_enter() it must make sure that it is functionable. A check
against rcu_irq_enter_disabled() is added with a WARN_ON_ONCE() as no trace
event should ever be used in that part of RCU. If the warning is triggered,
then the trace event is ignored.
Restructure the __DO_TRACE() a bit to get rid of the prercu and postrcu,
and just have an rcucheck that does the work from within the _DO_TRACE()
macro. gcc optimization will compile out the rcucheck=0 case.
Link: http://lkml.kernel.org/r/20170405093207.404f8deb@gandalf.local.home
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
include/linux/tracepoint.h | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index f72fcfe..4ecb116 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -128,7 +128,7 @@ extern void syscall_unregfunc(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, prercu, postrcu) \
+#define __DO_TRACE(tp, proto, args, cond, rcucheck) \
do { \
struct tracepoint_func *it_func_ptr; \
void *it_func; \
@@ -136,7 +136,11 @@ extern void syscall_unregfunc(void);
\
if (!(cond)) \
return; \
- prercu; \
+ if (rcucheck) { \
+ if (WARN_ON_ONCE(rcu_irq_enter_disabled())) \
+ return; \
+ rcu_irq_enter_irqson(); \
+ } \
rcu_read_lock_sched_notrace(); \
it_func_ptr = rcu_dereference_sched((tp)->funcs); \
if (it_func_ptr) { \
@@ -147,20 +151,19 @@ extern void syscall_unregfunc(void);
} while ((++it_func_ptr)->func); \
} \
rcu_read_unlock_sched_notrace(); \
- postrcu; \
+ if (rcucheck) \
+ rcu_irq_exit_irqson(); \
} while (0)
#ifndef MODULE
-#define __DECLARE_TRACE_RCU(name, proto, args, cond, data_proto, data_args) \
+#define __DECLARE_TRACE_RCU(name, proto, args, cond, data_proto, data_args) \
static inline void trace_##name##_rcuidle(proto) \
{ \
if (static_key_false(&__tracepoint_##name.key)) \
__DO_TRACE(&__tracepoint_##name, \
TP_PROTO(data_proto), \
TP_ARGS(data_args), \
- TP_CONDITION(cond), \
- rcu_irq_enter_irqson(), \
- rcu_irq_exit_irqson()); \
+ TP_CONDITION(cond),1); \
}
#else
#define __DECLARE_TRACE_RCU(name, proto, args, cond, data_proto, data_args)
@@ -186,7 +189,7 @@ extern void syscall_unregfunc(void);
__DO_TRACE(&__tracepoint_##name, \
TP_PROTO(data_proto), \
TP_ARGS(data_args), \
- TP_CONDITION(cond),,); \
+ TP_CONDITION(cond),0); \
if (IS_ENABLED(CONFIG_LOCKDEP) && (cond)) { \
rcu_read_lock_sched_notrace(); \
rcu_dereference_sched(__tracepoint_##name.funcs);\
--
2.9.3
[toc] | [prev] | [next] | [standalone]
| From | Steven Rostedt <rostedt@goodmis.org> |
|---|---|
| Date | 2017-04-07 19:30 +0200 |
| Subject | Re: [PATCH 7/5] tracing: Make sure rcu_irq_enter() can work for trace_*_rcuidle() trace events |
| Message-ID | <ttFRU-1SP-9@gated-at.bofh.it> |
| In reply to | #1618985 |
> > use rcu_irq_enter() it must make sure that it is functionable. A check > > I don't think functionable is the word you are looking for here. Perhaps > "must make sure that it can be invoked" ? Well, rcu_irq_enter() doesn't function in that location. And it's a change log, not in the code. I think it gets the point across enough. -- Steve
[toc] | [prev] | [next] | [standalone]
| From | Steven Rostedt <rostedt@goodmis.org> |
|---|---|
| Date | 2017-04-07 19:50 +0200 |
| Subject | [PATCH 7/5 v2] tracing: Make sure rcu_irq_enter() can work for trace_*_rcuidle() trace events |
| Message-ID | <ttGbg-21h-7@gated-at.bofh.it> |
| In reply to | #1618985 |
From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Stack tracing discovered that there's a small location inside the RCU
infrastructure where calling rcu_irq_enter() does not work. As trace events
use rcu_irq_enter() it must make sure that it is functionable. A check
against rcu_irq_enter_disabled() is added with a WARN_ON_ONCE() as no trace
event should ever be used in that part of RCU. If the warning is triggered,
then the trace event is ignored.
Link: http://lkml.kernel.org/r/20170405093207.404f8deb@gandalf.local.home
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
Mathieu, is this better? (yeah, i left in functionable, because I like
that word ;-)
include/linux/tracepoint.h | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index f72fcfe..352f32a 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -151,7 +151,12 @@ extern void syscall_unregfunc(void);
} while (0)
#ifndef MODULE
-#define __DECLARE_TRACE_RCU(name, proto, args, cond, data_proto, data_args) \
+#define TRACE_RCU_IRQ_ENTER_CHECK \
+ if (WARN_ON_ONCE(rcu_irq_enter_disabled())) \
+ return; \
+ rcu_irq_enter_irqson()
+
+#define __DECLARE_TRACE_RCU(name, proto, args, cond, data_proto, data_args) \
static inline void trace_##name##_rcuidle(proto) \
{ \
if (static_key_false(&__tracepoint_##name.key)) \
@@ -159,7 +164,7 @@ extern void syscall_unregfunc(void);
TP_PROTO(data_proto), \
TP_ARGS(data_args), \
TP_CONDITION(cond), \
- rcu_irq_enter_irqson(), \
+ PARAMS(TRACE_RCU_IRQ_ENTER_CHECK), \
rcu_irq_exit_irqson()); \
}
#else
--
2.9.3
[toc] | [prev] | [standalone]
Page 2 of 2 — ← Prev page 1 [2]
Back to top | Article view | linux.kernel
csiph-web