Messages in this thread | | | Date | Fri, 31 Oct 2008 09:40:41 +0900 | From | KAMEZAWA Hiroyuki <> | Subject | Re: [PATCH] Add hierarchical accounting to cpu accounting controller |
| |
On Thu, 30 Oct 2008 22:46:22 +0530 Dhaval Giani <dhaval@linux.vnet.ibm.com> wrote:
> On Mon, Oct 27, 2008 at 04:25:01PM +0800, Li Zefan wrote: > > >>>> So in technical terms this patch looks fine now. There's still the > > >>>> question of whether it's OK to change the existing API, since it's > > >>>> been in the kernel in its currently (non-hierarchical) form for > > >>>> several releases now. > > >> Hmm... Can we consider this as an API change ? Currently cpuacct.usage > > >> readers of a parent accounting group are missing the usage contributions > > >> from its children groups. I would consider this patch as fixing the > > >> above problem by correctly reflecting the cpu usage for every accounting > > >> group. > > >> > > > > > > If a particular application desires to derive the usage of its > > > immediate tasks and does not care about subcgroups, it is a simple > > > iteration (after this fix) > > > > > > cpuacct - sigma(cpuacct_child) > > > > > > and currently if we cared about child accounting, we could do > > > > > > cpuacct + recursively(sigma(cpuacct_child)) > > > > > > In that sense this fix makes more sense, but like Paul said we need to > > > figure out if it is an API change. My take is that it is a BUG fix, > > > since we do care about child subgroups in accounting. > > > > > > > cpuacct was designed to count cpu usage of a group of tasks, and now some people > > want it to also take child group's usage into account, so I think this is a feature > > request but not a bug fix. > > > > I disagree. The child is a part of the parent's hierarchy, and therefore > its usage should reflect in the parent's usage. >
In my point of view, there is no big difference. It's just whether we need a tool in userland or not. If there is no performance impact, I have no objections.
One request from me is add Documentation/controllers/cpuacct.txt or some to explain "what we see".
Thanks, -Kame
| |