linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/3] followups to debug_pagealloc improvements through page_owner
@ 2019-10-07  9:18 Vlastimil Babka
  2019-10-07  9:18 ` [PATCH v3 1/3] mm, page_owner: fix off-by-one error in __set_page_owner_handle() Vlastimil Babka
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Vlastimil Babka @ 2019-10-07  9:18 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, linux-kernel, kasan-dev, Qian Cai, Kirill A. Shutemov,
	Matthew Wilcox, Mel Gorman, Michal Hocko, Vlastimil Babka,
	Andrey Ryabinin, Dmitry Vyukov, Kirill A. Shutemov, Walter Wu

Changes since v2 [3]:

- Qian Cai suggested that the extra boot option and page_ext ops is unnecessary
  for a debugging option, unless somebody really complains about the overhead,
  with numbers. So patch 2 is greatly simplified.

These are followups to [1] which made it to Linus meanwhile. Patches 1 and 3
are based on Kirill's review, patch 2 on KASAN request [2]. It would be nice
if all of this made it to 5.4 with [1] already there (or at least Patch 1).

[1] https://lore.kernel.org/linux-mm/20190820131828.22684-1-vbabka@suse.cz/
[2] https://lore.kernel.org/linux-arm-kernel/20190911083921.4158-1-walter-zh.wu@mediatek.com/
[3] https://lore.kernel.org/linux-mm/20190930122916.14969-1-vbabka@suse.cz/

Vlastimil Babka (3):
  mm, page_owner: fix off-by-one error in __set_page_owner_handle()
  mm, page_owner: decouple freeing stack trace from debug_pagealloc
  mm, page_owner: rename flag indicating that page is allocated

 Documentation/dev-tools/kasan.rst |  3 ++
 include/linux/page_ext.h          | 10 +++++-
 mm/page_ext.c                     | 23 +++++--------
 mm/page_owner.c                   | 55 +++++++++++--------------------
 4 files changed, 41 insertions(+), 50 deletions(-)

-- 
2.23.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v3 1/3] mm, page_owner: fix off-by-one error in __set_page_owner_handle()
  2019-10-07  9:18 [PATCH v3 0/3] followups to debug_pagealloc improvements through page_owner Vlastimil Babka
@ 2019-10-07  9:18 ` Vlastimil Babka
  2019-10-07  9:18 ` [PATCH v3 2/3] mm, page_owner: decouple freeing stack trace from debug_pagealloc Vlastimil Babka
  2019-10-07  9:18 ` [PATCH v3 3/3] mm, page_owner: rename flag indicating that page is allocated Vlastimil Babka
  2 siblings, 0 replies; 5+ messages in thread
From: Vlastimil Babka @ 2019-10-07  9:18 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, linux-kernel, kasan-dev, Qian Cai, Kirill A. Shutemov,
	Matthew Wilcox, Mel Gorman, Michal Hocko, Vlastimil Babka,
	Kirill A . Shutemov

