* [PATCH] memcg: fix Bad page state after replace_page_cache @ 2012-04-19 6:34 ` Hugh Dickins 0 siblings, 0 replies; 10+ messages in thread From: Hugh Dickins @ 2012-04-19 6:34 UTC (permalink / raw) To: Linus Torvalds Cc: Andrew Morton, Miklos Szeredi, KAMEZAWA Hiroyuki, Johannes Weiner, Michal Hocko, linux-kernel, linux-mm My 9ce70c0240d0 "memcg: fix deadlock by inverting lrucare nesting" put a nasty little bug into v3.3's version of mem_cgroup_replace_page_cache(), sometimes used for FUSE. Replacing __mem_cgroup_commit_charge_lrucare() by __mem_cgroup_commit_charge(), I used the "pc" pointer set up earlier: but it's for oldpage, and needs now to be for newpage. Once oldpage was freed, its PageCgroupUsed bit (cleared above but set again here) caused "Bad page state" messages - and perhaps worse, being missed from newpage. (I didn't find this by using FUSE, but in reusing the function for tmpfs.) Signed-off-by: Hugh Dickins <hughd@google.com> Cc: stable@vger.kernel.org [v3.3 only] --- mm/memcontrol.c | 1 + 1 file changed, 1 insertion(+) --- 3.4-rc3/mm/memcontrol.c 2012-04-15 20:47:37.151777506 -0700 +++ linux/mm/memcontrol.c 2012-04-18 22:29:18.490639511 -0700 @@ -3392,6 +3392,7 @@ void mem_cgroup_replace_page_cache(struc * the newpage may be on LRU(or pagevec for LRU) already. We lock * LRU while we overwrite pc->mem_cgroup. */ + pc = lookup_page_cgroup(newpage); __mem_cgroup_commit_charge(memcg, newpage, 1, pc, type, true); } -- 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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] memcg: fix Bad page state after replace_page_cache @ 2012-04-19 6:34 ` Hugh Dickins 0 siblings, 0 replies; 10+ messages in thread From: Hugh Dickins @ 2012-04-19 6:34 UTC (permalink / raw) To: Linus Torvalds Cc: Andrew Morton, Miklos Szeredi, KAMEZAWA Hiroyuki, Johannes Weiner, Michal Hocko, linux-kernel, linux-mm My 9ce70c0240d0 "memcg: fix deadlock by inverting lrucare nesting" put a nasty little bug into v3.3's version of mem_cgroup_replace_page_cache(), sometimes used for FUSE. Replacing __mem_cgroup_commit_charge_lrucare() by __mem_cgroup_commit_charge(), I used the "pc" pointer set up earlier: but it's for oldpage, and needs now to be for newpage. Once oldpage was freed, its PageCgroupUsed bit (cleared above but set again here) caused "Bad page state" messages - and perhaps worse, being missed from newpage. (I didn't find this by using FUSE, but in reusing the function for tmpfs.) Signed-off-by: Hugh Dickins <hughd@google.com> Cc: stable@vger.kernel.org [v3.3 only] --- mm/memcontrol.c | 1 + 1 file changed, 1 insertion(+) --- 3.4-rc3/mm/memcontrol.c 2012-04-15 20:47:37.151777506 -0700 +++ linux/mm/memcontrol.c 2012-04-18 22:29:18.490639511 -0700 @@ -3392,6 +3392,7 @@ void mem_cgroup_replace_page_cache(struc * the newpage may be on LRU(or pagevec for LRU) already. We lock * LRU while we overwrite pc->mem_cgroup. */ + pc = lookup_page_cgroup(newpage); __mem_cgroup_commit_charge(memcg, newpage, 1, pc, type, true); } ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] memcg: fix Bad page state after replace_page_cache 2012-04-19 6:34 ` Hugh Dickins @ 2012-04-19 10:20 ` Michal Hocko -1 siblings, 0 replies; 10+ messages in thread From: Michal Hocko @ 2012-04-19 10:20 UTC (permalink / raw) To: Hugh Dickins Cc: Linus Torvalds, Andrew Morton, Miklos Szeredi, KAMEZAWA Hiroyuki, Johannes Weiner, linux-kernel, linux-mm On Wed 18-04-12 23:34:46, Hugh Dickins wrote: > My 9ce70c0240d0 "memcg: fix deadlock by inverting lrucare nesting" put a > nasty little bug into v3.3's version of mem_cgroup_replace_page_cache(), > sometimes used for FUSE. Replacing __mem_cgroup_commit_charge_lrucare() > by __mem_cgroup_commit_charge(), I used the "pc" pointer set up earlier: > but it's for oldpage, and needs now to be for newpage. Once oldpage was > freed, its PageCgroupUsed bit (cleared above but set again here) caused > "Bad page state" messages - and perhaps worse, being missed from newpage. > (I didn't find this by using FUSE, but in reusing the function for tmpfs.) > > Signed-off-by: Hugh Dickins <hughd@google.com> Acked-by: Michal Hocko <mhocko@suse.cz> Thanks > Cc: stable@vger.kernel.org [v3.3 only] > --- > > mm/memcontrol.c | 1 + > 1 file changed, 1 insertion(+) > > --- 3.4-rc3/mm/memcontrol.c 2012-04-15 20:47:37.151777506 -0700 > +++ linux/mm/memcontrol.c 2012-04-18 22:29:18.490639511 -0700 > @@ -3392,6 +3392,7 @@ void mem_cgroup_replace_page_cache(struc > * the newpage may be on LRU(or pagevec for LRU) already. We lock > * LRU while we overwrite pc->mem_cgroup. > */ > + pc = lookup_page_cgroup(newpage); > __mem_cgroup_commit_charge(memcg, newpage, 1, pc, type, true); > } > > > -- > 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/ . > Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ > Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> -- Michal Hocko SUSE Labs SUSE LINUX s.r.o. Lihovarska 1060/12 190 00 Praha 9 Czech Republic -- 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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] memcg: fix Bad page state after replace_page_cache @ 2012-04-19 10:20 ` Michal Hocko 0 siblings, 0 replies; 10+ messages in thread From: Michal Hocko @ 2012-04-19 10:20 UTC (permalink / raw) To: Hugh Dickins Cc: Linus Torvalds, Andrew Morton, Miklos Szeredi, KAMEZAWA Hiroyuki, Johannes Weiner, linux-kernel, linux-mm On Wed 18-04-12 23:34:46, Hugh Dickins wrote: > My 9ce70c0240d0 "memcg: fix deadlock by inverting lrucare nesting" put a > nasty little bug into v3.3's version of mem_cgroup_replace_page_cache(), > sometimes used for FUSE. Replacing __mem_cgroup_commit_charge_lrucare() > by __mem_cgroup_commit_charge(), I used the "pc" pointer set up earlier: > but it's for oldpage, and needs now to be for newpage. Once oldpage was > freed, its PageCgroupUsed bit (cleared above but set again here) caused > "Bad page state" messages - and perhaps worse, being missed from newpage. > (I didn't find this by using FUSE, but in reusing the function for tmpfs.) > > Signed-off-by: Hugh Dickins <hughd@google.com> Acked-by: Michal Hocko <mhocko@suse.cz> Thanks > Cc: stable@vger.kernel.org [v3.3 only] > --- > > mm/memcontrol.c | 1 + > 1 file changed, 1 insertion(+) > > --- 3.4-rc3/mm/memcontrol.c 2012-04-15 20:47:37.151777506 -0700 > +++ linux/mm/memcontrol.c 2012-04-18 22:29:18.490639511 -0700 > @@ -3392,6 +3392,7 @@ void mem_cgroup_replace_page_cache(struc > * the newpage may be on LRU(or pagevec for LRU) already. We lock > * LRU while we overwrite pc->mem_cgroup. > */ > + pc = lookup_page_cgroup(newpage); > __mem_cgroup_commit_charge(memcg, newpage, 1, pc, type, true); > } > > > -- > 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/ . > Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ > Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> -- Michal Hocko SUSE Labs SUSE LINUX s.r.o. Lihovarska 1060/12 190 00 Praha 9 Czech Republic ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] memcg: fix Bad page state after replace_page_cache 2012-04-19 6:34 ` Hugh Dickins @ 2012-04-20 1:47 ` KAMEZAWA Hiroyuki -1 siblings, 0 replies; 10+ messages in thread From: KAMEZAWA Hiroyuki @ 2012-04-20 1:47 UTC (permalink / raw) To: Hugh Dickins Cc: Linus Torvalds, Andrew Morton, Miklos Szeredi, Johannes Weiner, Michal Hocko, linux-kernel, linux-mm (2012/04/19 15:34), Hugh Dickins wrote: > My 9ce70c0240d0 "memcg: fix deadlock by inverting lrucare nesting" put a > nasty little bug into v3.3's version of mem_cgroup_replace_page_cache(), > sometimes used for FUSE. Replacing __mem_cgroup_commit_charge_lrucare() > by __mem_cgroup_commit_charge(), I used the "pc" pointer set up earlier: > but it's for oldpage, and needs now to be for newpage. Once oldpage was > freed, its PageCgroupUsed bit (cleared above but set again here) caused > "Bad page state" messages - and perhaps worse, being missed from newpage. > (I didn't find this by using FUSE, but in reusing the function for tmpfs.) > > Signed-off-by: Hugh Dickins <hughd@google.com> > Cc: stable@vger.kernel.org [v3.3 only] Thanks, Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > --- > > mm/memcontrol.c | 1 + > 1 file changed, 1 insertion(+) > > --- 3.4-rc3/mm/memcontrol.c 2012-04-15 20:47:37.151777506 -0700 > +++ linux/mm/memcontrol.c 2012-04-18 22:29:18.490639511 -0700 > @@ -3392,6 +3392,7 @@ void mem_cgroup_replace_page_cache(struc > * the newpage may be on LRU(or pagevec for LRU) already. We lock > * LRU while we overwrite pc->mem_cgroup. > */ > + pc = lookup_page_cgroup(newpage); > __mem_cgroup_commit_charge(memcg, newpage, 1, pc, type, true); > } > > > -- > 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/ . > Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ > Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> > > -- 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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] memcg: fix Bad page state after replace_page_cache @ 2012-04-20 1:47 ` KAMEZAWA Hiroyuki 0 siblings, 0 replies; 10+ messages in thread From: KAMEZAWA Hiroyuki @ 2012-04-20 1:47 UTC (permalink / raw) To: Hugh Dickins Cc: Linus Torvalds, Andrew Morton, Miklos Szeredi, Johannes Weiner, Michal Hocko, linux-kernel, linux-mm (2012/04/19 15:34), Hugh Dickins wrote: > My 9ce70c0240d0 "memcg: fix deadlock by inverting lrucare nesting" put a > nasty little bug into v3.3's version of mem_cgroup_replace_page_cache(), > sometimes used for FUSE. Replacing __mem_cgroup_commit_charge_lrucare() > by __mem_cgroup_commit_charge(), I used the "pc" pointer set up earlier: > but it's for oldpage, and needs now to be for newpage. Once oldpage was > freed, its PageCgroupUsed bit (cleared above but set again here) caused > "Bad page state" messages - and perhaps worse, being missed from newpage. > (I didn't find this by using FUSE, but in reusing the function for tmpfs.) > > Signed-off-by: Hugh Dickins <hughd@google.com> > Cc: stable@vger.kernel.org [v3.3 only] Thanks, Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > --- > > mm/memcontrol.c | 1 + > 1 file changed, 1 insertion(+) > > --- 3.4-rc3/mm/memcontrol.c 2012-04-15 20:47:37.151777506 -0700 > +++ linux/mm/memcontrol.c 2012-04-18 22:29:18.490639511 -0700 > @@ -3392,6 +3392,7 @@ void mem_cgroup_replace_page_cache(struc > * the newpage may be on LRU(or pagevec for LRU) already. We lock > * LRU while we overwrite pc->mem_cgroup. > */ > + pc = lookup_page_cgroup(newpage); > __mem_cgroup_commit_charge(memcg, newpage, 1, pc, type, true); > } > > > -- > 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/ . > Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ > Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] memcg: fix Bad page state after replace_page_cache 2012-04-19 6:34 ` Hugh Dickins @ 2012-04-24 18:22 ` Johannes Weiner -1 siblings, 0 replies; 10+ messages in thread From: Johannes Weiner @ 2012-04-24 18:22 UTC (permalink / raw) To: Hugh Dickins Cc: Linus Torvalds, Andrew Morton, Miklos Szeredi, KAMEZAWA Hiroyuki, Michal Hocko, linux-kernel, linux-mm On Wed, Apr 18, 2012 at 11:34:46PM -0700, Hugh Dickins wrote: > My 9ce70c0240d0 "memcg: fix deadlock by inverting lrucare nesting" put a > nasty little bug into v3.3's version of mem_cgroup_replace_page_cache(), > sometimes used for FUSE. Replacing __mem_cgroup_commit_charge_lrucare() > by __mem_cgroup_commit_charge(), I used the "pc" pointer set up earlier: > but it's for oldpage, and needs now to be for newpage. Once oldpage was > freed, its PageCgroupUsed bit (cleared above but set again here) caused > "Bad page state" messages - and perhaps worse, being missed from newpage. > (I didn't find this by using FUSE, but in reusing the function for tmpfs.) > > Signed-off-by: Hugh Dickins <hughd@google.com> > Cc: stable@vger.kernel.org [v3.3 only] Acked-by: Johannes Weiner <hannes@cmpxchg.org> This is the second bug of this nature and none of the callsites are actually using the pc themselves, so we should probably move the pc lookup into __mem_cgroup_commit_charge() itself. --- From: Johannes Weiner <hannes@cmpxchg.org> Subject: [patch] mm: memcg: move pc lookup point to commit_charge() None of the callsites actually need the page_cgroup descriptor themselves, so just pass the page and do the look up in there. We already had two bugs (6568d4a 'mm: memcg: update the correct soft limit tree during migration' and 'memcg: fix Bad page state after replace_page_cache') where the passed page and pc were not referring to the same page frame. Signed-off-by: Johannes Weiner <hannes@cmpxchg.org> --- mm/memcontrol.c | 17 +++++------------ 1 files changed, 5 insertions(+), 12 deletions(-) diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 884e936..1a28dd8 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -2461,10 +2461,10 @@ struct mem_cgroup *try_get_mem_cgroup_from_page(struct page *page) static void __mem_cgroup_commit_charge(struct mem_cgroup *memcg, struct page *page, unsigned int nr_pages, - struct page_cgroup *pc, enum charge_type ctype, bool lrucare) { + struct page_cgroup *pc = lookup_page_cgroup(page); struct zone *uninitialized_var(zone); bool was_on_lru = false; bool anon; @@ -2701,7 +2701,6 @@ static int mem_cgroup_charge_common(struct page *page, struct mm_struct *mm, { struct mem_cgroup *memcg = NULL; unsigned int nr_pages = 1; - struct page_cgroup *pc; bool oom = true; int ret; @@ -2715,11 +2714,10 @@ static int mem_cgroup_charge_common(struct page *page, struct mm_struct *mm, oom = false; } - pc = lookup_page_cgroup(page); ret = __mem_cgroup_try_charge(mm, gfp_mask, nr_pages, &memcg, oom); if (ret == -ENOMEM) return ret; - __mem_cgroup_commit_charge(memcg, page, nr_pages, pc, ctype, false); + __mem_cgroup_commit_charge(memcg, page, nr_pages, ctype, false); return 0; } @@ -2816,16 +2814,13 @@ static void __mem_cgroup_commit_charge_swapin(struct page *page, struct mem_cgroup *memcg, enum charge_type ctype) { - struct page_cgroup *pc; - if (mem_cgroup_disabled()) return; if (!memcg) return; cgroup_exclude_rmdir(&memcg->css); - pc = lookup_page_cgroup(page); - __mem_cgroup_commit_charge(memcg, page, 1, pc, ctype, true); + __mem_cgroup_commit_charge(memcg, page, 1, ctype, true); /* * Now swap is on-memory. This means this page may be * counted both as mem and swap....double count. @@ -3254,14 +3249,13 @@ int mem_cgroup_prepare_migration(struct page *page, * page. In the case new page is migrated but not remapped, new page's * mapcount will be finally 0 and we call uncharge in end_migration(). */ - pc = lookup_page_cgroup(newpage); if (PageAnon(page)) ctype = MEM_CGROUP_CHARGE_TYPE_MAPPED; else if (page_is_file_cache(page)) ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; else ctype = MEM_CGROUP_CHARGE_TYPE_SHMEM; - __mem_cgroup_commit_charge(memcg, newpage, 1, pc, ctype, false); + __mem_cgroup_commit_charge(memcg, newpage, 1, ctype, false); return ret; } @@ -3348,8 +3342,7 @@ void mem_cgroup_replace_page_cache(struct page *oldpage, * the newpage may be on LRU(or pagevec for LRU) already. We lock * LRU while we overwrite pc->mem_cgroup. */ - pc = lookup_page_cgroup(newpage); - __mem_cgroup_commit_charge(memcg, newpage, 1, pc, type, true); + __mem_cgroup_commit_charge(memcg, newpage, 1, type, true); } #ifdef CONFIG_DEBUG_VM -- 1.7.7.6 -- 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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] memcg: fix Bad page state after replace_page_cache @ 2012-04-24 18:22 ` Johannes Weiner 0 siblings, 0 replies; 10+ messages in thread From: Johannes Weiner @ 2012-04-24 18:22 UTC (permalink / raw) To: Hugh Dickins Cc: Linus Torvalds, Andrew Morton, Miklos Szeredi, KAMEZAWA Hiroyuki, Michal Hocko, linux-kernel, linux-mm On Wed, Apr 18, 2012 at 11:34:46PM -0700, Hugh Dickins wrote: > My 9ce70c0240d0 "memcg: fix deadlock by inverting lrucare nesting" put a > nasty little bug into v3.3's version of mem_cgroup_replace_page_cache(), > sometimes used for FUSE. Replacing __mem_cgroup_commit_charge_lrucare() > by __mem_cgroup_commit_charge(), I used the "pc" pointer set up earlier: > but it's for oldpage, and needs now to be for newpage. Once oldpage was > freed, its PageCgroupUsed bit (cleared above but set again here) caused > "Bad page state" messages - and perhaps worse, being missed from newpage. > (I didn't find this by using FUSE, but in reusing the function for tmpfs.) > > Signed-off-by: Hugh Dickins <hughd@google.com> > Cc: stable@vger.kernel.org [v3.3 only] Acked-by: Johannes Weiner <hannes@cmpxchg.org> This is the second bug of this nature and none of the callsites are actually using the pc themselves, so we should probably move the pc lookup into __mem_cgroup_commit_charge() itself. --- From: Johannes Weiner <hannes@cmpxchg.org> Subject: [patch] mm: memcg: move pc lookup point to commit_charge() None of the callsites actually need the page_cgroup descriptor themselves, so just pass the page and do the look up in there. We already had two bugs (6568d4a 'mm: memcg: update the correct soft limit tree during migration' and 'memcg: fix Bad page state after replace_page_cache') where the passed page and pc were not referring to the same page frame. Signed-off-by: Johannes Weiner <hannes@cmpxchg.org> --- mm/memcontrol.c | 17 +++++------------ 1 files changed, 5 insertions(+), 12 deletions(-) diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 884e936..1a28dd8 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -2461,10 +2461,10 @@ struct mem_cgroup *try_get_mem_cgroup_from_page(struct page *page) static void __mem_cgroup_commit_charge(struct mem_cgroup *memcg, struct page *page, unsigned int nr_pages, - struct page_cgroup *pc, enum charge_type ctype, bool lrucare) { + struct page_cgroup *pc = lookup_page_cgroup(page); struct zone *uninitialized_var(zone); bool was_on_lru = false; bool anon; @@ -2701,7 +2701,6 @@ static int mem_cgroup_charge_common(struct page *page, struct mm_struct *mm, { struct mem_cgroup *memcg = NULL; unsigned int nr_pages = 1; - struct page_cgroup *pc; bool oom = true; int ret; @@ -2715,11 +2714,10 @@ static int mem_cgroup_charge_common(struct page *page, struct mm_struct *mm, oom = false; } - pc = lookup_page_cgroup(page); ret = __mem_cgroup_try_charge(mm, gfp_mask, nr_pages, &memcg, oom); if (ret == -ENOMEM) return ret; - __mem_cgroup_commit_charge(memcg, page, nr_pages, pc, ctype, false); + __mem_cgroup_commit_charge(memcg, page, nr_pages, ctype, false); return 0; } @@ -2816,16 +2814,13 @@ static void __mem_cgroup_commit_charge_swapin(struct page *page, struct mem_cgroup *memcg, enum charge_type ctype) { - struct page_cgroup *pc; - if (mem_cgroup_disabled()) return; if (!memcg) return; cgroup_exclude_rmdir(&memcg->css); - pc = lookup_page_cgroup(page); - __mem_cgroup_commit_charge(memcg, page, 1, pc, ctype, true); + __mem_cgroup_commit_charge(memcg, page, 1, ctype, true); /* * Now swap is on-memory. This means this page may be * counted both as mem and swap....double count. @@ -3254,14 +3249,13 @@ int mem_cgroup_prepare_migration(struct page *page, * page. In the case new page is migrated but not remapped, new page's * mapcount will be finally 0 and we call uncharge in end_migration(). */ - pc = lookup_page_cgroup(newpage); if (PageAnon(page)) ctype = MEM_CGROUP_CHARGE_TYPE_MAPPED; else if (page_is_file_cache(page)) ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; else ctype = MEM_CGROUP_CHARGE_TYPE_SHMEM; - __mem_cgroup_commit_charge(memcg, newpage, 1, pc, ctype, false); + __mem_cgroup_commit_charge(memcg, newpage, 1, ctype, false); return ret; } @@ -3348,8 +3342,7 @@ void mem_cgroup_replace_page_cache(struct page *oldpage, * the newpage may be on LRU(or pagevec for LRU) already. We lock * LRU while we overwrite pc->mem_cgroup. */ - pc = lookup_page_cgroup(newpage); - __mem_cgroup_commit_charge(memcg, newpage, 1, pc, type, true); + __mem_cgroup_commit_charge(memcg, newpage, 1, type, true); } #ifdef CONFIG_DEBUG_VM -- 1.7.7.6 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] memcg: fix Bad page state after replace_page_cache 2012-04-24 18:22 ` Johannes Weiner @ 2012-04-24 19:13 ` Hugh Dickins -1 siblings, 0 replies; 10+ messages in thread From: Hugh Dickins @ 2012-04-24 19:13 UTC (permalink / raw) To: Johannes Weiner Cc: Linus Torvalds, Andrew Morton, Miklos Szeredi, KAMEZAWA Hiroyuki, Michal Hocko, linux-kernel, linux-mm On Tue, 24 Apr 2012, Johannes Weiner wrote: > From: Johannes Weiner <hannes@cmpxchg.org> > Subject: [patch] mm: memcg: move pc lookup point to commit_charge() > > None of the callsites actually need the page_cgroup descriptor > themselves, so just pass the page and do the look up in there. > > We already had two bugs (6568d4a 'mm: memcg: update the correct soft > limit tree during migration' and 'memcg: fix Bad page state after > replace_page_cache') where the passed page and pc were not referring > to the same page frame. > > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org> Excellent, thank you: Acked-by: Hugh Dickins <hughd@google.com> > --- > mm/memcontrol.c | 17 +++++------------ > 1 files changed, 5 insertions(+), 12 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 884e936..1a28dd8 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -2461,10 +2461,10 @@ struct mem_cgroup *try_get_mem_cgroup_from_page(struct page *page) > static void __mem_cgroup_commit_charge(struct mem_cgroup *memcg, > struct page *page, > unsigned int nr_pages, > - struct page_cgroup *pc, > enum charge_type ctype, > bool lrucare) > { > + struct page_cgroup *pc = lookup_page_cgroup(page); > struct zone *uninitialized_var(zone); > bool was_on_lru = false; > bool anon; > @@ -2701,7 +2701,6 @@ static int mem_cgroup_charge_common(struct page *page, struct mm_struct *mm, > { > struct mem_cgroup *memcg = NULL; > unsigned int nr_pages = 1; > - struct page_cgroup *pc; > bool oom = true; > int ret; > > @@ -2715,11 +2714,10 @@ static int mem_cgroup_charge_common(struct page *page, struct mm_struct *mm, > oom = false; > } > > - pc = lookup_page_cgroup(page); > ret = __mem_cgroup_try_charge(mm, gfp_mask, nr_pages, &memcg, oom); > if (ret == -ENOMEM) > return ret; > - __mem_cgroup_commit_charge(memcg, page, nr_pages, pc, ctype, false); > + __mem_cgroup_commit_charge(memcg, page, nr_pages, ctype, false); > return 0; > } > > @@ -2816,16 +2814,13 @@ static void > __mem_cgroup_commit_charge_swapin(struct page *page, struct mem_cgroup *memcg, > enum charge_type ctype) > { > - struct page_cgroup *pc; > - > if (mem_cgroup_disabled()) > return; > if (!memcg) > return; > cgroup_exclude_rmdir(&memcg->css); > > - pc = lookup_page_cgroup(page); > - __mem_cgroup_commit_charge(memcg, page, 1, pc, ctype, true); > + __mem_cgroup_commit_charge(memcg, page, 1, ctype, true); > /* > * Now swap is on-memory. This means this page may be > * counted both as mem and swap....double count. > @@ -3254,14 +3249,13 @@ int mem_cgroup_prepare_migration(struct page *page, > * page. In the case new page is migrated but not remapped, new page's > * mapcount will be finally 0 and we call uncharge in end_migration(). > */ > - pc = lookup_page_cgroup(newpage); > if (PageAnon(page)) > ctype = MEM_CGROUP_CHARGE_TYPE_MAPPED; > else if (page_is_file_cache(page)) > ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; > else > ctype = MEM_CGROUP_CHARGE_TYPE_SHMEM; > - __mem_cgroup_commit_charge(memcg, newpage, 1, pc, ctype, false); > + __mem_cgroup_commit_charge(memcg, newpage, 1, ctype, false); > return ret; > } > > @@ -3348,8 +3342,7 @@ void mem_cgroup_replace_page_cache(struct page *oldpage, > * the newpage may be on LRU(or pagevec for LRU) already. We lock > * LRU while we overwrite pc->mem_cgroup. > */ > - pc = lookup_page_cgroup(newpage); > - __mem_cgroup_commit_charge(memcg, newpage, 1, pc, type, true); > + __mem_cgroup_commit_charge(memcg, newpage, 1, type, true); > } > > #ifdef CONFIG_DEBUG_VM > -- > 1.7.7.6 -- 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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] memcg: fix Bad page state after replace_page_cache @ 2012-04-24 19:13 ` Hugh Dickins 0 siblings, 0 replies; 10+ messages in thread From: Hugh Dickins @ 2012-04-24 19:13 UTC (permalink / raw) To: Johannes Weiner Cc: Linus Torvalds, Andrew Morton, Miklos Szeredi, KAMEZAWA Hiroyuki, Michal Hocko, linux-kernel, linux-mm On Tue, 24 Apr 2012, Johannes Weiner wrote: > From: Johannes Weiner <hannes@cmpxchg.org> > Subject: [patch] mm: memcg: move pc lookup point to commit_charge() > > None of the callsites actually need the page_cgroup descriptor > themselves, so just pass the page and do the look up in there. > > We already had two bugs (6568d4a 'mm: memcg: update the correct soft > limit tree during migration' and 'memcg: fix Bad page state after > replace_page_cache') where the passed page and pc were not referring > to the same page frame. > > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org> Excellent, thank you: Acked-by: Hugh Dickins <hughd@google.com> > --- > mm/memcontrol.c | 17 +++++------------ > 1 files changed, 5 insertions(+), 12 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 884e936..1a28dd8 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -2461,10 +2461,10 @@ struct mem_cgroup *try_get_mem_cgroup_from_page(struct page *page) > static void __mem_cgroup_commit_charge(struct mem_cgroup *memcg, > struct page *page, > unsigned int nr_pages, > - struct page_cgroup *pc, > enum charge_type ctype, > bool lrucare) > { > + struct page_cgroup *pc = lookup_page_cgroup(page); > struct zone *uninitialized_var(zone); > bool was_on_lru = false; > bool anon; > @@ -2701,7 +2701,6 @@ static int mem_cgroup_charge_common(struct page *page, struct mm_struct *mm, > { > struct mem_cgroup *memcg = NULL; > unsigned int nr_pages = 1; > - struct page_cgroup *pc; > bool oom = true; > int ret; > > @@ -2715,11 +2714,10 @@ static int mem_cgroup_charge_common(struct page *page, struct mm_struct *mm, > oom = false; > } > > - pc = lookup_page_cgroup(page); > ret = __mem_cgroup_try_charge(mm, gfp_mask, nr_pages, &memcg, oom); > if (ret == -ENOMEM) > return ret; > - __mem_cgroup_commit_charge(memcg, page, nr_pages, pc, ctype, false); > + __mem_cgroup_commit_charge(memcg, page, nr_pages, ctype, false); > return 0; > } > > @@ -2816,16 +2814,13 @@ static void > __mem_cgroup_commit_charge_swapin(struct page *page, struct mem_cgroup *memcg, > enum charge_type ctype) > { > - struct page_cgroup *pc; > - > if (mem_cgroup_disabled()) > return; > if (!memcg) > return; > cgroup_exclude_rmdir(&memcg->css); > > - pc = lookup_page_cgroup(page); > - __mem_cgroup_commit_charge(memcg, page, 1, pc, ctype, true); > + __mem_cgroup_commit_charge(memcg, page, 1, ctype, true); > /* > * Now swap is on-memory. This means this page may be > * counted both as mem and swap....double count. > @@ -3254,14 +3249,13 @@ int mem_cgroup_prepare_migration(struct page *page, > * page. In the case new page is migrated but not remapped, new page's > * mapcount will be finally 0 and we call uncharge in end_migration(). > */ > - pc = lookup_page_cgroup(newpage); > if (PageAnon(page)) > ctype = MEM_CGROUP_CHARGE_TYPE_MAPPED; > else if (page_is_file_cache(page)) > ctype = MEM_CGROUP_CHARGE_TYPE_CACHE; > else > ctype = MEM_CGROUP_CHARGE_TYPE_SHMEM; > - __mem_cgroup_commit_charge(memcg, newpage, 1, pc, ctype, false); > + __mem_cgroup_commit_charge(memcg, newpage, 1, ctype, false); > return ret; > } > > @@ -3348,8 +3342,7 @@ void mem_cgroup_replace_page_cache(struct page *oldpage, > * the newpage may be on LRU(or pagevec for LRU) already. We lock > * LRU while we overwrite pc->mem_cgroup. > */ > - pc = lookup_page_cgroup(newpage); > - __mem_cgroup_commit_charge(memcg, newpage, 1, pc, type, true); > + __mem_cgroup_commit_charge(memcg, newpage, 1, type, true); > } > > #ifdef CONFIG_DEBUG_VM > -- > 1.7.7.6 ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-04-24 19:13 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-04-19 6:34 [PATCH] memcg: fix Bad page state after replace_page_cache Hugh Dickins 2012-04-19 6:34 ` Hugh Dickins 2012-04-19 10:20 ` Michal Hocko 2012-04-19 10:20 ` Michal Hocko 2012-04-20 1:47 ` KAMEZAWA Hiroyuki 2012-04-20 1:47 ` KAMEZAWA Hiroyuki 2012-04-24 18:22 ` Johannes Weiner 2012-04-24 18:22 ` Johannes Weiner 2012-04-24 19:13 ` Hugh Dickins 2012-04-24 19:13 ` Hugh Dickins
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.