LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] tracepoint: rcuidle: use rcu_is_watching() and tree-rcu
@ 2020-03-10 20:20 Mathieu Desnoyers
2020-03-10 20:53 ` Paul E. McKenney
0 siblings, 1 reply; 6+ messages in thread
From: Mathieu Desnoyers @ 2020-03-10 20:20 UTC (permalink / raw)
To: Thomas Gleixner, Steven Rostedt
Cc: linux-kernel, Mathieu Desnoyers, Joel Fernandes,
Paul E. McKenney, Peter Zijlstra, Frederic Weisbecker,
Ingo Molnar
commit e6753f23d961 ("tracepoint: Make rcuidle tracepoint callers use
SRCU") aimed at improving performance of rcuidle tracepoints by using
SRCU rather than temporarily enabling tree-rcu every time.
commit 865e63b04e9b ("tracing: Add back in rcu_irq_enter/exit_irqson()
for rcuidle tracepoints") adds back the high-overhead enabling of
tree-rcu because perf expects RCU to be watching when called from
rcuidle tracepoints.
It turns out that by using "rcu_is_watching()" and conditionally
calling the high-overhead rcu_irq_enter/exit_irqson(), the original
motivation for using SRCU in the first place disappears.
I suspect that the original benchmarks justifying the introduction
of SRCU to handle rcuidle tracepoints was caused by preempt/irq
tracepoints, which are typically invoked from contexts that have
RCU watching.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Joel Fernandes <joel@joelfernandes.org>
CC: "Paul E. McKenney" <paulmck@kernel.org>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Ingo Molnar <mingo@kernel.org>
---
include/linux/tracepoint.h | 25 ++++++++++---------------
1 file changed, 10 insertions(+), 15 deletions(-)
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 1fb11daa5c53..8e0e94fee29a 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -165,25 +165,22 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
void *it_func; \
void *__data; \
int __maybe_unused __idx = 0; \
+ bool __exit_rcu = false; \
\
if (!(cond)) \
return; \
\
- /* srcu can't be used from NMI */ \
- WARN_ON_ONCE(rcuidle && in_nmi()); \
- \
- /* keep srcu and sched-rcu usage consistent */ \
- preempt_disable_notrace(); \
- \
/* \
- * For rcuidle callers, use srcu since sched-rcu \
- * doesn't work from the idle path. \
+ * For rcuidle callers, temporarily enable RCU if \
+ * it is not currently watching. \
*/ \
- if (rcuidle) { \
- __idx = srcu_read_lock_notrace(&tracepoint_srcu);\
+ if (rcuidle && !rcu_is_watching()) { \
rcu_irq_enter_irqson(); \
+ __exit_rcu = true; \
} \
\
+ preempt_disable_notrace(); \
+ \
it_func_ptr = rcu_dereference_raw((tp)->funcs); \
\
if (it_func_ptr) { \
@@ -194,12 +191,10 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
} while ((++it_func_ptr)->func); \
} \
\
- if (rcuidle) { \
- rcu_irq_exit_irqson(); \
- srcu_read_unlock_notrace(&tracepoint_srcu, __idx);\
- } \
- \
preempt_enable_notrace(); \
+ \
+ if (__exit_rcu) \
+ rcu_irq_exit_irqson(); \
} while (0)
#ifndef MODULE
--
2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tracepoint: rcuidle: use rcu_is_watching() and tree-rcu
2020-03-10 20:20 [PATCH] tracepoint: rcuidle: use rcu_is_watching() and tree-rcu Mathieu Desnoyers
@ 2020-03-10 20:53 ` Paul E. McKenney
2020-03-10 23:44 ` Mathieu Desnoyers
0 siblings, 1 reply; 6+ messages in thread
From: Paul E. McKenney @ 2020-03-10 20:53 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Thomas Gleixner, Steven Rostedt, linux-kernel, Joel Fernandes,
Peter Zijlstra, Frederic Weisbecker, Ingo Molnar
On Tue, Mar 10, 2020 at 04:20:54PM -0400, Mathieu Desnoyers wrote:
> commit e6753f23d961 ("tracepoint: Make rcuidle tracepoint callers use
> SRCU") aimed at improving performance of rcuidle tracepoints by using
> SRCU rather than temporarily enabling tree-rcu every time.
>
> commit 865e63b04e9b ("tracing: Add back in rcu_irq_enter/exit_irqson()
> for rcuidle tracepoints") adds back the high-overhead enabling of
> tree-rcu because perf expects RCU to be watching when called from
> rcuidle tracepoints.
>
> It turns out that by using "rcu_is_watching()" and conditionally
> calling the high-overhead rcu_irq_enter/exit_irqson(), the original
> motivation for using SRCU in the first place disappears.
Adding Alexei on CC for his thoughts, given that these were his
benchmarks. I believe that he also has additional use cases.
But given the use cases you describe, this seems plausible. This does
mean that tracepoints cannot be attached to the CPU-hotplug code that
runs on the incoming/outgoing CPU early/late in that process, though
that might be OK.
Thanx, Paul
> I suspect that the original benchmarks justifying the introduction
> of SRCU to handle rcuidle tracepoints was caused by preempt/irq
> tracepoints, which are typically invoked from contexts that have
> RCU watching.
>
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> CC: Thomas Gleixner <tglx@linutronix.de>
> CC: Steven Rostedt <rostedt@goodmis.org>
> CC: Joel Fernandes <joel@joelfernandes.org>
> CC: "Paul E. McKenney" <paulmck@kernel.org>
> CC: Peter Zijlstra <peterz@infradead.org>
> CC: Frederic Weisbecker <fweisbec@gmail.com>
> CC: Ingo Molnar <mingo@kernel.org>
> ---
> include/linux/tracepoint.h | 25 ++++++++++---------------
> 1 file changed, 10 insertions(+), 15 deletions(-)
>
> diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
> index 1fb11daa5c53..8e0e94fee29a 100644
> --- a/include/linux/tracepoint.h
> +++ b/include/linux/tracepoint.h
> @@ -165,25 +165,22 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
> void *it_func; \
> void *__data; \
> int __maybe_unused __idx = 0; \
> + bool __exit_rcu = false; \
> \
> if (!(cond)) \
> return; \
> \
> - /* srcu can't be used from NMI */ \
> - WARN_ON_ONCE(rcuidle && in_nmi()); \
> - \
> - /* keep srcu and sched-rcu usage consistent */ \
> - preempt_disable_notrace(); \
> - \
> /* \
> - * For rcuidle callers, use srcu since sched-rcu \
> - * doesn't work from the idle path. \
> + * For rcuidle callers, temporarily enable RCU if \
> + * it is not currently watching. \
> */ \
> - if (rcuidle) { \
> - __idx = srcu_read_lock_notrace(&tracepoint_srcu);\
> + if (rcuidle && !rcu_is_watching()) { \
> rcu_irq_enter_irqson(); \
> + __exit_rcu = true; \
> } \
> \
> + preempt_disable_notrace(); \
> + \
> it_func_ptr = rcu_dereference_raw((tp)->funcs); \
> \
> if (it_func_ptr) { \
> @@ -194,12 +191,10 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
> } while ((++it_func_ptr)->func); \
> } \
> \
> - if (rcuidle) { \
> - rcu_irq_exit_irqson(); \
> - srcu_read_unlock_notrace(&tracepoint_srcu, __idx);\
> - } \
> - \
> preempt_enable_notrace(); \
> + \
> + if (__exit_rcu) \
> + rcu_irq_exit_irqson(); \
> } while (0)
>
> #ifndef MODULE
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tracepoint: rcuidle: use rcu_is_watching() and tree-rcu
2020-03-10 20:53 ` Paul E. McKenney
@ 2020-03-10 23:44 ` Mathieu Desnoyers
2020-03-10 23:52 ` Paul E. McKenney
0 siblings, 1 reply; 6+ messages in thread
From: Mathieu Desnoyers @ 2020-03-10 23:44 UTC (permalink / raw)
To: paulmck
Cc: Thomas Gleixner, rostedt, linux-kernel, Joel Fernandes, Google,
Peter Zijlstra, fweisbec, Ingo Molnar
----- On Mar 10, 2020, at 4:53 PM, paulmck paulmck@kernel.org wrote:
> On Tue, Mar 10, 2020 at 04:20:54PM -0400, Mathieu Desnoyers wrote:
>> commit e6753f23d961 ("tracepoint: Make rcuidle tracepoint callers use
>> SRCU") aimed at improving performance of rcuidle tracepoints by using
>> SRCU rather than temporarily enabling tree-rcu every time.
>>
>> commit 865e63b04e9b ("tracing: Add back in rcu_irq_enter/exit_irqson()
>> for rcuidle tracepoints") adds back the high-overhead enabling of
>> tree-rcu because perf expects RCU to be watching when called from
>> rcuidle tracepoints.
>>
>> It turns out that by using "rcu_is_watching()" and conditionally
>> calling the high-overhead rcu_irq_enter/exit_irqson(), the original
>> motivation for using SRCU in the first place disappears.
>
> Adding Alexei on CC for his thoughts, given that these were his
> benchmarks. I believe that he also has additional use cases.
Good point! Sorry I forgot to add Alexei to my CC list for that
patch.
> But given the use cases you describe, this seems plausible. This does
> mean that tracepoints cannot be attached to the CPU-hotplug code that
> runs on the incoming/outgoing CPU early/late in that process, though
> that might be OK.
Do you mean standard tracepoints or rcuidle tracepoints ?
Are there any such tracepoints early/late in the cpu hotplug code today ?
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tracepoint: rcuidle: use rcu_is_watching() and tree-rcu
2020-03-10 23:44 ` Mathieu Desnoyers
@ 2020-03-10 23:52 ` Paul E. McKenney
2020-03-10 23:56 ` Mathieu Desnoyers
0 siblings, 1 reply; 6+ messages in thread
From: Paul E. McKenney @ 2020-03-10 23:52 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Thomas Gleixner, rostedt, linux-kernel, Joel Fernandes, Google,
Peter Zijlstra, fweisbec, Ingo Molnar
On Tue, Mar 10, 2020 at 07:44:33PM -0400, Mathieu Desnoyers wrote:
> ----- On Mar 10, 2020, at 4:53 PM, paulmck paulmck@kernel.org wrote:
>
> > On Tue, Mar 10, 2020 at 04:20:54PM -0400, Mathieu Desnoyers wrote:
> >> commit e6753f23d961 ("tracepoint: Make rcuidle tracepoint callers use
> >> SRCU") aimed at improving performance of rcuidle tracepoints by using
> >> SRCU rather than temporarily enabling tree-rcu every time.
> >>
> >> commit 865e63b04e9b ("tracing: Add back in rcu_irq_enter/exit_irqson()
> >> for rcuidle tracepoints") adds back the high-overhead enabling of
> >> tree-rcu because perf expects RCU to be watching when called from
> >> rcuidle tracepoints.
> >>
> >> It turns out that by using "rcu_is_watching()" and conditionally
> >> calling the high-overhead rcu_irq_enter/exit_irqson(), the original
> >> motivation for using SRCU in the first place disappears.
> >
> > Adding Alexei on CC for his thoughts, given that these were his
> > benchmarks. I believe that he also has additional use cases.
>
> Good point! Sorry I forgot to add Alexei to my CC list for that
> patch.
>
> > But given the use cases you describe, this seems plausible. This does
> > mean that tracepoints cannot be attached to the CPU-hotplug code that
> > runs on the incoming/outgoing CPU early/late in that process, though
> > that might be OK.
>
> Do you mean standard tracepoints or rcuidle tracepoints ?
>
> Are there any such tracepoints early/late in the cpu hotplug code today ?
I have no idea. You would know better than I. However, I would expect
that the same common-code issue that applies to exception-entry/exit
code might also apply to the CPU hotplug code.
Thanx, Paul
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tracepoint: rcuidle: use rcu_is_watching() and tree-rcu
2020-03-10 23:52 ` Paul E. McKenney
@ 2020-03-10 23:56 ` Mathieu Desnoyers
2020-03-11 0:08 ` Paul E. McKenney
0 siblings, 1 reply; 6+ messages in thread
From: Mathieu Desnoyers @ 2020-03-10 23:56 UTC (permalink / raw)
To: paulmck
Cc: Thomas Gleixner, rostedt, linux-kernel, Joel Fernandes, Google,
Peter Zijlstra, fweisbec, Ingo Molnar
----- On Mar 10, 2020, at 7:52 PM, paulmck paulmck@kernel.org wrote:
> On Tue, Mar 10, 2020 at 07:44:33PM -0400, Mathieu Desnoyers wrote:
>> ----- On Mar 10, 2020, at 4:53 PM, paulmck paulmck@kernel.org wrote:
>>
>> > On Tue, Mar 10, 2020 at 04:20:54PM -0400, Mathieu Desnoyers wrote:
>> >> commit e6753f23d961 ("tracepoint: Make rcuidle tracepoint callers use
>> >> SRCU") aimed at improving performance of rcuidle tracepoints by using
>> >> SRCU rather than temporarily enabling tree-rcu every time.
>> >>
>> >> commit 865e63b04e9b ("tracing: Add back in rcu_irq_enter/exit_irqson()
>> >> for rcuidle tracepoints") adds back the high-overhead enabling of
>> >> tree-rcu because perf expects RCU to be watching when called from
>> >> rcuidle tracepoints.
>> >>
>> >> It turns out that by using "rcu_is_watching()" and conditionally
>> >> calling the high-overhead rcu_irq_enter/exit_irqson(), the original
>> >> motivation for using SRCU in the first place disappears.
>> >
>> > Adding Alexei on CC for his thoughts, given that these were his
>> > benchmarks. I believe that he also has additional use cases.
>>
>> Good point! Sorry I forgot to add Alexei to my CC list for that
>> patch.
>>
>> > But given the use cases you describe, this seems plausible. This does
>> > mean that tracepoints cannot be attached to the CPU-hotplug code that
>> > runs on the incoming/outgoing CPU early/late in that process, though
>> > that might be OK.
>>
>> Do you mean standard tracepoints or rcuidle tracepoints ?
>>
>> Are there any such tracepoints early/late in the cpu hotplug code today ?
>
> I have no idea. You would know better than I. However, I would expect
> that the same common-code issue that applies to exception-entry/exit
> code might also apply to the CPU hotplug code.
I would also expect early/late CPU hotplug states to fall into the same
category of "low-level entry/exit" code which Thomas would like to hide
from instrumentation.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tracepoint: rcuidle: use rcu_is_watching() and tree-rcu
2020-03-10 23:56 ` Mathieu Desnoyers
@ 2020-03-11 0:08 ` Paul E. McKenney
0 siblings, 0 replies; 6+ messages in thread
From: Paul E. McKenney @ 2020-03-11 0:08 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Thomas Gleixner, rostedt, linux-kernel, Joel Fernandes, Google,
Peter Zijlstra, fweisbec, Ingo Molnar
On Tue, Mar 10, 2020 at 07:56:17PM -0400, Mathieu Desnoyers wrote:
> ----- On Mar 10, 2020, at 7:52 PM, paulmck paulmck@kernel.org wrote:
>
> > On Tue, Mar 10, 2020 at 07:44:33PM -0400, Mathieu Desnoyers wrote:
> >> ----- On Mar 10, 2020, at 4:53 PM, paulmck paulmck@kernel.org wrote:
> >>
> >> > On Tue, Mar 10, 2020 at 04:20:54PM -0400, Mathieu Desnoyers wrote:
> >> >> commit e6753f23d961 ("tracepoint: Make rcuidle tracepoint callers use
> >> >> SRCU") aimed at improving performance of rcuidle tracepoints by using
> >> >> SRCU rather than temporarily enabling tree-rcu every time.
> >> >>
> >> >> commit 865e63b04e9b ("tracing: Add back in rcu_irq_enter/exit_irqson()
> >> >> for rcuidle tracepoints") adds back the high-overhead enabling of
> >> >> tree-rcu because perf expects RCU to be watching when called from
> >> >> rcuidle tracepoints.
> >> >>
> >> >> It turns out that by using "rcu_is_watching()" and conditionally
> >> >> calling the high-overhead rcu_irq_enter/exit_irqson(), the original
> >> >> motivation for using SRCU in the first place disappears.
> >> >
> >> > Adding Alexei on CC for his thoughts, given that these were his
> >> > benchmarks. I believe that he also has additional use cases.
> >>
> >> Good point! Sorry I forgot to add Alexei to my CC list for that
> >> patch.
> >>
> >> > But given the use cases you describe, this seems plausible. This does
> >> > mean that tracepoints cannot be attached to the CPU-hotplug code that
> >> > runs on the incoming/outgoing CPU early/late in that process, though
> >> > that might be OK.
> >>
> >> Do you mean standard tracepoints or rcuidle tracepoints ?
> >>
> >> Are there any such tracepoints early/late in the cpu hotplug code today ?
> >
> > I have no idea. You would know better than I. However, I would expect
> > that the same common-code issue that applies to exception-entry/exit
> > code might also apply to the CPU hotplug code.
>
> I would also expect early/late CPU hotplug states to fall into the same
> category of "low-level entry/exit" code which Thomas would like to hide
> from instrumentation.
That decision I must leave to you guys.
Thanx, Paul
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-03-11 0:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-10 20:20 [PATCH] tracepoint: rcuidle: use rcu_is_watching() and tree-rcu Mathieu Desnoyers
2020-03-10 20:53 ` Paul E. McKenney
2020-03-10 23:44 ` Mathieu Desnoyers
2020-03-10 23:52 ` Paul E. McKenney
2020-03-10 23:56 ` Mathieu Desnoyers
2020-03-11 0:08 ` Paul E. McKenney
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).