Re: [patch] Make prof_counter use per-cpu areas patch 1/4 -- x86 arch

Andrew Morton
2003-01-13T20:01:46Z

On Monday 13 January 2003 04:28 am, Ravikiran G Thirumalai wrote:
>
> Hi,
> Here's a patchset to make prof_counter use percpu area infrastructure.
> ...
> /* initialize the CPU structures (moved from smp_boot_cpus) */
> for(i=0; i<NR_CPUS; i++) {
> - prof_counter[i] = 1;
> + per_cpu(prof_counter, i) = 1;

Please always use the cpu_online() test here.

Yes, it's a bit awkward and yes, we should have a for_each_online_cpu()
helper, but that didn't happen.

The reason (apart from increased efficiency) is that at some time we may
make the per-cpu data areas only exist on online CPUs. We allocate the
area from the CPU's node-local memory when it is coming online.

Code such as the above will oops with that change in place.

We did have all of this running, but I never submitted the patch for reasons
of general scariness and lack of expressed interest from NUMA people.

<dig, dig>

Here it is:

The current per-cpu memory areas are allocated at startup for NR_CPUS,
so we're really not saving much memory from them.

This is a problem for kernel/timer.c (128 kbytes), kernel/sched.c (78
kbytes) and presumably other places.

The patch from Rutsy allocates per-cpu areas only for those CPUs which
may actually exist, before each one comes online.

So the per-cpu storage for not-possible CPUs does not exist, and
accessing them will oops.

init/main.c | 40 ++++++++++++++++++++++++++++------------
1 files changed, 28 insertions(+), 12 deletions(-)

--- 2.5.43/init/main.c~per-cpu-allocation Fri Oct 18 01:19:56 2002
+++ 2.5.43-akpm/init/main.c Fri Oct 18 01:22:32 2002
@@ -304,32 +304,39 @@ static void __init smp_init(void)
#define smp_init() do { } while (0)
#endif

-static inline void setup_per_cpu_areas(void) { }
+static inline void setup_per_cpu_area(unsigned int cpu) { }
static inline void smp_prepare_cpus(unsigned int maxcpus) { }

#else

#ifdef __GENERIC_PER_CPU
+/* Created by linker magic */
+extern char __per_cpu_start[], __per_cpu_end[];
+
unsigned long __per_cpu_offset[NR_CPUS];

-static void __init setup_per_cpu_areas(void)
+/* Sets up per-cpu area for boot CPU. */
+static void __init setup_per_cpu_area(unsigned int cpu)
{
- unsigned long size, i;
+ unsigned long size;
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);
+ /* First CPU happens really early... */
+ if (cpu == smp_processor_id())
+ ptr = alloc_bootmem(size);
+ else
+ ptr = kmalloc(size, GFP_ATOMIC);

- for (i = 0; i < NR_CPUS; i++, ptr += size) {
- __per_cpu_offset[i] = ptr - __per_cpu_start;
- memcpy(ptr, __per_cpu_start, size);
- }
+ if (ptr == NULL)
+ panic("failed to allocate per-cpu area for cpu %d\n", cpu);
+
+ __per_cpu_offset[cpu] = ptr - __per_cpu_start;
+ memcpy(ptr, __per_cpu_start, size);
}
#endif /* !__GENERIC_PER_CPU */

@@ -338,7 +345,16 @@ static void __init smp_init(void)
{
unsigned int i;

- /* FIXME: This should be done in userspace --RR */
+ for (i = 0; i < NR_CPUS; i++) {
+ if (cpu_possible(i)) {
+ if (i != smp_processor_id())
+ setup_per_cpu_area(i);
+ } else {
+ /* Force a NULL deref on use */
+ __per_cpu_offset[i] = (char *)0 - __per_cpu_start;
+ }
+ }
+
for (i = 0; i < NR_CPUS; i++) {
if (num_online_cpus() >= max_cpus)
break;
@@ -390,7 +406,7 @@ asmlinkage void __init start_kernel(void
lock_kernel();
printk(linux_banner);
setup_arch(&command_line);
- setup_per_cpu_areas();
+ setup_per_cpu_area(smp_processor_id());
build_all_zonelists();
printk("Kernel command line: %s\n", saved_command_line);
parse_options(command_line);

.