CPU ops and a rework of per cpu data handling on x86_64

ACPI uses NR_CPUS in various loops and in some it accesses per cpu data of processors that are not present(!) and that will never be present. The pointers to per cpu data are typically not initialized for processors that are not present. So we seem to be reading something here from offset 0 in memory.

Make ACPI use nr_cpu_ids instead. That stops at the end of the possible processors.

Convert one loop to NR_CPUS to use the cpu_possible map instead.

Signed-off-by Christoph Lameter <[email protected]>

---
 drivers/acpi/processor_core.c |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Index: linux-2.6/drivers/acpi/processor_core.c
===================================================================

drivers/acpi/processor_core.c +7 -7

@@ -494,7 +494,7 @@ static int get_cpu_id(acpi_handle handle
 	if (apic_id == -1)
 		return apic_id;
 
-	for (i = 0; i < NR_CPUS; ++i) {
+	for_each_possible_cpu(i) {
 		if (cpu_physical_id(i) == apic_id)
 			return i;
 	}

@@ -638,7 +638,7 @@ static int __cpuinit acpi_processor_star
 		return 0;
 	}
 
-	BUG_ON((pr->id >= NR_CPUS) || (pr->id < 0));
+	BUG_ON((pr->id >= nr_cpu_ids) || (pr->id < 0));
 
 	/*
 	 * Buggy BIOS check

@@ -771,7 +771,7 @@ static int acpi_processor_remove(struct 
 
 	pr = acpi_driver_data(device);
 
-	if (pr->id >= NR_CPUS) {
+	if (pr->id >= nr_cpu_ids) {
 		kfree(pr);
 		return 0;
 	}

@@ -842,7 +842,7 @@ int acpi_processor_device_add(acpi_handl
 	if (!pr)
 		return -ENODEV;
 
-	if ((pr->id >= 0) && (pr->id < NR_CPUS)) {
+	if ((pr->id >= 0) && (pr->id < nr_cpu_ids)) {
 		kobject_uevent(&(*device)->dev.kobj, KOBJ_ONLINE);
 	}
 	return 0;

@@ -880,13 +880,13 @@ acpi_processor_hotplug_notify(acpi_handl
 			break;
 		}
 
-		if (pr->id >= 0 && (pr->id < NR_CPUS)) {
+		if (pr->id >= 0 && (pr->id < nr_cpu_ids)) {
 			kobject_uevent(&device->dev.kobj, KOBJ_OFFLINE);
 			break;
 		}
 
 		result = acpi_processor_start(device);
-		if ((!result) && ((pr->id >= 0) && (pr->id < NR_CPUS))) {
+		if ((!result) && ((pr->id >= 0) && (pr->id < nr_cpu_ids))) {
 			kobject_uevent(&device->dev.kobj, KOBJ_ONLINE);
 		} else {
 			printk(KERN_ERR PREFIX "Device [%s] failed to start\n",

@@ -909,7 +909,7 @@ acpi_processor_hotplug_notify(acpi_handl
 			return;
 		}
 
-		if ((pr->id < NR_CPUS) && (cpu_present(pr->id)))
+		if ((pr->id < nr_cpu_ids) && (cpu_present(pr->id)))
 			kobject_uevent(&device->dev.kobj, KOBJ_OFFLINE);
 		break;
 	default:

Signature · 1 line

-- 

Quoted 2 lines

Convert one loop to NR_CPUS to use the cpu_possible map instead.

I'm just wondering how broken this is. Is there any assumption that there is no holes in the online cpu map in this code ?

ACPI uses NR_CPUS in various loops and in some it accesses per cpu data of processors that are not present(!) and that will never be present. The pointers to per cpu data are typically not initialized for processors that are not present. So we seem to be reading something here from offset 0 in memory.

Make ACPI use nr_cpu_ids instead. That stops at the end of the possible processors.

Convert one loop to NR_CPUS to use the cpu_possible map instead.

Signed-off-by Christoph Lameter <[email protected]>

---
 drivers/acpi/processor_core.c |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Index: linux-2.6/drivers/acpi/processor_core.c
===================================================================

drivers/acpi/processor_core.c +7 -7

@@ -494,7 +494,7 @@ static int get_cpu_id(acpi_handle handle
 	if (apic_id == -1)
 		return apic_id;
 
-	for (i = 0; i < NR_CPUS; ++i) {
+	for_each_possible_cpu(i) {
 		if (cpu_physical_id(i) == apic_id)
 			return i;
 	}

@@ -638,7 +638,7 @@ static int __cpuinit acpi_processor_star
 		return 0;
 	}
 
-	BUG_ON((pr->id >= NR_CPUS) || (pr->id < 0));
+	BUG_ON((pr->id >= nr_cpu_ids) || (pr->id < 0));
 
 	/*
 	 * Buggy BIOS check

@@ -771,7 +771,7 @@ static int acpi_processor_remove(struct 
 
 	pr = acpi_driver_data(device);
 
-	if (pr->id >= NR_CPUS) {
+	if (pr->id >= nr_cpu_ids) {
 		kfree(pr);
 		return 0;
 	}

@@ -842,7 +842,7 @@ int acpi_processor_device_add(acpi_handl
 	if (!pr)
 		return -ENODEV;
 
-	if ((pr->id >= 0) && (pr->id < NR_CPUS)) {
+	if ((pr->id >= 0) && (pr->id < nr_cpu_ids)) {
 		kobject_uevent(&(*device)->dev.kobj, KOBJ_ONLINE);
 	}
 	return 0;

@@ -880,13 +880,13 @@ acpi_processor_hotplug_notify(acpi_handl
 			break;
 		}
 
-		if (pr->id >= 0 && (pr->id < NR_CPUS)) {
+		if (pr->id >= 0 && (pr->id < nr_cpu_ids)) {
 			kobject_uevent(&device->dev.kobj, KOBJ_OFFLINE);
 			break;
 		}
 
 		result = acpi_processor_start(device);
-		if ((!result) && ((pr->id >= 0) && (pr->id < NR_CPUS))) {
+		if ((!result) && ((pr->id >= 0) && (pr->id < nr_cpu_ids))) {
 			kobject_uevent(&device->dev.kobj, KOBJ_ONLINE);
 		} else {
 			printk(KERN_ERR PREFIX "Device [%s] failed to start\n",

@@ -909,7 +909,7 @@ acpi_processor_hotplug_notify(acpi_handl
 			return;
 		}
 
-		if ((pr->id < NR_CPUS) && (cpu_present(pr->id)))
+		if ((pr->id < nr_cpu_ids) && (cpu_present(pr->id)))
 			kobject_uevent(&device->dev.kobj, KOBJ_OFFLINE);
 		break;
 	default:

I would suggest to separate that one and fast-track it, possibly even for 2.6.24

Currently the per cpu subsystem is not able to use the atomic capabilities of the processors we have.

This adds new functionality that allows the optimizing of per cpu variable handliong. It in particular provides a simple way to exploit atomic operations

to avoid having to disable itnerrupts or add an per cpu offset.

F.e. current implementations may do

unsigned long flags;
struct stat_struct *p;

local_irq_save(flags);
/* Calculate address of per processor area */
p = CPU_PTR(stat, smp_processor_id());
p->counter++;
local_irq_restore(flags);

This whole segment can be replaced by a single CPU operation

CPU_INC(stat->counter);

And on most processors it is possible to perform the increment with a single processor instruction. Processors have segment registers, global registers and per cpu mappings of per cpu areas for that purpose.

The problem is that the current schemes cannot utilize those features. local_t is not really addressing the issue since the offset calculation is not solved. local_t is x86 processor specific. This solution here can utilize other methods than just the x86 instruction set.

On x86 the above CPU_INC translated into a single instruction:

inc %%gs:(&stat->counter)

This instruction is interrupt safe since it can either be completed or not.

The determination of the correct per cpu area for the current processor does not require access to smp_processor_id() (expensive...). The gs register is used to provide a processor specific offset to the respective per cpu area where the per cpu variabvle resides.

Note tha the counter offset into the struct was added *before* the segment

selector was added. This is necessary to avoid calculation, In the past we first determine the address of the stats structure on the respective processor and then added the field offset. However, the offset may as well be added earlier.

If stat was declared via DECLARE_PER_CPU then this patchset is capoable of

convincing the linker to provide the proper base address. In that case no calculations are necessary.

Should the stats structure be reachable via a register then the address calculation capabilities can be leverages to avoid calculations.

On IA64 the same will result in another single instruction using the factor that we have a virtual address that always maps to the local per cpu area.

fetchadd &stat->counter + (VCPU_BASE - __per_cpu_base)

The access is forced into the per cpu address reachable via the virtualized address. Again the counter field offset is eadded to the offset. The access

is then similarly a singular instruction thing as on x86.

In order to be able to exploit the atomicity of this instructions we introduce a series of new functions that take a BASE pointer (a pointer into the area of cpu 0 which is the canonical base).

CPU_READ()
CPU_WRITE()
CPU_INC
CPU_DEC
CPU_ADD
CPU_SUB
CPU_XCHG
CPU_CMPXCHG

Signed-off-by Christoph Lameter <[email protected]>

---
 include/linux/percpu.h |  156 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 156 insertions(+)

Index: linux-2.6/include/linux/percpu.h
===================================================================

include/linux/percpu.h +8 -0

@@ -190,4 +190,160 @@ void cpu_free(void *cpu_pointer, unsigne
  */
 void *boot_cpu_alloc(unsigned long size);
 
+/*
+ * Fast Atomic per cpu operations.
+ *
+ * The following operations can be overridden by arches to implement fast
+ * and efficient operations. The operations are atomic meaning that the
+ * determination of the processor, the calculation of the address and the
+ * operation on the data is an atomic operation.
+ */

include/linux/percpu.h +22 -0

+ * interrupts are already off.
+ */
+
+#define __CPU_READ(obj)				\
+({						\
+	typeof(obj) x;				\
+	x = *THIS_CPU(&(obj));			\
+	(x);					\
+})
+
+#define __CPU_WRITE(obj, value)			\
+({						\
+	*THIS_CPU((&(obj)) = value;		\
+})
+
+#define __CPU_ADD(obj, value)			\
+({						\
+	*THIS_CPU(&(obj)) += value;		\
+})
+
+
+#define __CPU_INC(addr) __CPU_ADD(addr, 1)

include/linux/percpu.h +13 -0

+#define CPU_WRITE(addr, value)			\
+({						\
+	__CPU_WRITE(addr, value);		\
+})
+
+#define CPU_ADD(addr, value)			\
+({						\
+	unsigned long flags;			\
+	local_irq_save(flags);			\
+	__CPU_ADD(addr, value);			\
+	local_irq_restore(flags);		\
+})
+

include/linux/percpu.h +4 -0

+})
+
+#endif /* CONFIG_FAST_CPU_OPS */
+
 #endif /* __LINUX_PERCPU_H */

Signature · 1 line

-- 

Very interesting patch! I did not expect we could mix local atomic ops with per CPU offsets in an atomic manner.. brilliant :)

