From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753342Ab2DMPX4 (ORCPT ); Fri, 13 Apr 2012 11:23:56 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44359 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751308Ab2DMPXy (ORCPT ); Fri, 13 Apr 2012 11:23:54 -0400 Date: Fri, 13 Apr 2012 11:23:50 -0400 From: Vivek Goyal To: Tejun Heo Cc: axboe@kernel.dk, ctalbott@google.com, rni@google.com, linux-kernel@vger.kernel.org, cgroups@vger.kernel.org, containers@lists.linux-foundation.org Subject: Re: [PATCH 7/8] blkcg: implement per-queue policy activation Message-ID: <20120413152350.GD26383@redhat.com> References: <1334273380-30233-1-git-send-email-tj@kernel.org> <1334273380-30233-8-git-send-email-tj@kernel.org> <20120413152122.GC26383@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20120413152122.GC26383@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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