Hi Stephane,
Here's version 4 of the patch. It contains two changes:
1) The optimization to get rid of the per-cpu ctx_arch variable. To
create this patch, I pulled down your latest git tree, and this time
when I added the __get_cpu_var(pmu_ctx) call, I did not get an error
because of the lack of EXPORT_PER_CPU_SYMBOL. Did you change something?
Anyway, it appears to work correctly without any the addition of the
macro.
2) I changed the callback mode of the hrtimer from
HRTIMER_CB_IRQSAFE_UNLOCKED to HRTIMER_CB_IRQSAFE_PERCPU because we need
to be guaranteed that the callback runs on the same CPU where it it was
started. From my reading of the description in hrtimers.h, the
HRTIMER_CB_IRQSAFE_*UNLOCKED* mode is not needed for perfmon_power6's
use of the timer.
Please let me know if there are issues with this patch, and thanks for
your consideration.
Regards,
- Corey
Corey Ashford
Software Engineer
IBM Linux Technology Center, Linux Toolchain
Beaverton, OR
503-578-3507
cjash...@us.ibm.com
diff --git a/arch/powerpc/perfmon/perfmon_power6.c
b/arch/powerpc/perfmon/perfmon_power6.c
index 7882feb..87fe6bf 100644
--- a/arch/powerpc/perfmon/perfmon_power6.c
+++ b/arch/powerpc/perfmon/perfmon_power6.c
@@ -25,6 +25,8 @@
#include <linux/module.h>
#include <linux/perfmon_kern.h>
+#include <linux/hrtimer.h>
+
MODULE_AUTHOR("Corey Ashford <cjash...@us.ibm.com>");
MODULE_DESCRIPTION("POWER6 PMU description table");
MODULE_LICENSE("GPL");
@@ -71,10 +73,16 @@ static struct pfm_regmap_desc pfm_power6_pmd_desc[] = {
#define PFM_PM_NUM_PMDS ARRAY_SIZE(pfm_power6_pmd_desc)
-u32 pmc5_start_save[NR_CPUS];
-u32 pmc6_start_save[NR_CPUS];
+struct pmc5_6_per_cpu {
+ u32 pmc5_start_save;
+ u32 pmc6_start_save;
+ struct hrtimer timer;
+};
+
+static DEFINE_PER_CPU(struct pmc5_6_per_cpu, pmc5_6_percpu);
+
+#define PMC5_6_PERCPU __get_cpu_var(pmc5_6_percpu)
-static struct timer_list pmc5_6_update[NR_CPUS];
u64 enable_cntrs_cnt;
u64 disable_cntrs_cnt;
u64 call_delta;
@@ -83,10 +91,9 @@ u64 pm1_4_interrupt;
/* need ctx_arch for kernel timer. Can't get it in context of the kernel
* timer.
*/
-struct pfm_arch_context *pmc5_6_ctx_arch[NR_CPUS];
-long int update_time;
+ktime_t update_time;
-static void delta(int cpu_num, struct pfm_arch_context *ctx_arch)
+static void delta(struct pfm_arch_context *ctx_arch)
{
u32 tmp5, tmp6;
@@ -100,22 +107,21 @@ static void delta(int cpu_num, struct pfm_arch_context
*ctx_arch)
* arithmetic for the deltas to come out correct (especially in the
* presence of a 32-bit counter wrap).
*/
- ctx_arch->powergs_pmc5 += (u64)(tmp5 - pmc5_start_save[cpu_num]);
- ctx_arch->powergs_pmc6 += (u64)(tmp6 - pmc6_start_save[cpu_num]);
+ ctx_arch->powergs_pmc5 += (u64)(tmp5 - PMC5_6_PERCPU.pmc5_start_save);
+ ctx_arch->powergs_pmc6 += (u64)(tmp6 - PMC5_6_PERCPU.pmc6_start_save);
- pmc5_start_save[cpu_num] = tmp5;
- pmc6_start_save[cpu_num] = tmp6;
-
- return;
+ PMC5_6_PERCPU.pmc5_start_save = tmp5;
+ PMC5_6_PERCPU.pmc6_start_save = tmp6;
}
-static void pmc5_6_updater(unsigned long cpu_num)
+static enum hrtimer_restart pmc5_6_updater(struct hrtimer *hrtimer)
{
/* update the virtual pmd 5 and pmd 6 counters */
- delta(cpu_num, pmc5_6_ctx_arch[cpu_num]);
- mod_timer(&pmc5_6_update[cpu_num], jiffies + update_time);
+ delta(pfm_ctx_arch(__get_cpu_var(pmu_ctx)));
+ hrtimer_forward_now(&PMC5_6_PERCPU.timer, update_time);
+ return HRTIMER_RESTART;
}
@@ -185,7 +191,6 @@ static void pfm_power6_write_pmd(unsigned int cnum, u64
value)
static u64 pfm_power6_sread(struct pfm_context *ctx, unsigned int cnum)
{
struct pfm_arch_context *ctx_arch = pfm_ctx_arch(ctx);
- int cpu_num = smp_processor_id();
/* On POWER 6 PMC5 and PMC6 are implemented as
* virtual counters. See comment in pfm_power6_pmd_desc
@@ -194,11 +199,11 @@ static u64 pfm_power6_sread(struct pfm_context *ctx,
unsigned int cnum)
switch (pfm_pmu_conf->pmd_desc[cnum].hw_addr) {
case SPRN_PMC5:
- return ctx_arch->powergs_pmc5 + (u64)((u32)mfspr(SPRN_PMC5) -
pmc5_start_save[cpu_num]);
+ return ctx_arch->powergs_pmc5 + (u64)((u32)mfspr(SPRN_PMC5) -
PMC5_6_PERCPU.pmc5_start_save);
break;
case SPRN_PMC6:
- return ctx_arch->powergs_pmc6 + (u64)((u32)mfspr(SPRN_PMC6) -
pmc6_start_save[cpu_num]);
+ return ctx_arch->powergs_pmc6 + (u64)((u32)mfspr(SPRN_PMC6) -
PMC5_6_PERCPU.pmc6_start_save);
break;
case PFM_DELTA_TB:
@@ -222,16 +227,15 @@ void pfm_power6_swrite(struct pfm_context *ctx, unsigned
int cnum,
u64 val)
{
struct pfm_arch_context *ctx_arch = pfm_ctx_arch(ctx);
- int cpu_num = smp_processor_id();
switch (pfm_pmu_conf->pmd_desc[cnum].hw_addr) {
case SPRN_PMC5:
- pmc5_start_save[cpu_num] = mfspr(SPRN_PMC5);
+ PMC5_6_PERCPU.pmc5_start_save = mfspr(SPRN_PMC5);
ctx_arch->powergs_pmc5 = val;
break;
case SPRN_PMC6:
- pmc6_start_save[cpu_num] = mfspr(SPRN_PMC6);
+ PMC5_6_PERCPU.pmc6_start_save = mfspr(SPRN_PMC6);
ctx_arch->powergs_pmc6 = val;
break;
@@ -284,7 +288,6 @@ static void pfm_power6_enable_counters(struct pfm_context
*ctx,
{
unsigned int i, max_pmc;
- int cpu_num = smp_processor_id();
struct pfm_arch_context *ctx_arch;
enable_cntrs_cnt++;
@@ -300,8 +303,8 @@ static void pfm_power6_enable_counters(struct pfm_context
*ctx,
pfm_power6_write_pmc(i - 1, set->pmcs[i - 1]);
/* save current free running HW event count */
- pmc5_start_save[cpu_num] = mfspr(SPRN_PMC5);
- pmc6_start_save[cpu_num] = mfspr(SPRN_PMC6);
+ PMC5_6_PERCPU.pmc5_start_save = mfspr(SPRN_PMC5);
+ PMC5_6_PERCPU.pmc6_start_save = mfspr(SPRN_PMC6);
ctx_arch->delta_purr_start = mfspr(SPRN_PURR);
@@ -314,19 +317,10 @@ static void pfm_power6_enable_counters(struct pfm_context
*ctx,
/* Start kernel timer for this cpu to periodically update
* the virtual counters.
*/
- init_timer(&pmc5_6_update[cpu_num]);
- pmc5_6_update[cpu_num].function = pmc5_6_updater;
- pmc5_6_update[cpu_num].data = (unsigned long) cpu_num;
- pmc5_6_update[cpu_num].expires = jiffies + update_time;
- /* context for this timer, timer will be removed if context
- * is switched because the counters will be stopped first.
- * NEEDS WORK, I think this is all ok, a little concerned about a
- * race between the kernel timer going off right as the counters
- * are being stopped and the context switching. Need to think
- * about this.
- */
- pmc5_6_ctx_arch[cpu_num] = ctx_arch;
- add_timer(&pmc5_6_update[cpu_num]);
+
+ if (! hrtimer_active(&PMC5_6_PERCPU.timer)) {
+ hrtimer_start(&PMC5_6_PERCPU.timer, update_time,
HRTIMER_MODE_REL);
+ }
}
/**
@@ -337,7 +331,6 @@ static void pfm_power6_disable_counters(struct pfm_context
*ctx,
struct pfm_event_set *set)
{
struct pfm_arch_context *ctx_arch;
- int cpu_num = smp_processor_id();
disable_cntrs_cnt++;
@@ -345,14 +338,14 @@ static void pfm_power6_disable_counters(struct
pfm_context *ctx,
mtspr(SPRN_MMCR0, mfspr(SPRN_MMCR0) | MMCR0_FC);
asm volatile ("sync");
- /* delete kernel update timer */
- del_timer_sync(&pmc5_6_update[cpu_num]);
+ /* cancel the pmc5/6 update timer */
+ hrtimer_cancel(&PMC5_6_PERCPU.timer);
/* Update the virtual pmd 5 and 6 counters from the free running
* HW counters
*/
ctx_arch = pfm_ctx_arch(ctx);
- delta(cpu_num, ctx_arch);
+ delta(ctx_arch);
ctx_arch->delta_tb +=
(((u64)mfspr(SPRN_TBRU) << 32) | mfspr(SPRN_TBRL))
@@ -480,6 +473,12 @@ static struct pfm_pmu_config pfm_power6_pmu_conf = {
.owner = THIS_MODULE
};
+static void init_pmc5_6_per_cpu(void *info) {
+ hrtimer_init(&PMC5_6_PERCPU.timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+ PMC5_6_PERCPU.timer.function = pmc5_6_updater;
+ PMC5_6_PERCPU.timer.cb_mode = HRTIMER_CB_IRQSAFE_PERCPU;
+}
+
static int __init pfm_power6_pmu_init_module(void)
{
int ret;
@@ -498,15 +497,22 @@ static int __init pfm_power6_pmu_init_module(void)
*/
# define MAX_EVENT_RATE (ppc_proc_freq * 2)
-
- /*
- * Calculate the time, in jiffies, it takes for event counter 5 or
- * 6 to completely wrap when counting at the max event rate, and
- * then figure on sampling at twice that rate.
- */
- update_time = (((unsigned long)HZ * OVERFLOW_VALUE)
- / ((unsigned long)MAX_EVENT_RATE)) / 2;
-
+# define SAFETY_FACTOR 2
+ {
+ u64 update_time_ns;
+
+ /*
+ * Calculate the time, in seconds + nanoseconds, it takes for
+ * counter 5 or 6 to completely wrap when counting at the max
+ * event rate, and then figure on sampling at half that period.
+ */
+ update_time_ns = ((NSEC_PER_SEC * OVERFLOW_VALUE) /
+ MAX_EVENT_RATE) / SAFETY_FACTOR;
+ update_time = ktime_set(
+ update_time_ns / NSEC_PER_SEC,
+ update_time_ns % NSEC_PER_SEC);
+ }
+ on_each_cpu(init_pmc5_6_per_cpu, NULL, 1);
ret = pfm_pmu_register(&pfm_power6_pmu_conf);
return ret;
}
------------------------------------------------------------------------------
This SF.net email is sponsored by:
SourcForge Community
SourceForge wants to tell your story.
http://p.sf.net/sfu/sf-spreadtheword
_______________________________________________
perfmon2-devel mailing list
perfmon2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/perfmon2-devel
