linux-kernel - RE: [PATCH v3 3/3] cpuacct: split usage into user_usage and sys

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 17 Mar 2016 17:42:48 +0800
From:	Zhao Lei <zhaolei@...fujitsu.com>
To:	'Peter Zijlstra' <peterz@...radead.org>
CC:	<linux-kernel@...r.kernel.org>, 'Tejun Heo' <htejun@...il.com>,
	'Yang Dongsheng' <yangds.fnst@...fujitsu.com>
Subject: RE: [PATCH v3 3/3] cpuacct: split usage into user_usage and sys_usage

Hi, Peter Zijlstra

> -----Original Message-----
> From: Peter Zijlstra [mailto:peterz@...radead.org]
> Sent: Thursday, March 17, 2016 4:40 PM
> To: Zhao Lei <zhaolei@...fujitsu.com>
> Cc: linux-kernel@...r.kernel.org; Tejun Heo <htejun@...il.com>; Yang
> Dongsheng <yangds.fnst@...fujitsu.com>
> 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