As noted by Kirill, commit 7e2f2a0cd17c ("mm, page_owner: record page owner for
each subpage") has introduced an off-by-one error in __set_page_owner_handle()
when looking up page_ext for subpages. As a result, the head page page_owner
info is set twice, while for the last tail page, it's not set at all.

Fix this and also make the code more efficient by advancing the page_ext
pointer we already have, instead of calling lookup_page_ext() for each subpage.
Since the full size of struct page_ext is not known at compile time, we can't
use a simple page_ext++ statement, so introduce a page_ext_next() inline
function for that.

Reported-by: Kirill A. Shutemov <kirill@shutemov.name>
Fixes: 7e2f2a0cd17c ("mm, page_owner: record page owner for each subpage")
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
 include/linux/page_ext.h |  8 ++++++++
 mm/page_ext.c            | 23 +++++++++--------------
 mm/page_owner.c          | 15 +++++++--------
 3 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/include/linux/page_ext.h b/include/linux/page_ext.h
index 682fd465df06..5e856512bafb 100644
--- a/include/linux/page_ext.h
+++ b/include/linux/page_ext.h
@@ -36,6 +36,7 @@ struct page_ext {
 	unsigned long flags;
 };
 
+extern unsigned long page_ext_size;
 extern void pgdat_page_ext_init(struct pglist_data *pgdat);
 
 #ifdef CONFIG_SPARSEMEM
@@ -52,6 +53,13 @@ static inline void page_ext_init(void)
 
 struct page_ext *lookup_page_ext(const struct page *page);
 
+static inline struct page_ext *page_ext_next(struct page_ext *curr)
+{
+	void *next = curr;
+	next += page_ext_size;
+	return next;
+}
+
 #else /* !CONFIG_PAGE_EXTENSION */
 struct page_ext;
 
diff --git a/mm/page_ext.c b/mm/page_ext.c
index 5f5769c7db3b..4ade843ff588 100644
--- a/mm/page_ext.c
+++ b/mm/page_ext.c
@@ -67,8 +67,9 @@ static struct page_ext_operations *page_ext_ops[] = {
 #endif
 };
 
+unsigned long page_ext_size = sizeof(struct page_ext);
+
 static unsigned long total_usage;
-static unsigned long extra_mem;
 
 static bool __init invoke_need_callbacks(void)
 {
@@ -78,9 +79,8 @@ static bool __init invoke_need_callbacks(void)
 
 	for (i = 0; i < entries; i++) {
 		if (page_ext_ops[i]->need && page_ext_ops[i]->need()) {
-			page_ext_ops[i]->offset = sizeof(struct page_ext) +
-						extra_mem;
-			extra_mem += page_ext_ops[i]->size;
+			page_ext_ops[i]->offset = page_ext_size;
+			page_ext_size += page_ext_ops[i]->size;
 			need = true;
 		}
 	}
@@ -99,14 +99,9 @@ static void __init invoke_init_callbacks(void)
 	}
 }
 
-static unsigned long get_entry_size(void)
-{
-	return sizeof(struct page_ext) + extra_mem;
-}
-
 static inline struct page_ext *get_entry(void *base, unsigned long index)
 {
-	return base + get_entry_size() * index;
+	return base + page_ext_size * index;
 }
 
 #if !defined(CONFIG_SPARSEMEM)
@@ -156,7 +151,7 @@ static int __init alloc_node_page_ext(int nid)
 		!IS_ALIGNED(node_end_pfn(nid), MAX_ORDER_NR_PAGES))
 		nr_pages += MAX_ORDER_NR_PAGES;
 
-	table_size = get_entry_size() * nr_pages;
+	table_size = page_ext_size * nr_pages;
 
 	base = memblock_alloc_try_nid(
 			table_size, PAGE_SIZE, __pa(MAX_DMA_ADDRESS),
@@ -234,7 +229,7 @@ static int __meminit init_section_page_ext(unsigned long pfn, int nid)
 	if (section->page_ext)
 		return 0;
 
-	table_size = get_entry_size() * PAGES_PER_SECTION;
+	table_size = page_ext_size * PAGES_PER_SECTION;
 	base = alloc_page_ext(table_size, nid);
 
 	/*
@@ -254,7 +249,7 @@ static int __meminit init_section_page_ext(unsigned long pfn, int nid)
 	 * we need to apply a mask.
 	 */
 	pfn &= PAGE_SECTION_MASK;
-	section->page_ext = (void *)base - get_entry_size() * pfn;
+	section->page_ext = (void *)base - page_ext_size * pfn;
 	total_usage += table_size;
 	return 0;
 }
@@ -267,7 +262,7 @@ static void free_page_ext(void *addr)
 		struct page *page = virt_to_page(addr);
 		size_t table_size;
 
-		table_size = get_entry_size() * PAGES_PER_SECTION;
+		table_size = page_ext_size * PAGES_PER_SECTION;
 
 		BUG_ON(PageReserved(page));
 		kmemleak_free(addr);
diff --git a/mm/page_owner.c b/mm/page_owner.c
index dee931184788..d3cf5d336ccf 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -156,10 +156,10 @@ void __reset_page_owner(struct page *page, unsigned int order)
 		handle = save_stack(GFP_NOWAIT | __GFP_NOWARN);
 #endif
 
