2008-01-18 18:33:55

by Mike Travis

[permalink] [raw]
Subject: [PATCH 5/5] x86: Add debug of invalid per_cpu map accesses

Provide a means to trap usages of per_cpu map variables before
they are setup. Define CONFIG_DEBUG_PER_CPU_MAPS to activate.

Signed-off-by: Mike Travis <[email protected]>
---
arch/x86/Kconfig.debug | 12 ++++++++++++
arch/x86/mm/numa_64.c | 3 +++
include/asm-x86/topology.h | 7 +++++++
3 files changed, 22 insertions(+)

--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -47,6 +47,18 @@ config DEBUG_PAGEALLOC
This results in a large slowdown, but helps to find certain types
of memory corruptions.

+config DEBUG_PER_CPU_MAPS
+ bool "Debug access to per_cpu maps"
+ depends on DEBUG_KERNEL
+ depends on X86_64_SMP
+ default n
+ help
+ Say Y to verify that the per_cpu map being accessed has
+ been setup. Adds a fair amount of code to kernel memory
+ and decreases performance.
+
+ Say N if unsure.
+
config DEBUG_RODATA
bool "Write protect kernel read-only data structures"
depends on DEBUG_KERNEL
--- a/arch/x86/mm/numa_64.c
+++ b/arch/x86/mm/numa_64.c
@@ -37,6 +37,9 @@ u16 x86_cpu_to_node_map_init[NR_CPUS] =
void *x86_cpu_to_node_map_early_ptr;
DEFINE_PER_CPU(u16, x86_cpu_to_node_map) = NUMA_NO_NODE;
EXPORT_PER_CPU_SYMBOL(x86_cpu_to_node_map);
+#ifdef CONFIG_DEBUG_PER_CPU_MAPS
+EXPORT_SYMBOL(x86_cpu_to_node_map_early_ptr);
+#endif

u16 apicid_to_node[MAX_LOCAL_APIC] __cpuinitdata = {
[0 ... MAX_LOCAL_APIC-1] = NUMA_NO_NODE
--- a/include/asm-x86/topology.h
+++ b/include/asm-x86/topology.h
@@ -66,6 +66,13 @@ static inline int early_cpu_to_node(int

static inline int cpu_to_node(int cpu)
{
+#ifdef CONFIG_DEBUG_PER_CPU_MAPS
+ if(x86_cpu_to_node_map_early_ptr) {
+ printk("KERN_NOTICE cpu_to_node(%d): usage too early!\n",
+ (int)cpu);
+ BUG();
+ }
+#endif
if(per_cpu_offset(cpu))
return per_cpu(x86_cpu_to_node_map, cpu);
else

--


2008-01-18 18:35:40

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH 5/5] x86: Add debug of invalid per_cpu map accesses

On Friday 18 January 2008 19:30:16 [email protected] wrote:
> Provide a means to trap usages of per_cpu map variables before
> they are setup. Define CONFIG_DEBUG_PER_CPU_MAPS to activate.

Are you sure that debug option is generally useful enough
to merge? It seems very specific to your patchkit, but I'm not
sure it would be worth carrying forever in the kernel.

Better would be probably to just unmap those areas anyways.

-Andi

2008-01-18 18:49:18

by Mike Travis

[permalink] [raw]
Subject: Re: [PATCH 5/5] x86: Add debug of invalid per_cpu map accesses

Andi Kleen wrote:
> On Friday 18 January 2008 19:30:16 [email protected] wrote:
>> Provide a means to trap usages of per_cpu map variables before
>> they are setup. Define CONFIG_DEBUG_PER_CPU_MAPS to activate.
>
> Are you sure that debug option is generally useful enough
> to merge? It seems very specific to your patchkit, but I'm not
> sure it would be worth carrying forever in the kernel.
>
> Better would be probably to just unmap those areas anyways.
>
> -Andi

I thought for a round of testing it'd be worthwhile, particularly
testing randconfig. The next version of percpu changes are coming
soon and I can remove them then. Ideally, you are right, it'd
be nice to not have the percpu memory mapped for processors that are
not present.

One thing that comes into play is that (soon) the boot_pda will be
in the per_cpu area and it's maintained for all "possible" cpus as
it keeps track of some info on "off-lined" cpus. What I was thinking
is that perhaps no memory is allocated (and mapped) until a cpu
becomes not only possible, but probable. This means that until a
cpu is brought online, or discovered via ACPI, then there is no percpu
area and access to an invalid percpu area causes a kernel fault.

Thanks,
Mike

2008-01-18 18:56:51

by Christoph Lameter

[permalink] [raw]
Subject: Re: [PATCH 5/5] x86: Add debug of invalid per_cpu map accesses

On Fri, 18 Jan 2008, Andi Kleen wrote:

> On Friday 18 January 2008 19:30:16 [email protected] wrote:
> > Provide a means to trap usages of per_cpu map variables before
> > they are setup. Define CONFIG_DEBUG_PER_CPU_MAPS to activate.
>
> Are you sure that debug option is generally useful enough
> to merge? It seems very specific to your patchkit, but I'm not
> sure it would be worth carrying forever in the kernel.
>
> Better would be probably to just unmap those areas anyways.

Its generally useful also for the cpu_alloc changes that may lead
to the moving around of early initialization code for consolidation of
code between i386 and x86_64 once Mike's initial patchset is in.

2008-01-18 20:50:27

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 5/5] x86: Add debug of invalid per_cpu map accesses


* Andi Kleen <[email protected]> wrote:

> On Friday 18 January 2008 19:30:16 [email protected] wrote:
> > Provide a means to trap usages of per_cpu map variables before they
> > are setup. Define CONFIG_DEBUG_PER_CPU_MAPS to activate.
>
> Are you sure that debug option is generally useful enough to merge? It
> seems very specific to your patchkit, but I'm not sure it would be
> worth carrying forever in the kernel.

yeah, i think it's simple enough.

Ingo