Re: [patch] mm, memcg: add anon_hugepage stat

[David Rientjes <rientjes@google.com>]

On Fri, 26 Apr 2013, Michal Hocko wrote:

> Yes, useful and I had it on my todo list for quite some time. Never got
> to it though. Thanks!
> 

I think I'll add an anon_pages counter as well for non-thp for comparison, 
and probably do it in the same patch.

The problem is that we don't always have the memcg context for the page 
when calling page_add_anon_rmap() or page_remove_rmap().

 [ An example in this patch is in page_remove_rmap() where I was calling 
   mem_cgroup_update_page_stat() after mem_cgroup_uncharge_page(). ]

For example, in unuse_pte():

	if (page == swapcache)
		page_add_anon_rmap(page, vma, addr);
	else /* ksm created a completely new copy */
		page_add_new_anon_rmap(page, vma, addr);
	mem_cgroup_commit_charge_swapin(page, memcg);

There are a couple of options to fix this and I really don't have a strong 
preference for which one we go with:

 - pass struct mem_cgroup * to page_add_anon_rmap() and 
   page_remove_rmap(), such as "memcg" in the above example), or

 - separate out the anon page/hugepage ZVC accounting entirely from these 
   two functions and add a followup call to a new function dedicated for 
   this purpose once the memcg commit has been done.

I'm leaning toward doing the latter just because it's cleaner, but it 
means page_remove_rmap() picks up a return value (anon or not?) and 
page_add_anon_rmap() picks up a return value (_mapcount == 0?).

Comments?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>