Messages in this thread | | | Date | Fri, 27 Jun 2008 17:52:01 +0900 | From | KAMEZAWA Hiroyuki <> | Subject | Re: [-mm][PATCH 8/10] fix shmem page migration incorrectness on memcgroup |
| |
On Fri, 27 Jun 2008 16:57:56 +0900 "MinChan Kim" <minchan.kim@gmail.com> wrote:
> On Fri, Jun 27, 2008 at 2:41 PM, KOSAKI Motohiro > <kosaki.motohiro@jp.fujitsu.com> wrote: > >> > mem_cgroup_uncharge() against old page is done after radix-tree-replacement. > >> > And there were special handling to ingore swap-cache page. But, shmem can > >> > be swap-cache and file-cache at the same time. Chekcing PageSwapCache() is > >> > not correct here. Check PageAnon() instead. > >> > >> When/How shmem can be both swap-cache and file-cache ? > >> I can't understand that situation. > > > > Hi > > > > see, > > > > shmem_writepage() > > -> add_to_swap_cache() > > -> SetPageSwapCache() > > > > > > BTW: his file-cache mean !Anon, not mean !SwapBacked. > > Hi KOSAKI-san. > Thanks for explaining. > > In the migrate_page_move_mapping, the page was already locked in unmap_and_move. > Also, we have a lock for that page for calling shmem_writepage. > > So I think race problem between shmem_writepage and > migrate_page_move_mapping don't occur. > But I am not sure I am right. > > If I am wrong, could you tell me when race problem happen ? :) > You are right. I misundestood the swap/shmem code. there is no race. Hmm...
But situation is a bit complicated. - shmem's page is charged as file-cache. - shmem's swap cache is still charged by mem_cgroup_cache_charge() because it's implicitly (to memcg) converted to swap cache. - anon's swap cache is charged by mem_cgroup_uncharge_cache_page()
So, uncharging swap-cache of shmem by mem_cgroup_uncharge_cache_page() is valid. Checking PageSwapCache() was bad and Cheking PageAnon() is good. (From maintainance view)
I think the patch is valid but my patch description contains wrong information. Andrew, could you drop this ? I'll rewrite the patch description.
Sorry, -Kame
| |