Re: [PATCH 1/1] alpha/perf: Account for time spent handling PMU interrupts

[Magnus Lindholm]

Hi Matt,

On Tue, Nov 10, 2026 at 3:05 AM Matt Turner <mattst88@xxxxxxxxx> wrote:
>
> On Sat, Nov 7, 2026 at 4:19 AM Magnus Lindholm <linmag7@xxxxxxxxx> wrote:
> > Alpha does not report PMU interrupt duration to perf, preventing its
> > adaptive throttling from reducing an excessively costly sampling rate.
>
> Nice find, and thanks for the thorough testing write-up. The logic looks
> right to me: time_init() runs before early initcalls, and since it zeroes
> est_cycle_freq when the HWRPB value is within tolerance, the
> "est_cycle_freq ? : hwrpb->cycle_freq" idiom picks the same frequency it
> did. A few small things below.
>
> It would be worth adding a sentence on the symptom from the cover letter
> (short sampling periods leave the machine unresponsive with RCU
> grace-period kthread starvation reports) so the commit stands on its own.
>

Agree, I'll update the commit message in next version.

> > +#include <linux/math64.h>
>
> On 64-bit, div64_u64() is just a plain division, so this include only
> serves that one call. See below for an alternative that drops it.
>
> > +extern unsigned long est_cycle_freq;
>
> This is already declared in arch/alpha/kernel/proto.h. Please
> #include "proto.h" instead, like the rest of arch/alpha/kernel does.
>

yes, will do.

> > +static unsigned long pmu_cycle_freq __read_mostly;
>
> Since this is only written from an early_initcall, __ro_after_init seems
> a better fit.
>
> > +     u32 start = get_cycles();
> > +     u32 cycles;
> > +
> > +     alpha_perf_event_irq_inner(la_ptr, regs);
> > +     /* Unsigned subtraction handles a wrap of the low 32-bit counter. */
> > +     cycles = (u32)get_cycles() - start;
>
> cycles_t is already a 32-bit unsigned int on Alpha, so the cast isn't
> needed. Declaring these as cycles_t would read a bit more naturally.
>

you' re right.

> > +     if (likely(pmu_cycle_freq))
> > +             perf_sample_event_took(div64_u64((u64)cycles * NSEC_PER_SEC,
> > +                                              pmu_cycle_freq));
>
> Alpha has no integer divide instruction, so this compiles to a __divqu
> millicode call on every PMU interrupt. It's small next to the handler
> cost you measured, but it's easy to avoid: compute a mult/shift pair
> once at init with clocks_calc_mult_shift() and convert with
> mul_u64_u32_shr(cycles, mult, shift) here.
>

True, that makes sense. I'll fix this in next version.

> > +     /* Match the calibrated frequency selected by time_init(). */
> > +     pmu_cycle_freq = est_cycle_freq ? est_cycle_freq : hwrpb->cycle_freq;
>
> Not for this patch, but this is now the second copy of that selection
> (show_cpuinfo() has the other). Having time_init() store the frequency
> it chose in a global would let both just read it. Fine as a follow-up.
>
> With the proto.h and cast nits fixed (the rest at your discretion),
>
> Reviewed-by <mattst88@xxxxxxxxx>
>
> Thanks,
> Matt


Thanks for taking the time to look at this, I'll post a v2

Regards

Magnus