* [PATCH -mm] memcg: fix handling of shmem migration @ 2008-09-17 4:31 ` Daisuke Nishimura 0 siblings, 0 replies; 24+ messages in thread From: Daisuke Nishimura @ 2008-09-17 4:31 UTC (permalink / raw) To: linux-kernel, linux-mm; +Cc: akpm, balbir, kamezawa.hiroyu, xemul, nishimura PG_swapbacked flag of newpage should be set(if needed) before mem_cgroup_prepare_migration, because mem_cgroup_charge_common checks the flag and determines whether it sets PAGE_CGROUP_FLAG_FILE or not. Before this patch, if migrating shmem/tmpfs pages, newpage would be charged with PAGE_CGROUP_FLAG_FILE set, while oldpage has been charged without the flag. The problem here is mem_cgroup_move_lists doesn't clear(or set) the PAGE_CGROUP_FLAG_FILE flag, so pc->flags of the newpage remains PAGE_CGROUP_FLAG_FILE set even when the pc is moved to another lru(anon) by mem_cgroup_move_lists. And this leads to incorrect MEM_CGROUP_ZSTAT. (In my test, I see an underflow of MEM_CGROUP_ZSTAT(active_file). As a result, mem_cgroup_calc_reclaim returns very huge number and causes soft lockup on page reclaim.) I'm not sure if mem_cgroup_move_lists should handle PAGE_CGROUP_FLAG_FILE or not(I suppose it should be used to move between active <-> inactive, not anon <-> file), I moved SetPageSwapBacked(newpage) before mem_cgroup_prepare_migration. Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> --- mm/migrate.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/mm/migrate.c b/mm/migrate.c index 577d481..7343463 100644 --- a/mm/migrate.c +++ b/mm/migrate.c @@ -586,8 +586,6 @@ static int move_to_new_page(struct page *newpage, struct page *page) /* Prepare mapping for the new page.*/ newpage->index = page->index; newpage->mapping = page->mapping; - if (PageSwapBacked(page)) - SetPageSwapBacked(newpage); mapping = page_mapping(page); if (!mapping) @@ -636,6 +634,8 @@ static int unmap_and_move(new_page_t get_new_page, unsigned long private, goto move_newpage; } + if (PageSwapBacked(page)) + SetPageSwapBacked(newpage); charge = mem_cgroup_prepare_migration(page, newpage); if (charge == -ENOMEM) { rc = -ENOMEM; ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH -mm] memcg: fix handling of shmem migration @ 2008-09-17 4:31 ` Daisuke Nishimura 0 siblings, 0 replies; 24+ messages in thread From: Daisuke Nishimura @ 2008-09-17 4:31 UTC (permalink / raw) To: linux-kernel, linux-mm; +Cc: akpm, balbir, kamezawa.hiroyu, xemul, nishimura PG_swapbacked flag of newpage should be set(if needed) before mem_cgroup_prepare_migration, because mem_cgroup_charge_common checks the flag and determines whether it sets PAGE_CGROUP_FLAG_FILE or not. Before this patch, if migrating shmem/tmpfs pages, newpage would be charged with PAGE_CGROUP_FLAG_FILE set, while oldpage has been charged without the flag. The problem here is mem_cgroup_move_lists doesn't clear(or set) the PAGE_CGROUP_FLAG_FILE flag, so pc->flags of the newpage remains PAGE_CGROUP_FLAG_FILE set even when the pc is moved to another lru(anon) by mem_cgroup_move_lists. And this leads to incorrect MEM_CGROUP_ZSTAT. (In my test, I see an underflow of MEM_CGROUP_ZSTAT(active_file). As a result, mem_cgroup_calc_reclaim returns very huge number and causes soft lockup on page reclaim.) I'm not sure if mem_cgroup_move_lists should handle PAGE_CGROUP_FLAG_FILE or not(I suppose it should be used to move between active <-> inactive, not anon <-> file), I moved SetPageSwapBacked(newpage) before mem_cgroup_prepare_migration. Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> --- mm/migrate.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/mm/migrate.c b/mm/migrate.c index 577d481..7343463 100644 --- a/mm/migrate.c +++ b/mm/migrate.c @@ -586,8 +586,6 @@ static int move_to_new_page(struct page *newpage, struct page *page) /* Prepare mapping for the new page.*/ newpage->index = page->index; newpage->mapping = page->mapping; - if (PageSwapBacked(page)) - SetPageSwapBacked(newpage); mapping = page_mapping(page); if (!mapping) @@ -636,6 +634,8 @@ static int unmap_and_move(new_page_t get_new_page, unsigned long private, goto move_newpage; } + if (PageSwapBacked(page)) + SetPageSwapBacked(newpage); charge = mem_cgroup_prepare_migration(page, newpage); if (charge == -ENOMEM) { rc = -ENOMEM; -- 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> ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH -mm] memcg: fix handling of shmem migration 2008-09-17 4:31 ` Daisuke Nishimura @ 2008-09-17 5:46 ` KAMEZAWA Hiroyuki -1 siblings, 0 replies; 24+ messages in thread From: KAMEZAWA Hiroyuki @ 2008-09-17 5:46 UTC (permalink / raw) To: Daisuke Nishimura; +Cc: linux-kernel, linux-mm, akpm, balbir, xemul On Wed, 17 Sep 2008 13:31:49 +0900 Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote: > PG_swapbacked flag of newpage should be set(if needed) before > mem_cgroup_prepare_migration, because mem_cgroup_charge_common > checks the flag and determines whether it sets PAGE_CGROUP_FLAG_FILE or not. > > Before this patch, if migrating shmem/tmpfs pages, newpage would be > charged with PAGE_CGROUP_FLAG_FILE set, while oldpage has been charged > without the flag. > Nice catch ! Thank you. Hmm, should I add MEM_CGROUP_CHARGE_TYPE_SHMEM rather than setting flag to newpage ? Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > The problem here is mem_cgroup_move_lists doesn't clear(or set) > the PAGE_CGROUP_FLAG_FILE flag, so pc->flags of the newpage > remains PAGE_CGROUP_FLAG_FILE set even when the pc is moved to > another lru(anon) by mem_cgroup_move_lists. And this leads to > incorrect MEM_CGROUP_ZSTAT. > (In my test, I see an underflow of MEM_CGROUP_ZSTAT(active_file). > As a result, mem_cgroup_calc_reclaim returns very huge number and > causes soft lockup on page reclaim.) > > I'm not sure if mem_cgroup_move_lists should handle PAGE_CGROUP_FLAG_FILE > or not(I suppose it should be used to move between active <-> inactive, > not anon <-> file), I moved SetPageSwapBacked(newpage) before > mem_cgroup_prepare_migration. > > > Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> > --- > mm/migrate.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/mm/migrate.c b/mm/migrate.c > index 577d481..7343463 100644 > --- a/mm/migrate.c > +++ b/mm/migrate.c > @@ -586,8 +586,6 @@ static int move_to_new_page(struct page *newpage, struct page *page) > /* Prepare mapping for the new page.*/ > newpage->index = page->index; > newpage->mapping = page->mapping; > - if (PageSwapBacked(page)) > - SetPageSwapBacked(newpage); > > mapping = page_mapping(page); > if (!mapping) > @@ -636,6 +634,8 @@ static int unmap_and_move(new_page_t get_new_page, unsigned long private, > goto move_newpage; > } > > + if (PageSwapBacked(page)) > + SetPageSwapBacked(newpage); > charge = mem_cgroup_prepare_migration(page, newpage); > if (charge == -ENOMEM) { > rc = -ENOMEM; > ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH -mm] memcg: fix handling of shmem migration @ 2008-09-17 5:46 ` KAMEZAWA Hiroyuki 0 siblings, 0 replies; 24+ messages in thread From: KAMEZAWA Hiroyuki @ 2008-09-17 5:46 UTC (permalink / raw) To: Daisuke Nishimura; +Cc: linux-kernel, linux-mm, akpm, balbir, xemul On Wed, 17 Sep 2008 13:31:49 +0900 Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote: > PG_swapbacked flag of newpage should be set(if needed) before > mem_cgroup_prepare_migration, because mem_cgroup_charge_common > checks the flag and determines whether it sets PAGE_CGROUP_FLAG_FILE or not. > > Before this patch, if migrating shmem/tmpfs pages, newpage would be > charged with PAGE_CGROUP_FLAG_FILE set, while oldpage has been charged > without the flag. > Nice catch ! Thank you. Hmm, should I add MEM_CGROUP_CHARGE_TYPE_SHMEM rather than setting flag to newpage ? Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > The problem here is mem_cgroup_move_lists doesn't clear(or set) > the PAGE_CGROUP_FLAG_FILE flag, so pc->flags of the newpage > remains PAGE_CGROUP_FLAG_FILE set even when the pc is moved to > another lru(anon) by mem_cgroup_move_lists. And this leads to > incorrect MEM_CGROUP_ZSTAT. > (In my test, I see an underflow of MEM_CGROUP_ZSTAT(active_file). > As a result, mem_cgroup_calc_reclaim returns very huge number and > causes soft lockup on page reclaim.) > > I'm not sure if mem_cgroup_move_lists should handle PAGE_CGROUP_FLAG_FILE > or not(I suppose it should be used to move between active <-> inactive, > not anon <-> file), I moved SetPageSwapBacked(newpage) before > mem_cgroup_prepare_migration. > > > Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> > --- > mm/migrate.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/mm/migrate.c b/mm/migrate.c > index 577d481..7343463 100644 > --- a/mm/migrate.c > +++ b/mm/migrate.c > @@ -586,8 +586,6 @@ static int move_to_new_page(struct page *newpage, struct page *page) > /* Prepare mapping for the new page.*/ > newpage->index = page->index; > newpage->mapping = page->mapping; > - if (PageSwapBacked(page)) > - SetPageSwapBacked(newpage); > > mapping = page_mapping(page); > if (!mapping) > @@ -636,6 +634,8 @@ static int unmap_and_move(new_page_t get_new_page, unsigned long private, > goto move_newpage; > } > > + if (PageSwapBacked(page)) > + SetPageSwapBacked(newpage); > charge = mem_cgroup_prepare_migration(page, newpage); > if (charge == -ENOMEM) { > rc = -ENOMEM; > -- 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> ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH -mm] memcg: fix handling of shmem migration 2008-09-17 5:46 ` KAMEZAWA Hiroyuki @ 2008-09-17 5:50 ` KAMEZAWA Hiroyuki -1 siblings, 0 replies; 24+ messages in thread From: KAMEZAWA Hiroyuki @ 2008-09-17 5:50 UTC (permalink / raw) To: KAMEZAWA Hiroyuki Cc: Daisuke Nishimura, linux-kernel, linux-mm, akpm, balbir, xemul On Wed, 17 Sep 2008 14:46:59 +0900 KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote: > On Wed, 17 Sep 2008 13:31:49 +0900 > Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote: > > > PG_swapbacked flag of newpage should be set(if needed) before > > mem_cgroup_prepare_migration, because mem_cgroup_charge_common > > checks the flag and determines whether it sets PAGE_CGROUP_FLAG_FILE or not. > > > > Before this patch, if migrating shmem/tmpfs pages, newpage would be > > charged with PAGE_CGROUP_FLAG_FILE set, while oldpage has been charged > > without the flag. > > > Nice catch ! > Thank you. > > Hmm, should I add MEM_CGROUP_CHARGE_TYPE_SHMEM rather than > setting flag to newpage ? > > Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > I acked but.. can't this change moved into memcontrol.c ? Thanks, -Kame ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH -mm] memcg: fix handling of shmem migration @ 2008-09-17 5:50 ` KAMEZAWA Hiroyuki 0 siblings, 0 replies; 24+ messages in thread From: KAMEZAWA Hiroyuki @ 2008-09-17 5:50 UTC (permalink / raw) To: KAMEZAWA Hiroyuki Cc: Daisuke Nishimura, linux-kernel, linux-mm, akpm, balbir, xemul On Wed, 17 Sep 2008 14:46:59 +0900 KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote: > On Wed, 17 Sep 2008 13:31:49 +0900 > Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote: > > > PG_swapbacked flag of newpage should be set(if needed) before > > mem_cgroup_prepare_migration, because mem_cgroup_charge_common > > checks the flag and determines whether it sets PAGE_CGROUP_FLAG_FILE or not. > > > > Before this patch, if migrating shmem/tmpfs pages, newpage would be > > charged with PAGE_CGROUP_FLAG_FILE set, while oldpage has been charged > > without the flag. > > > Nice catch ! > Thank you. > > Hmm, should I add MEM_CGROUP_CHARGE_TYPE_SHMEM rather than > setting flag to newpage ? > > Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > I acked but.. can't this change moved into memcontrol.c ? Thanks, -Kame -- 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> ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH -mm] memcg: fix handling of shmem migration 2008-09-17 5:50 ` KAMEZAWA Hiroyuki @ 2008-09-17 6:19 ` Daisuke Nishimura -1 siblings, 0 replies; 24+ messages in thread From: Daisuke Nishimura @ 2008-09-17 6:19 UTC (permalink / raw) To: KAMEZAWA Hiroyuki; +Cc: nishimura, linux-kernel, linux-mm, akpm, balbir, xemul On Wed, 17 Sep 2008 14:50:03 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote: > On Wed, 17 Sep 2008 14:46:59 +0900 > KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote: > > > On Wed, 17 Sep 2008 13:31:49 +0900 > > Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote: > > > > > PG_swapbacked flag of newpage should be set(if needed) before > > > mem_cgroup_prepare_migration, because mem_cgroup_charge_common > > > checks the flag and determines whether it sets PAGE_CGROUP_FLAG_FILE or not. > > > > > > Before this patch, if migrating shmem/tmpfs pages, newpage would be > > > charged with PAGE_CGROUP_FLAG_FILE set, while oldpage has been charged > > > without the flag. > > > > > Nice catch ! > > Thank you. > > > > Hmm, should I add MEM_CGROUP_CHARGE_TYPE_SHMEM rather than > > setting flag to newpage ? > > > > Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > > > I acked but.. can't this change moved into memcontrol.c ? > Hmm, something like this? --- @@ -734,6 +734,9 @@ int mem_cgroup_prepare_migration(struct page *page, struct page *newpa if (mem_cgroup_subsys.disabled) return 0; + if (PageSwapBacked(page)) + SetPageSwapBacked(newpage); + lock_page_cgroup(page); pc = page_get_page_cgroup(page); if (pc) { --- Or, adding MEM_CGROUP_CHARGE_TYPE_SHMEM and --- @@ -740,7 +740,10 @@ int mem_cgroup_prepare_migration(struct page *page, struct page *newp mem = pc->mem_cgroup; css_get(&mem->css); if (pc->flags & PAGE_CGROUP_FLAG_CACHE) - ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; + if (page_is_file_cache(page)) + ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; + else + ctype = MEM_CGROUP_CHARGE_TYPE_SHMEM; } unlock_page_cgroup(page); if (mem) { --- (Of course, mem_cgroup_charge_common should be modified too.) Thanks, Daisuke Nishimura. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH -mm] memcg: fix handling of shmem migration @ 2008-09-17 6:19 ` Daisuke Nishimura 0 siblings, 0 replies; 24+ messages in thread From: Daisuke Nishimura @ 2008-09-17 6:19 UTC (permalink / raw) To: KAMEZAWA Hiroyuki; +Cc: nishimura, linux-kernel, linux-mm, akpm, balbir, xemul On Wed, 17 Sep 2008 14:50:03 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote: > On Wed, 17 Sep 2008 14:46:59 +0900 > KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote: > > > On Wed, 17 Sep 2008 13:31:49 +0900 > > Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote: > > > > > PG_swapbacked flag of newpage should be set(if needed) before > > > mem_cgroup_prepare_migration, because mem_cgroup_charge_common > > > checks the flag and determines whether it sets PAGE_CGROUP_FLAG_FILE or not. > > > > > > Before this patch, if migrating shmem/tmpfs pages, newpage would be > > > charged with PAGE_CGROUP_FLAG_FILE set, while oldpage has been charged > > > without the flag. > > > > > Nice catch ! > > Thank you. > > > > Hmm, should I add MEM_CGROUP_CHARGE_TYPE_SHMEM rather than > > setting flag to newpage ? > > > > Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > > > I acked but.. can't this change moved into memcontrol.c ? > Hmm, something like this? --- @@ -734,6 +734,9 @@ int mem_cgroup_prepare_migration(struct page *page, struct page *newpa if (mem_cgroup_subsys.disabled) return 0; + if (PageSwapBacked(page)) + SetPageSwapBacked(newpage); + lock_page_cgroup(page); pc = page_get_page_cgroup(page); if (pc) { --- Or, adding MEM_CGROUP_CHARGE_TYPE_SHMEM and --- @@ -740,7 +740,10 @@ int mem_cgroup_prepare_migration(struct page *page, struct page *newp mem = pc->mem_cgroup; css_get(&mem->css); if (pc->flags & PAGE_CGROUP_FLAG_CACHE) - ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; + if (page_is_file_cache(page)) + ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; + else + ctype = MEM_CGROUP_CHARGE_TYPE_SHMEM; } unlock_page_cgroup(page); if (mem) { --- (Of course, mem_cgroup_charge_common should be modified too.) Thanks, Daisuke Nishimura. -- 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> ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH -mm] memcg: fix handling of shmem migration 2008-09-17 6:19 ` Daisuke Nishimura @ 2008-09-17 6:38 ` KAMEZAWA Hiroyuki -1 siblings, 0 replies; 24+ messages in thread From: KAMEZAWA Hiroyuki @ 2008-09-17 6:38 UTC (permalink / raw) To: Daisuke Nishimura; +Cc: linux-kernel, linux-mm, akpm, balbir, xemul On Wed, 17 Sep 2008 15:19:51 +0900 Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote: > Hmm, something like this? > > --- > @@ -734,6 +734,9 @@ int mem_cgroup_prepare_migration(struct page *page, struct page *newpa > if (mem_cgroup_subsys.disabled) > return 0; > > + if (PageSwapBacked(page)) > + SetPageSwapBacked(newpage); > + > lock_page_cgroup(page); > pc = page_get_page_cgroup(page); > if (pc) { > --- > > Or, adding MEM_CGROUP_CHARGE_TYPE_SHMEM and > > --- > @@ -740,7 +740,10 @@ int mem_cgroup_prepare_migration(struct page *page, struct page *newp > mem = pc->mem_cgroup; > css_get(&mem->css); > if (pc->flags & PAGE_CGROUP_FLAG_CACHE) > - ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; > + if (page_is_file_cache(page)) > + ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; > + else > + ctype = MEM_CGROUP_CHARGE_TYPE_SHMEM; > } > unlock_page_cgroup(page); > if (mem) { > --- > (Of course, mem_cgroup_charge_common should be modified too.) > like this :) I don't want to change logic in migration.c (and this is special case handling for memcg.) Thanks, -Kame ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH -mm] memcg: fix handling of shmem migration @ 2008-09-17 6:38 ` KAMEZAWA Hiroyuki 0 siblings, 0 replies; 24+ messages in thread From: KAMEZAWA Hiroyuki @ 2008-09-17 6:38 UTC (permalink / raw) To: Daisuke Nishimura; +Cc: linux-kernel, linux-mm, akpm, balbir, xemul On Wed, 17 Sep 2008 15:19:51 +0900 Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote: > Hmm, something like this? > > --- > @@ -734,6 +734,9 @@ int mem_cgroup_prepare_migration(struct page *page, struct page *newpa > if (mem_cgroup_subsys.disabled) > return 0; > > + if (PageSwapBacked(page)) > + SetPageSwapBacked(newpage); > + > lock_page_cgroup(page); > pc = page_get_page_cgroup(page); > if (pc) { > --- > > Or, adding MEM_CGROUP_CHARGE_TYPE_SHMEM and > > --- > @@ -740,7 +740,10 @@ int mem_cgroup_prepare_migration(struct page *page, struct page *newp > mem = pc->mem_cgroup; > css_get(&mem->css); > if (pc->flags & PAGE_CGROUP_FLAG_CACHE) > - ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; > + if (page_is_file_cache(page)) > + ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; > + else > + ctype = MEM_CGROUP_CHARGE_TYPE_SHMEM; > } > unlock_page_cgroup(page); > if (mem) { > --- > (Of course, mem_cgroup_charge_common should be modified too.) > like this :) I don't want to change logic in migration.c (and this is special case handling for memcg.) Thanks, -Kame -- 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> ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH -mm] memcg: fix handling of shmem migration 2008-09-17 6:38 ` KAMEZAWA Hiroyuki @ 2008-09-17 6:45 ` Daisuke Nishimura -1 siblings, 0 replies; 24+ messages in thread From: Daisuke Nishimura @ 2008-09-17 6:45 UTC (permalink / raw) To: KAMEZAWA Hiroyuki; +Cc: nishimura, linux-kernel, linux-mm, akpm, balbir, xemul On Wed, 17 Sep 2008 15:38:26 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote: > On Wed, 17 Sep 2008 15:19:51 +0900 > Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote: > > > Hmm, something like this? > > > > --- > > @@ -734,6 +734,9 @@ int mem_cgroup_prepare_migration(struct page *page, struct page *newpa > > if (mem_cgroup_subsys.disabled) > > return 0; > > > > + if (PageSwapBacked(page)) > > + SetPageSwapBacked(newpage); > > + > > lock_page_cgroup(page); > > pc = page_get_page_cgroup(page); > > if (pc) { > > --- > > > > Or, adding MEM_CGROUP_CHARGE_TYPE_SHMEM and > > > > --- > > @@ -740,7 +740,10 @@ int mem_cgroup_prepare_migration(struct page *page, struct page *newp > > mem = pc->mem_cgroup; > > css_get(&mem->css); > > if (pc->flags & PAGE_CGROUP_FLAG_CACHE) > > - ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; > > + if (page_is_file_cache(page)) > > + ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; > > + else > > + ctype = MEM_CGROUP_CHARGE_TYPE_SHMEM; > > } > > unlock_page_cgroup(page); > > if (mem) { > > --- > > (Of course, mem_cgroup_charge_common should be modified too.) > > > like this :) I don't want to change logic in migration.c > (and this is special case handling for memcg.) > OK. I'll rewrite and resend it later. Thanks, Daisuke Nishimura. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH -mm] memcg: fix handling of shmem migration @ 2008-09-17 6:45 ` Daisuke Nishimura 0 siblings, 0 replies; 24+ messages in thread From: Daisuke Nishimura @ 2008-09-17 6:45 UTC (permalink / raw) To: KAMEZAWA Hiroyuki; +Cc: nishimura, linux-kernel, linux-mm, akpm, balbir, xemul On Wed, 17 Sep 2008 15:38:26 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote: > On Wed, 17 Sep 2008 15:19:51 +0900 > Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote: > > > Hmm, something like this? > > > > --- > > @@ -734,6 +734,9 @@ int mem_cgroup_prepare_migration(struct page *page, struct page *newpa > > if (mem_cgroup_subsys.disabled) > > return 0; > > > > + if (PageSwapBacked(page)) > > + SetPageSwapBacked(newpage); > > + > > lock_page_cgroup(page); > > pc = page_get_page_cgroup(page); > > if (pc) { > > --- > > > > Or, adding MEM_CGROUP_CHARGE_TYPE_SHMEM and > > > > --- > > @@ -740,7 +740,10 @@ int mem_cgroup_prepare_migration(struct page *page, struct page *newp > > mem = pc->mem_cgroup; > > css_get(&mem->css); > > if (pc->flags & PAGE_CGROUP_FLAG_CACHE) > > - ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; > > + if (page_is_file_cache(page)) > > + ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; > > + else > > + ctype = MEM_CGROUP_CHARGE_TYPE_SHMEM; > > } > > unlock_page_cgroup(page); > > if (mem) { > > --- > > (Of course, mem_cgroup_charge_common should be modified too.) > > > like this :) I don't want to change logic in migration.c > (and this is special case handling for memcg.) > OK. I'll rewrite and resend it later. Thanks, Daisuke Nishimura. -- 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> ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH -mm] memcg: fix handling of shmem migration(v2) 2008-09-17 6:45 ` Daisuke Nishimura @ 2008-09-17 7:55 ` Daisuke Nishimura -1 siblings, 0 replies; 24+ messages in thread From: Daisuke Nishimura @ 2008-09-17 7:55 UTC (permalink / raw) To: KAMEZAWA Hiroyuki; +Cc: nishimura, linux-kernel, linux-mm, akpm, balbir, xemul Before this patch, if migrating shmem/tmpfs pages, newpage would be charged with PAGE_CGROUP_FLAG_FILE set, while oldpage has been charged without the flag. The problem here is mem_cgroup_move_lists doesn't clear(or set) the PAGE_CGROUP_FLAG_FILE flag, so pc->flags of the newpage remains PAGE_CGROUP_FLAG_FILE set even when the pc is moved to another lru(anon) by mem_cgroup_move_lists. And this leads to incorrect MEM_CGROUP_ZSTAT. (In my test, I see an underflow of MEM_CGROUP_ZSTAT(active_file). As a result, mem_cgroup_calc_reclaim returns very huge number and causes soft lockup on page reclaim.) I'm not sure if mem_cgroup_move_lists should handle PAGE_CGROUP_FLAG_FILE or not(I suppose it should be used to move between active <-> inactive, not anon <-> file), I added MEM_CGROUP_CHARGE_TYPE_SHMEM for precharge at shmem's page migration. ChangeLog: v1->v2 - instead of modifying migrate.c, modify memcontrol.c only. - add MEM_CGROUP_CHARGE_TYPE_SHMEM. Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> --- mm/memcontrol.c | 13 ++++++++++--- 1 files changed, 10 insertions(+), 3 deletions(-) diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 2979d22..ef8812d 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -179,6 +179,7 @@ enum charge_type { MEM_CGROUP_CHARGE_TYPE_CACHE = 0, MEM_CGROUP_CHARGE_TYPE_MAPPED, MEM_CGROUP_CHARGE_TYPE_FORCE, /* used by force_empty */ + MEM_CGROUP_CHARGE_TYPE_SHMEM, /* used by page migration of shmem */ }; /* @@ -579,8 +580,10 @@ static int mem_cgroup_charge_common(struct page *page, struct mm_struct *mm, pc->flags |= PAGE_CGROUP_FLAG_FILE; else pc->flags |= PAGE_CGROUP_FLAG_ACTIVE; - } else + } else if (ctype == MEM_CGROUP_CHARGE_TYPE_MAPPED) pc->flags = PAGE_CGROUP_FLAG_ACTIVE; + else /* MEM_CGROUP_CHARGE_TYPE_SHMEM */ + pc->flags = PAGE_CGROUP_FLAG_CACHE | PAGE_CGROUP_FLAG_ACTIVE; lock_page_cgroup(page); if (unlikely(page_get_page_cgroup(page))) { @@ -739,8 +742,12 @@ int mem_cgroup_prepare_migration(struct page *page, struct page *newpage) if (pc) { mem = pc->mem_cgroup; css_get(&mem->css); - if (pc->flags & PAGE_CGROUP_FLAG_CACHE) - ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; + if (pc->flags & PAGE_CGROUP_FLAG_CACHE) { + if (page_is_file_cache(page)) + ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; + else + ctype = MEM_CGROUP_CHARGE_TYPE_SHMEM; + } } unlock_page_cgroup(page); if (mem) { ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH -mm] memcg: fix handling of shmem migration(v2) @ 2008-09-17 7:55 ` Daisuke Nishimura 0 siblings, 0 replies; 24+ messages in thread From: Daisuke Nishimura @ 2008-09-17 7:55 UTC (permalink / raw) To: KAMEZAWA Hiroyuki; +Cc: nishimura, linux-kernel, linux-mm, akpm, balbir, xemul Before this patch, if migrating shmem/tmpfs pages, newpage would be charged with PAGE_CGROUP_FLAG_FILE set, while oldpage has been charged without the flag. The problem here is mem_cgroup_move_lists doesn't clear(or set) the PAGE_CGROUP_FLAG_FILE flag, so pc->flags of the newpage remains PAGE_CGROUP_FLAG_FILE set even when the pc is moved to another lru(anon) by mem_cgroup_move_lists. And this leads to incorrect MEM_CGROUP_ZSTAT. (In my test, I see an underflow of MEM_CGROUP_ZSTAT(active_file). As a result, mem_cgroup_calc_reclaim returns very huge number and causes soft lockup on page reclaim.) I'm not sure if mem_cgroup_move_lists should handle PAGE_CGROUP_FLAG_FILE or not(I suppose it should be used to move between active <-> inactive, not anon <-> file), I added MEM_CGROUP_CHARGE_TYPE_SHMEM for precharge at shmem's page migration. ChangeLog: v1->v2 - instead of modifying migrate.c, modify memcontrol.c only. - add MEM_CGROUP_CHARGE_TYPE_SHMEM. Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> --- mm/memcontrol.c | 13 ++++++++++--- 1 files changed, 10 insertions(+), 3 deletions(-) diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 2979d22..ef8812d 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -179,6 +179,7 @@ enum charge_type { MEM_CGROUP_CHARGE_TYPE_CACHE = 0, MEM_CGROUP_CHARGE_TYPE_MAPPED, MEM_CGROUP_CHARGE_TYPE_FORCE, /* used by force_empty */ + MEM_CGROUP_CHARGE_TYPE_SHMEM, /* used by page migration of shmem */ }; /* @@ -579,8 +580,10 @@ static int mem_cgroup_charge_common(struct page *page, struct mm_struct *mm, pc->flags |= PAGE_CGROUP_FLAG_FILE; else pc->flags |= PAGE_CGROUP_FLAG_ACTIVE; - } else + } else if (ctype == MEM_CGROUP_CHARGE_TYPE_MAPPED) pc->flags = PAGE_CGROUP_FLAG_ACTIVE; + else /* MEM_CGROUP_CHARGE_TYPE_SHMEM */ + pc->flags = PAGE_CGROUP_FLAG_CACHE | PAGE_CGROUP_FLAG_ACTIVE; lock_page_cgroup(page); if (unlikely(page_get_page_cgroup(page))) { @@ -739,8 +742,12 @@ int mem_cgroup_prepare_migration(struct page *page, struct page *newpage) if (pc) { mem = pc->mem_cgroup; css_get(&mem->css); - if (pc->flags & PAGE_CGROUP_FLAG_CACHE) - ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; + if (pc->flags & PAGE_CGROUP_FLAG_CACHE) { + if (page_is_file_cache(page)) + ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; + else + ctype = MEM_CGROUP_CHARGE_TYPE_SHMEM; + } } unlock_page_cgroup(page); if (mem) { -- 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> ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH -mm] memcg: fix handling of shmem migration(v2) 2008-09-17 7:55 ` Daisuke Nishimura @ 2008-09-17 9:18 ` KAMEZAWA Hiroyuki -1 siblings, 0 replies; 24+ messages in thread From: KAMEZAWA Hiroyuki @ 2008-09-17 9:18 UTC (permalink / raw) To: Daisuke Nishimura; +Cc: linux-kernel, linux-mm, akpm, balbir, xemul On Wed, 17 Sep 2008 16:55:44 +0900 Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote: > Before this patch, if migrating shmem/tmpfs pages, newpage would be > charged with PAGE_CGROUP_FLAG_FILE set, while oldpage has been charged > without the flag. > > The problem here is mem_cgroup_move_lists doesn't clear(or set) > the PAGE_CGROUP_FLAG_FILE flag, so pc->flags of the newpage > remains PAGE_CGROUP_FLAG_FILE set even when the pc is moved to > another lru(anon) by mem_cgroup_move_lists. And this leads to > incorrect MEM_CGROUP_ZSTAT. > (In my test, I see an underflow of MEM_CGROUP_ZSTAT(active_file). > As a result, mem_cgroup_calc_reclaim returns very huge number and > causes soft lockup on page reclaim.) > > I'm not sure if mem_cgroup_move_lists should handle PAGE_CGROUP_FLAG_FILE > or not(I suppose it should be used to move between active <-> inactive, > not anon <-> file), I added MEM_CGROUP_CHARGE_TYPE_SHMEM for precharge > at shmem's page migration. > > > ChangeLog: v1->v2 > - instead of modifying migrate.c, modify memcontrol.c only. > - add MEM_CGROUP_CHARGE_TYPE_SHMEM. > I'll fix mem_cgroup_charge_cache_page() to use TYPE_SHMEM later. Thank you. Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH -mm] memcg: fix handling of shmem migration(v2) @ 2008-09-17 9:18 ` KAMEZAWA Hiroyuki 0 siblings, 0 replies; 24+ messages in thread From: KAMEZAWA Hiroyuki @ 2008-09-17 9:18 UTC (permalink / raw) To: Daisuke Nishimura; +Cc: linux-kernel, linux-mm, akpm, balbir, xemul On Wed, 17 Sep 2008 16:55:44 +0900 Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote: > Before this patch, if migrating shmem/tmpfs pages, newpage would be > charged with PAGE_CGROUP_FLAG_FILE set, while oldpage has been charged > without the flag. > > The problem here is mem_cgroup_move_lists doesn't clear(or set) > the PAGE_CGROUP_FLAG_FILE flag, so pc->flags of the newpage > remains PAGE_CGROUP_FLAG_FILE set even when the pc is moved to > another lru(anon) by mem_cgroup_move_lists. And this leads to > incorrect MEM_CGROUP_ZSTAT. > (In my test, I see an underflow of MEM_CGROUP_ZSTAT(active_file). > As a result, mem_cgroup_calc_reclaim returns very huge number and > causes soft lockup on page reclaim.) > > I'm not sure if mem_cgroup_move_lists should handle PAGE_CGROUP_FLAG_FILE > or not(I suppose it should be used to move between active <-> inactive, > not anon <-> file), I added MEM_CGROUP_CHARGE_TYPE_SHMEM for precharge > at shmem's page migration. > > > ChangeLog: v1->v2 > - instead of modifying migrate.c, modify memcontrol.c only. > - add MEM_CGROUP_CHARGE_TYPE_SHMEM. > I'll fix mem_cgroup_charge_cache_page() to use TYPE_SHMEM later. Thank you. Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> -- 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> ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH -mm] memcg: fix handling of shmem migration(v2) 2008-09-17 7:55 ` Daisuke Nishimura @ 2008-09-17 22:51 ` Andrew Morton -1 siblings, 0 replies; 24+ messages in thread From: Andrew Morton @ 2008-09-17 22:51 UTC (permalink / raw) To: Daisuke Nishimura Cc: kamezawa.hiroyu, nishimura, linux-kernel, linux-mm, balbir, xemul On Wed, 17 Sep 2008 16:55:44 +0900 Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote: > Before this patch, if migrating shmem/tmpfs pages, newpage would be > charged with PAGE_CGROUP_FLAG_FILE set, while oldpage has been charged > without the flag. > > The problem here is mem_cgroup_move_lists doesn't clear(or set) > the PAGE_CGROUP_FLAG_FILE flag, so pc->flags of the newpage > remains PAGE_CGROUP_FLAG_FILE set even when the pc is moved to > another lru(anon) by mem_cgroup_move_lists. And this leads to > incorrect MEM_CGROUP_ZSTAT. > (In my test, I see an underflow of MEM_CGROUP_ZSTAT(active_file). > As a result, mem_cgroup_calc_reclaim returns very huge number and > causes soft lockup on page reclaim.) > > I'm not sure if mem_cgroup_move_lists should handle PAGE_CGROUP_FLAG_FILE > or not(I suppose it should be used to move between active <-> inactive, > not anon <-> file), I added MEM_CGROUP_CHARGE_TYPE_SHMEM for precharge > at shmem's page migration. > > > ChangeLog: v1->v2 > - instead of modifying migrate.c, modify memcontrol.c only. > - add MEM_CGROUP_CHARGE_TYPE_SHMEM. > > > Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> > --- > mm/memcontrol.c | 13 ++++++++++--- > 1 files changed, 10 insertions(+), 3 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 2979d22..ef8812d 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -179,6 +179,7 @@ enum charge_type { > MEM_CGROUP_CHARGE_TYPE_CACHE = 0, > MEM_CGROUP_CHARGE_TYPE_MAPPED, > MEM_CGROUP_CHARGE_TYPE_FORCE, /* used by force_empty */ > + MEM_CGROUP_CHARGE_TYPE_SHMEM, /* used by page migration of shmem */ > }; > > /* > @@ -579,8 +580,10 @@ static int mem_cgroup_charge_common(struct page *page, struct mm_struct *mm, > pc->flags |= PAGE_CGROUP_FLAG_FILE; > else > pc->flags |= PAGE_CGROUP_FLAG_ACTIVE; > - } else > + } else if (ctype == MEM_CGROUP_CHARGE_TYPE_MAPPED) > pc->flags = PAGE_CGROUP_FLAG_ACTIVE; > + else /* MEM_CGROUP_CHARGE_TYPE_SHMEM */ > + pc->flags = PAGE_CGROUP_FLAG_CACHE | PAGE_CGROUP_FLAG_ACTIVE; > > lock_page_cgroup(page); > if (unlikely(page_get_page_cgroup(page))) { > @@ -739,8 +742,12 @@ int mem_cgroup_prepare_migration(struct page *page, struct page *newpage) > if (pc) { > mem = pc->mem_cgroup; > css_get(&mem->css); > - if (pc->flags & PAGE_CGROUP_FLAG_CACHE) > - ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; > + if (pc->flags & PAGE_CGROUP_FLAG_CACHE) { > + if (page_is_file_cache(page)) > + ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; > + else > + ctype = MEM_CGROUP_CHARGE_TYPE_SHMEM; > + } > } > unlock_page_cgroup(page); > if (mem) { I queued this as a fix against vmscan-split-lru-lists-into-anon-file-sets.patch. Was that appropriate? If the bug you're fixing here is also present in mainline then I'll need to ask for a tested patch against mainline, please. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH -mm] memcg: fix handling of shmem migration(v2) @ 2008-09-17 22:51 ` Andrew Morton 0 siblings, 0 replies; 24+ messages in thread From: Andrew Morton @ 2008-09-17 22:51 UTC (permalink / raw) To: Daisuke Nishimura; +Cc: kamezawa.hiroyu, linux-kernel, linux-mm, balbir, xemul On Wed, 17 Sep 2008 16:55:44 +0900 Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote: > Before this patch, if migrating shmem/tmpfs pages, newpage would be > charged with PAGE_CGROUP_FLAG_FILE set, while oldpage has been charged > without the flag. > > The problem here is mem_cgroup_move_lists doesn't clear(or set) > the PAGE_CGROUP_FLAG_FILE flag, so pc->flags of the newpage > remains PAGE_CGROUP_FLAG_FILE set even when the pc is moved to > another lru(anon) by mem_cgroup_move_lists. And this leads to > incorrect MEM_CGROUP_ZSTAT. > (In my test, I see an underflow of MEM_CGROUP_ZSTAT(active_file). > As a result, mem_cgroup_calc_reclaim returns very huge number and > causes soft lockup on page reclaim.) > > I'm not sure if mem_cgroup_move_lists should handle PAGE_CGROUP_FLAG_FILE > or not(I suppose it should be used to move between active <-> inactive, > not anon <-> file), I added MEM_CGROUP_CHARGE_TYPE_SHMEM for precharge > at shmem's page migration. > > > ChangeLog: v1->v2 > - instead of modifying migrate.c, modify memcontrol.c only. > - add MEM_CGROUP_CHARGE_TYPE_SHMEM. > > > Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> > --- > mm/memcontrol.c | 13 ++++++++++--- > 1 files changed, 10 insertions(+), 3 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 2979d22..ef8812d 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -179,6 +179,7 @@ enum charge_type { > MEM_CGROUP_CHARGE_TYPE_CACHE = 0, > MEM_CGROUP_CHARGE_TYPE_MAPPED, > MEM_CGROUP_CHARGE_TYPE_FORCE, /* used by force_empty */ > + MEM_CGROUP_CHARGE_TYPE_SHMEM, /* used by page migration of shmem */ > }; > > /* > @@ -579,8 +580,10 @@ static int mem_cgroup_charge_common(struct page *page, struct mm_struct *mm, > pc->flags |= PAGE_CGROUP_FLAG_FILE; > else > pc->flags |= PAGE_CGROUP_FLAG_ACTIVE; > - } else > + } else if (ctype == MEM_CGROUP_CHARGE_TYPE_MAPPED) > pc->flags = PAGE_CGROUP_FLAG_ACTIVE; > + else /* MEM_CGROUP_CHARGE_TYPE_SHMEM */ > + pc->flags = PAGE_CGROUP_FLAG_CACHE | PAGE_CGROUP_FLAG_ACTIVE; > > lock_page_cgroup(page); > if (unlikely(page_get_page_cgroup(page))) { > @@ -739,8 +742,12 @@ int mem_cgroup_prepare_migration(struct page *page, struct page *newpage) > if (pc) { > mem = pc->mem_cgroup; > css_get(&mem->css); > - if (pc->flags & PAGE_CGROUP_FLAG_CACHE) > - ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; > + if (pc->flags & PAGE_CGROUP_FLAG_CACHE) { > + if (page_is_file_cache(page)) > + ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; > + else > + ctype = MEM_CGROUP_CHARGE_TYPE_SHMEM; > + } > } > unlock_page_cgroup(page); > if (mem) { I queued this as a fix against vmscan-split-lru-lists-into-anon-file-sets.patch. Was that appropriate? If the bug you're fixing here is also present in mainline then I'll need to ask for a tested patch against mainline, please. -- 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> ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH -mm] memcg: fix handling of shmem migration(v2) 2008-09-17 22:51 ` Andrew Morton @ 2008-09-18 2:03 ` Daisuke Nishimura -1 siblings, 0 replies; 24+ messages in thread From: Daisuke Nishimura @ 2008-09-18 2:03 UTC (permalink / raw) To: Andrew Morton Cc: nishimura, kamezawa.hiroyu, linux-kernel, linux-mm, balbir, xemul On Wed, 17 Sep 2008 15:51:12 -0700, Andrew Morton <akpm@linux-foundation.org> wrote: > On Wed, 17 Sep 2008 16:55:44 +0900 > Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote: > > > Before this patch, if migrating shmem/tmpfs pages, newpage would be > > charged with PAGE_CGROUP_FLAG_FILE set, while oldpage has been charged > > without the flag. > > > > The problem here is mem_cgroup_move_lists doesn't clear(or set) > > the PAGE_CGROUP_FLAG_FILE flag, so pc->flags of the newpage > > remains PAGE_CGROUP_FLAG_FILE set even when the pc is moved to > > another lru(anon) by mem_cgroup_move_lists. And this leads to > > incorrect MEM_CGROUP_ZSTAT. > > (In my test, I see an underflow of MEM_CGROUP_ZSTAT(active_file). > > As a result, mem_cgroup_calc_reclaim returns very huge number and > > causes soft lockup on page reclaim.) > > > > I'm not sure if mem_cgroup_move_lists should handle PAGE_CGROUP_FLAG_FILE > > or not(I suppose it should be used to move between active <-> inactive, > > not anon <-> file), I added MEM_CGROUP_CHARGE_TYPE_SHMEM for precharge > > at shmem's page migration. > > > > > > ChangeLog: v1->v2 > > - instead of modifying migrate.c, modify memcontrol.c only. > > - add MEM_CGROUP_CHARGE_TYPE_SHMEM. > > > > > > Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> > > --- > > mm/memcontrol.c | 13 ++++++++++--- > > 1 files changed, 10 insertions(+), 3 deletions(-) > > > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > index 2979d22..ef8812d 100644 > > --- a/mm/memcontrol.c > > +++ b/mm/memcontrol.c > > @@ -179,6 +179,7 @@ enum charge_type { > > MEM_CGROUP_CHARGE_TYPE_CACHE = 0, > > MEM_CGROUP_CHARGE_TYPE_MAPPED, > > MEM_CGROUP_CHARGE_TYPE_FORCE, /* used by force_empty */ > > + MEM_CGROUP_CHARGE_TYPE_SHMEM, /* used by page migration of shmem */ > > }; > > > > /* > > @@ -579,8 +580,10 @@ static int mem_cgroup_charge_common(struct page *page, struct mm_struct *mm, > > pc->flags |= PAGE_CGROUP_FLAG_FILE; > > else > > pc->flags |= PAGE_CGROUP_FLAG_ACTIVE; > > - } else > > + } else if (ctype == MEM_CGROUP_CHARGE_TYPE_MAPPED) > > pc->flags = PAGE_CGROUP_FLAG_ACTIVE; > > + else /* MEM_CGROUP_CHARGE_TYPE_SHMEM */ > > + pc->flags = PAGE_CGROUP_FLAG_CACHE | PAGE_CGROUP_FLAG_ACTIVE; > > > > lock_page_cgroup(page); > > if (unlikely(page_get_page_cgroup(page))) { > > @@ -739,8 +742,12 @@ int mem_cgroup_prepare_migration(struct page *page, struct page *newpage) > > if (pc) { > > mem = pc->mem_cgroup; > > css_get(&mem->css); > > - if (pc->flags & PAGE_CGROUP_FLAG_CACHE) > > - ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; > > + if (pc->flags & PAGE_CGROUP_FLAG_CACHE) { > > + if (page_is_file_cache(page)) > > + ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; > > + else > > + ctype = MEM_CGROUP_CHARGE_TYPE_SHMEM; > > + } > > } > > unlock_page_cgroup(page); > > if (mem) { > > I queued this as a fix against > vmscan-split-lru-lists-into-anon-file-sets.patch. Was that appropriate? > Yes, thanks. > If the bug you're fixing here is also present in mainline then I'll > need to ask for a tested patch against mainline, please. > I don't think this bug exist in mainline, where memcg have only two ZSTAT(active/inactive) and mem_cgroup_move_lists can handle them properly. Thanks, Daisuke Nishimura. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH -mm] memcg: fix handling of shmem migration(v2) @ 2008-09-18 2:03 ` Daisuke Nishimura 0 siblings, 0 replies; 24+ messages in thread From: Daisuke Nishimura @ 2008-09-18 2:03 UTC (permalink / raw) To: Andrew Morton Cc: nishimura, kamezawa.hiroyu, linux-kernel, linux-mm, balbir, xemul On Wed, 17 Sep 2008 15:51:12 -0700, Andrew Morton <akpm@linux-foundation.org> wrote: > On Wed, 17 Sep 2008 16:55:44 +0900 > Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote: > > > Before this patch, if migrating shmem/tmpfs pages, newpage would be > > charged with PAGE_CGROUP_FLAG_FILE set, while oldpage has been charged > > without the flag. > > > > The problem here is mem_cgroup_move_lists doesn't clear(or set) > > the PAGE_CGROUP_FLAG_FILE flag, so pc->flags of the newpage > > remains PAGE_CGROUP_FLAG_FILE set even when the pc is moved to > > another lru(anon) by mem_cgroup_move_lists. And this leads to > > incorrect MEM_CGROUP_ZSTAT. > > (In my test, I see an underflow of MEM_CGROUP_ZSTAT(active_file). > > As a result, mem_cgroup_calc_reclaim returns very huge number and > > causes soft lockup on page reclaim.) > > > > I'm not sure if mem_cgroup_move_lists should handle PAGE_CGROUP_FLAG_FILE > > or not(I suppose it should be used to move between active <-> inactive, > > not anon <-> file), I added MEM_CGROUP_CHARGE_TYPE_SHMEM for precharge > > at shmem's page migration. > > > > > > ChangeLog: v1->v2 > > - instead of modifying migrate.c, modify memcontrol.c only. > > - add MEM_CGROUP_CHARGE_TYPE_SHMEM. > > > > > > Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> > > --- > > mm/memcontrol.c | 13 ++++++++++--- > > 1 files changed, 10 insertions(+), 3 deletions(-) > > > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > index 2979d22..ef8812d 100644 > > --- a/mm/memcontrol.c > > +++ b/mm/memcontrol.c > > @@ -179,6 +179,7 @@ enum charge_type { > > MEM_CGROUP_CHARGE_TYPE_CACHE = 0, > > MEM_CGROUP_CHARGE_TYPE_MAPPED, > > MEM_CGROUP_CHARGE_TYPE_FORCE, /* used by force_empty */ > > + MEM_CGROUP_CHARGE_TYPE_SHMEM, /* used by page migration of shmem */ > > }; > > > > /* > > @@ -579,8 +580,10 @@ static int mem_cgroup_charge_common(struct page *page, struct mm_struct *mm, > > pc->flags |= PAGE_CGROUP_FLAG_FILE; > > else > > pc->flags |= PAGE_CGROUP_FLAG_ACTIVE; > > - } else > > + } else if (ctype == MEM_CGROUP_CHARGE_TYPE_MAPPED) > > pc->flags = PAGE_CGROUP_FLAG_ACTIVE; > > + else /* MEM_CGROUP_CHARGE_TYPE_SHMEM */ > > + pc->flags = PAGE_CGROUP_FLAG_CACHE | PAGE_CGROUP_FLAG_ACTIVE; > > > > lock_page_cgroup(page); > > if (unlikely(page_get_page_cgroup(page))) { > > @@ -739,8 +742,12 @@ int mem_cgroup_prepare_migration(struct page *page, struct page *newpage) > > if (pc) { > > mem = pc->mem_cgroup; > > css_get(&mem->css); > > - if (pc->flags & PAGE_CGROUP_FLAG_CACHE) > > - ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; > > + if (pc->flags & PAGE_CGROUP_FLAG_CACHE) { > > + if (page_is_file_cache(page)) > > + ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; > > + else > > + ctype = MEM_CGROUP_CHARGE_TYPE_SHMEM; > > + } > > } > > unlock_page_cgroup(page); > > if (mem) { > > I queued this as a fix against > vmscan-split-lru-lists-into-anon-file-sets.patch. Was that appropriate? > Yes, thanks. > If the bug you're fixing here is also present in mainline then I'll > need to ask for a tested patch against mainline, please. > I don't think this bug exist in mainline, where memcg have only two ZSTAT(active/inactive) and mem_cgroup_move_lists can handle them properly. Thanks, Daisuke Nishimura. -- 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> ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH -mm] memcg: fix handling of shmem migration(v2) 2008-09-17 22:51 ` Andrew Morton @ 2008-09-18 2:38 ` KAMEZAWA Hiroyuki -1 siblings, 0 replies; 24+ messages in thread From: KAMEZAWA Hiroyuki @ 2008-09-18 2:38 UTC (permalink / raw) To: Andrew Morton; +Cc: Daisuke Nishimura, linux-kernel, linux-mm, balbir, xemul On Wed, 17 Sep 2008 15:51:12 -0700 Andrew Morton <akpm@linux-foundation.org> wrote: > On Wed, 17 Sep 2008 16:55:44 +0900 > Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote: > > > Before this patch, if migrating shmem/tmpfs pages, newpage would be > > charged with PAGE_CGROUP_FLAG_FILE set, while oldpage has been charged > > without the flag. > > > > The problem here is mem_cgroup_move_lists doesn't clear(or set) > > the PAGE_CGROUP_FLAG_FILE flag, so pc->flags of the newpage > > remains PAGE_CGROUP_FLAG_FILE set even when the pc is moved to > > another lru(anon) by mem_cgroup_move_lists. And this leads to > > incorrect MEM_CGROUP_ZSTAT. > > (In my test, I see an underflow of MEM_CGROUP_ZSTAT(active_file). > > As a result, mem_cgroup_calc_reclaim returns very huge number and > > causes soft lockup on page reclaim.) > > > > I'm not sure if mem_cgroup_move_lists should handle PAGE_CGROUP_FLAG_FILE > > or not(I suppose it should be used to move between active <-> inactive, > > not anon <-> file), I added MEM_CGROUP_CHARGE_TYPE_SHMEM for precharge > > at shmem's page migration. > > > > > > ChangeLog: v1->v2 > > - instead of modifying migrate.c, modify memcontrol.c only. > > - add MEM_CGROUP_CHARGE_TYPE_SHMEM. > > > > > > Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> > > --- > > mm/memcontrol.c | 13 ++++++++++--- > > 1 files changed, 10 insertions(+), 3 deletions(-) > > > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > index 2979d22..ef8812d 100644 > > --- a/mm/memcontrol.c > > +++ b/mm/memcontrol.c > > @@ -179,6 +179,7 @@ enum charge_type { > > MEM_CGROUP_CHARGE_TYPE_CACHE = 0, > > MEM_CGROUP_CHARGE_TYPE_MAPPED, > > MEM_CGROUP_CHARGE_TYPE_FORCE, /* used by force_empty */ > > + MEM_CGROUP_CHARGE_TYPE_SHMEM, /* used by page migration of shmem */ > > }; > > > > /* > > @@ -579,8 +580,10 @@ static int mem_cgroup_charge_common(struct page *page, struct mm_struct *mm, > > pc->flags |= PAGE_CGROUP_FLAG_FILE; > > else > > pc->flags |= PAGE_CGROUP_FLAG_ACTIVE; > > - } else > > + } else if (ctype == MEM_CGROUP_CHARGE_TYPE_MAPPED) > > pc->flags = PAGE_CGROUP_FLAG_ACTIVE; > > + else /* MEM_CGROUP_CHARGE_TYPE_SHMEM */ > > + pc->flags = PAGE_CGROUP_FLAG_CACHE | PAGE_CGROUP_FLAG_ACTIVE; > > > > lock_page_cgroup(page); > > if (unlikely(page_get_page_cgroup(page))) { > > @@ -739,8 +742,12 @@ int mem_cgroup_prepare_migration(struct page *page, struct page *newpage) > > if (pc) { > > mem = pc->mem_cgroup; > > css_get(&mem->css); > > - if (pc->flags & PAGE_CGROUP_FLAG_CACHE) > > - ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; > > + if (pc->flags & PAGE_CGROUP_FLAG_CACHE) { > > + if (page_is_file_cache(page)) > > + ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; > > + else > > + ctype = MEM_CGROUP_CHARGE_TYPE_SHMEM; > > + } > > } > > unlock_page_cgroup(page); > > if (mem) { > > I queued this as a fix against > vmscan-split-lru-lists-into-anon-file-sets.patch. Was that appropriate? > > If the bug you're fixing here is also present in mainline then I'll > need to ask for a tested patch against mainline, please. > I think this bug depends on split-lru patch set. Mayne not in mainline yet...? Thanks, -Kame ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH -mm] memcg: fix handling of shmem migration(v2) @ 2008-09-18 2:38 ` KAMEZAWA Hiroyuki 0 siblings, 0 replies; 24+ messages in thread From: KAMEZAWA Hiroyuki @ 2008-09-18 2:38 UTC (permalink / raw) To: Andrew Morton; +Cc: Daisuke Nishimura, linux-kernel, linux-mm, balbir, xemul On Wed, 17 Sep 2008 15:51:12 -0700 Andrew Morton <akpm@linux-foundation.org> wrote: > On Wed, 17 Sep 2008 16:55:44 +0900 > Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote: > > > Before this patch, if migrating shmem/tmpfs pages, newpage would be > > charged with PAGE_CGROUP_FLAG_FILE set, while oldpage has been charged > > without the flag. > > > > The problem here is mem_cgroup_move_lists doesn't clear(or set) > > the PAGE_CGROUP_FLAG_FILE flag, so pc->flags of the newpage > > remains PAGE_CGROUP_FLAG_FILE set even when the pc is moved to > > another lru(anon) by mem_cgroup_move_lists. And this leads to > > incorrect MEM_CGROUP_ZSTAT. > > (In my test, I see an underflow of MEM_CGROUP_ZSTAT(active_file). > > As a result, mem_cgroup_calc_reclaim returns very huge number and > > causes soft lockup on page reclaim.) > > > > I'm not sure if mem_cgroup_move_lists should handle PAGE_CGROUP_FLAG_FILE > > or not(I suppose it should be used to move between active <-> inactive, > > not anon <-> file), I added MEM_CGROUP_CHARGE_TYPE_SHMEM for precharge > > at shmem's page migration. > > > > > > ChangeLog: v1->v2 > > - instead of modifying migrate.c, modify memcontrol.c only. > > - add MEM_CGROUP_CHARGE_TYPE_SHMEM. > > > > > > Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> > > --- > > mm/memcontrol.c | 13 ++++++++++--- > > 1 files changed, 10 insertions(+), 3 deletions(-) > > > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > index 2979d22..ef8812d 100644 > > --- a/mm/memcontrol.c > > +++ b/mm/memcontrol.c > > @@ -179,6 +179,7 @@ enum charge_type { > > MEM_CGROUP_CHARGE_TYPE_CACHE = 0, > > MEM_CGROUP_CHARGE_TYPE_MAPPED, > > MEM_CGROUP_CHARGE_TYPE_FORCE, /* used by force_empty */ > > + MEM_CGROUP_CHARGE_TYPE_SHMEM, /* used by page migration of shmem */ > > }; > > > > /* > > @@ -579,8 +580,10 @@ static int mem_cgroup_charge_common(struct page *page, struct mm_struct *mm, > > pc->flags |= PAGE_CGROUP_FLAG_FILE; > > else > > pc->flags |= PAGE_CGROUP_FLAG_ACTIVE; > > - } else > > + } else if (ctype == MEM_CGROUP_CHARGE_TYPE_MAPPED) > > pc->flags = PAGE_CGROUP_FLAG_ACTIVE; > > + else /* MEM_CGROUP_CHARGE_TYPE_SHMEM */ > > + pc->flags = PAGE_CGROUP_FLAG_CACHE | PAGE_CGROUP_FLAG_ACTIVE; > > > > lock_page_cgroup(page); > > if (unlikely(page_get_page_cgroup(page))) { > > @@ -739,8 +742,12 @@ int mem_cgroup_prepare_migration(struct page *page, struct page *newpage) > > if (pc) { > > mem = pc->mem_cgroup; > > css_get(&mem->css); > > - if (pc->flags & PAGE_CGROUP_FLAG_CACHE) > > - ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; > > + if (pc->flags & PAGE_CGROUP_FLAG_CACHE) { > > + if (page_is_file_cache(page)) > > + ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; > > + else > > + ctype = MEM_CGROUP_CHARGE_TYPE_SHMEM; > > + } > > } > > unlock_page_cgroup(page); > > if (mem) { > > I queued this as a fix against > vmscan-split-lru-lists-into-anon-file-sets.patch. Was that appropriate? > > If the bug you're fixing here is also present in mainline then I'll > need to ask for a tested patch against mainline, please. > I think this bug depends on split-lru patch set. Mayne not in mainline yet...? Thanks, -Kame -- 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> ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH -mm] memcg: fix handling of shmem migration(v2) 2008-09-18 2:38 ` KAMEZAWA Hiroyuki @ 2008-09-18 5:43 ` Balbir Singh -1 siblings, 0 replies; 24+ messages in thread From: Balbir Singh @ 2008-09-18 5:43 UTC (permalink / raw) To: KAMEZAWA Hiroyuki Cc: Andrew Morton, Daisuke Nishimura, linux-kernel, linux-mm, xemul KAMEZAWA Hiroyuki wrote: > On Wed, 17 Sep 2008 15:51:12 -0700 > Andrew Morton <akpm@linux-foundation.org> wrote: > >> On Wed, 17 Sep 2008 16:55:44 +0900 >> Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote: >> >>> Before this patch, if migrating shmem/tmpfs pages, newpage would be >>> charged with PAGE_CGROUP_FLAG_FILE set, while oldpage has been charged >>> without the flag. >>> >>> The problem here is mem_cgroup_move_lists doesn't clear(or set) >>> the PAGE_CGROUP_FLAG_FILE flag, so pc->flags of the newpage >>> remains PAGE_CGROUP_FLAG_FILE set even when the pc is moved to >>> another lru(anon) by mem_cgroup_move_lists. And this leads to >>> incorrect MEM_CGROUP_ZSTAT. >>> (In my test, I see an underflow of MEM_CGROUP_ZSTAT(active_file). >>> As a result, mem_cgroup_calc_reclaim returns very huge number and >>> causes soft lockup on page reclaim.) >>> >>> I'm not sure if mem_cgroup_move_lists should handle PAGE_CGROUP_FLAG_FILE >>> or not(I suppose it should be used to move between active <-> inactive, >>> not anon <-> file), I added MEM_CGROUP_CHARGE_TYPE_SHMEM for precharge >>> at shmem's page migration. >>> >>> >>> ChangeLog: v1->v2 >>> - instead of modifying migrate.c, modify memcontrol.c only. >>> - add MEM_CGROUP_CHARGE_TYPE_SHMEM. >>> >>> >>> Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> >>> --- >>> mm/memcontrol.c | 13 ++++++++++--- >>> 1 files changed, 10 insertions(+), 3 deletions(-) >>> >>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c >>> index 2979d22..ef8812d 100644 >>> --- a/mm/memcontrol.c >>> +++ b/mm/memcontrol.c >>> @@ -179,6 +179,7 @@ enum charge_type { >>> MEM_CGROUP_CHARGE_TYPE_CACHE = 0, >>> MEM_CGROUP_CHARGE_TYPE_MAPPED, >>> MEM_CGROUP_CHARGE_TYPE_FORCE, /* used by force_empty */ >>> + MEM_CGROUP_CHARGE_TYPE_SHMEM, /* used by page migration of shmem */ >>> }; >>> >>> /* >>> @@ -579,8 +580,10 @@ static int mem_cgroup_charge_common(struct page *page, struct mm_struct *mm, >>> pc->flags |= PAGE_CGROUP_FLAG_FILE; >>> else >>> pc->flags |= PAGE_CGROUP_FLAG_ACTIVE; >>> - } else >>> + } else if (ctype == MEM_CGROUP_CHARGE_TYPE_MAPPED) >>> pc->flags = PAGE_CGROUP_FLAG_ACTIVE; >>> + else /* MEM_CGROUP_CHARGE_TYPE_SHMEM */ >>> + pc->flags = PAGE_CGROUP_FLAG_CACHE | PAGE_CGROUP_FLAG_ACTIVE; >>> >>> lock_page_cgroup(page); >>> if (unlikely(page_get_page_cgroup(page))) { >>> @@ -739,8 +742,12 @@ int mem_cgroup_prepare_migration(struct page *page, struct page *newpage) >>> if (pc) { >>> mem = pc->mem_cgroup; >>> css_get(&mem->css); >>> - if (pc->flags & PAGE_CGROUP_FLAG_CACHE) >>> - ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; >>> + if (pc->flags & PAGE_CGROUP_FLAG_CACHE) { >>> + if (page_is_file_cache(page)) >>> + ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; >>> + else >>> + ctype = MEM_CGROUP_CHARGE_TYPE_SHMEM; >>> + } >>> } >>> unlock_page_cgroup(page); >>> if (mem) { >> I queued this as a fix against >> vmscan-split-lru-lists-into-anon-file-sets.patch. Was that appropriate? >> >> If the bug you're fixing here is also present in mainline then I'll >> need to ask for a tested patch against mainline, please. >> > I think this bug depends on split-lru patch set. > Mayne not in mainline yet...? Yes, that sounds correct to me, this should be a -mm only patch. -- Balbir ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH -mm] memcg: fix handling of shmem migration(v2) @ 2008-09-18 5:43 ` Balbir Singh 0 siblings, 0 replies; 24+ messages in thread From: Balbir Singh @ 2008-09-18 5:43 UTC (permalink / raw) To: KAMEZAWA Hiroyuki Cc: Andrew Morton, Daisuke Nishimura, linux-kernel, linux-mm, xemul KAMEZAWA Hiroyuki wrote: > On Wed, 17 Sep 2008 15:51:12 -0700 > Andrew Morton <akpm@linux-foundation.org> wrote: > >> On Wed, 17 Sep 2008 16:55:44 +0900 >> Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote: >> >>> Before this patch, if migrating shmem/tmpfs pages, newpage would be >>> charged with PAGE_CGROUP_FLAG_FILE set, while oldpage has been charged >>> without the flag. >>> >>> The problem here is mem_cgroup_move_lists doesn't clear(or set) >>> the PAGE_CGROUP_FLAG_FILE flag, so pc->flags of the newpage >>> remains PAGE_CGROUP_FLAG_FILE set even when the pc is moved to >>> another lru(anon) by mem_cgroup_move_lists. And this leads to >>> incorrect MEM_CGROUP_ZSTAT. >>> (In my test, I see an underflow of MEM_CGROUP_ZSTAT(active_file). >>> As a result, mem_cgroup_calc_reclaim returns very huge number and >>> causes soft lockup on page reclaim.) >>> >>> I'm not sure if mem_cgroup_move_lists should handle PAGE_CGROUP_FLAG_FILE >>> or not(I suppose it should be used to move between active <-> inactive, >>> not anon <-> file), I added MEM_CGROUP_CHARGE_TYPE_SHMEM for precharge >>> at shmem's page migration. >>> >>> >>> ChangeLog: v1->v2 >>> - instead of modifying migrate.c, modify memcontrol.c only. >>> - add MEM_CGROUP_CHARGE_TYPE_SHMEM. >>> >>> >>> Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> >>> --- >>> mm/memcontrol.c | 13 ++++++++++--- >>> 1 files changed, 10 insertions(+), 3 deletions(-) >>> >>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c >>> index 2979d22..ef8812d 100644 >>> --- a/mm/memcontrol.c >>> +++ b/mm/memcontrol.c >>> @@ -179,6 +179,7 @@ enum charge_type { >>> MEM_CGROUP_CHARGE_TYPE_CACHE = 0, >>> MEM_CGROUP_CHARGE_TYPE_MAPPED, >>> MEM_CGROUP_CHARGE_TYPE_FORCE, /* used by force_empty */ >>> + MEM_CGROUP_CHARGE_TYPE_SHMEM, /* used by page migration of shmem */ >>> }; >>> >>> /* >>> @@ -579,8 +580,10 @@ static int mem_cgroup_charge_common(struct page *page, struct mm_struct *mm, >>> pc->flags |= PAGE_CGROUP_FLAG_FILE; >>> else >>> pc->flags |= PAGE_CGROUP_FLAG_ACTIVE; >>> - } else >>> + } else if (ctype == MEM_CGROUP_CHARGE_TYPE_MAPPED) >>> pc->flags = PAGE_CGROUP_FLAG_ACTIVE; >>> + else /* MEM_CGROUP_CHARGE_TYPE_SHMEM */ >>> + pc->flags = PAGE_CGROUP_FLAG_CACHE | PAGE_CGROUP_FLAG_ACTIVE; >>> >>> lock_page_cgroup(page); >>> if (unlikely(page_get_page_cgroup(page))) { >>> @@ -739,8 +742,12 @@ int mem_cgroup_prepare_migration(struct page *page, struct page *newpage) >>> if (pc) { >>> mem = pc->mem_cgroup; >>> css_get(&mem->css); >>> - if (pc->flags & PAGE_CGROUP_FLAG_CACHE) >>> - ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; >>> + if (pc->flags & PAGE_CGROUP_FLAG_CACHE) { >>> + if (page_is_file_cache(page)) >>> + ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; >>> + else >>> + ctype = MEM_CGROUP_CHARGE_TYPE_SHMEM; >>> + } >>> } >>> unlock_page_cgroup(page); >>> if (mem) { >> I queued this as a fix against >> vmscan-split-lru-lists-into-anon-file-sets.patch. Was that appropriate? >> >> If the bug you're fixing here is also present in mainline then I'll >> need to ask for a tested patch against mainline, please. >> > I think this bug depends on split-lru patch set. > Mayne not in mainline yet...? Yes, that sounds correct to me, this should be a -mm only patch. -- Balbir -- 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> ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2008-09-18 5:45 UTC | newest] Thread overview: 24+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-09-17 4:31 [PATCH -mm] memcg: fix handling of shmem migration Daisuke Nishimura 2008-09-17 4:31 ` Daisuke Nishimura 2008-09-17 5:46 ` KAMEZAWA Hiroyuki 2008-09-17 5:46 ` KAMEZAWA Hiroyuki 2008-09-17 5:50 ` KAMEZAWA Hiroyuki 2008-09-17 5:50 ` KAMEZAWA Hiroyuki 2008-09-17 6:19 ` Daisuke Nishimura 2008-09-17 6:19 ` Daisuke Nishimura 2008-09-17 6:38 ` KAMEZAWA Hiroyuki 2008-09-17 6:38 ` KAMEZAWA Hiroyuki 2008-09-17 6:45 ` Daisuke Nishimura 2008-09-17 6:45 ` Daisuke Nishimura 2008-09-17 7:55 ` [PATCH -mm] memcg: fix handling of shmem migration(v2) Daisuke Nishimura 2008-09-17 7:55 ` Daisuke Nishimura 2008-09-17 9:18 ` KAMEZAWA Hiroyuki 2008-09-17 9:18 ` KAMEZAWA Hiroyuki 2008-09-17 22:51 ` Andrew Morton 2008-09-17 22:51 ` Andrew Morton 2008-09-18 2:03 ` Daisuke Nishimura 2008-09-18 2:03 ` Daisuke Nishimura 2008-09-18 2:38 ` KAMEZAWA Hiroyuki 2008-09-18 2:38 ` KAMEZAWA Hiroyuki 2008-09-18 5:43 ` Balbir Singh 2008-09-18 5:43 ` Balbir Singh
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.