Skip to contentSkip to conversation

memcg: add memcg sanity checks at allocating and freeing pages

PATCH In review Johannes Weiner 3 participants 3 messages Feb 3, 2011 – Feb 4, 2011

Subsystems (3)
Memory management Page allocator memcg
lore
Share & tools
lore

Johannes Weiner

author maintainer
#0 lore raw

PATCH 1/1 memcg: add memcg sanity checks at allocating and freeing pages

From: Daisuke Nishimura <[email protected]>

This patch add checks at allocating or freeing a page whether the page is used (iow, charged) from the view point of memcg.

This check may be useful in debugging a problem and we did similar checks before the commit 52d4b9ac(memcg: allocate all page_cgroup at boot).

This patch adds some overheads at allocating or freeing memory, so it's enabled only when CONFIG_DEBUG_VM is enabled.

Signed-off-by Daisuke Nishimura <[email protected]>
Signed-off-by Johannes Weiner <[email protected]>
---
 include/linux/memcontrol.h |   17 +++++++++++++++++
 mm/memcontrol.c            |   30 ++++++++++++++++++++++++++++++
 mm/page_alloc.c            |    8 ++++++--
 3 files changed, 53 insertions(+), 2 deletions(-)
include/linux/memcontrol.h +17 -0
index a1a1e53..3da48ae 100644
@@ -150,6 +150,10 @@ u64 mem_cgroup_get_limit(struct mem_cgroup *mem);
 void mem_cgroup_split_huge_fixup(struct page *head, struct page *tail);
 #endif
 
+#ifdef CONFIG_DEBUG_VM
+bool mem_cgroup_bad_page_check(struct page *page);
+void mem_cgroup_print_bad_page(struct page *page);
+#endif
 #else /* CONFIG_CGROUP_MEM_RES_CTLR */
 struct mem_cgroup;
 

@@ -346,5 +350,18 @@ static inline void mem_cgroup_split_huge_fixup(struct page *head,
 
 #endif /* CONFIG_CGROUP_MEM_CONT */
 
+#if !defined(CONFIG_CGROUP_MEM_RES_CTLR) || !defined(CONFIG_DEBUG_VM)
+static inline bool
+mem_cgroup_bad_page_check(struct page *page)
+{
+	return false;
+}
+
+static inline void
+mem_cgroup_print_bad_page(struct page *page)
+{
+}
+#endif
+
 #endif /* _LINUX_MEMCONTROL_H */
mm/memcontrol.c +30 -0
index 6abaa10..2ed1b33 100644
@@ -3016,6 +3016,36 @@ int mem_cgroup_shmem_charge_fallback(struct page *page,
 	return ret;
 }
 
+#ifdef CONFIG_DEBUG_VM
+static struct page_cgroup *lookup_page_cgroup_used(struct page *page)
+{
+	struct page_cgroup *pc;
+
+	pc = lookup_page_cgroup(page);
+	if (likely(pc) && PageCgroupUsed(pc))
+		return pc;
+	return NULL;
+}
+
+bool mem_cgroup_bad_page_check(struct page *page)
+{
+	if (mem_cgroup_disabled())
+		return false;
+
+	return lookup_page_cgroup_used(page) != NULL;
+}
+
+void mem_cgroup_print_bad_page(struct page *page)
+{
+	struct page_cgroup *pc;
+
+	pc = lookup_page_cgroup_used(page);
+	if (pc)
+		printk(KERN_ALERT "pc:%p pc->flags:%ld pc->mem_cgroup:%p\n",
+		       pc, pc->flags, pc->mem_cgroup);
+}
+#endif
+
 static DEFINE_MUTEX(set_limit_mutex);
 
 static int mem_cgroup_resize_limit(struct mem_cgroup *memcg,
mm/page_alloc.c +6 -2
index 26df268..60e58b0 100644
@@ -53,6 +53,7 @@
 #include <linux/compaction.h>
 #include <trace/events/kmem.h>
 #include <linux/ftrace_event.h>
+#include <linux/memcontrol.h>
 
 #include <asm/tlbflush.h>
 #include <asm/div64.h>

@@ -565,7 +566,8 @@ static inline int free_pages_check(struct page *page)
 	if (unlikely(page_mapcount(page) |
 		(page->mapping != NULL)  |
 		(atomic_read(&page->_count) != 0) |
-		(page->flags & PAGE_FLAGS_CHECK_AT_FREE))) {
+		(page->flags & PAGE_FLAGS_CHECK_AT_FREE) |
+		(mem_cgroup_bad_page_check(page)))) {
 		bad_page(page);
 		return 1;
 	}

@@ -750,7 +752,8 @@ static inline int check_new_page(struct page *page)
 	if (unlikely(page_mapcount(page) |
 		(page->mapping != NULL)  |
 		(atomic_read(&page->_count) != 0)  |
-		(page->flags & PAGE_FLAGS_CHECK_AT_PREP))) {
+		(page->flags & PAGE_FLAGS_CHECK_AT_PREP) |
+		(mem_cgroup_bad_page_check(page)))) {
 		bad_page(page);
 		return 1;
 	}

