2024-02-15 21:59:09

by Oscar Salvador

[permalink] [raw]
Subject: [PATCH v10 0/7] page_owner: print stacks and their outstanding allocations

Changes v9 -> v10
- Fix unwanted change in patch#2
- Collect Acked-by and Reviewed-by from Marco and Vlastimil
for the missing patches
- Fix stack_record count by substracting 1 in stack_print by Vlastimil

Changes v8 -> v9
- Fix handle-0 for the very first stack_record entry
- Collect Acked-by and Reviewed-by from Marco and Vlastimil
- Adressed feedback from Marco and Vlastimil
- stack_print() no longer allocates a memory buffer, prints directly
using seq_printf: by Vlastimil
- Added two static struct stack for dummy_handle and faiure_handle
- add_stack_record_to_list() now filters out the gfp_mask the same way
stackdepot does, for consistency
- Rename set_threshold to count_threshold

Changes v7 -> v8
- Rebased on top of -next
- page_owner maintains its own stack_records list now
- Kill auxiliary stackdepot function to traverse buckets
- page_owner_stacks is now a directory with 'show_stacks'
and 'set_threshold'
- Update Documentation/mm/page_owner.rst
- Adressed feedback from Marco

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 directory called 'page_owner_stacks'
under 'sys/kernel//debug' with a read-only file called 'show_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 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 'set_threshold' within
'page_owner_stacks' directory, and by writing a value to it, the stacks
which refcount is below such value will be filtered out.

A PoC can be found below:

# cat /sys/kernel/debug/page_owner_stacks/show_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_stacks/set_threshold
# cat /sys/kernel/debug/page_owner_stacks/show_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 (7):
lib/stackdepot: Fix first entry having a 0-handle
lib/stackdepot: Move stack_record struct definition into the header
mm,page_owner: Maintain own list of stack_records structs
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
mm,page_owner: Update Documentation regarding page_owner_stacks

Documentation/mm/page_owner.rst | 45 +++++++
include/linux/stackdepot.h | 58 +++++++++
lib/stackdepot.c | 65 +++--------
mm/page_owner.c | 200 +++++++++++++++++++++++++++++++-
4 files changed, 318 insertions(+), 50 deletions(-)

--
2.43.0



2024-02-15 21:59:10

by Oscar Salvador

[permalink] [raw]
Subject: [PATCH v10 5/7] mm,page_owner: Display all stacks and their count

This patch adds a new directory called 'page_owner_stacks' under
/sys/kernel/debug/, with a file called 'show_stacks' in it.
Reading from that file will show all stacks that were added by page_owner
followed by their counting, giving us a clear overview of stack <-> count
relationship.

E.g:

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: 4578

The seq stack_{start,next} functions will iterate through the list
stack_list in order to print all stacks.

Signed-off-by: Oscar Salvador <[email protected]>
Acked-by: Marco Elver <[email protected]>
Reviewed-by: Vlastimil Babka <[email protected]>
---
mm/page_owner.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 92 insertions(+), 1 deletion(-)

diff --git a/mm/page_owner.c b/mm/page_owner.c
index df6a923af5de..e99fbf822dd6 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -171,7 +171,13 @@ static void add_stack_record_to_list(struct stack_record *stack_record,

spin_lock_irqsave(&stack_list_lock, flags);
stack->next = stack_list;
- stack_list = stack;
+ /*
+ * This pairs with smp_load_acquire() from function
+ * stack_start(). This guarantees that stack_start()
+ * will see an updated stack_list before starting to
+ * traverse the list.
+ */
+ smp_store_release(&stack_list, stack);
spin_unlock_irqrestore(&stack_list_lock, flags);
}

@@ -805,8 +811,90 @@ static const struct file_operations proc_page_owner_operations = {
.llseek = lseek_page_owner,
};

