On Tue, 6 Dec 2011, KAMEZAWA Hiroyuki wrote: > On Mon, 5 Dec 2011 23:36:34 -0800 (PST) > Hugh Dickins <hughd@xxxxxxxxxx> wrote: > > Hmm, at first glance at the patch, it seems far complicated than > I expected Right, this is just a rollup of assorted changes, yet to be presented properly as an understandable series. > and added much checks and hooks to lru path... Actually, I think it removes more than it adds; while trying not to increase the overhead of lookup_page_cgroup()s and locking. > > Okay, here it is: my usual mix of cleanup and functional changes. > > There's work by Ying and others in here - will apportion authorship > > more fairly when splitting. If you're looking through it at all, > > the place to start would be memcontrol.c's lock_page_lru_irqsave(). > > > > Thank you. This seems inetersting patch. Hmm...what I think of now is.. > In most case, pages are newly allocated and charged ,and then, added to LRU. > pc->mem_cgroup never changes while pages are on LRU. > > I have a fix for corner cases as to do > > 1. lock lru > 2. remove-page-from-lru > 3. overwrite pc->mem_cgroup > 4. add page to lru again > 5. unlock lru That is indeed the sequence which __mem_cgroup_commit_charge() follows after the patch. But it optimizes out the majority of cases when no such lru operations are needed (optimizations best presented in a separate patch), while being careful about the tricky case when the page is on lru_add_pvecs, and may get on to an lru at any moment. And since it uses a separate lock for each memcg-zone's set of lrus, must take care that both lock and lru in 4 and 5 are different from those in 1 and 2. > > And blindly believe pc->mem_cgroup regardless of PCG_USED bit at LRU handling. That's right. The difficulty comes when Used is cleared while the page is off lru, or page removed from lru while Used is clear: once lock is dropped, we have no hold on the memcg, and must move to root lru lest the old memcg get deleted. The old Used + AcctLRU + pc->mem_cgroup puppetry used to achieve that quite cleverly; but in distributing zone lru_locks over memcgs, we went through a lot of crashes before we understood the subtlety of it; and in most places were just fighting the way it shifted underneath us. Now mem_cgroup_move_uncharged_to_root() makes the move explicit, in just a few places. > > Hm, per-zone-per-memcg lru locking is much easier if > - we igonore PCG_USED bit at lru handling I may or may not agree with you, depending on what you mean! > - we never overwrite pc->mem_cgroup if the page is on LRU. That's not the way I was thinking of it, but I think that's what we're doing. > - if page may be added to LRU by pagevec etc.. while we overwrite > pc->mem_cgroup, we always take lru_lock. This is our corner case. Yes, the tricky case I mention above. > > isn't it ? I posted a series of patch. I'm glad if you give me a > quick review. I haven't glanced yet, will do so after an hour or two. Hugh -- 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
