Hello, On Wed, Jun 05, 2013 at 03:45:52PM -0400, Johannes Weiner wrote: > I'm not sure what you are suggesting. Synchroneously invalidate every > individual iterator upwards the hierarchy every time a cgroup is > destroyed? Yeap. > The invalidation I am talking about is what we do by increasing the > dead counts. This lazily invalidates all the weak pointers in the > iterators of the hierarchy root. > > Of course if you do a synchroneous invalidation of individual > iterators, we don't need weak pointers anymore and RCU is enough, but > that would mean nr_levels * nr_nodes * nr_zones * nr_priority_levels > invalidation operations per destruction, whereas the weak pointers are > invalidated with one atomic_inc() per nesting level. While it does have to traverse the arrays, it's still bound by the depth of nesting and cgroup destruction is a pretty cold path. I don't think it'd matter that much. > As I said, the weak pointers are only a few lines of code that can be > neatly self-contained (see the invalidate, load, store functions > below). Please convince me that your alternative solution will save > complexity to such an extent that either the memory waste of > indefinite css pinning, or the computational overhead of non-lazy > iterator cleanup, is justifiable. The biggest issue I see with the weak pointer is that it's special and tricky. If this is something which is absolutely necessary, it should be somewhere more generic. Also, if we can use the usual RCU deref with O(depth) cleanup in the cold path, I don't see how this deviation is justifiable. For people who've been looking at it for long enough, it probably isn't that different from using plain RCU but that's just because that person has spent the time to build that pattern into his/her brain. We now have a lot of people accustomed to plain RCU usages which in itself is tricky already and introducing new constructs is actively deterimental to maintainability. We sure can do that when there's no alternative but I don't think avoiding synchronous cleanup on cgroup destruction path is a good enough reason. It feels like an over-engineering to me. Another thing is that this matters the most when there are continuous creation and destruction of cgroups and the weak pointer implementation would keep resetting the iteration to the beginning. Depending on timing, it'd be able to live-lock reclaim cursor to the beginning of iteration even with fairly low rate of destruction, right? It can be pretty bad high up the tree. With synchronous cleanup, depending on how it's implemented, it can be made to keep the iteration position. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe cgroups" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
