linux-kernel - Re: [PATCH] fix idle ticks in cpu summary line of /proc/stat

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 for Android: free password hash cracker in your pocket

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

Date:	Mon, 12 Mar 2012 20:18:33 +0530
From:	"Srivatsa S. Bhat" <srivatsa.bhat@...ux.vnet.ibm.com>
To:	Michal Hocko <mhocko@...e.cz>
CC:	Martin Schwidefsky <schwidefsky@...ibm.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] fix idle ticks in cpu summary line of /proc/stat

On 03/12/2012 07:47 PM, Martin Schwidefsky wrote:

> On Mon, 12 Mar 2012 13:17:26 +0100
> Michal Hocko <mhocko@...e.cz> wrote:
> 
>> Goot catch. But I think that the following fix should be better because
>> it doesn't change the semantic of the function. What do you think?
> ..
>> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
>> index 7656642..dec767f 100644
>> --- a/kernel/time/tick-sched.c
>> +++ b/kernel/time/tick-sched.c
>> @@ -221,7 +221,7 @@ u64 get_cpu_idle_time_us(int cpu, u64 *last_update_time)
>>  		update_ts_time_stats(cpu, ts, now, last_update_time);
>>  		idle = ts->idle_sleeptime;
>>  	} else {
>> -		if (ts->idle_active && !nr_iowait_cpu(cpu)) {
>> +		if (cpu_online(cpu) && ts->idle_active && !nr_iowait_cpu(cpu)) {
>>  			ktime_t delta = ktime_sub(now, ts->idle_entrytime);
>>
>>  			idle = ktime_add(ts->idle_sleeptime, delta);
>> @@ -262,7 +262,7 @@ u64 get_cpu_iowait_time_us(int cpu, u64 *last_update_time)
>>  		update_ts_time_stats(cpu, ts, now, last_update_time);
>>  		iowait = ts->iowait_sleeptime;
>>  	} else {
>> -		if (ts->idle_active && nr_iowait_cpu(cpu) > 0) {
>> +		if (cpu_online(cpu) && ts->idle_active && nr_iowait_cpu(cpu) > 0) {
>>  			ktime_t delta = ktime_sub(now, ts->idle_entrytime);
>>
>>  			iowait = ktime_add(ts->iowait_sleeptime, delta);
> 
> I would prefer an early exit from the functions. The target cpu is offline,
> who guarantees that the "struct tick_sched" for the cpu contains anything
> useful?
> 


Also, what about the case where last_update_time is non-NULL?
With Martin's patch update_ts_time_stats() won't be called for offline cpus,
whereas with Michal's patch it will be called and hence the counters will get
updated.. We don't want to update counters for offline cpus right?

Regards,
Srivatsa S. Bhat

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/