Skip to contentSkip to conversation

Per-cpu UP unification

PATCH In review Rusty Russell 2 participants 3 messages May 20, 2003 – May 21, 2003

Subsystems (3)
Memory management Generic include/asm header files Per-CPU memory allocator
lore
Share & tools
lore

Rusty Russell

author
#0 lore raw

PATCH 1/3 Per-cpu UP unification

[ Untested on ia64, but fairly trivial if I've broken something ].

Name: Unification of per-cpu headers for non-SMP
Author: Rusty Russell
Status: Trivial

D: Move non-SMP per-cpu operations from asm-*/percpu.h to
D: linux/percpu.h. There is no reason why archs would want to have
D: their own versions of non-SMP per-cpu operations, and making each
D: arch override them is just silly.

include/asm-generic/percpu.h +0 -16
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .14436-linux-2.5.69-bk13/include/asm-generic/percpu.h .14436-linux-2.5.69-bk13.updated/include/asm-generic/percpu.h
@@ -17,22 +17,6 @@ extern unsigned long __per_cpu_offset[NR
 #define per_cpu(var, cpu) (*RELOC_HIDE(&var##__per_cpu, __per_cpu_offset[cpu]))
 #define __get_cpu_var(var) per_cpu(var, smp_processor_id())
 
-#else /* ! SMP */
-
-/* Can't define per-cpu variables in modules.  Sorry --RR */
-#ifndef MODULE
-#define DEFINE_PER_CPU(type, name) \
-    __typeof__(type) name##__per_cpu
-#endif
-
-#define per_cpu(var, cpu)			((void)cpu, var##__per_cpu)
-#define __get_cpu_var(var)			var##__per_cpu
-
 #endif	/* SMP */
 
-#define DECLARE_PER_CPU(type, name) extern __typeof__(type) name##__per_cpu
-
-#define EXPORT_PER_CPU_SYMBOL(var) EXPORT_SYMBOL(var##__per_cpu)
-#define EXPORT_PER_CPU_SYMBOL_GPL(var) EXPORT_SYMBOL_GPL(var##__per_cpu)
-
 #endif /* _ASM_GENERIC_PERCPU_H_ */
include/asm-ia64/percpu.h +3 -9
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .14436-linux-2.5.69-bk13/include/asm-ia64/percpu.h .14436-linux-2.5.69-bk13.updated/include/asm-ia64/percpu.h
@@ -17,23 +17,17 @@
 
 #include <linux/threads.h>
 
+#ifdef CONFIG_SMP
 extern unsigned long __per_cpu_offset[NR_CPUS];
 
 #ifndef MODULE
 #define DEFINE_PER_CPU(type, name) \
     __attribute__((__section__(".data.percpu"))) __typeof__(type) name##__per_cpu
 #endif
-#define DECLARE_PER_CPU(type, name) extern __typeof__(type) name##__per_cpu
 
 #define __get_cpu_var(var)	(var##__per_cpu)
-#ifdef CONFIG_SMP
-# define per_cpu(var, cpu)	(*RELOC_HIDE(&var##__per_cpu, __per_cpu_offset[cpu]))
-#else
-# define per_cpu(var, cpu)	((void)cpu, __get_cpu_var(var))
-#endif
-
-#define EXPORT_PER_CPU_SYMBOL(var) EXPORT_SYMBOL(var##__per_cpu)
-#define EXPORT_PER_CPU_SYMBOL_GPL(var) EXPORT_SYMBOL_GPL(var##__per_cpu)
+#define per_cpu(var, cpu)	(*RELOC_HIDE(&var##__per_cpu, __per_cpu_offset[cpu]))
+#endif /* CONFIG_SMP */
 
 extern void setup_per_cpu_areas (void);
include/linux/percpu.h +14 -0
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .14436-linux-2.5.69-bk13/include/linux/percpu.h .14436-linux-2.5.69-bk13.updated/include/linux/percpu.h
@@ -44,6 +44,15 @@ static inline void kfree_percpu(const vo
 }
 static inline void kmalloc_percpu_init(void) { }
 
+/* Can't define per-cpu variables in modules.  Sorry --RR */
+#ifndef MODULE
+#define DEFINE_PER_CPU(type, name) \
+    __typeof__(type) name##__per_cpu
+#endif
+
+#define per_cpu(var, cpu)			((void)cpu, var##__per_cpu)
+#define __get_cpu_var(var)			var##__per_cpu
+
 #endif /* CONFIG_SMP */
 
 /* 

@@ -68,4 +77,9 @@ static inline void kmalloc_percpu_init(v
 #define get_cpu_ptr(ptr) per_cpu_ptr(ptr, get_cpu())
 #define put_cpu_ptr(ptr) put_cpu()
 
+#define DECLARE_PER_CPU(type, name) extern __typeof__(type) name##__per_cpu
+
+#define EXPORT_PER_CPU_SYMBOL(var) EXPORT_SYMBOL(var##__per_cpu)
+#define EXPORT_PER_CPU_SYMBOL_GPL(var) EXPORT_SYMBOL_GPL(var##__per_cpu)
+
 #endif /* __LINUX_PERCPU_H */

_GPL(var) EXPORT_SYMBOL_GPL(var##__per_cpu)
+
#endif /* __LINUX_PERCPU_H */
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

#1 lore raw
Quoted 6 lines · Rusty Russell

[ Untested on ia64, but fairly trivial if I've broken something ].

Name: Unification of per-cpu headers for non-SMP
Author: Rusty Russell
Status: Trivial

I applied all these to the ppc64 kernel (you missed ppc64 btw) and it dies.

Quite late in boot, during floppy_init->init_disk_stats->alloc_percpu.

I'm reduced to debugging with printk on ppc64. __alloc_percpu() calls new_block(), loops around and then dies in here:

#define D() printk("at %s:%d\n", __FILE__, __LINE__)

			D();
			/* Transfer extra to previous block. */
			if (b->size[i-1] < 0)
				b->size[i-1] -= extra;
			else
				b->size[i-1] += extra;
			b->size[i] -= extra;
			ptr += extra;

			D();

Not sure what happened - no oops, no xmon, no sysrq, no nuthin. It even manages to lock up minicom on the other end of the cable. Impressed.

author
#2 lore raw

In message <[email protected]> you write:

Quoted 8 lines
Quoted 6 lines · Rusty Russell

[ Untested on ia64, but fairly trivial if I've broken something ].

Name: Unification of per-cpu headers for non-SMP
Author: Rusty Russell
Status: Trivial

Quoted 6 lines

I applied all these to the ppc64 kernel (you missed ppc64 btw) and it dies.

Quite late in boot, during floppy_init->init_disk_stats->alloc_percpu.

I'm reduced to debugging with printk on ppc64. __alloc_percpu() calls new_block(), loops around and then dies in here:

Hmm, Works For Me(TM). Here's my test code: change init_alloc_percpu() to call test_percpu() explicitly at the end (so before you hit the current bug).

I'll test on bk14 here, and if that passes I'll seek a PPC64 box...

Thanks!
Rusty.

Name: kmalloc testing patch
Author: Rusty Russell
Status: Tested on 2.5.69-bk13
Depends: Misc/kmalloc_percpu-full.patch.gz

D: This adds simple test code to kmalloc percpu.

mm/percpu.c +153 -0
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal working-2.5.69-bk13-kmalloc_percpu-full/mm/percpu.c working-2.5.69-bk13-kmalloc_percpu-debug/mm/percpu.c
@@ -245,6 +245,159 @@ void free_percpu(const void *freeme)
 	BUG();
 }
 
+#if 1
+#include <linux/random.h>
+
+static void check_values(unsigned int *ptrs[],
+			 unsigned int sizes[],
+			 unsigned int num)
+{
+	unsigned int cpu, i;
+	unsigned char *ptr;
+
+	if (!ptrs[num])
+		return;
+
+	for (cpu = 0; cpu < NR_CPUS; cpu++) {
+		ptr = (unsigned char *)per_cpu_ptr(ptrs[num], cpu);
+		for (i = 0; i < sizes[num]; i++) {
+			if (ptr[i] != (unsigned char)(num + cpu))
+				BUG();
+		}
+	}
+}
+
+static void check_blocklen(void)
+{
+	struct pcpu_block *i;
+	unsigned int count = 0;
+
+	list_for_each_entry(i, &pcpu_blocks, list)
+		count++;
+
+	printk("Total blocks = %u\n", count);
+}
+
+static void dump_blocks(const char *start_or_end)
+{
+	struct pcpu_block *i;
+
+	list_for_each_entry(i, &pcpu_blocks, list) {
+		int expected_blocks = 1;
+
+		if (i->base_ptr == percpu_base)
+			expected_blocks = 2;
+		if (i->num_used != expected_blocks) {
+			printk("Block %p has %u subs at %s\n",
+			       i->base_ptr, i->num_used, start_or_end);
+		}
+	}
+}	
+
+static void mymemset(void *ptr, int c, unsigned long len)
+{
+	unsigned char *p = ptr;
+	while (len > 0) {
+		*p = c;
+		p++;
+		len--;
+	}
+}		
+
+static int random(void)
+{
+	unsigned short s;
+
+	get_random_bytes(&s, sizeof(s));
+	return s;
+}
+
+static int test_percpu(void)
+{
+	unsigned int i, allocs, frees;
+	unsigned int *ptr;
+	static unsigned int *ptrs[PERCPU_MAX], sizes[PERCPU_MAX];
+
+	dump_blocks("start");
+	allocs = frees = 0;
+	ptr = __alloc_percpu(4, 4); allocs++;
+	printk("This cpu = %p (%u)\n",
+	       __get_cpu_ptr(ptr), *__get_cpu_ptr(ptr));
+	for (i = 0; i < NR_CPUS; i++) {
+		printk("&ptr[i] == %p (%u)\n",
+		       per_cpu_ptr(ptr, i), *per_cpu_ptr(ptr, i));
+		*per_cpu_ptr(ptr, i) = i;
+	}
+	free_percpu(ptr); frees++;
+
+	BUG_ON(allocs != frees);
+
+	for (i = 4; i < PERCPU_MAX; i+=27) {
+		unsigned int j;
+		ptrs[i] = __alloc_percpu(i, 4); allocs++;
+		for (j = 0; j < NR_CPUS; j++) {
+			mymemset(per_cpu_ptr(ptrs[i], j), 0, i);
+			*per_cpu_ptr(ptrs[i], j) = i;
+		}
+	}
+
+	for (i = 4; i < PERCPU_MAX; i+=27) {
+		unsigned int j;
+		for (j = 0; j < NR_CPUS; j++)
+			if (*per_cpu_ptr(ptrs[i], j) != i)
+				BUG();
+	}
+	for (i = 4; i < PERCPU_MAX; i+=27) {
+		free_percpu(ptrs[i]); frees++;
+		ptrs[i] = NULL;
+	}
+
+	BUG_ON(allocs != frees);
+
+	/* Randomized test. */
+	for (i = 0; i < 10000; i++) {
+		unsigned int j = random() % PERCPU_MAX;
+		if (!ptrs[j]) {
+			unsigned int cpu;
+
+			sizes[j] = random() % PERCPU_MAX;
+			if (sizes[j] < 4)
+				sizes[j] = 4;
+			ptrs[j] = __alloc_percpu(sizes[j], 1<<(random()%L1_CACHE_SHIFT));
+			allocs++;
+
+			for (cpu = 0; cpu < NR_CPUS; cpu++)
+				memset(per_cpu_ptr(ptrs[j], cpu), j+cpu,
+				       sizes[j]);
+		} else {
+			if (random() % 1000 == 0) {
+				printk("c\n");
+				for (j = 0; j < PERCPU_MAX; j++)
+					check_values(ptrs, sizes, j);
+			} else {
+				check_values(ptrs, sizes, j);
+				free_percpu(ptrs[j]); frees++;
+				ptrs[j] = NULL;
+			}
+		}
+		if (i % (10000/10) == 0)
+			printk(".\n");
+	}
+	check_blocklen();
+
+	for (i = 0; i < PERCPU_MAX; i++) {
+		if (ptrs[i]) {
+			free_percpu(ptrs[i]); frees++;
+			ptrs[i] = NULL;
+		}
+	}
+	BUG_ON(allocs != frees);
+	dump_blocks("end");
+	return 0;
+}
+late_initcall(test_percpu);
+#endif
+
 unsigned long __per_cpu_offset[NR_CPUS];
 EXPORT_SYMBOL(__per_cpu_offset);

--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.