+	page_ext = lookup_page_ext(page);
+	if (unlikely(!page_ext))
+		return;
 	for (i = 0; i < (1 << order); i++) {
-		page_ext = lookup_page_ext(page + i);
-		if (unlikely(!page_ext))
-			continue;
 		__clear_bit(PAGE_EXT_OWNER_ACTIVE, &page_ext->flags);
 #ifdef CONFIG_DEBUG_PAGEALLOC
 		if (debug_pagealloc_enabled()) {
@@ -167,6 +167,7 @@ void __reset_page_owner(struct page *page, unsigned int order)
 			page_owner->free_handle = handle;
 		}
 #endif
+		page_ext = page_ext_next(page_ext);
 	}
 }
 
@@ -186,7 +187,7 @@ static inline void __set_page_owner_handle(struct page *page,
 		__set_bit(PAGE_EXT_OWNER, &page_ext->flags);
 		__set_bit(PAGE_EXT_OWNER_ACTIVE, &page_ext->flags);
 
-		page_ext = lookup_page_ext(page + i);
+		page_ext = page_ext_next(page_ext);
 	}
 }
 
@@ -224,12 +225,10 @@ void __split_page_owner(struct page *page, unsigned int order)
 	if (unlikely(!page_ext))
 		return;
 
-	page_owner = get_page_owner(page_ext);
-	page_owner->order = 0;
-	for (i = 1; i < (1 << order); i++) {
-		page_ext = lookup_page_ext(page + i);
+	for (i = 0; i < (1 << order); i++) {
 		page_owner = get_page_owner(page_ext);
 		page_owner->order = 0;
+		page_ext = page_ext_next(page_ext);
 	}
 }
 
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v3 2/3] mm, page_owner: decouple freeing stack trace from debug_pagealloc
  2019-10-07  9:18 [PATCH v3 0/3] followups to debug_pagealloc improvements through page_owner Vlastimil Babka
  2019-10-07  9:18 ` [PATCH v3 1/3] mm, page_owner: fix off-by-one error in __set_page_owner_handle() Vlastimil Babka
@ 2019-10-07  9:18 ` Vlastimil Babka
  2019-10-07 10:41   ` Qian Cai
  2019-10-07  9:18 ` [PATCH v3 3/3] mm, page_owner: rename flag indicating that page is allocated Vlastimil Babka
  2 siblings, 1 reply; 5+ messages in thread
From: Vlastimil Babka @ 2019-10-07  9:18 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, linux-kernel, kasan-dev, Qian Cai, Kirill A. Shutemov,
	Matthew Wilcox, Mel Gorman, Michal Hocko, Vlastimil Babka,
	Dmitry Vyukov, Walter Wu, Andrey Ryabinin

The commit 8974558f49a6 ("mm, page_owner, debug_pagealloc: save and dump
freeing stack trace") enhanced page_owner to also store freeing stack trace,
when debug_pagealloc is also enabled. KASAN would also like to do this [1] to
improve error reports to debug e.g. UAF issues. Kirill has suggested that the
freeing stack trace saving should be also possible to be enabled separately
from KASAN or debug_pagealloc, i.e. with an extra boot option. Qian argued that
we have enough options already, and avoiding the extra overhead is not worth
the complications in the case of a debugging option. Kirill noted that the
extra stack handle in struct page_owner requires 0.1% of memory.

This patch therefore enables free stack saving whenever page_owner is enabled,
regardless of whether debug_pagealloc or KASAN is also enabled. KASAN kernels
booted with page_owner=on will thus benefit from the improved error reports.

[1] https://bugzilla.kernel.org/show_bug.cgi?id=203967

Suggested-by: Dmitry Vyukov <dvyukov@google.com>
Suggested-by: Walter Wu <walter-zh.wu@mediatek.com>
Suggested-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
Suggested-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Suggested-by: Qian Cai <cai@lca.pw>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
 Documentation/dev-tools/kasan.rst |  3 +++
 mm/page_owner.c                   | 28 +++++++---------------------
 2 files changed, 10 insertions(+), 21 deletions(-)

diff --git a/Documentation/dev-tools/kasan.rst b/Documentation/dev-tools/kasan.rst
index b72d07d70239..525296121d89 100644
--- a/Documentation/dev-tools/kasan.rst
+++ b/Documentation/dev-tools/kasan.rst
@@ -41,6 +41,9 @@ smaller binary while the latter is 1.1 - 2 times faster.
 Both KASAN modes work with both SLUB and SLAB memory allocators.
 For better bug detection and nicer reporting, enable CONFIG_STACKTRACE.
 
+To augment reports with last allocation and freeing stack of the physical page,
+it is recommended to enable also CONFIG_PAGE_OWNER and boot with page_owner=on.
+
 To disable instrumentation for specific files or directories, add a line
 similar to the following to the respective kernel Makefile:
 
diff --git a/mm/page_owner.c b/mm/page_owner.c
index d3cf5d336ccf..de1916ac3e24 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -24,12 +24,10 @@ struct page_owner {
 	short last_migrate_reason;
 	gfp_t gfp_mask;
 	depot_stack_handle_t handle;
-#ifdef CONFIG_DEBUG_PAGEALLOC
 	depot_stack_handle_t free_handle;
-#endif
 };
 
-static bool page_owner_disabled = true;
+static bool page_owner_enabled = false;
 DEFINE_STATIC_KEY_FALSE(page_owner_inited);
 
 static depot_stack_handle_t dummy_handle;
@@ -44,7 +42,7 @@ static int __init early_page_owner_param(char *buf)
 		return -EINVAL;
 
 	if (strcmp(buf, "on") == 0)
-		page_owner_disabled = false;
+		page_owner_enabled = true;
 
 	return 0;
 }
@@ -52,10 +50,7 @@ early_param("page_owner", early_page_owner_param);
 
 static bool need_page_owner(void)
 {
-	if (page_owner_disabled)
-		return false;
-
-	return true;
+	return page_owner_enabled;
 }
 
 static __always_inline depot_stack_handle_t create_dummy_stack(void)
@@ -84,7 +79,7 @@ static noinline void register_early_stack(void)
 
 static void init_page_owner(void)
 {
-	if (page_owner_disabled)
+	if (!page_owner_enabled)
 		return;
 
 	register_dummy_stack();
@@ -148,25 +143,18 @@ void __reset_page_owner(struct page *page, unsigned int order)
 {
 	int i;
 	struct page_ext *page_ext;
-#ifdef CONFIG_DEBUG_PAGEALLOC
 	depot_stack_handle_t handle = 0;
 	struct page_owner *page_owner;
 
-	if (debug_pagealloc_enabled())
-		handle = save_stack(GFP_NOWAIT | __GFP_NOWARN);
-#endif
+	handle = save_stack(GFP_NOWAIT | __GFP_NOWARN);
 
 	page_ext = lookup_page_ext(page);
 	if (unlikely(!page_ext))
 		return;
 	for (i = 0; i < (1 << order); i++) {
 		__clear_bit(PAGE_EXT_OWNER_ACTIVE, &page_ext->flags);
-#ifdef CONFIG_DEBUG_PAGEALLOC
-		if (debug_pagealloc_enabled()) {
-			page_owner = get_page_owner(page_ext);
-			page_owner->free_handle = handle;
-		}
-#endif
+		page_owner = get_page_owner(page_ext);
+		page_owner->free_handle = handle;
 		page_ext = page_ext_next(page_ext);
 	}
 }
@@ -450,7 +438,6 @@ void __dump_page_owner(struct page *page)
 		stack_trace_print(entries, nr_entries, 0);
 	}
 
-#ifdef CONFIG_DEBUG_PAGEALLOC
 	handle = READ_ONCE(page_owner->free_handle);
 	if (!handle) {
 		pr_alert("page_owner free stack trace missing\n");
@@ -459,7 +446,6 @@ void __dump_page_owner(struct page *page)
 		pr_alert("page last free stack trace:\n");
 		stack_trace_print(entries, nr_entries, 0);
 	}
-#endif
 
 	if (page_owner->last_migrate_reason != -1)
 		pr_alert("page has been migrated, last migrate reason: %s\n",
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v3 3/3] mm, page_owner: rename flag indicating that page is allocated
  2019-10-07  9:18 [PATCH v3 0/3] followups to debug_pagealloc improvements through page_owner Vlastimil Babka
  2019-10-07  9:18 ` [PATCH v3 1/3] mm, page_owner: fix off-by-one error in __set_page_owner_handle() Vlastimil Babka
  2019-10-07  9:18 ` [PATCH v3 2/3] mm, page_owner: decouple freeing stack trace from debug_pagealloc Vlastimil Babka
