Re: [PATCH v6 05/13] alpha/perf: ignore overflow interrupts the counter does not account for

[Magnus Lindholm]

Hi Matt,

On Wed, Nov 4, 2026 at 7:49 PM Matt Turner <mattst88@xxxxxxxxx> wrote:
>
> The interrupt handler adds a full counter period to the count of the
> event on the counter that interrupted, which goes wrong in two cases.
>
> Rewriting a counter does not cancel its pending interrupt: wrperfmon has
> no way to, and PALcode clears the request only when delivering it. An
> interrupt pending when an event is reloaded, or when perf moves another
> event onto the counter, is then taken as an overflow of a counter that
> has not overflowed. It adds a bogus period to the count and can emit a
> false sample.
>
> And alpha_perf_event_update() already adds the period when a read finds
> that the counter overflowed before its interrupt was taken. The
> interrupt then adds it again.
>
> An overflow leaves the counter just past zero, below the value it was
> written with, while a counter that has not overflowed is at or above it.
> Ignore an interrupt for a counter in the latter state, unless a read
> has already counted the overflow, in which case add nothing. For this to
> hold, write the counters at most half their range below overflow, as
> armpmu_event_set_period() does. Counting events then take an interrupt
> every 2^19 events rather than every 2^20.
>
> Fixes: 979f867191f8 ("alpha: implement HW performance events on the EV67 and later CPUs")
> Suggested-by: Ian Rogers <irogers@xxxxxxxxxx>
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Matt Turner <mattst88@xxxxxxxxx>
> ---
>  arch/alpha/kernel/perf_event.c | 59 +++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 52 insertions(+), 7 deletions(-)
>
> diff --git a/arch/alpha/kernel/perf_event.c b/arch/alpha/kernel/perf_event.c
> index ecec78c355cd..dd7fc4ac1396 100644
> --- a/arch/alpha/kernel/perf_event.c
> +++ b/arch/alpha/kernel/perf_event.c
> @@ -30,6 +30,9 @@
>  #define MAX_HWEVENTS 3
>  #define PMC_NO_INDEX -1
>
> +/* hw_perf_event.flags */
> +#define ALPHA_PERF_FLAG_WRAPPED                0x1
> +
>  /* For tracking PMCs and the hw events they monitor on each CPU. */
>  struct cpu_hw_events {
>         int                     enabled;
> @@ -315,10 +318,16 @@ static int alpha_perf_event_set_period(struct perf_event *event,
>         if (unlikely(left < alpha_pmu->pmc_left[idx]))
>                 left = alpha_pmu->pmc_left[idx];
>
> -       if (left > (long)alpha_pmu->pmc_max_period[idx])
> -               left = alpha_pmu->pmc_max_period[idx];
> +       /*
> +        * Start at most half the counter's range below overflow, so that the
> +        * value an overflow leaves, just past zero, is always below the value
> +        * written. See alpha_perf_event_ovf().
> +        */
> +       if (left > (long)(alpha_pmu->pmc_max_period[idx] >> 1))
> +               left = alpha_pmu->pmc_max_period[idx] >> 1;
>
>         local64_set(&hwc->prev_count, (unsigned long)(-left));
> +       hwc->flags &= ~ALPHA_PERF_FLAG_WRAPPED;
>
>         alpha_write_pmc(idx, (unsigned long)(-left));
>
> @@ -338,9 +347,9 @@ static int alpha_perf_event_set_period(struct perf_event *event,
>   * zero.  The value passed via ovf by the interrupt handler corrects for
>   * overflow.
>   *
> - * This can be racey on rare occasions -- a call to this routine can occur
> - * with an overflowed counter just before the PMI service routine is called.
> - * The check for delta negative hopefully always rectifies this situation.
> + * A call to this routine can occur with an overflowed counter just before
> + * the PMI service routine is called. A negative delta detects this, as the
> + * counter is always written at most half its range below overflow.
>   */
>  static unsigned long alpha_perf_event_update(struct perf_event *event,
>                                         struct hw_perf_event *hwc, int idx, long ovf)
> @@ -359,10 +368,12 @@ static unsigned long alpha_perf_event_update(struct perf_event *event,
>         delta = (new_raw_count - (prev_raw_count & alpha_pmu->pmc_count_mask[idx])) + ovf;
>
>         /* It is possible on very rare occasions that the PMC has overflowed
> -        * but the interrupt is yet to come.  Detect and fix this situation.
> +        * but the interrupt is yet to come.  Detect and fix this situation,
> +        * and tell the interrupt not to count the overflow again.
>          */
>         if (unlikely(delta < 0)) {
>                 delta += alpha_pmu->pmc_max_period[idx] + 1;
> +               hwc->flags |= ALPHA_PERF_FLAG_WRAPPED;
>         }
>
>         local64_add(delta, &event->count);
> @@ -835,6 +846,32 @@ void perf_event_print_debug(void)
>  }
>
>
> +/*
> + * Return the overflow that an interrupt from counter idx adds to the count,
> + * or -1 if the counter has not overflowed since it was written.
> + *
> + * Rewriting a counter does not cancel its pending interrupt: wrperfmon has
> + * no way to, and PALcode clears the request only when delivering it. So an
> + * interrupt can arrive after the counter was reloaded or given to another
> + * event. The counter is then at or above the value written, whereas an
> + * overflow leaves it just past zero, below that value.
> + */
> +static long alpha_perf_event_ovf(struct hw_perf_event *hwc, int idx)
> +{
> +       unsigned long prev;
> +
> +       /* alpha_perf_event_update() has already counted this overflow. */
> +       if (hwc->flags & ALPHA_PERF_FLAG_WRAPPED)
> +               return 0;
> +
> +       prev = local64_read(&hwc->prev_count) & alpha_pmu->pmc_count_mask[idx];
> +       if (alpha_read_pmc(idx) >= prev)
> +               return -1;
> +
> +       return alpha_pmu->pmc_max_period[idx] + 1;
> +}
> +
> +
>  /*
>   * Performance Monitoring Interrupt Service Routine called when a PMC
>   * overflows.  The PMC that overflowed is passed in la_ptr.
> @@ -847,6 +884,7 @@ static void alpha_perf_event_irq_handler(unsigned long la_ptr,
>         struct perf_event *event;
>         struct hw_perf_event *hwc;
>         int idx, j;
> +       long ovf;
>
>         __this_cpu_inc(irq_pmi_count);
>         cpuc = this_cpu_ptr(&cpu_hw_events);
> @@ -891,7 +929,14 @@ static void alpha_perf_event_irq_handler(unsigned long la_ptr,
>         }
>
>         hwc = &event->hw;
> -       alpha_perf_event_update(event, hwc, idx, alpha_pmu->pmc_max_period[idx]+1);
> +       ovf = alpha_perf_event_ovf(hwc, idx);
> +       if (ovf < 0) {
> +               /* The interrupt was pending when the counter was written. */
> +               wrperfmon(PERFMON_CMD_ENABLE, cpuc->idx_mask);
> +               return;
> +       }
> +
> +       alpha_perf_event_update(event, hwc, idx, ovf);
>         perf_sample_data_init(&data, 0, hwc->last_period);
>
>         if (alpha_perf_event_set_period(event, hwc, idx))
>
> --
> 2.55.0
>

The wrapped flag addresses my concern about a read accounting for an
overflow before its PMI arrives. I checked that sequence and the
stale-PMI-after-reload case with simulated counter values.

On my ES40, repeated reads while multiplexing Aggregate and ProfileMe
events passed on all three CPUs. I didn't instrument the stale PMI
itself on hardware.

Reviewed-by: Magnus Lindholm <linmag7@xxxxxxxxx>
Tested-by: Magnus Lindholm <linmag7@xxxxxxxxx>