mbox series

[v6,0/4] page_owner: print stacks and their counter

Message ID 20231120084300.4368-1-osalvador@suse.de
Headers
Series page_owner: print stacks and their counter |

Message

Oscar Salvador Nov. 20, 2023, 8:42 a.m. UTC 
  Changes v5 -> v6:
     - Rebase on top of v6.7-rc1
     - Move stack_record struct to the header
     - Addressed feedback from Vlastimil
       (some code tweaks and changelogs suggestions)

Changes v4 -> v5:
     - Addressed feedback from Alexander Potapenko

Changes v3 -> v4:
     - Rebase (long time has passed)
     - Use boolean instead of enum for action by Alexander Potapenko
     - (I left some feedback untouched because it's been long and
        would like to discuss it here now instead of re-vamping
        and old thread)

Changes v2 -> v3:
     - Replace interface in favor of seq operations (suggested by Vlastimil)
     - Use debugfs interface to store/read valued (suggested by Ammar)

Hi,

page_owner is a great debug functionality tool that lets us know
about all pages that have been allocated/freed and their stacktrace.
This comes very handy when one has to debug memory leaks, since with
some scripting we can see outstanding allocations, which might end up
pointing to a leak.

In my experience, that is one of the most useful cases, but it can get
really tedious to screen through all pages and try to reconstruct the
stack <-> allocated/freed relationship. There is a lot of noise
to cancel off.

This patchset aims to ease that by adding a new functionality into page_owner.
What this does is to create a new read-only file "page_owner_stacks",
which prints out only the stacktraces followed by their outstanding
number of allocations (being that the times the stacktrace has allocated
but not freed yet).

This lets us have a clear overview of stacks <-> allocated/freed relationship
without the need to fiddle with pages and trying to match free stacktraces
with allocated stacktraces.

This is achieved by adding a new refcount_t field in the stack_record struct,
incrementing that refcount_t everytime the same stacktrace allocates,
and decrementing it when it frees a page. Details can be seen in the
respective patches.

We also create another file called "page_owner_threshold", which let us
specify a threshold, so when reading from "page_owner_stacks",
we will only see those stacktraces which counting goes beyond the
threshold we specified.

A PoC can be found below:

 # cat /sys/kernel/debug/page_owner_threshold
  0
 # cat /sys/kernel/debug/page_owner_stacks > stacks_full.txt
 # head -32 stacks_full.txt
  prep_new_page+0x10d/0x180
  get_page_from_freelist+0x1bd6/0x1e10
  __alloc_pages+0x194/0x360
  alloc_page_interleave+0x13/0x90
  new_slab+0x31d/0x530
  ___slab_alloc+0x5d7/0x720
  __slab_alloc.isra.85+0x4a/0x90
  kmem_cache_alloc+0x455/0x4a0
  acpi_ps_alloc_op+0x57/0x8f
  acpi_ps_create_scope_op+0x12/0x23
  acpi_ps_execute_method+0x102/0x2c1
  acpi_ns_evaluate+0x343/0x4da
  acpi_evaluate_object+0x1cb/0x392
  acpi_run_osc+0x135/0x260
  acpi_init+0x165/0x4ed
  do_one_initcall+0x3e/0x200
 stack count: 2
 
  free_pcp_prepare+0x287/0x5c0
  free_unref_page+0x1c/0xd0
  __mmdrop+0x50/0x160
  finish_task_switch+0x249/0x2b0
  __schedule+0x2c3/0x960
  schedule+0x44/0xb0
  futex_wait_queue+0x70/0xd0
  futex_wait+0x160/0x250
  do_futex+0x11c/0x1b0
  __x64_sys_futex+0x5e/0x1d0
  do_syscall_64+0x37/0x90
  entry_SYSCALL_64_after_hwframe+0x63/0xcd
 stack count: 1
 
  
 
 # echo 10000 > /sys/kernel/debug/page_owner_threshold
 # cat /sys/kernel/debug/page_owner_stacks > stacks_10000.txt
 # cat stacks_10000.txt 
  prep_new_page+0x10d/0x180
  get_page_from_freelist+0x1bd6/0x1e10
  __alloc_pages+0x194/0x360
  folio_alloc+0x17/0x40
  page_cache_ra_unbounded+0x96/0x170
  filemap_get_pages+0x23d/0x5e0
  filemap_read+0xbf/0x3a0
  __kernel_read+0x136/0x2f0
  kernel_read_file+0x197/0x2d0
  kernel_read_file_from_fd+0x54/0x90
  __do_sys_finit_module+0x89/0x120
  do_syscall_64+0x37/0x90
  entry_SYSCALL_64_after_hwframe+0x63/0xcd
 stack count: 36195
 
  prep_new_page+0x10d/0x180
  get_page_from_freelist+0x1bd6/0x1e10
  __alloc_pages+0x194/0x360
  folio_alloc+0x17/0x40
  page_cache_ra_unbounded+0x96/0x170
  filemap_get_pages+0x23d/0x5e0
  filemap_read+0xbf/0x3a0
  new_sync_read+0x106/0x180
  vfs_read+0x16f/0x190
  ksys_read+0xa5/0xe0
  do_syscall_64+0x37/0x90
  entry_SYSCALL_64_after_hwframe+0x63/0xcd
 stack count: 44484
 
  prep_new_page+0x10d/0x180
  get_page_from_freelist+0x1bd6/0x1e10
  __alloc_pages+0x194/0x360
  folio_alloc+0x17/0x40
  page_cache_ra_unbounded+0x96/0x170
  filemap_get_pages+0xdd/0x5e0
  filemap_read+0xbf/0x3a0
  new_sync_read+0x106/0x180
  vfs_read+0x16f/0x190
  ksys_read+0xa5/0xe0
  do_syscall_64+0x37/0x90
  entry_SYSCALL_64_after_hwframe+0x63/0xcd
 stack count: 17874

