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));
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];
}
}
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);
}
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;
}
> Date: Mon, 13 Jan 2003 18:08:25 +0530
> From: Ravikiran G Thirumalai <[email protected]>
> 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
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);
.
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.
From: Pete Zaitcev <[email protected]>
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.
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));
Ravikiran G Thirumalai <[email protected]> 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.