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

[Zi Yan]

On 29 Apr 2026, at 7:22, zhen.ni wrote:

> 在 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@xxxxxxxxxxxx/
>>>>> Suggested-by: Zi Yan <ziy@xxxxxxxxxx>
>>>>> Signed-off-by: Zhen Ni <zhen.ni@xxxxxxxxxxxx>
>>>>> ---
>>>>>
>>>>> 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

Those preparatory patches usually add actual functions, not just types.
This patch also adds a user visible folder that has no function.
I do not see any reason this standalone patch is necessary.
>
> 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,
Yan, Zi