Re: [PATCH v20 11/11] perf: arm_pmuv3: Add support for the Branch Record Buffer Extension (BRBE)

Rob Herring
2025-02-24T12:46:49Z

On Mon, Feb 24, 2025 at 6:25 AM Leo Yan <leo.yan@arm.com> wrote:
>
> On Tue, Feb 18, 2025 at 02:40:06PM -0600, Rob Herring (Arm) wrote:
> >
> > From: Anshuman Khandual <anshuman.khandual@arm.com>
>
> [...]
>
> > BRBE records are invalidated whenever events are reconfigured, a new
> > task is scheduled in, or after recording is paused (and the records
> > have been recorded for the event). The architecture allows branch
> > records to be invalidated by the PE under implementation defined
> > conditions. It is expected that these conditions are rare.
>
> [...]
>
> > +static void armv8pmu_sched_task(struct perf_event_pmu_context *pmu_ctx, bool sched_in)
> > +{
> > + struct arm_pmu *armpmu = *this_cpu_ptr(&cpu_armpmu);
> > + struct pmu_hw_events *hw_events = this_cpu_ptr(armpmu->hw_events);
> > +
> > + if (!hw_events->branch_users)
> > + return;
> > +
> > + if (sched_in)
> > + brbe_invalidate();
> > +}
>
> Just a minor concern. I don't see any handling for task migration.
> E.g., for a task is migrated from one CPU to another CPU, I expect we
> need to save and restore branch records based on BRBE injection. So
> far, the driver simply invalidates all records.
>
> I think this topic is very likely discussed before. If this is the
> case, please ignore my comment. Except this, the code looks good
> to me.

Not really discussed on the list, but that was present in v18 (though
not functional because .sched_task() hook wasn't actually enabled) and
Mark removed it. His work is here[1].The only comment was:

Note: saving/restoring at context-switch doesn't interact well with
event rotation (e.g. if filters change)

Rob

[1] https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/commit/?h=arm64/brbe&id=642985af34d2d6f54e76995380cf24d512078c56