On Mon, 2010-09-13 at 14:15 +0200, Peter Zijlstra wrote:
> 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>
Damn I suck.. Please try this one.
---
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
@@ -850,7 +850,7 @@ static void alpha_perf_event_irq_handler
/* Interrupts coming too quickly; "throttle" the
* counter, i.e., disable it for a little while.
*/
- cpuc->idx_mask &= ~(1UL<<idx);
+ alpha_pmu_stop(event, 0);
}
}
wrperfmon(PERFMON_CMD_ENABLE, cpuc->idx_mask);
--
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
