[PATCH 0/2] perf/x86/amd/uncore: Drop redundant counter slot searches

[PATCH 0/2] perf/x86/amd/uncore: Drop redundant counter slot searches

[Usama Arif]

Both amd_uncore_add() and amd_uncore_del() search ctx->events[] for an
event whose counter slot is already recorded in event->hw.idx.

Perf serializes ->add() and ->del() for an event, and the driver never
moves an installed event between slots, so an installed event can only
be at the recorded index. Both searches are redundant.

Neither callback is normally hot. That changes once more events target
a PMU than it has counters. Perf then multiplexes them: every mux tick
deschedules the resident set and schedules the next one, so both callbacks
run for each rotated event on every tick.

Meta's fleet-wide profiles attribute 1.1% of kernel CPU time, excluding
do_idle(), to amd_uncore_add() and amd_uncore_del() combined. On the
host that exposed this, a workload holding 15,782 perf event file
descriptors drove 56,575 calls per second into each callback from mux
rotation, against a 16-counter DF PMU.

Patch 1 removes the scan in amd_uncore_add(). It walks every counter
before the free slot search, which is the common case once multiplexing
has scheduled the event out.

Patch 2 makes amd_uncore_del() free the recorded slot directly instead
of rescanning from slot zero. Deleting all events from a full PMU with
N counters drops from N * (N + 1) / 2 compare-exchanges to N.

No functional change intended.

Usama Arif (2):
  perf/x86/amd/uncore: Remove redundant event slot scan
  perf/x86/amd/uncore: Free counter slot by index

 arch/x86/events/amd/uncore.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

-- 
2.53.0-Meta

[PATCH 1/2] perf/x86/amd/uncore: Remove redundant event slot scan

[Usama Arif]

amd_uncore_add() first checks the slot recorded in event->hw.idx. If
that misses, it scans ctx->events[] for the event before looking for a
free slot.

Perf serializes ->add() and ->del() for an event. Initialization sets
idx to -1. A successful ->add() claims a slot and records its index
before returning, while ->del() clears the slot before resetting idx.
CPU context migration follows the same delete/add sequence.

Thus an installed event can only reside at the recorded index. If the
direct check misses, the event is not present in ctx->events[].

Remove the redundant scan. This avoids walking all counters before each
new slot search after multiplexing has scheduled an event out.

On a host running a production workload in the Meta fleet, amd_uncore_add()
was called 56,575 times per second from mux rotation. Its 16-counter DF PMU
makes each failed search scan 16 pointers across two cache lines.

Cc: Sandipan Das <sandipan.das@amd.com>
Signed-off-by: Usama Arif <usama.arif@linux.dev>
---
 arch/x86/events/amd/uncore.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/arch/x86/events/amd/uncore.c b/arch/x86/events/amd/uncore.c
index 7181973b5b127..53be8efaedc5d 100644
--- a/arch/x86/events/amd/uncore.c
+++ b/arch/x86/events/amd/uncore.c
@@ -206,17 +206,15 @@ static int amd_uncore_add(struct perf_event *event, int flags)
 	struct amd_uncore_ctx *ctx = *per_cpu_ptr(pmu->ctx, event->cpu);
 	struct hw_perf_event *hwc = &event->hw;
 
-	/* are we already assigned? */
+	/*
+	 * Perf serializes ->add() and ->del() for an event. A successful
+	 * ->add() records the claimed slot in hwc->idx before returning, and
+	 * ->del() clears that slot before resetting hwc->idx. Therefore, an
+	 * existing assignment must be at hwc->idx.
+	 */
 	if (hwc->idx != -1 && ctx->events[hwc->idx] == event)
 		goto out;
 
-	for (i = 0; i < pmu->num_counters; i++) {
-		if (ctx->events[i] == event) {
-			hwc->idx = i;
-			goto out;
-		}
-	}
-
 	/* if not, take the first available counter */
 	hwc->idx = -1;
 	for (i = 0; i < pmu->num_counters; i++) {
-- 
2.53.0-Meta

[PATCH 2/2] perf/x86/amd/uncore: Free counter slot by index

[Usama Arif]

amd_uncore_del() scans ctx->events[] from slot zero and attempts a
compare-exchange until it finds the event.

amd_uncore_add() records the slot it claimed in event->hw.idx. Perf
calls ->del() only after a successful ->add(), and the driver never
moves an installed event between slots. The index therefore remains
valid until deletion.

Use the recorded index directly and retain the compare-exchange
ownership check. Warn if it fails, as that means the driver lost track
of the slot.

Deleting all events from a full PMU with N counters now requires N
compare-exchanges instead of N * (N + 1) / 2.

On a host running a production workload in the Meta fleet, amd_uncore_del()
was called 56,575 times per second from mux rotation.

Cc: Sandipan Das <sandipan.das@amd.com>
Signed-off-by: Usama Arif <usama.arif@linux.dev>
---
 arch/x86/events/amd/uncore.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/arch/x86/events/amd/uncore.c b/arch/x86/events/amd/uncore.c
index 53be8efaedc5d..39287640bf1ea 100644
--- a/arch/x86/events/amd/uncore.c
+++ b/arch/x86/events/amd/uncore.c
@@ -246,19 +246,15 @@ static int amd_uncore_add(struct perf_event *event, int flags)
 
 static void amd_uncore_del(struct perf_event *event, int flags)
 {
-	int i;
 	struct amd_uncore_pmu *pmu = event_to_amd_uncore_pmu(event);
 	struct amd_uncore_ctx *ctx = *per_cpu_ptr(pmu->ctx, event->cpu);
 	struct hw_perf_event *hwc = &event->hw;
+	struct perf_event *old = event;
 
 	event->pmu->stop(event, PERF_EF_UPDATE);
 
-	for (i = 0; i < pmu->num_counters; i++) {
-		struct perf_event *tmp = event;
-
-		if (try_cmpxchg(&ctx->events[i], &tmp, NULL))
-			break;
-	}
+	/* ->del() follows a successful ->add(), so hwc->idx owns this slot. */
+	WARN_ON_ONCE(!try_cmpxchg(&ctx->events[hwc->idx], &old, NULL));
 
 	hwc->idx = -1;
 }
-- 
2.53.0-Meta