\
> diff --git a/arch/powerpc/kernel/perf_event.c
> b/arch/powerpc/kernel/perf_event.c
> index 97e0ae4..0a5178f 100644
> --- a/arch/powerpc/kernel/perf_event.c
> +++ b/arch/powerpc/kernel/perf_event.c
> @@ -416,6 +416,15 @@ static void power_pmu_read(struct perf_event *event)
> prev = local64_read(&event->hw.prev_count);
> barrier();
> val = read_pmc(event->hw.idx);
> + /*
> + * POWER7 can roll back counter values, if the new value is
> + * smaller than the previous value it will cause the delta
> + * and the counter to have bogus values. If this is the
> + * case skip updating anything until the counter grows again.
> + * This can lead to a small lack of precision in the counters.
> + */
> + if (val < prev)
> + return;
> } while (local64_cmpxchg(&event->hw.prev_count, prev, val) != prev);
So if the counters can rollover, the above is still wrong... prev could
be ffffffff and val could be 1 ... for example. In this case you really
need to substract and get the sign of the result I'd think...
Cheers,
Ben.
> /* The counters are only 32 bits wide */
> @@ -439,7 +448,8 @@ static void freeze_limited_counters(struct cpu_hw_events
> *cpuhw,
> unsigned long pmc5, unsigned long pmc6)
> {
> struct perf_event *event;
> - u64 val, prev, delta;
> + u64 val, prev;
> + s32 delta;
> int i;
>
> for (i = 0; i < cpuhw->n_limited; ++i) {
> @@ -449,8 +459,13 @@ static void freeze_limited_counters(struct cpu_hw_events
> *cpuhw,
> val = (event->hw.idx == 5) ? pmc5 : pmc6;
> prev = local64_read(&event->hw.prev_count);
> event->hw.idx = 0;
> - delta = (val - prev) & 0xfffffffful;
> - local64_add(delta, &event->count);
> + /*
> + * The PerfMon registers are only 32 bits wide so the
> + * delta should not overflow.
> + */
> + delta = val - prev;
> + if (delta > 0)
> + local64_add(delta, &event->count);
> }
> }
>
> @@ -458,14 +473,16 @@ static void thaw_limited_counters(struct cpu_hw_events
> *cpuhw,
> unsigned long pmc5, unsigned long pmc6)
> {
> struct perf_event *event;
> - u64 val;
> + u64 val, prev;
> int i;
>
> for (i = 0; i < cpuhw->n_limited; ++i) {
> event = cpuhw->limited_counter[i];
> event->hw.idx = cpuhw->limited_hwidx[i];
> val = (event->hw.idx == 5) ? pmc5 : pmc6;
> - local64_set(&event->hw.prev_count, val);
> + prev = local64_read(&event->hw.prev_count);
> + if (val > prev)
> + local64_set(&event->hw.prev_count, val);
> perf_event_update_userpage(event);
> }
> }
> @@ -1187,7 +1204,8 @@ static void record_and_restart(struct perf_event
> *event, unsigned long val,
> struct pt_regs *regs, int nmi)
> {
> u64 period = event->hw.sample_period;
> - s64 prev, delta, left;
> + s64 prev, left;
> + s32 delta;
> int record = 0;
>
> if (event->hw.state & PERF_HES_STOPPED) {
> @@ -1197,7 +1215,9 @@ static void record_and_restart(struct perf_event
> *event, unsigned long val,
>
> /* we don't have to worry about interrupts here */
> prev = local64_read(&event->hw.prev_count);
> - delta = (val - prev) & 0xfffffffful;
> + delta = val - prev;
> + if (delta < 0)
> + delta = 0;
> local64_add(delta, &event->count);
>
> /*
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
