Hi, Peter Zijlstra > -----Original Message----- > From: Peter Zijlstra [mailto:[email protected]] > Sent: Thursday, March 17, 2016 4:40 PM > To: Zhao Lei <[email protected]> > Cc: [email protected]; Tejun Heo <[email protected]>; Yang > Dongsheng <[email protected]> > 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
