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

[Zhao Lei]

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