[PATCH] sched/isolation: Defer freeing of the bootmem housekeeping cpumasks

Ionut Nechita <[email protected]>
Newsgroups gmane.linux.kernel,gmane.linux.kernel.mm
Message-ID <[email protected]>
housekeeping_setup() allocates the housekeeping cpumasks from memblock
while parsing the command line, long before the page allocator exists.
housekeeping_init() then reallocates them with kmalloc(), so that a later
runtime update can free the old mask with kfree(), and releases the
memblock allocations with memblock_free().

That release is not safe where it currently sits. housekeeping_init() is
called from start_kernel() after mm_core_init(), so slab_is_available()
is already true and memblock_phys_free() takes the __free_reserved_area()
path. But it is still called long before page_alloc_init_late(), so with
CONFIG_DEFERRED_STRUCT_PAGE_INIT=y the deferred part of the memory map is
not initialized yet, and __free_reserved_area() refuses to touch it:

  Cannot free reserved memory because of deferred initialization of the memory map
  WARNING: mm/memblock.c:904 at __free_reserved_area+0xde/0xf0, CPU#0: swapper/0/0
  Call Trace:
   memblock_phys_free+0xe4/0x120
   housekeeping_init+0x149/0x170
   start_kernel+0x5b6/0x800
   x86_64_start_reservations+0x24/0x30
   x86_64_start_kernel+0xd7/0xe0
   common_startup_64+0x13e/0x151

The warning fires once per housekeeping type in use -- four splats on a
112-CPU two-socket machine booted with both isolcpus= and nohz_full= --
and taints the kernel with G W. No memory is actually lost either way:
a cpumask is much smaller than a page, so __free_reserved_area() has no
whole page to hand back to the buddy allocator in the first place.

Record the bootmem masks instead and release them from a core_initcall,
which runs after page_alloc_init_late() has initialized the deferred
memory map. early_initcall() would still be too early: kernel_init_freeable()
runs do_pre_smp_initcalls() before page_alloc_init_late().

Masks that housekeeping_init() did not manage to replace, because
kmalloc() failed, are never recorded and therefore stay live, preserving
the existing error behaviour.

Fixes: 27c3a5967f05 ("sched/isolation: Convert housekeeping cpumasks to rcu pointers")
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221804
Link: https://lore.kernel.org/linux-mm/[email protected]/
Suggested-by: Mike Rapoport (Microsoft) <[email protected]>
Signed-off-by: Ionut Nechita <[email protected]>
---
Reproduced on a Dell PowerEdge R750 (2x Xeon Gold 6330, 112 CPUs, 2 NUMA
nodes, 128 GiB) running v7.2-rc5 with PREEMPT_RT and
CONFIG_DEFERRED_STRUCT_PAGE_INIT=y, booted with:

  rcu_nocbs=8-55,64-111 nohz_full=managed_irq,nohz,domain,8-55,64-111
  isolcpus=managed_irq,nohz,domain,8-55,64-111 kthread_cpus=0-3,56-59
  irqaffinity=4-7,60-63 rcutree.kthread_prio=21

Four splats at boot without this patch, none with it. Also build-tested
on x86_64 defconfig plus CONFIG_NO_HZ_FULL and
CONFIG_DEFERRED_STRUCT_PAGE_INIT.

The bad memblock_free() has been there since 27c3a5967f05 in v7.0, but it
only became visible in v7.1, when 59bd1d914bb5 ("memblock: warn when
freeing reserved memory before memory map is initialized") added the
WARN. Since nothing is actually leaked and the effect is a warning plus a
G W taint, I did not Cc stable -- say the word if you would rather have
it there.

I also considered Mike's second suggestion, of having housekeeping_setup()
make a single memblock allocation covering all HK_TYPE_MAX masks and
freeing that one region later. It touches more code for no additional
benefit here, so I went with the simpler variant.

 kernel/sched/isolation.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c
index ef152d401fe2..5c36e33bac50 100644
--- a/kernel/sched/isolation.c
+++ b/kernel/sched/isolation.c
@@ -28,6 +28,17 @@ struct housekeeping {
 
 static struct housekeeping housekeeping;
 
+/*
+ * Bootmem cpumasks that housekeeping_init() has replaced with kmalloc()ed
+ * copies. They can't be released right there: housekeeping_init() runs once
+ * the page allocator and slab are up, but before page_alloc_init_late() has
+ * initialized the deferred part of the memory map, and memblock_free() then
+ * reaches __free_reserved_area(), which refuses to touch a memory map that
+ * isn't fully initialized yet. Record them instead and release them from a
+ * core_initcall, which runs after page_alloc_init_late().
+ */
+static struct cpumask *housekeeping_bootmem_masks[HK_TYPE_MAX] __initdata;
+
 bool housekeeping_enabled(enum hk_type type)
 {
 	return !!(READ_ONCE(housekeeping.flags) & BIT(type));
@@ -189,10 +200,21 @@ void __init housekeeping_init(void)
 		WARN_ON_ONCE(cpumask_empty(omask));
 		cpumask_copy(nmask, omask);
 		RCU_INIT_POINTER(housekeeping.cpumasks[type], nmask);
-		memblock_free(omask, cpumask_size());
+		housekeeping_bootmem_masks[type] = omask;
 	}
 }
 
+static int __init housekeeping_free_bootmem_masks(void)
+{
+	enum hk_type type;
+
+	for (type = 0; type < HK_TYPE_MAX; type++)
+		memblock_free(housekeeping_bootmem_masks[type], cpumask_size());
+
+	return 0;
+}
+core_initcall(housekeeping_free_bootmem_masks);
+
 static void __init housekeeping_setup_type(enum hk_type type,
 					   cpumask_var_t housekeeping_staging)
 {

base-commit: 2d2338c93da79b3bfe4b6099a931d9468d539952
-- 
2.55.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.