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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c54063d6-c6d0-cd8c-40e3-5185258d71dd@linux.alibaba.com>
Date:   Wed, 16 Oct 2019 12:24:09 +0800
From:   Lai Jiangshan <laijs@...ux.alibaba.com>
To:     paulmck@...nel.org
Cc:     linux-kernel@...r.kernel.org,
        Lai Jiangshan <jiangshanlai@...il.com>,
        Josh Triplett <josh@...htriplett.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        Joel Fernandes <joel@...lfernandes.org>, rcu@...r.kernel.org
Subject: 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 +0000, Lai Jiangshan wrote:
>> "rcu_wait" is incorrct here, use "rcu_run" instead.
>>
>> Signed-off-by: Lai Jiangshan <jiangshanlai@...il.com>
>> Signed-off-by: Lai Jiangshan <laijs@...ux.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@..._wait"));
>> +		trace_rcu_utilization(TPS("Start CPU kthread@..._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@..._wait"));
>> +			trace_rcu_utilization(TPS("End CPU kthread@..._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
>>