Re: [PATCH] cfq: Remove useless css reference get

[Jens Axboe <jens.axboe@oracle.com>]

On Wed, Dec 16 2009, Vivek Goyal wrote:
> On Wed, Dec 16, 2009 at 04:38:43PM +0800, Gui Jianfeng wrote:
> > There's no need to take css reference here, for the caller
> > has already called rcu_read_lock() to prevent cgroup from 
> > being removed.
> > 
> > Signed-off-by: Gui Jianfeng <guijianfeng@cn.fujitsu.com>
> > Reviewed-by: Li Zefan <lizf@cn.fujitsu.com>
> 
> Hi Gui,
> 
> How would an rcu lock protect against the possibility that blkiocg_destroy()
> has not already been called on another cpu? rcu lock will make sure that
> cgroup and blkio_cgroup object should still be around as long as I am
> holding rcu lock but will not protect against deletion path being executed
> on another cpu? So I don't want to end up in following situation.
> 
>      cpu1                      cpu2
> 
> rcu_read_lock()
> 			      blkiocg_destroy()
> 
> blkiocg_add_blkio_group()
> rcu_read_unlock()
> 
> I don't want to add blkg object on a potentially dead blkio_cgroup object
> which will go away. Does this protection is provided by generic cgroup
> code where blkiocg_destroy() will not be called if I have got cgroup
> pointer under rcu lock?

Hmm, looking at blkiocg_destroy(), it seems to free blkcg directly. You
need an RCU grace period to pass in between, if you expect blkcg to be
valid under rcu_read_lock() always. Or is ->destroy() called from an RCU
callback? Checks... OK, so it looks like cgroup guarantees
synchronize_rcu() before calling ->destroy(), so that should be safe.

Why is it doing both rcu_read_lock() and grabbing the update lock?

On the real topic, it'll of course only guarentee that blkcg is valid
inside the rcu read lock. If it needs to be valid afterwards, the
reference must be grabbed.

> Currently we are deriving cgroup information from task context so task is
> still inside cgroup, so cgroup can't be deleted anyway. What if I was 
> deriving cgroup information from bio, will taking css object reference be
> necessary in that case or just cgroup pointer under rcu lock is sufficient
> to preclude the race against cgroup deletion path?

It's guarenteed that ->destroy() cannot be called while someone is under
rcu_read_lock().

-- 
Jens Axboe