Re: [PATCH 3/3] rcu/trace: Add name of the source for gp_seq to prevent confusion
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
Re: [PATCH 3/3] rcu/trace: Add name of the source for gp_seq to prevent confusion
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
Re: [PATCH 3/3] rcu/trace: Add name of the source for gp_seq to prevent confusion
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
Re: [PATCH 3/3] rcu/trace: Add name of the source for gp_seq to prevent confusion
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!