cpu alloc / cpu ops v3: Optimize per cpu access

include/asm-ia64/page.h line 45

Quoted 6 lines

The per cpu allocator requires more per cpu space and we are already near the limit on IA64. Increase the maximum size of the IA64 per cpu area from 64K to 128K.

include/asm-ia64/page.h +1 -1

-#define PERCPU_PAGE_SHIFT	16	/* log2() of max. size of per-CPU area */
+#define PERCPU_PAGE_SHIFT	17	/* log2() of max. size of per-CPU area */

Don't you need some more changes to the alt_dtlb_miss handler in ivt.S for this to work? 128K is not a supported pagesize on any processor model.

include/asm-ia64/page.h line 45

Quoted 6 lines

The per cpu allocator requires more per cpu space and we are already near the limit on IA64. Increase the maximum size of the IA64 per cpu area from 64K to 128K.

include/asm-ia64/page.h +1 -1

-#define PERCPU_PAGE_SHIFT	16	/* log2() of max. size of per-CPU area */
+#define PERCPU_PAGE_SHIFT	17	/* log2() of max. size of per-CPU area */

Don't you need some more changes to the alt_dtlb_miss handler in ivt.S for this to work? 128K is not a supported pagesize on any processor model.

Quoted 6 lines

include/asm-ia64/page.h line 45

Quoted 2 lines

include/asm-ia64/page.h +1 -1

-#define PERCPU_PAGE_SHIFT	16	/* log2() of max. size of per-CPU area */
+#define PERCPU_PAGE_SHIFT	17	/* log2() of max. size of per-CPU area */

Don't you need some more changes to the alt_dtlb_miss handler in ivt.S for this to work? 128K is not a supported pagesize on any processor model.

Ok so this needs to be 18?

The per cpu allocator allows dynamic allocation of memory on all processors simultaneously. A bitmap is used to track used areas. The allocator implements tight packing to reduce the cache footprint and increase speed since cacheline contention is typically not a concern for memory mainly used by a single cpu. Small objects will fill up gaps left by larger allocations that required alignments.

The size of the cpu_alloc area can be changed via make menuconfig.

...

mm/Kconfig +2 -0

+	help
+	  Sets the maximum amount of memory that can be allocated via cpu_alloc

Index: linux-2.6/mm/Makefile
===================================================================

mm/Makefile +1 -1

@@ -11,7 +11,7 @@ obj-y			:= bootmem.o filemap.o mempool.o
 			   maccess.o page_alloc.o page-writeback.o pdflush.o \
 			   readahead.o swap.o truncate.o vmscan.o \
 			   prio_tree.o util.o mmzone.o vmstat.o backing-dev.o \
-			   page_isolation.o $(mmu-y)
+			   page_isolation.o cpu_alloc.o $(mmu-y)
 
 obj-$(CONFIG_PROC_PAGE_MONITOR) += pagewalk.o
 obj-$(CONFIG_BOUNCE)	+= bounce.o

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

mm/cpu_alloc.c new file +28 -0

@@ -0,0 +1,167 @@
+/*
+ * Cpu allocator - Manage objects allocated for each processor
+ *
+ * (C) 2008 SGI, Christoph Lameter <[email protected]>
+ * 	Basic implementation with allocation and free from a dedicated per
+ * 	cpu area.
+ *
+ * The per cpu allocator allows dynamic allocation of memory on all
+ * processor simultaneously. A bitmap is used to track used areas.
+ * The allocator implements tight packing to reduce the cache footprint
+ * and increase speed since cacheline contention is typically not a concern
+ * for memory mainly used by a single cpu. Small objects will fill up gaps
+ * left by larger allocations that required alignments.
+ */
+#include <linux/mm.h>
+#include <linux/mmzone.h>
+#include <linux/module.h>
+#include <linux/percpu.h>
+#include <linux/bitmap.h>
+#include <asm/sections.h>
+
+/*
+ * Basic allocation unit. A bit map is created to track the use of each
+ * UNIT_SIZE element in the cpu area.
+ */
+#define UNIT_TYPE int
+#define UNIT_SIZE sizeof(UNIT_TYPE)
+#define UNITS (CONFIG_CPU_ALLOC_SIZE / UNIT_SIZE)

mm/cpu_alloc.c new file +15 -0

