[v7,0/4] page_owner: print stacks and their outstanding allocations

  Changes v6 -> v7:
     - Rebased on top of Andrey Konovalov's libstackdepot patchset
     - Reformulated the changelogs

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)


page_owner is a great debug functionality tool that lets us know
about all pages that have been allocated/freed and their specific
stacktrace.
This comes very handy when debugging memory leaks, since with
some scripting we can see the outstanding allocations, which might point
to a memory 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, becoming most of the time a
daunting and slow process when we have tons of allocation/free operations. 

This patchset aims to ease that by adding a new functionality into
page_owner.
This functionality creates a new read-only file called "page_owner_stacks",
which prints out all the stacks followed by their outstanding number
of allocations (being that the times the stacktrace has allocated
but not freed yet).
This gives us a clear and a quick overview of stacks <-> allocated/free.

We take advantage of the new refcount_f field that stack_record struct
gained, and increment/decrement the stack refcount on every
__set_page_owner() (alloc operation) and __reset_page_owner (free operation)
call.

Unfortunately, we cannot use the new stackdepot api
STACK_DEPOT_FLAG_{GET,PUT} because it does not fulfill page_owner needs,
meaning we would have to special case things, at which point
makes more sense for page_owner to do its own {dec,inc}rementing
of the stacks.
E.g: Using STACK_DEPOT_FLAG_PUT, once the refcount reaches 0,
such stack gets evicted, so page_owner would lose information.

This patch also creates a new file called 'page_owner_threshold'.
By writing a value to it, the stacks which refcount is below such
value will be filtered out.

In order to better exercise the path in stack_depot_get_next_stack(),
I artificially filled the buckets with more than one stack, making sure
I was getting all of then when reading from it.

On a side note, stack_depot_get_next_stack() could be somehow reconstructed
to be in page_owner code, but we would have to move stack_table
into the header, so page_owner can access it.
I can do that if that's preferred, so stackdepot.c would not get "poluted".

A PoC can be found below:

 # cat /sys/kernel/debug/page_owner_stacks > page_owner_full_stacks.txt
 # head -40 page_owner_full_stacks.txt 
  prep_new_page+0xa9/0x120
  get_page_from_freelist+0x801/0x2210
  __alloc_pages+0x18b/0x350
  alloc_pages_mpol+0x91/0x1f0
  folio_alloc+0x14/0x50
  filemap_alloc_folio+0xb2/0x100
  page_cache_ra_unbounded+0x96/0x180
  filemap_get_pages+0xfd/0x590
  filemap_read+0xcc/0x330
  blkdev_read_iter+0xb8/0x150
  vfs_read+0x285/0x320
  ksys_read+0xa5/0xe0
  do_syscall_64+0x80/0x160
  entry_SYSCALL_64_after_hwframe+0x6e/0x76
 stack_count: 521



  prep_new_page+0xa9/0x120
  get_page_from_freelist+0x801/0x2210
  __alloc_pages+0x18b/0x350
  alloc_pages_mpol+0x91/0x1f0
  folio_alloc+0x14/0x50
  filemap_alloc_folio+0xb2/0x100
  __filemap_get_folio+0x14a/0x490
  ext4_write_begin+0xbd/0x4b0 [ext4]
  generic_perform_write+0xc1/0x1e0
  ext4_buffered_write_iter+0x68/0xe0 [ext4]
  ext4_file_write_iter+0x70/0x740 [ext4]
  vfs_write+0x33d/0x420
  ksys_write+0xa5/0xe0
  do_syscall_64+0x80/0x160
  entry_SYSCALL_64_after_hwframe+0x6e/0x76
 stack_count: 4609
..
..

 # echo 5000 > /sys/kernel/debug/page_owner_threshold 
 # cat /sys/kernel/debug/page_owner_stacks > page_owner_full_stacks_5000.txt
 # head -40 page_owner_full_stacks_5000.txt 
  prep_new_page+0xa9/0x120
  get_page_from_freelist+0x801/0x2210
  __alloc_pages+0x18b/0x350
  alloc_pages_mpol+0x91/0x1f0
  folio_alloc+0x14/0x50
  filemap_alloc_folio+0xb2/0x100
  __filemap_get_folio+0x14a/0x490
  ext4_write_begin+0xbd/0x4b0 [ext4]
  generic_perform_write+0xc1/0x1e0
  ext4_buffered_write_iter+0x68/0xe0 [ext4]
  ext4_file_write_iter+0x70/0x740 [ext4]
  vfs_write+0x33d/0x420
  ksys_pwrite64+0x75/0x90
  do_syscall_64+0x80/0x160
  entry_SYSCALL_64_after_hwframe+0x6e/0x76
 stack_count: 6781



  prep_new_page+0xa9/0x120
  get_page_from_freelist+0x801/0x2210
  __alloc_pages+0x18b/0x350
  pcpu_populate_chunk+0xec/0x350
  pcpu_balance_workfn+0x2d1/0x4a0
  process_scheduled_works+0x84/0x380
  worker_thread+0x12a/0x2a0
  kthread+0xe3/0x110
  ret_from_fork+0x30/0x50
  ret_from_fork_asm+0x1b/0x30
 stack_count: 8641

Oscar Salvador (4):
  lib/stackdepot: Move stack_record struct definition into the header
  mm,page_owner: Implement the tracking of the stacks count
  mm,page_owner: Display all stacks and their count
  mm,page_owner: Filter out stacks by a threshold

 include/linux/stackdepot.h |  72 ++++++++++++++++++++
 lib/stackdepot.c           |  97 ++++++++++++++------------
 mm/page_owner.c            | 136 +++++++++++++++++++++++++++++++++++++
 3 files changed, 262 insertions(+), 43 deletions(-)
  

