Messages in this thread | | | Subject | Re: [GIT PULL] cputime patch for 2.6.30-rc6 | From | Peter Zijlstra <> | Date | Tue, 19 May 2009 11:31:28 +0200 |
| |
On Tue, 2009-05-19 at 11:00 +0200, Martin Schwidefsky wrote: > On Mon, 18 May 2009 17:28:53 +0100 (BST) > Michael Abbott <michael@araneidae.co.uk> wrote: > > > > > + for_each_possible_cpu(i) > > > > + idletime = cputime64_add(idletime, kstat_cpu(i).cpustat.idle); > > > > + idletime = cputime64_to_clock_t(idletime); > > > > > > > > do_posix_clock_monotonic_gettime(&uptime); > > > > monotonic_to_bootbased(&uptime); > > > > > > This is a world readable proc file, adding a for_each_possible_cpu() in > > > there scares me a little (this wouldn't be the first and only such case > > > though). > > > > > > Suppose you have lots of cpus, and all those cpus are dirtying those > > > cachelines (who's updating idle time when they're idle?), then this loop > > > can cause a massive cacheline bounce fest. > > > > > > Then think about userspace doing: > > > while :; do cat /proc/uptime > /dev/null; done > > > > Well, the offending code derives pretty well directly from /proc/stat, > > which is used, for example, by top. So if there is an issue then I guess > > it already exists. > > > > There is a pending problem in this code: for a multiple cpu system we'll > > end up with more idle time than elapsed time, which is not really very > > nice. Unfortunately *something* has to be done here, as it looks as if > > .utime and .stime (at least for init_task) have lost any meaning. I sort > > of though of dividing by number of cpus, but that's not going to work very > > well.. > > 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.
| |