Re: [PATCH] perf/arm-cmn: Allow userspace to select the PMU's CPU

[Okanovic, Haris]

Hi Robin,

> > All of the PMU's recurring work therefore lands on one CPU. This is
> > problematic on systems which reserve particular CPUs for latency
> > sensitive work or confine background activity to a chosen set of
> > housekeeping CPUs.

> > There is no way to set it explicitly. perf_event_open() and 'perf stat
> > -C' have no effect because arm_cmn_event_init() overwrites event->cpu;
> > /proc/irq/*/smp_affinity is refused for the DTC interrupts, which are
> > requested with IRQF_NOBALANCING because their affinity has to follow the
> > owning CPU.

> All system PMU drivers have the same concern in this regard - why
> should arm-cmn be special?

As I mentioned earlier, we can improve performance on certain system
topologies by assigning PMU work to housekeeping CPUs.

arm-cmn has no constraints around CPU assignment, so this is possible.
Every register access is MMIO and the DTC interrupts can be affinitised
anywhere, so any online CPU can own it. That's what makes "write any
online CPU" a sound interface here. I've moved it to CPUs in both NUMA
nodes on the two platforms I tested.

You're right that the concern is general, but a single interface isn't
easily shared, because the set of CPUs that a PMU may be driven from
is platform/device-specific:

arm_dsu_pmu, for instance, can only be driven from the CPUs attached to
the DSU, which it already exposes as "associated_cpus" and enforces in
event_init(); hisi_uncore_pmu publishes the same attribute.

Intel uncore is per-die: MSR-accessed boxes must be read from a CPU on
the target die, since rdmsr reads the executing CPU. Accepting an
arbitrary CPU there would silently read a different die's counters.

Do you have an alternate API in mind?

> What prevents 
> perf_event_open racing against this update such that the new event->cpu 
> still ends up with the old value, and thus will no longer be correctly 
> synchronised against other scheduling calls/overflow interrupts/etc.?

> Now yes, I think technically that race might already exist in a tiny 
> window due to the order of hotplug callbacks (yet another reason why I'd 
> like to clean up hotplug handling...), but at worst it's still 
> relatively benign if the old CPU is actually going offline, since it 
> will very soon reach a state where that misconfigured new event just 
> won't work - since trying to schedule or read it depends on 
> cross-calling a CPU that's now gone - but at least it's then not capable 
> of corrupting _other_ events.

You're right, nothing prevents it today. I assumed the cpus_read_lock() in
the store covered this, but perf_event_open() doesn't take it. I'll look into
closing that window in a v2.

--
Haris Okanovic
AWS Graviton