* [PATCH -mm] memcg: fix handling of shmem migration
@ 2008-09-17 4:31 Daisuke Nishimura
2008-09-17 5:46 ` KAMEZAWA Hiroyuki
0 siblings, 1 reply; 12+ 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 [flat|nested] 12+ messages in thread* Re: [PATCH -mm] memcg: fix handling of shmem migration
2008-09-17 4:31 [PATCH -mm] memcg: fix handling of shmem migration Daisuke Nishimura
@ 2008-09-17 5:46 ` KAMEZAWA Hiroyuki
2008-09-17 5:50 ` KAMEZAWA Hiroyuki
0 siblings, 1 reply; 12+ 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] 12+ 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
2008-09-17 6:19 ` Daisuke Nishimura
0 siblings, 1 reply; 12+ 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] 12+ 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
2008-09-17 6:38 ` KAMEZAWA Hiroyuki
0 siblings, 1 reply; 12+ 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] 12+ 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
2008-09-17 6:45 ` Daisuke Nishimura
0 siblings, 1 reply; 12+ 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] 12+ 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
2008-09-17 7:55 ` [PATCH -mm] memcg: fix handling of shmem migration(v2) Daisuke Nishimura
0 siblings, 1 reply; 12+ 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] 12+ 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
2008-09-17 9:18 ` KAMEZAWA Hiroyuki
2008-09-17 22:51 ` Andrew Morton
0 siblings, 2 replies; 12+ 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 [flat|nested] 12+ messages in thread* Re: [PATCH -mm] memcg: fix handling of shmem migration(v2)
2008-09-17 7:55 ` [PATCH -mm] memcg: fix handling of shmem migration(v2) Daisuke Nishimura
@ 2008-09-17 9:18 ` KAMEZAWA Hiroyuki
2008-09-17 22:51 ` Andrew Morton
1 sibling, 0 replies; 12+ 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] 12+ messages in thread
* Re: [PATCH -mm] memcg: fix handling of shmem migration(v2)
2008-09-17 7:55 ` [PATCH -mm] memcg: fix handling of shmem migration(v2) Daisuke Nishimura
2008-09-17 9:18 ` KAMEZAWA Hiroyuki
@ 2008-09-17 22:51 ` Andrew Morton
2008-09-18 2:03 ` Daisuke Nishimura
2008-09-18 2:38 ` KAMEZAWA Hiroyuki
1 sibling, 2 replies; 12+ 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] 12+ 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
2008-09-18 2:38 ` KAMEZAWA Hiroyuki
1 sibling, 0 replies; 12+ 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] 12+ 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
@ 2008-09-18 2:38 ` KAMEZAWA Hiroyuki
2008-09-18 5:43 ` Balbir Singh
1 sibling, 1 reply; 12+ 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] 12+ 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
0 siblings, 0 replies; 12+ 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] 12+ messages in thread
end of thread, other threads:[~2008-09-18 5:44 UTC | newest]
Thread overview: 12+ 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 5:46 ` KAMEZAWA Hiroyuki
2008-09-17 5:50 ` KAMEZAWA Hiroyuki
2008-09-17 6:19 ` Daisuke Nishimura
2008-09-17 6:38 ` KAMEZAWA Hiroyuki
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 9:18 ` KAMEZAWA Hiroyuki
2008-09-17 22:51 ` Andrew Morton
2008-09-18 2:03 ` Daisuke Nishimura
2008-09-18 2:38 ` KAMEZAWA Hiroyuki
2008-09-18 5:43 ` Balbir Singh
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox