memcg: bad page if page_cgroup when free
Replace free_hot_cold_page's VM_BUG_ON(page_get_page_cgroup(page)) by a "Bad
page state" and clear: most users don't have CONFIG_DEBUG_VM on, and if it
were set here, it'd likely cause corruption when the page is reused.
Don't use page_assign_page_cgroup to clear it: that should be private to
memcontrol.c, and always called with the lock taken; and memmap_init_zone
doesn't need it either - like page->mapping and other pointers throughout the
kernel, Linux assumes pointers in zeroed structures are NULL pointers.
Instead use page_reset_bad_cgroup, added to memcontrol.h for this only.
Signed-off-by: Hugh Dickins <hugh@veritas.com>
Cc: David Rientjes <rientjes@google.com>
Acked-by: Balbir Singh <balbir@linux.vnet.ibm.com>
Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Hirokazu Takahashi <taka@valinux.co.jp>
Cc: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
Cc: Paul Menage <menage@google.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 56432ff..70789df 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -29,8 +29,9 @@
extern void mm_init_cgroup(struct mm_struct *mm, struct task_struct *p);
extern void mm_free_cgroup(struct mm_struct *mm);
-extern void page_assign_page_cgroup(struct page *page,
- struct page_cgroup *pc);
+
+#define page_reset_bad_cgroup(page) ((page)->page_cgroup = 0)
+
extern struct page_cgroup *page_get_page_cgroup(struct page *page);
extern int mem_cgroup_charge(struct page *page, struct mm_struct *mm,
gfp_t gfp_mask);
@@ -82,8 +83,7 @@
{
}
-static inline void page_assign_page_cgroup(struct page *page,
- struct page_cgroup *pc)
+static inline void page_reset_bad_cgroup(struct page *page)
{
}
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index afdd406..9e170d3 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -140,11 +140,17 @@
/*
* We use the lower bit of the page->page_cgroup pointer as a bit spin
- * lock. We need to ensure that page->page_cgroup is atleast two
- * byte aligned (based on comments from Nick Piggin)
+ * lock. We need to ensure that page->page_cgroup is at least two
+ * byte aligned (based on comments from Nick Piggin). But since
+ * bit_spin_lock doesn't actually set that lock bit in a non-debug
+ * uniprocessor kernel, we should avoid setting it here too.
*/
#define PAGE_CGROUP_LOCK_BIT 0x0
-#define PAGE_CGROUP_LOCK (1 << PAGE_CGROUP_LOCK_BIT)
+#if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
+#define PAGE_CGROUP_LOCK (1 << PAGE_CGROUP_LOCK_BIT)
+#else
+#define PAGE_CGROUP_LOCK 0x0
+#endif
/*
* A page_cgroup page is associated with every page descriptor. The
@@ -271,19 +277,10 @@
&page->page_cgroup);
}
-void page_assign_page_cgroup(struct page *page, struct page_cgroup *pc)
+static void page_assign_page_cgroup(struct page *page, struct page_cgroup *pc)
{
- int locked;
-
- /*
- * While resetting the page_cgroup we might not hold the
- * page_cgroup lock. free_hot_cold_page() is an example
- * of such a scenario
- */
- if (pc)
- VM_BUG_ON(!page_cgroup_locked(page));
- locked = (page->page_cgroup & PAGE_CGROUP_LOCK);
- page->page_cgroup = ((unsigned long)pc | locked);
+ VM_BUG_ON(!page_cgroup_locked(page));
+ page->page_cgroup = ((unsigned long)pc | PAGE_CGROUP_LOCK);
}
struct page_cgroup *page_get_page_cgroup(struct page *page)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index e76cf94..402a504 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -222,13 +222,19 @@
static void bad_page(struct page *page)
{
- printk(KERN_EMERG "Bad page state in process '%s'\n"
- KERN_EMERG "page:%p flags:0x%0*lx mapping:%p mapcount:%d count:%d\n"
- KERN_EMERG "Trying to fix it up, but a reboot is needed\n"
- KERN_EMERG "Backtrace:\n",
+ void *pc = page_get_page_cgroup(page);
+
+ printk(KERN_EMERG "Bad page state in process '%s'\n" KERN_EMERG
+ "page:%p flags:0x%0*lx mapping:%p mapcount:%d count:%d\n",
current->comm, page, (int)(2*sizeof(unsigned long)),
(unsigned long)page->flags, page->mapping,
page_mapcount(page), page_count(page));
+ if (pc) {
+ printk(KERN_EMERG "cgroup:%p\n", pc);
+ page_reset_bad_cgroup(page);
+ }
+ printk(KERN_EMERG "Trying to fix it up, but a reboot is needed\n"
+ KERN_EMERG "Backtrace:\n");
dump_stack();
page->flags &= ~(1 << PG_lru |
1 << PG_private |
@@ -454,6 +460,7 @@
{
if (unlikely(page_mapcount(page) |
(page->mapping != NULL) |
+ (page_get_page_cgroup(page) != NULL) |
(page_count(page) != 0) |
(page->flags & (
1 << PG_lru |
@@ -603,6 +610,7 @@
{
if (unlikely(page_mapcount(page) |
(page->mapping != NULL) |
+ (page_get_page_cgroup(page) != NULL) |
(page_count(page) != 0) |
(page->flags & (
1 << PG_lru |
@@ -989,7 +997,6 @@
if (!PageHighMem(page))
debug_check_no_locks_freed(page_address(page), PAGE_SIZE);
- VM_BUG_ON(page_get_page_cgroup(page));
arch_free_page(page, 0);
kernel_map_pages(page, 1, 0);
@@ -2528,7 +2535,6 @@
set_page_links(page, zone, nid, pfn);
init_page_count(page);
reset_page_mapcount(page);
- page_assign_page_cgroup(page, NULL);
SetPageReserved(page);
/*