On Thu 06-06-13 17:48:24, Tejun Heo wrote: > 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, OK, I thought that the predicate signature made it clear that its purpose is to _check_ whether visiting makes sense rather than _visit_ that node and work with the node. That is the reason why I didn't include state parameter which would be expected for the full visitor. Maybe using const would make it even more clear. I can update documentation for the predicate to make it more clear. > 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. Good. As I said many times, memcg iterators could see some clean ups. > So, let's go ahead as proposed. Thanks! > I'll try to rework the iterator on top of it, and my aplogies to > Michal for being over-the-top. Thanks! -- Michal Hocko SUSE Labs -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>