Messages in this thread | | | Date | Fri, 3 Feb 2012 12:59:10 -0800 | From | Tejun Heo <> | Subject | Re: [PATCH UPDATED 11/11] blkcg: unify blkg's for blkcg policies |
| |
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 step.
> - 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?
Thanks.
-- tejun
| |