Oscar Salvador (4):
  lib/stackdepot: Add a refcount field in stack_record
  lib/stackdepot: Move stack_record struct definition into the header
  mm,page_owner: Add page_owner_stacks file to print out only stacks and
    their counter
  mm,page_owner: Filter out stacks by a threshold counter

 include/linux/stackdepot.h |  38 ++++++++++++
 lib/stackdepot.c           | 114 ++++++++++++++++++++++--------------
 mm/page_owner.c            | 115 +++++++++++++++++++++++++++++++++++++
 3 files changed, 224 insertions(+), 43 deletions(-)

Comments

Vlastimil Babka Nov. 20, 2023, 9:07 a.m. UTC | #1 
On 11/20/23 09:42, Oscar Salvador wrote:
> Changes v5 -> v6:
>      - Rebase on top of v6.7-rc1

Hi,

I think at this point it would be better to rebase on top of
https://lore.kernel.org/all/cover.1698077459.git.andreyknvl@google.com/
which already contains the refcount field in stack_record.
(and maybe help with the review of that series as well? Hopefully it can get
to mm/unstable and -next towards 6.8 soon :)

Vlastimil
Andrey Konovalov Nov. 20, 2023, 5:54 p.m. UTC | #2 
On Mon, Nov 20, 2023 at 10:07 AM Vlastimil Babka <vbabka@suse.cz> wrote:
>
> On 11/20/23 09:42, Oscar Salvador wrote:
> > Changes v5 -> v6:
> >      - Rebase on top of v6.7-rc1
>
> Hi,
>
> I think at this point it would be better to rebase on top of
> https://lore.kernel.org/all/cover.1698077459.git.andreyknvl@google.com/
> which already contains the refcount field in stack_record.
> (and maybe help with the review of that series as well? Hopefully it can get
> to mm/unstable and -next towards 6.8 soon :)

Yes, please; I just mailed a v4 that is based on v6.7-rc2:
https://lore.kernel.org/linux-mm/cover.1700502145.git.andreyknvl@google.com/T/#t

You should also be able to use the new
stack_depot_save_flags(STACK_DEPOT_FLAG_GET)/stack_depot_put API to
keep only the relevant stack traces.

Thank you!