On Fri,  9 Feb 2024 00:45:35 +0100 Oscar Salvador <osalvador@suse.de> wrote:

> page_owner is a great debug functionality tool that lets us know
> about all pages that have been allocated/freed and their specific
> stacktrace.
> This comes very handy when debugging memory leaks, since with
> some scripting we can see the outstanding allocations, which might point
> to a memory 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, becoming most of the time a
> daunting and slow process when we have tons of allocation/free operations. 
> 
> This patchset aims to ease that by adding a new functionality into
> page_owner.
> This functionality creates a new read-only file called "page_owner_stacks",

The full path would be appreciated.

> which prints out all the stacks followed by their outstanding number
> of allocations (being that the times the stacktrace has allocated
> but not freed yet).
> This gives us a clear and a quick overview of stacks <-> allocated/free.
> 
> We take advantage of the new refcount_f field that stack_record struct
> gained, and increment/decrement the stack refcount on every
> __set_page_owner() (alloc operation) and __reset_page_owner (free operation)
> call.
> 
> Unfortunately, we cannot use the new stackdepot api
> STACK_DEPOT_FLAG_{GET,PUT} because it does not fulfill page_owner needs,
> meaning we would have to special case things, at which point
> makes more sense for page_owner to do its own {dec,inc}rementing
> of the stacks.
> E.g: Using STACK_DEPOT_FLAG_PUT, once the refcount reaches 0,
> such stack gets evicted, so page_owner would lose information.
> 
> This patch also creates a new file called 'page_owner_threshold'.
> By writing a value to it, the stacks which refcount is below such
> value will be filtered out.
> 
> In order to better exercise the path in stack_depot_get_next_stack(),
> I artificially filled the buckets with more than one stack, making sure
> I was getting all of then when reading from it.
> 
> On a side note, stack_depot_get_next_stack() could be somehow reconstructed
> to be in page_owner code, but we would have to move stack_table
> into the header, so page_owner can access it.
> I can do that if that's preferred, so stackdepot.c would not get "poluted".
> 
> A PoC can be found below:
> 
>  # cat /sys/kernel/debug/page_owner_stacks > page_owner_full_stacks.txt

Oh, there it is.  I wonder why we didn't use /sys/kernel/mm/

Would a new /sys/kernel/debug/page_owner_something/ be a good idea?  We
might add more things later.  Then it can be
/sys/kernel/debug/page_owner_something/full_stacks. 
/sys/kernel/debug/page_owner/ would be nice, but it's too late for
that.

> Oscar Salvador (4):
>   lib/stackdepot: Move stack_record struct definition into the header
>   mm,page_owner: Implement the tracking of the stacks count
>   mm,page_owner: Display all stacks and their count
>   mm,page_owner: Filter out stacks by a threshold
> 
>  include/linux/stackdepot.h |  72 ++++++++++++++++++++
>  lib/stackdepot.c           |  97 ++++++++++++++------------
>  mm/page_owner.c            | 136 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 262 insertions(+), 43 deletions(-)

Documentation/mm/page_owner.rst?
  

On Fri, 9 Feb 2024 at 00:45, Oscar Salvador <osalvador@suse.de> wrote:
>
> Changes v6 -> v7:
>      - Rebased on top of Andrey Konovalov's libstackdepot patchset
>      - Reformulated the changelogs

Overall looks fine, but I think it's not rebased against the latest
stackdepot version in -next. That version re-introduces variable-sized
records and there's some trickery required so you do not end up with
refcount warnings.
  

On Thu, Feb 08, 2024 at 04:28:18PM -0800, Andrew Morton wrote:
> Oh, there it is.  I wonder why we didn't use /sys/kernel/mm/

I just followed the convention we use at the moment, which happens
to be /sys/kernel/debug/page_owner_xxx.

> Would a new /sys/kernel/debug/page_owner_something/ be a good idea?  We
> might add more things later.  Then it can be
> /sys/kernel/debug/page_owner_something/full_stacks. 
> /sys/kernel/debug/page_owner/ would be nice, but it's too late for
> that.

Well, we could certainly do that, so we do not scatter the files
in /sys/kernel/debug/ but rather gather them in pagE_owner directory.
Let me see if I can come up with a proper name.

> > Oscar Salvador (4):
> >   lib/stackdepot: Move stack_record struct definition into the header
> >   mm,page_owner: Implement the tracking of the stacks count
> >   mm,page_owner: Display all stacks and their count
> >   mm,page_owner: Filter out stacks by a threshold
> > 
> >  include/linux/stackdepot.h |  72 ++++++++++++++++++++
> >  lib/stackdepot.c           |  97 ++++++++++++++------------
> >  mm/page_owner.c            | 136 +++++++++++++++++++++++++++++++++++++
> >  3 files changed, 262 insertions(+), 43 deletions(-)
> 
> Documentation/mm/page_owner.rst?

Heh, we are definitely going to need some documentation.
Let me prepare something.

Thanks Andrew
  

On Fri, Feb 09, 2024 at 09:03:21AM +0100, Marco Elver wrote:
> On Fri, 9 Feb 2024 at 00:45, Oscar Salvador <osalvador@suse.de> wrote:
> >
> > Changes v6 -> v7:
> >      - Rebased on top of Andrey Konovalov's libstackdepot patchset
> >      - Reformulated the changelogs
> 
> Overall looks fine, but I think it's not rebased against the latest
> stackdepot version in -next. That version re-introduces variable-sized
> records and there's some trickery required so you do not end up with
> refcount warnings.

Thanks a lot Marco for having a look and for the feedback, that's highly
appreciated.