[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1317818294.6766.16.camel@twins>
Date: Wed, 05 Oct 2011 14:38:14 +0200
From: Peter Zijlstra <a.p.zijlstra@...llo.nl>
To: Glauber Costa <glommer@...allels.com>
Cc: linux-kernel@...r.kernel.org, paul@...lmenage.org,
lizf@...fujitsu.com, daniel.lezcano@...e.fr,
jbottomley@...allels.com
Subject: Re: [PATCH 04/10] Display /proc/stat information per cgroup
On Wed, 2011-10-05 at 16:10 +0400, Glauber Costa wrote:
> On 10/05/2011 12:56 PM, Peter Zijlstra wrote:
> > On Sun, 2011-10-02 at 23:21 +0400, Glauber Costa wrote:
> >> +struct kernel_stat *task_group_kstat(struct task_struct *p)
> >> +{
> >> + struct task_group *tg;
> >> + struct kernel_stat *kstat;
> >> +
> >> + rcu_read_lock();
> >> + tg = task_group(p);
> >> + kstat = tg->cpustat;
> >> + rcu_read_unlock();
> >> + return kstat;
> >> +}
> >
> > Who keeps tg alive and kicking while you poke at its (cpustat) member?
>
> * All calls to this function currently pass current as a parameter
> (Okay, maybe it is too generic and we should pass nothing at all, and
> grab current within it)
> * rcu_read_lock() guarantees that current will exist during this call,
> and task_group won't change. (right?)
The thing I worry about is:
A (pid n) B
kstat = task_group_kstat()
echo n > /cgroup/something-else/pid
rmdir /cgroup/group-that-had-A
<timer interrupt>
RCU complete
<softirq>
kfree(tg) etc..
kstat->foo++; <-- *BOOM*
The only way to avoid someone moving you around is by holding some
cgroup lock, task->alloc_lock, task->pi_lock or the rq->lock where task
runs. Alternatively keep rcu_read_lock() around the entire kstat usage.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists