* [PATCH 0/3][RFC] mm/page:_owner: track page free call chain
@ 2016-07-02 16:16 ` Sergey Senozhatsky
0 siblings, 0 replies; 24+ messages in thread
From: Sergey Senozhatsky @ 2016-07-02 16:16 UTC (permalink / raw)
To: Joonsoo Kim
Cc: Andrew Morton, Vlastimil Babka, linux-mm, linux-kernel,
Sergey Senozhatsky, Sergey Senozhatsky
Hello,
RFC
Page owner tracks a call chain that has allocated a page, this
patch set extends it with a PAGE_OWNER_TRACK_FREE functionality,
that now also tracks a call chain that has freed the page.
Page dump, thus, now has the following format:
a) page allocated backtrace
b) page free backtrace
c) backtrace of path that has trigger bad page
For a quick example of a case when this new b) part can make a
difference please see 0003.
Sergey Senozhatsky (3):
mm/page_owner: rename page_owner functions
mm/page_owner: rename PAGE_EXT_OWNER flag
mm/page_owner: track page free call chain
include/linux/page_ext.h | 15 ++++++-
include/linux/page_owner.h | 16 +++----
lib/Kconfig.debug | 10 +++++
mm/page_alloc.c | 4 +-
mm/page_owner.c | 110 +++++++++++++++++++++++++++++++--------------
mm/vmstat.c | 5 ++-
6 files changed, 113 insertions(+), 47 deletions(-)
--
2.9.0.37.g6d523a3
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 0/3][RFC] mm/page:_owner: track page free call chain
@ 2016-07-02 16:16 ` Sergey Senozhatsky
0 siblings, 0 replies; 24+ messages in thread
From: Sergey Senozhatsky @ 2016-07-02 16:16 UTC (permalink / raw)
To: Joonsoo Kim
Cc: Andrew Morton, Vlastimil Babka, linux-mm, linux-kernel,
Sergey Senozhatsky, Sergey Senozhatsky
Hello,
RFC
Page owner tracks a call chain that has allocated a page, this
patch set extends it with a PAGE_OWNER_TRACK_FREE functionality,
that now also tracks a call chain that has freed the page.
Page dump, thus, now has the following format:
a) page allocated backtrace
b) page free backtrace
c) backtrace of path that has trigger bad page
For a quick example of a case when this new b) part can make a
difference please see 0003.
Sergey Senozhatsky (3):
mm/page_owner: rename page_owner functions
mm/page_owner: rename PAGE_EXT_OWNER flag
mm/page_owner: track page free call chain
include/linux/page_ext.h | 15 ++++++-
include/linux/page_owner.h | 16 +++----
lib/Kconfig.debug | 10 +++++
mm/page_alloc.c | 4 +-
mm/page_owner.c | 110 +++++++++++++++++++++++++++++++--------------
mm/vmstat.c | 5 ++-
6 files changed, 113 insertions(+), 47 deletions(-)
--
2.9.0.37.g6d523a3
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/3] mm/page_owner: rename page_owner functions
2016-07-02 16:16 ` Sergey Senozhatsky
@ 2016-07-02 16:16 ` Sergey Senozhatsky
-1 siblings, 0 replies; 24+ messages in thread
From: Sergey Senozhatsky @ 2016-07-02 16:16 UTC (permalink / raw)
To: Joonsoo Kim
Cc: Andrew Morton, Vlastimil Babka, linux-mm, linux-kernel,
Sergey Senozhatsky, Sergey Senozhatsky
A cosmetic change:
rename set_page_owner() and reset_page_owner() functions to
page_owner_alloc_pages()/page_owner_free_pages(). This is
sort of a preparation step for PAGE_OWNER_TRACK_FREE patch.
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
include/linux/page_owner.h | 16 ++++++++--------
mm/page_alloc.c | 4 ++--
mm/page_owner.c | 6 +++---
3 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/include/linux/page_owner.h b/include/linux/page_owner.h
index 30583ab..7b25953 100644
--- a/include/linux/page_owner.h
+++ b/include/linux/page_owner.h
@@ -7,25 +7,25 @@
extern struct static_key_false page_owner_inited;
extern struct page_ext_operations page_owner_ops;
-extern void __reset_page_owner(struct page *page, unsigned int order);
-extern void __set_page_owner(struct page *page,
+extern void __page_owner_free_pages(struct page *page, unsigned int order);
+extern void __page_owner_alloc_pages(struct page *page,
unsigned int order, gfp_t gfp_mask);
extern void __split_page_owner(struct page *page, unsigned int order);
extern void __copy_page_owner(struct page *oldpage, struct page *newpage);
extern void __set_page_owner_migrate_reason(struct page *page, int reason);
extern void __dump_page_owner(struct page *page);
-static inline void reset_page_owner(struct page *page, unsigned int order)
+static inline void page_owner_free_pages(struct page *page, unsigned int order)
{
if (static_branch_unlikely(&page_owner_inited))
- __reset_page_owner(page, order);
+ __page_owner_free_pages(page, order);
}
-static inline void set_page_owner(struct page *page,
+static inline void page_owner_alloc_pages(struct page *page,
unsigned int order, gfp_t gfp_mask)
{
if (static_branch_unlikely(&page_owner_inited))
- __set_page_owner(page, order, gfp_mask);
+ __page_owner_alloc_pages(page, order, gfp_mask);
}
static inline void split_page_owner(struct page *page, unsigned int order)
@@ -49,10 +49,10 @@ static inline void dump_page_owner(struct page *page)
__dump_page_owner(page);
}
#else
-static inline void reset_page_owner(struct page *page, unsigned int order)
+static inline void page_owner_free_pages(struct page *page, unsigned int order)
{
}
-static inline void set_page_owner(struct page *page,
+static inline void page_owner_alloc_pages(struct page *page,
unsigned int order, gfp_t gfp_mask)
{
}
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 63801c7..5e3cba4 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1031,7 +1031,7 @@ static __always_inline bool free_pages_prepare(struct page *page,
page_cpupid_reset_last(page);
page->flags &= ~PAGE_FLAGS_CHECK_AT_PREP;
- reset_page_owner(page, order);
+ page_owner_free_pages(page, order);
if (!PageHighMem(page)) {
debug_check_no_locks_freed(page_address(page),
@@ -1770,7 +1770,7 @@ void post_alloc_hook(struct page *page, unsigned int order, gfp_t gfp_flags)
kernel_map_pages(page, 1 << order, 1);
kernel_poison_pages(page, 1 << order, 1);
kasan_alloc_pages(page, order);
- set_page_owner(page, order, gfp_flags);
+ page_owner_alloc_pages(page, order, gfp_flags);
}
static void prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags,
diff --git a/mm/page_owner.c b/mm/page_owner.c
index 8fa5083..fde443a 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -85,7 +85,7 @@ struct page_ext_operations page_owner_ops = {
.init = init_page_owner,
};
-void __reset_page_owner(struct page *page, unsigned int order)
+void __page_owner_free_pages(struct page *page, unsigned int order)
{
int i;
struct page_ext *page_ext;
@@ -147,7 +147,7 @@ static noinline depot_stack_handle_t save_stack(gfp_t flags)
return handle;
}
-noinline void __set_page_owner(struct page *page, unsigned int order,
+noinline void __page_owner_alloc_pages(struct page *page, unsigned int order,
gfp_t gfp_mask)
{
struct page_ext *page_ext = lookup_page_ext(page);
@@ -452,7 +452,7 @@ static void init_pages_in_zone(pg_data_t *pgdat, struct zone *zone)
continue;
/* Found early allocated page */
- set_page_owner(page, 0, 0);
+ __page_owner_alloc_pages(page, 0, 0);
count++;
}
}
--
2.9.0.37.g6d523a3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 1/3] mm/page_owner: rename page_owner functions
@ 2016-07-02 16:16 ` Sergey Senozhatsky
0 siblings, 0 replies; 24+ messages in thread
From: Sergey Senozhatsky @ 2016-07-02 16:16 UTC (permalink / raw)
To: Joonsoo Kim
Cc: Andrew Morton, Vlastimil Babka, linux-mm, linux-kernel,
Sergey Senozhatsky, Sergey Senozhatsky
A cosmetic change:
rename set_page_owner() and reset_page_owner() functions to
page_owner_alloc_pages()/page_owner_free_pages(). This is
sort of a preparation step for PAGE_OWNER_TRACK_FREE patch.
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
include/linux/page_owner.h | 16 ++++++++--------
mm/page_alloc.c | 4 ++--
mm/page_owner.c | 6 +++---
3 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/include/linux/page_owner.h b/include/linux/page_owner.h
index 30583ab..7b25953 100644
--- a/include/linux/page_owner.h
+++ b/include/linux/page_owner.h
@@ -7,25 +7,25 @@
extern struct static_key_false page_owner_inited;
extern struct page_ext_operations page_owner_ops;
-extern void __reset_page_owner(struct page *page, unsigned int order);
-extern void __set_page_owner(struct page *page,
+extern void __page_owner_free_pages(struct page *page, unsigned int order);
+extern void __page_owner_alloc_pages(struct page *page,
unsigned int order, gfp_t gfp_mask);
extern void __split_page_owner(struct page *page, unsigned int order);
extern void __copy_page_owner(struct page *oldpage, struct page *newpage);
extern void __set_page_owner_migrate_reason(struct page *page, int reason);
extern void __dump_page_owner(struct page *page);
-static inline void reset_page_owner(struct page *page, unsigned int order)
+static inline void page_owner_free_pages(struct page *page, unsigned int order)
{
if (static_branch_unlikely(&page_owner_inited))
- __reset_page_owner(page, order);
+ __page_owner_free_pages(page, order);
}
-static inline void set_page_owner(struct page *page,
+static inline void page_owner_alloc_pages(struct page *page,
unsigned int order, gfp_t gfp_mask)
{
if (static_branch_unlikely(&page_owner_inited))
- __set_page_owner(page, order, gfp_mask);
+ __page_owner_alloc_pages(page, order, gfp_mask);
}
static inline void split_page_owner(struct page *page, unsigned int order)
@@ -49,10 +49,10 @@ static inline void dump_page_owner(struct page *page)
__dump_page_owner(page);
}
#else
-static inline void reset_page_owner(struct page *page, unsigned int order)
+static inline void page_owner_free_pages(struct page *page, unsigned int order)
{
}
-static inline void set_page_owner(struct page *page,
+static inline void page_owner_alloc_pages(struct page *page,
unsigned int order, gfp_t gfp_mask)
{
}
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 63801c7..5e3cba4 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1031,7 +1031,7 @@ static __always_inline bool free_pages_prepare(struct page *page,
page_cpupid_reset_last(page);
page->flags &= ~PAGE_FLAGS_CHECK_AT_PREP;
- reset_page_owner(page, order);
+ page_owner_free_pages(page, order);
if (!PageHighMem(page)) {
debug_check_no_locks_freed(page_address(page),
@@ -1770,7 +1770,7 @@ void post_alloc_hook(struct page *page, unsigned int order, gfp_t gfp_flags)
kernel_map_pages(page, 1 << order, 1);
kernel_poison_pages(page, 1 << order, 1);
kasan_alloc_pages(page, order);
- set_page_owner(page, order, gfp_flags);
+ page_owner_alloc_pages(page, order, gfp_flags);
}
static void prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags,
diff --git a/mm/page_owner.c b/mm/page_owner.c
index 8fa5083..fde443a 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -85,7 +85,7 @@ struct page_ext_operations page_owner_ops = {
.init = init_page_owner,
};
-void __reset_page_owner(struct page *page, unsigned int order)
+void __page_owner_free_pages(struct page *page, unsigned int order)
{
int i;
struct page_ext *page_ext;
@@ -147,7 +147,7 @@ static noinline depot_stack_handle_t save_stack(gfp_t flags)
return handle;
}
-noinline void __set_page_owner(struct page *page, unsigned int order,
+noinline void __page_owner_alloc_pages(struct page *page, unsigned int order,
gfp_t gfp_mask)
{
struct page_ext *page_ext = lookup_page_ext(page);
@@ -452,7 +452,7 @@ static void init_pages_in_zone(pg_data_t *pgdat, struct zone *zone)
continue;
/* Found early allocated page */
- set_page_owner(page, 0, 0);
+ __page_owner_alloc_pages(page, 0, 0);
count++;
}
}
--
2.9.0.37.g6d523a3
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/3] mm/page_owner: rename PAGE_EXT_OWNER flag
2016-07-02 16:16 ` Sergey Senozhatsky
@ 2016-07-02 16:16 ` Sergey Senozhatsky
-1 siblings, 0 replies; 24+ messages in thread
From: Sergey Senozhatsky @ 2016-07-02 16:16 UTC (permalink / raw)
To: Joonsoo Kim
Cc: Andrew Morton, Vlastimil Babka, linux-mm, linux-kernel,
Sergey Senozhatsky, Sergey Senozhatsky
A cosmetic change:
PAGE_OWNER_TRACK_FREE will introduce one more page_owner
flag: PAGE_EXT_OWNER_FREE. To make names symmetrical, rename
PAGE_EXT_OWNER to PAGE_EXT_OWNER_ALLOC.
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
include/linux/page_ext.h | 2 +-
mm/page_owner.c | 12 ++++++------
mm/vmstat.c | 2 +-
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/include/linux/page_ext.h b/include/linux/page_ext.h
index 03f2a3e..66ba2bb 100644
--- a/include/linux/page_ext.h
+++ b/include/linux/page_ext.h
@@ -26,7 +26,7 @@ struct page_ext_operations {
enum page_ext_flags {
PAGE_EXT_DEBUG_POISON, /* Page is poisoned */
PAGE_EXT_DEBUG_GUARD,
- PAGE_EXT_OWNER,
+ PAGE_EXT_OWNER_ALLOC,
#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 fde443a..4acccb7 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -94,7 +94,7 @@ void __page_owner_free_pages(struct page *page, unsigned int order)
page_ext = lookup_page_ext(page + i);
if (unlikely(!page_ext))
continue;
- __clear_bit(PAGE_EXT_OWNER, &page_ext->flags);
+ __clear_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags);
}
}
@@ -160,7 +160,7 @@ noinline void __page_owner_alloc_pages(struct page *page, unsigned int order,
page_ext->gfp_mask = gfp_mask;
page_ext->last_migrate_reason = -1;
- __set_bit(PAGE_EXT_OWNER, &page_ext->flags);
+ __set_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags);
}
void __set_page_owner_migrate_reason(struct page *page, int reason)
@@ -207,7 +207,7 @@ void __copy_page_owner(struct page *oldpage, struct page *newpage)
* in that case we also don't need to explicitly clear the info from
* the new page, which will be freed.
*/
- __set_bit(PAGE_EXT_OWNER, &new_ext->flags);
+ __set_bit(PAGE_EXT_OWNER_ALLOC, &new_ext->flags);
}
static ssize_t
@@ -301,7 +301,7 @@ void __dump_page_owner(struct page *page)
gfp_mask = page_ext->gfp_mask;
mt = gfpflags_to_migratetype(gfp_mask);
- if (!test_bit(PAGE_EXT_OWNER, &page_ext->flags)) {
+ if (!test_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags)) {
pr_alert("page_owner info is not active (free page?)\n");
return;
}
@@ -374,7 +374,7 @@ read_page_owner(struct file *file, char __user *buf, size_t count, loff_t *ppos)
* Some pages could be missed by concurrent allocation or free,
* because we don't hold the zone lock.
*/
- if (!test_bit(PAGE_EXT_OWNER, &page_ext->flags))
+ if (!test_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags))
continue;
/*
@@ -448,7 +448,7 @@ static void init_pages_in_zone(pg_data_t *pgdat, struct zone *zone)
continue;
/* Maybe overraping zone */
- if (test_bit(PAGE_EXT_OWNER, &page_ext->flags))
+ if (test_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags))
continue;
/* Found early allocated page */
diff --git a/mm/vmstat.c b/mm/vmstat.c
index 7997f529..63ef65f 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -1070,7 +1070,7 @@ static void pagetypeinfo_showmixedcount_print(struct seq_file *m,
if (unlikely(!page_ext))
continue;
- if (!test_bit(PAGE_EXT_OWNER, &page_ext->flags))
+ if (!test_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags))
continue;
page_mt = gfpflags_to_migratetype(page_ext->gfp_mask);
--
2.9.0.37.g6d523a3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/3] mm/page_owner: rename PAGE_EXT_OWNER flag
@ 2016-07-02 16:16 ` Sergey Senozhatsky
0 siblings, 0 replies; 24+ messages in thread
From: Sergey Senozhatsky @ 2016-07-02 16:16 UTC (permalink / raw)
To: Joonsoo Kim
Cc: Andrew Morton, Vlastimil Babka, linux-mm, linux-kernel,
Sergey Senozhatsky, Sergey Senozhatsky
A cosmetic change:
PAGE_OWNER_TRACK_FREE will introduce one more page_owner
flag: PAGE_EXT_OWNER_FREE. To make names symmetrical, rename
PAGE_EXT_OWNER to PAGE_EXT_OWNER_ALLOC.
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
include/linux/page_ext.h | 2 +-
mm/page_owner.c | 12 ++++++------
mm/vmstat.c | 2 +-
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/include/linux/page_ext.h b/include/linux/page_ext.h
index 03f2a3e..66ba2bb 100644
--- a/include/linux/page_ext.h
+++ b/include/linux/page_ext.h
@@ -26,7 +26,7 @@ struct page_ext_operations {
enum page_ext_flags {
PAGE_EXT_DEBUG_POISON, /* Page is poisoned */
PAGE_EXT_DEBUG_GUARD,
- PAGE_EXT_OWNER,
+ PAGE_EXT_OWNER_ALLOC,
#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 fde443a..4acccb7 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -94,7 +94,7 @@ void __page_owner_free_pages(struct page *page, unsigned int order)
page_ext = lookup_page_ext(page + i);
if (unlikely(!page_ext))
continue;
- __clear_bit(PAGE_EXT_OWNER, &page_ext->flags);
+ __clear_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags);
}
}
@@ -160,7 +160,7 @@ noinline void __page_owner_alloc_pages(struct page *page, unsigned int order,
page_ext->gfp_mask = gfp_mask;
page_ext->last_migrate_reason = -1;
- __set_bit(PAGE_EXT_OWNER, &page_ext->flags);
+ __set_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags);
}
void __set_page_owner_migrate_reason(struct page *page, int reason)
@@ -207,7 +207,7 @@ void __copy_page_owner(struct page *oldpage, struct page *newpage)
* in that case we also don't need to explicitly clear the info from
* the new page, which will be freed.
*/
- __set_bit(PAGE_EXT_OWNER, &new_ext->flags);
+ __set_bit(PAGE_EXT_OWNER_ALLOC, &new_ext->flags);
}
static ssize_t
@@ -301,7 +301,7 @@ void __dump_page_owner(struct page *page)
gfp_mask = page_ext->gfp_mask;
mt = gfpflags_to_migratetype(gfp_mask);
- if (!test_bit(PAGE_EXT_OWNER, &page_ext->flags)) {
+ if (!test_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags)) {
pr_alert("page_owner info is not active (free page?)\n");
return;
}
@@ -374,7 +374,7 @@ read_page_owner(struct file *file, char __user *buf, size_t count, loff_t *ppos)
* Some pages could be missed by concurrent allocation or free,
* because we don't hold the zone lock.
*/
- if (!test_bit(PAGE_EXT_OWNER, &page_ext->flags))
+ if (!test_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags))
continue;
/*
@@ -448,7 +448,7 @@ static void init_pages_in_zone(pg_data_t *pgdat, struct zone *zone)
continue;
/* Maybe overraping zone */
- if (test_bit(PAGE_EXT_OWNER, &page_ext->flags))
+ if (test_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags))
continue;
/* Found early allocated page */
diff --git a/mm/vmstat.c b/mm/vmstat.c
index 7997f529..63ef65f 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -1070,7 +1070,7 @@ static void pagetypeinfo_showmixedcount_print(struct seq_file *m,
if (unlikely(!page_ext))
continue;
- if (!test_bit(PAGE_EXT_OWNER, &page_ext->flags))
+ if (!test_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags))
continue;
page_mt = gfpflags_to_migratetype(page_ext->gfp_mask);
--
2.9.0.37.g6d523a3
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 3/3] mm/page_owner: track page free call chain
2016-07-02 16:16 ` Sergey Senozhatsky
@ 2016-07-02 16:16 ` Sergey Senozhatsky
-1 siblings, 0 replies; 24+ messages in thread
From: Sergey Senozhatsky @ 2016-07-02 16:16 UTC (permalink / raw)
To: Joonsoo Kim
Cc: Andrew Morton, Vlastimil Babka, linux-mm, linux-kernel,
Sergey Senozhatsky, Sergey Senozhatsky
Introduce PAGE_OWNER_TRACK_FREE config option to extend page owner with
free_pages() tracking functionality. This adds to the dump_page_owner()
output an additional backtrace, that tells us what path has freed the
page.
Aa a trivial example, let's assume that do_some_foo() has an error - extra
put_page() on error return path, and the function is also getting preempted,
letting some other task to allocate the same page, which is then mistakenly
getting freed once again by do_some_foo().
CPUA CPUB
void do_some_foo(void)
{
page = alloc_page();
if (error) {
put_page(page);
goto out;
}
...
out:
<<preempted>>
void do_some_bar()
{
page = alloc_page();
...
<<preempted>>
...
put_page(page);
}
<<use freed page>>
put_page(page);
}
Backtrace:
BUG: Bad page state in process cc1 pfn:bae1d
page:ffffea0002eb8740 count:-1 mapcount:0 mapping: (null) index:0x0
flags: 0x4000000000000000()
page dumped because: nonzero _count
page allocated via order 0, migratetype Unmovable, gfp_mask 0x2000200(GFP_NOWAIT|__GFP_NOWARN)
[<ffffffff8101bc9c>] save_stack_trace+0x26/0x41
[<ffffffff81110fe4>] save_stack+0x46/0xc3
[<ffffffff81111481>] __page_owner_alloc_pages+0x24/0x41
[<ffffffff810c9867>] post_alloc_hook+0x1e/0x20
[<ffffffff810ca63d>] get_page_from_freelist+0x4fd/0x756
[<ffffffff810cadea>] __alloc_pages_nodemask+0xe7/0xbcf
[<ffffffff810cb8e4>] __get_free_pages+0x12/0x40
[<ffffffff810e6b64>] __tlb_remove_page_size.part.12+0x37/0x78
[<ffffffff810e6d9b>] __tlb_remove_page_size+0x21/0x23
[<ffffffff810e7ff2>] unmap_page_range+0x63a/0x75b
[<ffffffff810e81cf>] unmap_single_vma+0xbc/0xc6
[<ffffffff810e82d2>] unmap_vmas+0x35/0x44
[<ffffffff810ee6f4>] exit_mmap+0x5a/0xec
[<ffffffff810385b4>] mmput+0x4a/0xdc
[<ffffffff8103dff7>] do_exit+0x398/0x8de
[<ffffffff8103e5ae>] do_group_exit+0x45/0xb0
page freed via order 0, migratetype Unmovable, gfp_mask 0x2000200(GFP_NOWAIT|__GFP_NOWARN)
[<ffffffff8101bc9c>] save_stack_trace+0x26/0x41
[<ffffffff81110fe4>] save_stack+0x46/0xc3
[<ffffffff81111411>] __page_owner_free_pages+0x25/0x71
[<ffffffff810c9f0a>] free_hot_cold_page+0x1d6/0x1ea
[<ffffffff810d03e1>] __put_page+0x37/0x3a
[<ffffffff8115b8da>] do_some_foo()+0x8a/0x8e
...
Modules linked in: ....
CPU: 3 PID: 1274 Comm: cc1 Not tainted 4.7.0-rc5-next-20160701-dbg-00009-ge01494f-dirty #535
0000000000000000 ffff8800aeea3c18 ffffffff811e67ca ffffea0002eb8740
ffffffff8175675e ffff8800aeea3c40 ffffffff810c87f5 0000000000000000
ffffffff81880b40 ffff880137d98438 ffff8800aeea3c50 ffffffff810c88d5
Call Trace:
[<ffffffff811e67ca>] dump_stack+0x68/0x92
[<ffffffff810c87f5>] bad_page+0xf8/0x11e
[<ffffffff810c88d5>] check_new_page_bad+0x63/0x65
[<ffffffff810ca36a>] get_page_from_freelist+0x22a/0x756
[<ffffffff810cadea>] __alloc_pages_nodemask+0xe7/0xbcf
[<ffffffff81073a43>] ? trace_hardirqs_on_caller+0x16d/0x189
[<ffffffff810ede8d>] ? vma_merge+0x159/0x249
[<ffffffff81074aa0>] ? __lock_acquire+0x2ac/0x15c7
[<ffffffff81034ace>] pte_alloc_one+0x1b/0x67
[<ffffffff810e922b>] __pte_alloc+0x19/0xa6
[<ffffffff810eb09f>] handle_mm_fault+0x409/0xc59
[<ffffffff810309f6>] __do_page_fault+0x1d8/0x3ac
[<ffffffff81030bf7>] do_page_fault+0xc/0xe
[<ffffffff814a84af>] page_fault+0x1f/0x30
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
include/linux/page_ext.h | 13 ++++++-
lib/Kconfig.debug | 10 +++++
mm/page_owner.c | 98 ++++++++++++++++++++++++++++++++++--------------
mm/vmstat.c | 3 ++
4 files changed, 95 insertions(+), 29 deletions(-)
diff --git a/include/linux/page_ext.h b/include/linux/page_ext.h
index 66ba2bb..90bd44a 100644
--- a/include/linux/page_ext.h
+++ b/include/linux/page_ext.h
@@ -27,12 +27,23 @@ enum page_ext_flags {
PAGE_EXT_DEBUG_POISON, /* Page is poisoned */
PAGE_EXT_DEBUG_GUARD,
PAGE_EXT_OWNER_ALLOC,
+ PAGE_EXT_OWNER_FREE,
#if defined(CONFIG_IDLE_PAGE_TRACKING) && !defined(CONFIG_64BIT)
PAGE_EXT_YOUNG,
PAGE_EXT_IDLE,
#endif
};
+#ifdef CONFIG_PAGE_OWNER
+enum page_owner_handles {
+ PAGE_OWNER_HANDLE_ALLOC,
+#ifdef CONFIG_PAGE_OWNER_TRACK_FREE
+ PAGE_OWNER_HANDLE_FREE,
+#endif
+ PAGE_OWNER_HANDLE_MAX
+};
+#endif
+
/*
* Page Extension can be considered as an extended mem_map.
* A page_ext page is associated with every page descriptor. The
@@ -46,7 +57,7 @@ struct page_ext {
unsigned int order;
gfp_t gfp_mask;
int last_migrate_reason;
- depot_stack_handle_t handle;
+ depot_stack_handle_t handles[PAGE_OWNER_HANDLE_MAX];
#endif
};
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 0f99819..20ac03b 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -260,6 +260,16 @@ config PAGE_OWNER
If unsure, say N.
+config PAGE_OWNER_TRACK_FREE
+ bool "Track page free call chains"
+ depends on PAGE_OWNER
+ help
+ Page owner keeps track of what call chain is the owner of a page (has
+ allocated the page), this option enables an additional functionality
+ to also track what call chain has freed the page.
+
+ If unsure, say N.
+
config DEBUG_FS
bool "Debug Filesystem"
select SRCU
diff --git a/mm/page_owner.c b/mm/page_owner.c
index 4acccb7..5a108d6 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -13,6 +13,13 @@
#define PAGE_OWNER_STACK_DEPTH (16)
+static const char *page_owner_handles_names[PAGE_OWNER_HANDLE_MAX] = {
+ "page allocated",
+#ifdef CONFIG_PAGE_OWNER_TRACK_FREE
+ "page freed",
+#endif
+};
+
static bool page_owner_disabled = true;
DEFINE_STATIC_KEY_FALSE(page_owner_inited);
@@ -85,19 +92,6 @@ struct page_ext_operations page_owner_ops = {
.init = init_page_owner,
};
-void __page_owner_free_pages(struct page *page, unsigned int order)
-{
- int i;
- struct page_ext *page_ext;
-
- for (i = 0; i < (1 << order); i++) {
- page_ext = lookup_page_ext(page + i);
- if (unlikely(!page_ext))
- continue;
- __clear_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags);
- }
-}
-
static inline bool check_recursive_alloc(struct stack_trace *trace,
unsigned long ip)
{
@@ -147,6 +141,45 @@ static noinline depot_stack_handle_t save_stack(gfp_t flags)
return handle;
}
+#ifdef CONFIG_PAGE_OWNER_TRACK_FREE
+void __page_owner_free_pages(struct page *page, unsigned int order)
+{
+ int i;
+ depot_stack_handle_t handle;
+ struct page_ext *page_ext = lookup_page_ext(page);
+
+ if (unlikely(!page_ext))
+ return;
+
+ handle = save_stack(0);
+ page_ext->handles[PAGE_OWNER_HANDLE_FREE] = handle;
+ __set_bit(PAGE_EXT_OWNER_FREE, &page_ext->flags);
+
+ for (i = 1; i < (1 << order); i++) {
+ struct page_ext *ext = lookup_page_ext(page + 1);
+
+ if (unlikely(!ext))
+ continue;
+ ext->handles[PAGE_OWNER_HANDLE_FREE] = handle;
+ __set_bit(PAGE_EXT_OWNER_FREE, &ext->flags);
+ }
+}
+#else
+void __page_owner_free_pages(struct page *page, unsigned int order)
+{
+ int i;
+ struct page_ext *page_ext;
+
+ for (i = 0; i < (1 << order); i++) {
+ page_ext = lookup_page_ext(page + i);
+
+ if (unlikely(!page_ext))
+ continue;
+ __clear_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags);
+ }
+}
+#endif
+
noinline void __page_owner_alloc_pages(struct page *page, unsigned int order,
gfp_t gfp_mask)
{
@@ -155,7 +188,7 @@ noinline void __page_owner_alloc_pages(struct page *page, unsigned int order,
if (unlikely(!page_ext))
return;
- page_ext->handle = save_stack(gfp_mask);
+ page_ext->handles[PAGE_OWNER_HANDLE_ALLOC] = save_stack(gfp_mask);
page_ext->order = order;
page_ext->gfp_mask = gfp_mask;
page_ext->last_migrate_reason = -1;
@@ -189,6 +222,7 @@ void __copy_page_owner(struct page *oldpage, struct page *newpage)
{
struct page_ext *old_ext = lookup_page_ext(oldpage);
struct page_ext *new_ext = lookup_page_ext(newpage);
+ int i;
if (unlikely(!old_ext || !new_ext))
return;
@@ -196,7 +230,9 @@ void __copy_page_owner(struct page *oldpage, struct page *newpage)
new_ext->order = old_ext->order;
new_ext->gfp_mask = old_ext->gfp_mask;
new_ext->last_migrate_reason = old_ext->last_migrate_reason;
- new_ext->handle = old_ext->handle;
+
+ for (i = 0; i < PAGE_OWNER_HANDLE_MAX; i++)
+ new_ext->handles[i] = old_ext->handles[i];
/*
* We don't clear the bit on the oldpage as it's going to be freed
@@ -292,7 +328,7 @@ void __dump_page_owner(struct page *page)
};
depot_stack_handle_t handle;
gfp_t gfp_mask;
- int mt;
+ int mt, i;
if (unlikely(!page_ext)) {
pr_alert("There is not page extension available.\n");
@@ -301,25 +337,31 @@ void __dump_page_owner(struct page *page)
gfp_mask = page_ext->gfp_mask;
mt = gfpflags_to_migratetype(gfp_mask);
- if (!test_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags)) {
+ if (!test_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags) &&
+ !test_bit(PAGE_EXT_OWNER_FREE, &page_ext->flags)) {
pr_alert("page_owner info is not active (free page?)\n");
return;
}
- handle = READ_ONCE(page_ext->handle);
- if (!handle) {
- pr_alert("page_owner info is not active (free page?)\n");
- return;
- }
+ for (i = 0; i < PAGE_OWNER_HANDLE_MAX; i++) {
+ handle = READ_ONCE(page_ext->handles[i]);
+ if (!handle) {
+ pr_alert("page_owner info is not active for `%s'\n",
+ page_owner_handles_names[i]);
+ continue;
+ }
- depot_fetch_stack(handle, &trace);
- pr_alert("page allocated via order %u, migratetype %s, gfp_mask %#x(%pGg)\n",
- page_ext->order, migratetype_names[mt], gfp_mask, &gfp_mask);
- print_stack_trace(&trace, 0);
+ depot_fetch_stack(handle, &trace);
+ pr_alert("%s via order %u, migratetype %s, gfp_mask %#x(%pGg)\n",
+ page_owner_handles_names[i], page_ext->order,
+ migratetype_names[mt], gfp_mask, &gfp_mask);
+ print_stack_trace(&trace, 0);
- if (page_ext->last_migrate_reason != -1)
+ if (page_ext->last_migrate_reason == -1)
+ continue;
pr_alert("page has been migrated, last migrate reason: %s\n",
migrate_reason_names[page_ext->last_migrate_reason]);
+ }
}
static ssize_t
@@ -381,7 +423,7 @@ read_page_owner(struct file *file, char __user *buf, size_t count, loff_t *ppos)
* Access to page_ext->handle isn't synchronous so we should
* be careful to access it.
*/
- handle = READ_ONCE(page_ext->handle);
+ handle = READ_ONCE(page_ext->handles[PAGE_OWNER_HANDLE_ALLOC]);
if (!handle)
continue;
diff --git a/mm/vmstat.c b/mm/vmstat.c
index 63ef65f..4ff0135 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -1073,6 +1073,9 @@ static void pagetypeinfo_showmixedcount_print(struct seq_file *m,
if (!test_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags))
continue;
+ if (!test_bit(PAGE_EXT_OWNER_FREE, &page_ext->flags))
+ continue;
+
page_mt = gfpflags_to_migratetype(page_ext->gfp_mask);
if (pageblock_mt != page_mt) {
if (is_migrate_cma(pageblock_mt))
--
2.9.0.37.g6d523a3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 3/3] mm/page_owner: track page free call chain
@ 2016-07-02 16:16 ` Sergey Senozhatsky
0 siblings, 0 replies; 24+ messages in thread
From: Sergey Senozhatsky @ 2016-07-02 16:16 UTC (permalink / raw)
To: Joonsoo Kim
Cc: Andrew Morton, Vlastimil Babka, linux-mm, linux-kernel,
Sergey Senozhatsky, Sergey Senozhatsky
Introduce PAGE_OWNER_TRACK_FREE config option to extend page owner with
free_pages() tracking functionality. This adds to the dump_page_owner()
output an additional backtrace, that tells us what path has freed the
page.
Aa a trivial example, let's assume that do_some_foo() has an error - extra
put_page() on error return path, and the function is also getting preempted,
letting some other task to allocate the same page, which is then mistakenly
getting freed once again by do_some_foo().
CPUA CPUB
void do_some_foo(void)
{
page = alloc_page();
if (error) {
put_page(page);
goto out;
}
...
out:
<<preempted>>
void do_some_bar()
{
page = alloc_page();
...
<<preempted>>
...
put_page(page);
}
<<use freed page>>
put_page(page);
}
Backtrace:
BUG: Bad page state in process cc1 pfn:bae1d
page:ffffea0002eb8740 count:-1 mapcount:0 mapping: (null) index:0x0
flags: 0x4000000000000000()
page dumped because: nonzero _count
page allocated via order 0, migratetype Unmovable, gfp_mask 0x2000200(GFP_NOWAIT|__GFP_NOWARN)
[<ffffffff8101bc9c>] save_stack_trace+0x26/0x41
[<ffffffff81110fe4>] save_stack+0x46/0xc3
[<ffffffff81111481>] __page_owner_alloc_pages+0x24/0x41
[<ffffffff810c9867>] post_alloc_hook+0x1e/0x20
[<ffffffff810ca63d>] get_page_from_freelist+0x4fd/0x756
[<ffffffff810cadea>] __alloc_pages_nodemask+0xe7/0xbcf
[<ffffffff810cb8e4>] __get_free_pages+0x12/0x40
[<ffffffff810e6b64>] __tlb_remove_page_size.part.12+0x37/0x78
[<ffffffff810e6d9b>] __tlb_remove_page_size+0x21/0x23
[<ffffffff810e7ff2>] unmap_page_range+0x63a/0x75b
[<ffffffff810e81cf>] unmap_single_vma+0xbc/0xc6
[<ffffffff810e82d2>] unmap_vmas+0x35/0x44
[<ffffffff810ee6f4>] exit_mmap+0x5a/0xec
[<ffffffff810385b4>] mmput+0x4a/0xdc
[<ffffffff8103dff7>] do_exit+0x398/0x8de
[<ffffffff8103e5ae>] do_group_exit+0x45/0xb0
page freed via order 0, migratetype Unmovable, gfp_mask 0x2000200(GFP_NOWAIT|__GFP_NOWARN)
[<ffffffff8101bc9c>] save_stack_trace+0x26/0x41
[<ffffffff81110fe4>] save_stack+0x46/0xc3
[<ffffffff81111411>] __page_owner_free_pages+0x25/0x71
[<ffffffff810c9f0a>] free_hot_cold_page+0x1d6/0x1ea
[<ffffffff810d03e1>] __put_page+0x37/0x3a
[<ffffffff8115b8da>] do_some_foo()+0x8a/0x8e
...
Modules linked in: ....
CPU: 3 PID: 1274 Comm: cc1 Not tainted 4.7.0-rc5-next-20160701-dbg-00009-ge01494f-dirty #535
0000000000000000 ffff8800aeea3c18 ffffffff811e67ca ffffea0002eb8740
ffffffff8175675e ffff8800aeea3c40 ffffffff810c87f5 0000000000000000
ffffffff81880b40 ffff880137d98438 ffff8800aeea3c50 ffffffff810c88d5
Call Trace:
[<ffffffff811e67ca>] dump_stack+0x68/0x92
[<ffffffff810c87f5>] bad_page+0xf8/0x11e
[<ffffffff810c88d5>] check_new_page_bad+0x63/0x65
[<ffffffff810ca36a>] get_page_from_freelist+0x22a/0x756
[<ffffffff810cadea>] __alloc_pages_nodemask+0xe7/0xbcf
[<ffffffff81073a43>] ? trace_hardirqs_on_caller+0x16d/0x189
[<ffffffff810ede8d>] ? vma_merge+0x159/0x249
[<ffffffff81074aa0>] ? __lock_acquire+0x2ac/0x15c7
[<ffffffff81034ace>] pte_alloc_one+0x1b/0x67
[<ffffffff810e922b>] __pte_alloc+0x19/0xa6
[<ffffffff810eb09f>] handle_mm_fault+0x409/0xc59
[<ffffffff810309f6>] __do_page_fault+0x1d8/0x3ac
[<ffffffff81030bf7>] do_page_fault+0xc/0xe
[<ffffffff814a84af>] page_fault+0x1f/0x30
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
include/linux/page_ext.h | 13 ++++++-
lib/Kconfig.debug | 10 +++++
mm/page_owner.c | 98 ++++++++++++++++++++++++++++++++++--------------
mm/vmstat.c | 3 ++
4 files changed, 95 insertions(+), 29 deletions(-)
diff --git a/include/linux/page_ext.h b/include/linux/page_ext.h
index 66ba2bb..90bd44a 100644
--- a/include/linux/page_ext.h
+++ b/include/linux/page_ext.h
@@ -27,12 +27,23 @@ enum page_ext_flags {
PAGE_EXT_DEBUG_POISON, /* Page is poisoned */
PAGE_EXT_DEBUG_GUARD,
PAGE_EXT_OWNER_ALLOC,
+ PAGE_EXT_OWNER_FREE,
#if defined(CONFIG_IDLE_PAGE_TRACKING) && !defined(CONFIG_64BIT)
PAGE_EXT_YOUNG,
PAGE_EXT_IDLE,
#endif
};
+#ifdef CONFIG_PAGE_OWNER
+enum page_owner_handles {
+ PAGE_OWNER_HANDLE_ALLOC,
+#ifdef CONFIG_PAGE_OWNER_TRACK_FREE
+ PAGE_OWNER_HANDLE_FREE,
+#endif
+ PAGE_OWNER_HANDLE_MAX
+};
+#endif
+
/*
* Page Extension can be considered as an extended mem_map.
* A page_ext page is associated with every page descriptor. The
@@ -46,7 +57,7 @@ struct page_ext {
unsigned int order;
gfp_t gfp_mask;
int last_migrate_reason;
- depot_stack_handle_t handle;
+ depot_stack_handle_t handles[PAGE_OWNER_HANDLE_MAX];
#endif
};
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 0f99819..20ac03b 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -260,6 +260,16 @@ config PAGE_OWNER
If unsure, say N.
+config PAGE_OWNER_TRACK_FREE
+ bool "Track page free call chains"
+ depends on PAGE_OWNER
+ help
+ Page owner keeps track of what call chain is the owner of a page (has
+ allocated the page), this option enables an additional functionality
+ to also track what call chain has freed the page.
+
+ If unsure, say N.
+
config DEBUG_FS
bool "Debug Filesystem"
select SRCU
diff --git a/mm/page_owner.c b/mm/page_owner.c
index 4acccb7..5a108d6 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -13,6 +13,13 @@
#define PAGE_OWNER_STACK_DEPTH (16)
+static const char *page_owner_handles_names[PAGE_OWNER_HANDLE_MAX] = {
+ "page allocated",
+#ifdef CONFIG_PAGE_OWNER_TRACK_FREE
+ "page freed",
+#endif
+};
+
static bool page_owner_disabled = true;
DEFINE_STATIC_KEY_FALSE(page_owner_inited);
@@ -85,19 +92,6 @@ struct page_ext_operations page_owner_ops = {
.init = init_page_owner,
};
-void __page_owner_free_pages(struct page *page, unsigned int order)
-{
- int i;
- struct page_ext *page_ext;
-
- for (i = 0; i < (1 << order); i++) {
- page_ext = lookup_page_ext(page + i);
- if (unlikely(!page_ext))
- continue;
- __clear_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags);
- }
-}
-
static inline bool check_recursive_alloc(struct stack_trace *trace,
unsigned long ip)
{
@@ -147,6 +141,45 @@ static noinline depot_stack_handle_t save_stack(gfp_t flags)
return handle;
}
+#ifdef CONFIG_PAGE_OWNER_TRACK_FREE
+void __page_owner_free_pages(struct page *page, unsigned int order)
+{
+ int i;
+ depot_stack_handle_t handle;
+ struct page_ext *page_ext = lookup_page_ext(page);
+
+ if (unlikely(!page_ext))
+ return;
+
+ handle = save_stack(0);
+ page_ext->handles[PAGE_OWNER_HANDLE_FREE] = handle;
+ __set_bit(PAGE_EXT_OWNER_FREE, &page_ext->flags);
+
+ for (i = 1; i < (1 << order); i++) {
+ struct page_ext *ext = lookup_page_ext(page + 1);
+
+ if (unlikely(!ext))
+ continue;
+ ext->handles[PAGE_OWNER_HANDLE_FREE] = handle;
+ __set_bit(PAGE_EXT_OWNER_FREE, &ext->flags);
+ }
+}
+#else
+void __page_owner_free_pages(struct page *page, unsigned int order)
+{
+ int i;
+ struct page_ext *page_ext;
+
+ for (i = 0; i < (1 << order); i++) {
+ page_ext = lookup_page_ext(page + i);
+
+ if (unlikely(!page_ext))
+ continue;
+ __clear_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags);
+ }
+}
+#endif
+
noinline void __page_owner_alloc_pages(struct page *page, unsigned int order,
gfp_t gfp_mask)
{
@@ -155,7 +188,7 @@ noinline void __page_owner_alloc_pages(struct page *page, unsigned int order,
if (unlikely(!page_ext))
return;
- page_ext->handle = save_stack(gfp_mask);
+ page_ext->handles[PAGE_OWNER_HANDLE_ALLOC] = save_stack(gfp_mask);
page_ext->order = order;
page_ext->gfp_mask = gfp_mask;
page_ext->last_migrate_reason = -1;
@@ -189,6 +222,7 @@ void __copy_page_owner(struct page *oldpage, struct page *newpage)
{
struct page_ext *old_ext = lookup_page_ext(oldpage);
struct page_ext *new_ext = lookup_page_ext(newpage);
+ int i;
if (unlikely(!old_ext || !new_ext))
return;
@@ -196,7 +230,9 @@ void __copy_page_owner(struct page *oldpage, struct page *newpage)
new_ext->order = old_ext->order;
new_ext->gfp_mask = old_ext->gfp_mask;
new_ext->last_migrate_reason = old_ext->last_migrate_reason;
- new_ext->handle = old_ext->handle;
+
+ for (i = 0; i < PAGE_OWNER_HANDLE_MAX; i++)
+ new_ext->handles[i] = old_ext->handles[i];
/*
* We don't clear the bit on the oldpage as it's going to be freed
@@ -292,7 +328,7 @@ void __dump_page_owner(struct page *page)
};
depot_stack_handle_t handle;
gfp_t gfp_mask;
- int mt;
+ int mt, i;
if (unlikely(!page_ext)) {
pr_alert("There is not page extension available.\n");
@@ -301,25 +337,31 @@ void __dump_page_owner(struct page *page)
gfp_mask = page_ext->gfp_mask;
mt = gfpflags_to_migratetype(gfp_mask);
- if (!test_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags)) {
+ if (!test_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags) &&
+ !test_bit(PAGE_EXT_OWNER_FREE, &page_ext->flags)) {
pr_alert("page_owner info is not active (free page?)\n");
return;
}
- handle = READ_ONCE(page_ext->handle);
- if (!handle) {
- pr_alert("page_owner info is not active (free page?)\n");
- return;
- }
+ for (i = 0; i < PAGE_OWNER_HANDLE_MAX; i++) {
+ handle = READ_ONCE(page_ext->handles[i]);
+ if (!handle) {
+ pr_alert("page_owner info is not active for `%s'\n",
+ page_owner_handles_names[i]);
+ continue;
+ }
- depot_fetch_stack(handle, &trace);
- pr_alert("page allocated via order %u, migratetype %s, gfp_mask %#x(%pGg)\n",
- page_ext->order, migratetype_names[mt], gfp_mask, &gfp_mask);
- print_stack_trace(&trace, 0);
+ depot_fetch_stack(handle, &trace);
+ pr_alert("%s via order %u, migratetype %s, gfp_mask %#x(%pGg)\n",
+ page_owner_handles_names[i], page_ext->order,
+ migratetype_names[mt], gfp_mask, &gfp_mask);
+ print_stack_trace(&trace, 0);
- if (page_ext->last_migrate_reason != -1)
+ if (page_ext->last_migrate_reason == -1)
+ continue;
pr_alert("page has been migrated, last migrate reason: %s\n",
migrate_reason_names[page_ext->last_migrate_reason]);
+ }
}
static ssize_t
@@ -381,7 +423,7 @@ read_page_owner(struct file *file, char __user *buf, size_t count, loff_t *ppos)
* Access to page_ext->handle isn't synchronous so we should
* be careful to access it.
*/
- handle = READ_ONCE(page_ext->handle);
+ handle = READ_ONCE(page_ext->handles[PAGE_OWNER_HANDLE_ALLOC]);
if (!handle)
continue;
diff --git a/mm/vmstat.c b/mm/vmstat.c
index 63ef65f..4ff0135 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -1073,6 +1073,9 @@ static void pagetypeinfo_showmixedcount_print(struct seq_file *m,
if (!test_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags))
continue;
+ if (!test_bit(PAGE_EXT_OWNER_FREE, &page_ext->flags))
+ continue;
+
page_mt = gfpflags_to_migratetype(page_ext->gfp_mask);
if (pageblock_mt != page_mt) {
if (is_migrate_cma(pageblock_mt))
--
2.9.0.37.g6d523a3
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] mm/page_owner: track page free call chain
2016-07-02 16:16 ` Sergey Senozhatsky
@ 2016-07-03 5:59 ` Sergey Senozhatsky
-1 siblings, 0 replies; 24+ messages in thread
From: Sergey Senozhatsky @ 2016-07-03 5:59 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Joonsoo Kim, Andrew Morton, Vlastimil Babka, linux-mm,
linux-kernel, Sergey Senozhatsky
On (07/03/16 01:16), Sergey Senozhatsky wrote:
[..]
> +#ifdef CONFIG_PAGE_OWNER_TRACK_FREE
> +void __page_owner_free_pages(struct page *page, unsigned int order)
> +{
> + int i;
> + depot_stack_handle_t handle;
> + struct page_ext *page_ext = lookup_page_ext(page);
> +
> + if (unlikely(!page_ext))
> + return;
> +
> + handle = save_stack(0);
> + page_ext->handles[PAGE_OWNER_HANDLE_FREE] = handle;
> + __set_bit(PAGE_EXT_OWNER_FREE, &page_ext->flags);
> +
> + for (i = 1; i < (1 << order); i++) {
> + struct page_ext *ext = lookup_page_ext(page + 1);
> +
> + if (unlikely(!ext))
> + continue;
> + ext->handles[PAGE_OWNER_HANDLE_FREE] = handle;
> + __set_bit(PAGE_EXT_OWNER_FREE, &ext->flags);
> + }
> +}
I think this one is missing __clear_bit(PAGE_EXT_OWNER_ALLOC).
---
mm/page_owner.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/mm/page_owner.c b/mm/page_owner.c
index 5a108d6..699922c 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -154,6 +154,7 @@ void __page_owner_free_pages(struct page *page, unsigned int order)
handle = save_stack(0);
page_ext->handles[PAGE_OWNER_HANDLE_FREE] = handle;
__set_bit(PAGE_EXT_OWNER_FREE, &page_ext->flags);
+ __clear_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags);
for (i = 1; i < (1 << order); i++) {
struct page_ext *ext = lookup_page_ext(page + 1);
@@ -162,6 +163,7 @@ void __page_owner_free_pages(struct page *page, unsigned int order)
continue;
ext->handles[PAGE_OWNER_HANDLE_FREE] = handle;
__set_bit(PAGE_EXT_OWNER_FREE, &ext->flags);
+ __clear_bit(PAGE_EXT_OWNER_ALLOC, &ext->flags);
}
}
#else
--
2.9.0.rc1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] mm/page_owner: track page free call chain
@ 2016-07-03 5:59 ` Sergey Senozhatsky
0 siblings, 0 replies; 24+ messages in thread
From: Sergey Senozhatsky @ 2016-07-03 5:59 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Joonsoo Kim, Andrew Morton, Vlastimil Babka, linux-mm,
linux-kernel, Sergey Senozhatsky
On (07/03/16 01:16), Sergey Senozhatsky wrote:
[..]
> +#ifdef CONFIG_PAGE_OWNER_TRACK_FREE
> +void __page_owner_free_pages(struct page *page, unsigned int order)
> +{
> + int i;
> + depot_stack_handle_t handle;
> + struct page_ext *page_ext = lookup_page_ext(page);
> +
> + if (unlikely(!page_ext))
> + return;
> +
> + handle = save_stack(0);
> + page_ext->handles[PAGE_OWNER_HANDLE_FREE] = handle;
> + __set_bit(PAGE_EXT_OWNER_FREE, &page_ext->flags);
> +
> + for (i = 1; i < (1 << order); i++) {
> + struct page_ext *ext = lookup_page_ext(page + 1);
> +
> + if (unlikely(!ext))
> + continue;
> + ext->handles[PAGE_OWNER_HANDLE_FREE] = handle;
> + __set_bit(PAGE_EXT_OWNER_FREE, &ext->flags);
> + }
> +}
I think this one is missing __clear_bit(PAGE_EXT_OWNER_ALLOC).
---
mm/page_owner.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/mm/page_owner.c b/mm/page_owner.c
index 5a108d6..699922c 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -154,6 +154,7 @@ void __page_owner_free_pages(struct page *page, unsigned int order)
handle = save_stack(0);
page_ext->handles[PAGE_OWNER_HANDLE_FREE] = handle;
__set_bit(PAGE_EXT_OWNER_FREE, &page_ext->flags);
+ __clear_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags);
for (i = 1; i < (1 << order); i++) {
struct page_ext *ext = lookup_page_ext(page + 1);
@@ -162,6 +163,7 @@ void __page_owner_free_pages(struct page *page, unsigned int order)
continue;
ext->handles[PAGE_OWNER_HANDLE_FREE] = handle;
__set_bit(PAGE_EXT_OWNER_FREE, &ext->flags);
+ __clear_bit(PAGE_EXT_OWNER_ALLOC, &ext->flags);
}
}
#else
--
2.9.0.rc1
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] mm/page_owner: track page free call chain
2016-07-02 16:16 ` Sergey Senozhatsky
@ 2016-07-04 4:57 ` Joonsoo Kim
-1 siblings, 0 replies; 24+ messages in thread
From: Joonsoo Kim @ 2016-07-04 4:57 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Andrew Morton, Vlastimil Babka, linux-mm, linux-kernel,
Sergey Senozhatsky
On Sun, Jul 03, 2016 at 01:16:56AM +0900, Sergey Senozhatsky wrote:
> Introduce PAGE_OWNER_TRACK_FREE config option to extend page owner with
> free_pages() tracking functionality. This adds to the dump_page_owner()
> output an additional backtrace, that tells us what path has freed the
> page.
Hmm... Do you have other ideas to use this feature? Following example is
just to detect use-after-free and we have other good tools for it
(KASAN or DEBUG_PAGEALLOC) so I'm not sure whether it's useful or not.
And, if there is a good usage, I think that we don't need to add new
Kconfig, PAGE_OWNER_TRACK_FREE. It only takes 8 bytes per page and
it's not that big in this debugging context.
Thanks.
>
> Aa a trivial example, let's assume that do_some_foo() has an error - extra
> put_page() on error return path, and the function is also getting preempted,
> letting some other task to allocate the same page, which is then mistakenly
> getting freed once again by do_some_foo().
>
> CPUA CPUB
>
> void do_some_foo(void)
> {
> page = alloc_page();
> if (error) {
> put_page(page);
> goto out;
> }
> ...
> out:
> <<preempted>>
> void do_some_bar()
> {
> page = alloc_page();
> ...
> <<preempted>>
> ...
> put_page(page);
> }
> <<use freed page>>
> put_page(page);
> }
>
>
> Backtrace:
>
>
> BUG: Bad page state in process cc1 pfn:bae1d
> page:ffffea0002eb8740 count:-1 mapcount:0 mapping: (null) index:0x0
> flags: 0x4000000000000000()
> page dumped because: nonzero _count
> page allocated via order 0, migratetype Unmovable, gfp_mask 0x2000200(GFP_NOWAIT|__GFP_NOWARN)
> [<ffffffff8101bc9c>] save_stack_trace+0x26/0x41
> [<ffffffff81110fe4>] save_stack+0x46/0xc3
> [<ffffffff81111481>] __page_owner_alloc_pages+0x24/0x41
> [<ffffffff810c9867>] post_alloc_hook+0x1e/0x20
> [<ffffffff810ca63d>] get_page_from_freelist+0x4fd/0x756
> [<ffffffff810cadea>] __alloc_pages_nodemask+0xe7/0xbcf
> [<ffffffff810cb8e4>] __get_free_pages+0x12/0x40
> [<ffffffff810e6b64>] __tlb_remove_page_size.part.12+0x37/0x78
> [<ffffffff810e6d9b>] __tlb_remove_page_size+0x21/0x23
> [<ffffffff810e7ff2>] unmap_page_range+0x63a/0x75b
> [<ffffffff810e81cf>] unmap_single_vma+0xbc/0xc6
> [<ffffffff810e82d2>] unmap_vmas+0x35/0x44
> [<ffffffff810ee6f4>] exit_mmap+0x5a/0xec
> [<ffffffff810385b4>] mmput+0x4a/0xdc
> [<ffffffff8103dff7>] do_exit+0x398/0x8de
> [<ffffffff8103e5ae>] do_group_exit+0x45/0xb0
> page freed via order 0, migratetype Unmovable, gfp_mask 0x2000200(GFP_NOWAIT|__GFP_NOWARN)
> [<ffffffff8101bc9c>] save_stack_trace+0x26/0x41
> [<ffffffff81110fe4>] save_stack+0x46/0xc3
> [<ffffffff81111411>] __page_owner_free_pages+0x25/0x71
> [<ffffffff810c9f0a>] free_hot_cold_page+0x1d6/0x1ea
> [<ffffffff810d03e1>] __put_page+0x37/0x3a
> [<ffffffff8115b8da>] do_some_foo()+0x8a/0x8e
> ...
> Modules linked in: ....
> CPU: 3 PID: 1274 Comm: cc1 Not tainted 4.7.0-rc5-next-20160701-dbg-00009-ge01494f-dirty #535
> 0000000000000000 ffff8800aeea3c18 ffffffff811e67ca ffffea0002eb8740
> ffffffff8175675e ffff8800aeea3c40 ffffffff810c87f5 0000000000000000
> ffffffff81880b40 ffff880137d98438 ffff8800aeea3c50 ffffffff810c88d5
> Call Trace:
> [<ffffffff811e67ca>] dump_stack+0x68/0x92
> [<ffffffff810c87f5>] bad_page+0xf8/0x11e
> [<ffffffff810c88d5>] check_new_page_bad+0x63/0x65
> [<ffffffff810ca36a>] get_page_from_freelist+0x22a/0x756
> [<ffffffff810cadea>] __alloc_pages_nodemask+0xe7/0xbcf
> [<ffffffff81073a43>] ? trace_hardirqs_on_caller+0x16d/0x189
> [<ffffffff810ede8d>] ? vma_merge+0x159/0x249
> [<ffffffff81074aa0>] ? __lock_acquire+0x2ac/0x15c7
> [<ffffffff81034ace>] pte_alloc_one+0x1b/0x67
> [<ffffffff810e922b>] __pte_alloc+0x19/0xa6
> [<ffffffff810eb09f>] handle_mm_fault+0x409/0xc59
> [<ffffffff810309f6>] __do_page_fault+0x1d8/0x3ac
> [<ffffffff81030bf7>] do_page_fault+0xc/0xe
> [<ffffffff814a84af>] page_fault+0x1f/0x30
>
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> ---
> include/linux/page_ext.h | 13 ++++++-
> lib/Kconfig.debug | 10 +++++
> mm/page_owner.c | 98 ++++++++++++++++++++++++++++++++++--------------
> mm/vmstat.c | 3 ++
> 4 files changed, 95 insertions(+), 29 deletions(-)
>
> diff --git a/include/linux/page_ext.h b/include/linux/page_ext.h
> index 66ba2bb..90bd44a 100644
> --- a/include/linux/page_ext.h
> +++ b/include/linux/page_ext.h
> @@ -27,12 +27,23 @@ enum page_ext_flags {
> PAGE_EXT_DEBUG_POISON, /* Page is poisoned */
> PAGE_EXT_DEBUG_GUARD,
> PAGE_EXT_OWNER_ALLOC,
> + PAGE_EXT_OWNER_FREE,
> #if defined(CONFIG_IDLE_PAGE_TRACKING) && !defined(CONFIG_64BIT)
> PAGE_EXT_YOUNG,
> PAGE_EXT_IDLE,
> #endif
> };
>
> +#ifdef CONFIG_PAGE_OWNER
> +enum page_owner_handles {
> + PAGE_OWNER_HANDLE_ALLOC,
> +#ifdef CONFIG_PAGE_OWNER_TRACK_FREE
> + PAGE_OWNER_HANDLE_FREE,
> +#endif
> + PAGE_OWNER_HANDLE_MAX
> +};
> +#endif
> +
> /*
> * Page Extension can be considered as an extended mem_map.
> * A page_ext page is associated with every page descriptor. The
> @@ -46,7 +57,7 @@ struct page_ext {
> unsigned int order;
> gfp_t gfp_mask;
> int last_migrate_reason;
> - depot_stack_handle_t handle;
> + depot_stack_handle_t handles[PAGE_OWNER_HANDLE_MAX];
> #endif
> };
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 0f99819..20ac03b 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -260,6 +260,16 @@ config PAGE_OWNER
>
> If unsure, say N.
>
> +config PAGE_OWNER_TRACK_FREE
> + bool "Track page free call chains"
> + depends on PAGE_OWNER
> + help
> + Page owner keeps track of what call chain is the owner of a page (has
> + allocated the page), this option enables an additional functionality
> + to also track what call chain has freed the page.
> +
> + If unsure, say N.
> +
> config DEBUG_FS
> bool "Debug Filesystem"
> select SRCU
> diff --git a/mm/page_owner.c b/mm/page_owner.c
> index 4acccb7..5a108d6 100644
> --- a/mm/page_owner.c
> +++ b/mm/page_owner.c
> @@ -13,6 +13,13 @@
>
> #define PAGE_OWNER_STACK_DEPTH (16)
>
> +static const char *page_owner_handles_names[PAGE_OWNER_HANDLE_MAX] = {
> + "page allocated",
> +#ifdef CONFIG_PAGE_OWNER_TRACK_FREE
> + "page freed",
> +#endif
> +};
> +
> static bool page_owner_disabled = true;
> DEFINE_STATIC_KEY_FALSE(page_owner_inited);
>
> @@ -85,19 +92,6 @@ struct page_ext_operations page_owner_ops = {
> .init = init_page_owner,
> };
>
> -void __page_owner_free_pages(struct page *page, unsigned int order)
> -{
> - int i;
> - struct page_ext *page_ext;
> -
> - for (i = 0; i < (1 << order); i++) {
> - page_ext = lookup_page_ext(page + i);
> - if (unlikely(!page_ext))
> - continue;
> - __clear_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags);
> - }
> -}
> -
> static inline bool check_recursive_alloc(struct stack_trace *trace,
> unsigned long ip)
> {
> @@ -147,6 +141,45 @@ static noinline depot_stack_handle_t save_stack(gfp_t flags)
> return handle;
> }
>
> +#ifdef CONFIG_PAGE_OWNER_TRACK_FREE
> +void __page_owner_free_pages(struct page *page, unsigned int order)
> +{
> + int i;
> + depot_stack_handle_t handle;
> + struct page_ext *page_ext = lookup_page_ext(page);
> +
> + if (unlikely(!page_ext))
> + return;
> +
> + handle = save_stack(0);
> + page_ext->handles[PAGE_OWNER_HANDLE_FREE] = handle;
> + __set_bit(PAGE_EXT_OWNER_FREE, &page_ext->flags);
> +
> + for (i = 1; i < (1 << order); i++) {
> + struct page_ext *ext = lookup_page_ext(page + 1);
> +
> + if (unlikely(!ext))
> + continue;
> + ext->handles[PAGE_OWNER_HANDLE_FREE] = handle;
> + __set_bit(PAGE_EXT_OWNER_FREE, &ext->flags);
> + }
> +}
> +#else
> +void __page_owner_free_pages(struct page *page, unsigned int order)
> +{
> + int i;
> + struct page_ext *page_ext;
> +
> + for (i = 0; i < (1 << order); i++) {
> + page_ext = lookup_page_ext(page + i);
> +
> + if (unlikely(!page_ext))
> + continue;
> + __clear_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags);
> + }
> +}
> +#endif
> +
> noinline void __page_owner_alloc_pages(struct page *page, unsigned int order,
> gfp_t gfp_mask)
> {
> @@ -155,7 +188,7 @@ noinline void __page_owner_alloc_pages(struct page *page, unsigned int order,
> if (unlikely(!page_ext))
> return;
>
> - page_ext->handle = save_stack(gfp_mask);
> + page_ext->handles[PAGE_OWNER_HANDLE_ALLOC] = save_stack(gfp_mask);
> page_ext->order = order;
> page_ext->gfp_mask = gfp_mask;
> page_ext->last_migrate_reason = -1;
> @@ -189,6 +222,7 @@ void __copy_page_owner(struct page *oldpage, struct page *newpage)
> {
> struct page_ext *old_ext = lookup_page_ext(oldpage);
> struct page_ext *new_ext = lookup_page_ext(newpage);
> + int i;
>
> if (unlikely(!old_ext || !new_ext))
> return;
> @@ -196,7 +230,9 @@ void __copy_page_owner(struct page *oldpage, struct page *newpage)
> new_ext->order = old_ext->order;
> new_ext->gfp_mask = old_ext->gfp_mask;
> new_ext->last_migrate_reason = old_ext->last_migrate_reason;
> - new_ext->handle = old_ext->handle;
> +
> + for (i = 0; i < PAGE_OWNER_HANDLE_MAX; i++)
> + new_ext->handles[i] = old_ext->handles[i];
>
> /*
> * We don't clear the bit on the oldpage as it's going to be freed
> @@ -292,7 +328,7 @@ void __dump_page_owner(struct page *page)
> };
> depot_stack_handle_t handle;
> gfp_t gfp_mask;
> - int mt;
> + int mt, i;
>
> if (unlikely(!page_ext)) {
> pr_alert("There is not page extension available.\n");
> @@ -301,25 +337,31 @@ void __dump_page_owner(struct page *page)
> gfp_mask = page_ext->gfp_mask;
> mt = gfpflags_to_migratetype(gfp_mask);
>
> - if (!test_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags)) {
> + if (!test_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags) &&
> + !test_bit(PAGE_EXT_OWNER_FREE, &page_ext->flags)) {
> pr_alert("page_owner info is not active (free page?)\n");
> return;
> }
>
> - handle = READ_ONCE(page_ext->handle);
> - if (!handle) {
> - pr_alert("page_owner info is not active (free page?)\n");
> - return;
> - }
> + for (i = 0; i < PAGE_OWNER_HANDLE_MAX; i++) {
> + handle = READ_ONCE(page_ext->handles[i]);
> + if (!handle) {
> + pr_alert("page_owner info is not active for `%s'\n",
> + page_owner_handles_names[i]);
> + continue;
> + }
>
> - depot_fetch_stack(handle, &trace);
> - pr_alert("page allocated via order %u, migratetype %s, gfp_mask %#x(%pGg)\n",
> - page_ext->order, migratetype_names[mt], gfp_mask, &gfp_mask);
> - print_stack_trace(&trace, 0);
> + depot_fetch_stack(handle, &trace);
> + pr_alert("%s via order %u, migratetype %s, gfp_mask %#x(%pGg)\n",
> + page_owner_handles_names[i], page_ext->order,
> + migratetype_names[mt], gfp_mask, &gfp_mask);
> + print_stack_trace(&trace, 0);
>
> - if (page_ext->last_migrate_reason != -1)
> + if (page_ext->last_migrate_reason == -1)
> + continue;
> pr_alert("page has been migrated, last migrate reason: %s\n",
> migrate_reason_names[page_ext->last_migrate_reason]);
> + }
> }
>
> static ssize_t
> @@ -381,7 +423,7 @@ read_page_owner(struct file *file, char __user *buf, size_t count, loff_t *ppos)
> * Access to page_ext->handle isn't synchronous so we should
> * be careful to access it.
> */
> - handle = READ_ONCE(page_ext->handle);
> + handle = READ_ONCE(page_ext->handles[PAGE_OWNER_HANDLE_ALLOC]);
> if (!handle)
> continue;
>
> diff --git a/mm/vmstat.c b/mm/vmstat.c
> index 63ef65f..4ff0135 100644
> --- a/mm/vmstat.c
> +++ b/mm/vmstat.c
> @@ -1073,6 +1073,9 @@ static void pagetypeinfo_showmixedcount_print(struct seq_file *m,
> if (!test_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags))
> continue;
>
> + if (!test_bit(PAGE_EXT_OWNER_FREE, &page_ext->flags))
> + continue;
> +
> page_mt = gfpflags_to_migratetype(page_ext->gfp_mask);
> if (pageblock_mt != page_mt) {
> if (is_migrate_cma(pageblock_mt))
> --
> 2.9.0.37.g6d523a3
>
> --
> 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/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] mm/page_owner: track page free call chain
@ 2016-07-04 4:57 ` Joonsoo Kim
0 siblings, 0 replies; 24+ messages in thread
From: Joonsoo Kim @ 2016-07-04 4:57 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Andrew Morton, Vlastimil Babka, linux-mm, linux-kernel,
Sergey Senozhatsky
On Sun, Jul 03, 2016 at 01:16:56AM +0900, Sergey Senozhatsky wrote:
> Introduce PAGE_OWNER_TRACK_FREE config option to extend page owner with
> free_pages() tracking functionality. This adds to the dump_page_owner()
> output an additional backtrace, that tells us what path has freed the
> page.
Hmm... Do you have other ideas to use this feature? Following example is
just to detect use-after-free and we have other good tools for it
(KASAN or DEBUG_PAGEALLOC) so I'm not sure whether it's useful or not.
And, if there is a good usage, I think that we don't need to add new
Kconfig, PAGE_OWNER_TRACK_FREE. It only takes 8 bytes per page and
it's not that big in this debugging context.
Thanks.
>
> Aa a trivial example, let's assume that do_some_foo() has an error - extra
> put_page() on error return path, and the function is also getting preempted,
> letting some other task to allocate the same page, which is then mistakenly
> getting freed once again by do_some_foo().
>
> CPUA CPUB
>
> void do_some_foo(void)
> {
> page = alloc_page();
> if (error) {
> put_page(page);
> goto out;
> }
> ...
> out:
> <<preempted>>
> void do_some_bar()
> {
> page = alloc_page();
> ...
> <<preempted>>
> ...
> put_page(page);
> }
> <<use freed page>>
> put_page(page);
> }
>
>
> Backtrace:
>
>
> BUG: Bad page state in process cc1 pfn:bae1d
> page:ffffea0002eb8740 count:-1 mapcount:0 mapping: (null) index:0x0
> flags: 0x4000000000000000()
> page dumped because: nonzero _count
> page allocated via order 0, migratetype Unmovable, gfp_mask 0x2000200(GFP_NOWAIT|__GFP_NOWARN)
> [<ffffffff8101bc9c>] save_stack_trace+0x26/0x41
> [<ffffffff81110fe4>] save_stack+0x46/0xc3
> [<ffffffff81111481>] __page_owner_alloc_pages+0x24/0x41
> [<ffffffff810c9867>] post_alloc_hook+0x1e/0x20
> [<ffffffff810ca63d>] get_page_from_freelist+0x4fd/0x756
> [<ffffffff810cadea>] __alloc_pages_nodemask+0xe7/0xbcf
> [<ffffffff810cb8e4>] __get_free_pages+0x12/0x40
> [<ffffffff810e6b64>] __tlb_remove_page_size.part.12+0x37/0x78
> [<ffffffff810e6d9b>] __tlb_remove_page_size+0x21/0x23
> [<ffffffff810e7ff2>] unmap_page_range+0x63a/0x75b
> [<ffffffff810e81cf>] unmap_single_vma+0xbc/0xc6
> [<ffffffff810e82d2>] unmap_vmas+0x35/0x44
> [<ffffffff810ee6f4>] exit_mmap+0x5a/0xec
> [<ffffffff810385b4>] mmput+0x4a/0xdc
> [<ffffffff8103dff7>] do_exit+0x398/0x8de
> [<ffffffff8103e5ae>] do_group_exit+0x45/0xb0
> page freed via order 0, migratetype Unmovable, gfp_mask 0x2000200(GFP_NOWAIT|__GFP_NOWARN)
> [<ffffffff8101bc9c>] save_stack_trace+0x26/0x41
> [<ffffffff81110fe4>] save_stack+0x46/0xc3
> [<ffffffff81111411>] __page_owner_free_pages+0x25/0x71
> [<ffffffff810c9f0a>] free_hot_cold_page+0x1d6/0x1ea
> [<ffffffff810d03e1>] __put_page+0x37/0x3a
> [<ffffffff8115b8da>] do_some_foo()+0x8a/0x8e
> ...
> Modules linked in: ....
> CPU: 3 PID: 1274 Comm: cc1 Not tainted 4.7.0-rc5-next-20160701-dbg-00009-ge01494f-dirty #535
> 0000000000000000 ffff8800aeea3c18 ffffffff811e67ca ffffea0002eb8740
> ffffffff8175675e ffff8800aeea3c40 ffffffff810c87f5 0000000000000000
> ffffffff81880b40 ffff880137d98438 ffff8800aeea3c50 ffffffff810c88d5
> Call Trace:
> [<ffffffff811e67ca>] dump_stack+0x68/0x92
> [<ffffffff810c87f5>] bad_page+0xf8/0x11e
> [<ffffffff810c88d5>] check_new_page_bad+0x63/0x65
> [<ffffffff810ca36a>] get_page_from_freelist+0x22a/0x756
> [<ffffffff810cadea>] __alloc_pages_nodemask+0xe7/0xbcf
> [<ffffffff81073a43>] ? trace_hardirqs_on_caller+0x16d/0x189
> [<ffffffff810ede8d>] ? vma_merge+0x159/0x249
> [<ffffffff81074aa0>] ? __lock_acquire+0x2ac/0x15c7
> [<ffffffff81034ace>] pte_alloc_one+0x1b/0x67
> [<ffffffff810e922b>] __pte_alloc+0x19/0xa6
> [<ffffffff810eb09f>] handle_mm_fault+0x409/0xc59
> [<ffffffff810309f6>] __do_page_fault+0x1d8/0x3ac
> [<ffffffff81030bf7>] do_page_fault+0xc/0xe
> [<ffffffff814a84af>] page_fault+0x1f/0x30
>
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> ---
> include/linux/page_ext.h | 13 ++++++-
> lib/Kconfig.debug | 10 +++++
> mm/page_owner.c | 98 ++++++++++++++++++++++++++++++++++--------------
> mm/vmstat.c | 3 ++
> 4 files changed, 95 insertions(+), 29 deletions(-)
>
> diff --git a/include/linux/page_ext.h b/include/linux/page_ext.h
> index 66ba2bb..90bd44a 100644
> --- a/include/linux/page_ext.h
> +++ b/include/linux/page_ext.h
> @@ -27,12 +27,23 @@ enum page_ext_flags {
> PAGE_EXT_DEBUG_POISON, /* Page is poisoned */
> PAGE_EXT_DEBUG_GUARD,
> PAGE_EXT_OWNER_ALLOC,
> + PAGE_EXT_OWNER_FREE,
> #if defined(CONFIG_IDLE_PAGE_TRACKING) && !defined(CONFIG_64BIT)
> PAGE_EXT_YOUNG,
> PAGE_EXT_IDLE,
> #endif
> };
>
> +#ifdef CONFIG_PAGE_OWNER
> +enum page_owner_handles {
> + PAGE_OWNER_HANDLE_ALLOC,
> +#ifdef CONFIG_PAGE_OWNER_TRACK_FREE
> + PAGE_OWNER_HANDLE_FREE,
> +#endif
> + PAGE_OWNER_HANDLE_MAX
> +};
> +#endif
> +
> /*
> * Page Extension can be considered as an extended mem_map.
> * A page_ext page is associated with every page descriptor. The
> @@ -46,7 +57,7 @@ struct page_ext {
> unsigned int order;
> gfp_t gfp_mask;
> int last_migrate_reason;
> - depot_stack_handle_t handle;
> + depot_stack_handle_t handles[PAGE_OWNER_HANDLE_MAX];
> #endif
> };
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 0f99819..20ac03b 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -260,6 +260,16 @@ config PAGE_OWNER
>
> If unsure, say N.
>
> +config PAGE_OWNER_TRACK_FREE
> + bool "Track page free call chains"
> + depends on PAGE_OWNER
> + help
> + Page owner keeps track of what call chain is the owner of a page (has
> + allocated the page), this option enables an additional functionality
> + to also track what call chain has freed the page.
> +
> + If unsure, say N.
> +
> config DEBUG_FS
> bool "Debug Filesystem"
> select SRCU
> diff --git a/mm/page_owner.c b/mm/page_owner.c
> index 4acccb7..5a108d6 100644
> --- a/mm/page_owner.c
> +++ b/mm/page_owner.c
> @@ -13,6 +13,13 @@
>
> #define PAGE_OWNER_STACK_DEPTH (16)
>
> +static const char *page_owner_handles_names[PAGE_OWNER_HANDLE_MAX] = {
> + "page allocated",
> +#ifdef CONFIG_PAGE_OWNER_TRACK_FREE
> + "page freed",
> +#endif
> +};
> +
> static bool page_owner_disabled = true;
> DEFINE_STATIC_KEY_FALSE(page_owner_inited);
>
> @@ -85,19 +92,6 @@ struct page_ext_operations page_owner_ops = {
> .init = init_page_owner,
> };
>
> -void __page_owner_free_pages(struct page *page, unsigned int order)
> -{
> - int i;
> - struct page_ext *page_ext;
> -
> - for (i = 0; i < (1 << order); i++) {
> - page_ext = lookup_page_ext(page + i);
> - if (unlikely(!page_ext))
> - continue;
> - __clear_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags);
> - }
> -}
> -
> static inline bool check_recursive_alloc(struct stack_trace *trace,
> unsigned long ip)
> {
> @@ -147,6 +141,45 @@ static noinline depot_stack_handle_t save_stack(gfp_t flags)
> return handle;
> }
>
> +#ifdef CONFIG_PAGE_OWNER_TRACK_FREE
> +void __page_owner_free_pages(struct page *page, unsigned int order)
> +{
> + int i;
> + depot_stack_handle_t handle;
> + struct page_ext *page_ext = lookup_page_ext(page);
> +
> + if (unlikely(!page_ext))
> + return;
> +
> + handle = save_stack(0);
> + page_ext->handles[PAGE_OWNER_HANDLE_FREE] = handle;
> + __set_bit(PAGE_EXT_OWNER_FREE, &page_ext->flags);
> +
> + for (i = 1; i < (1 << order); i++) {
> + struct page_ext *ext = lookup_page_ext(page + 1);
> +
> + if (unlikely(!ext))
> + continue;
> + ext->handles[PAGE_OWNER_HANDLE_FREE] = handle;
> + __set_bit(PAGE_EXT_OWNER_FREE, &ext->flags);
> + }
> +}
> +#else
> +void __page_owner_free_pages(struct page *page, unsigned int order)
> +{
> + int i;
> + struct page_ext *page_ext;
> +
> + for (i = 0; i < (1 << order); i++) {
> + page_ext = lookup_page_ext(page + i);
> +
> + if (unlikely(!page_ext))
> + continue;
> + __clear_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags);
> + }
> +}
> +#endif
> +
> noinline void __page_owner_alloc_pages(struct page *page, unsigned int order,
> gfp_t gfp_mask)
> {
> @@ -155,7 +188,7 @@ noinline void __page_owner_alloc_pages(struct page *page, unsigned int order,
> if (unlikely(!page_ext))
> return;
>
> - page_ext->handle = save_stack(gfp_mask);
> + page_ext->handles[PAGE_OWNER_HANDLE_ALLOC] = save_stack(gfp_mask);
> page_ext->order = order;
> page_ext->gfp_mask = gfp_mask;
> page_ext->last_migrate_reason = -1;
> @@ -189,6 +222,7 @@ void __copy_page_owner(struct page *oldpage, struct page *newpage)
> {
> struct page_ext *old_ext = lookup_page_ext(oldpage);
> struct page_ext *new_ext = lookup_page_ext(newpage);
> + int i;
>
> if (unlikely(!old_ext || !new_ext))
> return;
> @@ -196,7 +230,9 @@ void __copy_page_owner(struct page *oldpage, struct page *newpage)
> new_ext->order = old_ext->order;
> new_ext->gfp_mask = old_ext->gfp_mask;
> new_ext->last_migrate_reason = old_ext->last_migrate_reason;
> - new_ext->handle = old_ext->handle;
> +
> + for (i = 0; i < PAGE_OWNER_HANDLE_MAX; i++)
> + new_ext->handles[i] = old_ext->handles[i];
>
> /*
> * We don't clear the bit on the oldpage as it's going to be freed
> @@ -292,7 +328,7 @@ void __dump_page_owner(struct page *page)
> };
> depot_stack_handle_t handle;
> gfp_t gfp_mask;
> - int mt;
> + int mt, i;
>
> if (unlikely(!page_ext)) {
> pr_alert("There is not page extension available.\n");
> @@ -301,25 +337,31 @@ void __dump_page_owner(struct page *page)
> gfp_mask = page_ext->gfp_mask;
> mt = gfpflags_to_migratetype(gfp_mask);
>
> - if (!test_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags)) {
> + if (!test_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags) &&
> + !test_bit(PAGE_EXT_OWNER_FREE, &page_ext->flags)) {
> pr_alert("page_owner info is not active (free page?)\n");
> return;
> }
>
> - handle = READ_ONCE(page_ext->handle);
> - if (!handle) {
> - pr_alert("page_owner info is not active (free page?)\n");
> - return;
> - }
> + for (i = 0; i < PAGE_OWNER_HANDLE_MAX; i++) {
> + handle = READ_ONCE(page_ext->handles[i]);
> + if (!handle) {
> + pr_alert("page_owner info is not active for `%s'\n",
> + page_owner_handles_names[i]);
> + continue;
> + }
>
> - depot_fetch_stack(handle, &trace);
> - pr_alert("page allocated via order %u, migratetype %s, gfp_mask %#x(%pGg)\n",
> - page_ext->order, migratetype_names[mt], gfp_mask, &gfp_mask);
> - print_stack_trace(&trace, 0);
> + depot_fetch_stack(handle, &trace);
> + pr_alert("%s via order %u, migratetype %s, gfp_mask %#x(%pGg)\n",
> + page_owner_handles_names[i], page_ext->order,
> + migratetype_names[mt], gfp_mask, &gfp_mask);
> + print_stack_trace(&trace, 0);
>
> - if (page_ext->last_migrate_reason != -1)
> + if (page_ext->last_migrate_reason == -1)
> + continue;
> pr_alert("page has been migrated, last migrate reason: %s\n",
> migrate_reason_names[page_ext->last_migrate_reason]);
> + }
> }
>
> static ssize_t
> @@ -381,7 +423,7 @@ read_page_owner(struct file *file, char __user *buf, size_t count, loff_t *ppos)
> * Access to page_ext->handle isn't synchronous so we should
> * be careful to access it.
> */
> - handle = READ_ONCE(page_ext->handle);
> + handle = READ_ONCE(page_ext->handles[PAGE_OWNER_HANDLE_ALLOC]);
> if (!handle)
> continue;
>
> diff --git a/mm/vmstat.c b/mm/vmstat.c
> index 63ef65f..4ff0135 100644
> --- a/mm/vmstat.c
> +++ b/mm/vmstat.c
> @@ -1073,6 +1073,9 @@ static void pagetypeinfo_showmixedcount_print(struct seq_file *m,
> if (!test_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags))
> continue;
>
> + if (!test_bit(PAGE_EXT_OWNER_FREE, &page_ext->flags))
> + continue;
> +
> page_mt = gfpflags_to_migratetype(page_ext->gfp_mask);
> if (pageblock_mt != page_mt) {
> if (is_migrate_cma(pageblock_mt))
> --
> 2.9.0.37.g6d523a3
>
> --
> 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/ .
> 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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] mm/page_owner: track page free call chain
2016-07-04 4:57 ` Joonsoo Kim
@ 2016-07-04 5:07 ` Sergey Senozhatsky
-1 siblings, 0 replies; 24+ messages in thread
From: Sergey Senozhatsky @ 2016-07-04 5:07 UTC (permalink / raw)
To: Joonsoo Kim
Cc: Sergey Senozhatsky, Andrew Morton, Vlastimil Babka, linux-mm,
linux-kernel, Sergey Senozhatsky
Hello,
On (07/04/16 13:57), Joonsoo Kim wrote:
> On Sun, Jul 03, 2016 at 01:16:56AM +0900, Sergey Senozhatsky wrote:
> > Introduce PAGE_OWNER_TRACK_FREE config option to extend page owner with
> > free_pages() tracking functionality. This adds to the dump_page_owner()
> > output an additional backtrace, that tells us what path has freed the
> > page.
>
> Hmm... Do you have other ideas to use this feature? Following example is
> just to detect use-after-free and we have other good tools for it
> (KASAN or DEBUG_PAGEALLOC) so I'm not sure whether it's useful or not.
there is no kasan for ARM32, for example (apart from the fact that
it's really hard to use kasan sometimes due to its cpu cycles and
memory requirements).
educate me, will DEBUG_PAGEALLOC tell us what path has triggered the
extra put_page()? hm... does ARM32 provide ARCH_SUPPORTS_DEBUG_PAGEALLOC?
-ss
>
> And, if there is a good usage, I think that we don't need to add new
> Kconfig, PAGE_OWNER_TRACK_FREE. It only takes 8 bytes per page and
> it's not that big in this debugging context.
>
> Thanks.
>
> >
> > Aa a trivial example, let's assume that do_some_foo() has an error - extra
> > put_page() on error return path, and the function is also getting preempted,
> > letting some other task to allocate the same page, which is then mistakenly
> > getting freed once again by do_some_foo().
> >
> > CPUA CPUB
> >
> > void do_some_foo(void)
> > {
> > page = alloc_page();
> > if (error) {
> > put_page(page);
> > goto out;
> > }
> > ...
> > out:
> > <<preempted>>
> > void do_some_bar()
> > {
> > page = alloc_page();
> > ...
> > <<preempted>>
> > ...
> > put_page(page);
> > }
> > <<use freed page>>
> > put_page(page);
> > }
> >
> >
> > Backtrace:
> >
> >
> > BUG: Bad page state in process cc1 pfn:bae1d
> > page:ffffea0002eb8740 count:-1 mapcount:0 mapping: (null) index:0x0
> > flags: 0x4000000000000000()
> > page dumped because: nonzero _count
> > page allocated via order 0, migratetype Unmovable, gfp_mask 0x2000200(GFP_NOWAIT|__GFP_NOWARN)
> > [<ffffffff8101bc9c>] save_stack_trace+0x26/0x41
> > [<ffffffff81110fe4>] save_stack+0x46/0xc3
> > [<ffffffff81111481>] __page_owner_alloc_pages+0x24/0x41
> > [<ffffffff810c9867>] post_alloc_hook+0x1e/0x20
> > [<ffffffff810ca63d>] get_page_from_freelist+0x4fd/0x756
> > [<ffffffff810cadea>] __alloc_pages_nodemask+0xe7/0xbcf
> > [<ffffffff810cb8e4>] __get_free_pages+0x12/0x40
> > [<ffffffff810e6b64>] __tlb_remove_page_size.part.12+0x37/0x78
> > [<ffffffff810e6d9b>] __tlb_remove_page_size+0x21/0x23
> > [<ffffffff810e7ff2>] unmap_page_range+0x63a/0x75b
> > [<ffffffff810e81cf>] unmap_single_vma+0xbc/0xc6
> > [<ffffffff810e82d2>] unmap_vmas+0x35/0x44
> > [<ffffffff810ee6f4>] exit_mmap+0x5a/0xec
> > [<ffffffff810385b4>] mmput+0x4a/0xdc
> > [<ffffffff8103dff7>] do_exit+0x398/0x8de
> > [<ffffffff8103e5ae>] do_group_exit+0x45/0xb0
> > page freed via order 0, migratetype Unmovable, gfp_mask 0x2000200(GFP_NOWAIT|__GFP_NOWARN)
> > [<ffffffff8101bc9c>] save_stack_trace+0x26/0x41
> > [<ffffffff81110fe4>] save_stack+0x46/0xc3
> > [<ffffffff81111411>] __page_owner_free_pages+0x25/0x71
> > [<ffffffff810c9f0a>] free_hot_cold_page+0x1d6/0x1ea
> > [<ffffffff810d03e1>] __put_page+0x37/0x3a
> > [<ffffffff8115b8da>] do_some_foo()+0x8a/0x8e
> > ...
> > Modules linked in: ....
> > CPU: 3 PID: 1274 Comm: cc1 Not tainted 4.7.0-rc5-next-20160701-dbg-00009-ge01494f-dirty #535
> > 0000000000000000 ffff8800aeea3c18 ffffffff811e67ca ffffea0002eb8740
> > ffffffff8175675e ffff8800aeea3c40 ffffffff810c87f5 0000000000000000
> > ffffffff81880b40 ffff880137d98438 ffff8800aeea3c50 ffffffff810c88d5
> > Call Trace:
> > [<ffffffff811e67ca>] dump_stack+0x68/0x92
> > [<ffffffff810c87f5>] bad_page+0xf8/0x11e
> > [<ffffffff810c88d5>] check_new_page_bad+0x63/0x65
> > [<ffffffff810ca36a>] get_page_from_freelist+0x22a/0x756
> > [<ffffffff810cadea>] __alloc_pages_nodemask+0xe7/0xbcf
> > [<ffffffff81073a43>] ? trace_hardirqs_on_caller+0x16d/0x189
> > [<ffffffff810ede8d>] ? vma_merge+0x159/0x249
> > [<ffffffff81074aa0>] ? __lock_acquire+0x2ac/0x15c7
> > [<ffffffff81034ace>] pte_alloc_one+0x1b/0x67
> > [<ffffffff810e922b>] __pte_alloc+0x19/0xa6
> > [<ffffffff810eb09f>] handle_mm_fault+0x409/0xc59
> > [<ffffffff810309f6>] __do_page_fault+0x1d8/0x3ac
> > [<ffffffff81030bf7>] do_page_fault+0xc/0xe
> > [<ffffffff814a84af>] page_fault+0x1f/0x30
> >
> > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > ---
> > include/linux/page_ext.h | 13 ++++++-
> > lib/Kconfig.debug | 10 +++++
> > mm/page_owner.c | 98 ++++++++++++++++++++++++++++++++++--------------
> > mm/vmstat.c | 3 ++
> > 4 files changed, 95 insertions(+), 29 deletions(-)
> >
> > diff --git a/include/linux/page_ext.h b/include/linux/page_ext.h
> > index 66ba2bb..90bd44a 100644
> > --- a/include/linux/page_ext.h
> > +++ b/include/linux/page_ext.h
> > @@ -27,12 +27,23 @@ enum page_ext_flags {
> > PAGE_EXT_DEBUG_POISON, /* Page is poisoned */
> > PAGE_EXT_DEBUG_GUARD,
> > PAGE_EXT_OWNER_ALLOC,
> > + PAGE_EXT_OWNER_FREE,
> > #if defined(CONFIG_IDLE_PAGE_TRACKING) && !defined(CONFIG_64BIT)
> > PAGE_EXT_YOUNG,
> > PAGE_EXT_IDLE,
> > #endif
> > };
> >
> > +#ifdef CONFIG_PAGE_OWNER
> > +enum page_owner_handles {
> > + PAGE_OWNER_HANDLE_ALLOC,
> > +#ifdef CONFIG_PAGE_OWNER_TRACK_FREE
> > + PAGE_OWNER_HANDLE_FREE,
> > +#endif
> > + PAGE_OWNER_HANDLE_MAX
> > +};
> > +#endif
> > +
> > /*
> > * Page Extension can be considered as an extended mem_map.
> > * A page_ext page is associated with every page descriptor. The
> > @@ -46,7 +57,7 @@ struct page_ext {
> > unsigned int order;
> > gfp_t gfp_mask;
> > int last_migrate_reason;
> > - depot_stack_handle_t handle;
> > + depot_stack_handle_t handles[PAGE_OWNER_HANDLE_MAX];
> > #endif
> > };
> >
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index 0f99819..20ac03b 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -260,6 +260,16 @@ config PAGE_OWNER
> >
> > If unsure, say N.
> >
> > +config PAGE_OWNER_TRACK_FREE
> > + bool "Track page free call chains"
> > + depends on PAGE_OWNER
> > + help
> > + Page owner keeps track of what call chain is the owner of a page (has
> > + allocated the page), this option enables an additional functionality
> > + to also track what call chain has freed the page.
> > +
> > + If unsure, say N.
> > +
> > config DEBUG_FS
> > bool "Debug Filesystem"
> > select SRCU
> > diff --git a/mm/page_owner.c b/mm/page_owner.c
> > index 4acccb7..5a108d6 100644
> > --- a/mm/page_owner.c
> > +++ b/mm/page_owner.c
> > @@ -13,6 +13,13 @@
> >
> > #define PAGE_OWNER_STACK_DEPTH (16)
> >
> > +static const char *page_owner_handles_names[PAGE_OWNER_HANDLE_MAX] = {
> > + "page allocated",
> > +#ifdef CONFIG_PAGE_OWNER_TRACK_FREE
> > + "page freed",
> > +#endif
> > +};
> > +
> > static bool page_owner_disabled = true;
> > DEFINE_STATIC_KEY_FALSE(page_owner_inited);
> >
> > @@ -85,19 +92,6 @@ struct page_ext_operations page_owner_ops = {
> > .init = init_page_owner,
> > };
> >
> > -void __page_owner_free_pages(struct page *page, unsigned int order)
> > -{
> > - int i;
> > - struct page_ext *page_ext;
> > -
> > - for (i = 0; i < (1 << order); i++) {
> > - page_ext = lookup_page_ext(page + i);
> > - if (unlikely(!page_ext))
> > - continue;
> > - __clear_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags);
> > - }
> > -}
> > -
> > static inline bool check_recursive_alloc(struct stack_trace *trace,
> > unsigned long ip)
> > {
> > @@ -147,6 +141,45 @@ static noinline depot_stack_handle_t save_stack(gfp_t flags)
> > return handle;
> > }
> >
> > +#ifdef CONFIG_PAGE_OWNER_TRACK_FREE
> > +void __page_owner_free_pages(struct page *page, unsigned int order)
> > +{
> > + int i;
> > + depot_stack_handle_t handle;
> > + struct page_ext *page_ext = lookup_page_ext(page);
> > +
> > + if (unlikely(!page_ext))
> > + return;
> > +
> > + handle = save_stack(0);
> > + page_ext->handles[PAGE_OWNER_HANDLE_FREE] = handle;
> > + __set_bit(PAGE_EXT_OWNER_FREE, &page_ext->flags);
> > +
> > + for (i = 1; i < (1 << order); i++) {
> > + struct page_ext *ext = lookup_page_ext(page + 1);
> > +
> > + if (unlikely(!ext))
> > + continue;
> > + ext->handles[PAGE_OWNER_HANDLE_FREE] = handle;
> > + __set_bit(PAGE_EXT_OWNER_FREE, &ext->flags);
> > + }
> > +}
> > +#else
> > +void __page_owner_free_pages(struct page *page, unsigned int order)
> > +{
> > + int i;
> > + struct page_ext *page_ext;
> > +
> > + for (i = 0; i < (1 << order); i++) {
> > + page_ext = lookup_page_ext(page + i);
> > +
> > + if (unlikely(!page_ext))
> > + continue;
> > + __clear_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags);
> > + }
> > +}
> > +#endif
> > +
> > noinline void __page_owner_alloc_pages(struct page *page, unsigned int order,
> > gfp_t gfp_mask)
> > {
> > @@ -155,7 +188,7 @@ noinline void __page_owner_alloc_pages(struct page *page, unsigned int order,
> > if (unlikely(!page_ext))
> > return;
> >
> > - page_ext->handle = save_stack(gfp_mask);
> > + page_ext->handles[PAGE_OWNER_HANDLE_ALLOC] = save_stack(gfp_mask);
> > page_ext->order = order;
> > page_ext->gfp_mask = gfp_mask;
> > page_ext->last_migrate_reason = -1;
> > @@ -189,6 +222,7 @@ void __copy_page_owner(struct page *oldpage, struct page *newpage)
> > {
> > struct page_ext *old_ext = lookup_page_ext(oldpage);
> > struct page_ext *new_ext = lookup_page_ext(newpage);
> > + int i;
> >
> > if (unlikely(!old_ext || !new_ext))
> > return;
> > @@ -196,7 +230,9 @@ void __copy_page_owner(struct page *oldpage, struct page *newpage)
> > new_ext->order = old_ext->order;
> > new_ext->gfp_mask = old_ext->gfp_mask;
> > new_ext->last_migrate_reason = old_ext->last_migrate_reason;
> > - new_ext->handle = old_ext->handle;
> > +
> > + for (i = 0; i < PAGE_OWNER_HANDLE_MAX; i++)
> > + new_ext->handles[i] = old_ext->handles[i];
> >
> > /*
> > * We don't clear the bit on the oldpage as it's going to be freed
> > @@ -292,7 +328,7 @@ void __dump_page_owner(struct page *page)
> > };
> > depot_stack_handle_t handle;
> > gfp_t gfp_mask;
> > - int mt;
> > + int mt, i;
> >
> > if (unlikely(!page_ext)) {
> > pr_alert("There is not page extension available.\n");
> > @@ -301,25 +337,31 @@ void __dump_page_owner(struct page *page)
> > gfp_mask = page_ext->gfp_mask;
> > mt = gfpflags_to_migratetype(gfp_mask);
> >
> > - if (!test_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags)) {
> > + if (!test_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags) &&
> > + !test_bit(PAGE_EXT_OWNER_FREE, &page_ext->flags)) {
> > pr_alert("page_owner info is not active (free page?)\n");
> > return;
> > }
> >
> > - handle = READ_ONCE(page_ext->handle);
> > - if (!handle) {
> > - pr_alert("page_owner info is not active (free page?)\n");
> > - return;
> > - }
> > + for (i = 0; i < PAGE_OWNER_HANDLE_MAX; i++) {
> > + handle = READ_ONCE(page_ext->handles[i]);
> > + if (!handle) {
> > + pr_alert("page_owner info is not active for `%s'\n",
> > + page_owner_handles_names[i]);
> > + continue;
> > + }
> >
> > - depot_fetch_stack(handle, &trace);
> > - pr_alert("page allocated via order %u, migratetype %s, gfp_mask %#x(%pGg)\n",
> > - page_ext->order, migratetype_names[mt], gfp_mask, &gfp_mask);
> > - print_stack_trace(&trace, 0);
> > + depot_fetch_stack(handle, &trace);
> > + pr_alert("%s via order %u, migratetype %s, gfp_mask %#x(%pGg)\n",
> > + page_owner_handles_names[i], page_ext->order,
> > + migratetype_names[mt], gfp_mask, &gfp_mask);
> > + print_stack_trace(&trace, 0);
> >
> > - if (page_ext->last_migrate_reason != -1)
> > + if (page_ext->last_migrate_reason == -1)
> > + continue;
> > pr_alert("page has been migrated, last migrate reason: %s\n",
> > migrate_reason_names[page_ext->last_migrate_reason]);
> > + }
> > }
> >
> > static ssize_t
> > @@ -381,7 +423,7 @@ read_page_owner(struct file *file, char __user *buf, size_t count, loff_t *ppos)
> > * Access to page_ext->handle isn't synchronous so we should
> > * be careful to access it.
> > */
> > - handle = READ_ONCE(page_ext->handle);
> > + handle = READ_ONCE(page_ext->handles[PAGE_OWNER_HANDLE_ALLOC]);
> > if (!handle)
> > continue;
> >
> > diff --git a/mm/vmstat.c b/mm/vmstat.c
> > index 63ef65f..4ff0135 100644
> > --- a/mm/vmstat.c
> > +++ b/mm/vmstat.c
> > @@ -1073,6 +1073,9 @@ static void pagetypeinfo_showmixedcount_print(struct seq_file *m,
> > if (!test_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags))
> > continue;
> >
> > + if (!test_bit(PAGE_EXT_OWNER_FREE, &page_ext->flags))
> > + continue;
> > +
> > page_mt = gfpflags_to_migratetype(page_ext->gfp_mask);
> > if (pageblock_mt != page_mt) {
> > if (is_migrate_cma(pageblock_mt))
> > --
> > 2.9.0.37.g6d523a3
> >
> > --
> > 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/ .
> > Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] mm/page_owner: track page free call chain
@ 2016-07-04 5:07 ` Sergey Senozhatsky
0 siblings, 0 replies; 24+ messages in thread
From: Sergey Senozhatsky @ 2016-07-04 5:07 UTC (permalink / raw)
To: Joonsoo Kim
Cc: Sergey Senozhatsky, Andrew Morton, Vlastimil Babka, linux-mm,
linux-kernel, Sergey Senozhatsky
Hello,
On (07/04/16 13:57), Joonsoo Kim wrote:
> On Sun, Jul 03, 2016 at 01:16:56AM +0900, Sergey Senozhatsky wrote:
> > Introduce PAGE_OWNER_TRACK_FREE config option to extend page owner with
> > free_pages() tracking functionality. This adds to the dump_page_owner()
> > output an additional backtrace, that tells us what path has freed the
> > page.
>
> Hmm... Do you have other ideas to use this feature? Following example is
> just to detect use-after-free and we have other good tools for it
> (KASAN or DEBUG_PAGEALLOC) so I'm not sure whether it's useful or not.
there is no kasan for ARM32, for example (apart from the fact that
it's really hard to use kasan sometimes due to its cpu cycles and
memory requirements).
educate me, will DEBUG_PAGEALLOC tell us what path has triggered the
extra put_page()? hm... does ARM32 provide ARCH_SUPPORTS_DEBUG_PAGEALLOC?
-ss
>
> And, if there is a good usage, I think that we don't need to add new
> Kconfig, PAGE_OWNER_TRACK_FREE. It only takes 8 bytes per page and
> it's not that big in this debugging context.
>
> Thanks.
>
> >
> > Aa a trivial example, let's assume that do_some_foo() has an error - extra
> > put_page() on error return path, and the function is also getting preempted,
> > letting some other task to allocate the same page, which is then mistakenly
> > getting freed once again by do_some_foo().
> >
> > CPUA CPUB
> >
> > void do_some_foo(void)
> > {
> > page = alloc_page();
> > if (error) {
> > put_page(page);
> > goto out;
> > }
> > ...
> > out:
> > <<preempted>>
> > void do_some_bar()
> > {
> > page = alloc_page();
> > ...
> > <<preempted>>
> > ...
> > put_page(page);
> > }
> > <<use freed page>>
> > put_page(page);
> > }
> >
> >
> > Backtrace:
> >
> >
> > BUG: Bad page state in process cc1 pfn:bae1d
> > page:ffffea0002eb8740 count:-1 mapcount:0 mapping: (null) index:0x0
> > flags: 0x4000000000000000()
> > page dumped because: nonzero _count
> > page allocated via order 0, migratetype Unmovable, gfp_mask 0x2000200(GFP_NOWAIT|__GFP_NOWARN)
> > [<ffffffff8101bc9c>] save_stack_trace+0x26/0x41
> > [<ffffffff81110fe4>] save_stack+0x46/0xc3
> > [<ffffffff81111481>] __page_owner_alloc_pages+0x24/0x41
> > [<ffffffff810c9867>] post_alloc_hook+0x1e/0x20
> > [<ffffffff810ca63d>] get_page_from_freelist+0x4fd/0x756
> > [<ffffffff810cadea>] __alloc_pages_nodemask+0xe7/0xbcf
> > [<ffffffff810cb8e4>] __get_free_pages+0x12/0x40
> > [<ffffffff810e6b64>] __tlb_remove_page_size.part.12+0x37/0x78
> > [<ffffffff810e6d9b>] __tlb_remove_page_size+0x21/0x23
> > [<ffffffff810e7ff2>] unmap_page_range+0x63a/0x75b
> > [<ffffffff810e81cf>] unmap_single_vma+0xbc/0xc6
> > [<ffffffff810e82d2>] unmap_vmas+0x35/0x44
> > [<ffffffff810ee6f4>] exit_mmap+0x5a/0xec
> > [<ffffffff810385b4>] mmput+0x4a/0xdc
> > [<ffffffff8103dff7>] do_exit+0x398/0x8de
> > [<ffffffff8103e5ae>] do_group_exit+0x45/0xb0
> > page freed via order 0, migratetype Unmovable, gfp_mask 0x2000200(GFP_NOWAIT|__GFP_NOWARN)
> > [<ffffffff8101bc9c>] save_stack_trace+0x26/0x41
> > [<ffffffff81110fe4>] save_stack+0x46/0xc3
> > [<ffffffff81111411>] __page_owner_free_pages+0x25/0x71
> > [<ffffffff810c9f0a>] free_hot_cold_page+0x1d6/0x1ea
> > [<ffffffff810d03e1>] __put_page+0x37/0x3a
> > [<ffffffff8115b8da>] do_some_foo()+0x8a/0x8e
> > ...
> > Modules linked in: ....
> > CPU: 3 PID: 1274 Comm: cc1 Not tainted 4.7.0-rc5-next-20160701-dbg-00009-ge01494f-dirty #535
> > 0000000000000000 ffff8800aeea3c18 ffffffff811e67ca ffffea0002eb8740
> > ffffffff8175675e ffff8800aeea3c40 ffffffff810c87f5 0000000000000000
> > ffffffff81880b40 ffff880137d98438 ffff8800aeea3c50 ffffffff810c88d5
> > Call Trace:
> > [<ffffffff811e67ca>] dump_stack+0x68/0x92
> > [<ffffffff810c87f5>] bad_page+0xf8/0x11e
> > [<ffffffff810c88d5>] check_new_page_bad+0x63/0x65
> > [<ffffffff810ca36a>] get_page_from_freelist+0x22a/0x756
> > [<ffffffff810cadea>] __alloc_pages_nodemask+0xe7/0xbcf
> > [<ffffffff81073a43>] ? trace_hardirqs_on_caller+0x16d/0x189
> > [<ffffffff810ede8d>] ? vma_merge+0x159/0x249
> > [<ffffffff81074aa0>] ? __lock_acquire+0x2ac/0x15c7
> > [<ffffffff81034ace>] pte_alloc_one+0x1b/0x67
> > [<ffffffff810e922b>] __pte_alloc+0x19/0xa6
> > [<ffffffff810eb09f>] handle_mm_fault+0x409/0xc59
> > [<ffffffff810309f6>] __do_page_fault+0x1d8/0x3ac
> > [<ffffffff81030bf7>] do_page_fault+0xc/0xe
> > [<ffffffff814a84af>] page_fault+0x1f/0x30
> >
> > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > ---
> > include/linux/page_ext.h | 13 ++++++-
> > lib/Kconfig.debug | 10 +++++
> > mm/page_owner.c | 98 ++++++++++++++++++++++++++++++++++--------------
> > mm/vmstat.c | 3 ++
> > 4 files changed, 95 insertions(+), 29 deletions(-)
> >
> > diff --git a/include/linux/page_ext.h b/include/linux/page_ext.h
> > index 66ba2bb..90bd44a 100644
> > --- a/include/linux/page_ext.h
> > +++ b/include/linux/page_ext.h
> > @@ -27,12 +27,23 @@ enum page_ext_flags {
> > PAGE_EXT_DEBUG_POISON, /* Page is poisoned */
> > PAGE_EXT_DEBUG_GUARD,
> > PAGE_EXT_OWNER_ALLOC,
> > + PAGE_EXT_OWNER_FREE,
> > #if defined(CONFIG_IDLE_PAGE_TRACKING) && !defined(CONFIG_64BIT)
> > PAGE_EXT_YOUNG,
> > PAGE_EXT_IDLE,
> > #endif
> > };
> >
> > +#ifdef CONFIG_PAGE_OWNER
> > +enum page_owner_handles {
> > + PAGE_OWNER_HANDLE_ALLOC,
> > +#ifdef CONFIG_PAGE_OWNER_TRACK_FREE
> > + PAGE_OWNER_HANDLE_FREE,
> > +#endif
> > + PAGE_OWNER_HANDLE_MAX
> > +};
> > +#endif
> > +
> > /*
> > * Page Extension can be considered as an extended mem_map.
> > * A page_ext page is associated with every page descriptor. The
> > @@ -46,7 +57,7 @@ struct page_ext {
> > unsigned int order;
> > gfp_t gfp_mask;
> > int last_migrate_reason;
> > - depot_stack_handle_t handle;
> > + depot_stack_handle_t handles[PAGE_OWNER_HANDLE_MAX];
> > #endif
> > };
> >
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index 0f99819..20ac03b 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -260,6 +260,16 @@ config PAGE_OWNER
> >
> > If unsure, say N.
> >
> > +config PAGE_OWNER_TRACK_FREE
> > + bool "Track page free call chains"
> > + depends on PAGE_OWNER
> > + help
> > + Page owner keeps track of what call chain is the owner of a page (has
> > + allocated the page), this option enables an additional functionality
> > + to also track what call chain has freed the page.
> > +
> > + If unsure, say N.
> > +
> > config DEBUG_FS
> > bool "Debug Filesystem"
> > select SRCU
> > diff --git a/mm/page_owner.c b/mm/page_owner.c
> > index 4acccb7..5a108d6 100644
> > --- a/mm/page_owner.c
> > +++ b/mm/page_owner.c
> > @@ -13,6 +13,13 @@
> >
> > #define PAGE_OWNER_STACK_DEPTH (16)
> >
> > +static const char *page_owner_handles_names[PAGE_OWNER_HANDLE_MAX] = {
> > + "page allocated",
> > +#ifdef CONFIG_PAGE_OWNER_TRACK_FREE
> > + "page freed",
> > +#endif
> > +};
> > +
> > static bool page_owner_disabled = true;
> > DEFINE_STATIC_KEY_FALSE(page_owner_inited);
> >
> > @@ -85,19 +92,6 @@ struct page_ext_operations page_owner_ops = {
> > .init = init_page_owner,
> > };
> >
> > -void __page_owner_free_pages(struct page *page, unsigned int order)
> > -{
> > - int i;
> > - struct page_ext *page_ext;
> > -
> > - for (i = 0; i < (1 << order); i++) {
> > - page_ext = lookup_page_ext(page + i);
> > - if (unlikely(!page_ext))
> > - continue;
> > - __clear_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags);
> > - }
> > -}
> > -
> > static inline bool check_recursive_alloc(struct stack_trace *trace,
> > unsigned long ip)
> > {
> > @@ -147,6 +141,45 @@ static noinline depot_stack_handle_t save_stack(gfp_t flags)
> > return handle;
> > }
> >
> > +#ifdef CONFIG_PAGE_OWNER_TRACK_FREE
> > +void __page_owner_free_pages(struct page *page, unsigned int order)
> > +{
> > + int i;
> > + depot_stack_handle_t handle;
> > + struct page_ext *page_ext = lookup_page_ext(page);
> > +
> > + if (unlikely(!page_ext))
> > + return;
> > +
> > + handle = save_stack(0);
> > + page_ext->handles[PAGE_OWNER_HANDLE_FREE] = handle;
> > + __set_bit(PAGE_EXT_OWNER_FREE, &page_ext->flags);
> > +
> > + for (i = 1; i < (1 << order); i++) {
> > + struct page_ext *ext = lookup_page_ext(page + 1);
> > +
> > + if (unlikely(!ext))
> > + continue;
> > + ext->handles[PAGE_OWNER_HANDLE_FREE] = handle;
> > + __set_bit(PAGE_EXT_OWNER_FREE, &ext->flags);
> > + }
> > +}
> > +#else
> > +void __page_owner_free_pages(struct page *page, unsigned int order)
> > +{
> > + int i;
> > + struct page_ext *page_ext;
> > +
> > + for (i = 0; i < (1 << order); i++) {
> > + page_ext = lookup_page_ext(page + i);
> > +
> > + if (unlikely(!page_ext))
> > + continue;
> > + __clear_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags);
> > + }
> > +}
> > +#endif
> > +
> > noinline void __page_owner_alloc_pages(struct page *page, unsigned int order,
> > gfp_t gfp_mask)
> > {
> > @@ -155,7 +188,7 @@ noinline void __page_owner_alloc_pages(struct page *page, unsigned int order,
> > if (unlikely(!page_ext))
> > return;
> >
> > - page_ext->handle = save_stack(gfp_mask);
> > + page_ext->handles[PAGE_OWNER_HANDLE_ALLOC] = save_stack(gfp_mask);
> > page_ext->order = order;
> > page_ext->gfp_mask = gfp_mask;
> > page_ext->last_migrate_reason = -1;
> > @@ -189,6 +222,7 @@ void __copy_page_owner(struct page *oldpage, struct page *newpage)
> > {
> > struct page_ext *old_ext = lookup_page_ext(oldpage);
> > struct page_ext *new_ext = lookup_page_ext(newpage);
> > + int i;
> >
> > if (unlikely(!old_ext || !new_ext))
> > return;
> > @@ -196,7 +230,9 @@ void __copy_page_owner(struct page *oldpage, struct page *newpage)
> > new_ext->order = old_ext->order;
> > new_ext->gfp_mask = old_ext->gfp_mask;
> > new_ext->last_migrate_reason = old_ext->last_migrate_reason;
> > - new_ext->handle = old_ext->handle;
> > +
> > + for (i = 0; i < PAGE_OWNER_HANDLE_MAX; i++)
> > + new_ext->handles[i] = old_ext->handles[i];
> >
> > /*
> > * We don't clear the bit on the oldpage as it's going to be freed
> > @@ -292,7 +328,7 @@ void __dump_page_owner(struct page *page)
> > };
> > depot_stack_handle_t handle;
> > gfp_t gfp_mask;
> > - int mt;
> > + int mt, i;
> >
> > if (unlikely(!page_ext)) {
> > pr_alert("There is not page extension available.\n");
> > @@ -301,25 +337,31 @@ void __dump_page_owner(struct page *page)
> > gfp_mask = page_ext->gfp_mask;
> > mt = gfpflags_to_migratetype(gfp_mask);
> >
> > - if (!test_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags)) {
> > + if (!test_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags) &&
> > + !test_bit(PAGE_EXT_OWNER_FREE, &page_ext->flags)) {
> > pr_alert("page_owner info is not active (free page?)\n");
> > return;
> > }
> >
> > - handle = READ_ONCE(page_ext->handle);
> > - if (!handle) {
> > - pr_alert("page_owner info is not active (free page?)\n");
> > - return;
> > - }
> > + for (i = 0; i < PAGE_OWNER_HANDLE_MAX; i++) {
> > + handle = READ_ONCE(page_ext->handles[i]);
> > + if (!handle) {
> > + pr_alert("page_owner info is not active for `%s'\n",
> > + page_owner_handles_names[i]);
> > + continue;
> > + }
> >
> > - depot_fetch_stack(handle, &trace);
> > - pr_alert("page allocated via order %u, migratetype %s, gfp_mask %#x(%pGg)\n",
> > - page_ext->order, migratetype_names[mt], gfp_mask, &gfp_mask);
> > - print_stack_trace(&trace, 0);
> > + depot_fetch_stack(handle, &trace);
> > + pr_alert("%s via order %u, migratetype %s, gfp_mask %#x(%pGg)\n",
> > + page_owner_handles_names[i], page_ext->order,
> > + migratetype_names[mt], gfp_mask, &gfp_mask);
> > + print_stack_trace(&trace, 0);
> >
> > - if (page_ext->last_migrate_reason != -1)
> > + if (page_ext->last_migrate_reason == -1)
> > + continue;
> > pr_alert("page has been migrated, last migrate reason: %s\n",
> > migrate_reason_names[page_ext->last_migrate_reason]);
> > + }
> > }
> >
> > static ssize_t
> > @@ -381,7 +423,7 @@ read_page_owner(struct file *file, char __user *buf, size_t count, loff_t *ppos)
> > * Access to page_ext->handle isn't synchronous so we should
> > * be careful to access it.
> > */
> > - handle = READ_ONCE(page_ext->handle);
> > + handle = READ_ONCE(page_ext->handles[PAGE_OWNER_HANDLE_ALLOC]);
> > if (!handle)
> > continue;
> >
> > diff --git a/mm/vmstat.c b/mm/vmstat.c
> > index 63ef65f..4ff0135 100644
> > --- a/mm/vmstat.c
> > +++ b/mm/vmstat.c
> > @@ -1073,6 +1073,9 @@ static void pagetypeinfo_showmixedcount_print(struct seq_file *m,
> > if (!test_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags))
> > continue;
> >
> > + if (!test_bit(PAGE_EXT_OWNER_FREE, &page_ext->flags))
> > + continue;
> > +
> > page_mt = gfpflags_to_migratetype(page_ext->gfp_mask);
> > if (pageblock_mt != page_mt) {
> > if (is_migrate_cma(pageblock_mt))
> > --
> > 2.9.0.37.g6d523a3
> >
> > --
> > 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/ .
> > 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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] mm/page_owner: track page free call chain
2016-07-04 5:07 ` Sergey Senozhatsky
@ 2016-07-04 5:29 ` Joonsoo Kim
-1 siblings, 0 replies; 24+ messages in thread
From: Joonsoo Kim @ 2016-07-04 5:29 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Sergey Senozhatsky, Andrew Morton, Vlastimil Babka, linux-mm,
linux-kernel
On Mon, Jul 04, 2016 at 02:07:30PM +0900, Sergey Senozhatsky wrote:
> Hello,
>
> On (07/04/16 13:57), Joonsoo Kim wrote:
> > On Sun, Jul 03, 2016 at 01:16:56AM +0900, Sergey Senozhatsky wrote:
> > > Introduce PAGE_OWNER_TRACK_FREE config option to extend page owner with
> > > free_pages() tracking functionality. This adds to the dump_page_owner()
> > > output an additional backtrace, that tells us what path has freed the
> > > page.
> >
> > Hmm... Do you have other ideas to use this feature? Following example is
> > just to detect use-after-free and we have other good tools for it
> > (KASAN or DEBUG_PAGEALLOC) so I'm not sure whether it's useful or not.
>
> there is no kasan for ARM32, for example (apart from the fact that
> it's really hard to use kasan sometimes due to its cpu cycles and
> memory requirements).
Hmm... for debugging purpose, KASAN provides many more things so IMO it's
better to implement/support KASAN in ARM32 rather than expand
PAGE_OWNER for free.
>
> educate me, will DEBUG_PAGEALLOC tell us what path has triggered the
> extra put_page()? hm... does ARM32 provide ARCH_SUPPORTS_DEBUG_PAGEALLOC?
Hmm... Now, I notice that PAGE_OWNER_TRACK_FREE will detect
double-free rather than use-after-free. DEBUG_PAGEALLOC doesn't catch
double-free but it can be implemented easily. In this case, we can
show call path for second free.
AFAIK, ARM32 doesn't support ARCH_SUPPORTS_DEBUG_PAGEALLOC.
Thanks.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] mm/page_owner: track page free call chain
@ 2016-07-04 5:29 ` Joonsoo Kim
0 siblings, 0 replies; 24+ messages in thread
From: Joonsoo Kim @ 2016-07-04 5:29 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Sergey Senozhatsky, Andrew Morton, Vlastimil Babka, linux-mm,
linux-kernel
On Mon, Jul 04, 2016 at 02:07:30PM +0900, Sergey Senozhatsky wrote:
> Hello,
>
> On (07/04/16 13:57), Joonsoo Kim wrote:
> > On Sun, Jul 03, 2016 at 01:16:56AM +0900, Sergey Senozhatsky wrote:
> > > Introduce PAGE_OWNER_TRACK_FREE config option to extend page owner with
> > > free_pages() tracking functionality. This adds to the dump_page_owner()
> > > output an additional backtrace, that tells us what path has freed the
> > > page.
> >
> > Hmm... Do you have other ideas to use this feature? Following example is
> > just to detect use-after-free and we have other good tools for it
> > (KASAN or DEBUG_PAGEALLOC) so I'm not sure whether it's useful or not.
>
> there is no kasan for ARM32, for example (apart from the fact that
> it's really hard to use kasan sometimes due to its cpu cycles and
> memory requirements).
Hmm... for debugging purpose, KASAN provides many more things so IMO it's
better to implement/support KASAN in ARM32 rather than expand
PAGE_OWNER for free.
>
> educate me, will DEBUG_PAGEALLOC tell us what path has triggered the
> extra put_page()? hm... does ARM32 provide ARCH_SUPPORTS_DEBUG_PAGEALLOC?
Hmm... Now, I notice that PAGE_OWNER_TRACK_FREE will detect
double-free rather than use-after-free. DEBUG_PAGEALLOC doesn't catch
double-free but it can be implemented easily. In this case, we can
show call path for second free.
AFAIK, ARM32 doesn't support ARCH_SUPPORTS_DEBUG_PAGEALLOC.
Thanks.
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] mm/page_owner: track page free call chain
2016-07-04 5:29 ` Joonsoo Kim
@ 2016-07-04 5:45 ` Sergey Senozhatsky
-1 siblings, 0 replies; 24+ messages in thread
From: Sergey Senozhatsky @ 2016-07-04 5:45 UTC (permalink / raw)
To: Joonsoo Kim
Cc: Sergey Senozhatsky, Sergey Senozhatsky, Andrew Morton,
Vlastimil Babka, linux-mm, linux-kernel
On (07/04/16 14:29), Joonsoo Kim wrote:
> > > On Sun, Jul 03, 2016 at 01:16:56AM +0900, Sergey Senozhatsky wrote:
> > > > Introduce PAGE_OWNER_TRACK_FREE config option to extend page owner with
> > > > free_pages() tracking functionality. This adds to the dump_page_owner()
> > > > output an additional backtrace, that tells us what path has freed the
> > > > page.
> > >
> > > Hmm... Do you have other ideas to use this feature? Following example is
> > > just to detect use-after-free and we have other good tools for it
> > > (KASAN or DEBUG_PAGEALLOC) so I'm not sure whether it's useful or not.
> >
> > there is no kasan for ARM32, for example (apart from the fact that
> > it's really hard to use kasan sometimes due to its cpu cycles and
> > memory requirements).
>
> Hmm... for debugging purpose, KASAN provides many more things so IMO it's
> better to implement/support KASAN in ARM32 rather than expand
> PAGE_OWNER for free.
>
hm, the last time I checked kasan didn't catch that extra put_page() on
x86_64. AFAIK, kasan on ARM32 is a bit hard to do properly
http://www.serverphorums.com/read.php?12,1206479,1281087#msg-1281087
I've played with kasan on arm32 (an internal custom version)... and
extended page_owner turned out to be *incomparably* easier and faster
to use (especially paired with stackdepot).
> > educate me, will DEBUG_PAGEALLOC tell us what path has triggered the
> > extra put_page()? hm... does ARM32 provide ARCH_SUPPORTS_DEBUG_PAGEALLOC?
>
> Hmm... Now, I notice that PAGE_OWNER_TRACK_FREE will detect
> double-free rather than use-after-free.
well, yes. current hits bad_page(), page_owner helps to find out who
stole and spoiled it from under current.
CPU a CPU b
alloc_page()
put_page() << legitimate
alloc_page()
err:
put_page() << legitimate, again.
<< but is actually buggy.
put_page() << double free. but we need
<< to report put_page() from
<< CPU a.
-ss
> DEBUG_PAGEALLOC doesn't catch double-free but it can be implemented
> easily. In this case, we can show call path for second free.
>
> AFAIK, ARM32 doesn't support ARCH_SUPPORTS_DEBUG_PAGEALLOC.
>
> Thanks.
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] mm/page_owner: track page free call chain
@ 2016-07-04 5:45 ` Sergey Senozhatsky
0 siblings, 0 replies; 24+ messages in thread
From: Sergey Senozhatsky @ 2016-07-04 5:45 UTC (permalink / raw)
To: Joonsoo Kim
Cc: Sergey Senozhatsky, Sergey Senozhatsky, Andrew Morton,
Vlastimil Babka, linux-mm, linux-kernel
On (07/04/16 14:29), Joonsoo Kim wrote:
> > > On Sun, Jul 03, 2016 at 01:16:56AM +0900, Sergey Senozhatsky wrote:
> > > > Introduce PAGE_OWNER_TRACK_FREE config option to extend page owner with
> > > > free_pages() tracking functionality. This adds to the dump_page_owner()
> > > > output an additional backtrace, that tells us what path has freed the
> > > > page.
> > >
> > > Hmm... Do you have other ideas to use this feature? Following example is
> > > just to detect use-after-free and we have other good tools for it
> > > (KASAN or DEBUG_PAGEALLOC) so I'm not sure whether it's useful or not.
> >
> > there is no kasan for ARM32, for example (apart from the fact that
> > it's really hard to use kasan sometimes due to its cpu cycles and
> > memory requirements).
>
> Hmm... for debugging purpose, KASAN provides many more things so IMO it's
> better to implement/support KASAN in ARM32 rather than expand
> PAGE_OWNER for free.
>
hm, the last time I checked kasan didn't catch that extra put_page() on
x86_64. AFAIK, kasan on ARM32 is a bit hard to do properly
http://www.serverphorums.com/read.php?12,1206479,1281087#msg-1281087
I've played with kasan on arm32 (an internal custom version)... and
extended page_owner turned out to be *incomparably* easier and faster
to use (especially paired with stackdepot).
> > educate me, will DEBUG_PAGEALLOC tell us what path has triggered the
> > extra put_page()? hm... does ARM32 provide ARCH_SUPPORTS_DEBUG_PAGEALLOC?
>
> Hmm... Now, I notice that PAGE_OWNER_TRACK_FREE will detect
> double-free rather than use-after-free.
well, yes. current hits bad_page(), page_owner helps to find out who
stole and spoiled it from under current.
CPU a CPU b
alloc_page()
put_page() << legitimate
alloc_page()
err:
put_page() << legitimate, again.
<< but is actually buggy.
put_page() << double free. but we need
<< to report put_page() from
<< CPU a.
-ss
> DEBUG_PAGEALLOC doesn't catch double-free but it can be implemented
> easily. In this case, we can show call path for second free.
>
> AFAIK, ARM32 doesn't support ARCH_SUPPORTS_DEBUG_PAGEALLOC.
>
> Thanks.
>
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] mm/page_owner: track page free call chain
2016-07-04 5:45 ` Sergey Senozhatsky
@ 2016-07-04 7:29 ` Joonsoo Kim
-1 siblings, 0 replies; 24+ messages in thread
From: Joonsoo Kim @ 2016-07-04 7:29 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Sergey Senozhatsky, Andrew Morton, Vlastimil Babka, linux-mm,
linux-kernel
On Mon, Jul 04, 2016 at 02:45:24PM +0900, Sergey Senozhatsky wrote:
> On (07/04/16 14:29), Joonsoo Kim wrote:
> > > > On Sun, Jul 03, 2016 at 01:16:56AM +0900, Sergey Senozhatsky wrote:
> > > > > Introduce PAGE_OWNER_TRACK_FREE config option to extend page owner with
> > > > > free_pages() tracking functionality. This adds to the dump_page_owner()
> > > > > output an additional backtrace, that tells us what path has freed the
> > > > > page.
> > > >
> > > > Hmm... Do you have other ideas to use this feature? Following example is
> > > > just to detect use-after-free and we have other good tools for it
> > > > (KASAN or DEBUG_PAGEALLOC) so I'm not sure whether it's useful or not.
> > >
> > > there is no kasan for ARM32, for example (apart from the fact that
> > > it's really hard to use kasan sometimes due to its cpu cycles and
> > > memory requirements).
> >
> > Hmm... for debugging purpose, KASAN provides many more things so IMO it's
> > better to implement/support KASAN in ARM32 rather than expand
> > PAGE_OWNER for free.
> >
>
> hm, the last time I checked kasan didn't catch that extra put_page() on
Indeed. It seems that kasan only catch double-free of slab object.
> x86_64. AFAIK, kasan on ARM32 is a bit hard to do properly
> http://www.serverphorums.com/read.php?12,1206479,1281087#msg-1281087
Okay.
> I've played with kasan on arm32 (an internal custom version)... and
> extended page_owner turned out to be *incomparably* easier and faster
> to use (especially paired with stackdepot).
Okay.
>
> > > educate me, will DEBUG_PAGEALLOC tell us what path has triggered the
> > > extra put_page()? hm... does ARM32 provide ARCH_SUPPORTS_DEBUG_PAGEALLOC?
> >
> > Hmm... Now, I notice that PAGE_OWNER_TRACK_FREE will detect
> > double-free rather than use-after-free.
>
> well, yes. current hits bad_page(), page_owner helps to find out who
> stole and spoiled it from under current.
>
> CPU a CPU b
>
> alloc_page()
> put_page() << legitimate
> alloc_page()
> err:
> put_page() << legitimate, again.
> << but is actually buggy.
>
> put_page() << double free. but we need
> << to report put_page() from
> << CPU a.
Okay. I think that this patch make finding offending user easier
but it looks like it is a partial solution to detect double-free.
See following example.
CPU a CPU b
alloc_page()
put_page() << legitimate
alloc_page()
err:
put_page() << legitimate, again.
<< but is actually buggy.
alloc_page()
put_page() <<
legitimate,
again.
put_page() << Will report the bug and
page_owner have legitimate call stack.
In kasan, quarantine is used to provide some delay for real free and
it makes use-after-free detection more robust. Double-free also can be
benefit from it. Anyway, I will not object more since it looks
the simplest way to improve doublue-free detection for the page
at least for now.
Thanks.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] mm/page_owner: track page free call chain
@ 2016-07-04 7:29 ` Joonsoo Kim
0 siblings, 0 replies; 24+ messages in thread
From: Joonsoo Kim @ 2016-07-04 7:29 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Sergey Senozhatsky, Andrew Morton, Vlastimil Babka, linux-mm,
linux-kernel
On Mon, Jul 04, 2016 at 02:45:24PM +0900, Sergey Senozhatsky wrote:
> On (07/04/16 14:29), Joonsoo Kim wrote:
> > > > On Sun, Jul 03, 2016 at 01:16:56AM +0900, Sergey Senozhatsky wrote:
> > > > > Introduce PAGE_OWNER_TRACK_FREE config option to extend page owner with
> > > > > free_pages() tracking functionality. This adds to the dump_page_owner()
> > > > > output an additional backtrace, that tells us what path has freed the
> > > > > page.
> > > >
> > > > Hmm... Do you have other ideas to use this feature? Following example is
> > > > just to detect use-after-free and we have other good tools for it
> > > > (KASAN or DEBUG_PAGEALLOC) so I'm not sure whether it's useful or not.
> > >
> > > there is no kasan for ARM32, for example (apart from the fact that
> > > it's really hard to use kasan sometimes due to its cpu cycles and
> > > memory requirements).
> >
> > Hmm... for debugging purpose, KASAN provides many more things so IMO it's
> > better to implement/support KASAN in ARM32 rather than expand
> > PAGE_OWNER for free.
> >
>
> hm, the last time I checked kasan didn't catch that extra put_page() on
Indeed. It seems that kasan only catch double-free of slab object.
> x86_64. AFAIK, kasan on ARM32 is a bit hard to do properly
> http://www.serverphorums.com/read.php?12,1206479,1281087#msg-1281087
Okay.
> I've played with kasan on arm32 (an internal custom version)... and
> extended page_owner turned out to be *incomparably* easier and faster
> to use (especially paired with stackdepot).
Okay.
>
> > > educate me, will DEBUG_PAGEALLOC tell us what path has triggered the
> > > extra put_page()? hm... does ARM32 provide ARCH_SUPPORTS_DEBUG_PAGEALLOC?
> >
> > Hmm... Now, I notice that PAGE_OWNER_TRACK_FREE will detect
> > double-free rather than use-after-free.
>
> well, yes. current hits bad_page(), page_owner helps to find out who
> stole and spoiled it from under current.
>
> CPU a CPU b
>
> alloc_page()
> put_page() << legitimate
> alloc_page()
> err:
> put_page() << legitimate, again.
> << but is actually buggy.
>
> put_page() << double free. but we need
> << to report put_page() from
> << CPU a.
Okay. I think that this patch make finding offending user easier
but it looks like it is a partial solution to detect double-free.
See following example.
CPU a CPU b
alloc_page()
put_page() << legitimate
alloc_page()
err:
put_page() << legitimate, again.
<< but is actually buggy.
alloc_page()
put_page() <<
legitimate,
again.
put_page() << Will report the bug and
page_owner have legitimate call stack.
In kasan, quarantine is used to provide some delay for real free and
it makes use-after-free detection more robust. Double-free also can be
benefit from it. Anyway, I will not object more since it looks
the simplest way to improve doublue-free detection for the page
at least for now.
Thanks.
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] mm/page_owner: track page free call chain
2016-07-04 7:29 ` Joonsoo Kim
@ 2016-07-04 7:53 ` Sergey Senozhatsky
-1 siblings, 0 replies; 24+ messages in thread
From: Sergey Senozhatsky @ 2016-07-04 7:53 UTC (permalink / raw)
To: Joonsoo Kim
Cc: Sergey Senozhatsky, Sergey Senozhatsky, Andrew Morton,
Vlastimil Babka, linux-mm, linux-kernel
On (07/04/16 16:29), Joonsoo Kim wrote:
[..]
> > well, yes. current hits bad_page(), page_owner helps to find out who
> > stole and spoiled it from under current.
> >
> > CPU a CPU b
> >
> > alloc_page()
> > put_page() << legitimate
> > alloc_page()
> > err:
> > put_page() << legitimate, again.
> > << but is actually buggy.
> >
> > put_page() << double free. but we need
> > << to report put_page() from
> > << CPU a.
>
> Okay. I think that this patch make finding offending user easier
> but it looks like it is a partial solution to detect double-free.
> See following example.
>
> CPU a CPU b
>
> alloc_page()
> put_page() << legitimate
> alloc_page()
> err:
> put_page() << legitimate, again.
> << but is actually buggy.
>
> alloc_page()
>
> put_page() <<
> legitimate,
> again.
> put_page() << Will report the bug and
> page_owner have legitimate call stack.
good case. I think it will report "put_page()" from CPU b (the path that
actually dropped page refcount to zero and freed it), and alloc_page()
from CPU a. _might_ sound like a clue.
I agree, there are cases when this approach will not work out perfectly.
tracing refcount modification is probably the only reliable solution,
but given that sometimes it's unclear how to reproduce the bug, one can
end up looking at tons of traces.
> In kasan, quarantine is used to provide some delay for real free and
> it makes use-after-free detection more robust. Double-free also can be
> benefit from it. Anyway, I will not object more since it looks
> the simplest way to improve doublue-free detection for the page
> at least for now.
thanks!
there are things in the patch (it's an RFC after all) that I don't like.
in particular, I cut the corner in __dump_page_owner(). it now shows the
same text for both _ALLOC and _FREE handlers. I didn't want to add
additional ->order to page_ext. I can update the text to, e.g.
page allocated via order ... page_ext->order
and
page freed, WAS allocated via order ... page_ext->order
or extend page_ext and keep alloc and free ->order separately.
do you have any preferences here?
-ss
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] mm/page_owner: track page free call chain
@ 2016-07-04 7:53 ` Sergey Senozhatsky
0 siblings, 0 replies; 24+ messages in thread
From: Sergey Senozhatsky @ 2016-07-04 7:53 UTC (permalink / raw)
To: Joonsoo Kim
Cc: Sergey Senozhatsky, Sergey Senozhatsky, Andrew Morton,
Vlastimil Babka, linux-mm, linux-kernel
On (07/04/16 16:29), Joonsoo Kim wrote:
[..]
> > well, yes. current hits bad_page(), page_owner helps to find out who
> > stole and spoiled it from under current.
> >
> > CPU a CPU b
> >
> > alloc_page()
> > put_page() << legitimate
> > alloc_page()
> > err:
> > put_page() << legitimate, again.
> > << but is actually buggy.
> >
> > put_page() << double free. but we need
> > << to report put_page() from
> > << CPU a.
>
> Okay. I think that this patch make finding offending user easier
> but it looks like it is a partial solution to detect double-free.
> See following example.
>
> CPU a CPU b
>
> alloc_page()
> put_page() << legitimate
> alloc_page()
> err:
> put_page() << legitimate, again.
> << but is actually buggy.
>
> alloc_page()
>
> put_page() <<
> legitimate,
> again.
> put_page() << Will report the bug and
> page_owner have legitimate call stack.
good case. I think it will report "put_page()" from CPU b (the path that
actually dropped page refcount to zero and freed it), and alloc_page()
from CPU a. _might_ sound like a clue.
I agree, there are cases when this approach will not work out perfectly.
tracing refcount modification is probably the only reliable solution,
but given that sometimes it's unclear how to reproduce the bug, one can
end up looking at tons of traces.
> In kasan, quarantine is used to provide some delay for real free and
> it makes use-after-free detection more robust. Double-free also can be
> benefit from it. Anyway, I will not object more since it looks
> the simplest way to improve doublue-free detection for the page
> at least for now.
thanks!
there are things in the patch (it's an RFC after all) that I don't like.
in particular, I cut the corner in __dump_page_owner(). it now shows the
same text for both _ALLOC and _FREE handlers. I didn't want to add
additional ->order to page_ext. I can update the text to, e.g.
page allocated via order ... page_ext->order
and
page freed, WAS allocated via order ... page_ext->order
or extend page_ext and keep alloc and free ->order separately.
do you have any preferences here?
-ss
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 3/3] mm/page_owner: track page free call chain
2016-07-08 12:11 [RFC][PATCH v2 " Sergey Senozhatsky
@ 2016-07-11 14:27 ` Sergey Senozhatsky
0 siblings, 0 replies; 24+ messages in thread
From: Sergey Senozhatsky @ 2016-07-11 14:27 UTC (permalink / raw)
To: Joonsoo Kim
Cc: Andrew Morton, Vlastimil Babka, linux-mm, linux-kernel,
Sergey Senozhatsky, Sergey Senozhatsky
Extend page_owner with free_pages() tracking functionality. This adds to the
dump_page_owner() output an additional backtrace, that tells us what path has
freed the page.
Aa a trivial example, let's assume that do_some_foo() has an error - an extra
put_page() on error return path, and the function is also getting preempted,
letting some other task to allocate the same page, which is then 'mistakenly'
getting freed once again by do_some_foo().
CPUA CPUB
void do_some_foo(void)
{
page = alloc_page();
if (error) {
put_page(page);
goto out;
}
...
out:
<<preempted>>
void do_some_bar()
{
page = alloc_page();
...
<<preempted>>
...
put_page(page);
}
<<use freed page>>
put_page(page);
}
With the existing implementation we would see only CPUB's backtrace
from bad_page. The extended page_owner would also report CPUA's
put_page(), which can be a helpful hint.
Backtrace:
BUG: Bad page state in process cc1 pfn:bae1d
page:ffffea0002eb8740 count:-1 mapcount:0 mapping: (null) index:0x0
flags: 0x4000000000000000()
page dumped because: nonzero _count
page allocated via order 0, migratetype Unmovable, gfp_mask 0x2000200(GFP_NOWAIT|__GFP_NOWARN)
[<ffffffff8101bc9c>] save_stack_trace+0x26/0x41
[<ffffffff81110fe4>] save_stack+0x46/0xc3
[<ffffffff81111481>] __page_owner_alloc_pages+0x24/0x41
[<ffffffff810c9867>] post_alloc_hook+0x1e/0x20
[<ffffffff810ca63d>] get_page_from_freelist+0x4fd/0x756
[<ffffffff810cadea>] __alloc_pages_nodemask+0xe7/0xbcf
[<ffffffff810cb8e4>] __get_free_pages+0x12/0x40
[<ffffffff810e6b64>] __tlb_remove_page_size.part.12+0x37/0x78
[<ffffffff810e6d9b>] __tlb_remove_page_size+0x21/0x23
[<ffffffff810e7ff2>] unmap_page_range+0x63a/0x75b
[<ffffffff810e81cf>] unmap_single_vma+0xbc/0xc6
[<ffffffff810e82d2>] unmap_vmas+0x35/0x44
[<ffffffff810ee6f4>] exit_mmap+0x5a/0xec
[<ffffffff810385b4>] mmput+0x4a/0xdc
[<ffffffff8103dff7>] do_exit+0x398/0x8de
[<ffffffff8103e5ae>] do_group_exit+0x45/0xb0
page freed, was allocated via order 0, migratetype Unmovable, gfp_mask 0x2000200(GFP_NOWAIT|__GFP_NOWARN)
[<ffffffff8101bc9c>] save_stack_trace+0x26/0x41
[<ffffffff81110fe4>] save_stack+0x46/0xc3
[<ffffffff81111411>] __page_owner_free_pages+0x25/0x71
[<ffffffff810c9f0a>] free_hot_cold_page+0x1d6/0x1ea
[<ffffffff810d03e1>] __put_page+0x37/0x3a
[<ffffffff8115b8da>] do_some_foo()+0x8a/0x8e
...
Modules linked in: ....
CPU: 3 PID: 1274 Comm: cc1 Not tainted 4.7.0-rc5-next-20160701-dbg-00009-ge01494f-dirty #535
0000000000000000 ffff8800aeea3c18 ffffffff811e67ca ffffea0002eb8740
ffffffff8175675e ffff8800aeea3c40 ffffffff810c87f5 0000000000000000
ffffffff81880b40 ffff880137d98438 ffff8800aeea3c50 ffffffff810c88d5
Call Trace:
[<ffffffff811e67ca>] dump_stack+0x68/0x92
[<ffffffff810c87f5>] bad_page+0xf8/0x11e
[<ffffffff810c88d5>] check_new_page_bad+0x63/0x65
[<ffffffff810ca36a>] get_page_from_freelist+0x22a/0x756
[<ffffffff810cadea>] __alloc_pages_nodemask+0xe7/0xbcf
[<ffffffff81073a43>] ? trace_hardirqs_on_caller+0x16d/0x189
[<ffffffff810ede8d>] ? vma_merge+0x159/0x249
[<ffffffff81074aa0>] ? __lock_acquire+0x2ac/0x15c7
[<ffffffff81034ace>] pte_alloc_one+0x1b/0x67
[<ffffffff810e922b>] __pte_alloc+0x19/0xa6
[<ffffffff810eb09f>] handle_mm_fault+0x409/0xc59
[<ffffffff810309f6>] __do_page_fault+0x1d8/0x3ac
[<ffffffff81030bf7>] do_page_fault+0xc/0xe
[<ffffffff814a84af>] page_fault+0x1f/0x30
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
include/linux/page_ext.h | 11 ++++++-
mm/page_owner.c | 74 ++++++++++++++++++++++++++++++------------------
2 files changed, 56 insertions(+), 29 deletions(-)
diff --git a/include/linux/page_ext.h b/include/linux/page_ext.h
index 66ba2bb..0cccc94 100644
--- a/include/linux/page_ext.h
+++ b/include/linux/page_ext.h
@@ -27,12 +27,21 @@ enum page_ext_flags {
PAGE_EXT_DEBUG_POISON, /* Page is poisoned */
PAGE_EXT_DEBUG_GUARD,
PAGE_EXT_OWNER_ALLOC,
+ PAGE_EXT_OWNER_FREE,
#if defined(CONFIG_IDLE_PAGE_TRACKING) && !defined(CONFIG_64BIT)
PAGE_EXT_YOUNG,
PAGE_EXT_IDLE,
#endif
};
+#ifdef CONFIG_PAGE_OWNER
+enum page_owner_handles {
+ PAGE_OWNER_HANDLE_ALLOC,
+ PAGE_OWNER_HANDLE_FREE,
+ PAGE_OWNER_HANDLE_MAX
+};
+#endif
+
/*
* Page Extension can be considered as an extended mem_map.
* A page_ext page is associated with every page descriptor. The
@@ -46,7 +55,7 @@ struct page_ext {
unsigned int order;
gfp_t gfp_mask;
int last_migrate_reason;
- depot_stack_handle_t handle;
+ depot_stack_handle_t handles[PAGE_OWNER_HANDLE_MAX];
#endif
};
diff --git a/mm/page_owner.c b/mm/page_owner.c
index 4acccb7..c431ac4 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -13,6 +13,11 @@
#define PAGE_OWNER_STACK_DEPTH (16)
+static const char *page_owner_handles_names[PAGE_OWNER_HANDLE_MAX] = {
+ "page allocated",
+ "page freed, was allocated",
+};
+
static bool page_owner_disabled = true;
DEFINE_STATIC_KEY_FALSE(page_owner_inited);
@@ -85,19 +90,6 @@ struct page_ext_operations page_owner_ops = {
.init = init_page_owner,
};
-void __page_owner_free_pages(struct page *page, unsigned int order)
-{
- int i;
- struct page_ext *page_ext;
-
- for (i = 0; i < (1 << order); i++) {
- page_ext = lookup_page_ext(page + i);
- if (unlikely(!page_ext))
- continue;
- __clear_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags);
- }
-}
-
static inline bool check_recursive_alloc(struct stack_trace *trace,
unsigned long ip)
{
@@ -147,6 +139,23 @@ static noinline depot_stack_handle_t save_stack(gfp_t flags)
return handle;
}
+void __page_owner_free_pages(struct page *page, unsigned int order)
+{
+ int i;
+ depot_stack_handle_t handle = save_stack(0);
+
+ for (i = 0; i < (1 << order); i++) {
+ struct page_ext *page_ext = lookup_page_ext(page + i);
+
+ if (unlikely(!page_ext))
+ continue;
+
+ page_ext->handles[PAGE_OWNER_HANDLE_FREE] = handle;
+ __set_bit(PAGE_EXT_OWNER_FREE, &page_ext->flags);
+ __clear_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags);
+ }
+}
+
noinline void __page_owner_alloc_pages(struct page *page, unsigned int order,
gfp_t gfp_mask)
{
@@ -155,7 +164,7 @@ noinline void __page_owner_alloc_pages(struct page *page, unsigned int order,
if (unlikely(!page_ext))
return;
- page_ext->handle = save_stack(gfp_mask);
+ page_ext->handles[PAGE_OWNER_HANDLE_ALLOC] = save_stack(gfp_mask);
page_ext->order = order;
page_ext->gfp_mask = gfp_mask;
page_ext->last_migrate_reason = -1;
@@ -189,6 +198,7 @@ void __copy_page_owner(struct page *oldpage, struct page *newpage)
{
struct page_ext *old_ext = lookup_page_ext(oldpage);
struct page_ext *new_ext = lookup_page_ext(newpage);
+ int i;
if (unlikely(!old_ext || !new_ext))
return;
@@ -196,7 +206,9 @@ void __copy_page_owner(struct page *oldpage, struct page *newpage)
new_ext->order = old_ext->order;
new_ext->gfp_mask = old_ext->gfp_mask;
new_ext->last_migrate_reason = old_ext->last_migrate_reason;
- new_ext->handle = old_ext->handle;
+
+ for (i = 0; i < PAGE_OWNER_HANDLE_MAX; i++)
+ new_ext->handles[i] = old_ext->handles[i];
/*
* We don't clear the bit on the oldpage as it's going to be freed
@@ -292,7 +304,7 @@ void __dump_page_owner(struct page *page)
};
depot_stack_handle_t handle;
gfp_t gfp_mask;
- int mt;
+ int mt, i;
if (unlikely(!page_ext)) {
pr_alert("There is not page extension available.\n");
@@ -301,25 +313,31 @@ void __dump_page_owner(struct page *page)
gfp_mask = page_ext->gfp_mask;
mt = gfpflags_to_migratetype(gfp_mask);
- if (!test_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags)) {
+ if (!test_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags) &&
+ !test_bit(PAGE_EXT_OWNER_FREE, &page_ext->flags)) {
pr_alert("page_owner info is not active (free page?)\n");
return;
}
- handle = READ_ONCE(page_ext->handle);
- if (!handle) {
- pr_alert("page_owner info is not active (free page?)\n");
- return;
- }
+ for (i = 0; i < PAGE_OWNER_HANDLE_MAX; i++) {
+ handle = READ_ONCE(page_ext->handles[i]);
+ if (!handle) {
+ pr_alert("page_owner info is not active for `%s'\n",
+ page_owner_handles_names[i]);
+ continue;
+ }
- depot_fetch_stack(handle, &trace);
- pr_alert("page allocated via order %u, migratetype %s, gfp_mask %#x(%pGg)\n",
- page_ext->order, migratetype_names[mt], gfp_mask, &gfp_mask);
- print_stack_trace(&trace, 0);
+ depot_fetch_stack(handle, &trace);
+ pr_alert("%s via order %u, migratetype %s, gfp_mask %#x(%pGg)\n",
+ page_owner_handles_names[i], page_ext->order,
+ migratetype_names[mt], gfp_mask, &gfp_mask);
+ print_stack_trace(&trace, 0);
- if (page_ext->last_migrate_reason != -1)
+ if (page_ext->last_migrate_reason == -1)
+ continue;
pr_alert("page has been migrated, last migrate reason: %s\n",
migrate_reason_names[page_ext->last_migrate_reason]);
+ }
}
static ssize_t
@@ -381,7 +399,7 @@ read_page_owner(struct file *file, char __user *buf, size_t count, loff_t *ppos)
* Access to page_ext->handle isn't synchronous so we should
* be careful to access it.
*/
- handle = READ_ONCE(page_ext->handle);
+ handle = READ_ONCE(page_ext->handles[PAGE_OWNER_HANDLE_ALLOC]);
if (!handle)
continue;
--
2.9.0.243.g5c589a7
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 3/3] mm/page_owner: track page free call chain
@ 2016-07-11 14:27 ` Sergey Senozhatsky
0 siblings, 0 replies; 24+ messages in thread
From: Sergey Senozhatsky @ 2016-07-11 14:27 UTC (permalink / raw)
To: Joonsoo Kim
Cc: Andrew Morton, Vlastimil Babka, linux-mm, linux-kernel,
Sergey Senozhatsky, Sergey Senozhatsky
Extend page_owner with free_pages() tracking functionality. This adds to the
dump_page_owner() output an additional backtrace, that tells us what path has
freed the page.
Aa a trivial example, let's assume that do_some_foo() has an error - an extra
put_page() on error return path, and the function is also getting preempted,
letting some other task to allocate the same page, which is then 'mistakenly'
getting freed once again by do_some_foo().
CPUA CPUB
void do_some_foo(void)
{
page = alloc_page();
if (error) {
put_page(page);
goto out;
}
...
out:
<<preempted>>
void do_some_bar()
{
page = alloc_page();
...
<<preempted>>
...
put_page(page);
}
<<use freed page>>
put_page(page);
}
With the existing implementation we would see only CPUB's backtrace
from bad_page. The extended page_owner would also report CPUA's
put_page(), which can be a helpful hint.
Backtrace:
BUG: Bad page state in process cc1 pfn:bae1d
page:ffffea0002eb8740 count:-1 mapcount:0 mapping: (null) index:0x0
flags: 0x4000000000000000()
page dumped because: nonzero _count
page allocated via order 0, migratetype Unmovable, gfp_mask 0x2000200(GFP_NOWAIT|__GFP_NOWARN)
[<ffffffff8101bc9c>] save_stack_trace+0x26/0x41
[<ffffffff81110fe4>] save_stack+0x46/0xc3
[<ffffffff81111481>] __page_owner_alloc_pages+0x24/0x41
[<ffffffff810c9867>] post_alloc_hook+0x1e/0x20
[<ffffffff810ca63d>] get_page_from_freelist+0x4fd/0x756
[<ffffffff810cadea>] __alloc_pages_nodemask+0xe7/0xbcf
[<ffffffff810cb8e4>] __get_free_pages+0x12/0x40
[<ffffffff810e6b64>] __tlb_remove_page_size.part.12+0x37/0x78
[<ffffffff810e6d9b>] __tlb_remove_page_size+0x21/0x23
[<ffffffff810e7ff2>] unmap_page_range+0x63a/0x75b
[<ffffffff810e81cf>] unmap_single_vma+0xbc/0xc6
[<ffffffff810e82d2>] unmap_vmas+0x35/0x44
[<ffffffff810ee6f4>] exit_mmap+0x5a/0xec
[<ffffffff810385b4>] mmput+0x4a/0xdc
[<ffffffff8103dff7>] do_exit+0x398/0x8de
[<ffffffff8103e5ae>] do_group_exit+0x45/0xb0
page freed, was allocated via order 0, migratetype Unmovable, gfp_mask 0x2000200(GFP_NOWAIT|__GFP_NOWARN)
[<ffffffff8101bc9c>] save_stack_trace+0x26/0x41
[<ffffffff81110fe4>] save_stack+0x46/0xc3
[<ffffffff81111411>] __page_owner_free_pages+0x25/0x71
[<ffffffff810c9f0a>] free_hot_cold_page+0x1d6/0x1ea
[<ffffffff810d03e1>] __put_page+0x37/0x3a
[<ffffffff8115b8da>] do_some_foo()+0x8a/0x8e
...
Modules linked in: ....
CPU: 3 PID: 1274 Comm: cc1 Not tainted 4.7.0-rc5-next-20160701-dbg-00009-ge01494f-dirty #535
0000000000000000 ffff8800aeea3c18 ffffffff811e67ca ffffea0002eb8740
ffffffff8175675e ffff8800aeea3c40 ffffffff810c87f5 0000000000000000
ffffffff81880b40 ffff880137d98438 ffff8800aeea3c50 ffffffff810c88d5
Call Trace:
[<ffffffff811e67ca>] dump_stack+0x68/0x92
[<ffffffff810c87f5>] bad_page+0xf8/0x11e
[<ffffffff810c88d5>] check_new_page_bad+0x63/0x65
[<ffffffff810ca36a>] get_page_from_freelist+0x22a/0x756
[<ffffffff810cadea>] __alloc_pages_nodemask+0xe7/0xbcf
[<ffffffff81073a43>] ? trace_hardirqs_on_caller+0x16d/0x189
[<ffffffff810ede8d>] ? vma_merge+0x159/0x249
[<ffffffff81074aa0>] ? __lock_acquire+0x2ac/0x15c7
[<ffffffff81034ace>] pte_alloc_one+0x1b/0x67
[<ffffffff810e922b>] __pte_alloc+0x19/0xa6
[<ffffffff810eb09f>] handle_mm_fault+0x409/0xc59
[<ffffffff810309f6>] __do_page_fault+0x1d8/0x3ac
[<ffffffff81030bf7>] do_page_fault+0xc/0xe
[<ffffffff814a84af>] page_fault+0x1f/0x30
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
include/linux/page_ext.h | 11 ++++++-
mm/page_owner.c | 74 ++++++++++++++++++++++++++++++------------------
2 files changed, 56 insertions(+), 29 deletions(-)
diff --git a/include/linux/page_ext.h b/include/linux/page_ext.h
index 66ba2bb..0cccc94 100644
--- a/include/linux/page_ext.h
+++ b/include/linux/page_ext.h
@@ -27,12 +27,21 @@ enum page_ext_flags {
PAGE_EXT_DEBUG_POISON, /* Page is poisoned */
PAGE_EXT_DEBUG_GUARD,
PAGE_EXT_OWNER_ALLOC,
+ PAGE_EXT_OWNER_FREE,
#if defined(CONFIG_IDLE_PAGE_TRACKING) && !defined(CONFIG_64BIT)
PAGE_EXT_YOUNG,
PAGE_EXT_IDLE,
#endif
};
+#ifdef CONFIG_PAGE_OWNER
+enum page_owner_handles {
+ PAGE_OWNER_HANDLE_ALLOC,
+ PAGE_OWNER_HANDLE_FREE,
+ PAGE_OWNER_HANDLE_MAX
+};
+#endif
+
/*
* Page Extension can be considered as an extended mem_map.
* A page_ext page is associated with every page descriptor. The
@@ -46,7 +55,7 @@ struct page_ext {
unsigned int order;
gfp_t gfp_mask;
int last_migrate_reason;
- depot_stack_handle_t handle;
+ depot_stack_handle_t handles[PAGE_OWNER_HANDLE_MAX];
#endif
};
diff --git a/mm/page_owner.c b/mm/page_owner.c
index 4acccb7..c431ac4 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -13,6 +13,11 @@
#define PAGE_OWNER_STACK_DEPTH (16)
+static const char *page_owner_handles_names[PAGE_OWNER_HANDLE_MAX] = {
+ "page allocated",
+ "page freed, was allocated",
+};
+
static bool page_owner_disabled = true;
DEFINE_STATIC_KEY_FALSE(page_owner_inited);
@@ -85,19 +90,6 @@ struct page_ext_operations page_owner_ops = {
.init = init_page_owner,
};
-void __page_owner_free_pages(struct page *page, unsigned int order)
-{
- int i;
- struct page_ext *page_ext;
-
- for (i = 0; i < (1 << order); i++) {
- page_ext = lookup_page_ext(page + i);
- if (unlikely(!page_ext))
- continue;
- __clear_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags);
- }
-}
-
static inline bool check_recursive_alloc(struct stack_trace *trace,
unsigned long ip)
{
@@ -147,6 +139,23 @@ static noinline depot_stack_handle_t save_stack(gfp_t flags)
return handle;
}
+void __page_owner_free_pages(struct page *page, unsigned int order)
+{
+ int i;
+ depot_stack_handle_t handle = save_stack(0);
+
+ for (i = 0; i < (1 << order); i++) {
+ struct page_ext *page_ext = lookup_page_ext(page + i);
+
+ if (unlikely(!page_ext))
+ continue;
+
+ page_ext->handles[PAGE_OWNER_HANDLE_FREE] = handle;
+ __set_bit(PAGE_EXT_OWNER_FREE, &page_ext->flags);
+ __clear_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags);
+ }
+}
+
noinline void __page_owner_alloc_pages(struct page *page, unsigned int order,
gfp_t gfp_mask)
{
@@ -155,7 +164,7 @@ noinline void __page_owner_alloc_pages(struct page *page, unsigned int order,
if (unlikely(!page_ext))
return;
- page_ext->handle = save_stack(gfp_mask);
+ page_ext->handles[PAGE_OWNER_HANDLE_ALLOC] = save_stack(gfp_mask);
page_ext->order = order;
page_ext->gfp_mask = gfp_mask;
page_ext->last_migrate_reason = -1;
@@ -189,6 +198,7 @@ void __copy_page_owner(struct page *oldpage, struct page *newpage)
{
struct page_ext *old_ext = lookup_page_ext(oldpage);
struct page_ext *new_ext = lookup_page_ext(newpage);
+ int i;
if (unlikely(!old_ext || !new_ext))
return;
@@ -196,7 +206,9 @@ void __copy_page_owner(struct page *oldpage, struct page *newpage)
new_ext->order = old_ext->order;
new_ext->gfp_mask = old_ext->gfp_mask;
new_ext->last_migrate_reason = old_ext->last_migrate_reason;
- new_ext->handle = old_ext->handle;
+
+ for (i = 0; i < PAGE_OWNER_HANDLE_MAX; i++)
+ new_ext->handles[i] = old_ext->handles[i];
/*
* We don't clear the bit on the oldpage as it's going to be freed
@@ -292,7 +304,7 @@ void __dump_page_owner(struct page *page)
};
depot_stack_handle_t handle;
gfp_t gfp_mask;
- int mt;
+ int mt, i;
if (unlikely(!page_ext)) {
pr_alert("There is not page extension available.\n");
@@ -301,25 +313,31 @@ void __dump_page_owner(struct page *page)
gfp_mask = page_ext->gfp_mask;
mt = gfpflags_to_migratetype(gfp_mask);
- if (!test_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags)) {
+ if (!test_bit(PAGE_EXT_OWNER_ALLOC, &page_ext->flags) &&
+ !test_bit(PAGE_EXT_OWNER_FREE, &page_ext->flags)) {
pr_alert("page_owner info is not active (free page?)\n");
return;
}
- handle = READ_ONCE(page_ext->handle);
- if (!handle) {
- pr_alert("page_owner info is not active (free page?)\n");
- return;
- }
+ for (i = 0; i < PAGE_OWNER_HANDLE_MAX; i++) {
+ handle = READ_ONCE(page_ext->handles[i]);
+ if (!handle) {
+ pr_alert("page_owner info is not active for `%s'\n",
+ page_owner_handles_names[i]);
+ continue;
+ }
- depot_fetch_stack(handle, &trace);
- pr_alert("page allocated via order %u, migratetype %s, gfp_mask %#x(%pGg)\n",
- page_ext->order, migratetype_names[mt], gfp_mask, &gfp_mask);
- print_stack_trace(&trace, 0);
+ depot_fetch_stack(handle, &trace);
+ pr_alert("%s via order %u, migratetype %s, gfp_mask %#x(%pGg)\n",
+ page_owner_handles_names[i], page_ext->order,
+ migratetype_names[mt], gfp_mask, &gfp_mask);
+ print_stack_trace(&trace, 0);
- if (page_ext->last_migrate_reason != -1)
+ if (page_ext->last_migrate_reason == -1)
+ continue;
pr_alert("page has been migrated, last migrate reason: %s\n",
migrate_reason_names[page_ext->last_migrate_reason]);
+ }
}
static ssize_t
@@ -381,7 +399,7 @@ read_page_owner(struct file *file, char __user *buf, size_t count, loff_t *ppos)
* Access to page_ext->handle isn't synchronous so we should
* be careful to access it.
*/
- handle = READ_ONCE(page_ext->handle);
+ handle = READ_ONCE(page_ext->handles[PAGE_OWNER_HANDLE_ALLOC]);
if (!handle)
continue;
--
2.9.0.243.g5c589a7
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 24+ messages in thread
end of thread, other threads:[~2016-07-11 14:28 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-02 16:16 [PATCH 0/3][RFC] mm/page:_owner: track page free call chain Sergey Senozhatsky
2016-07-02 16:16 ` Sergey Senozhatsky
2016-07-02 16:16 ` [PATCH 1/3] mm/page_owner: rename page_owner functions Sergey Senozhatsky
2016-07-02 16:16 ` Sergey Senozhatsky
2016-07-02 16:16 ` [PATCH 2/3] mm/page_owner: rename PAGE_EXT_OWNER flag Sergey Senozhatsky
2016-07-02 16:16 ` Sergey Senozhatsky
2016-07-02 16:16 ` [PATCH 3/3] mm/page_owner: track page free call chain Sergey Senozhatsky
2016-07-02 16:16 ` Sergey Senozhatsky
2016-07-03 5:59 ` Sergey Senozhatsky
2016-07-03 5:59 ` Sergey Senozhatsky
2016-07-04 4:57 ` Joonsoo Kim
2016-07-04 4:57 ` Joonsoo Kim
2016-07-04 5:07 ` Sergey Senozhatsky
2016-07-04 5:07 ` Sergey Senozhatsky
2016-07-04 5:29 ` Joonsoo Kim
2016-07-04 5:29 ` Joonsoo Kim
2016-07-04 5:45 ` Sergey Senozhatsky
2016-07-04 5:45 ` Sergey Senozhatsky
2016-07-04 7:29 ` Joonsoo Kim
2016-07-04 7:29 ` Joonsoo Kim
2016-07-04 7:53 ` Sergey Senozhatsky
2016-07-04 7:53 ` Sergey Senozhatsky
2016-07-08 12:11 [RFC][PATCH v2 " Sergey Senozhatsky
2016-07-11 14:27 ` [PATCH " Sergey Senozhatsky
2016-07-11 14:27 ` Sergey Senozhatsky
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.