@@ -5693,4 +5696,5 @@ void dump_page(struct page *page)
 		page, atomic_read(&page->_count), page_mapcount(page),
 		page->mapping, page->index);
 	dump_page_flags(page->flags);
+	mem_cgroup_print_bad_page(page);
 }
Signature · 8 lines
1.7.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  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:"[email protected]"> [email protected] </a>
Acked-by on v1 1/1
#1 lore raw
Quoted 5 lines · On Thu, 3 Feb 2011 15:15:33 +0100 Johannes Weiner <[email protected]> wrote:

From: Daisuke Nishimura <[email protected]>

This patch add checks at allocating or freeing a page whether the page is used (iow, charged) from the view point of memcg.

Quoted 8 lines

This check may be useful in debugging a problem and we did similar checks before the commit 52d4b9ac(memcg: allocate all page_cgroup at boot).

This patch adds some overheads at allocating or freeing memory, so it's enabled only when CONFIG_DEBUG_VM is enabled.

Signed-off-by Daisuke Nishimura <[email protected]>
Signed-off-by Johannes Weiner <[email protected]>
Acked-by KAMEZAWA Hiroyuki <[email protected]>
#2 lore raw
In reply to Johannes Weiner
Quoted 5 lines · On Thu, 3 Feb 2011 15:15:33 +0100 Johannes Weiner <[email protected]> wrote:

From: Daisuke Nishimura <[email protected]>

This patch add checks at allocating or freeing a page whether the page is used (iow, charged) from the view point of memcg.

Quoted 8 lines

This check may be useful in debugging a problem and we did similar checks before the commit 52d4b9ac(memcg: allocate all page_cgroup at boot).

This patch adds some overheads at allocating or freeing memory, so it's enabled only when CONFIG_DEBUG_VM is enabled.

Signed-off-by Daisuke Nishimura <[email protected]>
Signed-off-by Johannes Weiner <[email protected]>

Thank you for picking up this patch. You actually remind me about this patch :)

I have one comment.

Review

Quoted 11 lines
(quoted diff) +11 -0
+void mem_cgroup_print_bad_page(struct page *page)
+{
+	struct page_cgroup *pc;
+
+	pc = lookup_page_cgroup_used(page);
+	if (pc)
+		printk(KERN_ALERT "pc:%p pc->flags:%ld pc->mem_cgroup:%p\n",
+		       pc, pc->flags, pc->mem_cgroup);
+}
+#endif
+

When I posted this patch before, I received a comment that showing the path of the cgroup would be better.
This is a additional patch to show the path.

===
From: Daisuke Nishimura <[email protected]>

Let's try to show the path name of the cgroup too.

Signed-off-by Daisuke Nishimura <[email protected]>
---
 mm/memcontrol.c |   20 ++++++++++++++++++--
 1 files changed, 18 insertions(+), 2 deletions(-)
mm/memcontrol.c +18 -2
index 2ed1b33..3c14d20 100644
@@ -3040,9 +3040,25 @@ void mem_cgroup_print_bad_page(struct page *page)
 	struct page_cgroup *pc;
 
 	pc = lookup_page_cgroup_used(page);
-	if (pc)
-		printk(KERN_ALERT "pc:%p pc->flags:%ld pc->mem_cgroup:%p\n",
+	if (pc) {
+		int ret = -1;
+		char *path;
+
+		printk(KERN_ALERT "pc:%p pc->flags:%ld pc->mem_cgroup:%p",
 		       pc, pc->flags, pc->mem_cgroup);
+
+		path = kmalloc(PATH_MAX, GFP_KERNEL);
+		if (path) {
+			rcu_read_lock();
+			ret = cgroup_path(pc->mem_cgroup->css.cgroup,
+							path, PATH_MAX);
+			rcu_read_unlock();
+		}
+
+		printk(KERN_ALERT "(%s)\n",
+				(ret < 0) ? "cannot get the path" : path);
+		kfree(path);
+	}
 }
 #endif
Signature · 8 lines
1.7.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  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:"[email protected]"> [email protected] </a>