linux-kernel - Re: [patch -v4 4/8] memcg: enhance memcg iterator to support predicates

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]

Message-ID: <20130607004824.GA16160@htj.dyndns.org>
Date:	Thu, 6 Jun 2013 17:48:24 -0700
From:	Tejun Heo <tj@...nel.org>
To:	Michal Hocko <mhocko@...e.cz>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Johannes Weiner <hannes@...xchg.org>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	linux-mm@...ck.org, cgroups@...r.kernel.org,
	linux-kernel@...r.kernel.org, Ying Han <yinghan@...gle.com>,
	Hugh Dickins <hughd@...gle.com>,
	Glauber Costa <glommer@...allels.com>,
	Michel Lespinasse <walken@...gle.com>,
	Greg Thelen <gthelen@...gle.com>,
	Balbir Singh <bsingharora@...il.com>
Subject: Re: [patch -v4 4/8] memcg: enhance memcg iterator to support
 predicates

On Wed, Jun 05, 2013 at 02:09:38AM -0700, Tejun Heo wrote:
> On Wed, Jun 05, 2013 at 11:07:39AM +0200, Michal Hocko wrote:
> > On Wed 05-06-13 01:58:49, Tejun Heo wrote:
> > [...]
> > > Anyways, so you aren't gonna try the skipping thing?
> > 
> > As I said. I do not consider this a priority for the said reasons (i
> > will not repeat them).
> 
> That's a weird way to respond.  Alright, whatever, let me give it a
> shot then.

So, there were some private exchanges and here's my main issue with
the addition of predicate callback to mem_cgroup_iter_cond().

There are two common patterns that are used to implement iteration.
One is the good ol' callback based one - ie. call_fn_on_each(fn) type
interface.  The other is something which can be used as part of flow
control by the user - be it give_me_next_elem() or for_each() type
loop macro.  In majority of cases, especially for anything generic,
the latter is considered to be the better choice because, while a bit
more challenging to implement usually, it's a lot less cumbersome for
the users of the interface.

mem_cgroup_iter_cond() seems icky to me because the predicate callback
is essentially visit callback, so now we end up with
give_me_next_elem() with visit callback, which is fundamentally
superflous.  If it were properly call_fn_on_each(fn), the return
values would be CONTINUE, SKIP_SUBTREE or ABORT, which makes more
sense to me.  Sure, it can be said that the predicate callback is for
a different purpose but it really doesn't change that the interface
now is visiting the same node in two different places.  If it were
something remotely widely used, it won't take much time developing
braindamaged usages where part is being done inside the predicate
callback and the rest is done outside without clear reason why just
because of natural code growth.  I don't think this is the type of
construct that we want in kernel in general.

That said, it also is true that doing this is the shortest path to
implementing subtree skip given how the iterator is put together
currently and the series as a whole reduces significant amount of
complexity, so it is an acceptable tradeoff to proceed with this
implementation with later restructuring of the iterator.

So, let's go ahead as proposed.  I'll try to rework the iterator on
top of it, and my aplogies to Michal for being over-the-top.

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/