+static void *stack_start(struct seq_file *m, loff_t *ppos)
+{
+ struct stack *stack;
+
+ if (*ppos == -1UL)
+ return NULL;
+
+ if (!*ppos) {
+ /*
+ * This pairs with smp_store_release() from function
+ * add_stack_record_to_list(), so we get a consistent
+ * value of stack_list.
+ */
+ stack = smp_load_acquire(&stack_list);
+ } else {
+ stack = m->private;
+ stack = stack->next;
+ }
+
+ m->private = stack;
+
+ return stack;
+}
+
+static void *stack_next(struct seq_file *m, void *v, loff_t *ppos)
+{
+ struct stack *stack = v;
+
+ stack = stack->next;
+ *ppos = stack ? *ppos + 1 : -1UL;
+ m->private = stack;
+
+ return stack;
+}
+
+static int stack_print(struct seq_file *m, void *v)
+{
+ int i, stack_count;
+ struct stack *stack = v;
+ unsigned long *entries;
+ unsigned long nr_entries;
+ struct stack_record *stack_record = stack->stack_record;
+
+ nr_entries = stack_record->size;
+ entries = stack_record->entries;
+ stack_count = refcount_read(&stack_record->count) - 1;
+
+ if (!nr_entries || nr_entries < 0 || stack_count < 1)
+ return 0;
+
+ for (i = 0; i < nr_entries; i++)
+ seq_printf(m, " %pS\n", (void *)entries[i]);
+ seq_printf(m, "stack_count: %d\n\n", stack_count);
+
+ return 0;
+}
+
+static void stack_stop(struct seq_file *m, void *v)
+{
+}
+
+static const struct seq_operations page_owner_stack_op = {
+ .start = stack_start,
+ .next = stack_next,
+ .stop = stack_stop,
+ .show = stack_print
+};
+
+static int page_owner_stack_open(struct inode *inode, struct file *file)
+{
+ return seq_open_private(file, &page_owner_stack_op, 0);
+}
+
+static const struct file_operations page_owner_stack_operations = {
+ .open = page_owner_stack_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = seq_release,
+};
+
static int __init pageowner_init(void)
{
+ struct dentry *dir;
+
if (!static_branch_unlikely(&page_owner_inited)) {
pr_info("page_owner is disabled\n");
return 0;
@@ -814,6 +902,9 @@ static int __init pageowner_init(void)

debugfs_create_file("page_owner", 0400, NULL, NULL,
&proc_page_owner_operations);
+ dir = debugfs_create_dir("page_owner_stacks", NULL);
+ debugfs_create_file("show_stacks", 0400, dir, NULL,
+ &page_owner_stack_operations);

return 0;
}
--
2.43.0


2024-02-15 21:59:30

by Oscar Salvador

[permalink] [raw]
Subject: [PATCH v10 6/7] mm,page_owner: Filter out stacks by a threshold

We want to be able to filter out the stacks based on a threshold we can
can tune.
By writing to 'count_threshold' file, we can adjust the threshold value.

Signed-off-by: Oscar Salvador <[email protected]>
Reviewed-by: Vlastimil Babka <[email protected]>
---
mm/page_owner.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/mm/page_owner.c b/mm/page_owner.c
index e99fbf822dd6..e56c1e92eccf 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -846,6 +846,8 @@ static void *stack_next(struct seq_file *m, void *v, loff_t *ppos)
return stack;
}

+static unsigned long page_owner_stack_threshold;
+
static int stack_print(struct seq_file *m, void *v)
{
int i, stack_count;
@@ -858,7 +860,8 @@ static int stack_print(struct seq_file *m, void *v)
entries = stack_record->entries;
stack_count = refcount_read(&stack_record->count) - 1;

- if (!nr_entries || nr_entries < 0 || stack_count < 1)
+ if (!nr_entries || nr_entries < 0 || stack_count < 1 ||
+ stack_count < page_owner_stack_threshold)
return 0;

for (i = 0; i < nr_entries; i++)
@@ -891,6 +894,22 @@ static const struct file_operations page_owner_stack_operations = {
.release = seq_release,
};

+static int page_owner_threshold_get(void *data, u64 *val)
+{
+ *val = READ_ONCE(page_owner_stack_threshold);
+ return 0;
+}
+
+static int page_owner_threshold_set(void *data, u64 val)
+{
+ WRITE_ONCE(page_owner_stack_threshold, val);
+ return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(proc_page_owner_threshold, &page_owner_threshold_get,
+ &page_owner_threshold_set, "%llu");
+
+
static int __init pageowner_init(void)
{
struct dentry *dir;
@@ -905,6 +924,8 @@ static int __init pageowner_init(void)
dir = debugfs_create_dir("page_owner_stacks", NULL);
debugfs_create_file("show_stacks", 0400, dir, NULL,
&page_owner_stack_operations);
+ debugfs_create_file("count_threshold", 0600, dir, NULL,
+ &proc_page_owner_threshold);

return 0;
}
--
2.43.0


2024-02-15 21:59:59

by Oscar Salvador

[permalink] [raw]
Subject: [PATCH v10 7/7] mm,page_owner: Update Documentation regarding page_owner_stacks

Update page_owner documentation including the new page_owner_stacks
feature to show how it can be used.

Signed-off-by: Oscar Salvador <[email protected]>
Reviewed-by: Vlastimil Babka <[email protected]>
Reviewed-by: Marco Elver <[email protected]>
---
Documentation/mm/page_owner.rst | 45 +++++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)

diff --git a/Documentation/mm/page_owner.rst b/Documentation/mm/page_owner.rst
index 62e3f7ab23cc..0d0334cd5179 100644
--- a/Documentation/mm/page_owner.rst
+++ b/Documentation/mm/page_owner.rst
@@ -24,6 +24,11 @@ fragmentation statistics can be obtained through gfp flag information of
each page. It is already implemented and activated if page owner is
enabled. Other usages are more than welcome.

+It can also be used to show all the stacks and their outstanding
+allocations, which gives us a quick overview of where the memory is going
+without the need to screen through all the pages and match the allocation
+and free operation.
+
page owner is disabled by default. So, if you'd like to use it, you need
to add "page_owner=on" to your boot cmdline. If the kernel is built
with page owner and page owner is disabled in runtime due to not enabling
@@ -68,6 +73,46 @@ Usage

4) Analyze information from page owner::

