Re: [PATCH UPDATED 11/11] blkcg: unify blkg's for blkcg policies

From: Tejun Heo
Date: Fri Feb 03 2012 - 15:59:14 EST

Hey, Vivek.

On Fri, Feb 03, 2012 at 02:41:05PM -0500, Vivek Goyal wrote:
> On Wed, Feb 01, 2012 at 04:37:30PM -0800, Tejun Heo wrote:
> > As a transitional step to untangle blkg management, elvswitch and
> > policy [de]registration, all blkgs except the root blkg are being shot
> > down during elvswitch and bypass. This patch adds blkg_root_update()
> > to update root blkg in place on policy change. This is hacky and racy
> > but should be good enough as interim step until we get locking
> > simplified and switch over to proper in-place update for all blkgs.
> - So we don't shoot down root group over elevator switch and policy
> changes because we are not sure if we will be able to alloc new
> group? It is not like elevator where we don't free the old one till
> we have made sure that new one is allocated and initialized properly.

No, because we policies cache root group and we don't have mechanism
to update them. I could have added that but root group management
should be moved to blkcg core anyway and in-place update will be
applied to all blkgs, so I just chose a dirty shortcut as an interim

> - I am assuming that we will change blkg_destroy_all() later to also
> take policy as argument and only destroy policy data of respective
> policy and not the whole group. (Well I guess we can destroy the whole
> group if it was only policy on the group).

Yeap, that's what's scheduled.

> [..]
> > static struct blkio_group *blkg_alloc(struct blkio_cgroup *blkcg,
> > - struct request_queue *q,
> > - struct blkio_policy_type *pol)
> > + struct request_queue *q)
> Comment before this function still mentions "pol" as function argument.

Will update.

> [..]
> > @@ -776,43 +786,49 @@ blkiocg_reset_stats(struct cgroup *cgrou
> > #endif
> >
> > blkcg = cgroup_to_blkio_cgroup(cgroup);
> > + spin_lock(&blkio_list_lock);
> > spin_lock_irq(&blkcg->lock);
> Isn't blkcg lock enough to protect against policy registration/deregistration.
> A policy can not add/delete a group to cgroup list without blkcg list.

But pol list can change regardless of that, no?


To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at