When offlining empties a zone, zone_pcp_reset() frees its pagesets while
the vmstat worker can still hold one across cond_resched():
CPU 0 CPU 1
refresh_cpu_vm_stats()
pcp = zone->per_cpu_pageset
cond_resched()
offline_pages()
zone_pcp_reset()
free_percpu(zone->per_cpu_pageset)
decay_pcp_high(zone, this_cpu_ptr(pcp))
decay_pcp_high() then reads, and can write, memory the percpu allocator
may already have handed out again. With kprobes holding the worker
across the offline, it used the freed pageset in 6 of 6 runs, and
rewrote a new allocation at the freed address in 1 of them.
Keep the pagesets for the life of the zone, as pgdat->per_cpu_nodestats
already is, and only fold the stats in zone_pcp_reset().
Commit 340175b7d14d ("mm/hotplug: free zone->pageset when a zone becomes
empty") freed them because onlining the zone again allocated a new set
and leaked the old one. setup_zone_pageset() now reuses the existing
set, and zone_pcp_init() keeps it when the node is re-added, so nothing
is allocated twice. The pageset address stayed the same across repeated
offline/online cycles and across removing and re-adding the node.
The pagesets take 320 bytes per possible CPU: 30KiB per zone with 96
CPUs, 160KiB with 512 and 2.5MiB with 8192. A populated zone already
holds this memory, we just no longer give it back on offline.
Fixes: 340175b7d14d ("mm/hotplug: free zone->pageset when a zone becomes empty")
Cc: [email protected]
Signed-off-by: Gregory Price (Meta) <[email protected]>
---
mm/page_alloc.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 96ae09c8099be..dfca77764f1fb 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6369,6 +6369,12 @@ void __meminit setup_zone_pageset(struct zone *zone)
{
int cpu;
+ /* Reuse the pagesets zone_pcp_reset() kept. */
+ if (zone->per_cpu_pageset != &boot_pageset) {
+ zone_set_pageset_high_and_batch(zone, 0);
+ 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);
@@ -6465,9 +6471,12 @@ __meminit void zone_pcp_init(struct zone *zone)
* per cpu subsystem is not up at this point. The following code
* relies on the ability of the linker to provide the
* offset of a (static) per cpu variable into the per cpu area.
+ * A re-added node keeps the pagesets zone_pcp_reset() kept.
*/
- zone->per_cpu_pageset = &boot_pageset;
- zone->per_cpu_zonestats = &boot_zonestats;
+ if (!zone->per_cpu_pageset || zone->per_cpu_pageset == &boot_pageset) {
+ zone->per_cpu_pageset = &boot_pageset;
+ zone->per_cpu_zonestats = &boot_zonestats;
+ }
zone->pageset_high_min = BOOT_PAGESET_HIGH;
zone->pageset_high_max = BOOT_PAGESET_HIGH;
zone->pageset_batch = BOOT_PAGESET_BATCH;
@@ -7749,6 +7758,10 @@ void zone_pcp_enable(struct zone *zone)
mutex_unlock(&pcp_batch_high_lock);
}
+/*
+ * Keep the pagesets of an emptied zone: refresh_cpu_vm_stats() can hold
+ * one across cond_resched(), and nothing waits for it to let go.
+ */
void zone_pcp_reset(struct zone *zone)
{
int cpu;
@@ -7759,12 +7772,6 @@ void zone_pcp_reset(struct zone *zone)
pzstats = per_cpu_ptr(zone->per_cpu_zonestats, cpu);
drain_zonestat(zone, pzstats);
}
- free_percpu(zone->per_cpu_pageset);
- zone->per_cpu_pageset = &boot_pageset;
- if (zone->per_cpu_zonestats != &boot_zonestats) {
- free_percpu(zone->per_cpu_zonestats);
- zone->per_cpu_zonestats = &boot_zonestats;
- }
}
}
--
2.53.0-Meta