+ cat /sys/kernel/debug/page_owner_stacks/show_stacks > stacks.txt
+ cat stacks.txt
+ prep_new_page+0xa9/0x120
+ get_page_from_freelist+0x7e6/0x2140
+ __alloc_pages+0x18a/0x370
+ new_slab+0xc8/0x580
+ ___slab_alloc+0x1f2/0xaf0
+ __slab_alloc.isra.86+0x22/0x40
+ kmem_cache_alloc+0x31b/0x350
+ __khugepaged_enter+0x39/0x100
+ dup_mmap+0x1c7/0x5ce
+ copy_process+0x1afe/0x1c90
+ kernel_clone+0x9a/0x3c0
+ __do_sys_clone+0x66/0x90
+ do_syscall_64+0x7f/0x160
+ entry_SYSCALL_64_after_hwframe+0x6c/0x74
+ stack_count: 234
+ ...
+ ...
+ echo 7000 > /sys/kernel/debug/page_owner_stacks/count_threshold
+ cat /sys/kernel/debug/page_owner_stacks/show_stacks> stacks_7000.txt
+ cat stacks_7000.txt
+ prep_new_page+0xa9/0x120
+ get_page_from_freelist+0x7e6/0x2140
+ __alloc_pages+0x18a/0x370
+ alloc_pages_mpol+0xdf/0x1e0
+ folio_alloc+0x14/0x50
+ filemap_alloc_folio+0xb0/0x100
+ page_cache_ra_unbounded+0x97/0x180
+ filemap_fault+0x4b4/0x1200
+ __do_fault+0x2d/0x110
+ do_pte_missing+0x4b0/0xa30
+ __handle_mm_fault+0x7fa/0xb70
+ handle_mm_fault+0x125/0x300
+ do_user_addr_fault+0x3c9/0x840
+ exc_page_fault+0x68/0x150
+ asm_exc_page_fault+0x22/0x30
+ stack_count: 8248
+ ...
+
cat /sys/kernel/debug/page_owner > page_owner_full.txt
./page_owner_sort page_owner_full.txt sorted_page_owner.txt

--
2.43.0


2024-02-15 23:37:33

by Andrey Konovalov

[permalink] [raw]
Subject: Re: [PATCH v10 0/7] page_owner: print stacks and their outstanding allocations

