linux-kernel - Re: [PATCH 7/9] block: implement bio_associate

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 17 Feb 2012 14:38:59 -0800
From:	Tejun Heo <tj@...nel.org>
To:	Vivek Goyal <vgoyal@...hat.com>
Cc:	axboe@...nel.dk, ctalbott@...gle.com, rni@...gle.com,
	linux-kernel@...r.kernel.org,
	Kent Overstreet <koverstreet@...gle.com>
Subject: Re: [PATCH 7/9] block: implement bio_associate_current()

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/