Skip to contentSkip to conversation

x86_64: mark x86_cpu_to_node_map_init to __initdata like other xx_init

PATCH In review Yinghai Lu 2 participants 2 messages Jan 28, 2008

Subsystems (2)
x86 x86 MM
lore
Share & tools
lore

Yinghai Lu

author
#0 lore raw

PATCH 1/1 x86_64: mark x86_cpu_to_node_map_init to __initdata like other xx_init

[PATCH] x86_64: mark x86_cpu_to_node_map_init to __initdata like other xx_init

Signed-off-by Yinghai Lu <[email protected]>
arch/x86/mm/numa_64.c +1 -1
index f0e5cab..d7af3fd 100644
@@ -31,7 +31,7 @@ bootmem_data_t plat_node_bdata[MAX_NUMNODES];
 
 struct memnode memnode;
 
-int x86_cpu_to_node_map_init[NR_CPUS] = {
+int x86_cpu_to_node_map_init[NR_CPUS] __initdata = {
 	[0 ... NR_CPUS-1] = NUMA_NO_NODE
 };
 void *x86_cpu_to_node_map_early_ptr;
include/asm-x86/topology.h +1 -1
index 8af05a9..d3340de 100644
@@ -35,7 +35,7 @@ extern int cpu_to_node_map[];
 
 #else
 DECLARE_PER_CPU(int, x86_cpu_to_node_map);
-extern int x86_cpu_to_node_map_init[];
+extern int __initdata x86_cpu_to_node_map_init[];
 extern void *x86_cpu_to_node_map_early_ptr;
 /* Returns the number of the current Node. */
 #define numa_node_id()		(early_cpu_to_node(raw_smp_processor_id()))
#1 lore raw

Review

Quoted 4 lines · Yinghai Lu
(quoted diff) +1 -1
-int x86_cpu_to_node_map_init[NR_CPUS] = {
+int x86_cpu_to_node_map_init[NR_CPUS] __initdata = {
 	[0 ... NR_CPUS-1] = NUMA_NO_NODE
 };

i remember some linker warning here. While this array should indeed only be used in early init, that decision is dynamic and our linker warnings do not notice it. There's a special marker for such cases:
__initdata_refok. But ... i'm slightly nervous about turning off a vital warning like that.

Sam, it would be nice to have a DEBUG_INITDATA mode of operation: in this case free_initmem() would not truly free those pages but would unmap them via:

   kernel_map_pages(page, nrpages, 0);

could be made dependent on DEBUG_PAGEALLOC.

If this debugging is enabled then if any code references it, we get a hard page fault. If we had a debug mode like that then bugs in this area would not go unnoticed.

Or perhaps just make this part of normal DEBUG_PAGEALLOC. Like the patch below on top of latest x86.git. Hm?

Ingo

------------------->
Subject: x86: init memory debugging
From: Ingo Molnar <[email protected]>

debug incorrect/late access to init memory, by permanently unmapping the init memory ranges. Depends on CONFIG_DEBUG_PAGEALLOC=y.

Signed-off-by Ingo Molnar <[email protected]>
---
 arch/x86/mm/init_32.c |   12 ++++++++++++
 arch/x86/mm/init_64.c |   11 +++++++++++
 2 files changed, 23 insertions(+)
arch/x86/mm/init_32.c +12 -0
Index: linux-x86.q/arch/x86/mm/init_32.c
===================================================================
@@ -794,6 +794,18 @@ void free_init_pages(char *what, unsigne
 	unsigned long addr;
 
 	/*
+	 * If debugging page accesses then do not free this memory but
+	 * mark them not present - any buggy init-section access will
+	 * create a kernel page fault:
+	 */
+#ifdef CONFIG_DEBUG_PAGEALLOC
+	printk(KERN_INFO "debug: unmapping init memory %08lx..%08lx\n",
+		begin, PAGE_ALIGN(end));
+	set_memory_np(begin, (end - begin) >> PAGE_SHIFT);
+	return;
+#endif
+	set_memory_rw(begin, (end - begin) >> PAGE_SHIFT);
+	/*
 	 * We just marked the kernel text read only above, now that
 	 * we are going to free part of that, we need to make that
 	 * writeable first.
arch/x86/mm/init_64.c +11 -0
Index: linux-x86.q/arch/x86/mm/init_64.c
===================================================================
@@ -580,6 +580,17 @@ void free_init_pages(char *what, unsigne
 	if (begin >= end)
 		return;
 
+	/*
+	 * If debugging page accesses then do not free this memory but
+	 * mark them not present - any buggy init-section access will
+	 * create a kernel page fault:
+	 */
+#ifdef CONFIG_DEBUG_PAGEALLOC
+	printk(KERN_INFO "debug: unmapping init memory %08lx..%08lx\n",
+		begin, PAGE_ALIGN(end));
+	set_memory_np(begin, (end - begin) >> PAGE_SHIFT);
+	return;
+#endif
 	printk(KERN_INFO "Freeing %s: %luk freed\n", what, (end - begin) >> 10);
 
 	for (addr = begin; addr < end; addr += PAGE_SIZE) {