+	spin_lock_irqsave(&cpu_alloc_map_lock, flags);
+
+	first = 1;
+	start = first_free;
+
+	for ( ; ; ) {
+
+		start = find_next_zero_bit(cpu_alloc_map, UNITS, start);
+		if (start >= UNITS)
+			goto out_of_memory;
+
+		if (first)
+			first_free = start;
+
+		/*

mm/cpu_alloc.c new file +34 -0

+	if (first)
+		first_free = start + units;
+
+	if (start + units > UNITS)
+		goto out_of_memory;
+
+	set_map(start, units);
+	__count_vm_events(CPU_BYTES, units * UNIT_SIZE);
+
+	spin_unlock_irqrestore(&cpu_alloc_map_lock, flags);
+
+	ptr = per_cpu_var(area) + start;
+
+	if (gfpflags & __GFP_ZERO) {
+		int cpu;
+
+		for_each_possible_cpu(cpu)
+			memset(CPU_PTR(ptr, cpu), 0, size);
+	}
+
+	return ptr;
+
+out_of_memory:
+	spin_unlock_irqrestore(&cpu_alloc_map_lock, flags);
+	return NULL;
+}
+EXPORT_SYMBOL(cpu_alloc);
+
+/*
+ * Free an object. The pointer must be a cpu pointer allocated
+ * via cpu_alloc.
+ */
+void cpu_free(void *start, unsigned long size)
+{

mm/cpu_alloc.c new file +11 -0

+	spin_lock_irqsave(&cpu_alloc_map_lock, flags);
+
+	clear_map(index, units);
+	__count_vm_events(CPU_BYTES, -units * UNIT_SIZE);
+
+	if (index < first_free)
+		first_free = index;
+
+	spin_unlock_irqrestore(&cpu_alloc_map_lock, flags);
+}
+EXPORT_SYMBOL(cpu_free);

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

mm/vmstat.c +1 -0

@@ -653,6 +653,7 @@ static const char * const vmstat_text[] 
 	"allocstall",
 
 	"pgrotated",
+	"cpu_bytes",
 #ifdef CONFIG_HUGETLB_PAGE
 	"htlb_buddy_alloc_success",
 	"htlb_buddy_alloc_fail",

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

include/linux/percpu.h +18 -0

@@ -135,4 +135,50 @@ static inline void percpu_free(void *__p
 #define free_percpu(ptr)	percpu_free((ptr))
 #define per_cpu_ptr(ptr, cpu)	percpu_ptr((ptr), (cpu))
 
+
+/*
+ * cpu allocator definitions
+ *
+ * The cpu allocator allows allocating an instance of an object for each
+ * processor and the use of a single pointer to access all instances
+ * of the object. cpu_alloc provides optimized means for accessing the
+ * instance of the object belonging to the currently executing processor
+ * as well as special atomic operations on fields of objects of the
+ * currently executing processor.
+ *
+ * Cpu objects are typically small. The allocator packs them tightly
+ * to increase the chance on each access that a per cpu object is already
+ * cached. Alignments may be specified but the intent is to align the data
+ * properly due to cpu alignment constraints and not to avoid cacheline
+ * contention. Any holes left by aligning objects are filled up with smaller
+ * objects that are allocated later.
+ *

include/linux/percpu.h +6 -0

+/*
+ * Raw calls
+ */
+void *cpu_alloc(unsigned long size, gfp_t flags, unsigned long align);
+void cpu_free(void *cpu_pointer, unsigned long size);
+
 #endif /* __LINUX_PERCPU_H */

mm/Kconfig config NR_QUICK · line 211

Quoted 3 lines

mm/Kconfig +3 -0

+config CPU_ALLOC_SIZE
+	int "Size of cpu alloc area"
+	default "30000"

strange choice of a default? I guess it makes it clear that there's no particular advantage in making it a power-of-two or anything like that.

mm/cpu_alloc.c line 38

Quoted 4 lines

mm/cpu_alloc.c new file +4 -0

+static int size_to_units(unsigned long size)
+{
+	return DIV_ROUND_UP(size, UNIT_SIZE);
+}

Perhaps it should return UNIT_TYPE? (ugh).

I guess there's no need to ever change that type, so no?

mm/cpu_alloc.c line 45

Quoted 3 lines

mm/cpu_alloc.c new file +3 -0

+static DEFINE_SPINLOCK(cpu_alloc_map_lock);
+static DECLARE_BITMAP(cpu_alloc_map, UNITS);
+static int first_free;		/* First known free unit */

Would be nicer to move these above size_to_units(), IMO.

mm/cpu_alloc.c line 56

Quoted 5 lines

mm/cpu_alloc.c new file +5 -0

+static void set_map(int start, int length)
+{
+	while (length-- > 0)
+		__set_bit(start++, cpu_alloc_map);
+}

bitmap_fill()?

mm/cpu_alloc.c line 67

Quoted 6 lines

+ */

mm/cpu_alloc.c new file +5 -0

+static void clear_map(int start, int length)
+{
+	while (length-- > 0)
+		__clear_bit(start++, cpu_alloc_map);
+}

bitmap_zero()?

mm/cpu_alloc.c line 84

Quoted 2 lines

mm/cpu_alloc.c new file +2 -0

+	if (!size)
+		return ZERO_SIZE_PTR;

OK, so we reuse ZERO_SIZE_PTR from kmalloc.

mm/cpu_alloc.c line 110

Quoted 3 lines

mm/cpu_alloc.c new file +3 -0

+		start++;
+		first = 0;
+	}

This is kinda bitmap_find_free_region(), only bitmap_find_free_region() isn't quite strong enough.

Generally I think it would have been better if you had added new primitives to the bitmap library (or enhanced existing ones) and used them here, rather than implementing private functionality.

mm/cpu_alloc.c line 155

Quoted 4 lines

mm/cpu_alloc.c new file +4 -0

+
+	BUG_ON(index >= UNITS ||
+		!test_bit(index, cpu_alloc_map) ||
+		!test_bit(index + units - 1, cpu_alloc_map));

If this assertion triggers for someone, you'll wish like hell that it had been implemented as three separate BUG_ONs.

include/linux/percpu.h static inline void percpu_free(void *__p · line 165

Quoted 4 lines

+ */

include/linux/percpu.h +3 -0

+
+/* Return a pointer to the instance of a object for a particular processor */
+#define CPU_PTR(__p, __cpu)	SHIFT_PERCPU_PTR((__p), per_cpu_offset(__cpu))

eek, a major interface function which is ALL IN CAPS!

can we do this in lower-case? In a C function?

Quoted 7 lines

mm/cpu_alloc.c line 110

Quoted 3 lines

mm/cpu_alloc.c new file +3 -0

+		start++;
+		first = 0;
+	}

This is kinda bitmap_find_free_region(), only bitmap_find_free_region() isn't quite strong enough.

Quoted 3 lines

Generally I think it would have been better if you had added new primitives to the bitmap library (or enhanced existing ones) and used them here, rather than implementing private functionality.

The scope of the patchset is already fairly large.

The search here is
different and not performance critical. Not sure if this is useful for other purposes.

Quoted 7 lines

include/linux/percpu.h static inline void percpu_free(void *__p · line 165

Quoted 4 lines

+ */

include/linux/percpu.h +3 -0

+
+/* Return a pointer to the instance of a object for a particular processor */
+#define CPU_PTR(__p, __cpu)	SHIFT_PERCPU_PTR((__p), per_cpu_offset(__cpu))

eek, a major interface function which is ALL IN CAPS!

Quoted 1 line

can we do this in lower-case? In a C function?

No. This is a macro and therefore uppercase (there is macro magic going on that ppl need to be aware of). AFAICR you wanted it this way last year. C function not possible because of the type checking.

Quoted 6 lines

Quoted 5 lines

Quoted 1 line

+#define CPU_PTR(__p, __cpu) SHIFT_PERCPU_PTR((__p), per_cpu_offset(__cpu))

eek, a major interface function which is ALL IN CAPS!

can we do this in lower-case? In a C function?

Quoted 3 lines

No. This is a macro and therefore uppercase (there is macro magic going on that ppl need to be aware of). AFAICR you wanted it this way last year. C function not possible because of the type checking.

urgh. This is a C-convention versus kernel-convention thing. The C convention exists for very good reasons. But it sure does suck.

What do others think?

Quoted 6 lines · Andrew Morton

mm/Kconfig config NR_QUICK · line 211

Quoted 3 lines

mm/Kconfig +3 -0

+config CPU_ALLOC_SIZE
+	int "Size of cpu alloc area"
+	default "30000"

strange choice of a default? I guess it makes it clear that there's no particular advantage in making it a power-of-two or anything like that.

The cpu alloc has a cpu_bytes field in vmstat that shows how much memory is being used. 30000 seemed to be reasonable after staring at these numbers for awhile.

30000 is suitable for both of 32bits/64bits arch ?

mm/cpu_alloc.c line 56

Quoted 10 lines

mm/cpu_alloc.c new file +10 -0

+/*
+ * Mark an object as used in the cpu_alloc_map
+ *
+ * Must hold cpu_alloc_map_lock
+ */
+static void set_map(int start, int length)
+{
+	while (length-- > 0)
+		__set_bit(start++, cpu_alloc_map);
+}

bitmap_fill()?

mm/cpu_alloc.c line 67

Quoted 10 lines

mm/cpu_alloc.c new file +10 -0

+/*
+ * Mark an area as freed.
+ *
+ * Must hold cpu_alloc_map_lock
+ */
+static void clear_map(int start, int length)
+{
+	while (length-- > 0)
+		__clear_bit(start++, cpu_alloc_map);
+}

bitmap_zero()?

mm/cpu_alloc.c line 84

Quoted 10 lines

mm/cpu_alloc.c new file +10 -0

+void *cpu_alloc(unsigned long size, gfp_t gfpflags, unsigned long align)
+{
+	unsigned long start;
+	int units = size_to_units(size);
+	void *ptr;
+	int first;
+	unsigned long flags;
+
+	if (!size)
+		return ZERO_SIZE_PTR;

OK, so we reuse ZERO_SIZE_PTR from kmalloc.

Quoted 15 lines

mm/cpu_alloc.c new file +15 -0

+	spin_lock_irqsave(&cpu_alloc_map_lock, flags);
+
+	first = 1;
+	start = first_free;
+
+	for ( ; ; ) {
+
+		start = find_next_zero_bit(cpu_alloc_map, UNITS, start);
+		if (start >= UNITS)
+			goto out_of_memory;
+
+		if (first)
+			first_free = start;
+
+		/*

mm/cpu_alloc.c line 110

Quoted 10 lines

mm/cpu_alloc.c new file +10 -0

+		 * Check alignment and that there is enough space after
+		 * the starting unit.
+		 */
+		if (start % (align / UNIT_SIZE) == 0 &&
+			find_next_bit(cpu_alloc_map, UNITS, start + 1)
+							>= start + units)
+				break;
+		start++;
+		first = 0;
+	}

This is kinda bitmap_find_free_region(), only bitmap_find_free_region() isn't quite strong enough.

Generally I think it would have been better if you had added new primitives to the bitmap library (or enhanced existing ones) and used them here, rather than implementing private functionality.

The per cpu allocator allows dynamic allocation of memory on all processors simultaneously. A bitmap is used to track used areas. The allocator implements tight packing to reduce the cache footprint and increase speed since cacheline contention is typically not a concern for memory mainly used by a single cpu. Small objects will fill up gaps left by larger allocations that required alignments.

The size of the cpu_alloc area can be changed via make menuconfig.

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

---
 include/linux/percpu.h |   46 +++++++++++++
 include/linux/vmstat.h |    2 
 mm/Kconfig             |    6 +
 mm/Makefile            |    2 
 mm/cpu_alloc.c         |  167 +++++++++++++++++++++++++++++++++++++++++++++++++
 mm/vmstat.c            |    1 
 6 files changed, 222 insertions(+), 2 deletions(-)

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

include/linux/vmstat.h +1 -1

@@ -37,7 +37,7 @@ enum vm_event_item { PGPGIN, PGPGOUT, PS
 		FOR_ALL_ZONES(PGSCAN_KSWAPD),
 		FOR_ALL_ZONES(PGSCAN_DIRECT),
 		PGINODESTEAL, SLABS_SCANNED, KSWAPD_STEAL, KSWAPD_INODESTEAL,
-		PAGEOUTRUN, ALLOCSTALL, PGROTATED,
+		PAGEOUTRUN, ALLOCSTALL, PGROTATED, CPU_BYTES,
 #ifdef CONFIG_HUGETLB_PAGE
 		HTLB_BUDDY_PGALLOC, HTLB_BUDDY_PGALLOC_FAIL,
 #endif

Index: linux-2.6/mm/Kconfig
===================================================================

mm/Kconfig +6 -0

@@ -205,3 +205,9 @@ config NR_QUICK
 config VIRT_TO_BUS
 	def_bool y
 	depends on !ARCH_NO_VIRT_TO_BUS
+
+config CPU_ALLOC_SIZE
+	int "Size of cpu alloc area"
+	default "30000"
+	help
+	  Sets the maximum amount of memory that can be allocated via cpu_alloc

Index: linux-2.6/mm/Makefile
===================================================================

mm/Makefile +1 -1

@@ -11,7 +11,7 @@ obj-y			:= bootmem.o filemap.o mempool.o
 			   maccess.o page_alloc.o page-writeback.o pdflush.o \
 			   readahead.o swap.o truncate.o vmscan.o \
 			   prio_tree.o util.o mmzone.o vmstat.o backing-dev.o \
-			   page_isolation.o $(mmu-y)
+			   page_isolation.o cpu_alloc.o $(mmu-y)
 
 obj-$(CONFIG_PROC_PAGE_MONITOR) += pagewalk.o
 obj-$(CONFIG_BOUNCE)	+= bounce.o

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

mm/cpu_alloc.c new file +20 -0

@@ -0,0 +1,167 @@
+/*
+ * Cpu allocator - Manage objects allocated for each processor
+ *
+ * (C) 2008 SGI, Christoph Lameter <[email protected]>
+ * 	Basic implementation with allocation and free from a dedicated per
+ * 	cpu area.
+ *
+ * The per cpu allocator allows dynamic allocation of memory on all
+ * processor simultaneously. A bitmap is used to track used areas.
+ * The allocator implements tight packing to reduce the cache footprint
+ * and increase speed since cacheline contention is typically not a concern
+ * for memory mainly used by a single cpu. Small objects will fill up gaps
+ * left by larger allocations that required alignments.
+ */
+#include <linux/mm.h>
+#include <linux/mmzone.h>
+#include <linux/module.h>
+#include <linux/percpu.h>
+#include <linux/bitmap.h>
+#include <asm/sections.h>

mm/cpu_alloc.c new file +137 -0

+
+/*
+ * How many units are needed for an object of a given size
+ */
+static int size_to_units(unsigned long size)
+{
+	return DIV_ROUND_UP(size, UNIT_SIZE);
+}
+
+/*
+ * Lock to protect the bitmap and the meta data for the cpu allocator.
+ */
+static DEFINE_SPINLOCK(cpu_alloc_map_lock);
+static DECLARE_BITMAP(cpu_alloc_map, UNITS);
+static int first_free;		/* First known free unit */
+
+/*
+ * Mark an object as used in the cpu_alloc_map
+ *
+ * Must hold cpu_alloc_map_lock
+ */
+static void set_map(int start, int length)
+{
+	while (length-- > 0)
+		__set_bit(start++, cpu_alloc_map);
+}
+
+/*
+ * Mark an area as freed.
+ *
+ * Must hold cpu_alloc_map_lock
+ */
+static void clear_map(int start, int length)
+{
+	while (length-- > 0)
+		__clear_bit(start++, cpu_alloc_map);
+}
+
+/*
+ * Allocate an object of a certain size
+ *
+ * Returns a special pointer that can be used with CPU_PTR to find the
+ * address of the object for a certain cpu.
+ */
+void *cpu_alloc(unsigned long size, gfp_t gfpflags, unsigned long align)
+{
+	unsigned long start;
+	int units = size_to_units(size);
+	void *ptr;
+	int first;
+	unsigned long flags;
+
+	if (!size)
+		return ZERO_SIZE_PTR;
+
+	spin_lock_irqsave(&cpu_alloc_map_lock, flags);
+
+	first = 1;
+	start = first_free;
+
+	for ( ; ; ) {
+
+		start = find_next_zero_bit(cpu_alloc_map, UNITS, start);
+		if (start >= UNITS)
+			goto out_of_memory;
+
+		if (first)
+			first_free = start;
+
+		/*
+		 * Check alignment and that there is enough space after
+		 * the starting unit.
+		 */
+		if (start % (align / UNIT_SIZE) == 0 &&
+			find_next_bit(cpu_alloc_map, UNITS, start + 1)
+							>= start + units)
+				break;
+		start++;
+		first = 0;
+	}
+
+	if (first)
+		first_free = start + units;
+
+	if (start + units > UNITS)
+		goto out_of_memory;
+
+	set_map(start, units);
+	__count_vm_events(CPU_BYTES, units * UNIT_SIZE);
+
+	spin_unlock_irqrestore(&cpu_alloc_map_lock, flags);
+
+	ptr = per_cpu_var(area) + start;
+
+	if (gfpflags & __GFP_ZERO) {
+		int cpu;
+
+		for_each_possible_cpu(cpu)
+			memset(CPU_PTR(ptr, cpu), 0, size);
+	}
+
+	return ptr;
+
+out_of_memory:
+	spin_unlock_irqrestore(&cpu_alloc_map_lock, flags);
+	return NULL;
+}
+EXPORT_SYMBOL(cpu_alloc);
+
+/*
+ * Free an object. The pointer must be a cpu pointer allocated
+ * via cpu_alloc.
+ */
+void cpu_free(void *start, unsigned long size)
+{
+	unsigned long units = size_to_units(size);
+	unsigned long index = (int *)start - per_cpu_var(area);
+	unsigned long flags;
+
+	if (!start || start == ZERO_SIZE_PTR)
+		return;
+
+	BUG_ON(index >= UNITS ||
+		!test_bit(index, cpu_alloc_map) ||
+		!test_bit(index + units - 1, cpu_alloc_map));
+
+	spin_lock_irqsave(&cpu_alloc_map_lock, flags);
+
+	clear_map(index, units);
+	__count_vm_events(CPU_BYTES, -units * UNIT_SIZE);
+
+	if (index < first_free)
+		first_free = index;
+
+	spin_unlock_irqrestore(&cpu_alloc_map_lock, flags);
+}
+EXPORT_SYMBOL(cpu_free);

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

mm/vmstat.c +1 -0

@@ -653,6 +653,7 @@ static const char * const vmstat_text[] 
 	"allocstall",
 
 	"pgrotated",
+	"cpu_bytes",
 #ifdef CONFIG_HUGETLB_PAGE
 	"htlb_buddy_alloc_success",
 	"htlb_buddy_alloc_fail",

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

include/linux/percpu.h +46 -0

@@ -135,4 +135,50 @@ static inline void percpu_free(void *__p
 #define free_percpu(ptr)	percpu_free((ptr))
 #define per_cpu_ptr(ptr, cpu)	percpu_ptr((ptr), (cpu))
 
+
+/*
+ * cpu allocator definitions
+ *
+ * The cpu allocator allows allocating an instance of an object for each
+ * processor and the use of a single pointer to access all instances
+ * of the object. cpu_alloc provides optimized means for accessing the
+ * instance of the object belonging to the currently executing processor
+ * as well as special atomic operations on fields of objects of the
+ * currently executing processor.
+ *
+ * Cpu objects are typically small. The allocator packs them tightly
+ * to increase the chance on each access that a per cpu object is already
+ * cached. Alignments may be specified but the intent is to align the data
+ * properly due to cpu alignment constraints and not to avoid cacheline
+ * contention. Any holes left by aligning objects are filled up with smaller
+ * objects that are allocated later.
+ *
+ * Cpu data can be allocated using CPU_ALLOC. The resulting pointer is
+ * pointing to the instance of the variable in the per cpu area provided
+ * by the loader. It is generally an error to use the pointer directly
+ * unless we are booting the system.
+ *
+ * __GFP_ZERO may be passed as a flag to zero the allocated memory.
+ */
+
+/* Return a pointer to the instance of a object for a particular processor */
+#define CPU_PTR(__p, __cpu)	SHIFT_PERCPU_PTR((__p), per_cpu_offset(__cpu))
+
+/*
+ * Return a pointer to the instance of the object belonging to the processor
+ * running the current code.
+ */
+#define THIS_CPU(__p)	SHIFT_PERCPU_PTR((__p), my_cpu_offset)
+#define __THIS_CPU(__p)	SHIFT_PERCPU_PTR((__p), __my_cpu_offset)
+
+#define CPU_ALLOC(type, flags)	((typeof(type) *)cpu_alloc(sizeof(type), \
+					(flags), __alignof__(type)))
+#define CPU_FREE(pointer)	cpu_free((pointer), sizeof(*(pointer)))
+
+/*
+ * Raw calls
+ */
+void *cpu_alloc(unsigned long size, gfp_t flags, unsigned long align);
+void cpu_free(void *cpu_pointer, unsigned long size);
+
 #endif /* __LINUX_PERCPU_H */

Quoted 2 lines

+static DEFINE_PER_CPU(UNIT_TYPE, area[UNITS]);

area[] is not guaranteed to be aligned on anything but 4 bytes.

If someone then needs to call cpu_alloc(8, GFP_KERNEL, 8), it might get an non aligned result.

Either you should add an __attribute__((__aligned__(PAGE_SIZE))), or take into account the real address of area[] in cpu_alloc() to avoid waste of up to PAGE_SIZE bytes
per cpu.

Quoted 7 lines · Eric Dumazet

Quoted 1 line

+static DEFINE_PER_CPU(UNIT_TYPE, area[UNITS]);

area[] is not guaranteed to be aligned on anything but 4 bytes.

If someone then needs to call cpu_alloc(8, GFP_KERNEL, 8), it might get an non aligned result.

Quoted 4 lines

Either you should add an __attribute__((__aligned__(PAGE_SIZE))), or take into account the real address of area[] in cpu_alloc() to avoid waste of up to PAGE_SIZE bytes
per cpu.

I think cacheline aligning should be sufficient. People should not allocate large page aligned objects here.

Quoted 7 lines · On Friday 30 May 2008 15:20:45 Christoph Lameter wrote:

Quoted 6 lines · Eric Dumazet

Quoted 1 line

+static DEFINE_PER_CPU(UNIT_TYPE, area[UNITS]);

area[] is not guaranteed to be aligned on anything but 4 bytes.

If someone then needs to call cpu_alloc(8, GFP_KERNEL, 8), it might get an non aligned result.

Quoted 6 lines

Quoted 4 lines

Either you should add an __attribute__((__aligned__(PAGE_SIZE))), or take into account the real address of area[] in cpu_alloc() to avoid waste of up to PAGE_SIZE bytes
per cpu.

I think cacheline aligning should be sufficient. People should not allocate large page aligned objects here.

I vaguely recall there were issues with this in the module code. They might be gone now, but failing to meet alignment contraints without a big warning would suck.

But modifying your code to consider the actual alignment is actually pretty trivial, AFAICT.

Cheers,
Rusty.

Quoted 21 lines · Rusty Russell

Quoted 8 lines · On Friday 30 May 2008 15:20:45 Christoph Lameter wrote:

Quoted 7 lines · Eric Dumazet

Quoted 1 line

+static DEFINE_PER_CPU(UNIT_TYPE, area[UNITS]);

area[] is not guaranteed to be aligned on anything but 4 bytes.

If someone then needs to call cpu_alloc(8, GFP_KERNEL, 8), it might get an non aligned result.

Quoted 7 lines

Quoted 4 lines

Either you should add an __attribute__((__aligned__(PAGE_SIZE))), or take into account the real address of area[] in cpu_alloc() to avoid waste of up to PAGE_SIZE bytes
per cpu.

I think cacheline aligning should be sufficient. People should not allocate large page aligned objects here.

I vaguely recall there were issues with this in the module code. They might be gone now, but failing to meet alignment contraints without a big warning would suck.

Quoted 5 lines

But modifying your code to consider the actual alignment is actually pretty trivial, AFAICT.

Cheers,
Rusty.

So paraphrasing my earlier email, we should add:

bitmap_find_free_area(bitmap, nbits, size, align, alignbase)

so that > cacheline alignment is possible?

My thinking is that if we do go to true dynamically sized cpu_alloc area then allocating PAGE_SIZE units may be both practical and worthwhile...?

Thanks,
Mike

Quoted 8 lines · Eric Dumazet

Quoted 3 lines

+static DEFINE_PER_CPU(UNIT_TYPE, area[UNITS]);

area[] is not guaranteed to be aligned on anything but 4 bytes.

If someone then needs to call cpu_alloc(8, GFP_KERNEL, 8), it might get an non aligned result.

Quoted 5 lines

Either you should add an __attribute__((__aligned__(PAGE_SIZE))), or take into account the real address of area[] in cpu_alloc() to avoid waste of up to PAGE_SIZE bytes
per cpu.

I think cacheline aligning should be sufficient. People should not allocate large page aligned objects here.

Quoted 7 lines · Eric Dumazet

Quoted 2 lines

+static DEFINE_PER_CPU(UNIT_TYPE, area[UNITS]);

area[] is not guaranteed to be aligned on anything but 4 bytes.

If someone then needs to call cpu_alloc(8, GFP_KERNEL, 8), it might get an non aligned result.

Quoted 4 lines

Either you should add an __attribute__((__aligned__(PAGE_SIZE))), or take into account the real address of area[] in cpu_alloc() to avoid waste of up to PAGE_SIZE bytes
per cpu.

I think cacheline aligning should be sufficient. People should not allocate large page aligned objects here.

Quoted 9 lines · Christoph Lameter

Quoted 7 lines · Eric Dumazet

Quoted 2 lines

+static DEFINE_PER_CPU(UNIT_TYPE, area[UNITS]);

area[] is not guaranteed to be aligned on anything but 4 bytes.

If someone then needs to call cpu_alloc(8, GFP_KERNEL, 8), it might get an non aligned result.

Quoted 6 lines

Quoted 4 lines

Either you should add an __attribute__((__aligned__(PAGE_SIZE))), or take into account the real address of area[] in cpu_alloc() to avoid waste of up to PAGE_SIZE bytes
per cpu.

I think cacheline aligning should be sufficient. People should not allocate large page aligned objects here.

I'm a bit confused. Why is DEFINE_PER_CPU_SHARED_ALIGNED() conditioned on ifdef MODULE?

        #ifdef MODULE
        #define SHARED_ALIGNED_SECTION ".data.percpu"
        #else
        #define SHARED_ALIGNED_SECTION ".data.percpu.shared_aligned"
        #endif

        #define DEFINE_PER_CPU_SHARED_ALIGNED(type, name)                       \
                __attribute__((__section__(SHARED_ALIGNED_SECTION)))            \
                PER_CPU_ATTRIBUTES __typeof__(type) per_cpu__##name             \
                ____cacheline_aligned_in_smp

Thanks,
Mike

Quoted 2 lines · Mike Travis

I'm a bit confused. Why is DEFINE_PER_CPU_SHARED_ALIGNED() conditioned on ifdef MODULE?

Because we had crashes when loading oprofile module, when a previous version of oprofile used to use DEFINE_PER_CPU_SHARED_ALIGNED variable

module loader only takes into account the special section ".data.percpu" and ignores ".data.percpu.shared_aligned"

I therefore submitted two patches :

I'm a bit confused. Why is DEFINE_PER_CPU_SHARED_ALIGNED() conditioned on ifdef MODULE?

        #ifdef MODULE
        #define SHARED_ALIGNED_SECTION ".data.percpu"
        #else
        #define SHARED_ALIGNED_SECTION ".data.percpu.shared_aligned"
        #endif

        #define DEFINE_PER_CPU_SHARED_ALIGNED(type, name)                       \
                __attribute__((__section__(SHARED_ALIGNED_SECTION)))            \
                PER_CPU_ATTRIBUTES __typeof__(type) per_cpu__##name             \
                ____cacheline_aligned_in_smp

Quoted 9 lines · Mike Travis

I'm a bit confused. Why is DEFINE_PER_CPU_SHARED_ALIGNED() conditioned on ifdef MODULE?

        #ifdef MODULE
        #define SHARED_ALIGNED_SECTION ".data.percpu"
        #else
        #define SHARED_ALIGNED_SECTION ".data.percpu.shared_aligned"
        #endif

Quoted 4 lines

        #define DEFINE_PER_CPU_SHARED_ALIGNED(type, name)                       \
                __attribute__((__section__(SHARED_ALIGNED_SECTION)))            \
                PER_CPU_ATTRIBUTES __typeof__(type) per_cpu__##name             \

____cacheline_aligned_in_smp

Looks wrong to me. There can be shared objects even without modules.

Well, MODULE is not CONFIG_MODULES :)

If compiling an object that is going to be statically linked to kernel, MODULE is not defined, so we have shared objects.

When compiling a module, we cannot *yet* use .data.percpu.shared_aligned section, since module loader wont handle this section.

Alternative is to change modules linking for all arches to merge .data.percpu{*} subsections correctly, or tell module loader to take into account all .data.percpu sections.

AFAIK no module uses DEFINE_PER_CPU_SHARED_ALIGNED() yet...

The per cpu allocator allows dynamic allocation of memory on all processors simultaneously. A bitmap is used to track used areas. The allocator implements tight packing to reduce the cache footprint and increase speed since cacheline contention is typically not a concern for memory mainly used by a single cpu. Small objects will fill up gaps left by larger allocations that required alignments.

The size of the cpu_alloc area can be changed via make menuconfig.

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

---
 include/linux/percpu.h |   46 +++++++++++++
 include/linux/vmstat.h |    2
 mm/Kconfig             |    6 +
 mm/Makefile            |    2
 mm/cpu_alloc.c         |  167
+++++++++++++++++++++++++++++++++++++++++++++++++ mm/vmstat.c            | 
  1
 6 files changed, 222 insertions(+), 2 deletions(-)

Index: linux-2.6/include/linux/vmstat.h
===================================================================
--- linux-2.6.orig/include/linux/vmstat.h 2008-05-29 19:41:21.000000000
-0700 +++ linux-2.6/include/linux/vmstat.h 2008-05-29 20:15:37.000000000
-0700 @@ -37,7 +37,7 @@ enum vm_event_item { PGPGIN, PGPGOUT, PS

include/linux/vmstat.h +1 -1

 		FOR_ALL_ZONES(PGSCAN_KSWAPD),
 		FOR_ALL_ZONES(PGSCAN_DIRECT),
 		PGINODESTEAL, SLABS_SCANNED, KSWAPD_STEAL, KSWAPD_INODESTEAL,
-		PAGEOUTRUN, ALLOCSTALL, PGROTATED,
+		PAGEOUTRUN, ALLOCSTALL, PGROTATED, CPU_BYTES,
 #ifdef CONFIG_HUGETLB_PAGE
 		HTLB_BUDDY_PGALLOC, HTLB_BUDDY_PGALLOC_FAIL,
 #endif

Index: linux-2.6/mm/Kconfig
===================================================================

mm/Kconfig +6 -0

@@ -205,3 +205,9 @@ config NR_QUICK
 config VIRT_TO_BUS
 	def_bool y
 	depends on !ARCH_NO_VIRT_TO_BUS
+
+config CPU_ALLOC_SIZE
+	int "Size of cpu alloc area"
+	default "30000"
+	help
+	  Sets the maximum amount of memory that can be allocated via cpu_alloc

Index: linux-2.6/mm/Makefile
===================================================================

mm/Makefile +1 -1

@@ -11,7 +11,7 @@ obj-y			:= bootmem.o filemap.o mempool.o
 			   maccess.o page_alloc.o page-writeback.o pdflush.o \
 			   readahead.o swap.o truncate.o vmscan.o \
 			   prio_tree.o util.o mmzone.o vmstat.o backing-dev.o \
-			   page_isolation.o $(mmu-y)
+			   page_isolation.o cpu_alloc.o $(mmu-y)
 
 obj-$(CONFIG_PROC_PAGE_MONITOR) += pagewalk.o
 obj-$(CONFIG_BOUNCE)	+= bounce.o

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

mm/cpu_alloc.c new file +11 -0

@@ -0,0 +1,167 @@
+/*
+ * Cpu allocator - Manage objects allocated for each processor
+ *
+ * (C) 2008 SGI, Christoph Lameter <[email protected]>
+ * 	Basic implementation with allocation and free from a dedicated per
+ * 	cpu area.
+ *
+ * The per cpu allocator allows dynamic allocation of memory on all
+ * processor simultaneously. A bitmap is used to track used areas.
+ * The allocator implements tight packing to reduce the cache footprint
+ * and increase speed since cacheline contention is typically not a

concern + * for memory mainly used by a single cpu. Small objects will fill up gaps + * left by larger allocations that required alignments.
+ */

mm/cpu_alloc.c new file +153 -0

+#include <linux/mm.h>
+#include <linux/mmzone.h>
+#include <linux/module.h>
+#include <linux/percpu.h>
+#include <linux/bitmap.h>
+#include <asm/sections.h>
+
+/*
+ * Basic allocation unit. A bit map is created to track the use of each
+ * UNIT_SIZE element in the cpu area.
+ */
+#define UNIT_TYPE int
+#define UNIT_SIZE sizeof(UNIT_TYPE)
+#define UNITS (CONFIG_CPU_ALLOC_SIZE / UNIT_SIZE)
+
+static DEFINE_PER_CPU(UNIT_TYPE, area[UNITS]);
+
+/*
+ * How many units are needed for an object of a given size
+ */
+static int size_to_units(unsigned long size)
+{
+	return DIV_ROUND_UP(size, UNIT_SIZE);
+}
+
+/*
+ * Lock to protect the bitmap and the meta data for the cpu allocator.
+ */
+static DEFINE_SPINLOCK(cpu_alloc_map_lock);
+static DECLARE_BITMAP(cpu_alloc_map, UNITS);
+static int first_free;		/* First known free unit */
+
+/*
+ * Mark an object as used in the cpu_alloc_map
+ *
+ * Must hold cpu_alloc_map_lock
+ */
+static void set_map(int start, int length)
+{
+	while (length-- > 0)
+		__set_bit(start++, cpu_alloc_map);
+}
+
+/*
+ * Mark an area as freed.
+ *
+ * Must hold cpu_alloc_map_lock
+ */
+static void clear_map(int start, int length)
+{
+	while (length-- > 0)
+		__clear_bit(start++, cpu_alloc_map);
+}
+
+/*
+ * Allocate an object of a certain size
+ *
+ * Returns a special pointer that can be used with CPU_PTR to find the
+ * address of the object for a certain cpu.
+ */
+void *cpu_alloc(unsigned long size, gfp_t gfpflags, unsigned long align)
+{
+	unsigned long start;
+	int units = size_to_units(size);
+	void *ptr;
+	int first;
+	unsigned long flags;
+
+	if (!size)
+		return ZERO_SIZE_PTR;
+
+	spin_lock_irqsave(&cpu_alloc_map_lock, flags);
+
+	first = 1;
+	start = first_free;
+
+	for ( ; ; ) {
+
+		start = find_next_zero_bit(cpu_alloc_map, UNITS, start);
+		if (start >= UNITS)
+			goto out_of_memory;
+
+		if (first)
+			first_free = start;
+
+		/*
+		 * Check alignment and that there is enough space after
+		 * the starting unit.
+		 */
+		if (start % (align / UNIT_SIZE) == 0 &&
+			find_next_bit(cpu_alloc_map, UNITS, start + 1)
+							>= start + units)
+				break;
+		start++;
+		first = 0;
+	}
+
+	if (first)
+		first_free = start + units;
+
+	if (start + units > UNITS)
+		goto out_of_memory;
+
+	set_map(start, units);
+	__count_vm_events(CPU_BYTES, units * UNIT_SIZE);
+
+	spin_unlock_irqrestore(&cpu_alloc_map_lock, flags);
+
+	ptr = per_cpu_var(area) + start;
+
+	if (gfpflags & __GFP_ZERO) {
+		int cpu;
+
+		for_each_possible_cpu(cpu)
+			memset(CPU_PTR(ptr, cpu), 0, size);
+	}
+
+	return ptr;
+
+out_of_memory:
+	spin_unlock_irqrestore(&cpu_alloc_map_lock, flags);
+	return NULL;
+}
+EXPORT_SYMBOL(cpu_alloc);
+
+/*
+ * Free an object. The pointer must be a cpu pointer allocated
+ * via cpu_alloc.
+ */
+void cpu_free(void *start, unsigned long size)
+{
+	unsigned long units = size_to_units(size);
+	unsigned long index = (int *)start - per_cpu_var(area);
+	unsigned long flags;
+
+	if (!start || start == ZERO_SIZE_PTR)
+		return;
+
+	BUG_ON(index >= UNITS ||
+		!test_bit(index, cpu_alloc_map) ||
+		!test_bit(index + units - 1, cpu_alloc_map));
+
+	spin_lock_irqsave(&cpu_alloc_map_lock, flags);
+
+	clear_map(index, units);
+	__count_vm_events(CPU_BYTES, -units * UNIT_SIZE);
+
+	if (index < first_free)
+		first_free = index;
+
+	spin_unlock_irqrestore(&cpu_alloc_map_lock, flags);
+}
+EXPORT_SYMBOL(cpu_free);

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

mm/vmstat.c +1 -0

@@ -653,6 +653,7 @@ static const char * const vmstat_text[]
 	"allocstall",
 
 	"pgrotated",
+	"cpu_bytes",
 #ifdef CONFIG_HUGETLB_PAGE
 	"htlb_buddy_alloc_success",
 	"htlb_buddy_alloc_fail",

Index: linux-2.6/include/linux/percpu.h
===================================================================
--- linux-2.6.orig/include/linux/percpu.h 2008-05-29 19:41:21.000000000
-0700 +++ linux-2.6/include/linux/percpu.h 2008-05-29 20:29:12.000000000
-0700 @@ -135,4 +135,50 @@ static inline void percpu_free(void *__p
#define free_percpu(ptr) percpu_free((ptr))
#define per_cpu_ptr(ptr, cpu) percpu_ptr((ptr), (cpu))

mm/vmstat.c +16 -0

+
+/*
+ * cpu allocator definitions
+ *
+ * The cpu allocator allows allocating an instance of an object for each
+ * processor and the use of a single pointer to access all instances
+ * of the object. cpu_alloc provides optimized means for accessing the
+ * instance of the object belonging to the currently executing processor
+ * as well as special atomic operations on fields of objects of the
+ * currently executing processor.
+ *
+ * Cpu objects are typically small. The allocator packs them tightly
+ * to increase the chance on each access that a per cpu object is already
+ * cached. Alignments may be specified but the intent is to align the data
+ * properly due to cpu alignment constraints and not to avoid cacheline
+ * contention. Any holes left by aligning objects are filled up with

smaller + * objects that are allocated later.
+ *
+ * Cpu data can be allocated using CPU_ALLOC. The resulting pointer is
+ * pointing to the instance of the variable in the per cpu area provided
+ * by the loader. It is generally an error to use the pointer directly
+ * unless we are booting the system.
+ *
+ * __GFP_ZERO may be passed as a flag to zero the allocated memory.
+ */

mm/vmstat.c +2 -0

+
+/* Return a pointer to the instance of a object for a particular processor

*/ +#define CPU_PTR(__p, __cpu) SHIFT_PERCPU_PTR((__p),
per_cpu_offset(__cpu)) +
+/*
+ * Return a pointer to the instance of the object belonging to the processor + * running the current code.
+ */

mm/vmstat.c +13 -0

+#define THIS_CPU(__p)	SHIFT_PERCPU_PTR((__p), my_cpu_offset)
+#define __THIS_CPU(__p)	SHIFT_PERCPU_PTR((__p), __my_cpu_offset)
+
+#define CPU_ALLOC(type, flags)	((typeof(type) *)cpu_alloc(sizeof(type), \
+					(flags), __alignof__(type)))
+#define CPU_FREE(pointer)	cpu_free((pointer), sizeof(*(pointer)))
+
+/*
+ * Raw calls
+ */
+void *cpu_alloc(unsigned long size, gfp_t flags, unsigned long align);
+void cpu_free(void *cpu_pointer, unsigned long size);
+
 #endif /* __LINUX_PERCPU_H */

Quoted 6 lines · On Friday 30 May 2008 13:56:22 Christoph Lameter wrote:

The per cpu allocator allows dynamic allocation of memory on all processors simultaneously. A bitmap is used to track used areas. The allocator implements tight packing to reduce the cache footprint and increase speed since cacheline contention is typically not a concern for memory mainly used by a single cpu. Small objects will fill up gaps left by larger allocations that required alignments.

Allocator seems nice and simple, similar to existing one in module.c (which predates cool bitmap operators).

Being able to do per-cpu allocations in an interrupt handler seems like encouraging a Bad Idea though: I'd be tempted to avoid the flags word, always zero, and use a mutex instead of a spinlock.

Cheers,
Rusty.

I haven't seen any further discussion on these aspects... is there a consensus to remove the flags from CPU_ALLOC() and use a mutex?

Index: linux-2.6/mm/Kconfig
===================================================================

Remove the builtin per cpu allocator from modules.c and use cpu_alloc instead.

The patch also removes PERCPU_ENOUGH_ROOM. The size of the cpu_alloc area is determined by CONFIG_CPU_AREA_SIZE. PERCPU_ENOUGH_ROOMs default was 8k. CONFIG_CPU_AREA_SIZE defaults to 30k. Thus we have more space to load modules.

...

kernel/module.c static struct module *load_module(void _ · line 1770

Quoted 2 lines

kernel/module.c +2 -0

+			printk(KERN_WARNING "%s: per-cpu alignment %li > %li\n",
+			mod->name, align, PAGE_SIZE);

Indenting broke.

Alas, PAGE_SIZE has, iirc, unsigned type on some architectures and unsigned long on others. I suspect you'll need to cast it to be able to print it.

kernel/module.c static struct module *load_module(void _ · line 1775

Quoted 3 lines

kernel/module.c +3 -0

+		percpu = cpu_alloc(size, GFP_KERNEL|__GFP_ZERO, align);
+		if (!percpu)
+			printk(KERN_WARNING "Could not allocate %lu bytes percpu data\n",

80-col bustage,.

A printk like this should, I think, identify what part of the kernel it came from.

But really, I don't think any printk should be present here. cpu_alloc() itself should dump the warning and the backtrace when it runs out. Because a cpu_alloc() failure is a major catastrophe. It probably means a reconfigure-and-reboot cycle.

Quoted 4 lines · Andrew Morton

kernel/module.c static struct module *load_module(void _ · line 1770

Quoted 2 lines

kernel/module.c +2 -0

+			printk(KERN_WARNING "%s: per-cpu alignment %li > %li\n",
+			mod->name, align, PAGE_SIZE);

Indenting broke.

Hmmm. Okay.

Quoted 3 lines

Alas, PAGE_SIZE has, iirc, unsigned type on some architectures and unsigned long on others. I suspect you'll need to cast it to be able to print it.

This is code that was moved.

Quoted 6 lines

kernel/module.c static struct module *load_module(void _ · line 1775

Quoted 3 lines

kernel/module.c +3 -0

+		percpu = cpu_alloc(size, GFP_KERNEL|__GFP_ZERO, align);
+		if (!percpu)
+			printk(KERN_WARNING "Could not allocate %lu bytes percpu data\n",

80-col bustage,.

Quoted 2 lines

A printk like this should, I think, identify what part of the kernel it came from.

Again moved code. Should I really do string separations for code that is moved?

Quoted 4 lines

But really, I don't think any printk should be present here. cpu_alloc() itself should dump the warning and the backtrace when it runs out. Because a cpu_alloc() failure is a major catastrophe. It probably means a reconfigure-and-reboot cycle.

The code has been able to deal with an allocpercpu failure in the past. Why would it have trouble with a cpu_alloc failure here?

Hmm, does cpu_free(NULL, 0) do something? Seems like it shouldn't, for symmetry with free().

kernel/module.c static struct module *load_module(void _ · line 1779

Quoted 7 lines

kernel/module.c +4 -0

+		percpu = cpu_alloc(size, GFP_KERNEL|__GFP_ZERO, align);
+		if (!percpu)
+			printk(KERN_WARNING "Could not allocate %lu bytes percpu data\n",
+										size);
 		if (!percpu) {
 			err = -ENOMEM;
 			goto free_mod;

OK, we've *never* had a report of the per-cpu alignment message, so I'd be happy to pass that through to cpu_alloc() and have it fail. Also, the if (!percpu) cases should be combined.

kernel/module.c static struct module *load_module(void _ · line 2032

Quoted 4 lines

free_percpu:

kernel/module.c +1 -1

 	if (percpu)
-		percpu_modfree(percpu);
+		cpu_free(percpu, percpu_size);

As above.

arch/sparc64/kernel/smp.c void __init real_setup_per_cpu_areas(voi · line 1457

Quoted 2 lines

arch/sparc64/kernel/smp.c +1 -0

+	goal = __per_cpu_size;

Where did __per_cpu_size come from? I missed it in the earlier patches...

Currently the per cpu subsystem is not able to use the atomic capabilities that are provided by many of the available processors.

This patch adds new functionality that allows the optimizing of per cpu variable handling. In particular it provides a simple way to exploit atomic operations in order to avoid having to disable interrupts or performing address calculation to access per cpu data.

F.e. Using our current methods we 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);

The segment can be replaced by a single atomic CPU operation:

	CPU_INC(stat->counter);

Most processors have instructions to perform the increment using a a single atomic instruction. Processors may have segment registers, global registers or per cpu mappings of per cpu areas that can be used to generate atomic instructions that combine the following in a single operation:

1. Adding of an offset / register to a base address
2. Read modify write operation on the address calculated by the instruction.

If 1+2 are combined in an instruction then the instruction is atomic vs interrupts. This means that percpu atomic operations do not need to disable interrupts to increments counters etc.

The existing methods in use in the kernel cannot utilize the power of these atomic instructions. local_t is not really addressing the issue since the offset calculation performed before the atomic operation. The operation is therefor not atomic. Disabling interrupt or preemption is required in order to use local_t.

local_t is also very specific to the x86 processor.

The solution here can
utilize other methods than just those provided by 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. Both adding of the offset and the read modify write are combined in one instruction.

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 variable resides.

Note that the counter offset into the struct was added *before* the segment selector was added. This is necessary to avoid calculations. 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. The adding of the per cpu offset (here through the gs register) must be done by the instruction used for atomic per cpu access.

If "stat" was declared via DECLARE_PER_CPU then this patchset is capable of convincing the linker to provide the proper base address. In that case no calculations are necessary.

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

On IA64 we can get the same combination of operations in a single instruction by using the virtual address that always maps to the local per cpu area:

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

The access is forced into the per cpu address reachable via the virtualized address. IA64 allows the embedding of an offset into the instruction. So the fetchadd can perform both the relocation of the pointer into the per cpu area as well as the atomic read modify write cycle.

In order to be able to exploit the atomicity of these instructions we introduce a series of new functions that take either:

1. A per cpu pointer as returned by cpu_alloc() or CPU_ALLOC().

2. A per cpu variable address as returned by per_cpu_var(<percpuvarname>).

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

include/linux/percpu.h +2 -0

@@ -179,4 +179,139 @@
 void *cpu_alloc(unsigned long size, gfp_t flags, unsigned long align);
 void cpu_free(void *cpu_pointer, unsigned long size);
 
+/*
+ * Fast atomic per cpu operations.

include/linux/percpu.h +122 -0

+/*
+ * Fallback in case the arch does not provide for atomic per cpu operations.
+ *
+ * The first group of macros is used when it is safe to update the per
+ * cpu variable because preemption is off (per cpu variables that are not
+ * updated from interrupt context) or because interrupts are already off.
+ */
+#define __CPU_READ(var)				\
+({						\
+	(*THIS_CPU(&(var)));			\
+})
+
+#define __CPU_WRITE(var, value)			\
+({						\
+	*THIS_CPU(&(var)) = (value);		\
+})
+
+#define __CPU_ADD(var, value)			\
+({						\
+	*THIS_CPU(&(var)) += (value);		\
+})
+
+#define __CPU_INC(var) __CPU_ADD((var), 1)
+#define __CPU_DEC(var) __CPU_ADD((var), -1)
+#define __CPU_SUB(var, value) __CPU_ADD((var), -(value))
+
+#define __CPU_CMPXCHG(var, old, new)		\
+({						\
+	typeof(obj) x;				\
+	typeof(obj) *p = THIS_CPU(&(obj));	\
+	x = *p;					\
+	if (x == (old))				\
+		*p = (new);			\
+	(x);					\
+})
+
+#define __CPU_XCHG(obj, new)			\
+({						\
+	typeof(obj) x;				\
+	typeof(obj) *p = THIS_CPU(&(obj));	\
+	x = *p;					\
+	*p = (new);				\
+	(x);					\
+})
+
+/*
+ * Second group used for per cpu variables that are not updated from an
+ * interrupt context. In that case we can simply disable preemption which
+ * may be free if the kernel is compiled without support for preemption.
+ */
+#define _CPU_READ __CPU_READ
+#define _CPU_WRITE __CPU_WRITE
+
+#define _CPU_ADD(var, value)			\
+({						\
+	preempt_disable();			\
+	__CPU_ADD((var), (value));		\
+	preempt_enable();			\
+})
+
+#define _CPU_INC(var) _CPU_ADD((var), 1)
+#define _CPU_DEC(var) _CPU_ADD((var), -1)
+#define _CPU_SUB(var, value) _CPU_ADD((var), -(value))
+
+#define _CPU_CMPXCHG(var, old, new)		\
+({						\
+	typeof(addr) x;				\
+	preempt_disable();			\
+	x = __CPU_CMPXCHG((var), (old), (new));	\
+	preempt_enable();			\
+	(x);					\
+})
+
+#define _CPU_XCHG(var, new)			\
+({						\
+	typeof(var) x;				\
+	preempt_disable();			\
+	x = __CPU_XCHG((var), (new));		\
+	preempt_enable();			\
+	(x);					\
+})
+
+/*
+ * Third group: Interrupt safe CPU functions
+ */
+#define CPU_READ __CPU_READ
+#define CPU_WRITE __CPU_WRITE
+
+#define CPU_ADD(var, value)			\
+({						\
+	unsigned long flags;			\
+	local_irq_save(flags);			\
+	__CPU_ADD((var), (value));		\
+	local_irq_restore(flags);		\
+})
+
+#define CPU_INC(var) CPU_ADD((var), 1)
+#define CPU_DEC(var) CPU_ADD((var), -1)
+#define CPU_SUB(var, value) CPU_ADD((var), -(value))
+
+#define CPU_CMPXCHG(var, old, new)		\
+({						\
+	unsigned long flags;			\
+	typeof(var) x;				\
+	local_irq_save(flags);			\
+	x = __CPU_CMPXCHG((var), (old), (new));	\
+	local_irq_restore(flags);		\
+	(x);					\
+})
+
+#define CPU_XCHG(var, new)			\
+({						\
+	unsigned long flags;			\
+	typeof(var) x;				\
+	local_irq_save(flags);			\
+	x = __CPU_XCHG((var), (new));		\
+	local_irq_restore(flags);		\
+	(x);					\
+})
+
+#endif /* CONFIG_HAVE_CPU_OPS */
+
 #endif /* __LINUX_PERCPU_H */

Quoted 5 lines

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

eh? That's what local_t is for?

Quoted 3 lines

The segment can be replaced by a single atomic CPU operation:

	CPU_INC(stat->counter);

hm, I guess this _has_ to be implemented as a macro. ho hum. But
please: "cpu_inc"?

Quoted 5 lines

The existing methods in use in the kernel cannot utilize the power of these atomic instructions. local_t is not really addressing the issue since the offset calculation performed before the atomic operation. The operation is therefor not atomic. Disabling interrupt or preemption is required in order to use local_t.

Your terminology is totally confusing here.

To me, an "atomic operation" is one which is atomic wrt other CPUs:
atomic_t, for example.

Here we're talking about atomic-wrt-this-cpu-only, yes?

Quoted 1 line

local_t is also very specific to the x86 processor.

And alpha, m32r, mips and powerpc, methinks. Probably others, but people just haven't got around to it.

I think I'll need to come back another time to understand all that ;)

Thanks for writing it up carefully.

I wonder if all this stuff should be in a new header file.

We could get lazy and include that header from percpu.h if needed.

Quoted 4 lines

Quoted 1 line

local_t is also very specific to the x86 processor.

And alpha, m32r, mips and powerpc, methinks. Probably others, but people just haven't got around to it.

No local_t does not do the relocation of the address to the correct percpu area. It requies disabling of interrupts etc.

Its not atomic (wrt
interrupts) because of that.

Quoted 3 lines

I think I'll need to come back another time to understand all that ;)

Thanks for writing it up carefully.

Well this stuff is so large in scope that I have difficulties keeping everything straight.

Quoted 3 lines

I wonder if all this stuff should be in a new header file.

We could get lazy and include that header from percpu.h if needed.

But then its related to percpu operations and relies extensively on the various percpu.h files in asm-generic and asm-arch and include/linux

Quoted 1 line

area. It requies disabling of interrupts etc.

No it doesn't. Look:

static inline void local_inc(local_t *l)

{
	asm volatile(_ASM_INC "%0"
		     : "+m" (l->a.counter));
}

Quoted 3 lines

Its not atomic (wrt
interrupts) because of that.

Yes it is.

Quoted 2 lines

But then its related to percpu operations and relies extensively on the various percpu.h files in asm-generic and asm-arch and include/linux

Well that should be fixed. We should never have mixed the alloc_percpu() and DEFINE_PER_CPU things inthe same header. They're different.

Quoted 12 lines · Andrew Morton

Quoted 1 line

area. It requies disabling of interrupts etc.

No it doesn't. Look:

static inline void local_inc(local_t *l)

{
	asm volatile(_ASM_INC "%0"

		     : "+m" (l->a.counter));

}

Quoted 4 lines

Quoted 2 lines

Its not atomic (wrt
interrupts) because of that.

Yes it is.

No its not! In order to increment a per cpu value you need to calculate the per cpu pointer address in the current per cpu segment.

Quoted 2 lines

No its not! In order to increment a per cpu value you need to calculate the per cpu pointer address in the current per cpu segment.

Christoph, you just missed it, that's all. Look at cpu_local_read et al in include/asm-i386/local.h (ie. before the x86 mergers chose the lowest common denominator one).

Quoted 6 lines · Rusty Russell

Quoted 2 lines

No its not! In order to increment a per cpu value you need to calculate the per cpu pointer address in the current per cpu segment.

Christoph, you just missed it, that's all. Look at cpu_local_read et al in include/asm-i386/local.h (ie. before the x86 mergers chose the lowest common denominator one).

There is no doubt that local_t does perform an atomic vs. interrupt inc for example. But its not usable. Because you need to determine the address of the local_t belonging to the current processor first.

As soon as you
have loaded a processor specific address you can no longer be preempted because that may change the processor and then the wrong address may be increment (and then we have a race again since now we are incrementing counters belonging to other processors). So local_t at mininum requires disabling preempt.

Believe me I have tried to use local_t repeatedly for vm statistics etc. It always fails on that issue.

cpu_local_inc() does all this: it takes the name of a local_t var, and is expected to increment this cpu's version of that. You ripped this out and called it CPU_INC().

Quoted 3 lines

cpu_local_inc() does all this: it takes the name of a local_t var, and is expected to increment this cpu's version of that. You ripped this out and called it CPU_INC().

Hi,

I'm attempting to test both approaches to compare the object generated in order to understand the issues involved here. Here's my code:

        void test_cpu_inc(int *s)
        {
                __CPU_INC(s);
        }

        void test_local_inc(local_t *t)
        {
                __local_inc(THIS_CPU(t));
        }

        void test_cpu_local_inc(local_t *t)
        {
                __cpu_local_inc(t);
        }

But I don't know how I can use cpu_local_inc because the pointer to the object is not &__get_cpu_var(l):

At the minimum, we would need a new local_t op to get the correct CPU_ALLOC'd pointer value for the increment. These new local_t ops for CPU_ALLOC'd variables could use CPU_XXX primitives to implement them, or just a base val_to_ptr primitive to replace __get_cpu_var().

I did notice this in local.h:

* X86_64: This could be done better if we moved the per cpu data directly
* after GS.

... which it now is, so true per_cpu variables could be optimized better as well.

Also, the above cpu_local_wrap(...) adds:

	#define cpu_local_wrap(l)               \
	({                                      \
	        preempt_disable();              \
	        (l);                            \
	        preempt_enable();               \
	})                                      \

... and there isn't a non-preemption version that I can find.

One other distinction is CPU_INC increments an arbitrary sized variable while local_inc requires a local_t variable. This may not make it usable in all cases.

Thanks,
Mike

Quoted 9 lines

Also, the above cpu_local_wrap(...) adds:

	#define cpu_local_wrap(l)               \
	({                                      \
	        preempt_disable();              \
	        (l);                            \
	        preempt_enable();               \
	})                                      \

Quoted 1 line

... and there isn't a non-preemption version that I can find.

Yes, this should be fixed. I thought i386 had optimized versions pre-merge, but I was wrong (%gs for per-cpu came later, and noone cleaned up these naive versions). Did you want me to write them?

Quoted 3 lines

One other distinction is CPU_INC increments an arbitrary sized variable while local_inc requires a local_t variable. This may not make it usable in all cases.

You might be right, but note that local_t is 64 bit on 64-bit platforms. And speculation of possible use cases isn't a good reason to rip out working infrastructure :)

Quoted 9 lines · Rusty Russell

Quoted 9 lines

Also, the above cpu_local_wrap(...) adds:

	#define cpu_local_wrap(l)               \
	({                                      \
	        preempt_disable();              \
	        (l);                            \
	        preempt_enable();               \
	})                                      \

Quoted 5 lines

Quoted 1 line

... and there isn't a non-preemption version that I can find.

Yes, this should be fixed. I thought i386 had optimized versions pre-merge, but I was wrong (%gs for per-cpu came later, and noone cleaned up these naive versions). Did you want me to write them?

How can that be fixed? You have no atomic instruction that calculates the per cpu address in one go.

And as long as that is the case you need to
disable preempt. Otherwise you may increment the per cpu variable of another processor because the process was rescheduled after the address was calculated but before the increment was done.

Quoted 6 lines

Quoted 3 lines

Yes, this should be fixed. I thought i386 had optimized versions pre-merge, but I was wrong (%gs for per-cpu came later, and noone cleaned up these naive versions). Did you want me to write them?

How can that be fixed? You have no atomic instruction that calculates the per cpu address in one go.

Huh? "incl %fs:varname" does exactly this.

Quoted 4 lines

And as long as that is the case you need to
disable preempt. Otherwise you may increment the per cpu variable of another processor because the process was rescheduled after the address was calculated but before the increment was done.

But of course, that is not a problem. You make local_t an atomic_t, and then it doesn't matter which CPU you incremented.

By definition if the caller cared, they would have had premption disabled.

Quoted 4 lines · Rusty Russell

Quoted 4 lines

Quoted 3 lines

Yes, this should be fixed. I thought i386 had optimized versions pre-merge, but I was wrong (%gs for per-cpu came later, and noone cleaned up these naive versions). Did you want me to write them?

Quoted 4 lines

Quoted 2 lines

How can that be fixed? You have no atomic instruction that calculates the per cpu address in one go.

Huh? "incl %fs:varname" does exactly this.

Right that is what the cpu alloc patches do. So you could implement cpu_local_inc on top of some of the cpu alloc patches.

Quoted 7 lines

Quoted 4 lines

And as long as that is the case you need to
disable preempt. Otherwise you may increment the per cpu variable of another processor because the process was rescheduled after the address was calculated but before the increment was done.

But of course, that is not a problem. You make local_t an atomic_t, and then it doesn't matter which CPU you incremented.

But then the whole point of local_t is gone. Why not use atomic_t in the first place?

Quoted 2 lines

By definition if the caller cared, they would have had premption disabled.

There are numerous instances where the caller does not care about preemption. Its just important that one per cpu counter is increment in the least intrusive way. See f.e. the VM event counters.

Quoted 2 lines

Right that is what the cpu alloc patches do. So you could implement cpu_local_inc on top of some of the cpu alloc patches.

Or you could just implement it today as a standalone patch.

Quoted 2 lines

But then the whole point of local_t is gone. Why not use atomic_t in the first place?

Because some archs can do better.

Quoted 6 lines

Quoted 2 lines

By definition if the caller cared, they would have had premption disabled.

There are numerous instances where the caller does not care about preemption. Its just important that one per cpu counter is increment in the least intrusive way. See f.e. the VM event counters.

Yes, and that's exactly the point. The VM event counters are exactly a case where you should have used cpu_local_inc.

Quoted 4 lines · Rusty Russell

Quoted 2 lines

Right that is what the cpu alloc patches do. So you could implement cpu_local_inc on top of some of the cpu alloc patches.

Or you could just implement it today as a standalone patch.

You need at least the zero basing to enable the use of the segment register on x86_64.

Quoted 4 lines

Quoted 2 lines

But then the whole point of local_t is gone. Why not use atomic_t in the first place?

Because some archs can do better.

The argument does not make any sense. First you want to use atomic_t then not?

You're being obtuse. See previous mail about the three possible implementations of local_t, and the comment in asm-generic/local.h.

The paths forward are clear:
1) Improve x86 local_t (mostly orthogonal to the others, but useful).

2) Implement extensible per-cpu areas.
3) Generalize per-cpu accessors.
4) Extend or replace the module.c per-cpu allocator to alloc from the other areas.
5) Convert alloc_percpu et al. to use the new code.

0000000000000013 <test_local_inc>:

  13:   55                      push   %rbp
  14:   65 48 8b 05 00 00 00    mov    %gs:0(%rip),%rax        # 1c <test_local_inc+0x9>
  1b:   00
  1c:   48 89 e5                mov    %rsp,%rbp
  1f:   48 ff 04 07             incq   (%rdi,%rax,1)
  23:   c9                      leaveq
  24:   c3                      retq

Quoted 2 lines

Believe me I have tried to use local_t repeatedly for vm statistics etc. It always fails on that issue.

Frankly, I am finding it increasingly easy to believe that you failed. But you are blaming the wrong thing.

There are three implementations of local_t which are obvious. The best is for architectures which can locate and increment a per-cpu var in one instruction (eg. x86). Otherwise, using atomic_t/atomic64_t for local_t provides a general solution. The other general solution would involve local_irq_disable()/increment/local_irq_enable().

My (fading) hope is that this idiocy is an abberation,

1. The x86 implementation does not exist because the segment register has so far not been available on x86_64. So you could not do the solution. You need the zero basing. Then you can use per_xxx_add in cpu_inc.

2. The general solution created overhead that is often not needed. If we would have done vm event counters with local_t then we would have atomic overhead for each increment on f.e. IA64. That was not acceptable. cpu_alloc never falls back to atomic operations.

3. local_t is based on the atomic logic. But percpu handling is fundamentally different in that accesses without the special macros are okay provided you are in a non preemptible or irq context! A local_t declaration makes such accesses impossible.

4. The modeling of local_t on atomic_t limits it to 32bit!

Quoted 1 line

4. The modeling of local_t on atomic_t limits it to 32bit!

Again wrong. And adding an exclamation mark doesn't make it true.

Quoted 3 lines · Rusty Russell

Quoted 1 line

4. The modeling of local_t on atomic_t limits it to 32bit!

Again wrong. And adding an exclamation mark doesn't make it true.

Ewww ... Its atomic_long_t ahh. Ok then there no 32 bit support. What about pointers?

Quoted 5 lines · Christoph Lameter

Quoted 3 lines · Rusty Russell

Quoted 1 line

4. The modeling of local_t on atomic_t limits it to 32bit!

Again wrong. And adding an exclamation mark doesn't make it true.

Quoted 2 lines

Ewww ... Its atomic_long_t ahh. Ok then there no 32 bit support. What about pointers?

sizeof(long) == sizeof(void *) in Linux, right?

If you were to support just a single data type, long would probably be the most useful. Still, it might be more consistent to support int and long, same as atomic.

Quoted 11 lines · On Thursday 12 June 2008 10:58:01 Nick Piggin wrote:

Quoted 7 lines · Christoph Lameter

Quoted 3 lines · Rusty Russell

Quoted 1 line

4. The modeling of local_t on atomic_t limits it to 32bit!

Again wrong. And adding an exclamation mark doesn't make it true.

Ewww ... Its atomic_long_t ahh. Ok then there no 32 bit support. What about pointers?

sizeof(long) == sizeof(void *) in Linux, right?

Quoted 3 lines

If you were to support just a single data type, long would probably be the most useful. Still, it might be more consistent to support int and long, same as atomic.

Sure, but in practice these tend to be simple counters: that could well change when dynamic percpu allocs become first class citizens, but let's not put the cart before the horse...

Per-cpu seems to be particularly prone to over-engineering: see commit 7ff6f08295d90ab20d25200ef485ebb45b1b8d71 from almost two years ago. Grepping here reveals that this infrastructure is still not used.

Quoted 12 lines · Rusty Russell

Quoted 11 lines · On Thursday 12 June 2008 10:58:01 Nick Piggin wrote:

Quoted 7 lines · Christoph Lameter

Quoted 3 lines · Rusty Russell

Quoted 1 line

4. The modeling of local_t on atomic_t limits it to 32bit!

Again wrong. And adding an exclamation mark doesn't make it true.

Ewww ... Its atomic_long_t ahh. Ok then there no 32 bit support. What about pointers?

sizeof(long) == sizeof(void *) in Linux, right?

Quoted 7 lines

Quoted 3 lines

If you were to support just a single data type, long would probably be the most useful. Still, it might be more consistent to support int and long, same as atomic.

Sure, but in practice these tend to be simple counters: that could well change when dynamic percpu allocs become first class citizens, but let's not put the cart before the horse...

Right, I was just responding to Christoph's puzzling question.

Quoted 3 lines

Per-cpu seems to be particularly prone to over-engineering: see commit 7ff6f08295d90ab20d25200ef485ebb45b1b8d71 from almost two years ago. Grepping here reveals that this infrastructure is still not used.

Hmm. Something like that needs the question asked "who uses this?" before it is merged I guess. If it were a trivial patch maybe not, but something like this that sits untested for so long is almost broken by definition ;)

Quoted 6 lines · Nick Piggin

Quoted 3 lines · Rusty Russell

Per-cpu seems to be particularly prone to over-engineering: see commit 7ff6f08295d90ab20d25200ef485ebb45b1b8d71 from almost two years ago. Grepping here reveals that this infrastructure is still not used.

Quoted 4 lines

Hmm. Something like that needs the question asked "who uses this?" before it is merged I guess. If it were a trivial patch maybe not, but something like this that sits untested for so long is almost broken by definition ;)

Some code of mine which didn't make it beyond -mm used this small per-cpu extension. So the commit you refer to was tested.

Populating the per cpu areas on demand is a good thing especially for configurations with a large number of processors. If we really go to support 4k processor by default then we need to allocate the smallest amount of per cpu structures necessary. Maybe ACPI or so can tell us how many processors are possible and we only allocate those. But it would be best if the percpu structures are only allocated for actually active processors.

cpu_possible_map should definitely be minimal, but your point is well made:
dynamic percpu could actually cut memory allocation. If we go for a hybrid scheme where static percpu is always allocated from the initial chunk, however, we still need the current pessimistic overallocation.

Mike's a clever guy, I'm sure he'll think of something :)

Quoted 4 lines · Rusty Russell

cpu_possible_map should definitely be minimal, but your point is well
made: dynamic percpu could actually cut memory allocation. If we go for a hybrid scheme where static percpu is always allocated from the initial chunk, however, we still need the current pessimistic overallocation.

The initial chunk would mean that the percpu areas all come from the same NUMA node. We really need to allocate from the node that is nearest to a processor (not all processors have processor local memory!).

It would be good to standardize the way that percpu areas are allocated. We have various ways of allocation now in various arches. init/main.c:setup_per_cpu_ares() needs to be generalized:

1. Allocate the per cpu areas in a NUMA aware fashions.

2. Have a function for instantiating a single per cpu area that can be used during cpu hotplug.

3. Some hooks for arches to override particular behavior as needed. F.e. IA64 allocates percpu structures in a special way. x86_64 needs to do some tricks for the pda etc etc.

Quoted 1 line

Mike's a clever guy, I'm sure he'll think of something :)

Hopefully. Otherwise he will ask me =-).

Quoted 3 lines

3. Some hooks for arches to override particular behavior as needed. F.e. IA64 allocates percpu structures in a special way. x86_64 needs to do some tricks for the pda etc etc.

IA64 is going to need some work, since dynamic percpu addresses won't be able to use their pinned TLB trick to get the local version.

Quoted 6 lines · Rusty Russell

Quoted 3 lines

3. Some hooks for arches to override particular behavior as needed. F.e. IA64 allocates percpu structures in a special way. x86_64 needs to do some tricks for the pda etc etc.

IA64 is going to need some work, since dynamic percpu addresses won't be able to use their pinned TLB trick to get the local version.

The ia64 hook could simply return the address of percpu area that was reserved when the per node memory layout was generated (which happens very early during node bootstrap).

Quoted 3 lines

The ia64 hook could simply return the address of percpu area that was reserved when the per node memory layout was generated (which happens very early during node bootstrap).

Apologies, this time I read the code. I thought IA64 used the pinned TLB area to access per-cpu vars under some circumstances, but they only do that via an arch-specific macro.

So creating new congruent mappings to expand the percpu area(s) is our main concern now?

Quoted 9 lines · On Tuesday 17 June 2008 00:52:08 Christoph Lameter wrote:

Quoted 5 lines · Rusty Russell

Quoted 3 lines

3. Some hooks for arches to override particular behavior as needed. F.e. IA64 allocates percpu structures in a special way. x86_64 needs to do some tricks for the pda etc etc.

IA64 is going to need some work, since dynamic percpu addresses won't be able to use their pinned TLB trick to get the local version.

The ia64 hook could simply return the address of percpu area that was reserved when the per node memory layout was generated (which happens very early during node bootstrap).

Apologies, this time I read the code. I thought IA64 used the pinned TLB area to access per-cpu vars under some circumstances, but they only do that via an arch-specific macro.

So creating new congruent mappings to expand the percpu area(s) is our main concern now?

Rusty.

On Thu, 29 May 2008 22:17:55 -0700 (PDT) Christoph Lameter

Quoted 2 lines

But then its related to percpu operations and relies extensively on the various percpu.h files in asm-generic and asm-arch and include/linux

Well that should be fixed. We should never have mixed the alloc_percpu() and DEFINE_PER_CPU things inthe same header. They're different.

otoh as you propose removing the old alloc_percpu() I guess the end result is no worse than what we presently have.

Quoted 7 lines · Andrew Morton

Quoted 5 lines

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

eh? That's what local_t is for?

No that is what local_t exactly cannot do.

Use cpu_alloc instead of allocpercpu.

Quoted 1 line · On Friday 30 May 2008 13:56:25 Christoph Lameter wrote:

Use cpu_alloc instead of allocpercpu.

These patches seem like useless churn.

Plus, the new code is uglier than the old code :(