On Tue, Aug 20, 2019 at 03:18:28PM +0200, Vlastimil Babka wrote:
> The debug_pagealloc functionality is useful to catch buggy page allocator users
> that cause e.g. use after free or double free. When page inconsistency is
> detected, debugging is often simpler by knowing the call stack of process that
> last allocated and freed the page. When page_owner is also enabled, we record
> the allocation stack trace, but not freeing.
>
> This patch therefore adds recording of freeing process stack trace to page
> owner info, if both page_owner and debug_pagealloc are configured and enabled.
> With only page_owner enabled, this info is not useful for the memory leak
> debugging use case. dump_page() is adjusted to print the info. An example
> result of calling __free_pages() twice may look like this (note the page last free
> stack trace):
>
> BUG: Bad page state in process bash pfn:13d8f8
> page:ffffc31984f63e00 refcount:-1 mapcount:0 mapping:0000000000000000 index:0x0
> flags: 0x1affff800000000()
> raw: 01affff800000000 dead000000000100 dead000000000122 0000000000000000
> raw: 0000000000000000 0000000000000000 ffffffffffffffff 0000000000000000
> page dumped because: nonzero _refcount
> page_owner tracks the page as freed
> page last allocated via order 0, migratetype Unmovable, gfp_mask 0xcc0(GFP_KERNEL)
> prep_new_page+0x143/0x150
> get_page_from_freelist+0x289/0x380
> __alloc_pages_nodemask+0x13c/0x2d0
> khugepaged+0x6e/0xc10
> kthread+0xf9/0x130
> ret_from_fork+0x3a/0x50
> page last free stack trace:
> free_pcp_prepare+0x134/0x1e0
> free_unref_page+0x18/0x90
> khugepaged+0x7b/0xc10
> kthread+0xf9/0x130
> ret_from_fork+0x3a/0x50
> Modules linked in:
> CPU: 3 PID: 271 Comm: bash Not tainted 5.3.0-rc4-2.g07a1a73-default+ #57
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.1-0-ga5cab58-prebuilt.qemu.org 04/01/2014
> Call Trace:
> dump_stack+0x85/0xc0
> bad_page.cold+0xba/0xbf
> rmqueue_pcplist.isra.0+0x6c5/0x6d0
> rmqueue+0x2d/0x810
> get_page_from_freelist+0x191/0x380
> __alloc_pages_nodemask+0x13c/0x2d0
> __get_free_pages+0xd/0x30
> __pud_alloc+0x2c/0x110
> copy_page_range+0x4f9/0x630
> dup_mmap+0x362/0x480
> dup_mm+0x68/0x110
> copy_process+0x19e1/0x1b40
> _do_fork+0x73/0x310
> __x64_sys_clone+0x75/0x80
> do_syscall_64+0x6e/0x1e0
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x7f10af854a10
> ...
>
> Signed-off-by: Vlastimil Babka <vbabka@xxxxxxx>
> ---
> .../admin-guide/kernel-parameters.txt | 2 +
> mm/Kconfig.debug | 4 +-
> mm/page_owner.c | 53 ++++++++++++++-----
> 3 files changed, 45 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 47d981a86e2f..e813a17d622e 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -809,6 +809,8 @@
> enables the feature at boot time. By default, it is
> disabled and the system will work mostly the same as a
> kernel built without CONFIG_DEBUG_PAGEALLOC.
> + Note: to get most of debug_pagealloc error reports, it's
> + useful to also enable the page_owner functionality.
> on: enable the feature
>
> debugpat [X86] Enable PAT debugging
> diff --git a/mm/Kconfig.debug b/mm/Kconfig.debug
> index 82b6a20898bd..327b3ebf23bf 100644
> --- a/mm/Kconfig.debug
> +++ b/mm/Kconfig.debug
> @@ -21,7 +21,9 @@ config DEBUG_PAGEALLOC
> Also, the state of page tracking structures is checked more often as
> pages are being allocated and freed, as unexpected state changes
> often happen for same reasons as memory corruption (e.g. double free,
> - use-after-free).
> + use-after-free). The error reports for these checks can be augmented
> + with stack traces of last allocation and freeing of the page, when
> + PAGE_OWNER is also selected and enabled on boot.
>
> For architectures which don't enable ARCH_SUPPORTS_DEBUG_PAGEALLOC,
> fill the pages with poison patterns after free_pages() and verify
> diff --git a/mm/page_owner.c b/mm/page_owner.c
> index 4a48e018dbdf..dee931184788 100644
> --- a/mm/page_owner.c
> +++ b/mm/page_owner.c
> @@ -24,6 +24,9 @@ struct page_owner {
> short last_migrate_reason;
> gfp_t gfp_mask;
> depot_stack_handle_t handle;
> +#ifdef CONFIG_DEBUG_PAGEALLOC
> + depot_stack_handle_t free_handle;
> +#endif
I think it's possible to add space for the second stack handle at runtime:
adjust page_owner_ops->size inside the ->need(). The second stack might be
useful beyond CONFIG_DEBUG_PAGEALLOC. We probably should not tie these
features.
--
Kirill A. Shutemov