On Thu, Feb 15, 2024 at 10:57 PM Oscar Salvador <[email protected]> wrote:
>
> Changes v9 -> v10
> - Fix unwanted change in patch#2
> - Collect Acked-by and Reviewed-by from Marco and Vlastimil
> for the missing patches
> - Fix stack_record count by substracting 1 in stack_print by Vlastimil
>
> Changes v8 -> v9
> - Fix handle-0 for the very first stack_record entry
> - Collect Acked-by and Reviewed-by from Marco and Vlastimil
> - Adressed feedback from Marco and Vlastimil
> - stack_print() no longer allocates a memory buffer, prints directly
> using seq_printf: by Vlastimil
> - Added two static struct stack for dummy_handle and faiure_handle
> - add_stack_record_to_list() now filters out the gfp_mask the same way
> stackdepot does, for consistency
> - Rename set_threshold to count_threshold
>
> Changes v7 -> v8
> - Rebased on top of -next
> - page_owner maintains its own stack_records list now
> - Kill auxiliary stackdepot function to traverse buckets
> - page_owner_stacks is now a directory with 'show_stacks'
> and 'set_threshold'
> - Update Documentation/mm/page_owner.rst
> - Adressed feedback from Marco
>
> 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 directory called 'page_owner_stacks'
> under 'sys/kernel//debug' with a read-only file called 'show_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 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 'set_threshold' within
> 'page_owner_stacks' directory, and by writing a value to it, the stacks
> which refcount is below such value will be filtered out.
>
> A PoC can be found below:
>
> # cat /sys/kernel/debug/page_owner_stacks/show_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_stacks/set_threshold
> # cat /sys/kernel/debug/page_owner_stacks/show_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 (7):
> lib/stackdepot: Fix first entry having a 0-handle
> lib/stackdepot: Move stack_record struct definition into the header
> mm,page_owner: Maintain own list of stack_records structs
> 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
> mm,page_owner: Update Documentation regarding page_owner_stacks
>
> Documentation/mm/page_owner.rst | 45 +++++++
> include/linux/stackdepot.h | 58 +++++++++
> lib/stackdepot.c | 65 +++--------
> mm/page_owner.c | 200 +++++++++++++++++++++++++++++++-
> 4 files changed, 318 insertions(+), 50 deletions(-)
>
> --
> 2.43.0
>

Acked-by: Andrey Konovalov <[email protected]>

2024-03-06 14:23:45

by Kefeng Wang

[permalink] [raw]
Subject: Re: [PATCH v10 6/7] mm,page_owner: Filter out stacks by a threshold

Hi

On 2024/2/16 5:59, Oscar Salvador wrote:
> We want to be able to filter out the stacks based on a threshold we can
> can tune.
> By writing to 'count_threshold' file, we can adjust the threshold value.
>
> Signed-off-by: Oscar Salvador <[email protected]>
> Reviewed-by: Vlastimil Babka <[email protected]>
> ---
> mm/page_owner.c | 23 ++++++++++++++++++++++-
> 1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/mm/page_owner.c b/mm/page_owner.c
> index e99fbf822dd6..e56c1e92eccf 100644
> --- a/mm/page_owner.c
> +++ b/mm/page_owner.c
> @@ -846,6 +846,8 @@ static void *stack_next(struct seq_file *m, void *v, loff_t *ppos)
> return stack;
> }
>
> +static unsigned long page_owner_stack_threshold;
> +
> static int stack_print(struct seq_file *m, void *v)
> {
> int i, stack_count;
> @@ -858,7 +860,8 @@ static int stack_print(struct seq_file *m, void *v)
> entries = stack_record->entries;
> stack_count = refcount_read(&stack_record->count) - 1;
>
> - if (!nr_entries || nr_entries < 0 || stack_count < 1)
> + if (!nr_entries || nr_entries < 0 || stack_count < 1 ||
> + stack_count < page_owner_stack_threshold)
> return 0;
>
> for (i = 0; i < nr_entries; i++)
> @@ -891,6 +894,22 @@ static const struct file_operations page_owner_stack_operations = {
> .release = seq_release,
> };
>
> +static int page_owner_threshold_get(void *data, u64 *val)
> +{
> + *val = READ_ONCE(page_owner_stack_threshold);
> + return 0;
> +}
> +
> +static int page_owner_threshold_set(void *data, u64 val)
> +{
> + WRITE_ONCE(page_owner_stack_threshold, val);
> + return 0;
> +}

no need READ/WRITE_ONCE between get/set, since simple_attr_read/write()
with a mutex, but set and stack_print could date race
> +
> +DEFINE_SIMPLE_ATTRIBUTE(proc_page_owner_threshold, &page_owner_threshold_get,
> + &page_owner_threshold_set, "%llu");

%llu\n"

In addition, since we have a stack_list, could we add a new feature to
dump stack info with threshold with OOM notifier, because many users
directly call alloc_page family, which makes it too difficult to debug
during OOM.

Thanks

> +
> +
> static int __init pageowner_init(void)
> {
> struct dentry *dir;
> @@ -905,6 +924,8 @@ static int __init pageowner_init(void)
> dir = debugfs_create_dir("page_owner_stacks", NULL);
> debugfs_create_file("show_stacks", 0400, dir, NULL,
> &page_owner_stack_operations);
> + debugfs_create_file("count_threshold", 0600, dir, NULL,
> + &proc_page_owner_threshold);
>
> return 0;
> }