Ok, I missed the other comments in this patch.
Here it goes:
On 11/16/2011 09:25 AM, Glauber Costa wrote:
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, get_idle_time(i));
- iowait = cputime64_add(iowait, get_iowait_time(i));
- 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);
- guest_nice = cputime64_add(guest_nice,
- kstat_cpu(i).cpustat.guest_nice);
- sum += kstat_cpu_irqs_sum(i);
- sum += arch_irq_stat_cpu(i);
+ user += kstat_cpu(i).cpustat[USER];
Half the time cputime64_add is preserved, half the time this patch
converts it to a naked '+='. Admittedly no one seems to usefully define
cputime64_add but why the conversion / inconsistency?
Because at least in this patchset of mine, cputime conversion is not a
goal, but a side effect. I wanted cpustat to be an array of u64, so I
converted users. There are some places in the code that still uses
cputime64 for other variables, and those I kept untouched.
That's to make the patch focused. The other variables can be easily
converted if we see value on it.
Outside of sched.c I did miss some. Those I can chase and convert.
+enum cpu_usage_stat {
+ USER,
+ NICE,
+ SYSTEM,
+ SOFTIRQ,
+ IRQ,
+ IDLE,
+ IOWAIT,
+ STEAL,
+ GUEST,
+ GUEST_NICE,
+ NR_STATS,
};
I suspect we want a more descriptive prefix here, e.g. CPUTIME_USER
Ok.
/* Add user time to cpustat. */
tmp = cputime_to_cputime64(cputime);
+
if (TASK_NICE(p)> 0)
We now that these are actually fields this could be:
field = TASK_NICE(p) > 0 ? CPUTIME_NICE : CPUTIME_USER;
Yes, absolutely.
@@ -3925,9 +3926,9 @@ static void account_guest_time(struct
task_struct *p, cputime_t cputime,
*/
static inline
void __account_system_time(struct task_struct *p, cputime_t cputime,
- cputime_t cputime_scaled, cputime64_t *target_cputime64)
+ cputime_t cputime_scaled, u64 *target_cputime64)
Having cpustat be an array means we can drop the pointer here and pass
the id.
And that's precisely what I do on a later patch, with account_field. I
can move that code here if you prefer.
--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html