kmalloc_percpu

Hi Andrew,

This is the kmalloc_percpu patch. I have another patch which
tests the allocator if you want to see that to. This is the precursor to per-cpu stuff in modules, but also allows other space gains for structures which currently embed per-cpu arrays (eg. your fuzzy counters).

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

Name: kmalloc_percpu to use same percpu operators
Author: Rusty Russell
Status: Tested on 2.5.68-bk11

D: By overallocating the per-cpu data at boot, we can make quite an
D: efficient allocator, and then use it to support per-cpu data in
D: modules (next patch).

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .29880-linux-2.5.69/include/asm-generic/percpu.h .29880-linux-2.5.69.updated/include/asm-generic/percpu.h
@@ -2,37 +2,11 @@
 #define _ASM_GENERIC_PERCPU_H_
 #include <linux/compiler.h>
 
-#define __GENERIC_PER_CPU
+/* Some archs may want to keep __per_cpu_offset for this CPU in a register,
+   or do their own allocation. */
 #ifdef CONFIG_SMP
-
-extern unsigned long __per_cpu_offset[NR_CPUS];
-
-/* Separate out the type, so (int[3], foo) works. */
-#ifndef MODULE
-#define DEFINE_PER_CPU(type, name) \
-    __attribute__((__section__(".data.percpu"))) __typeof__(type) name##__per_cpu
-#endif
-
-/* var is in discarded region: offset to particular copy we want */
-#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)
-
+#define __get_cpu_ptr(var) per_cpu_ptr(ptr, smp_processor_id())
+#define __NEED_SETUP_PER_CPU_AREAS
+#endif /* SMP */
 #endif /* _ASM_GENERIC_PERCPU_H_ */

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .29880-linux-2.5.69/include/linux/genhd.h .29880-linux-2.5.69.updated/include/linux/genhd.h
@@ -160,10 +160,9 @@ static inline void disk_stat_set_all(str
 #ifdef  CONFIG_SMP
 static inline int init_disk_stats(struct gendisk *disk)
 {
-	disk->dkstats = kmalloc_percpu(sizeof (struct disk_stats), GFP_KERNEL);
+	disk->dkstats = kmalloc_percpu(struct disk_stats);
 	if (!disk->dkstats)
 		return 0;
-	disk_stat_set_all(disk, 0);
 	return 1;
 }
 

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .29880-linux-2.5.69/include/net/ipv6.h .29880-linux-2.5.69.updated/include/net/ipv6.h
@@ -145,7 +145,7 @@ extern atomic_t			inet6_sock_nr;
 
 int snmp6_register_dev(struct inet6_dev *idev);
 int snmp6_unregister_dev(struct inet6_dev *idev);
-int snmp6_mib_init(void *ptr[2], size_t mibsize);
+int snmp6_mib_init(void *ptr[2], size_t mibsize, size_t mibalign);
 void snmp6_mib_free(void *ptr[2]);
 
 struct ip6_ra_chain

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .29880-linux-2.5.69/init/main.c .29880-linux-2.5.69.updated/init/main.c
@@ -301,35 +301,10 @@ static void __init smp_init(void)
 #define smp_init()	do { } while (0)
 #endif
 
-static inline void setup_per_cpu_areas(void) { }
 static inline void smp_prepare_cpus(unsigned int maxcpus) { }
 
 #else
 
-#ifdef __GENERIC_PER_CPU
-unsigned long __per_cpu_offset[NR_CPUS];
-
-static void __init setup_per_cpu_areas(void)
-{
-	unsigned long size, i;
-	char *ptr;
-	/* Created by linker magic */
-	extern char __per_cpu_start[], __per_cpu_end[];
-
-	/* Copy section for each CPU (we discard the original) */
-	size = ALIGN(__per_cpu_end - __per_cpu_start, SMP_CACHE_BYTES);
-	if (!size)
-		return;
-
-	ptr = alloc_bootmem(size * NR_CPUS);
-
-	for (i = 0; i < NR_CPUS; i++, ptr += size) {
-		__per_cpu_offset[i] = ptr - __per_cpu_start;
-		memcpy(ptr, __per_cpu_start, size);
-	}
-}
-#endif /* !__GENERIC_PER_CPU */
-
 /* Called by boot processor to activate the rest. */
 static void __init smp_init(void)
 {

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .29880-linux-2.5.69/kernel/ksyms.c .29880-linux-2.5.69.updated/kernel/ksyms.c
@@ -99,7 +99,7 @@ EXPORT_SYMBOL(remove_shrinker);
 EXPORT_SYMBOL(kmalloc);
 EXPORT_SYMBOL(kfree);
 #ifdef CONFIG_SMP
-EXPORT_SYMBOL(kmalloc_percpu);
+EXPORT_SYMBOL(__alloc_percpu);
 EXPORT_SYMBOL(kfree_percpu);
 EXPORT_SYMBOL(percpu_counter_mod);
 #endif

@@ -607,7 +607,7 @@ EXPORT_SYMBOL(init_thread_union);
 EXPORT_SYMBOL(tasklist_lock);
 EXPORT_SYMBOL(find_task_by_pid);
 EXPORT_SYMBOL(next_thread);
-#if defined(CONFIG_SMP) && defined(__GENERIC_PER_CPU)
+#if defined(CONFIG_SMP)
 EXPORT_SYMBOL(__per_cpu_offset);
 #endif
 

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .29880-linux-2.5.69/mm/Makefile .29880-linux-2.5.69.updated/mm/Makefile
@@ -12,3 +12,4 @@ obj-y			:= bootmem.o filemap.o mempool.o
 			   slab.o swap.o truncate.o vcache.o vmscan.o $(mmu-y)
 
 obj-$(CONFIG_SWAP)	+= page_io.o swap_state.o swapfile.o
+obj-$(CONFIG_SMP)	+= percpu.o

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .29880-linux-2.5.69/net/ipv4/route.c .29880-linux-2.5.69.updated/net/ipv4/route.c
@@ -2689,17 +2689,9 @@ int __init ip_rt_init(void)
 	ipv4_dst_ops.gc_thresh = (rt_hash_mask + 1);
 	ip_rt_max_size = (rt_hash_mask + 1) * 16;
 
-	rt_cache_stat = kmalloc_percpu(sizeof (struct rt_cache_stat),
-					GFP_KERNEL);
+	rt_cache_stat = kmalloc_percpu(struct rt_cache_stat);
 	if (!rt_cache_stat) 
 		goto out_enomem1;
-	for (i = 0; i < NR_CPUS; i++) {
-		if (cpu_possible(i)) {
-			memset(per_cpu_ptr(rt_cache_stat, i), 0,
-			       sizeof (struct rt_cache_stat));
-		}
-	}
-
 	devinet_init();
 	ip_fib_init();
 

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .29880-linux-2.5.69/net/ipv6/af_inet6.c .29880-linux-2.5.69.updated/net/ipv6/af_inet6.c
@@ -633,28 +633,20 @@ inet6_unregister_protosw(struct inet_pro
 }
 
 int
-snmp6_mib_init(void *ptr[2], size_t mibsize)
+snmp6_mib_init(void *ptr[2], size_t mibsize, size_t mibalign)
 {
 	int i;
 
 	if (ptr == NULL)
 		return -EINVAL;
 
-	ptr[0] = kmalloc_percpu(mibsize, GFP_KERNEL);
+	ptr[0] = __alloc_percpu(mibsize, mibalign);
 	if (!ptr[0])
 		goto err0;
 
-	ptr[1] = kmalloc_percpu(mibsize, GFP_KERNEL);
+	ptr[1] = __alloc_percpu(mibsize, mibalign);
 	if (!ptr[1])
 		goto err1;
-
-	/* Zero percpu version of the mibs */
-	for (i = 0; i < NR_CPUS; i++) {
-		if (cpu_possible(i)) {
-			memset(per_cpu_ptr(ptr[0], i), 0, mibsize);
-			memset(per_cpu_ptr(ptr[1], i), 0, mibsize);
-		}
-	}
 	return 0;
 
 err1:

@@ -676,11 +668,14 @@ snmp6_mib_free(void *ptr[2])
 
 static int __init init_ipv6_mibs(void)
 {
-	if (snmp6_mib_init((void **)ipv6_statistics, sizeof (struct ipv6_mib)) < 0)
+	if (snmp6_mib_init((void **)ipv6_statistics, sizeof (struct ipv6_mib),
+			   __alignof__(struct ipv6_mib)) < 0)
 		goto err_ip_mib;
-	if (snmp6_mib_init((void **)icmpv6_statistics, sizeof (struct icmpv6_mib)) < 0)
+	if (snmp6_mib_init((void **)icmpv6_statistics, sizeof (struct icmpv6_mib),
+			   __alignof__(struct icmpv6_mib)) < 0)
 		goto err_icmp_mib;
-	if (snmp6_mib_init((void **)udp_stats_in6, sizeof (struct udp_mib)) < 0)
+	if (snmp6_mib_init((void **)udp_stats_in6, sizeof (struct udp_mib),
+			   __alignof__(struct udp_mib)) < 0)
 		goto err_udp_mib;
 	return 0;
 

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .29880-linux-2.5.69/net/ipv6/proc.c .29880-linux-2.5.69.updated/net/ipv6/proc.c
@@ -226,7 +226,7 @@ int snmp6_register_dev(struct inet6_dev 
 	if (!idev || !idev->dev)
 		return -EINVAL;
 
-	if (snmp6_mib_init((void **)idev->stats.icmpv6, sizeof(struct icmpv6_mib)) < 0)
+	if (snmp6_mib_init((void **)idev->stats.icmpv6, sizeof(struct icmpv6_mib), __alignof__(struct icmpv6_mib)) < 0)
 		goto err_icmp;
 
 #ifdef CONFIG_PROC_FS

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .29880-linux-2.5.69/net/sctp/protocol.c .29880-linux-2.5.69.updated/net/sctp/protocol.c
@@ -855,26 +855,14 @@ static int __init init_sctp_mibs(void)
 {
 	int i;
 
-	sctp_statistics[0] = kmalloc_percpu(sizeof (struct sctp_mib),
-					    GFP_KERNEL);
+	sctp_statistics[0] = kmalloc_percpu(struct sctp_mib);
 	if (!sctp_statistics[0])
 		return -ENOMEM;
-	sctp_statistics[1] = kmalloc_percpu(sizeof (struct sctp_mib),
-					    GFP_KERNEL);
+	sctp_statistics[1] = kmalloc_percpu(struct sctp_mib);
 	if (!sctp_statistics[1]) {
 		kfree_percpu(sctp_statistics[0]);
 		return -ENOMEM;
 	}
-
-	/* Zero all percpu versions of the mibs */
-	for (i = 0; i < NR_CPUS; i++) {
-		if (cpu_possible(i)) {
-			memset(per_cpu_ptr(sctp_statistics[0], i), 0,
-					sizeof (struct sctp_mib));
-			memset(per_cpu_ptr(sctp_statistics[1], i), 0,
-					sizeof (struct sctp_mib));
-		}
-	}
 	return 0;
 
 }

How does it work? What restrictions does it have, and what compromises were made?

+#define PERCPU_POOL_SIZE 32768

What's this?

The current implementation of kmalloc_per_cpu() turned out to be fairly disappointing because of the number of derefs which were necessary to get at
the data in fastpaths. How does this implementation compare?

Thanks.

In message <[email protected]> you write:

OK. It has a size restriction: PERCPU_POOL_SIZE is the maximum total kmalloc_percpu + static DECLARE_PER_CPU you'll get, ever. This is the main downside. It's allocated at boot.

The __alloc_percpu allocator is extremely space efficient, by not insisting on cache-line aligning everything: __alloc_percpu(SIZE) overhead is sizeof(int), plus SIZE bytes (rounded up to alignment requirements) removed from per-cpu pool.

The allocator is fairly slow: they're not expected to be thrown around like candy.

It uses the same method as the static ones, so it's a single addition of __per_cpu_offset (assuming arch doesn't override implementation). This is a requirement for modules to use them (which was my aim: the other side effects are cream).

Hope that clarifies,
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

And is subject to fragmentation.

Is it not possible to go allocate another N * PERCPU_POOL_SIZE from slab if it runs out?

That way, PERCPU_POOL_SIZE only needs to be sized for non-modular static percpu data, which sounds more robust.

No, then you go back to things requireing multiple levels of dereferencing. It's hard to realloc() because you have to freeze the whole kernel to do that properly, and that is not simple at all.

I think the fixed size pool is perfectly reasonable.

David S. Miller <[email protected]>

In message <[email protected]> you write:

Actually, you can; my previous patch did this. But then all CPUS have to be one continuous allocation: since modern big-SMP machines are non-uniform, so you don't want this.

http://www.kernel.org/pub/linux/kernel/people/rusty/patches/Misc/kmalloc_percpu-orig.patch.gz

Yes. It's a tradeoff. I think it's worth it at the moment (although I'll add a limited printk to __alloc_percpu if it fails).

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

It's OK as long as nobody uses the feature! Once it starts to be commonly used (say, in driver ->open() methods) then we'll get into the same problems as with vmalloc exhaustion, vmalloc fragmentation, large physically-contig allocations, etc.

Ho-hum. Can the magical constant become a __setup thing?

From: Andrew Morton <[email protected]>
Date: Mon, 5 May 2003 21:28:16 -0700

It's OK as long as nobody uses the feature!

I think this is closer to say, allocation of kmap types, than it is to vmalloc() et al. (as you suggest).

Ho-hum. Can the magical constant become a __setup thing?

Remember that there are physical limitations, for example on ia64, as to how big this thing can be. So whatever any of us think about physical limitations, we have to deal with them anyways :-)

I think firstly, that we should define that this isn't something you be doing after module_init() (ie. your ->open() example, that's rediculious). Ideas on how to enforce this are welcome.

Next, we can calculate how much per-cpu space all the modules need. And because we can do that, we can preallocate slots if we wanted to in order to deal with whatever theoretical fragmentation problems you think exist (much like how Jakub Jelink's prelinking works).

I personally don't know how smart it is to let random modules use kmalloc_percpu() with impunity. But aparently someone thinks there is some value in that.

From: Rusty Russell <[email protected]>
Date: Tue, 06 May 2003 14:08:27 +1000

In message <[email protected]> you write:

Yes. It's a tradeoff. I think it's worth it at the moment (although I'll add a limited printk to __alloc_percpu if it fails).

I think you should BUG() if a module calls kmalloc_percpu() outside of mod->init(), this is actually implementable.

Andrew's example with some module doing kmalloc_percpu() inside of fops->open() is just rediculious.

The disk stats are already per-cpu. So, what happens when you offline/online a disk ? How do you allocate per-cpu memory during that ?

Thanks
Dipankar

crap. Modules deal with per-device and per-mount objects. If a module cannot use kmalloc_per_cpu on behalf of the primary object which it manages then the facility is simply not useful to modules.

The static DEFINE_PER_CPU allocation works OK in core kernel because core kernel does _not_ use per-instance objects. But modules do.

A case in point, which Rusty has twice mentioned, is the three per-mount fuzzy counters in the ext2 superblock. And lo, ext2 cannot use the code in this patch, because people want to scale to 4000 mounts.

In those very rare cases where a module wants allocation to be performed at module_init()-time (presumably global stats counters), they can use DEFINE_PER_CPU, so we should just not export kmalloc_per_cpu() to modules at all.

From: Andrew Morton <[email protected]>
Date: Mon, 5 May 2003 22:02:50 -0700

crap. Modules deal with per-device and per-mount objects. If a module cannot use kmalloc_per_cpu on behalf of the primary object which it manages then the facility is simply not useful to modules.

Ok then.

Please address the ia64 concerns then :-) It probably means we have to stay with the dereferencing stuff... at which point you might as well use normal kmalloc() and smp_processor_id() indexing inside of modules.

I think so. So we'd end up with:

- DEFINE_PER_CPU and kmalloc_percpu() work in core kernel, and use the 32k pool.

- DEFINE_PER_CPU in modules uses the 32k pool as well (core kernel does the allocation).

- kmalloc_per_cpu() is unavailble to modules (it ain't exported).

AFAICT the only thing which will break is sctp, which needs a trivial conversion to DEFINE_PER_CPU.

except sizeof(struct disk_stats)=44, so we run out of percpu space at 744 disks.

Can't think of anything very clever there, except to go and un-percpuify the disk stats. I think that's best, really - disk requests only come in at 100 to 200 per second - atomic_t's or int-plus-per-disk-spinlock will be fine.

From: Andrew Morton <[email protected]>
Date: Mon, 5 May 2003 23:42:48 -0700

Can't think of anything very clever there, except to go and un-percpuify the disk stats. I think that's best, really - disk requests only come in at 100 to 200 per second - atomic_t's or int-plus-per-disk-spinlock will be fine.

Use some spinlock we already have to be holding during the counter bumps.

Frankly, these things don't need to be %100 accurate. Using a new spinlock or an atomic_t for this seems rediculious.

Last time we looked at that, q->lock was already held in almost all the right places so yes, that'd work.

The disk_stats structure has an "in flight" member. If we don't have proper locking around that, disks will appear to have -3 requests in flight for all time, which would look a tad odd.

As far as I can see, queue lock _is_ held in all the right spot. At least where it matters, adding new samples.

So check for < 0 in flight? I totally agree with davem here.

Jens Axboe

If the disk_stats structure will never be accurate, it will show nonsense values. If it shows nonsense values, the values have no meaning anymore and we could remove them alltogether.

If some additions/subtractions will come in later it will not matter, but if the value we add to or subtract from is already wrong, then the error will really propagate to much.

Has somebody analyzed this? Can we tell userspace a maximum error value? Is the maximum error value acceptable?

Is the above questions are all answered with "no", then I (and several sysadmins) would prefer to just rip the stats or make them a compile time option and be exact, if we really want them.

What about that?

Most people just want them per system to measure their IO bandwidth and they want the spots (=per disk stats), just to analyze the problematic cases.

For the real performance, they could always compile out the per disk stats, once they are satisfied with the overall bandwidth.

Regards

Ingo Oeser

For what it's worth, the disk stats patch that vendors have been shipping forever with 2.4 (and that Christoph pushed into 2.4.20) has this problem, so at least people will be used to it.

	<b

From: Andrew Morton <[email protected]>
Date: Mon, 5 May 2003 22:48:15 -0700

I think so. So we'd end up with:

- DEFINE_PER_CPU and kmalloc_percpu() work in core kernel, and use the 32k pool.

- DEFINE_PER_CPU in modules uses the 32k pool as well (core kernel does the allocation).

- kmalloc_per_cpu() is unavailble to modules (it ain't exported).

AFAICT the only thing which will break is sctp, which needs a trivial conversion to DEFINE_PER_CPU.

Your grep is faulty, we're using kmalloc_percpu() in ipv6 for per-cpu and per-device icmp stats.

You solution doesn't work in that case. Also ipv4 will have the same problems if we make that modular at some point.

I also don't see how this fits in for your ext2 fuzzy counter stuff. It isn't a "module" for most people, I can't even remember if I've ever built ext2 non-statically. :-) It almost appears as if you are suggesting kmalloc_percpu() is not usable at all.

So there we have it, there are a total of 3 users of kmalloc_percpu() (ipv4/ipv6/diskstats) so let's decide if it's going to continue to live longer or not before there are any more. :-)

My grep is fine. It's the brain which failed ;)

Well it doesn't fit. With the proposals which we are discussing here, ext2 (and, we hope, soon ext3) would continue to use foo[NR_CPUS].

kmalloc_per_cpu() is useful at present. But with Rusty's patch it becomes less useful.

Aside: I rather think that 4000 filesystems isn't sane. 4000 disks is, but I find it hard to believe that people will have a separate fs on each one...

How about we leave kmalloc_per_cpu as-is (it uses kmalloc()), and only apply Rusty's new code to DEFINE_PER_CPU?

From: Andrew Morton <[email protected]>
Date: Mon, 5 May 2003 23:55:49 -0700

How about we leave kmalloc_per_cpu as-is (it uses kmalloc()), and only apply Rusty's new code to DEFINE_PER_CPU?

I propose to make it use kmalloc() all the time.

It simply doesn't make sense to use a pool given what you've shown me. If we've decided that any limit whatsover is bad, why impose any limit at all? Smells like bad design frankly.

Normal DEFINE_PER_CPU() need not a pool, therefore we don't need a pool for anything.

Make kmalloc_per_cpu() merely a convenience macro, made up of existing non-percpu primitives.

I think we're agreeing here.

The current kmalloc_percpu() is a wrapper around kmalloc. That seems OK to me.

What we _do_ want to solve is the problem that DEFINE_PERCPU() does not work in modules. Rusty's patch (reworked to not alter kmalloc_percpu) would suit that requirement.

(kiran has a new version of kmalloc_percpu() which may be faster than the current one, but for the purposes of this discussion it's equivalent).

From: Andrew Morton <[email protected]>
Date: Tue, 6 May 2003 00:22:29 -0700

I think we're agreeing here.

As just pointed out by dipankar the only issue is NUMA... so it has to be something more sophisticated than simply kmalloc()[smp_processor_id];

The proposed patch doesn't do anything about that either.

+ ptr = alloc_bootmem(PERCPU_POOL_SIZE * NR_CPUS);

So yes, we need an api which could be extended to use node-affine memory at some time in the future. I think we have that.

IIRC that can be overridden; I wrote something to do node-local per-cpu areas for i386 for some prior incarnations of per-cpu stuff and even posted it, and this looks like it bootstraps at the same time (before free_area_init_core() that is) and has the same override hook.

-- wli

You can just call alloc_bootmem_node for each CPU instead. It doesn't work on i386 at the moment (well, it'll work but come out of node 0), but it probably ought to.

M.

And that is not what we want to do in a NUMA box.

Thanks
Dipankar

In message <[email protected]> you write:

It's a tradeoff, but I think it's worth it for a kmalloc_percpu which is fast, space-efficient and numa-aware, since the code is needed anyway.

How about a compromise like the following (scaled with mem)? Untested, but you get the idea...

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

Name: kmalloc_percpu to use same percpu operators
Author: Rusty Russell
Status: Untested

D: By overallocating the per-cpu data at boot, we can make quite an
D: efficient allocator, and then use it to support per-cpu data in
D: modules (next patch).

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .26261-linux-2.5.69/include/asm-generic/percpu.h .26261-linux-2.5.69.updated/include/asm-generic/percpu.h
@@ -2,37 +2,11 @@
 #define _ASM_GENERIC_PERCPU_H_
 #include <linux/compiler.h>
 
-#define __GENERIC_PER_CPU
+/* Some archs may want to keep __per_cpu_offset for this CPU in a register,
+   or do their own allocation. */
 #ifdef CONFIG_SMP
-
-extern unsigned long __per_cpu_offset[NR_CPUS];
-
-/* Separate out the type, so (int[3], foo) works. */
-#ifndef MODULE
-#define DEFINE_PER_CPU(type, name) \
-    __attribute__((__section__(".data.percpu"))) __typeof__(type) name##__per_cpu
-#endif
-
-/* var is in discarded region: offset to particular copy we want */
-#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)
-
+#define __get_cpu_ptr(var) per_cpu_ptr(ptr, smp_processor_id())
+#define __NEED_SETUP_PER_CPU_AREAS
+#endif /* SMP */
 #endif /* _ASM_GENERIC_PERCPU_H_ */

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .26261-linux-2.5.69/include/linux/genhd.h .26261-linux-2.5.69.updated/include/linux/genhd.h
@@ -160,10 +160,9 @@ static inline void disk_stat_set_all(str
 #ifdef  CONFIG_SMP
 static inline int init_disk_stats(struct gendisk *disk)
 {
-	disk->dkstats = kmalloc_percpu(sizeof (struct disk_stats), GFP_KERNEL);
+	disk->dkstats = kmalloc_percpu(struct disk_stats);
 	if (!disk->dkstats)
 		return 0;
-	disk_stat_set_all(disk, 0);
 	return 1;
 }
 

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .26261-linux-2.5.69/include/net/ipv6.h .26261-linux-2.5.69.updated/include/net/ipv6.h
@@ -145,7 +145,7 @@ extern atomic_t			inet6_sock_nr;
 
 int snmp6_register_dev(struct inet6_dev *idev);
 int snmp6_unregister_dev(struct inet6_dev *idev);
-int snmp6_mib_init(void *ptr[2], size_t mibsize);
+int snmp6_mib_init(void *ptr[2], size_t mibsize, size_t mibalign);
 void snmp6_mib_free(void *ptr[2]);
 
 struct ip6_ra_chain

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .26261-linux-2.5.69/init/main.c .26261-linux-2.5.69.updated/init/main.c
@@ -301,35 +301,10 @@ static void __init smp_init(void)
 #define smp_init()	do { } while (0)
 #endif
 
-static inline void setup_per_cpu_areas(void) { }
 static inline void smp_prepare_cpus(unsigned int maxcpus) { }
 
 #else
 
-#ifdef __GENERIC_PER_CPU
-unsigned long __per_cpu_offset[NR_CPUS];
-
-static void __init setup_per_cpu_areas(void)
-{
-	unsigned long size, i;
-	char *ptr;
-	/* Created by linker magic */
-	extern char __per_cpu_start[], __per_cpu_end[];
-
-	/* Copy section for each CPU (we discard the original) */
-	size = ALIGN(__per_cpu_end - __per_cpu_start, SMP_CACHE_BYTES);
-	if (!size)
-		return;
-
-	ptr = alloc_bootmem(size * NR_CPUS);
-
-	for (i = 0; i < NR_CPUS; i++, ptr += size) {
-		__per_cpu_offset[i] = ptr - __per_cpu_start;
-		memcpy(ptr, __per_cpu_start, size);
-	}
-}
-#endif /* !__GENERIC_PER_CPU */
-
 /* Called by boot processor to activate the rest. */
 static void __init smp_init(void)
 {

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .26261-linux-2.5.69/kernel/ksyms.c .26261-linux-2.5.69.updated/kernel/ksyms.c
@@ -99,7 +99,7 @@ EXPORT_SYMBOL(remove_shrinker);
 EXPORT_SYMBOL(kmalloc);
 EXPORT_SYMBOL(kfree);
 #ifdef CONFIG_SMP
-EXPORT_SYMBOL(kmalloc_percpu);
+EXPORT_SYMBOL(__alloc_percpu);
 EXPORT_SYMBOL(kfree_percpu);
 EXPORT_SYMBOL(percpu_counter_mod);
 #endif

@@ -607,7 +607,7 @@ EXPORT_SYMBOL(init_thread_union);
 EXPORT_SYMBOL(tasklist_lock);
 EXPORT_SYMBOL(find_task_by_pid);
 EXPORT_SYMBOL(next_thread);
-#if defined(CONFIG_SMP) && defined(__GENERIC_PER_CPU)
+#if defined(CONFIG_SMP)
 EXPORT_SYMBOL(__per_cpu_offset);
 #endif
 

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .26261-linux-2.5.69/mm/Makefile .26261-linux-2.5.69.updated/mm/Makefile
@@ -12,3 +12,4 @@ obj-y			:= bootmem.o filemap.o mempool.o
 			   slab.o swap.o truncate.o vcache.o vmscan.o $(mmu-y)
 
 obj-$(CONFIG_SWAP)	+= page_io.o swap_state.o swapfile.o
+obj-$(CONFIG_SMP)	+= percpu.o

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .26261-linux-2.5.69/net/ipv4/route.c .26261-linux-2.5.69.updated/net/ipv4/route.c
@@ -2689,17 +2689,9 @@ int __init ip_rt_init(void)
 	ipv4_dst_ops.gc_thresh = (rt_hash_mask + 1);
 	ip_rt_max_size = (rt_hash_mask + 1) * 16;
 
-	rt_cache_stat = kmalloc_percpu(sizeof (struct rt_cache_stat),
-					GFP_KERNEL);
+	rt_cache_stat = kmalloc_percpu(struct rt_cache_stat);
 	if (!rt_cache_stat) 
 		goto out_enomem1;
-	for (i = 0; i < NR_CPUS; i++) {
-		if (cpu_possible(i)) {
-			memset(per_cpu_ptr(rt_cache_stat, i), 0,
-			       sizeof (struct rt_cache_stat));
-		}
-	}
-
 	devinet_init();
 	ip_fib_init();
 

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .26261-linux-2.5.69/net/ipv6/af_inet6.c .26261-linux-2.5.69.updated/net/ipv6/af_inet6.c
@@ -633,28 +633,20 @@ inet6_unregister_protosw(struct inet_pro
 }
 
 int
-snmp6_mib_init(void *ptr[2], size_t mibsize)
+snmp6_mib_init(void *ptr[2], size_t mibsize, size_t mibalign)
 {
 	int i;
 
 	if (ptr == NULL)
 		return -EINVAL;
 
-	ptr[0] = kmalloc_percpu(mibsize, GFP_KERNEL);
+	ptr[0] = __alloc_percpu(mibsize, mibalign);
 	if (!ptr[0])
 		goto err0;
 
-	ptr[1] = kmalloc_percpu(mibsize, GFP_KERNEL);
+	ptr[1] = __alloc_percpu(mibsize, mibalign);
 	if (!ptr[1])
 		goto err1;
-
-	/* Zero percpu version of the mibs */
-	for (i = 0; i < NR_CPUS; i++) {
-		if (cpu_possible(i)) {
-			memset(per_cpu_ptr(ptr[0], i), 0, mibsize);
-			memset(per_cpu_ptr(ptr[1], i), 0, mibsize);
-		}
-	}
 	return 0;
 
 err1:

@@ -676,11 +668,14 @@ snmp6_mib_free(void *ptr[2])
 
 static int __init init_ipv6_mibs(void)
 {
-	if (snmp6_mib_init((void **)ipv6_statistics, sizeof (struct ipv6_mib)) < 0)
+	if (snmp6_mib_init((void **)ipv6_statistics, sizeof (struct ipv6_mib),
+			   __alignof__(struct ipv6_mib)) < 0)
 		goto err_ip_mib;
-	if (snmp6_mib_init((void **)icmpv6_statistics, sizeof (struct icmpv6_mib)) < 0)
+	if (snmp6_mib_init((void **)icmpv6_statistics, sizeof (struct icmpv6_mib),
+			   __alignof__(struct icmpv6_mib)) < 0)
 		goto err_icmp_mib;
-	if (snmp6_mib_init((void **)udp_stats_in6, sizeof (struct udp_mib)) < 0)
+	if (snmp6_mib_init((void **)udp_stats_in6, sizeof (struct udp_mib),
+			   __alignof__(struct udp_mib)) < 0)
 		goto err_udp_mib;
 	return 0;
 

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .26261-linux-2.5.69/net/ipv6/proc.c .26261-linux-2.5.69.updated/net/ipv6/proc.c
@@ -226,7 +226,7 @@ int snmp6_register_dev(struct inet6_dev 
 	if (!idev || !idev->dev)
 		return -EINVAL;
 
-	if (snmp6_mib_init((void **)idev->stats.icmpv6, sizeof(struct icmpv6_mib)) < 0)
+	if (snmp6_mib_init((void **)idev->stats.icmpv6, sizeof(struct icmpv6_mib), __alignof__(struct icmpv6_mib)) < 0)
 		goto err_icmp;
 
 #ifdef CONFIG_PROC_FS

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .26261-linux-2.5.69/net/sctp/protocol.c .26261-linux-2.5.69.updated/net/sctp/protocol.c
@@ -855,26 +855,14 @@ static int __init init_sctp_mibs(void)
 {
 	int i;
 
-	sctp_statistics[0] = kmalloc_percpu(sizeof (struct sctp_mib),
-					    GFP_KERNEL);
+	sctp_statistics[0] = kmalloc_percpu(struct sctp_mib);
 	if (!sctp_statistics[0])
 		return -ENOMEM;
-	sctp_statistics[1] = kmalloc_percpu(sizeof (struct sctp_mib),
-					    GFP_KERNEL);
+	sctp_statistics[1] = kmalloc_percpu(struct sctp_mib);
 	if (!sctp_statistics[1]) {
 		kfree_percpu(sctp_statistics[0]);
 		return -ENOMEM;
 	}
-
-	/* Zero all percpu versions of the mibs */
-	for (i = 0; i < NR_CPUS; i++) {
-		if (cpu_possible(i)) {
-			memset(per_cpu_ptr(sctp_statistics[0], i), 0,
-					sizeof (struct sctp_mib));
-			memset(per_cpu_ptr(sctp_statistics[1], i), 0,
-					sizeof (struct sctp_mib));
-		}
-	}
 	return 0;
 
 }

I don't beleive that kmalloc_percpu() itself needs to be fast, as you say.

The code is _not_ NUMA-aware. Is it?

In message <[email protected]> you write:

No, it's arch-overridable. You know me, I don't mess with arch-specific stuff. 8)

Currently __setup is run far too late. Fortunately Bartlomiej has been working on moving __setup earlier, for other reasons (two-stage parameter parsing). In this case, it'd need to be before arch setup, hmm...

I figured that since the allocator is going to be there anyway, it made sense to express kmalloc_percpu() in those terms. If you think the price is too high, I can respect that.

The more people use the __per_cpu_offset instead of smp_processor_id(), the cheaper it gets, even to the stage where some archs might want to express smp_processor_id() in terms of __per_cpu_offset, rather than vice-versa. Given that it's also more space efficient (each var doesn't take one cacheline) too, I'd recommend __alloc_percpu() of eg. single ints for long-lived objects where I wouldn't recommend the current kmalloc_percpu().

Paul Mackerras points out that we could get the numa-aware allocation plus "one big alloc" properties by playing with page mappings: reserve 1MB of virtual address, and map more pages as required. I didn't think that we'd need that yet, though.

So, numerous options, and you're smarter than me, so you can decide 8) Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

This is somewhat painful to do (though possible) on i386. The cost of task migration would increase at the very least.

-- wli

Explain? We're not talking about having the same address mapped differently on different CPUs, we're just talking about something which is the equivalent of a sparsely-instantiated vmalloc.

Paul.

Same address mapped differently on different cpus is what I thought you meant. It does make sense, and besides, it only really matters when the thing is being switched in, so I think it's not such a big deal. e.g. mark per-thread mm context with the cpu it was prepped for, if they don't match at load-time then reset the kernel pmd's pgd entry in the per-thread pgd at the top level. x86 blows away the TLB at the drop of a hat anyway, and it wouldn't even introduce an extra whole-TLB flush. That said, I'm not terribly interested in hacking all that up even though the proper hooks to make it pure arch code appear to exist.

The vmallocspace bit is easier, though the virtualspace reservation could get uncomfortably large depending on how much is crammed in there. That can go node-local also. I guess it has some runtime arithmetic overhead vs. the per-cpu TLB entries in exchange for less complex code.

-- wli

Having to have a pgdir per thread would be a bit sucky, wouldn't it?

On PPCs with the hash-table based MMU, if we wanted to do different mappings of the same address on different CPUs, we would have to have a separate hash table for each CPU, which would chew up a lot of memory. On PPC64 machines with logical partitioning, I don't think the hypervisor would let you have a separate hash table for each CPU. On the flip side, PPC can afford a register to point to a per-cpu data area more easily than x86 can.

I was thinking of something like 64kB per cpu times 32 cpus = 2MB. Anyway, 32-bit machines with > 8 cpus are a pretty rare corner case. On 64-bit machines we have enough virtual space to give each cpu gigabytes of per-cpu data if we want to.

Paul.

Not as bad as it initially sounds; on non-PAE i386, it's 4KB and would hurt. On PAE i386, it's 32B and can be shoehorned, say, in thread_info. Then the rest is just a per-cpu kernel pmd and properly handling vmalloc faults (which are already handled properly for non-PAE vmallocspace). There might be other reasons to do it, like reducing the virtualspace overhead of the atomic kmap area, but it's not really time yet.

Well, presumably it'd have to be abstracted so the mechanism isn't exposed to core code if ever done. The arch code insulation appears to be there to keep one going, though not necessarily accessors. Probably the only reason to seriously think about it is that the arithmetic shows up as a disincentive on the register-starved FPOS's I'm stuck on.

2MB vmallocspace is doable; it'd need to be bigger or per-something besides cpus to hurt.

-- wli

Even if the space isn't a problem, having a full TLB flush on thread context switch sounds sucky. Per-node is sufficient for most things, and is much cheaper (update on cross-node migrate).

M.

At some point in the past, my attribution was stripped from:

The scheme described requires no TLB flushing or pgd switching when switching between threads in the same mm. Providing a true per-thread mapping, like one for the stack, might require an invlpg or two at each switch, but that wasn't mentioned here.

-- wli

In message <[email protected]> you write:

You misunderstand I think. All cpus would have the same mappings.

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

I suggest that we use your new mechanism to fix DEFINE_PERCPU() in modules, and leave kmalloc_percpu() as it is for now.

In message <[email protected]> you write:

Sure, we can revisit it later if it makes sense. After all, there's supposed to be this freeze thing...

I'll prepare a new, minimal patch.

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

In message <[email protected]> you write:

No, the allocator is dog-slow. If you want to use it on open, you need a new allocator, not one that does a linear search for blocks.

Well, 4000 will fit on 32-bit archs, easily. Yes, it's a limit.

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

In message <[email protected]> you write:

Absolutely. However, we're looking at an allocation at module insert, and maybe at each mount (if used for your fuzzy counters). Until we see this in practice, I don't think complicating the allocator is worth it.

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

Andrew,
Here is a comparision of kmalloc_percpu techniques as I see it,

Current Implementation:
1. Two dereferences to get to the per-cpu data
2. Allocates for cpu_possible cpus only, and can deal with sparse cpu nos

Rusty's Implementation
1. One extra memory reference (__per_cpu_offset)
2. allocates for NR_CPUS and probably breaks with sparse cpu nos?
3. Let you do per-cpu data in modules
4. fragmentation

The simpler patch I mailed you sometime back,
1. Minimal dereference overhead, offsets to per-cpu data calculated at compile time
2. allocates for NR_CPUS and problems with sparse cpu nos
3. Very Simple.

My guess is performancewise Rusty's iplementation and the simpler implementation of kmalloc_percpu will be comparable. (I'll run some tests to compare them and post them later). I am including the simpler kmalloc_percpu patch which I'd mailed to you earlier.

Thanks,
Kiran

diff -ruN -X dontdiff linux-2.5.65/include/linux/percpu.h kmalloc-new-2.5.65/include/linux/percpu.h
@@ -9,22 +9,14 @@
 #define put_cpu_var(var) preempt_enable()
 
 #ifdef CONFIG_SMP
-
-struct percpu_data {
-	void *ptrs[NR_CPUS];
-	void *blkp;
-};
-
 /* 
  * Use this to get to a cpu's version of the per-cpu object allocated using
  * kmalloc_percpu.  If you want to get "this cpu's version", maybe you want
  * to use get_cpu_ptr... 
  */ 
 #define per_cpu_ptr(ptr, cpu)                   \
-({                                              \
-        struct percpu_data *__p = (struct percpu_data *)~(unsigned long)(ptr); \
-        (__typeof__(ptr))__p->ptrs[(cpu)];	\
-})
+        ((__typeof__(ptr)) 			\
+		(RELOC_HIDE(ptr, ALIGN(sizeof (*ptr), SMP_CACHE_BYTES)*cpu))) 
 
 extern void *kmalloc_percpu(size_t size, int flags);
 extern void kfree_percpu(const void *);

diff -ruN -X dontdiff linux-2.5.65/mm/slab.c kmalloc-new-2.5.65/mm/slab.c
@@ -1951,31 +1951,7 @@
 void *
 kmalloc_percpu(size_t size, int flags)
 {
-	int i;
-	struct percpu_data *pdata = kmalloc(sizeof (*pdata), flags);
-
-	if (!pdata)
-		return NULL;
-
-	for (i = 0; i < NR_CPUS; i++) {
-		if (!cpu_possible(i))
-			continue;
-		pdata->ptrs[i] = kmalloc(size, flags);
-		if (!pdata->ptrs[i])
-			goto unwind_oom;
-	}
-
-	/* Catch derefs w/o wrappers */
-	return (void *) (~(unsigned long) pdata);
-
-unwind_oom:
-	while (--i >= 0) {
-		if (!cpu_possible(i))
-			continue;
-		kfree(pdata->ptrs[i]);
-	}
-	kfree(pdata);
-	return NULL;
+	return kmalloc(ALIGN(size, SMP_CACHE_BYTES)*NR_CPUS, flags);
 }
 #endif
 

@@ -2028,14 +2004,7 @@
 void
 kfree_percpu(const void *objp)
 {
-	int i;
-	struct percpu_data *p = (struct percpu_data *) (~(unsigned long) objp);
-
-	for (i = 0; i < NR_CPUS; i++) {
-		if (!cpu_possible(i))
-			continue;
-		kfree(p->ptrs[i]);
-	}
+	kfree(objp);
 }
 #endif
 

In message <[email protected]> you write:

And #3 is, in fact, the one I care about. The extra memory reference is already probably cachehot (all static per-cpu use it), and might be in a register on some archs.

Doesn't break with sparce CPU #s, but yes, it is inefficient.

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

If you don't reduce NR_CPUS with CONFIG_NR_CPUS, you waste space (32 bit folks won't like it) and if you say change CONFIG_NR_CPUS to 2,
and we have cpuid 4 on a 2 way you break right? If we have to address these issues at all, why can't we use the simpler kmalloc_percpu patch which I posted in the morning and avoid so much complexity and arch dependency?

Thanks,
Kiran

We can have something like that for !CONFIG_NUMA and a NUMA-aware allocator with additional dereferencing cost for CONFIG_NUMA. Hopefully gains from numa-awareness will more than offset dereferencing costs.

Thanks
Dipankar

In message <[email protected]> you write:

You misunderstand, I think: on *all* archs, smp_processor_id() < NR_CPUS is an axiom. Always has been.

The generic solution involved cpu_possible(), but it's too early in boot for that: attempts to make it a requirement have to change numerous archs. Not that I'm against that change...

Of course, an arch can override this default allocator, and may have more info with which to do optimal allocation.

Because we still need the __alloc_percpu for DECLARE_PER_CPU support in modules 8(

Hope that clarifies,
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Maybe we should treat this stuff like architecture ABIs that have a "small data" section?

Before elaborating, let me state that if we are going to use this for things like disk stats and per-interface ipv6 ICMP stats, a fixed size pool is absolutely unacceptable. People are configuring up thousands upon thousands of VLAN net devices today, and if IPV6 is enabled each of those will get a per-cpu ICMP stats block allocated. And as Andrew said, there can be thousands of block devices.

Therefore, for these per-cpu applications there must be no limits built into the mechanism.

Now, what I propose is that kmalloc_percpu() keeps it's current implementation. Then we have a kmalloc_percpu_small() that must only be invoked during module init and we limit the size to some reasonable amount. This kmalloc_percpu_small() uses the 32K pool or whatever, as does DECLARE_PERCPU in a module.

How about this?

David S. Miller <[email protected]>

I tried to run a test to compare this implementation, but got an oops. Here is the oops and the patch I was trying...

btw, why the change from kmalloc_percpu(size) to kmalloc_percpu(type)? You do kmalloc(sizeof (long)) for the usual kmalloc, but kmalloc_percpu(long) for percpu data...looks strange no?

Thanks,
Kiran

diff -ruN -X dontdiff linux-2.5.69/mm/swap.c rusty-1-2.5.69/mm/swap.c
@@ -356,14 +356,14 @@
  */
 #define ACCT_THRESHOLD	max(16, NR_CPUS * 2)
 
-static DEFINE_PER_CPU(long, committed_space) = 0;
+static long *committed_space;
 
 void vm_acct_memory(long pages)
 {
 	long *local;
 
 	preempt_disable();
-	local = &__get_cpu_var(committed_space);
+	local = per_cpu_ptr(committed_space, smp_processor_id());
 	*local += pages;
 	if (*local > ACCT_THRESHOLD || *local < -ACCT_THRESHOLD) {
 		atomic_add(*local, &vm_committed_space);

@@ -371,6 +371,12 @@
 	}
 	preempt_enable();
 }
+
+static int init_committed_space(void)
+{
+	committed_space = kmalloc_percpu(long);
+	return committed_space;
+}
 #endif
 
 

@@ -390,4 +396,6 @@
 	 * Right now other parts of the system means that we
 	 * _really_ don't want to cluster much more
 	 */
+	if (!init_committed_space)
+		panic("swap_setup\n");
 }

Unable to handle kernel paging request at virtual address 0115d040
 printing eip:
c01392ae
*pde = 1fccf001
*pte = 00000000
Oops: 0000 [#1]
CPU:    0
EIP:    0060:[<c01392ae>]    Not tainted
EFLAGS: 00010202

EIP is at vm_acct_memory+0x1e/0x60
eax: 00000000 ebx: dfcca4e0 ecx: 0115d040 edx: 00000001
esi: 00000001 edi: dff7e000 ebp: dff7fe3c esp: dff7fc7c
ds: 007b es: 007b ss: 0068
Process init (pid: 1, threadinfo=dff7e000 task=dff7c040)

Stack: c013fa8f 00000001 dffe4120 00000000 dfcd8720 dff7fe3c dfcca4e0 dfcd3dc0
       c0155966 00000001 dff7e000 bffe0000 dff7e000 dff7fe3c c02ba305 00000000
       c0171013 dff7fe3c 00000000 00000000 00000000 ffffffff 00000003 00000000
Call Trace:
 [<c013fa8f>] vm_enough_memory+0xf/0xc0
 [<c0155966>] setup_arg_pages+0x96/0x190
 [<c0171013>] load_elf_binary+0x4c3/0x9f0
 [<c01376ea>] cache_grow+0x1fa/0x2d0
 [<c0134ab9>] __alloc_pages+0x89/0x2f0
 [<c01556c8>] copy_strings+0x238/0x250
 [<c0170b50>] load_elf_binary+0x0/0x9f0
 [<c0156975>] search_binary_handler+0xa5/0x1f0
 [<c0155707>] copy_strings_kernel+0x27/0x30
 [<c0156c3f>] do_execve+0x17f/0x200
 [<c015821e>] getname+0x5e/0xa0
 [<c01078f0>] sys_execve+0x30/0x70
 [<c0109087>] syscall_call+0x7/0xb
 [<c01051b1>] init+0x151/0x1d0
 [<c0105060>] init+0x0/0x1d0
 [<c01070f5>] kernel_thread_helper+0x5/0x10

Code: 8b 01 01 c2 89 11 83 fa 40 7f 09 89 d0 83 f8 c0 7d 11 eb 02
 <0>Kernel panic: Attempted to kill init!

In message <[email protected]> you write:

Yes, I'd probably want to change the name, if Andrew had agreed to the concept. But the type is very convenient, because you want to know the alignment (kmalloc doesn't care, it just pads to cachline).

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

Yeah :(..sorry abt that, I'd actually tested an earlier patch of yours (one u sent to Dipankar) and I'd got an oops in __alloc_percpu (Same application but I'd actually called init_commmited_space earlier). So when I got a oops again with the newer patch, I just mailed it.
But still, even after actually calling init_committed_space with the newer patch (The first one which you mailed to Andrew in this thread) I get an oops at __alloc_percpu.

Here's the oops report...looks like the
struct pcpu_block *pcpu has NULL...

Thanks,
Kiran

Unable to handle kernel NULL pointer dereference at virtual address 00000000
 printing eip:
c014a5de
*pde = 00104001
*pte = 00000000
Oops: 0000 [#1]
CPU:    0
EIP:    0060:[<c014a5de>]    Not tainted
EFLAGS: 00010246

EIP is at __alloc_percpu+0x4e/0x190
eax: c02f9cbc ebx: c03a8328 ecx: 00000000 edx: 00000000
esi: 00000001 edi: 00000000 ebp: 00000000 esp: dff7ff94
ds: 007b es: 007b ss: 0068
Process swapper (pid: 1, threadinfo=dff7e000 task=dff7c040)

Stack: c01070f0 00000003 c03cfe20 c03ad080 c03a8328 00000001 00000020 00000000
       c01392f9 00000004 00000004 c036ccda c036cd26 c03a8328 c03607eb dff7e000
       00000000 c01050c2 00000020 c0105060 00000000 00000000 00000000 c01070f5
Call Trace:
 [<c01070f0>] kernel_thread_helper+0x0/0x10
 [<c01392f9>] init_committed_space+0x9/0x11
 [<c036ccda>] swap_setup+0x1a/0x30
 [<c036cd26>] kswapd_init+0x6/0x40
 [<c03607eb>] do_initcalls+0x3b/0x90
 [<c01050c2>] init+0x62/0x1d0
 [<c0105060>] init+0x0/0x1d0
 [<c01070f5>] kernel_thread_helper+0x5/0x10

Code: 66 83 39 00 0f 84 08 01 00 00 c7 44 24 04 fc ff ff ff 31 f6
 <0>Kernel panic: Attempted to kill init!

In message <[email protected]> you write:

Damn initialization order.

Change:

	__initcall(init_alloc_percpu);
to
	core_initcall(init_alloc_percpu);

Anyway, patch has been scrapped, at least for now. Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.