Re: [PATCH 2/7] rcu: fix tracepoint string when RCU CPU kthread runs

[Lai Jiangshan <laijs@linux.alibaba.com>]

On 2019/10/16 11:38 上午, Paul E. McKenney wrote:
> On Tue, Oct 15, 2019 at 10:23:57AM +0000, Lai Jiangshan wrote:
>> "rcu_wait" is incorrct here, use "rcu_run" instead.
>>
>> Signed-off-by: Lai Jiangshan <jiangshanlai@gmail.com>
>> Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
>> ---
>>   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
>>