Re: [PATCH 7/8] blkcg: implement per-queue policy activation

[Vivek Goyal]

On Fri, Apr 13, 2012 at 11:21:22AM -0400, Vivek Goyal wrote:
> On Thu, Apr 12, 2012 at 04:29:39PM -0700, Tejun Heo wrote:
> 
> [..]
> > @@ -111,12 +122,11 @@ static struct blkio_group *blkg_alloc(struct blkio_cgroup *blkcg,
> >  		struct blkio_policy_type *pol = blkio_policy[i];
> >  		struct blkg_policy_data *pd;
> >  
> > -		if (!pol)
> > +		if (!blkcg_policy_enabled(q, pol))
> >  			continue;
> >  
> >  		/* alloc per-policy data and attach it to blkg */
> > -		pd = kzalloc_node(sizeof(*pd) + pol->pdata_size, GFP_ATOMIC,
> > -				  q->node);
> > +		pd = kzalloc_node(blkg_pd_size(pol), GFP_ATOMIC, q->node);
> >  		if (!pd) {
> >  			blkg_free(blkg);
> >  			return NULL;
> > @@ -130,7 +140,7 @@ static struct blkio_group *blkg_alloc(struct blkio_cgroup *blkcg,
> >  	for (i = 0; i < BLKCG_MAX_POLS; i++) {
> >  		struct blkio_policy_type *pol = blkio_policy[i];
> >  
> > -		if (pol)
> > +		if (blkcg_policy_enabled(blkg->q, pol))
> >  			pol->ops.blkio_init_group_fn(blkg);
> >  	}
> 
> May be pol->ops.blkio_init_group_fn() can be called in the loop above
> where you are allocating the group. So you don't end up looping through
> policies twice and don't have to call blkcg_policy_enabled() twice.

Oh..., I guess you did it because you did not want to call into individual
policies till all the policy data allocations were successfully done.
Otherwise you need to call into policies again to undo all initializations.

So yes, it is fine as it is.

Thanks
Vivek
--
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/