Some nitpicking follows...

I think you could use extra () around old, new etc.. ?

Same here.

Review

Quoted 1 line

(quoted diff) +1 -0

+	(x);					\

() seems unneeded here, since x is local.

include/linux/percpu.h void cpu_free(void *cpu_pointer, unsigne · line 265

Quoted 8 lines

+ * In that case we can simply disable preemption which
+ * may be free if the kernel is compiled without preemption.
+ */

include/linux/percpu.h +5 -0

+
+#define _CPU_READ(addr)				\
+({						\
+	(__CPU_READ(addr));			\
+})

({ }) seems to be unneeded here.

Quoted 5 lines · Mathieu Desnoyers

Very interesting patch! I did not expect we could mix local atomic ops with per CPU offsets in an atomic manner.. brilliant :)

Some nitpicking follows...

Well this is a draft so I was not that thorough. The beast is getting too big. It would be good if I could get the first patches merged that just deal with the two allocators and then gradually work the rest.

Quoted 1 line

I think you could use extra () around old, new etc.. ?

Right.

Quoted 5 lines

Same here.

Review

Quoted 1 line

(quoted diff) +1 -0

+	(x);					\

() seems unneeded here, since x is local.

But (x) is returned to the "caller" of the macro so it should be specially marged.

Quoted 10 lines

