Re: [PATCH] cpuacct: add a branch prediction

From: KAMEZAWA Hiroyuki
Date: Thu Feb 26 2009 - 05:29:11 EST


Bharata B Rao:
> On Thu, Feb 26, 2009 at 2:10 PM, KAMEZAWA Hiroyuki
> <kamezawa.hiroyu@xxxxxxxxxxxxxx> wrote:
>> On Thu, 26 Feb 2009 16:35:33 +0800
>> Li Zefan <lizf@xxxxxxxxxxxxxx> wrote:
>>
>>> ca is not checked before hierarchy support, and it's a side-effect.
>>>
>>> Before cpuacct is initialized, css ==
>>> task->cgroups->subsys[cpuacct_subsys] == NULL,
>>> but ca = task_ca(tsk) is not necessarily NULL, unless struct
>>> cgroup_subsys_state is the
>>> first member of struct cpuacct.
>>>
>>> And the above code actually should be:
>>>
>>> do {
>>> &#160; &#160; &#160; ...
>>> } while (ca->parent);
>>>
>> I'll send no more objections to this patch itself.
>>
>> But IMHO, this loop is tooo bad, I think. The hierarchy information
>> should be gathered by
>> read-side and the total code should be
>
> How do you take care of accounting when a group gets deleted ?.
propagate accounts to the parent or root group if deleted.

> Wouldn't you loose some information due to deleted groups if you want
> to accumulate hierarchical stats only during read ?
>
Taking hierarchy mutex while reading will make read-side stable.
I haven't considered in detail but I wrote CSS ID and scan hierarchy
code to do this kind of "visit all css object under hierarchy" jobs.
I think you can catch what I mean by reading what memory.stat file of mem
cgroup does. (mmotm)

Thanks,
-Kame


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/