@ 2019-10-07  9:18 ` Vlastimil Babka
  2 siblings, 0 replies; 5+ messages in thread
From: Vlastimil Babka @ 2019-10-07  9:18 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, linux-kernel, kasan-dev, Qian Cai, Kirill A. Shutemov,
	Matthew Wilcox, Mel Gorman, Michal Hocko, Vlastimil Babka,
	Kirill A . Shutemov

Commit 37389167a281 ("mm, page_owner: keep owner info when freeing the page")
has introduced a flag PAGE_EXT_OWNER_ACTIVE to indicate that page is tracked as
being allocated.  Kirill suggested naming it PAGE_EXT_OWNER_ALLOCATED to make it
more clear, as "active is somewhat loaded term for a page".

Suggested-by: Kirill A. Shutemov <kirill@shutemov.name>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
 include/linux/page_ext.h |  2 +-
 mm/page_owner.c          | 12 ++++++------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/linux/page_ext.h b/include/linux/page_ext.h
index 5e856512bafb..cfce186f0c4e 100644
--- a/include/linux/page_ext.h
+++ b/include/linux/page_ext.h
@@ -18,7 +18,7 @@ struct page_ext_operations {
 
 enum page_ext_flags {
 	PAGE_EXT_OWNER,
-	PAGE_EXT_OWNER_ACTIVE,
+	PAGE_EXT_OWNER_ALLOCATED,
 #if defined(CONFIG_IDLE_PAGE_TRACKING) && !defined(CONFIG_64BIT)
 	PAGE_EXT_YOUNG,
 	PAGE_EXT_IDLE,
diff --git a/mm/page_owner.c b/mm/page_owner.c
index de1916ac3e24..e327bcd0380e 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -152,7 +152,7 @@ void __reset_page_owner(struct page *page, unsigned int order)
 	if (unlikely(!page_ext))
 		return;
 	for (i = 0; i < (1 << order); i++) {
-		__clear_bit(PAGE_EXT_OWNER_ACTIVE, &page_ext->flags);
+		__clear_bit(PAGE_EXT_OWNER_ALLOCATED, &page_ext->flags);
 		page_owner = get_page_owner(page_ext);
 		page_owner->free_handle = handle;
 		page_ext = page_ext_next(page_ext);
@@ -173,7 +173,7 @@ static inline void __set_page_owner_handle(struct page *page,
 		page_owner->gfp_mask = gfp_mask;
 		page_owner->last_migrate_reason = -1;
 		__set_bit(PAGE_EXT_OWNER, &page_ext->flags);
-		__set_bit(PAGE_EXT_OWNER_ACTIVE, &page_ext->flags);
+		__set_bit(PAGE_EXT_OWNER_ALLOCATED, &page_ext->flags);
 
 		page_ext = page_ext_next(page_ext);
 	}
@@ -247,7 +247,7 @@ void __copy_page_owner(struct page *oldpage, struct page *newpage)
 	 * the new page, which will be freed.
 	 */
 	__set_bit(PAGE_EXT_OWNER, &new_ext->flags);
-	__set_bit(PAGE_EXT_OWNER_ACTIVE, &new_ext->flags);
+	__set_bit(PAGE_EXT_OWNER_ALLOCATED, &new_ext->flags);
 }
 
 void pagetypeinfo_showmixedcount_print(struct seq_file *m,
@@ -307,7 +307,7 @@ void pagetypeinfo_showmixedcount_print(struct seq_file *m,
 			if (unlikely(!page_ext))
 				continue;
 
-			if (!test_bit(PAGE_EXT_OWNER_ACTIVE, &page_ext->flags))
+			if (!test_bit(PAGE_EXT_OWNER_ALLOCATED, &page_ext->flags))
 				continue;
 
 			page_owner = get_page_owner(page_ext);
@@ -422,7 +422,7 @@ void __dump_page_owner(struct page *page)
 		return;
 	}
 
-	if (test_bit(PAGE_EXT_OWNER_ACTIVE, &page_ext->flags))
+	if (test_bit(PAGE_EXT_OWNER_ALLOCATED, &page_ext->flags))
 		pr_alert("page_owner tracks the page as allocated\n");
 	else
 		pr_alert("page_owner tracks the page as freed\n");
@@ -512,7 +512,7 @@ read_page_owner(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 		 * Although we do have the info about past allocation of free
 		 * pages, it's not relevant for current memory usage.
 		 */
-		if (!test_bit(PAGE_EXT_OWNER_ACTIVE, &page_ext->flags))
+		if (!test_bit(PAGE_EXT_OWNER_ALLOCATED, &page_ext->flags))
 			continue;
 
 		page_owner = get_page_owner(page_ext);
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 2/3] mm, page_owner: decouple freeing stack trace from debug_pagealloc
  2019-10-07  9:18 ` [PATCH v3 2/3] mm, page_owner: decouple freeing stack trace from debug_pagealloc Vlastimil Babka
