RE: [PATCH v3 3/3] cpuacct: split usage into user_usage and sys_usage

From: Zhao Lei
Date: Thu Mar 17 2016 - 05:43:05 EST


Hi, Peter Zijlstra

> -----Original Message-----
> From: Peter Zijlstra [mailto:peterz@xxxxxxxxxxxxx]
> Sent: Thursday, March 17, 2016 4:40 PM
> To: Zhao Lei <zhaolei@xxxxxxxxxxxxxx>
> Cc: linux-kernel@xxxxxxxxxxxxxxx; Tejun Heo <htejun@xxxxxxxxx>; Yang
> Dongsheng <yangds.fnst@xxxxxxxxxxxxxx>
> Subject: Re: [PATCH v3 3/3] cpuacct: split usage into user_usage and sys_usage
>
> On Thu, Mar 17, 2016 at 12:19:44PM +0800, Zhao Lei wrote:
> > +static u64 __cpuusage_read(struct cgroup_subsys_state *css,
> > + enum cpuacct_usage_index index)
> > {
> > struct cpuacct *ca = css_ca(css);
> > u64 totalcpuusage = 0;
> > int i;
> >
> > for_each_present_cpu(i)
> > + totalcpuusage += cpuacct_cpuusage_read(ca, i, index);
> >
> > return totalcpuusage;
> > }
>
> Ok, so while looking over this, it mostly uses for_each_present_cpu(),
> which is already dubious, but then cpuacct_stats_show() uses
> for_each_online_cpu().
>
> Why is this? Why not always for_each_possible_cpu()?
>
> Surely, if you offline a cpu, you still want its stat to be included in
> your totals, otherwise your numbers will go backwards when you take a
> cpu offline.
>
I agree with you that for_each_possible_cpu() is best choice for above
code.

In corrent code,

1: cpuacct.usage_percpu only include present_cpus
If a cpu is hotplug out, it is not exist in above file.
2: cpuacct.usage only calculate present_cpus
If a cpu is hotplug out, this value maybe decreased.
3: cpuacct.stat only calculate online cpus
Obviously wrong.

Above 3 is easy to fix, but better to fix above 1 and 2 together,
in one word, to make ALL statics counts possible_cpu.

The problem is file output, currently,
# cat cpuacct.usage_percpu
689076136 1131883300
If we turn to use possible_cpu, above line will have
256 valuse, as:
# cat cpuacct.usage_percpu
689076136 1131883300 0 0 0 0 0 0 0 0 ...

Or we can only show present_cpu and non_present_cpu with !0 value,
and we need also need output a cpuindex, as:
# cat cpuacct.usage_percpu
[0] 689076136
[1] 1131883300
[3] 11111111
[50] 22222222
#

It will tell user more accurate information,
but both solution will change current cgroup interface.

So I suggest keeping current using of for_each_present_cpu,
and only modify for_each_online_cpu.

What is your opinion?

Thanks
Zhaolei