Re: [tip:perf/core] perf: Rework the PMU methods

From: Peter Zijlstra
Date: Mon Sep 13 2010 - 08:16:22 EST


On Sun, 2010-09-12 at 17:33 +1200, Michael Cree wrote:

> Yes, done. I also took the liberty to fix an undefined variable and
> multiple defined variable errors that were exposed by compilation. Will
> reply to this with the patch.

Thanks, and sorry for messing up Alpha that bad.. I have an alpha
compiler and I really through I compile tested it :/

> I've also tested it on a UP alpha. It worked well for a little while
> but after running 'perf top' for a number of seconds I got the following
> warning:

<snip warn>

> which is from the line in alpha_pmu_start() that checks that
> PERF_HES_STOPPED is set.
>
> I see that the backtrace is from the Alpha timer_interrupt() code which
> goes something like this:
>
> [do some stuff updating timer deltas then...]
>
> #ifndef CONFIG_SMP
> while (nticks--)
> update_process_times(user_mode(get_irq_regs()));
> #endif
>
> if (test_perf_event_pending()) {
> clear_perf_event_pending();
> perf_event_do_pending();
> }
>
> return IRQ_HANDLED;
> }
>
>
> When I added the code for handle pending events to the timer interrupt I
> hadn't realised that update_process_times() could call back into the
> perf code. I'm speculating here, but could it be that there is pending
> work to stop the HW counter, but the call to re-start it is beating the
> call to stop it?

Right, so the ->start() call came from perf_ctx_adjust_freq(), which
depending on whether perf_adjust_period() gets inlined, can have two
such calls.

Assuming it didn't inline (there's two callsites, which should defeat
the inline static functions with a single callsite heuristic), you hit
the unthrottle() call.

Ahh, the alpha throttle call should be using the fancy new stop function
too (will fold into your earlier patch if it indeed works):

As to the point you raised above, yes, I think it would be prudent to
call perf_event_do_pending() before update_process_times().


Signed-off-by: Peter Zijlstra <a.p.zijlstra@xxxxxxxxx>
---
Index: linux-2.6/arch/alpha/kernel/perf_event.c
===================================================================
--- linux-2.6.orig/arch/alpha/kernel/perf_event.c
+++ linux-2.6/arch/alpha/kernel/perf_event.c
@@ -825,14 +825,14 @@ static void alpha_perf_event_irq_handler
break;
}

+ event = cpuc->event[j];
+
if (unlikely(j == cpuc->n_events)) {
/* This can occur if the event is disabled right on a PMC overflow. */
- wrperfmon(PERFMON_CMD_ENABLE, cpuc->idx_mask);
+ alpha_pmu_stop(event, 0);
return;
}

- event = cpuc->event[j];
-
if (unlikely(!event)) {
/* This should never occur! */
irq_err_count++;

--
To unsubscribe from this list: send the line "unsubscribe linux-alpha" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html