linux-kernel - Re: [GIT PULL] cputime patch for 2.6.30-rc6

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]

Message-ID: <20090520100912.4f9c2037@skybase>
Date:	Wed, 20 May 2009 10:09:12 +0200
From:	Martin Schwidefsky <schwidefsky@...ibm.com>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	Michael Abbott <michael@...neidae.co.uk>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Jan Engelhardt <jengelh@...ozas.de>
Subject: Re: [GIT PULL] cputime patch for 2.6.30-rc6

On Tue, 19 May 2009 11:31:28 +0200
Peter Zijlstra <peterz@...radead.org> wrote:

> On Tue, 2009-05-19 at 11:00 +0200, Martin Schwidefsky wrote:
> > I don't see a problem here. In an idle multiple cpu system there IS
> > more idle time than elapsed time. What would makes sense is to compare
> > elapsed time * #cpus with the idle time. But then there is cpu hotplug
> > which forces you to look at the delta of two measuring points where the
> > number of cpus did not change.
> 
> Sure, this one case isn't that bad, esp. as you note its about idle
> time. However, see for example /proc/stat and fs/proc/stat.c:
> 
>         for_each_possible_cpu(i) {
>                 user = cputime64_add(user, kstat_cpu(i).cpustat.user);
>                 nice = cputime64_add(nice, kstat_cpu(i).cpustat.nice);
>                 system = cputime64_add(system, kstat_cpu(i).cpustat.system);
>                 idle = cputime64_add(idle, kstat_cpu(i).cpustat.idle);
>                 idle = cputime64_add(idle, arch_idle_time(i));
>                 iowait = cputime64_add(iowait, kstat_cpu(i).cpustat.iowait);
>                 irq = cputime64_add(irq, kstat_cpu(i).cpustat.irq);
>                 softirq = cputime64_add(softirq, kstat_cpu(i).cpustat.softirq);
>                 steal = cputime64_add(steal, kstat_cpu(i).cpustat.steal);
>                 guest = cputime64_add(guest, kstat_cpu(i).cpustat.guest);
>                 for_each_irq_nr(j) {
>                         sum += kstat_irqs_cpu(j, i);
>                 }
>                 sum += arch_irq_stat_cpu(i);
>         }
> 
> If that isn't a problem on a large machine, then I don't know what is.

Well, we better distinguish between the semantical problem and the
performance consideration, no? One thing is what the proc field is
supposed to contain, the other is how fast you can do it.

I have been refering to the semantical problem, but your point with the
performance is very valid as well. So
1) are we agreed that the second field of /proc/uptime should contain
the aggregate idle time of all cpus?
2) I agree that an endless loop of 'cat /proc/uptime' or
'cat /proc/stat' can have negative performance impact on a system with
many cpus. One way to deal with it would be to restrict access to the
interface. I do not like that idea to much. Another way would be to
limit the number of for_each_possible_cpu loops per second. Create
global variables that contain the aggregate values for the different
fields. If the last update has been too recent (e.g. less than 0.1
seconds ago), just print the old values again.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

--
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/