Optimize cgroup ctx switch and remove cpuctx->unique_pmu

First patch is a resend of ("perf/core: Make cgroup switch visit only cpuctxs with cgroup events") with updated change log.

Second patch cleans up the now unused cpuctx->unique_pmu.

David Carrillo-Cisneros (2):
perf/core: Make cgroup switch visit only cpuctxs with cgroup events perf/core: Remove perf_cpu_context::unique_pmu

 include/linux/perf_event.h |   2 +-
 kernel/events/core.c       | 129 ++++++++++++++++-----------------------------
 2 files changed, 47 insertions(+), 84 deletions(-)

2.11.0.483.g087da7b7c-goog

v3: This is a resend of ("perf/core: only check cpuctx with cgroup
events during cgroup switch") with updated changelog.
v2: Fix build when no CONFIG_CGROUP_PERF

This patch follows from a conversation in CQM/CMT's last series about speeding up the context switch for cgroup events:

https://patchwork.kernel.org/patch/9478617/

This is a low-hanging fruit optimization. It replaces the iteration over the "pmus" list in cgroup switch by an iteration over a new list that contains only cpuctxs with at least one cgroup event.

This is necessary because the number of pmus have increased over the years e.g modern x86 server systems have well above 50 pmus.
The iteration over the full pmu list is unneccessary and can be costly in heavy cache contention scenarios.

Below are some instrumentation measurements with 10, 50 and 90 percentiles of the total cost of context switch before and after this optimization for a simple array read/write microbenchark.

Contention
Level Nr events Before (us) After (us) Median
L2 L3 types (10%, 50%, 90%) (10%, 50%, 90% Speedup
--------------------------------------------------------------------------

Low   Low       1       (1.72, 2.42, 5.85)    (1.35, 1.64, 5.46)     29%
High  Low       1       (2.08, 4.56, 19.8)    (1720, 2.20, 13.7)     51%
High  High      1       (2.86, 10.4, 12.7)    (2.54, 4.32, 12.1)     58%

Low   Low       2       (1.98, 3.20, 6.89)    (1.68, 2.41, 8.89)     24%
High  Low       2       (2.48, 5.28, 22.4)    (2150, 3.69, 14.6)     30%
High  High      2       (3.32, 8.09, 13.9)    (2.80, 5.15, 13.7)     36%

where:
1 event type = cycles
2 event types = cycles,intel_cqm/llc_occupancy/

Contention L2 Low: workset < L2 cache size.

              High:  "     >>  L2   "     " .

Contention L3 Low: workset of task on all sockets < L3 cache size.
High: " " " " " " >> L3 " " .

Median Speedup is (50%ile Before - 50%ile After) / 50%ile Before

Unsurprisingly, the benefits of this optimization decrease with the number of cpuctxs with a cgroup events, yet, is never detrimental.

---
 include/linux/perf_event.h |  1 +
 kernel/events/core.c       | 98 +++++++++++++++++++++-------------------------
 2 files changed, 46 insertions(+), 53 deletions(-)

index 4741ecdb9817..10d223572855 100644
@@ -788,6 +788,7 @@ struct perf_cpu_context {
 	struct pmu			*unique_pmu;
 #ifdef CONFIG_CGROUP_PERF
 	struct perf_cgroup		*cgrp;
+	struct list_head		cgrp_cpuctx_entry;
 #endif
 
 	struct list_head		sched_cb_entry;

2.11.0.483.g087da7b7c-goog

While I haven't done any measurement of the overhead, this looks like a nice rework/cleanup.

Since this is only changing the management of cpu contexts, this shouldn't adversely affect systems with heterogeneous CPUs. I've also given this a spin on such a system, to no ill effect.

I have one (very minor) comment below, but either way:

Thanks for reviewing it. I made the lil' change you proposed and added your Ack.

cpuctx->unique_pmu was originally introduced as a way to identify cpuctxs with shared pmus in order to avoid visiting the same cpuctx more than once in a for_each_pmu loop.

cpuctx->unique_pmu == cpuctx->pmu in non-software task contexts since they have only one pmu per cpuctx. Since perf_pmu_sched_task is only called in hw contexts, this patch replaces cpuctx->unique_pmu by cpuctx->pmu in it.

The change above, together with the previous patch in this series, removed the remaining uses of cpuctx->unique_pmu, so we remove it altogether.

---
 include/linux/perf_event.h |  1 -
 kernel/events/core.c       | 31 +------------------------------
 2 files changed, 1 insertion(+), 31 deletions(-)

index 10d223572855..4f8a1f681aac 100644
@@ -785,7 +785,6 @@ struct perf_cpu_context {
 	ktime_t				hrtimer_interval;
 	unsigned int			hrtimer_active;
 
-	struct pmu			*unique_pmu;
 #ifdef CONFIG_CGROUP_PERF
 	struct perf_cgroup		*cgrp;
 	struct list_head		cgrp_cpuctx_entry;

index ef61a9e7328d..57324d67e128 100644
@@ -2883,7 +2883,7 @@ static void perf_pmu_sched_task(struct task_struct *prev,
 		return;
 
 	list_for_each_entry(cpuctx, this_cpu_ptr(&sched_cb_list), sched_cb_entry) {
-		pmu = cpuctx->unique_pmu; /* software PMUs will not have sched_task */
+		pmu = cpuctx->ctx.pmu; /* software PMUs will not have sched_task */
 
 		if (WARN_ON_ONCE(!pmu->sched_task))
 			continue;

@@ -8572,37 +8572,10 @@ static struct perf_cpu_context __percpu *find_pmu_context(int ctxn)
 	return NULL;
 }
 
-static void update_pmu_context(struct pmu *pmu, struct pmu *old_pmu)
-{
-	int cpu;
-
-	for_each_possible_cpu(cpu) {
-		struct perf_cpu_context *cpuctx;
-
-		cpuctx = per_cpu_ptr(pmu->pmu_cpu_context, cpu);
-
-		if (cpuctx->unique_pmu == old_pmu)
-			cpuctx->unique_pmu = pmu;
-	}
-}
-
 static void free_pmu_context(struct pmu *pmu)
 {
-	struct pmu *i;
-
 	mutex_lock(&pmus_lock);
-	/*
-	 * Like a real lame refcount.
-	 */
-	list_for_each_entry(i, &pmus, entry) {
-		if (i->pmu_cpu_context == pmu->pmu_cpu_context) {
-			update_pmu_context(i, pmu);
-			goto out;
-		}
-	}
-
 	free_percpu(pmu->pmu_cpu_context);
-out:
 	mutex_unlock(&pmus_lock);
 }
 

@@ -8806,8 +8779,6 @@ int perf_pmu_register(struct pmu *pmu, const char *name, int type)
 		cpuctx->ctx.pmu = pmu;
 
 		__perf_mux_hrtimer_init(cpuctx, cpu);
-
-		cpuctx->unique_pmu = pmu;
 	}
 
 got_cpu_context:

2.11.0.483.g087da7b7c-goog

I don't have any HW with a PMU that needs a sched_task callback, so I'm unable to give this a full test, but it looks sane to me. FWIW:

Thanks,
Mark.