Re: [PATCH 7/9] block: implement bio_associate_current()

[Tejun Heo]

Hello,

On Fri, Feb 17, 2012 at 05:29:09PM -0500, Vivek Goyal wrote:
> Don't we already keep track of task changing cgroup and record that
> state in ioc.
> 
> blkiocg_attach()
>   ioc_cgroup_changed()
> 
> I think in ioc_cgroup_changed() we can just drop the reference to previous
> blkcg and store reference to new blkcg?

Hmmm.... right, we have that; then, why doesn't cgroup change take
effect w/ cfq?  Maybe it actually works and I confused it with
stacking failure.  Will test again later.

But, anyways, ioc isn't keeping track of blkcg.  The changed thing is
necessary only because cfq is caching the relationship as associated
cfqqs.  I think it would be better if cfq can just compare the current
blkcg without requiring the async notification (or at least do it
synchronously).  The current way of handling it is kinda nasty.

> BTW, this change seems to be completely orthogonal to blkcg cleanup. May
> be it is a good idea to split it out in a separate patch series. It has
> nothing to do with rcu cleanup in blkcg.

At first, it required the locking update because I was determining
blkg association on bio issue and then passing it down.  That didn't
work with cfq caching the association, so it no longer has dependency.
It should can be a separate patchset.  This whole lot is gonna go in
as long sequential series of patches so I'm splitting them just so
that each posting isn't too huge at this point.

Thanks.

-- 
tejun
--
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/