@ 2019-10-07 10:41   ` Qian Cai
  0 siblings, 0 replies; 5+ messages in thread
From: Qian Cai @ 2019-10-07 10:41 UTC (permalink / raw)
  To: Vlastimil Babka
  Cc: Andrew Morton, linux-mm, linux-kernel, kasan-dev,
	Kirill A. Shutemov, Matthew Wilcox, Mel Gorman, Michal Hocko,
	Dmitry Vyukov, Walter Wu, Andrey Ryabinin



> On Oct 7, 2019, at 5:18 AM, Vlastimil Babka <vbabka@suse.cz> wrote:
> 
> The commit 8974558f49a6 ("mm, page_owner, debug_pagealloc: save and dump
> freeing stack trace") enhanced page_owner to also store freeing stack trace,
> when debug_pagealloc is also enabled. KASAN would also like to do this [1] to
> improve error reports to debug e.g. UAF issues. Kirill has suggested that the
> freeing stack trace saving should be also possible to be enabled separately
> from KASAN or debug_pagealloc, i.e. with an extra boot option. Qian argued that
> we have enough options already, and avoiding the extra overhead is not worth
> the complications in the case of a debugging option. Kirill noted that the
> extra stack handle in struct page_owner requires 0.1% of memory.
> 
> This patch therefore enables free stack saving whenever page_owner is enabled,
> regardless of whether debug_pagealloc or KASAN is also enabled. KASAN kernels
> booted with page_owner=on will thus benefit from the improved error reports.
> 
> [1] https://bugzilla.kernel.org/show_bug.cgi?id=203967
> 
> Suggested-by: Dmitry Vyukov <dvyukov@google.com>
> Suggested-by: Walter Wu <walter-zh.wu@mediatek.com>
> Suggested-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
> Suggested-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Suggested-by: Qian Cai <cai@lca.pw>
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> ---
> Documentation/dev-tools/kasan.rst |  3 +++
> mm/page_owner.c                   | 28 +++++++---------------------
> 2 files changed, 10 insertions(+), 21 deletions(-)

The diffstat looks nice!

Reviewed-by: Qian Cai <cai@lca.pw>

> 
> diff --git a/Documentation/dev-tools/kasan.rst b/Documentation/dev-tools/kasan.rst
> index b72d07d70239..525296121d89 100644
> --- a/Documentation/dev-tools/kasan.rst
> +++ b/Documentation/dev-tools/kasan.rst
> @@ -41,6 +41,9 @@ smaller binary while the latter is 1.1 - 2 times faster.
> Both KASAN modes work with both SLUB and SLAB memory allocators.
> For better bug detection and nicer reporting, enable CONFIG_STACKTRACE.
> 
> +To augment reports with last allocation and freeing stack of the physical page,
> +it is recommended to enable also CONFIG_PAGE_OWNER and boot with page_owner=on.
> +
> To disable instrumentation for specific files or directories, add a line
> similar to the following to the respective kernel Makefile:
> 
> diff --git a/mm/page_owner.c b/mm/page_owner.c
> index d3cf5d336ccf..de1916ac3e24 100644
> --- a/mm/page_owner.c
> +++ b/mm/page_owner.c
> @@ -24,12 +24,10 @@ struct page_owner {
>    short last_migrate_reason;
>    gfp_t gfp_mask;
>    depot_stack_handle_t handle;
> -#ifdef CONFIG_DEBUG_PAGEALLOC
>    depot_stack_handle_t free_handle;
> -#endif
> };
> 
> -static bool page_owner_disabled = true;
> +static bool page_owner_enabled = false;
> DEFINE_STATIC_KEY_FALSE(page_owner_inited);
> 
> static depot_stack_handle_t dummy_handle;
> @@ -44,7 +42,7 @@ static int __init early_page_owner_param(char *buf)
>        return -EINVAL;
> 
>    if (strcmp(buf, "on") == 0)
> -        page_owner_disabled = false;
> +        page_owner_enabled = true;
> 
>    return 0;
> }
> @@ -52,10 +50,7 @@ early_param("page_owner", early_page_owner_param);
> 
> static bool need_page_owner(void)
> {
> -    if (page_owner_disabled)
> -        return false;
> -
> -    return true;
> +    return page_owner_enabled;
> }
> 
> static __always_inline depot_stack_handle_t create_dummy_stack(void)
> @@ -84,7 +79,7 @@ static noinline void register_early_stack(void)
> 
> static void init_page_owner(void)
> {
> -    if (page_owner_disabled)
> +    if (!page_owner_enabled)
>        return;
> 
>    register_dummy_stack();
> @@ -148,25 +143,18 @@ void __reset_page_owner(struct page *page, unsigned int order)
> {
>    int i;
>    struct page_ext *page_ext;
> -#ifdef CONFIG_DEBUG_PAGEALLOC
>    depot_stack_handle_t handle = 0;
>    struct page_owner *page_owner;
> 
> -    if (debug_pagealloc_enabled())
> -        handle = save_stack(GFP_NOWAIT | __GFP_NOWARN);
> -#endif
> +    handle = save_stack(GFP_NOWAIT | __GFP_NOWARN);
> 
>    page_ext = lookup_page_ext(page);
>    if (unlikely(!page_ext))
>        return;
>    for (i = 0; i < (1 << order); i++) {
>        __clear_bit(PAGE_EXT_OWNER_ACTIVE, &page_ext->flags);
> -#ifdef CONFIG_DEBUG_PAGEALLOC
> -        if (debug_pagealloc_enabled()) {
> -            page_owner = get_page_owner(page_ext);
> -            page_owner->free_handle = handle;
> -        }
> -#endif
> +        page_owner = get_page_owner(page_ext);
> +        page_owner->free_handle = handle;
>        page_ext = page_ext_next(page_ext);
>    }
> }
> @@ -450,7 +438,6 @@ void __dump_page_owner(struct page *page)
>        stack_trace_print(entries, nr_entries, 0);
>    }
> 
> -#ifdef CONFIG_DEBUG_PAGEALLOC
>    handle = READ_ONCE(page_owner->free_handle);
>    if (!handle) {
>        pr_alert("page_owner free stack trace missing\n");
> @@ -459,7 +446,6 @@ void __dump_page_owner(struct page *page)
>        pr_alert("page last free stack trace:\n");
>        stack_trace_print(entries, nr_entries, 0);
>    }
> -#endif
> 
>    if (page_owner->last_migrate_reason != -1)
>        pr_alert("page has been migrated, last migrate reason: %s\n",
> -- 
> 2.23.0

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-10-07 10:41 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-07  9:18 [PATCH v3 0/3] followups to debug_pagealloc improvements through page_owner Vlastimil Babka
2019-10-07  9:18 ` [PATCH v3 1/3] mm, page_owner: fix off-by-one error in __set_page_owner_handle() Vlastimil Babka
2019-10-07  9:18 ` [PATCH v3 2/3] mm, page_owner: decouple freeing stack trace from debug_pagealloc Vlastimil Babka
2019-10-07 10:41   ` Qian Cai
2019-10-07  9:18 ` [PATCH v3 3/3] mm, page_owner: rename flag indicating that page is allocated Vlastimil Babka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).