Re: [PATCH 2/7] rcu: fix tracepoint string when RCU CPU kthread runs
On Wed, Oct 16, 2019 at 12:24:09PM +0800, Lai Jiangshan wrote:
>
>
> On 2019/10/16 11:38 上午, Paul E. McKenney wrote:
> > On Tue, Oct 15, 2019 at 10:23:57AM +, Lai Jiangshan wrote:
> > > "rcu_wait" is incorrct here, use "rcu_run" instead.
> > >
> > > Signed-off-by: Lai Jiangshan
> > > Signed-off-by: Lai Jiangshan
> > > ---
> > > kernel/rcu/tree.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > > index 278798e58698..c351fc280945 100644
> > > --- a/kernel/rcu/tree.c
> > > +++ b/kernel/rcu/tree.c
> > > @@ -2485,7 +2485,7 @@ static void rcu_cpu_kthread(unsigned int cpu)
> > > int spincnt;
> > > for (spincnt = 0; spincnt < 10; spincnt++) {
> > > - trace_rcu_utilization(TPS("Start CPU kthread@rcu_wait"));
> > > + trace_rcu_utilization(TPS("Start CPU kthread@rcu_run"));
> > > local_bh_disable();
> > > *statusp = RCU_KTHREAD_RUNNING;
> > > local_irq_disable();
> > > @@ -2496,7 +2496,7 @@ static void rcu_cpu_kthread(unsigned int cpu)
> > > rcu_core();
> > > local_bh_enable();
> > > if (*workp == 0) {
> > > - trace_rcu_utilization(TPS("End CPU kthread@rcu_wait"));
> > > + trace_rcu_utilization(TPS("End CPU kthread@rcu_run"));
> >
> > This one needs to stay as it was because this is where we wait when out
> > of work.
>
> I don't fully understand those TPS marks.
>
> If it is all about "where we wait when out of work", it ought to
> be "Start ... wait", rather than "End ... wait". The later one
> ("End ... wait") should be put before
> "for (spincnt = 0; spincnt < 10; spincnt++)" and remove
> the whole "rcu_run" as this patch suggested. To be honest,
> "rcu_run" is redundant since we already has TPS("Start RCU core").
>
> Any ways, patch2&3 lose their relevance and should be dropped.
> Looking forward to your improved version.
Given that most of RCU's overhead is now in kthreads and in RCU_SOFTIRQ,
perhaps trace_rcu_utilization() has outlived its usefulness, especially
given the prospect of an RCU_SOFTIRQ-specific kthread.
Thanx, Paul
Re: [PATCH 2/7] rcu: fix tracepoint string when RCU CPU kthread runs
On 2019/10/16 11:38 上午, Paul E. McKenney wrote:
On Tue, Oct 15, 2019 at 10:23:57AM +, Lai Jiangshan wrote:
"rcu_wait" is incorrct here, use "rcu_run" instead.
Signed-off-by: Lai Jiangshan
Signed-off-by: Lai Jiangshan
---
kernel/rcu/tree.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 278798e58698..c351fc280945 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -2485,7 +2485,7 @@ static void rcu_cpu_kthread(unsigned int cpu)
int spincnt;
for (spincnt = 0; spincnt < 10; spincnt++) {
- trace_rcu_utilization(TPS("Start CPU kthread@rcu_wait"));
+ trace_rcu_utilization(TPS("Start CPU kthread@rcu_run"));
local_bh_disable();
*statusp = RCU_KTHREAD_RUNNING;
local_irq_disable();
@@ -2496,7 +2496,7 @@ static void rcu_cpu_kthread(unsigned int cpu)
rcu_core();
local_bh_enable();
if (*workp == 0) {
- trace_rcu_utilization(TPS("End CPU kthread@rcu_wait"));
+ trace_rcu_utilization(TPS("End CPU kthread@rcu_run"));
This one needs to stay as it was because this is where we wait when out
of work.
I don't fully understand those TPS marks.
If it is all about "where we wait when out of work", it ought to
be "Start ... wait", rather than "End ... wait". The later one
("End ... wait") should be put before
"for (spincnt = 0; spincnt < 10; spincnt++)" and remove
the whole "rcu_run" as this patch suggested. To be honest,
"rcu_run" is redundant since we already has TPS("Start RCU core").
Any ways, patch2&3 lose their relevance and should be dropped.
Looking forward to your improved version.
Thanks,
Lai
So I took the first hunk and dropped this second hunk.
Please let me know if I am missing something.
Thanx, Paul
*statusp = RCU_KTHREAD_WAITING;
return;
}
--
2.20.1
Re: [PATCH 2/7] rcu: fix tracepoint string when RCU CPU kthread runs
On Tue, Oct 15, 2019 at 10:23:57AM +, Lai Jiangshan wrote:
> "rcu_wait" is incorrct here, use "rcu_run" instead.
>
> Signed-off-by: Lai Jiangshan
> Signed-off-by: Lai Jiangshan
> ---
> kernel/rcu/tree.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 278798e58698..c351fc280945 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -2485,7 +2485,7 @@ static void rcu_cpu_kthread(unsigned int cpu)
> int spincnt;
>
> for (spincnt = 0; spincnt < 10; spincnt++) {
> - trace_rcu_utilization(TPS("Start CPU kthread@rcu_wait"));
> + trace_rcu_utilization(TPS("Start CPU kthread@rcu_run"));
> local_bh_disable();
> *statusp = RCU_KTHREAD_RUNNING;
> local_irq_disable();
> @@ -2496,7 +2496,7 @@ static void rcu_cpu_kthread(unsigned int cpu)
> rcu_core();
> local_bh_enable();
> if (*workp == 0) {
> - trace_rcu_utilization(TPS("End CPU kthread@rcu_wait"));
> + trace_rcu_utilization(TPS("End CPU kthread@rcu_run"));
This one needs to stay as it was because this is where we wait when out
of work.
So I took the first hunk and dropped this second hunk.
Please let me know if I am missing something.
Thanx, Paul
> *statusp = RCU_KTHREAD_WAITING;
> return;
> }
> --
> 2.20.1
>
[PATCH 2/7] rcu: fix tracepoint string when RCU CPU kthread runs
"rcu_wait" is incorrct here, use "rcu_run" instead.
Signed-off-by: Lai Jiangshan
Signed-off-by: Lai Jiangshan
---
kernel/rcu/tree.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 278798e58698..c351fc280945 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -2485,7 +2485,7 @@ static void rcu_cpu_kthread(unsigned int cpu)
int spincnt;
for (spincnt = 0; spincnt < 10; spincnt++) {
- trace_rcu_utilization(TPS("Start CPU kthread@rcu_wait"));
+ trace_rcu_utilization(TPS("Start CPU kthread@rcu_run"));
local_bh_disable();
*statusp = RCU_KTHREAD_RUNNING;
local_irq_disable();
@@ -2496,7 +2496,7 @@ static void rcu_cpu_kthread(unsigned int cpu)
rcu_core();
local_bh_enable();
if (*workp == 0) {
- trace_rcu_utilization(TPS("End CPU kthread@rcu_wait"));
+ trace_rcu_utilization(TPS("End CPU kthread@rcu_run"));
*statusp = RCU_KTHREAD_WAITING;
return;
}
--
2.20.1
