Re: [PATCH v2 1/3] mm/page_owner: add filter infrastructure

["zhen.ni" <zhen.ni@easystack.cn>]

在 2026/4/28 21:13, Zi Yan 写道:
> On 27 Apr 2026, at 23:33, zhen.ni wrote:
> 
>> 在 2026/4/27 23:35, Zi Yan 写道:
>>> On 19 Apr 2026, at 11:55, Zhen Ni wrote:
>>>
>>>> Add data structure for page_owner filtering functionality and create
>>>> debugfs directory for filter controls.
>>>>
>>>> This adds:
>>>> - enum page_owner_print_mode with values for full_stack and stack_handle
>>>> - struct page_owner_filter with print_mode and nid_mask fields
>>>> - Static owner_filter instance initialized with default values
>>>> - page_owner_filter debugfs directory
>>>>
>>>> The filter infrastructure will be used to add print_mode and NUMA node
>>>> filtering capabilities in subsequent commits.
>>>>
>>>> Link: https://lore.kernel.org/linux-mm/20260417154638.22370-2-zhen.ni@easystack.cn/
>>>> Suggested-by: Zi Yan <ziy@nvidia.com>
>>>> Signed-off-by: Zhen Ni <zhen.ni@easystack.cn>
>>>> ---
>>>>
>>>> Changes in v2:
>>>> - Use enum page_owner_print_mode instead of bool 'compact' for better clarity
>>>> - Use nodemask_t instead of int 'nid' to support multi-node filtering
>>>> ---
>>>>    mm/page_owner.c | 20 +++++++++++++++++++-
>>>>    1 file changed, 19 insertions(+), 1 deletion(-)
>>>
>>> The patch can be folded into Patch 2. Otherwise, these new types are not
>>> used and page_owner_filter folder is just empty.
>>>
>>
>>
>> Thanks for your review and suggestion.
>>
>> I kept the 3-patch structure mainly for clear functional separation,
>> which also makes review easier. Patch 1 adds the infrastructure,
> 
> What is the point of adding unused code in a separate patch?
> Patch 1 has no functional addition and should be part of Patch 2.
> 
>> patch 2 adds print_mode filter, and patch 3 adds NUMA filter.
>>
>> I discussed this with Andrew and he has already accepted the
>> current 3-patch structure into mm-new for testing. Since he's okay
>> with it and mentioned that bisection holes are acceptable in this
> 
> The patches are still in mm-new, not mm-stable. It is totally acceptable
> to reorganize them.
> 
>> case, I'll keep the current structure in v3.
> 
> Please refrain from posting a new version when the discussion is still
> going on.
> 
>>
>>>>
>>>> diff --git a/mm/page_owner.c b/mm/page_owner.c
>>>> index 8178e0be557f..5884d883837e 100644
>>>> --- a/mm/page_owner.c
>>>> +++ b/mm/page_owner.c
>>>> @@ -54,6 +54,21 @@ struct stack_print_ctx {
>>>>    	u8 flags;
>>>>    };
>>>>
>>>> +enum page_owner_print_mode {
>>>> +	PAGE_OWNER_PRINT_FULL_STACK,
>>>> +	PAGE_OWNER_PRINT_STACK_HANDLE,
>>>> +};
>>>> +
>>>> +struct page_owner_filter {
>>>> +	enum page_owner_print_mode print_mode;
>>>> +	nodemask_t nid_mask;
>>>> +};
>>>> +
>>>> +static struct page_owner_filter owner_filter = {
>>>> +	.print_mode = PAGE_OWNER_PRINT_FULL_STACK,
>>>> +	.nid_mask = NODE_MASK_NONE,
>>>> +};
>>>> +
>>>>    static bool page_owner_enabled __initdata;
>>>>    DEFINE_STATIC_KEY_FALSE(page_owner_inited);
>>>>
>>>> @@ -973,7 +988,7 @@ DEFINE_SIMPLE_ATTRIBUTE(page_owner_threshold_fops, &page_owner_threshold_get,
>>>>
>>>>    static int __init pageowner_init(void)
>>>>    {
>>>> -	struct dentry *dir;
>>>> +	struct dentry *dir, *filter_dir;
>>>>
>>>>    	if (!static_branch_unlikely(&page_owner_inited)) {
>>>>    		pr_info("page_owner is disabled\n");
>>>> @@ -981,6 +996,9 @@ 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);
>>>> +
>>>>    	dir = debugfs_create_dir("page_owner_stacks", NULL);
>>>>    	debugfs_create_file("show_stacks", 0400, dir,
>>>>    			    (void *)(STACK_PRINT_FLAG_STACK |
>>>> -- 
>>>> 2.20.1
>>>
>>>
>>> Best Regards,
>>> Yan, Zi
>>>
>>>
>> Best regards,
>> Zhen
> 
> 
> Best Regards,
> Yan, Zi
> 
> 
Regarding your suggestion to fold patch 1 into patch 2:

I understand your point that patch 1 adds unused code and creates
an empty page_owner_filter folder. While a single patch should not have 
unused variables, in a patch series it's sometimes acceptable to have 
preparatory patches that add infrastructure for subsequent patches to use.

I found many examples in the kernel where "prepare" or "preparatory" 
patches add code that is only fully utilized in later patches

If you and other reviewers strongly prefer merging patches 1 and 2 to 
follow the guidance more strictly, I can do that in v4. I'm open to this 
change if the community feels it's the better approach.


Best regards,
Zhen