include/linux/percpu.h void cpu_free(void *cpu_pointer, unsigne · line 265

Quoted 8 lines

+ * In that case we can simply disable preemption which
+ * may be free if the kernel is compiled without preemption.
+ */

include/linux/percpu.h +5 -0

+
+#define _CPU_READ(addr)				\
+({						\
+	(__CPU_READ(addr));			\
+})

({ }) seems to be unneeded here.

Hmmm.... I wanted a consistent style.

Quoted 3 lines

But (x) is returned to the "caller" of the macro so it should be specially marged.

I don't think that it really matters.. the preprocessor already wraps all the ({ }) in a single statement, doesn't it ?

Since checkpatch.pl emits a warning when a one liner if() uses brackets, I guess compactness of code is preferred to a consistent style.

Using cpu alloc removes the needs for the per cpu arrays in the kmem_cache struct. These could get quite big if we have to support system of up to thousands of cpus. The use of alloc_percpu means that:

1. The size of kmem_cache for SMP configuration shrinks since we will only need 1 pointer instead of NR_CPUS. The same pointer can be used by all processors. Reduces cache footprint of the allocator.

2. We can dynamically size kmem_cache according to the actual nodes in the system meaning less memory overhead for configurations that may potentially support up to 1k NUMA nodes.

3. We can remove the diddle widdle with allocating and releasing kmem_cache_cpu structures when bringing up and shuttting down cpus. The allocpercpu logic will do it all for us. Removes some portions of the cpu hotplug functionality.

4. Fastpath performance increases by another 20% vs. the earlier improvements. Instead of having fastpath with 45-50 cycles it is now possible to get below 40.

Remove the CONFIG_FAST_CMPXCHG version since this patch makes SLUB use CPU ops instead.

Signed-off-by Christoph Lameter <[email protected]>

---
 arch/x86/Kconfig         |    4 
 include/linux/slub_def.h |    6 -
 mm/slub.c                |  229 ++++++++++-------------------------------------
 3 files changed, 52 insertions(+), 187 deletions(-)

Index: linux-2.6/include/linux/slub_def.h
===================================================================

include/linux/slub_def.h +1 -5

@@ -34,6 +34,7 @@ struct kmem_cache_node {
  * Slab cache management.
  */
 struct kmem_cache {
+	struct kmem_cache_cpu *cpu_slab;
 	/* Used for retriving partial slabs etc */
 	unsigned long flags;
 	int size;		/* The size of an object including meta data */

@@ -63,11 +64,6 @@ struct kmem_cache {
 	int defrag_ratio;
 	struct kmem_cache_node *node[MAX_NUMNODES];
 #endif
-#ifdef CONFIG_SMP
-	struct kmem_cache_cpu *cpu_slab[NR_CPUS];
-#else
-	struct kmem_cache_cpu cpu_slab;
-#endif
 };
 
 /*

Index: linux-2.6/mm/slub.c
===================================================================

mm/slub.c +1 -11

@@ -239,15 +239,6 @@ static inline struct kmem_cache_node *ge
 #endif
 }
 
-static inline struct kmem_cache_cpu *get_cpu_slab(struct kmem_cache *s, int cpu)
-{
-#ifdef CONFIG_SMP
-	return s->cpu_slab[cpu];
-#else
-	return &s->cpu_slab;
-#endif
-}
-
 /*
  * The end pointer in a slab is special. It points to the first object in the
  * slab but has bit 0 set to mark it.

@@ -1472,7 +1463,7 @@ static inline void flush_slab(struct kme
  */
 static inline void __flush_cpu_slab(struct kmem_cache *s, int cpu)
 {
-	struct kmem_cache_cpu *c = get_cpu_slab(s, cpu);
+	struct kmem_cache_cpu *c = CPU_PTR(s->cpu_slab, cpu);
 
 	if (likely(c && c->page))
 		flush_slab(s, c);

@@ -1487,15 +1478,7 @@ static void flush_cpu_slab(void *d)
 
 static void flush_all(struct kmem_cache *s)
 {
-#ifdef CONFIG_SMP

mm/slub.c +9 -0

 /*

@@ -1511,6 +1494,15 @@ static inline int node_match(struct kmem
 	return 1;
 }
 
+static inline int cpu_node_match(struct kmem_cache_cpu *c, int node)
+{
+#ifdef CONFIG_NUMA
+	if (node != -1 && __CPU_READ(c->node) != node)
+		return 0;
+#endif
+	return 1;
+}
+
 /* Allocate a new slab and make it the current cpu slab */
 static noinline unsigned long get_new_slab(struct kmem_cache *s,
 	struct kmem_cache_cpu **pc, gfp_t gfpflags, int node)

mm/slub.c +5 -3

 	if (c->page)
 		flush_slab(s, c);
 	c->page = page;

@@ -1554,16 +1546,18 @@ static noinline unsigned long get_new_sl
  * we need to allocate a new slab. This is slowest path since we may sleep.
  */
 static void *__slab_alloc(struct kmem_cache *s,
-		gfp_t gfpflags, int node, void *addr, struct kmem_cache_cpu *c)
+		gfp_t gfpflags, int node, void *addr)
 {
 	void **object;
 	unsigned long state;
-#ifdef CONFIG_FAST_CMPXCHG_LOCAL
+	struct kmem_cache_cpu *c;
+#ifdef CONFIG_FAST_CPU_OPS
 	unsigned long flags;
 
 	local_irq_save(flags);
 	preempt_enable_no_resched();
 #endif
+	c = THIS_CPU(s->cpu_slab);
 	if (likely(c->page)) {
 		state = slab_lock(c->page);
 

@@ -1597,7 +1591,7 @@ load_freelist:
 unlock_out:
 	slab_unlock(c->page, state);
 out:
-#ifdef CONFIG_FAST_CMPXCHG_LOCAL
+#ifdef CONFIG_FAST_CPU_OPS
 	preempt_disable();
 	local_irq_restore(flags);
 #endif

mm/slub.c +5 -5

+	c = s->cpu_slab;
 	do {
-		object = c->freelist;
-		if (unlikely(is_end(object) || !node_match(c, node))) {
-			object = __slab_alloc(s, gfpflags, node, addr, c);
-			if (unlikely(!object)) {
-				put_cpu();
+		object = __CPU_READ(c->freelist);
+		if (unlikely(is_end(object) ||
+					!cpu_node_match(c, node))) {
+			object = __slab_alloc(s, gfpflags, node, addr);

mm/slub.c +4 -4

 #else
 	unsigned long flags;
 
 	local_irq_save(flags);
-	c = get_cpu_slab(s, smp_processor_id());
+	c = THIS_CPU(s->cpu_slab);
 	if (unlikely((is_end(c->freelist)) || !node_match(c, node))) {
 
 		object = __slab_alloc(s, gfpflags, node, addr, c);

@@ -1709,7 +1701,7 @@ static void __slab_free(struct kmem_cach
 	void **object = (void *)x;
 	unsigned long state;
 
-#ifdef CONFIG_FAST_CMPXCHG_LOCAL
+#ifdef CONFIG_FAST_CPU_OPS
 	unsigned long flags;
 
 	local_irq_save(flags);

@@ -1739,7 +1731,7 @@ checks_ok:
 
 out_unlock:
 	slab_unlock(page, state);
-#ifdef CONFIG_FAST_CMPXCHG_LOCAL
+#ifdef CONFIG_FAST_CPU_OPS
 	local_irq_restore(flags);
 #endif
 	return;

@@ -1752,7 +1744,7 @@ slab_empty:
 		remove_partial(s, page);
 
 	slab_unlock(page, state);
-#ifdef CONFIG_FAST_CMPXCHG_LOCAL
+#ifdef CONFIG_FAST_CPU_OPS
 	local_irq_restore(flags);
 #endif
 	discard_slab(s, page);

@@ -1781,13 +1773,13 @@ static void __always_inline slab_free(st
 	void **object = (void *)x;
 	struct kmem_cache_cpu *c;
 

mm/slub.c +16 -136

 			break;
 		}
-		object[c->offset] = freelist;
-	} while (cmpxchg_local(&c->freelist, freelist, object) != freelist);
-	put_cpu();
+		object[__CPU_READ(c->offset)] = freelist;
+	} while (CPU_CMPXCHG(c->freelist, freelist, object) != freelist);
 #else
 	unsigned long flags;
 
 	local_irq_save(flags);
 	debug_check_no_locks_freed(object, s->objsize);
-	c = get_cpu_slab(s, smp_processor_id());
+	c = THIS_CPU(s->cpu_slab);
 	if (likely(page == c->page && c->node >= 0)) {
 		object[c->offset] = c->freelist;
 		c->freelist = object;

@@ -2015,130 +2007,19 @@ static void init_kmem_cache_node(struct 
 #endif
 }
 
-#ifdef CONFIG_SMP
-/*
- * Per cpu array for per cpu structures.
- *
- * The per cpu array places all kmem_cache_cpu structures from one processor
- * close together meaning that it becomes possible that multiple per cpu
- * structures are contained in one cacheline. This may be particularly
- * beneficial for the kmalloc caches.
- *
- * A desktop system typically has around 60-80 slabs. With 100 here we are
- * likely able to get per cpu structures for all caches from the array defined
- * here. We must be able to cover all kmalloc caches during bootstrap.
- *
- * If the per cpu array is exhausted then fall back to kmalloc
- * of individual cachelines. No sharing is possible then.
- */
-#define NR_KMEM_CACHE_CPU 100
-
-static DEFINE_PER_CPU(struct kmem_cache_cpu,
-				kmem_cache_cpu)[NR_KMEM_CACHE_CPU];
-
-static DEFINE_PER_CPU(struct kmem_cache_cpu *, kmem_cache_cpu_free);
-static cpumask_t kmem_cach_cpu_free_init_once = CPU_MASK_NONE;
-
-static struct kmem_cache_cpu *alloc_kmem_cache_cpu(struct kmem_cache *s,
-							int cpu, gfp_t flags)
-{
-	struct kmem_cache_cpu *c = per_cpu(kmem_cache_cpu_free, cpu);
-
-	if (c)
-		per_cpu(kmem_cache_cpu_free, cpu) =
-				(void *)c->freelist;
-	else {
-		/* Table overflow: So allocate ourselves */
-		c = kmalloc_node(
-			ALIGN(sizeof(struct kmem_cache_cpu), cache_line_size()),
-			flags, cpu_to_node(cpu));
-		if (!c)
-			return NULL;
-	}
-
-	init_kmem_cache_cpu(s, c);
-	return c;
-}
-
-static void free_kmem_cache_cpu(struct kmem_cache_cpu *c, int cpu)
-{
-	if (c < per_cpu(kmem_cache_cpu, cpu) ||
-			c > per_cpu(kmem_cache_cpu, cpu) + NR_KMEM_CACHE_CPU) {
-		kfree(c);
-		return;
-	}
-	c->freelist = (void *)per_cpu(kmem_cache_cpu_free, cpu);
-	per_cpu(kmem_cache_cpu_free, cpu) = c;
-}
-
-static void free_kmem_cache_cpus(struct kmem_cache *s)
-{
-	int cpu;
-
-	for_each_online_cpu(cpu) {
-		struct kmem_cache_cpu *c = get_cpu_slab(s, cpu);
-
-		if (c) {
-			s->cpu_slab[cpu] = NULL;
-			free_kmem_cache_cpu(c, cpu);
-		}
-	}
-}
-
 static int alloc_kmem_cache_cpus(struct kmem_cache *s, gfp_t flags)
 {
 	int cpu;
 
-	for_each_online_cpu(cpu) {
-		struct kmem_cache_cpu *c = get_cpu_slab(s, cpu);
+	s->cpu_slab = CPU_ALLOC(struct kmem_cache_cpu, flags);
 
-		if (c)
-			continue;
-
-		c = alloc_kmem_cache_cpu(s, cpu, flags);
-		if (!c) {
-			free_kmem_cache_cpus(s);
-			return 0;
-		}
-		s->cpu_slab[cpu] = c;
-	}
-	return 1;
-}
-
-/*
- * Initialize the per cpu array.
- */
-static void init_alloc_cpu_cpu(int cpu)
-{
-	int i;
-
-	if (cpu_isset(cpu, kmem_cach_cpu_free_init_once))
-		return;
-
-	for (i = NR_KMEM_CACHE_CPU - 1; i >= 0; i--)
-		free_kmem_cache_cpu(&per_cpu(kmem_cache_cpu, cpu)[i], cpu);
-
-	cpu_set(cpu, kmem_cach_cpu_free_init_once);
-}
-
-static void __init init_alloc_cpu(void)
-{
-	int cpu;
+	if (!s->cpu_slab)
+		return 0;
 
 	for_each_online_cpu(cpu)
-		init_alloc_cpu_cpu(cpu);
-  }
-
-#else
-static inline void free_kmem_cache_cpus(struct kmem_cache *s) {}
-static inline void init_alloc_cpu(void) {}
-
-static inline int alloc_kmem_cache_cpus(struct kmem_cache *s, gfp_t flags)
-{
-	init_kmem_cache_cpu(s, &s->cpu_slab);
+		init_kmem_cache_cpu(s, CPU_PTR(s->cpu_slab, cpu));
 	return 1;
 }
-#endif
 
 #ifdef CONFIG_NUMA
 /*

@@ -2452,9 +2333,8 @@ static inline int kmem_cache_close(struc
 	int node;
 
 	flush_all(s);
-
+	CPU_FREE(s->cpu_slab);
 	/* Attempt to free all objects */
-	free_kmem_cache_cpus(s);
 	for_each_node_state(node, N_NORMAL_MEMORY) {
 		struct kmem_cache_node *n = get_node(s, node);
 

@@ -2958,8 +2838,6 @@ void __init kmem_cache_init(void)
 	int i;
 	int caches = 0;
 
-	init_alloc_cpu();
-
 #ifdef CONFIG_NUMA
 	/*
 	 * Must first have the slab cache available for the allocations of the

@@ -3019,11 +2897,12 @@ void __init kmem_cache_init(void)
 	for (i = KMALLOC_SHIFT_LOW; i < PAGE_SHIFT; i++)
 		kmalloc_caches[i]. name =
 			kasprintf(GFP_KERNEL, "kmalloc-%d", 1 << i);
-
 #ifdef CONFIG_SMP
 	register_cpu_notifier(&slab_notifier);
-	kmem_size = offsetof(struct kmem_cache, cpu_slab) +
-				nr_cpu_ids * sizeof(struct kmem_cache_cpu *);
+#endif
+#ifdef CONFIG_NUMA
+	kmem_size = offsetof(struct kmem_cache, node) +
+				nr_node_ids * sizeof(struct kmem_cache_node *);
 #else
 	kmem_size = sizeof(struct kmem_cache);
 #endif

@@ -3120,7 +2999,7 @@ struct kmem_cache *kmem_cache_create(con
 		 * per cpu structures
 		 */
 		for_each_online_cpu(cpu)
-			get_cpu_slab(s, cpu)->objsize = s->objsize;
+			CPU_PTR(s->cpu_slab, cpu)->objsize = s->objsize;
 		s->inuse = max_t(int, s->inuse, ALIGN(size, sizeof(void *)));
 		up_write(&slub_lock);
 		if (sysfs_slab_alias(s, name))

@@ -3165,11 +3044,9 @@ static int __cpuinit slab_cpuup_callback
 	switch (action) {
 	case CPU_UP_PREPARE:
 	case CPU_UP_PREPARE_FROZEN:
-		init_alloc_cpu_cpu(cpu);
 		down_read(&slub_lock);
 		list_for_each_entry(s, &slab_caches, list)
-			s->cpu_slab[cpu] = alloc_kmem_cache_cpu(s, cpu,
-							GFP_KERNEL);
+			init_kmem_cache_cpu(s, __CPU_PTR(s->cpu_slab, cpu));
 		up_read(&slub_lock);
 		break;
 

@@ -3179,13 +3056,9 @@ static int __cpuinit slab_cpuup_callback
 	case CPU_DEAD_FROZEN:
 		down_read(&slub_lock);
 		list_for_each_entry(s, &slab_caches, list) {
-			struct kmem_cache_cpu *c = get_cpu_slab(s, cpu);
-
 			local_irq_save(flags);
 			__flush_cpu_slab(s, cpu);
 			local_irq_restore(flags);
-			free_kmem_cache_cpu(c, cpu);
-			s->cpu_slab[cpu] = NULL;
 		}
 		up_read(&slub_lock);
 		break;

@@ -3657,7 +3530,7 @@ static unsigned long slab_objects(struct
 	for_each_possible_cpu(cpu) {
 		struct page *page;
 		int node;
-		struct kmem_cache_cpu *c = get_cpu_slab(s, cpu);
+		struct kmem_cache_cpu *c = CPU_PTR(s->cpu_slab, cpu);
 
 		if (!c)
 			continue;

@@ -3724,7 +3597,7 @@ static int any_slab_objects(struct kmem_
 	int cpu;
 
 	for_each_possible_cpu(cpu) {
-		struct kmem_cache_cpu *c = get_cpu_slab(s, cpu);
+		struct kmem_cache_cpu *c = CPU_PTR(s->cpu_slab, cpu);
 
 		if (c && c->page)
 			return 1;

Index: linux-2.6/arch/x86/Kconfig
===================================================================

arch/x86/Kconfig +0 -4

@@ -112,10 +112,6 @@ config GENERIC_TIME_VSYSCALL
 	bool
 	default X86_64
 
-config FAST_CMPXCHG_LOCAL
-	bool
-	default y
-
 config ZONE_DMA32
 	bool
 	default X86_64

Signature · 1 line

-- 

mm/slub.c static void flush_cpu_slab(void *d) · line 1482

Quoted 12 lines

{

mm/slub.c +0 -8

-#ifdef CONFIG_SMP
 	on_each_cpu(flush_cpu_slab, s, 1, 1);
-#else
-	unsigned long flags;
-
-	local_irq_save(flags);
-	flush_cpu_slab(s);
-	local_irq_restore(flags);
-#endif
 }

Normally :

You can't use on_each_cpu if interrupts are already disabled. Therefore, the implementation using "local_irq_disable/enable" in smp.h for the UP case is semantically correct and there is no need to use a save/restore. So just using on_each_cpu should be enough here.

I also wonder about flush_cpu_slab execution on _other_ cpus. I am not convinced interrupts are disabled when it executes.. have I missing something ?

mm/slub.c static void __always_inline *slab_alloc( · line 1637

Quoted 3 lines

mm/slub.c +1 -2

-#ifdef CONFIG_FAST_CMPXCHG_LOCAL
-	c = get_cpu_slab(s, get_cpu());
+#ifdef CONFIG_FAST_CPU_OPS

I wonder.. are there some architectures that would provide fast cmpxchg_local but not fast cpu ops ?

mm/slub.c static void __always_inline *slab_alloc( · line 1649

Quoted 2 lines

mm/slub.c +2 -0

+	} while (CPU_CMPXCHG(c->freelist, object,
+			object[__CPU_READ(c->offset)]) != object);

Hrm. What happens here if we call __slab_alloc, get a valid object, then have a CPU_CMPXCHG that fails, restart the loop.. is this case taken care of or do we end up having an unreferenced object ? Maybe there is some logic in freelist that takes care of it ?

Also, we have to be aware that we can now change CPU between the

__CPU_READ and the CPU_CMPXCHG. (also : should it be a __CPU_CMPXCHG ?)

But since "object" contains information specific to the local CPU, the cmpxchg should fail if we are migrated and everything should be ok.

Hrm, actually, the

c = s->cpu_slab;

should probably be after the object = __CPU_READ(c->freelist); ?

The cpu_read acts as a safeguard checking that we do not change CPU between the read and the cmpxchg. If we are preempted between the "c" read and the cpu_read, we could do a !cpu_node_match(c, node) check that would apply to the wrong cpu.

Quoted 4 lines

mm/slub.c +0 -0

@@ -1800,19 +1792,19 @@ static void __always_inline slab_free(st
 		 * then any change of cpu_slab will cause the cmpxchg to fail
 		 * since the freelist pointers are unique per slab.
 		 */

mm/slub.c static void __always_inline slab_free(st · line 1797

Quoted 5 lines

mm/slub.c +3 -2

-		if (unlikely(page != c->page || c->node < 0)) {
-			__slab_free(s, page, x, addr, c->offset);
+		if (unlikely(page != __CPU_READ(c->page) ||
+					__CPU_READ(c->node) < 0)) {
+			__slab_free(s, page, x, addr, __CPU_READ(c->offset));

And same question as above : what happens if we fail after executing the __slab_free.. is it valid to do it twice ?

Quoted 4 lines

The cpu_read acts as a safeguard checking that we do not change CPU between the read and the cmpxchg. If we are preempted between the "c" read and the cpu_read, we could do a !cpu_node_match(c, node) check that would apply to the wrong cpu.

C is not pointing to a specific cpu. It can only be used in CPU_xx ops to address the currrent cpu.

Quoted 4 lines

Quoted 4 lines

mm/slub.c +0 -0

@@ -1800,19 +1792,19 @@ static void __always_inline slab_free(st
 		 * then any change of cpu_slab will cause the cmpxchg to fail
 		 * since the freelist pointers are unique per slab.
 		 */

Quoted 8 lines

mm/slub.c static void __always_inline slab_free(st · line 1797

Quoted 5 lines

mm/slub.c +3 -2

-		if (unlikely(page != c->page || c->node < 0)) {
-			__slab_free(s, page, x, addr, c->offset);
+		if (unlikely(page != __CPU_READ(c->page) ||
+					__CPU_READ(c->node) < 0)) {
+			__slab_free(s, page, x, addr, __CPU_READ(c->offset));

And same question as above : what happens if we fail after executing the __slab_free.. is it valid to do it twice ?

__slab_free is always successful and will never cause a repeat of the loop.

Quoted 8 lines

Quoted 7 lines

mm/slub.c static void __always_inline slab_free(st · line 1797

Quoted 4 lines

mm/slub.c +3 -1

-			__slab_free(s, page, x, addr, c->offset);
+		if (unlikely(page != __CPU_READ(c->page) ||
+					__CPU_READ(c->node) < 0)) {
+			__slab_free(s, page, x, addr, __CPU_READ(c->offset));

And same question as above : what happens if we fail after executing the __slab_free.. is it valid to do it twice ?

Quoted 2 lines

__slab_free is always successful and will never cause a repeat of the loop.

Then what happens if we are migrated between the end of __slab_free and the CPU_CMPXCHG ? The cmpxchg would fail, and the __slab_free will be done twice ?

Quoted 8 lines · Mathieu Desnoyers

Quoted 8 lines

Quoted 7 lines

mm/slub.c static void __always_inline slab_free(st · line 1797

Quoted 4 lines

mm/slub.c +3 -1

-			__slab_free(s, page, x, addr, c->offset);
+		if (unlikely(page != __CPU_READ(c->page) ||
+					__CPU_READ(c->node) < 0)) {
+			__slab_free(s, page, x, addr, __CPU_READ(c->offset));

And same question as above : what happens if we fail after executing the __slab_free.. is it valid to do it twice ?

Quoted 6 lines

Quoted 2 lines

__slab_free is always successful and will never cause a repeat of the loop.

Then what happens if we are migrated between the end of __slab_free and the CPU_CMPXCHG ? The cmpxchg would fail, and the __slab_free will be done twice ?

Look at the control flow. The __slab_free can never be done twice.

{
                freelist = __CPU_READ(c->freelist);
                barrier();
                if (unlikely(page != __CPU_READ(c->page) ||
                                        __CPU_READ(c->node) < 0)) {
                        __slab_free(s, page, x, addr);
                        break;
			^^^^^^^ no redo of the loop possible.

                }
                set_freepointer(s, object, freelist);
        } while (CPU_CMPXCHG(c->freelist, freelist, object) != freelist);

For the UP and SMP case map the area using 4k ptes. Typical use of per cpu data is around 16k for UP and SMP configurations. It goes up to 45k when the per cpu area is managed by cpu_alloc (see special x86_64 patchset). Allocating in 2M segments would be overkill.

For NUMA map the area using 2M PMDs. A large NUMA system may use lots of cpu data for the page allocator data alone. We typically have large amounts of memory around on those size. Using a 2M page size reduces TLB pressure for that case.

Some numbers for envisioned maximum configurations of NUMA systems:

4k cpu configurations with 1k nodes:

	4096 * 16MB = 64TB of virtual space.

Maximum theoretical configuration 16384 processors 1k nodes:

	16384 * 16MB = 256TB of virtual space.

Both fit within the established limits established.

You're making the assumption here that NUMA = large number of CPUs. This assumption is flat-out wrong.

On x86-64, most two-socket systems are still NUMA, and I would expect that most distro kernels probably compile in NUMA. However, burning megabytes of memory on a two-socket dual-core system when we're talking about tens of kilobytes used would be more than a wee bit insane.

Quoted 2 lines · H. Peter Anvin

You're making the assumption here that NUMA = large number of CPUs. This assumption is flat-out wrong.

Well maybe. Usually one gets to NUMA because the hardware gets too big to be handleed the UMA way.

Quoted 4 lines

On x86-64, most two-socket systems are still NUMA, and I would expect that most distro kernels probably compile in NUMA. However, burning megabytes of memory on a two-socket dual-core system when we're talking about tens of kilobytes used would be more than a wee bit insane.

Yeah yea but the latencies are minimal making the NUMA logic too expensive for most loads ... If you put a NUMA kernel onto those then performance drops (I think someone measures 15-30%?)

Quoted 2 lines · H. Peter Anvin

You're making the assumption here that NUMA = large number of CPUs. This assumption is flat-out wrong.

Well maybe. Usually one gets to NUMA because the hardware gets too big to be handleed the UMA way.

Quoted 2 lines · H. Peter Anvin

You're making the assumption here that NUMA = large number of CPUs. This assumption is flat-out wrong.

Well maybe. Usually one gets to NUMA because the hardware gets too big to be handleed the UMA way.

Quoted 4 lines

On x86-64, most two-socket systems are still NUMA, and I would expect that most distro kernels probably compile in NUMA. However, burning megabytes of memory on a two-socket dual-core system when we're talking about tens of kilobytes used would be more than a wee bit insane.

Yeah yea but the latencies are minimal making the NUMA logic too expensive for most loads ... If you put a NUMA kernel onto those then performance drops (I think someone measures 15-30%?)

Quoted 4 lines

Yeah yea but the latencies are minimal making the NUMA logic too expensive for most loads ... If you put a NUMA kernel onto those then performance drops (I think someone measures 15-30%?)

Small socket count systems are going to increasingly be NUMA in future. If CONFIG_NUMA hurts performance by that much on those systems, then the kernel is broken IMO.

4k cpu configurations with 1k nodes:

	4096 * 16MB = 64TB of virtual space.

Maximum theoretical configuration 16384 processors 1k nodes:

	16384 * 16MB = 256TB of virtual space.

Both fit within the established limits established.

I might be pointing out the obvious, but on x86-64 there is definitely not 256TB of VM available for this.

Quoted 2 lines · Andi Kleen

I might be pointing out the obvious, but on x86-64 there is definitely not 256TB of VM available for this.

Well maybe in the future.

One of the issues that I ran into is that I had to place the cpu area in between to make the offsets link right.

However, it would be best if the cpuarea came *after* the modules area. We only need linking that covers the per cpu area of processor 0.

So I think we have a 2GB area right?

1GB kernel
1GB - 1x per cpu area (128M?) modules?
cpu aree 0
---- 2GB limit
cpu area 1
cpu area 2
....

For that we would need to move the kernel down a bit. Can we do that?

Quoted 1 line

So I think we have a 2GB area right?

For everything that needs the -31bit offsets; that is everything linked

Quoted 8 lines

1GB kernel
1GB - 1x per cpu area (128M?) modules?
cpu aree 0
---- 2GB limit
cpu area 1
cpu area 2
....

Quoted 1 line

For that we would need to move the kernel down a bit. Can we do that?

The kernel model requires kernel and modules and everything else linked be in negative -31bit space. That is how the kernel code model is defined.

You could in theory move the modules, but then you would need to implement a full PIC dynamic linker for them first and also increase runtime overhead for them because they would need to use a GOT/PLT.

I suspect all of this would cause far more overhead all over the kernel than you could ever save with the per cpu data in your fast paths.

Right so I could move the kernel to

#define __PAGE_OFFSET _AC(0xffff810000000000, UL)
#define __START_KERNEL_map_AC(0xfffffff800000000, UL)

Quoted 3 lines

You could in theory move the modules, but then you would need to implement a full PIC dynamic linker for them first and also increase runtime overhead for them because they would need to use a GOT/PLT.

Why is it not possible to move the kernel lower while keeping bit 31 1?

Quoted 2 lines

I suspect all of this would cause far more overhead all over the kernel than you could ever save with the per cpu data in your fast paths.

Moving the kernel down a bit seems to be trivial without any of the weird solutions.

Quoted 6 lines

Right so I could move the kernel to

#define __PAGE_OFFSET _AC(0xffff810000000000, UL)
#define __START_KERNEL_map_AC(0xfffffff800000000, UL)

That is -31GB unless I'm miscounting. But it needs to be >= -2GB (31bits)

Right now it is at -2GB + 2MB, because it is loaded at physical +2MB so it's convenient to identity map there. In theory you could avoid that with some effort, but that would only buy you 2MB and would also break some early code and earlyprintk I believe.

Quoted 5 lines

Quoted 3 lines

You could in theory move the modules, but then you would need to implement a full PIC dynamic linker for them first and also increase runtime overhead for them because they would need to use a GOT/PLT.

Why is it not possible to move the kernel lower while keeping bit 31 1?

The kernel model relies on 32bit sign extension. This means bits [31;63] have to be all 1

Quoted 5 lines

Quoted 2 lines

I suspect all of this would cause far more overhead all over the kernel than you could ever save with the per cpu data in your fast paths.

Moving the kernel down a bit seems to be trivial without any of the weird solutions.

Another one I came up in the previous mail would be to do the linker reference variable allocation in [0;2GB] positive space; but do all real references only %gs relative. And keep the real data copy on some other address. That would be a similar trick to the old style x86-64 vsyscalls. It gets fairly messy in the linker map file though.

32bit sign extension for what? Absolute data references? The addressing that I have seen was IP relative. Thus I thought that the kernel could be moved lower.

But one can subtract too...

Hmmm... So the cpu area 0 could be put at
the beginning of the 2GB kernel area and then grow downwards from 0xffffffff80000000. The cost in terms of code is one subtract instruction for each per_cpu() or CPU_PTR()

The next thing doward from 0xffffffff80000000 is the vmemmap at 0xffffe20000000000, so ~32TB. If we leave 16TB for the vmemmap (a 16TB vmmemmap be able to map 2^(44 - 6 + 12) = 2^50 bytes more than currently supported by the processors)

then the remaining 16TB could be used to map 1GB per cpu for a 16k config. That is wildly overdoing it. Guess we could just do it with 1M anyways. Just to be safe we could do 128M. 128M x 16k = 2TB?

Would such a configuration be okay?

Quoted 1 line · On Wednesday 21 November 2007 02:16:11 Christoph Lameter wrote:

But one can subtract too...

The linker cannot subtract (unless you add a new relocation types)

All you need is a 2MB area (16MB is too large if you really want 16k CPUs someday) somewhere in the -2GB or probably better in +2GB. Then the linker puts stuff in there and you use the offsets for referencing relative to %gs.

Then for all CPUs (including CPU #0) you put the real mapping somewhere else, copy the reference data there (which also doesn't need to be on the offset the linker assigned, just on a constant offset from it somewhere in the normal kernel data) and off you go.

Then the reference data would be initdata and eventually freed. That is similar to how the current per cpu data works.

Quoted 5 lines

All you need is a 2MB area (16MB is too large if you really want 16k CPUs someday) somewhere in the -2GB or probably better in +2GB. Then the linker puts stuff in there and you use the offsets for referencing relative to %gs.

2MB * 16k = 32GB. Even with 4k cpus we will have 2M * 4k = 8GB both do not fit in the 2GB area.

The offset relative to %gs cannot be used if you have a loop and are calculating the addresses for all instances. That is what we are talking about. The CPU_xxx operations that are using the %gs register are fine and are not affected by the changes we are discussing.

Quoted 3 lines

Then the reference data would be initdata and eventually freed. That is similar to how the current per cpu data works.

Yes that is also how the current patchset works. I just do not understand what you want changed.

The whole mapping for all CPUs cannot fit into 2GB of course, but the reference linker managed range can.

Quoted 4 lines

The offset relative to %gs cannot be used if you have a loop and are calculating the addresses for all instances. That is what we are talking about. The CPU_xxx operations that are using the %gs register are fine and are not affected by the changes we are discussing.

Sure it can -- you just get the base address from a global array and then add the offset

Quoted 6 lines

Quoted 3 lines

Then the reference data would be initdata and eventually freed. That is similar to how the current per cpu data works.

Yes that is also how the current patchset works. I just do not understand what you want changed.

Anyways i think your current scheme cannot work (too much VM, placed at the wrong place; some wrong assumptions).

Quoted 4 lines · Christoph Lameter

Quoted 2 lines · Andi Kleen

I might be pointing out the obvious, but on x86-64 there is definitely not 256TB of VM available for this.

Well maybe in the future.

That would either require more than 4 levels or larger pages in page tables.

Quoted 2 lines

One of the issues that I ran into is that I had to place the cpu area in between to make the offsets link right.

Above -2GB, otherwise you cannot address them

This limitation shouldn't apply to the percpu area, since gs_base can be pointed anywhere in the address space -- in effect we're always indirect.

Quoted 3 lines

This limitation shouldn't apply to the percpu area, since gs_base can be pointed anywhere in the address space -- in effect we're always indirect.

The initial reference copy of the percpu area has to be addressed by the linker.

Hmm, in theory since it is not actually used by itself I suppose you could move it into positive space.

Quoted 3 lines · Andi Kleen

Quoted 1 line

This limitation shouldn't apply to the percpu area, since gs_base can be pointed anywhere in the address space -- in effect we're always indirect.

The initial reference copy of the percpu area has to be addressed by the linker.

Right that is important for the percpu references that can be folded by the linker in order to avoid address calculations.

Quoted 1 line

Hmm, in theory since it is not actually used by itself I suppose you could move it into positive space.

But the positive space is reserved for a processes memory.

But you wouldn't actually *use* this address space. It's just for the linker to know what address to tag the references with; it gets relocated by gs_base down into proper kernel space. The linker can stash the initialized reference copy at any address (LMA) which can be different from what it will be used at (VMA); that is not an issue.

To use %rip references, though, which are more efficient, you probably want to use offsets that are just below .text (at -2 GB); presumably
-2 GB-[max size of percpu section]. Again, however, no CPU actually needs to have its data stashed in that particular location; it's just an offset.

Quoted 1 line

This limitation shouldn't apply to the percpu area, since gs_base can be pointed anywhere in the address space -- in effect we're always indirect.

The initial reference copy of the percpu area has to be addressed by the linker.

Hmm, in theory since it is not actually used by itself I suppose you could move it into positive space.

Quoted 1 line · Christoph Lameter

On Tue, 20 Nov 2007, Andi Kleen wrote:

You could in theory move the modules, but then you would need to implement a full PIC dynamic linker for them first and also increase runtime overhead for them because they would need to use a GOT/PLT.

Signed-off-by Christoph Lameter <[email protected]>

---
 arch/ia64/kernel/crash.c |    2 +-
 drivers/base/cpu.c       |    2 +-
 kernel/kexec.c           |    4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

Index: linux-2.6/arch/ia64/kernel/crash.c
===================================================================

arch/ia64/kernel/crash.c +1 -1

@@ -71,7 +71,7 @@ crash_save_this_cpu(void)
 	dst[46] = (unsigned long)ia64_rse_skip_regs((unsigned long *)dst[46],
 			sof - sol);
 
-	buf = (u64 *) per_cpu_ptr(crash_notes, cpu);
+	buf = (u64 *) CPU_PTR(crash_notes, cpu);
 	if (!buf)
 		return;
 	buf = append_elf_note(buf, KEXEC_CORE_NOTE_NAME, NT_PRSTATUS, prstatus,

Index: linux-2.6/drivers/base/cpu.c
===================================================================

drivers/base/cpu.c +1 -1

@@ -95,7 +95,7 @@ static ssize_t show_crash_notes(struct s
 	 * boot up and this data does not change there after. Hence this
 	 * operation should be safe. No locking required.
 	 */
-	addr = __pa(per_cpu_ptr(crash_notes, cpunum));
+	addr = __pa(CPU_PTR(crash_notes, cpunum));
 	rc = sprintf(buf, "%Lx\n", addr);
 	return rc;
 }

Index: linux-2.6/kernel/kexec.c
===================================================================

kernel/kexec.c +1 -1

 	if (!buf)
 		return;
 	memset(&prstatus, 0, sizeof(prstatus));

@@ -1136,7 +1136,7 @@ void crash_save_cpu(struct pt_regs *regs
 static int __init crash_notes_memory_init(void)
 {
 	/* Allocate memory for saving cpu registers. */
-	crash_notes = alloc_percpu(note_buf_t);
+	crash_notes = CPU_ALLOC(note_buf_t, GFP_KERNEL|__GFP_ZERO);
 	if (!crash_notes) {
 		printk("Kexec: Memory allocation for saving cpu register"
 		" states failed\n");

Signature · 1 line

-- 

kernel/kexec.c void crash_save_cpu(struct pt_regs *regs · line 1125

Quoted 2 lines

kernel/kexec.c +1 -1

-	buf = (u32*)per_cpu_ptr(crash_notes, cpu);
+	buf = (u32*)CPU_PTR(crash_notes, cpu);

Nitpick : (u32 *)