Messages in this thread | | | From | Zhao Lei <> | Subject | RE: [PATCH v3 3/3] cpuacct: split usage into user_usage and sys_usage | Date | Thu, 17 Mar 2016 17:42:48 +0800 |
| |
Hi, Peter Zijlstra
> -----Original Message----- > From: Peter Zijlstra [mailto:peterz@infradead.org] > Sent: Thursday, March 17, 2016 4:40 PM > To: Zhao Lei <zhaolei@cn.fujitsu.com> > Cc: linux-kernel@vger.kernel.org; Tejun Heo <htejun@gmail.com>; Yang > Dongsheng <yangds.fnst@cn.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
| |