Re: [PATCH v5 02/12] mm/page_alloc: keep the pagesets of an empty zone

[Vlastimil Babka (SUSE)]

On 11/3/26 17:04, Zi Yan wrote:
> On Tue Nov 3, 2026 at 9:57 AM EST, Vlastimil Babka (SUSE) wrote:
>>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>>>> index f1048168c90c..1850a41f24f7 100644
>>>> --- a/mm/page_alloc.c
>>>> +++ b/mm/page_alloc.c
>>>> @@ -6308,6 +6308,10 @@ void __meminit setup_zone_pageset(struct zone *zone)
>>>>  {
>>>>  	int cpu;
>>>>  
>>>> +	/* The zone has been populated before, see zone_pcp_reset(). */
>>>> +	if (zone->per_cpu_pageset != &boot_pageset)
>>>> +		return;
>>>> +
>>>>  	/* Size may be 0 on !SMP && !NUMA */
>>>>  	if (sizeof(struct per_cpu_zonestat) > 0)
>>>>  		zone->per_cpu_zonestats = alloc_percpu(struct per_cpu_zonestat);
>>>> @@ -6400,6 +6404,10 @@ void __init setup_per_cpu_pageset(void)
>>>>  
>>>>  __meminit void zone_pcp_init(struct zone *zone)
>>>>  {
>>>> +	/* A node which is added again, see zone_pcp_reset(). */
>>> 
>>> it might be better to explain zone->per_cpu_pageset can only be NULL at
>>> boot time and it needs an initialization.
>>
>> Hmm.
>>
>>>> +	if (zone->per_cpu_pageset && zone->per_cpu_pageset != &boot_pageset)
>>>> +		return;
>>
>> Shouldn't then the check be just:
>>
>> if (zone->per_cpu_pageset)
>> 	return;
>>
>> Because if it's == &boot_pageset, we are just setting the same values that
>> area already there?
>>
> 
> Right. And zone_pcp_init(), called by zone_init_internals(), becomes a nop for
> free_area_init_core_hotplug(). Then I wonder if zone_init_internals() is
> needed for hotplug, but that belongs to a later cleanup.

Yep, that's for later cleanup. Right now the minimum that's correct and not
unnecessarily misleading is enough. So I've done this:

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 1850a41f24f7..20bfd11c5851 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6404,8 +6404,8 @@ void __init setup_per_cpu_pageset(void)

 __meminit void zone_pcp_init(struct zone *zone)
 {
-       /* A node which is added again, see zone_pcp_reset(). */
-       if (zone->per_cpu_pageset && zone->per_cpu_pageset != &boot_pageset)
+       /* This initialization only needs to be done once */
+       if (zone->per_cpu_pageset)
                return;

        /*