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

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

[Ravikiran G Thirumalai]

Hi,
Here's  a patchset to make prof_counter use percpu area infrastructure.
Right now, prof_counter is a NR_CPUS array which gets modified every
local timer interrupt causing cacheline ping-pong for i386, ppc, x86_64 and
sparc arches.  Other arches have made this var truly per-cpu.

This one's for i386 (voyager included).

Thanks,
Kiran


diff -ruN -X dontdiff linux-2.5.55/arch/i386/kernel/apic.c prof_counter-2.5.54/arch/i386/kernel/apic.c
--- linux-2.5.55/arch/i386/kernel/apic.c	Thu Jan  9 09:34:27 2003
+++ prof_counter-2.5.54/arch/i386/kernel/apic.c	Fri Jan 10 13:02:06 2003
@@ -52,7 +52,7 @@
 
 int prof_multiplier[NR_CPUS] = { 1, };
 int prof_old_multiplier[NR_CPUS] = { 1, };
-int prof_counter[NR_CPUS] = { 1, };
+DEFINE_PER_CPU(int, prof_counter) = 1;
 
 int get_maxlvt(void)
 {
@@ -997,7 +997,7 @@
 
 	x86_do_profile(regs);
 
-	if (--prof_counter[cpu] <= 0) {
+	if (--per_cpu(prof_counter, cpu) <= 0) {
 		/*
 		 * The multiplier may have changed since the last time we got
 		 * to this point as a result of the user writing to
@@ -1006,10 +1006,12 @@
 		 *
 		 * Interrupts are already masked off at this point.
 		 */
-		prof_counter[cpu] = prof_multiplier[cpu];
-		if (prof_counter[cpu] != prof_old_multiplier[cpu]) {
-			__setup_APIC_LVTT(calibration_result/prof_counter[cpu]);
-			prof_old_multiplier[cpu] = prof_counter[cpu];
+		per_cpu(prof_counter, cpu) = prof_multiplier[cpu];
+		if (per_cpu(prof_counter, cpu) != prof_old_multiplier[cpu]) {
+			__setup_APIC_LVTT(
+					calibration_result/
+					per_cpu(prof_counter, cpu));
+			prof_old_multiplier[cpu] = per_cpu(prof_counter, cpu);
 		}
 
 #ifdef CONFIG_SMP
diff -ruN -X dontdiff linux-2.5.55/arch/i386/kernel/smpboot.c prof_counter-2.5.54/arch/i386/kernel/smpboot.c
--- linux-2.5.55/arch/i386/kernel/smpboot.c	Thu Jan  9 09:34:14 2003
+++ prof_counter-2.5.54/arch/i386/kernel/smpboot.c	Fri Jan 10 13:52:02 2003
@@ -937,7 +937,7 @@
 
 extern int prof_multiplier[NR_CPUS];
 extern int prof_old_multiplier[NR_CPUS];
-extern int prof_counter[NR_CPUS];
+DECLARE_PER_CPU(int, prof_counter);
 
 static int boot_cpu_logical_apicid;
 /* Where the IO area was mapped on multiquad, always 0 otherwise */
@@ -955,7 +955,7 @@
 	 */
 
 	for (cpu = 0; cpu < NR_CPUS; cpu++) {
-		prof_counter[cpu] = 1;
+		per_cpu(prof_counter, cpu) = 1;
 		prof_old_multiplier[cpu] = 1;
 		prof_multiplier[cpu] = 1;
 	}
diff -ruN -X dontdiff linux-2.5.55/arch/i386/mach-voyager/voyager_smp.c prof_counter-2.5.54/arch/i386/mach-voyager/voyager_smp.c
--- linux-2.5.55/arch/i386/mach-voyager/voyager_smp.c	Thu Jan  9 09:34:00 2003
+++ prof_counter-2.5.54/arch/i386/mach-voyager/voyager_smp.c	Fri Jan 10 13:53:14 2003
@@ -236,7 +236,7 @@
 /* The per cpu profile stuff - used in smp_local_timer_interrupt */
 static unsigned int prof_multiplier[NR_CPUS] __cacheline_aligned = { 1, };
 static unsigned int prof_old_multiplier[NR_CPUS] __cacheline_aligned = { 1, };
-static unsigned int prof_counter[NR_CPUS] __cacheline_aligned = { 1, };
+static DEFINE_PER_CPU(unsigned int, prof_counter) =  1;
 
 /* the map used to check if a CPU has booted */
 static __u32 cpu_booted_map;
@@ -393,7 +393,7 @@
 
 	/* 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;
 		prof_old_multiplier[i] = 1;
 		prof_multiplier[i] = 1;
 		cpu_irq_affinity[i] = ~0;
@@ -1312,7 +1312,7 @@
 
 	x86_do_profile(regs);
 
-	if (--prof_counter[cpu] <= 0) {
+	if (--per_cpu(prof_counter, cpu) <= 0) {
 		/*
 		 * The multiplier may have changed since the last time we got
 		 * to this point as a result of the user writing to
@@ -1321,10 +1321,10 @@
 		 *
 		 * Interrupts are already masked off at this point.
 		 */
-		prof_counter[cpu] = prof_multiplier[cpu];
-		if (prof_counter[cpu] != prof_old_multiplier[cpu]) {
+		per_cpu(prof_counter,cpu) = prof_multiplier[cpu];
+		if (per_cpu(prof_counter, cpu) != prof_old_multiplier[cpu]) {
 			/* FIXME: need to update the vic timer tick here */
-			prof_old_multiplier[cpu] = prof_counter[cpu];
+			prof_old_multiplier[cpu] = per_cpu(prof_counter, cpu);
 		}
 
 		update_process_times(user_mode(regs));

Re: [patch] Make prof_counter use per-cpu areas patch 2/4 -- ppc arch

[Ravikiran G Thirumalai]

This one's for ppc.


diff -ruN -X dontdiff linux-2.5.55/arch/ppc/kernel/smp.c prof_counter-2.5.55/arch/ppc/kernel/smp.c
--- linux-2.5.55/arch/ppc/kernel/smp.c	Thu Jan  9 09:34:25 2003
+++ prof_counter-2.5.55/arch/ppc/kernel/smp.c	Mon Jan 13 14:20:04 2003
@@ -45,7 +45,7 @@
 atomic_t ipi_recv;
 atomic_t ipi_sent;
 unsigned int prof_multiplier[NR_CPUS] = { [1 ... NR_CPUS-1] = 1 };
-unsigned int prof_counter[NR_CPUS] = { [1 ... NR_CPUS-1] = 1 };
+DEFINE_PER_CPU(unsigned int, prof_counter) = 1;
 unsigned long cache_decay_ticks = HZ/100;
 unsigned long cpu_online_map = 1UL;
 unsigned long cpu_possible_map = 1UL;
@@ -90,9 +90,9 @@
 {
 	int cpu = smp_processor_id();
 
-	if (!--prof_counter[cpu]) {
+	if (!--per_cpu(prof_counter, cpu)) {
 		update_process_times(user_mode(regs));
-		prof_counter[cpu]=prof_multiplier[cpu];
+		per_cpu(prof_counter, cpu)=prof_multiplier[cpu];
 	}
 }
 

Re: [patch] Make prof_counter use per-cpu areas patch 3/4 -- x86_64 arch

[Ravikiran G Thirumalai]

This one's for x86_64


diff -ruN -X dontdiff linux-2.5.55/arch/x86_64/kernel/apic.c prof_counter-2.5.55/arch/x86_64/kernel/apic.c
--- linux-2.5.55/arch/x86_64/kernel/apic.c	Thu Jan  9 09:33:55 2003
+++ prof_counter-2.5.55/arch/x86_64/kernel/apic.c	Mon Jan 13 14:27:46 2003
@@ -39,7 +39,7 @@
 
 int prof_multiplier[NR_CPUS] = { 1, };
 int prof_old_multiplier[NR_CPUS] = { 1, };
-int prof_counter[NR_CPUS] = { 1, };
+DEFINE_PER_CPU(int, prof_counter) =  1;
 
 int get_maxlvt(void)
 {
@@ -901,7 +901,7 @@
 
 	x86_do_profile(regs);
 
-	if (--prof_counter[cpu] <= 0) {
+	if (--per_cpu(prof_counter, cpu) <= 0) {
 		/*
 		 * The multiplier may have changed since the last time we got
 		 * to this point as a result of the user writing to
@@ -910,10 +910,11 @@
 		 *
 		 * Interrupts are already masked off at this point.
 		 */
-		prof_counter[cpu] = prof_multiplier[cpu];
-		if (prof_counter[cpu] != prof_old_multiplier[cpu]) {
-			__setup_APIC_LVTT(calibration_result/prof_counter[cpu]);
-			prof_old_multiplier[cpu] = prof_counter[cpu];
+		per_cpu(prof_counter, cpu) = prof_multiplier[cpu];
+		if (per_cpu(prof_counter, cpu) != prof_old_multiplier[cpu]) {
+			__setup_APIC_LVTT(calibration_result/
+					per_cpu(prof_counter, cpu));
+			prof_old_multiplier[cpu] = per_cpu(prof_counter, cpu);
 		}
 
 #ifdef CONFIG_SMP
diff -ruN -X dontdiff linux-2.5.55/arch/x86_64/kernel/smpboot.c prof_counter-2.5.55/arch/x86_64/kernel/smpboot.c
--- linux-2.5.55/arch/x86_64/kernel/smpboot.c	Thu Jan  9 09:34:28 2003
+++ prof_counter-2.5.55/arch/x86_64/kernel/smpboot.c	Mon Jan 13 14:30:00 2003
@@ -774,7 +774,7 @@
 
 extern int prof_multiplier[NR_CPUS];
 extern int prof_old_multiplier[NR_CPUS];
-extern int prof_counter[NR_CPUS];
+DECLARE_PER_CPU(int, prof_counter);
 
 static void __init smp_boot_cpus(unsigned int max_cpus)
 {
@@ -787,7 +787,7 @@
 
 	for (apicid = 0; apicid < NR_CPUS; apicid++) {
 		x86_apicid_to_cpu[apicid] = -1;
-		prof_counter[apicid] = 1;
+		per_cpu(prof_counter, apicid) = 1;
 		prof_old_multiplier[apicid] = 1;
 		prof_multiplier[apicid] = 1;
 	}

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

[Ravikiran G Thirumalai]

This one's for sparc


diff -ruN -X dontdiff linux-2.5.55/arch/sparc/kernel/smp.c prof_counter-2.5.55/arch/sparc/kernel/smp.c
--- linux-2.5.55/arch/sparc/kernel/smp.c	Thu Jan  9 09:33:58 2003
+++ prof_counter-2.5.55/arch/sparc/kernel/smp.c	Mon Jan 13 14:35:15 2003
@@ -256,7 +256,7 @@
 }
 
 unsigned int prof_multiplier[NR_CPUS];
-unsigned int prof_counter[NR_CPUS];
+DEFINE_PER_CPU(unsigned int, prof_counter);
 extern unsigned int lvl14_resolution;
 
 int setup_profiling_timer(unsigned int multiplier)
diff -ruN -X dontdiff linux-2.5.55/arch/sparc/kernel/sun4d_smp.c prof_counter-2.5.55/arch/sparc/kernel/sun4d_smp.c
--- linux-2.5.55/arch/sparc/kernel/sun4d_smp.c	Thu Jan  9 09:33:55 2003
+++ prof_counter-2.5.55/arch/sparc/kernel/sun4d_smp.c	Mon Jan 13 14:37:21 2003
@@ -431,7 +431,7 @@
 }
 
 extern unsigned int prof_multiplier[NR_CPUS];
-extern unsigned int prof_counter[NR_CPUS];
+DECLARE_PER_CPU(unsigned int, prof_counter);
 
 extern void sparc_do_profile(unsigned long pc, unsigned long o7);
 
@@ -455,14 +455,14 @@
 	if(!user_mode(regs))
 		sparc_do_profile(regs->pc, regs->u_regs[UREG_RETPC]);
 
-	if(!--prof_counter[cpu]) {
+	if(!--per_cpu(prof_counter, cpu)) {
 		int user = user_mode(regs);
 
 		irq_enter();
 		update_process_times(user);
 		irq_exit();
 
-		prof_counter[cpu] = prof_multiplier[cpu];
+		per_cpu(prof_counter, cpu) = prof_multiplier[cpu];
 	}
 }
 
@@ -472,7 +472,7 @@
 {
 	int cpu = hard_smp4d_processor_id();
 
-	prof_counter[cpu] = prof_multiplier[cpu] = 1;
+	per_cpu(prof_counter, cpu) = prof_multiplier[cpu] = 1;
 	load_profile_irq(cpu, lvl14_resolution);
 }
 

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

[Pete Zaitcev]

> Date: 	Mon, 13 Jan 2003 18:08:25 +0530
> From: Ravikiran G Thirumalai <kiran@in.ibm.com>

> This one's for sparc
> -unsigned int prof_counter[NR_CPUS];
> +DEFINE_PER_CPU(unsigned int, prof_counter);

Thanks. I'll apply at next pull, or Dave will.

-- Pete

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

[Andrew Morton]

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);

.

Re: [patch] Make prof_counter use per-cpu areas patch 2/4 -- ppc arch

[Paul Mackerras]

Ravikiran G Thirumalai writes:

> This one's for ppc.

[snip]

> -unsigned int prof_counter[NR_CPUS] = { [1 ... NR_CPUS-1] = 1 };
> +DEFINE_PER_CPU(unsigned int, prof_counter) = 1;

I had already done something similar locally which I'll push to Linus
shortly.

Thanks,
Paul.

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

[David S. Miller]

   From: Pete Zaitcev <zaitcev@redhat.com>
   Date: Mon, 13 Jan 2003 11:49:08 -0500
   
   Thanks. I'll apply at next pull, or Dave will.

Pete, I'll take it from you at your leisure.

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

[Ravikiran G Thirumalai]

On Mon, Jan 13, 2003 at 12:10:47PM -0800, Andrew Morton wrote:
> 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.
> 

Even cpu_possible does not seem to be setup this early.  Seems like 
reinitialisation of prof_counter/prof_multiplier et al is redundant.
Here's a newer patch which removes this initialisation at smp_boot_cpus.
Works fine for me (tested same on a 4 way with difft profiling multipliers..
LOC interrupts seem to fire at the right intervals).

Only x86 and x86_64 arches had this reinits. Here's the corrected
x86 patch. x86_64 patch to follow

Thanks,
Kiran


diff -ruN -X dontdiff linux-2.5.58/arch/i386/kernel/apic.c prof_counter-2.5.58/arch/i386/kernel/apic.c
--- linux-2.5.58/arch/i386/kernel/apic.c	Tue Jan 14 11:29:32 2003
+++ prof_counter-2.5.58/arch/i386/kernel/apic.c	Thu Jan 16 16:16:53 2003
@@ -52,7 +52,7 @@
 
 int prof_multiplier[NR_CPUS] = { 1, };
 int prof_old_multiplier[NR_CPUS] = { 1, };
-int prof_counter[NR_CPUS] = { 1, };
+DEFINE_PER_CPU(int, prof_counter) = 1;
 
 int get_maxlvt(void)
 {
@@ -997,7 +997,7 @@
 
 	x86_do_profile(regs);
 
-	if (--prof_counter[cpu] <= 0) {
+	if (--per_cpu(prof_counter, cpu) <= 0) {
 		/*
 		 * The multiplier may have changed since the last time we got
 		 * to this point as a result of the user writing to
@@ -1006,10 +1006,12 @@
 		 *
 		 * Interrupts are already masked off at this point.
 		 */
-		prof_counter[cpu] = prof_multiplier[cpu];
-		if (prof_counter[cpu] != prof_old_multiplier[cpu]) {
-			__setup_APIC_LVTT(calibration_result/prof_counter[cpu]);
-			prof_old_multiplier[cpu] = prof_counter[cpu];
+		per_cpu(prof_counter, cpu) = prof_multiplier[cpu];
+		if (per_cpu(prof_counter, cpu) != prof_old_multiplier[cpu]) {
+			__setup_APIC_LVTT(
+					calibration_result/
+					per_cpu(prof_counter, cpu));
+			prof_old_multiplier[cpu] = per_cpu(prof_counter, cpu);
 		}
 
 #ifdef CONFIG_SMP
diff -ruN -X dontdiff linux-2.5.58/arch/i386/kernel/smpboot.c prof_counter-2.5.58/arch/i386/kernel/smpboot.c
--- linux-2.5.58/arch/i386/kernel/smpboot.c	Tue Jan 14 11:28:41 2003
+++ prof_counter-2.5.58/arch/i386/kernel/smpboot.c	Thu Jan 16 16:16:53 2003
@@ -935,10 +935,6 @@
  * Cycle through the processors sending APIC IPIs to boot each.
  */
 
-extern int prof_multiplier[NR_CPUS];
-extern int prof_old_multiplier[NR_CPUS];
-extern int prof_counter[NR_CPUS];
-
 static int boot_cpu_logical_apicid;
 /* Where the IO area was mapped on multiquad, always 0 otherwise */
 void *xquad_portio;
@@ -950,17 +946,6 @@
 	int apicid, cpu, bit;
 
 	/*
-	 * Initialize the logical to physical CPU number mapping
-	 * and the per-CPU profiling counter/multiplier
-	 */
-
-	for (cpu = 0; cpu < NR_CPUS; cpu++) {
-		prof_counter[cpu] = 1;
-		prof_old_multiplier[cpu] = 1;
-		prof_multiplier[cpu] = 1;
-	}
-
-	/*
 	 * Setup boot CPU information
 	 */
 	smp_store_cpu_info(0); /* Final full version of the data */
diff -ruN -X dontdiff linux-2.5.58/arch/i386/mach-voyager/voyager_smp.c prof_counter-2.5.58/arch/i386/mach-voyager/voyager_smp.c
--- linux-2.5.58/arch/i386/mach-voyager/voyager_smp.c	Tue Jan 14 11:28:33 2003
+++ prof_counter-2.5.58/arch/i386/mach-voyager/voyager_smp.c	Thu Jan 16 16:20:19 2003
@@ -236,7 +236,7 @@
 /* The per cpu profile stuff - used in smp_local_timer_interrupt */
 static unsigned int prof_multiplier[NR_CPUS] __cacheline_aligned = { 1, };
 static unsigned int prof_old_multiplier[NR_CPUS] __cacheline_aligned = { 1, };
-static unsigned int prof_counter[NR_CPUS] __cacheline_aligned = { 1, };
+static DEFINE_PER_CPU(unsigned int, prof_counter) =  1;
 
 /* the map used to check if a CPU has booted */
 static __u32 cpu_booted_map;
@@ -393,9 +393,6 @@
 
 	/* initialize the CPU structures (moved from smp_boot_cpus) */
 	for(i=0; i<NR_CPUS; i++) {
-		prof_counter[i] = 1;
-		prof_old_multiplier[i] = 1;
-		prof_multiplier[i] = 1;
 		cpu_irq_affinity[i] = ~0;
 	}
 	cpu_online_map = (1<<boot_cpu_id);
@@ -1312,7 +1309,7 @@
 
 	x86_do_profile(regs);
 
-	if (--prof_counter[cpu] <= 0) {
+	if (--per_cpu(prof_counter, cpu) <= 0) {
 		/*
 		 * The multiplier may have changed since the last time we got
 		 * to this point as a result of the user writing to
@@ -1321,10 +1318,10 @@
 		 *
 		 * Interrupts are already masked off at this point.
 		 */
-		prof_counter[cpu] = prof_multiplier[cpu];
-		if (prof_counter[cpu] != prof_old_multiplier[cpu]) {
+		per_cpu(prof_counter,cpu) = prof_multiplier[cpu];
+		if (per_cpu(prof_counter, cpu) != prof_old_multiplier[cpu]) {
 			/* FIXME: need to update the vic timer tick here */
-			prof_old_multiplier[cpu] = prof_counter[cpu];
+			prof_old_multiplier[cpu] = per_cpu(prof_counter, cpu);
 		}
 
 		update_process_times(user_mode(regs));

Re: [patch] Make prof_counter use per-cpu areas patch 3/4 -- x86_64 arch

[Ravikiran G Thirumalai]

On Mon, Jan 13, 2003 at 04:21:10PM +0100, Andi Kleen wrote:
> On Mon, Jan 13, 2003 at 06:06:02PM +0530, Ravikiran G Thirumalai wrote:
> > This one's for x86_64
> 
> Thanks, applied.
> 
> -Andi

Please apply this over the earlier patch.  As Andrew pointed out,
the above patch will cause crashes when per-cpu areas are modified to 
allocate for cpu_possible cpus only.

Reinit of prof_counter/prof_multiplier/prof_old_multiplier seems to be 
redundant (They are already statically inited).  Similar patch worked
for x86 for me.

Thanks,
Kiran


diff -ruN -X dontdiff linux-2.5.58/arch/x86_64/kernel/smpboot.c prof_counter-2.5.58/arch/x86_64/kernel/smpboot.c
--- linux-2.5.58/arch/x86_64/kernel/smpboot.c	Thu Jan 16 17:01:18 2003
+++ prof_counter-2.5.58/arch/x86_64/kernel/smpboot.c	Thu Jan 16 16:59:58 2003
@@ -772,24 +772,16 @@
  * Cycle through the processors sending APIC IPIs to boot each.
  */
 
-extern int prof_multiplier[NR_CPUS];
-extern int prof_old_multiplier[NR_CPUS];
-DECLARE_PER_CPU(int, prof_counter);
-
 static void __init smp_boot_cpus(unsigned int max_cpus)
 {
 	int apicid, cpu;
 
 	/*
 	 * Initialize the logical to physical CPU number mapping
-	 * and the per-CPU profiling counter/multiplier
 	 */
 
 	for (apicid = 0; apicid < NR_CPUS; apicid++) {
 		x86_apicid_to_cpu[apicid] = -1;
-		per_cpu(prof_counter, apicid) = 1;
-		prof_old_multiplier[apicid] = 1;
-		prof_multiplier[apicid] = 1;
 	}
 
 	/*

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

[Andrew Morton]

Ravikiran G Thirumalai <kiran@in.ibm.com> wrote:
>
> Even cpu_possible does not seem to be setup this early.  Seems like 
> reinitialisation of prof_counter/prof_multiplier et al is redundant.
> Here's a newer patch which removes this initialisation at smp_boot_cpus.
> Works fine for me (tested same on a 4 way with difft profiling multipliers..
> LOC interrupts seem to fire at the right intervals).

Things still look a bit fishy to me.

In apic.c:

	int prof_multiplier[NR_CPUS] = { 1, };
	int prof_old_multiplier[NR_CPUS] = { 1, };
	DEFINE_PER_CPU(int, prof_counter) = 1;

This means that all the prof_counter values are set to 1, but the multiplier
arrays have 1 in the zeroeth entry, and zero in the remaining entries.

The zero multipliers remain in place until someone runs
setup_profiling_timer().

One approach would be to initialise all members:

	int prof_multiplier[NR_CPUS] = { [ 0 ... NR_CPUS-1 ] = 1 };

But I think it would be better to put the multipliers into per-cpu memory as
well.  Something like:

struct profiling_info {
	int multiplier;
	int old_multiplier;
	int counter;
};

DEFINE_PER_CPU(struct profiling_info, profiling_info) = {1, 1, 1};

Perhaps?

Also bits and pieces of the profiling code seem to be randomly splattered all
over the place.  Consolidating it all into the one .c file would be nice.