Skip to contentSkip to conversation

kmemcheck: add hooks for the page allocator

RFC PATCH In review Vegard Nossum 2 participants 2 messages Nov 25, 2008 Memory management

lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/1 kmemcheck: add hooks for the page allocator

#1 lore raw
Quoted 21 lines · Vegard Nossum
mm/page_alloc.c +4 -0
@@ -23,6 +23,7 @@
 #include <linux/bootmem.h>
 #include <linux/compiler.h>
 #include <linux/kernel.h>
+#include <linux/kmemcheck.h>
 #include <linux/module.h>
 #include <linux/suspend.h>
 #include <linux/pagevec.h>
@@ -511,6 +512,9 @@ static void __free_pages_ok(struct page *page, unsigned int order)
 	int i;
 	int reserved = 0;
 
+	if (kmemcheck_page_is_tracked(page))
+		kmemcheck_free_shadow(page, order);
+
 	for (i = 0 ; i < (1 << order) ; ++i)
 		reserved += free_pages_check(page + i);
 	if (reserved)

mm/page_alloc.c static void free_hot_cold_page(struct page *page, int cold) · line 982

Quoted 5 lines
mm/page_alloc.c +2 -0
@@ -974,6 +978,9 @@ static void free_hot_cold_page(struct page *page, int cold)
 	struct per_cpu_pages *pcp;
 	unsigned long flags;
 
+	if (kmemcheck_page_is_tracked(page))
+		kmemcheck_free_shadow(page, 0);

I think it would be best to just integrate the kmemcheck_page_is_tracked() inside of kmemcheck_free_shadow(). It will be shorter and less error-prone.

Quoted 21 lines
mm/page_alloc.c +13 -0
 	if (PageAnon(page))
 		page->mapping = NULL;
 	if (free_pages_check(page))
@@ -1637,7 +1644,28 @@ nopage:
 		dump_stack();
 		show_mem();
 	}
+	return page;
 got_pg:
+	if (kmemcheck_enabled
+		&& !(gfp_mask & (__GFP_HIGHMEM | __GFP_IO | __GFP_NOTRACK)))
+	{
+		int nr_pages = 1 << order;
+
+		/*
+		 * NOTE: We choose to track GFP_ZERO pages too; in fact, they
+		 * can become uninitialized by copying uninitialized memory
+		 * into them.
+		 */
+
+		/* XXX: Can use zone->node for node? */

mm/page_alloc.c nopage: · line 1670

Quoted 10 lines
mm/page_alloc.c +8 -0
+		kmemcheck_alloc_shadow(page, order, gfp_mask, -1);
+
+		if (gfp_mask & __GFP_ZERO)
+			kmemcheck_mark_initialized_pages(page, nr_pages);
+		else
+			kmemcheck_mark_uninitialized_pages(page, nr_pages);
+	}
+
 	return page;
 }

That's too much gunk to add to a core function like free_hot_cold_page(). Can you please break this out into a function?

Does 'kmemcheck_enabled' get compiled down to a constant if ! CONFIG_KMEMCHECK? If not, it should.

Quoted 83 lines
mm/page_alloc.c +0 -0
 EXPORT_SYMBOL(__alloc_pages_internal);
mm/slab.c +10 -4
index 37deade..286c6a6 100644
@@ -1600,7 +1600,7 @@ static void *kmem_getpages(struct kmem_cache *cachep, gfp_t flags, int nodeid)
 	if (cachep->flags & SLAB_RECLAIM_ACCOUNT)
 		flags |= __GFP_RECLAIMABLE;
 
-	page = alloc_pages_node(nodeid, flags, cachep->gfporder);
+	page = alloc_pages_node(nodeid, flags & ~__GFP_NOTRACK, cachep->gfporder);
 	if (!page)
 		return NULL;
 
@@ -1614,8 +1614,14 @@ static void *kmem_getpages(struct kmem_cache *cachep, gfp_t flags, int nodeid)
 	for (i = 0; i < nr_pages; i++)
 		__SetPageSlab(page + i);
 
-	if (kmemcheck_enabled && !(cachep->flags & SLAB_NOTRACK))
-		kmemcheck_alloc_shadow(cachep, flags, nodeid, page, cachep->gfporder);
+	if (kmemcheck_enabled && !(cachep->flags & SLAB_NOTRACK)) {
+		kmemcheck_alloc_shadow(page, cachep->gfporder, flags, nodeid);
+
+		if (cachep->ctor)
+			kmemcheck_mark_uninitialized_pages(page, nr_pages);
+		else
+			kmemcheck_mark_unallocated_pages(page, nr_pages);
+	}
 
 	return page_address(page);
 }
@@ -1630,7 +1636,7 @@ static void kmem_freepages(struct kmem_cache *cachep, void *addr)
 	const unsigned long nr_freed = i;
 
 	if (kmemcheck_page_is_tracked(page))
-		kmemcheck_free_shadow(cachep, page, cachep->gfporder);
+		kmemcheck_free_shadow(page, cachep->gfporder);
 
 	if (cachep->flags & SLAB_RECLAIM_ACCOUNT)
 		sub_zone_page_state(page_zone(page),
mm/slub.c +15 -2
index adcb5e3..eb9855f 100644
@@ -1064,6 +1064,8 @@ static inline struct page *alloc_slab_page(gfp_t flags, int node,
 {
 	int order = oo_order(oo);
 
+	flags |= __GFP_NOTRACK;
+
 	if (node == -1)
 		return alloc_pages(flags, order);
 	else
@@ -1095,7 +1097,18 @@ static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node)
 	if (kmemcheck_enabled
 		&& !(s->flags & (SLAB_NOTRACK | DEBUG_DEFAULT_FLAGS)))
 	{
-		kmemcheck_alloc_shadow(s, flags, node, page, compound_order(page));
+		int pages = 1 << oo_order(oo);
+
+		kmemcheck_alloc_shadow(page, oo_order(oo), flags, node);
+
+		/*
+		 * Objects from caches that have a constructor don't get
+		 * cleared when they're allocated, so we need to do it here.
+		 */
+		if (s->ctor)
+			kmemcheck_mark_uninitialized_pages(page, pages);
+		else
+			kmemcheck_mark_unallocated_pages(page, pages);
 	}
 
 	page->objects = oo_objects(oo);
@@ -1172,7 +1185,7 @@ static void __free_slab(struct kmem_cache *s, struct page *page)
 	}
 
 	if (kmemcheck_page_is_tracked(page))
-		kmemcheck_free_shadow(s, page, compound_order(page));
+		kmemcheck_free_shadow(page, compound_order(page));
 
 	mod_zone_page_state(page_zone(page),
 		(s->flags & SLAB_RECLAIM_ACCOUNT) ?

mm/slub.c EXPORT_SYMBOL(__kmalloc); · line 2699

Quoted 10 lines
mm/slub.c +2 -2
@@ -2679,8 +2692,8 @@ EXPORT_SYMBOL(__kmalloc);
 
 static void *kmalloc_large_node(size_t size, gfp_t flags, int node)
 {
-	struct page *page = alloc_pages_node(node, flags | __GFP_COMP,
-						get_order(size));
+	struct page *page = alloc_pages_node(node,
+		flags | __GFP_COMP | __GFP_NOTRACK, get_order(size));
 
 	if (page)
 		return page_address(page);

Adding the new flag made that pretty ugly. Can you fix it up?

-- Dave

--
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/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>