Messages in this thread |  | | | Date | Thu, 26 Feb 2009 16:35:33 +0800 | | From | Li Zefan <> | | Subject | Re: [PATCH] cpuacct: add a branch prediction |
| |
KAMEZAWA Hiroyuki wrote: > On Thu, 26 Feb 2009 16:17:31 +0800 > Li Zefan <lizf@cn.fujitsu.com> wrote: > >> KAMEZAWA Hiroyuki wrote: >>> On Thu, 26 Feb 2009 15:40:15 +0800 >>> Li Zefan <lizf@cn.fujitsu.com> wrote: >>> >>>> cpuacct_charge() is in fast-path, and checking of !cpuacct_susys.active >>>> always returns false after cpuacct has been initialized at system boot. >>>> >>>> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com> >>>> --- >>>> kernel/sched.c | 2 +- >>>> 1 files changed, 1 insertions(+), 1 deletions(-) >>>> >>>> diff --git a/kernel/sched.c b/kernel/sched.c >>>> index 410eec4..fd2f7fc 100644 >>>> --- a/kernel/sched.c >>>> +++ b/kernel/sched.c >>>> @@ -9589,7 +9589,7 @@ static void cpuacct_charge(struct task_struct *tsk, u64 cputime) >>>> struct cpuacct *ca; >>>> int cpu; >>>> >>>> - if (!cpuacct_subsys.active) >>>> + if (unlikely(!cpuacct_subsys.active)) >>>> return; >>>> >>> (Just curious) >>> I wonder "ca = task_ca(tsk)" will return NULL if cpuacct subsys is not initalized. >> Yes, it will be NULL, and that's why we need this check. >> >>> Then, can we just remove this check ? >>> >> cpuacct_charge() can be called before cpuacct is initialized, so we have to check this >> case here. >> > My point is, >
Ah, I see.
> ca = task_ca(tsk) > for (; ca; ca->parent) { > ... > } >
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 { ... } while (ca->parent); > What is problem even if ca is NULL. >
|  |