Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935065AbZLPQkT (ORCPT ); Wed, 16 Dec 2009 11:40:19 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S935031AbZLPQkS (ORCPT ); Wed, 16 Dec 2009 11:40:18 -0500 Received: from mx1.redhat.com ([209.132.183.28]:28139 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935025AbZLPQkO (ORCPT ); Wed, 16 Dec 2009 11:40:14 -0500 Date: Wed, 16 Dec 2009 11:39:42 -0500 From: Vivek Goyal To: Gui Jianfeng Cc: Jens Axboe , Corrado Zoccolo , linux kernel mailing list , Li Zefan Subject: Re: [PATCH] cfq: Remove useless css reference get Message-ID: <20091216163941.GA2807@redhat.com> References: <4B289C93.3070603@cn.fujitsu.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4B289C93.3070603@cn.fujitsu.com> User-Agent: Mutt/1.5.19 (2009-01-05) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3928 Lines: 114 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 > Reviewed-by: Li Zefan 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? 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? Thanks Vivek > --- > block/blk-cgroup.c | 14 -------------- > block/blk-cgroup.h | 3 --- > block/cfq-iosched.c | 5 ----- > 3 files changed, 0 insertions(+), 22 deletions(-) > > diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c > index 1fa2654..cba28f4 100644 > --- a/block/blk-cgroup.c > +++ b/block/blk-cgroup.c > @@ -23,20 +23,6 @@ static LIST_HEAD(blkio_list); > struct blkio_cgroup blkio_root_cgroup = { .weight = 2*BLKIO_WEIGHT_DEFAULT }; > EXPORT_SYMBOL_GPL(blkio_root_cgroup); > > -bool blkiocg_css_tryget(struct blkio_cgroup *blkcg) > -{ > - if (!css_tryget(&blkcg->css)) > - return false; > - return true; > -} > -EXPORT_SYMBOL_GPL(blkiocg_css_tryget); > - > -void blkiocg_css_put(struct blkio_cgroup *blkcg) > -{ > - css_put(&blkcg->css); > -} > -EXPORT_SYMBOL_GPL(blkiocg_css_put); > - > struct blkio_cgroup *cgroup_to_blkio_cgroup(struct cgroup *cgroup) > { > return container_of(cgroup_subsys_state(cgroup, blkio_subsys_id), > diff --git a/block/blk-cgroup.h b/block/blk-cgroup.h > index 4d316df..84bf745 100644 > --- a/block/blk-cgroup.h > +++ b/block/blk-cgroup.h > @@ -43,9 +43,6 @@ struct blkio_group { > unsigned long sectors; > }; > > -extern bool blkiocg_css_tryget(struct blkio_cgroup *blkcg); > -extern void blkiocg_css_put(struct blkio_cgroup *blkcg); > - > typedef void (blkio_unlink_group_fn) (void *key, struct blkio_group *blkg); > typedef void (blkio_update_group_weight_fn) (struct blkio_group *blkg, > unsigned int weight); > diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c > index e2f8046..5d6b427 100644 > --- a/block/cfq-iosched.c > +++ b/block/cfq-iosched.c > @@ -958,10 +958,6 @@ cfq_find_alloc_cfqg(struct cfq_data *cfqd, struct cgroup *cgroup, int create) > struct backing_dev_info *bdi = &cfqd->queue->backing_dev_info; > unsigned int major, minor; > > - /* Do we need to take this reference */ > - if (!blkiocg_css_tryget(blkcg)) > - return NULL;; > - > cfqg = cfqg_of_blkg(blkiocg_lookup_group(blkcg, key)); > if (cfqg || !create) > goto done; > @@ -992,7 +988,6 @@ cfq_find_alloc_cfqg(struct cfq_data *cfqd, struct cgroup *cgroup, int create) > hlist_add_head(&cfqg->cfqd_node, &cfqd->cfqg_list); > > done: > - blkiocg_css_put(blkcg); > return cfqg; > } > > -- > 1.5.4.rc3 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/