Re: [PATCH v3 2/4] mm/page_owner: add print_mode filter

[SeongJae Park]

On Tue, 28 Apr 2026 15:11:10 +0800 Zhen Ni <zhen.ni@xxxxxxxxxxxx> wrote:

> Add print_mode functionality to reduce page_owner output size by
> printing only the stack handle instead of the full stack trace.
> 
> Example output with print_mode enabled:
>   Page allocated via order 0, mask 0x42800(GFP_NOWAIT|__GFP_COMP),
>   pid 1, tgid 1 (systemd), ts 349667370 ns
>   PFN 0xa00a2 type Unmovable Block 1280 type Unmovable
>   Flags 0x33fffe0000004124(referenced|lru|active|private|node=3|zone=0|
> lastcpupid=0x1ffff)
>   handle: 17432583
>   Charged to memcg /

Looks nice to me.  I have just trivial cosmetic comments below.

> 
> Print mode significantly reduces output size while preserving all
> other page allocation information. The correspondence between handles
> and stack traces can be obtained through the show_stacks_handles interface.

I understand the mode was introduced for the stack handle mode, but it exists
for both full stack and stack handle mode?  The wording makes me assume the
mode is only enabled vs disabled (boolean).  It is true that there are only two
modes, but I feel like this commit message might better written.

> 
> Link: https://lore.kernel.org/linux-mm/20260417154638.22370-3-zhen.ni@xxxxxxxxxxxx/

Seems this is a link to v1 of this patch.  I think adding the context or moving
this to the changelog [1] with the brief context explanation would be nice.

> Suggested-by: Zi Yan <ziy@xxxxxxxxxx>
> Signed-off-by: Zhen Ni <zhen.ni@xxxxxxxxxxxx>
> ---
> 
> Changes in v2:
> - Renamed from 'compact mode' to 'print_mode' for better clarity
> - Use enum values (0=full_stack, 1=stack_handle) instead of boolean
> - Update debugfs filename from 'compact' to 'print_mode'
> 
> Changes in v3:
> - No code changes
> ---
>  mm/page_owner.c | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/page_owner.c b/mm/page_owner.c
> index 5884d883837e..6d87b6948cfa 100644
> --- a/mm/page_owner.c
> +++ b/mm/page_owner.c
> @@ -590,7 +590,13 @@ print_page_owner(char __user *buf, size_t count, unsigned long pfn,
>  			migratetype_names[pageblock_mt],
>  			&page->flags);
>  
> -	ret += stack_depot_snprint(handle, kbuf + ret, count - ret, 0);
> +	/* Print mode: full stack or stack handle */

I think this comment should be useful if it was compact_mode.  But because now
it is called print_mode and values have good names that self explaining what
the mode is, I'm not sure if this comment is really needed.

> +	if (READ_ONCE(owner_filter.print_mode) == PAGE_OWNER_PRINT_STACK_HANDLE) {
> +		ret += scnprintf(kbuf + ret, count - ret,
> +				"handle: %d\n", handle);
> +	} else {
> +		ret += stack_depot_snprint(handle, kbuf + ret, count - ret, 0);
> +	}

Braces are not needed [2] here?

>  	if (ret >= count)
>  		goto err;
>  
> @@ -985,6 +991,24 @@ static int page_owner_threshold_set(void *data, u64 val)
>  DEFINE_SIMPLE_ATTRIBUTE(page_owner_threshold_fops, &page_owner_threshold_get,
>  			&page_owner_threshold_set, "%llu");
>  
> +static int page_owner_print_mode_get(void *data, u64 *val)
> +{
> +	*val = READ_ONCE(owner_filter.print_mode);
> +	return 0;
> +}
> +
> +static int page_owner_print_mode_set(void *data, u64 val)
> +{
> +	if (val > PAGE_OWNER_PRINT_STACK_HANDLE)
> +		return -EINVAL;
> +	WRITE_ONCE(owner_filter.print_mode, val);
> +	return 0;
> +}
> +
> +DEFINE_SIMPLE_ATTRIBUTE(page_owner_print_mode_fops,
> +			&page_owner_print_mode_get,
> +			&page_owner_print_mode_set, "%lld");
> +
>  
>  static int __init pageowner_init(void)
>  {
> @@ -998,6 +1022,8 @@ static int __init pageowner_init(void)
>  	debugfs_create_file("page_owner", 0400, NULL, NULL, &page_owner_fops);
>  
>  	filter_dir = debugfs_create_dir("page_owner_filter", NULL);
> +	debugfs_create_file("print_mode", 0600, filter_dir, NULL,
> +			    &page_owner_print_mode_fops);
>  
>  	dir = debugfs_create_dir("page_owner_stacks", NULL);
>  	debugfs_create_file("show_stacks", 0400, dir,
> -- 
> 2.20.1

[1] https://docs.kernel.org/process/submitting-patches.html#commentary
[2] https://docs.kernel.org/process/coding-style.html#placing-braces-and-spaces


Thanks,
SJ