* [RFC v1 0/2] RCU dyntick nesting counter cleanups @ 2019-08-27 1:33 Joel Fernandes (Google) 2019-08-28 0:11 ` Paul E. McKenney 0 siblings, 1 reply; 4+ messages in thread From: Joel Fernandes (Google) @ 2019-08-27 1:33 UTC (permalink / raw) To: linux-kernel Cc: Joel Fernandes (Google), Frederic Weisbecker, Jonathan Corbet, Josh Triplett, kernel-team, Lai Jiangshan, linux-doc, Mathieu Desnoyers, Mauro Carvalho Chehab, Paul E. McKenney, rcu, Steven Rostedt These patches clean up the usage of dynticks nesting counters simplifying the code, while preserving the usecases. It is a much needed simplification, makes the code less confusing, and prevents future bugs such as those that arise from forgetting that the dynticks_nmi_nesting counter is not a simple counter and can be "crowbarred" in common situations. Several nights of rcutorture testing with CONFIG_RCU_EQS_DEBUG on all RCU kernel configurations have survived without any splats. Further testing is in progress, hence marked as RFC! thanks, - Joel Joel Fernandes (Google) (2): rcu/tree: Clean up dynticks counter usage rcu/tree: Remove dynticks_nmi_nesting counter .../Data-Structures/Data-Structures.rst | 31 ++---- Documentation/RCU/stallwarn.txt | 6 +- kernel/rcu/rcu.h | 4 - kernel/rcu/tree.c | 98 +++++++++---------- kernel/rcu/tree.h | 4 +- kernel/rcu/tree_stall.h | 4 +- 6 files changed, 64 insertions(+), 83 deletions(-) -- 2.23.0.187.g17f5b7556c-goog ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC v1 0/2] RCU dyntick nesting counter cleanups 2019-08-27 1:33 [RFC v1 0/2] RCU dyntick nesting counter cleanups Joel Fernandes (Google) @ 2019-08-28 0:11 ` Paul E. McKenney 2019-08-28 18:26 ` [PATCH] rcu/dyntick-idle: Add better tracing Joel Fernandes (Google) 0 siblings, 1 reply; 4+ messages in thread From: Paul E. McKenney @ 2019-08-28 0:11 UTC (permalink / raw) To: Joel Fernandes (Google) Cc: linux-kernel, Frederic Weisbecker, Jonathan Corbet, Josh Triplett, kernel-team, Lai Jiangshan, linux-doc, Mathieu Desnoyers, Mauro Carvalho Chehab, rcu, Steven Rostedt On Mon, Aug 26, 2019 at 09:33:52PM -0400, Joel Fernandes (Google) wrote: > These patches clean up the usage of dynticks nesting counters simplifying the > code, while preserving the usecases. > > It is a much needed simplification, makes the code less confusing, and prevents > future bugs such as those that arise from forgetting that the > dynticks_nmi_nesting counter is not a simple counter and can be "crowbarred" in > common situations. > > Several nights of rcutorture testing with CONFIG_RCU_EQS_DEBUG on all RCU > kernel configurations have survived without any splats. > > Further testing is in progress, hence marked as RFC! My intent was to review this today, but this ran afoul of recent 3.10 work that made for some "interesting" review comments. Fortunately, I realized my mistake before sending the email. I then reviewed the current 2019 RCU dyntick-idle code. I am doing some (possibly redundant) rcutorture runs on them here, and will take a fresh look tomorrow. Thanx, Paul > thanks, > > - Joel > > Joel Fernandes (Google) (2): > rcu/tree: Clean up dynticks counter usage > rcu/tree: Remove dynticks_nmi_nesting counter > > .../Data-Structures/Data-Structures.rst | 31 ++---- > Documentation/RCU/stallwarn.txt | 6 +- > kernel/rcu/rcu.h | 4 - > kernel/rcu/tree.c | 98 +++++++++---------- > kernel/rcu/tree.h | 4 +- > kernel/rcu/tree_stall.h | 4 +- > 6 files changed, 64 insertions(+), 83 deletions(-) > > -- > 2.23.0.187.g17f5b7556c-goog > ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] rcu/dyntick-idle: Add better tracing 2019-08-28 0:11 ` Paul E. McKenney @ 2019-08-28 18:26 ` Joel Fernandes (Google) 2019-08-28 23:05 ` Paul E. McKenney 0 siblings, 1 reply; 4+ messages in thread From: Joel Fernandes (Google) @ 2019-08-28 18:26 UTC (permalink / raw) To: linux-kernel, paulmck Cc: Joel Fernandes (Google), Frederic Weisbecker, Jonathan Corbet, Josh Triplett, kernel-team, Lai Jiangshan, linux-doc, Mathieu Desnoyers, Mauro Carvalho Chehab, rcu, Steven Rostedt The dyntick-idle traces are a bit confusing. This patch makes it simpler and adds some missing cases such as EQS-enter because user vs idle mode. Following are the changes: (1) Add a new context field to trace_rcu_dyntick tracepoint. This context field can be "USER", "IDLE" or "IRQ". (2) Remove the "++=" and "--=" strings and replace them with "StillNonIdle". This is much easier on the eyes, and the -- and ++ are easily apparent in the dynticks_nesting counters we are printing anyway. This patch is based on the previous patches to simplify rcu_dyntick counters [1] and with these traces, I have verified the counters are working properly. [1] Link: https://lore.kernel.org/patchwork/patch/1120021/ Link: https://lore.kernel.org/patchwork/patch/1120022/ Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> --- include/trace/events/rcu.h | 13 ++++++++----- kernel/rcu/tree.c | 17 +++++++++++------ 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/include/trace/events/rcu.h b/include/trace/events/rcu.h index 66122602bd08..474c1f7e7104 100644 --- a/include/trace/events/rcu.h +++ b/include/trace/events/rcu.h @@ -449,12 +449,14 @@ TRACE_EVENT_RCU(rcu_fqs, */ TRACE_EVENT_RCU(rcu_dyntick, - TP_PROTO(const char *polarity, long oldnesting, long newnesting, atomic_t dynticks), + TP_PROTO(const char *polarity, const char *context, long oldnesting, + long newnesting, atomic_t dynticks), - TP_ARGS(polarity, oldnesting, newnesting, dynticks), + TP_ARGS(polarity, context, oldnesting, newnesting, dynticks), TP_STRUCT__entry( __field(const char *, polarity) + __field(const char *, context) __field(long, oldnesting) __field(long, newnesting) __field(int, dynticks) @@ -462,14 +464,15 @@ TRACE_EVENT_RCU(rcu_dyntick, TP_fast_assign( __entry->polarity = polarity; + __entry->context = context; __entry->oldnesting = oldnesting; __entry->newnesting = newnesting; __entry->dynticks = atomic_read(&dynticks); ), - TP_printk("%s %lx %lx %#3x", __entry->polarity, - __entry->oldnesting, __entry->newnesting, - __entry->dynticks & 0xfff) + TP_printk("%s %s %lx %lx %#3x", __entry->polarity, + __entry->context, __entry->oldnesting, __entry->newnesting, + __entry->dynticks & 0xfff) ); /* diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c index 1465a3e406f8..1a65919ec800 100644 --- a/kernel/rcu/tree.c +++ b/kernel/rcu/tree.c @@ -570,7 +570,8 @@ static void rcu_eqs_enter(bool user) } lockdep_assert_irqs_disabled(); - trace_rcu_dyntick(TPS("Start"), rdp->dynticks_nesting, 0, rdp->dynticks); + trace_rcu_dyntick(TPS("Start"), (user ? TPS("USER") : TPS("IDLE")), + rdp->dynticks_nesting, 0, rdp->dynticks); WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) && !user && !is_idle_task(current)); rdp = this_cpu_ptr(&rcu_data); do_nocb_deferred_wakeup(rdp); @@ -642,15 +643,17 @@ static __always_inline void rcu_nmi_exit_common(bool irq) * leave it in non-RCU-idle state. */ if (rdp->dynticks_nesting != 1) { - trace_rcu_dyntick(TPS("--="), rdp->dynticks_nesting, - rdp->dynticks_nesting - 2, rdp->dynticks); + trace_rcu_dyntick(TPS("StillNonIdle"), TPS("IRQ"), + rdp->dynticks_nesting, rdp->dynticks_nesting - 2, + rdp->dynticks); WRITE_ONCE(rdp->dynticks_nesting, /* No store tearing. */ rdp->dynticks_nesting - 2); return; } /* This NMI interrupted an RCU-idle CPU, restore RCU-idleness. */ - trace_rcu_dyntick(TPS("Startirq"), rdp->dynticks_nesting, 0, rdp->dynticks); + trace_rcu_dyntick(TPS("Start"), TPS("IRQ"), rdp->dynticks_nesting, 0, + rdp->dynticks); WRITE_ONCE(rdp->dynticks_nesting, 0); /* Avoid store tearing. */ if (irq) @@ -733,7 +736,8 @@ static void rcu_eqs_exit(bool user) rcu_dynticks_task_exit(); rcu_dynticks_eqs_exit(); rcu_cleanup_after_idle(); - trace_rcu_dyntick(TPS("End"), rdp->dynticks_nesting, 1, rdp->dynticks); + trace_rcu_dyntick(TPS("End"), (user ? TPS("USER") : TPS("IDLE")), + rdp->dynticks_nesting, 1, rdp->dynticks); WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) && !user && !is_idle_task(current)); WRITE_ONCE(rdp->dynticks_nesting, 1); @@ -825,7 +829,8 @@ static __always_inline void rcu_nmi_enter_common(bool irq) tick_dep_set_cpu(rdp->cpu, TICK_DEP_BIT_RCU); } - trace_rcu_dyntick(incby == 1 ? TPS("Endirq") : TPS("++="), + trace_rcu_dyntick(incby == 1 ? TPS("End") : TPS("StillNonIdle"), + TPS("IRQ"), rdp->dynticks_nesting, rdp->dynticks_nesting + incby, rdp->dynticks); -- 2.23.0.187.g17f5b7556c-goog ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] rcu/dyntick-idle: Add better tracing 2019-08-28 18:26 ` [PATCH] rcu/dyntick-idle: Add better tracing Joel Fernandes (Google) @ 2019-08-28 23:05 ` Paul E. McKenney 0 siblings, 0 replies; 4+ messages in thread From: Paul E. McKenney @ 2019-08-28 23:05 UTC (permalink / raw) To: Joel Fernandes (Google) Cc: linux-kernel, Frederic Weisbecker, Jonathan Corbet, Josh Triplett, kernel-team, Lai Jiangshan, linux-doc, Mathieu Desnoyers, Mauro Carvalho Chehab, rcu, Steven Rostedt On Wed, Aug 28, 2019 at 02:26:13PM -0400, Joel Fernandes (Google) wrote: > The dyntick-idle traces are a bit confusing. This patch makes it simpler > and adds some missing cases such as EQS-enter because user vs idle mode. > > Following are the changes: > (1) Add a new context field to trace_rcu_dyntick tracepoint. This > context field can be "USER", "IDLE" or "IRQ". > > (2) Remove the "++=" and "--=" strings and replace them with > "StillNonIdle". This is much easier on the eyes, and the -- and ++ > are easily apparent in the dynticks_nesting counters we are printing > anyway. > > This patch is based on the previous patches to simplify rcu_dyntick > counters [1] and with these traces, I have verified the counters are > working properly. > > [1] > Link: https://lore.kernel.org/patchwork/patch/1120021/ > Link: https://lore.kernel.org/patchwork/patch/1120022/ > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> Looks plausible to me. Thanx, Paul > --- > include/trace/events/rcu.h | 13 ++++++++----- > kernel/rcu/tree.c | 17 +++++++++++------ > 2 files changed, 19 insertions(+), 11 deletions(-) > > diff --git a/include/trace/events/rcu.h b/include/trace/events/rcu.h > index 66122602bd08..474c1f7e7104 100644 > --- a/include/trace/events/rcu.h > +++ b/include/trace/events/rcu.h > @@ -449,12 +449,14 @@ TRACE_EVENT_RCU(rcu_fqs, > */ > TRACE_EVENT_RCU(rcu_dyntick, > > - TP_PROTO(const char *polarity, long oldnesting, long newnesting, atomic_t dynticks), > + TP_PROTO(const char *polarity, const char *context, long oldnesting, > + long newnesting, atomic_t dynticks), > > - TP_ARGS(polarity, oldnesting, newnesting, dynticks), > + TP_ARGS(polarity, context, oldnesting, newnesting, dynticks), > > TP_STRUCT__entry( > __field(const char *, polarity) > + __field(const char *, context) > __field(long, oldnesting) > __field(long, newnesting) > __field(int, dynticks) > @@ -462,14 +464,15 @@ TRACE_EVENT_RCU(rcu_dyntick, > > TP_fast_assign( > __entry->polarity = polarity; > + __entry->context = context; > __entry->oldnesting = oldnesting; > __entry->newnesting = newnesting; > __entry->dynticks = atomic_read(&dynticks); > ), > > - TP_printk("%s %lx %lx %#3x", __entry->polarity, > - __entry->oldnesting, __entry->newnesting, > - __entry->dynticks & 0xfff) > + TP_printk("%s %s %lx %lx %#3x", __entry->polarity, > + __entry->context, __entry->oldnesting, __entry->newnesting, > + __entry->dynticks & 0xfff) > ); > > /* > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c > index 1465a3e406f8..1a65919ec800 100644 > --- a/kernel/rcu/tree.c > +++ b/kernel/rcu/tree.c > @@ -570,7 +570,8 @@ static void rcu_eqs_enter(bool user) > } > > lockdep_assert_irqs_disabled(); > - trace_rcu_dyntick(TPS("Start"), rdp->dynticks_nesting, 0, rdp->dynticks); > + trace_rcu_dyntick(TPS("Start"), (user ? TPS("USER") : TPS("IDLE")), > + rdp->dynticks_nesting, 0, rdp->dynticks); > WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) && !user && !is_idle_task(current)); > rdp = this_cpu_ptr(&rcu_data); > do_nocb_deferred_wakeup(rdp); > @@ -642,15 +643,17 @@ static __always_inline void rcu_nmi_exit_common(bool irq) > * leave it in non-RCU-idle state. > */ > if (rdp->dynticks_nesting != 1) { > - trace_rcu_dyntick(TPS("--="), rdp->dynticks_nesting, > - rdp->dynticks_nesting - 2, rdp->dynticks); > + trace_rcu_dyntick(TPS("StillNonIdle"), TPS("IRQ"), > + rdp->dynticks_nesting, rdp->dynticks_nesting - 2, > + rdp->dynticks); > WRITE_ONCE(rdp->dynticks_nesting, /* No store tearing. */ > rdp->dynticks_nesting - 2); > return; > } > > /* This NMI interrupted an RCU-idle CPU, restore RCU-idleness. */ > - trace_rcu_dyntick(TPS("Startirq"), rdp->dynticks_nesting, 0, rdp->dynticks); > + trace_rcu_dyntick(TPS("Start"), TPS("IRQ"), rdp->dynticks_nesting, 0, > + rdp->dynticks); > WRITE_ONCE(rdp->dynticks_nesting, 0); /* Avoid store tearing. */ > > if (irq) > @@ -733,7 +736,8 @@ static void rcu_eqs_exit(bool user) > rcu_dynticks_task_exit(); > rcu_dynticks_eqs_exit(); > rcu_cleanup_after_idle(); > - trace_rcu_dyntick(TPS("End"), rdp->dynticks_nesting, 1, rdp->dynticks); > + trace_rcu_dyntick(TPS("End"), (user ? TPS("USER") : TPS("IDLE")), > + rdp->dynticks_nesting, 1, rdp->dynticks); > WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) && !user && !is_idle_task(current)); > WRITE_ONCE(rdp->dynticks_nesting, 1); > > @@ -825,7 +829,8 @@ static __always_inline void rcu_nmi_enter_common(bool irq) > tick_dep_set_cpu(rdp->cpu, TICK_DEP_BIT_RCU); > } > > - trace_rcu_dyntick(incby == 1 ? TPS("Endirq") : TPS("++="), > + trace_rcu_dyntick(incby == 1 ? TPS("End") : TPS("StillNonIdle"), > + TPS("IRQ"), > rdp->dynticks_nesting, > rdp->dynticks_nesting + incby, rdp->dynticks); > > -- > 2.23.0.187.g17f5b7556c-goog > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-08-28 23:06 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-27 1:33 [RFC v1 0/2] RCU dyntick nesting counter cleanups Joel Fernandes (Google) 2019-08-28 0:11 ` Paul E. McKenney 2019-08-28 18:26 ` [PATCH] rcu/dyntick-idle: Add better tracing Joel Fernandes (Google) 2019-08-28 23:05 ` 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).