* [PATCH 1/3] rcu/trace: Print negative GP numbers correctly @ 2020-06-19 1:36 Joel Fernandes (Google) 2020-06-19 1:36 ` [PATCH 2/3] rcu/trace: Use gp_seq_req in acceleration's rcu_grace_period tracepoint Joel Fernandes (Google) 2020-06-19 1:36 ` [PATCH 3/3] rcu/trace: Add name of the source for gp_seq to prevent confusion Joel Fernandes (Google) 0 siblings, 2 replies; 7+ messages in thread From: Joel Fernandes (Google) @ 2020-06-19 1:36 UTC (permalink / raw) To: linux-kernel Cc: Joel Fernandes (Google), urezki, Ingo Molnar, Josh Triplett, Lai Jiangshan, Madhuparna Bhowmik, Mathieu Desnoyers, Paul E. McKenney, rcu, Steven Rostedt GP numbers start from -300 and gp_seq numbers start of -1200 (for a shift of 2). These negative numbers are printed as unsigned long which not only takes up more text space, but is rather confusing to the reader as they have to constantly expend energy to truncate the number. Just print the negative numbering directly. Cc: urezki@gmail.com Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> --- include/trace/events/rcu.h | 54 +++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/include/trace/events/rcu.h b/include/trace/events/rcu.h index ced71237b7e4a..155b5cb43cfd3 100644 --- a/include/trace/events/rcu.h +++ b/include/trace/events/rcu.h @@ -74,17 +74,17 @@ TRACE_EVENT_RCU(rcu_grace_period, TP_STRUCT__entry( __field(const char *, rcuname) - __field(unsigned long, gp_seq) + __field(long, gp_seq) __field(const char *, gpevent) ), TP_fast_assign( __entry->rcuname = rcuname; - __entry->gp_seq = gp_seq; + __entry->gp_seq = (long)gp_seq; __entry->gpevent = gpevent; ), - TP_printk("%s %lu %s", + TP_printk("%s %ld %s", __entry->rcuname, __entry->gp_seq, __entry->gpevent) ); @@ -114,8 +114,8 @@ TRACE_EVENT_RCU(rcu_future_grace_period, TP_STRUCT__entry( __field(const char *, rcuname) - __field(unsigned long, gp_seq) - __field(unsigned long, gp_seq_req) + __field(long, gp_seq) + __field(long, gp_seq_req) __field(u8, level) __field(int, grplo) __field(int, grphi) @@ -124,16 +124,16 @@ TRACE_EVENT_RCU(rcu_future_grace_period, TP_fast_assign( __entry->rcuname = rcuname; - __entry->gp_seq = gp_seq; - __entry->gp_seq_req = gp_seq_req; + __entry->gp_seq = (long)gp_seq; + __entry->gp_seq_req = (long)gp_seq_req; __entry->level = level; __entry->grplo = grplo; __entry->grphi = grphi; __entry->gpevent = gpevent; ), - TP_printk("%s %lu %lu %u %d %d %s", - __entry->rcuname, __entry->gp_seq, __entry->gp_seq_req, __entry->level, + TP_printk("%s %ld %ld %u %d %d %s", + __entry->rcuname, (long)__entry->gp_seq, (long)__entry->gp_seq_req, __entry->level, __entry->grplo, __entry->grphi, __entry->gpevent) ); @@ -153,7 +153,7 @@ TRACE_EVENT_RCU(rcu_grace_period_init, TP_STRUCT__entry( __field(const char *, rcuname) - __field(unsigned long, gp_seq) + __field(long, gp_seq) __field(u8, level) __field(int, grplo) __field(int, grphi) @@ -162,14 +162,14 @@ TRACE_EVENT_RCU(rcu_grace_period_init, TP_fast_assign( __entry->rcuname = rcuname; - __entry->gp_seq = gp_seq; + __entry->gp_seq = (long)gp_seq; __entry->level = level; __entry->grplo = grplo; __entry->grphi = grphi; __entry->qsmask = qsmask; ), - TP_printk("%s %lu %u %d %d %lx", + TP_printk("%s %ld %u %d %d %lx", __entry->rcuname, __entry->gp_seq, __entry->level, __entry->grplo, __entry->grphi, __entry->qsmask) ); @@ -197,17 +197,17 @@ TRACE_EVENT_RCU(rcu_exp_grace_period, TP_STRUCT__entry( __field(const char *, rcuname) - __field(unsigned long, gpseq) + __field(long, gpseq) __field(const char *, gpevent) ), TP_fast_assign( __entry->rcuname = rcuname; - __entry->gpseq = gpseq; + __entry->gpseq = (long)gpseq; __entry->gpevent = gpevent; ), - TP_printk("%s %lu %s", + TP_printk("%s %ld %s", __entry->rcuname, __entry->gpseq, __entry->gpevent) ); @@ -316,17 +316,17 @@ TRACE_EVENT_RCU(rcu_preempt_task, TP_STRUCT__entry( __field(const char *, rcuname) - __field(unsigned long, gp_seq) + __field(long, gp_seq) __field(int, pid) ), TP_fast_assign( __entry->rcuname = rcuname; - __entry->gp_seq = gp_seq; + __entry->gp_seq = (long)gp_seq; __entry->pid = pid; ), - TP_printk("%s %lu %d", + TP_printk("%s %ld %d", __entry->rcuname, __entry->gp_seq, __entry->pid) ); @@ -343,17 +343,17 @@ TRACE_EVENT_RCU(rcu_unlock_preempted_task, TP_STRUCT__entry( __field(const char *, rcuname) - __field(unsigned long, gp_seq) + __field(long, gp_seq) __field(int, pid) ), TP_fast_assign( __entry->rcuname = rcuname; - __entry->gp_seq = gp_seq; + __entry->gp_seq = (long)gp_seq; __entry->pid = pid; ), - TP_printk("%s %lu %d", __entry->rcuname, __entry->gp_seq, __entry->pid) + TP_printk("%s %ld %d", __entry->rcuname, __entry->gp_seq, __entry->pid) ); /* @@ -374,7 +374,7 @@ TRACE_EVENT_RCU(rcu_quiescent_state_report, TP_STRUCT__entry( __field(const char *, rcuname) - __field(unsigned long, gp_seq) + __field(long, gp_seq) __field(unsigned long, mask) __field(unsigned long, qsmask) __field(u8, level) @@ -385,7 +385,7 @@ TRACE_EVENT_RCU(rcu_quiescent_state_report, TP_fast_assign( __entry->rcuname = rcuname; - __entry->gp_seq = gp_seq; + __entry->gp_seq = (long)gp_seq; __entry->mask = mask; __entry->qsmask = qsmask; __entry->level = level; @@ -394,7 +394,7 @@ TRACE_EVENT_RCU(rcu_quiescent_state_report, __entry->gp_tasks = gp_tasks; ), - TP_printk("%s %lu %lx>%lx %u %d %d %u", + TP_printk("%s %ld %lx>%lx %u %d %d %u", __entry->rcuname, __entry->gp_seq, __entry->mask, __entry->qsmask, __entry->level, __entry->grplo, __entry->grphi, __entry->gp_tasks) @@ -415,19 +415,19 @@ TRACE_EVENT_RCU(rcu_fqs, TP_STRUCT__entry( __field(const char *, rcuname) - __field(unsigned long, gp_seq) + __field(long, gp_seq) __field(int, cpu) __field(const char *, qsevent) ), TP_fast_assign( __entry->rcuname = rcuname; - __entry->gp_seq = gp_seq; + __entry->gp_seq = (long)gp_seq; __entry->cpu = cpu; __entry->qsevent = qsevent; ), - TP_printk("%s %lu %d %s", + TP_printk("%s %ld %d %s", __entry->rcuname, __entry->gp_seq, __entry->cpu, __entry->qsevent) ); -- 2.27.0.111.gc72c7da667-goog ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] rcu/trace: Use gp_seq_req in acceleration's rcu_grace_period tracepoint 2020-06-19 1:36 [PATCH 1/3] rcu/trace: Print negative GP numbers correctly Joel Fernandes (Google) @ 2020-06-19 1:36 ` Joel Fernandes (Google) 2020-06-19 1:36 ` [PATCH 3/3] rcu/trace: Add name of the source for gp_seq to prevent confusion Joel Fernandes (Google) 1 sibling, 0 replies; 7+ messages in thread From: Joel Fernandes (Google) @ 2020-06-19 1:36 UTC (permalink / raw) To: linux-kernel Cc: Joel Fernandes (Google), Ingo Molnar, Josh Triplett, Lai Jiangshan, Madhuparna Bhowmik, Mathieu Desnoyers, Paul E. McKenney, rcu, Steven Rostedt, Uladzislau Rezki (Sony) During acceleration of CB, the rsp's gp_seq is rcu_seq_snap'd. This is the value used for acceleration - it is the value of gp_seq at which it is safe the execute all callbacks in the callback list. The rdp's gp_seq is not very useful for this scenario. Make rcu_grace_period report the gp_seq_req instead as it allows one to reason about how the acceleration works. Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> --- kernel/rcu/tree.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c index 9ef68dd249e1d..d0988a1c1079d 100644 --- a/kernel/rcu/tree.c +++ b/kernel/rcu/tree.c @@ -1464,9 +1464,10 @@ static bool rcu_accelerate_cbs(struct rcu_node *rnp, struct rcu_data *rdp) /* Trace depending on how much we were able to accelerate. */ if (rcu_segcblist_restempty(&rdp->cblist, RCU_WAIT_TAIL)) - trace_rcu_grace_period(rcu_state.name, rdp->gp_seq, TPS("AccWaitCB")); + trace_rcu_grace_period(rcu_state.name, gp_seq_req, TPS("AccWaitCB")); else - trace_rcu_grace_period(rcu_state.name, rdp->gp_seq, TPS("AccReadyCB")); + trace_rcu_grace_period(rcu_state.name, gp_seq_req, TPS("AccReadyCB")); + return ret; } -- 2.27.0.111.gc72c7da667-goog ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] rcu/trace: Add name of the source for gp_seq to prevent confusion 2020-06-19 1:36 [PATCH 1/3] rcu/trace: Print negative GP numbers correctly Joel Fernandes (Google) 2020-06-19 1:36 ` [PATCH 2/3] rcu/trace: Use gp_seq_req in acceleration's rcu_grace_period tracepoint Joel Fernandes (Google) @ 2020-06-19 1:36 ` Joel Fernandes (Google) 2020-06-19 2:07 ` Joel Fernandes 1 sibling, 1 reply; 7+ messages in thread From: Joel Fernandes (Google) @ 2020-06-19 1:36 UTC (permalink / raw) To: linux-kernel Cc: Joel Fernandes (Google), Ingo Molnar, Josh Triplett, Lai Jiangshan, Madhuparna Bhowmik, Mathieu Desnoyers, Paul E. McKenney, rcu, Steven Rostedt, Uladzislau Rezki (Sony) The gp_seq value can come from either of rdp, rsp or rnp. This makes the rcu_grace_period tracepoint extremely confusing since once if left constantly applying effort to reason about what a gp_seq means. Only the rsp is the global source of truth (most accurate GP info). The rnp can be off by ~1 and the rdp can be off by way more. Add some more context to traces to clarify where it comes from. While this can be resolved using some trace post-processing, such post- processing does need extra maintenance of scripts. Just make it easy by adding more context in the trace itself. Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> --- include/trace/events/rcu.h | 12 ++++++++---- kernel/rcu/tree.c | 32 ++++++++++++++++---------------- kernel/rcu/tree_plugin.h | 4 ++-- 3 files changed, 26 insertions(+), 22 deletions(-) diff --git a/include/trace/events/rcu.h b/include/trace/events/rcu.h index 155b5cb43cfd3..bf2d9f2e25f85 100644 --- a/include/trace/events/rcu.h +++ b/include/trace/events/rcu.h @@ -68,24 +68,28 @@ TRACE_EVENT(rcu_utilization, */ TRACE_EVENT_RCU(rcu_grace_period, - TP_PROTO(const char *rcuname, unsigned long gp_seq, const char *gpevent), + TP_PROTO(const char *rcuname, const char *gp_seq_src, + unsigned long gp_seq, const char *gpevent), - TP_ARGS(rcuname, gp_seq, gpevent), + TP_ARGS(rcuname, gp_seq_src, gp_seq, gpevent), TP_STRUCT__entry( __field(const char *, rcuname) + __field(const char *, gp_seq_src) __field(long, gp_seq) __field(const char *, gpevent) ), TP_fast_assign( __entry->rcuname = rcuname; + __entry->gp_seq_src = gp_seq_src; __entry->gp_seq = (long)gp_seq; __entry->gpevent = gpevent; ), - TP_printk("%s %ld %s", - __entry->rcuname, __entry->gp_seq, __entry->gpevent) + TP_printk("%s %s_gp_seq=%ld %s", + __entry->rcuname, __entry->gp_seq_src, + __entry->gp_seq, __entry->gpevent) ); /* diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c index d0988a1c1079d..b625811f529fa 100644 --- a/kernel/rcu/tree.c +++ b/kernel/rcu/tree.c @@ -1367,7 +1367,7 @@ static bool rcu_start_this_gp(struct rcu_node *rnp_start, struct rcu_data *rdp, trace_rcu_this_gp(rnp, rdp, gp_seq_req, TPS("NoGPkthread")); goto unlock_out; } - trace_rcu_grace_period(rcu_state.name, data_race(rcu_state.gp_seq), TPS("newreq")); + trace_rcu_grace_period(rcu_state.name, TPS("rsp"), data_race(rcu_state.gp_seq), TPS("newreq")); ret = true; /* Caller must wake GP kthread. */ unlock_out: /* Push furthest requested GP to leaf node and rcu_data structure. */ @@ -1464,9 +1464,9 @@ static bool rcu_accelerate_cbs(struct rcu_node *rnp, struct rcu_data *rdp) /* Trace depending on how much we were able to accelerate. */ if (rcu_segcblist_restempty(&rdp->cblist, RCU_WAIT_TAIL)) - trace_rcu_grace_period(rcu_state.name, gp_seq_req, TPS("AccWaitCB")); + trace_rcu_grace_period(rcu_state.name, TPS("req"), gp_seq_req, TPS("AccWaitCB")); else - trace_rcu_grace_period(rcu_state.name, gp_seq_req, TPS("AccReadyCB")); + trace_rcu_grace_period(rcu_state.name, TPS("req"), gp_seq_req, TPS("AccReadyCB")); return ret; } @@ -1566,7 +1566,7 @@ static bool __note_gp_changes(struct rcu_node *rnp, struct rcu_data *rdp) if (!offloaded) ret = rcu_advance_cbs(rnp, rdp); /* Advance CBs. */ rdp->core_needs_qs = false; - trace_rcu_grace_period(rcu_state.name, rdp->gp_seq, TPS("cpuend")); + trace_rcu_grace_period(rcu_state.name, TPS("rdp"), rdp->gp_seq, TPS("cpuend")); } else { if (!offloaded) ret = rcu_accelerate_cbs(rnp, rdp); /* Recent CBs. */ @@ -1582,7 +1582,7 @@ static bool __note_gp_changes(struct rcu_node *rnp, struct rcu_data *rdp) * set up to detect a quiescent state, otherwise don't * go looking for one. */ - trace_rcu_grace_period(rcu_state.name, rnp->gp_seq, TPS("cpustart")); + trace_rcu_grace_period(rcu_state.name, TPS("rnp"), rnp->gp_seq, TPS("cpustart")); need_qs = !!(rnp->qsmask & rdp->grpmask); rdp->cpu_no_qs.b.norm = need_qs; rdp->core_needs_qs = need_qs; @@ -1683,7 +1683,7 @@ static bool rcu_gp_init(void) /* Record GP times before starting GP, hence rcu_seq_start(). */ rcu_seq_start(&rcu_state.gp_seq); ASSERT_EXCLUSIVE_WRITER(rcu_state.gp_seq); - trace_rcu_grace_period(rcu_state.name, rcu_state.gp_seq, TPS("start")); + trace_rcu_grace_period(rcu_state.name, TPS("rsp"), rcu_state.gp_seq, TPS("start")); raw_spin_unlock_irq_rcu_node(rnp); /* @@ -1851,7 +1851,7 @@ static void rcu_gp_fqs_loop(void) WRITE_ONCE(rcu_state.jiffies_kick_kthreads, jiffies + (j ? 3 * j : 2)); } - trace_rcu_grace_period(rcu_state.name, rcu_state.gp_seq, + trace_rcu_grace_period(rcu_state.name, TPS("rsp"), rcu_state.gp_seq, TPS("fqswait")); rcu_state.gp_state = RCU_GP_WAIT_FQS; ret = swait_event_idle_timeout_exclusive( @@ -1866,7 +1866,7 @@ static void rcu_gp_fqs_loop(void) /* If time for quiescent-state forcing, do it. */ if (!time_after(rcu_state.jiffies_force_qs, jiffies) || (gf & RCU_GP_FLAG_FQS)) { - trace_rcu_grace_period(rcu_state.name, rcu_state.gp_seq, + trace_rcu_grace_period(rcu_state.name, TPS("rsp"), rcu_state.gp_seq, TPS("fqsstart")); rcu_gp_fqs(first_gp_fqs); gf = 0; @@ -1874,7 +1874,7 @@ static void rcu_gp_fqs_loop(void) first_gp_fqs = false; gf = rcu_state.cbovld ? RCU_GP_FLAG_OVLD : 0; } - trace_rcu_grace_period(rcu_state.name, rcu_state.gp_seq, + trace_rcu_grace_period(rcu_state.name, TPS("rsp"), rcu_state.gp_seq, TPS("fqsend")); cond_resched_tasks_rcu_qs(); WRITE_ONCE(rcu_state.gp_activity, jiffies); @@ -1885,7 +1885,7 @@ static void rcu_gp_fqs_loop(void) cond_resched_tasks_rcu_qs(); WRITE_ONCE(rcu_state.gp_activity, jiffies); WARN_ON(signal_pending(current)); - trace_rcu_grace_period(rcu_state.name, rcu_state.gp_seq, + trace_rcu_grace_period(rcu_state.name, TPS("rsp"), rcu_state.gp_seq, TPS("fqswaitsig")); ret = 1; /* Keep old FQS timing. */ j = jiffies; @@ -1968,7 +1968,7 @@ static void rcu_gp_cleanup(void) raw_spin_lock_irq_rcu_node(rnp); /* GP before ->gp_seq update. */ /* Declare grace period done, trace first to use old GP number. */ - trace_rcu_grace_period(rcu_state.name, rcu_state.gp_seq, TPS("end")); + trace_rcu_grace_period(rcu_state.name, TPS("rsp"), rcu_state.gp_seq, TPS("end")); rcu_seq_end(&rcu_state.gp_seq); ASSERT_EXCLUSIVE_WRITER(rcu_state.gp_seq); rcu_state.gp_state = RCU_GP_IDLE; @@ -1985,7 +1985,7 @@ static void rcu_gp_cleanup(void) if ((offloaded || !rcu_accelerate_cbs(rnp, rdp)) && needgp) { WRITE_ONCE(rcu_state.gp_flags, RCU_GP_FLAG_INIT); WRITE_ONCE(rcu_state.gp_req_activity, jiffies); - trace_rcu_grace_period(rcu_state.name, + trace_rcu_grace_period(rcu_state.name, TPS("rsp"), rcu_state.gp_seq, TPS("newreq")); } else { @@ -2005,7 +2005,7 @@ static int __noreturn rcu_gp_kthread(void *unused) /* Handle grace-period start. */ for (;;) { - trace_rcu_grace_period(rcu_state.name, rcu_state.gp_seq, + trace_rcu_grace_period(rcu_state.name, TPS("rsp"), rcu_state.gp_seq, TPS("reqwait")); rcu_state.gp_state = RCU_GP_WAIT_GPS; swait_event_idle_exclusive(rcu_state.gp_wq, @@ -2019,7 +2019,7 @@ static int __noreturn rcu_gp_kthread(void *unused) cond_resched_tasks_rcu_qs(); WRITE_ONCE(rcu_state.gp_activity, jiffies); WARN_ON(signal_pending(current)); - trace_rcu_grace_period(rcu_state.name, rcu_state.gp_seq, + trace_rcu_grace_period(rcu_state.name, TPS("rsp"), rcu_state.gp_seq, TPS("reqwaitsig")); } @@ -2263,7 +2263,7 @@ int rcutree_dying_cpu(unsigned int cpu) return 0; blkd = !!(rnp->qsmask & rdp->grpmask); - trace_rcu_grace_period(rcu_state.name, READ_ONCE(rnp->gp_seq), + trace_rcu_grace_period(rcu_state.name, TPS("rsp"), READ_ONCE(rnp->gp_seq), blkd ? TPS("cpuofl") : TPS("cpuofl-bgp")); return 0; } @@ -3875,7 +3875,7 @@ int rcutree_prepare_cpu(unsigned int cpu) rdp->core_needs_qs = false; rdp->rcu_iw_pending = false; rdp->rcu_iw_gp_seq = rdp->gp_seq - 1; - trace_rcu_grace_period(rcu_state.name, rdp->gp_seq, TPS("cpuonl")); + trace_rcu_grace_period(rcu_state.name, TPS("rdp"), rdp->gp_seq, TPS("cpuonl")); raw_spin_unlock_irqrestore_rcu_node(rnp, flags); rcu_prepare_kthreads(cpu); rcu_spawn_cpu_nocb_kthread(cpu); diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h index 982fc5be52698..7df8cd968f7fd 100644 --- a/kernel/rcu/tree_plugin.h +++ b/kernel/rcu/tree_plugin.h @@ -262,7 +262,7 @@ static void rcu_qs(void) { RCU_LOCKDEP_WARN(preemptible(), "rcu_qs() invoked with preemption enabled!!!\n"); if (__this_cpu_read(rcu_data.cpu_no_qs.s)) { - trace_rcu_grace_period(TPS("rcu_preempt"), + trace_rcu_grace_period(TPS("rcu_preempt"), TPS("rdp"), __this_cpu_read(rcu_data.gp_seq), TPS("cpuqs")); __this_cpu_write(rcu_data.cpu_no_qs.b.norm, false); @@ -788,7 +788,7 @@ static void rcu_qs(void) RCU_LOCKDEP_WARN(preemptible(), "rcu_qs() invoked with preemption enabled!!!"); if (!__this_cpu_read(rcu_data.cpu_no_qs.s)) return; - trace_rcu_grace_period(TPS("rcu_sched"), + trace_rcu_grace_period(TPS("rcu_sched"), TPS("rdp"), __this_cpu_read(rcu_data.gp_seq), TPS("cpuqs")); __this_cpu_write(rcu_data.cpu_no_qs.b.norm, false); if (!__this_cpu_read(rcu_data.cpu_no_qs.b.exp)) -- 2.27.0.111.gc72c7da667-goog ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] rcu/trace: Add name of the source for gp_seq to prevent confusion 2020-06-19 1:36 ` [PATCH 3/3] rcu/trace: Add name of the source for gp_seq to prevent confusion Joel Fernandes (Google) @ 2020-06-19 2:07 ` Joel Fernandes 2020-06-19 17:40 ` Paul E. McKenney 0 siblings, 1 reply; 7+ messages in thread From: Joel Fernandes @ 2020-06-19 2:07 UTC (permalink / raw) To: linux-kernel Cc: Ingo Molnar, Josh Triplett, Lai Jiangshan, Madhuparna Bhowmik, Mathieu Desnoyers, Paul E. McKenney, rcu, Steven Rostedt, Uladzislau Rezki (Sony) On Thu, Jun 18, 2020 at 09:36:41PM -0400, Joel Fernandes (Google) wrote: [...] > @@ -2019,7 +2019,7 @@ static int __noreturn rcu_gp_kthread(void *unused) > cond_resched_tasks_rcu_qs(); > WRITE_ONCE(rcu_state.gp_activity, jiffies); > WARN_ON(signal_pending(current)); > - trace_rcu_grace_period(rcu_state.name, rcu_state.gp_seq, > + trace_rcu_grace_period(rcu_state.name, TPS("rsp"), rcu_state.gp_seq, > TPS("reqwaitsig")); > } > > @@ -2263,7 +2263,7 @@ int rcutree_dying_cpu(unsigned int cpu) > return 0; > > blkd = !!(rnp->qsmask & rdp->grpmask); > - trace_rcu_grace_period(rcu_state.name, READ_ONCE(rnp->gp_seq), > + trace_rcu_grace_period(rcu_state.name, TPS("rsp"), READ_ONCE(rnp->gp_seq), This should be: TPS("rnp") :-( Happy to fix it up and resend if you'd like. Thanks! ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] rcu/trace: Add name of the source for gp_seq to prevent confusion 2020-06-19 2:07 ` Joel Fernandes @ 2020-06-19 17:40 ` Paul E. McKenney 2020-06-20 5:36 ` Joel Fernandes 0 siblings, 1 reply; 7+ messages in thread From: Paul E. McKenney @ 2020-06-19 17:40 UTC (permalink / raw) To: Joel Fernandes Cc: linux-kernel, Ingo Molnar, Josh Triplett, Lai Jiangshan, Madhuparna Bhowmik, Mathieu Desnoyers, rcu, Steven Rostedt, Uladzislau Rezki (Sony) On Thu, Jun 18, 2020 at 10:07:18PM -0400, Joel Fernandes wrote: > On Thu, Jun 18, 2020 at 09:36:41PM -0400, Joel Fernandes (Google) wrote: > [...] > > @@ -2019,7 +2019,7 @@ static int __noreturn rcu_gp_kthread(void *unused) > > cond_resched_tasks_rcu_qs(); > > WRITE_ONCE(rcu_state.gp_activity, jiffies); > > WARN_ON(signal_pending(current)); > > - trace_rcu_grace_period(rcu_state.name, rcu_state.gp_seq, > > + trace_rcu_grace_period(rcu_state.name, TPS("rsp"), rcu_state.gp_seq, > > TPS("reqwaitsig")); > > } > > > > @@ -2263,7 +2263,7 @@ int rcutree_dying_cpu(unsigned int cpu) > > return 0; > > > > blkd = !!(rnp->qsmask & rdp->grpmask); > > - trace_rcu_grace_period(rcu_state.name, READ_ONCE(rnp->gp_seq), > > + trace_rcu_grace_period(rcu_state.name, TPS("rsp"), READ_ONCE(rnp->gp_seq), > > This should be: TPS("rnp") :-( > > Happy to fix it up and resend if you'd like. Thanks! I queued and pushed 1/2 and 2/2, but again, I am still not at all convinced by 3/3. If you want to make RCU trace output human readable, post-processing will be needed. Thanx, Paul ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] rcu/trace: Add name of the source for gp_seq to prevent confusion 2020-06-19 17:40 ` Paul E. McKenney @ 2020-06-20 5:36 ` Joel Fernandes 2020-06-20 15:45 ` Paul E. McKenney 0 siblings, 1 reply; 7+ messages in thread From: Joel Fernandes @ 2020-06-20 5:36 UTC (permalink / raw) To: Paul E. McKenney Cc: linux-kernel, Ingo Molnar, Josh Triplett, Lai Jiangshan, Madhuparna Bhowmik, Mathieu Desnoyers, rcu, Steven Rostedt, Uladzislau Rezki (Sony) On Fri, Jun 19, 2020 at 10:40:01AM -0700, Paul E. McKenney wrote: > On Thu, Jun 18, 2020 at 10:07:18PM -0400, Joel Fernandes wrote: > > On Thu, Jun 18, 2020 at 09:36:41PM -0400, Joel Fernandes (Google) wrote: > > [...] > > > @@ -2019,7 +2019,7 @@ static int __noreturn rcu_gp_kthread(void *unused) > > > cond_resched_tasks_rcu_qs(); > > > WRITE_ONCE(rcu_state.gp_activity, jiffies); > > > WARN_ON(signal_pending(current)); > > > - trace_rcu_grace_period(rcu_state.name, rcu_state.gp_seq, > > > + trace_rcu_grace_period(rcu_state.name, TPS("rsp"), rcu_state.gp_seq, > > > TPS("reqwaitsig")); > > > } > > > > > > @@ -2263,7 +2263,7 @@ int rcutree_dying_cpu(unsigned int cpu) > > > return 0; > > > > > > blkd = !!(rnp->qsmask & rdp->grpmask); > > > - trace_rcu_grace_period(rcu_state.name, READ_ONCE(rnp->gp_seq), > > > + trace_rcu_grace_period(rcu_state.name, TPS("rsp"), READ_ONCE(rnp->gp_seq), > > > > This should be: TPS("rnp") :-( > > > > Happy to fix it up and resend if you'd like. Thanks! > > I queued and pushed 1/2 and 2/2. Thanks! > but again, I am still not at all > convinced by 3/3. If you want to make RCU trace output human > readable, post-processing will be needed. Or I could post-process the code before building it since the pattern seems easy to parse ;-) - Joel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] rcu/trace: Add name of the source for gp_seq to prevent confusion 2020-06-20 5:36 ` Joel Fernandes @ 2020-06-20 15:45 ` Paul E. McKenney 0 siblings, 0 replies; 7+ messages in thread From: Paul E. McKenney @ 2020-06-20 15:45 UTC (permalink / raw) To: Joel Fernandes Cc: linux-kernel, Ingo Molnar, Josh Triplett, Lai Jiangshan, Madhuparna Bhowmik, Mathieu Desnoyers, rcu, Steven Rostedt, Uladzislau Rezki (Sony) On Sat, Jun 20, 2020 at 01:36:08AM -0400, Joel Fernandes wrote: > On Fri, Jun 19, 2020 at 10:40:01AM -0700, Paul E. McKenney wrote: > > On Thu, Jun 18, 2020 at 10:07:18PM -0400, Joel Fernandes wrote: > > > On Thu, Jun 18, 2020 at 09:36:41PM -0400, Joel Fernandes (Google) wrote: > > > [...] > > > > @@ -2019,7 +2019,7 @@ static int __noreturn rcu_gp_kthread(void *unused) > > > > cond_resched_tasks_rcu_qs(); > > > > WRITE_ONCE(rcu_state.gp_activity, jiffies); > > > > WARN_ON(signal_pending(current)); > > > > - trace_rcu_grace_period(rcu_state.name, rcu_state.gp_seq, > > > > + trace_rcu_grace_period(rcu_state.name, TPS("rsp"), rcu_state.gp_seq, > > > > TPS("reqwaitsig")); > > > > } > > > > > > > > @@ -2263,7 +2263,7 @@ int rcutree_dying_cpu(unsigned int cpu) > > > > return 0; > > > > > > > > blkd = !!(rnp->qsmask & rdp->grpmask); > > > > - trace_rcu_grace_period(rcu_state.name, READ_ONCE(rnp->gp_seq), > > > > + trace_rcu_grace_period(rcu_state.name, TPS("rsp"), READ_ONCE(rnp->gp_seq), > > > > > > This should be: TPS("rnp") :-( > > > > > > Happy to fix it up and resend if you'd like. Thanks! > > > > I queued and pushed 1/2 and 2/2. > > Thanks! > > > but again, I am still not at all > > convinced by 3/3. If you want to make RCU trace output human > > readable, post-processing will be needed. > > Or I could post-process the code before building it since the pattern seems > easy to parse ;-) For this one thing, perhaps. For most other information of interest, doing so in-kernel would not be so good, for example, from a lock-contention viewpoint. Thanx, Paul ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-06-20 15:45 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-06-19 1:36 [PATCH 1/3] rcu/trace: Print negative GP numbers correctly Joel Fernandes (Google) 2020-06-19 1:36 ` [PATCH 2/3] rcu/trace: Use gp_seq_req in acceleration's rcu_grace_period tracepoint Joel Fernandes (Google) 2020-06-19 1:36 ` [PATCH 3/3] rcu/trace: Add name of the source for gp_seq to prevent confusion Joel Fernandes (Google) 2020-06-19 2:07 ` Joel Fernandes 2020-06-19 17:40 ` Paul E. McKenney 2020-06-20 5:36 ` Joel Fernandes 2020-06-20 15:45 ` 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).