public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Ravi Bangoria <ravi.bangoria@amd.com>
Cc: acme@kernel.org, alexander.shishkin@linux.intel.com,
	jolsa@redhat.com, namhyung@kernel.org, songliubraving@fb.com,
	eranian@google.com, alexey.budankov@linux.intel.com,
	ak@linux.intel.com, mark.rutland@arm.com, megha.dey@intel.com,
	frederic@kernel.org, maddy@linux.ibm.com, irogers@google.com,
	kim.phillips@amd.com, linux-kernel@vger.kernel.org,
	santosh.shukla@amd.com
Subject: Re: [RFC v2] perf: Rewrite core context handling
Date: Wed, 24 Aug 2022 16:59:07 +0200	[thread overview]
Message-ID: <YwY8u7gx6bO+RBcg@worktop.programming.kicks-ass.net> (raw)
In-Reply-To: <YwYWUbVvSAYseDaO@worktop.programming.kicks-ass.net>

On Wed, Aug 24, 2022 at 02:15:13PM +0200, Peter Zijlstra wrote:
> On Mon, Jun 13, 2022 at 04:35:11PM +0200, Peter Zijlstra wrote:
> >  void x86_pmu_update_cpu_context(struct pmu *pmu, int cpu)
> >  {
> > -	struct perf_cpu_context *cpuctx;
> > +	/* XXX: Don't need this quirk anymore */
> > +	/*struct perf_cpu_context *cpuctx;
> >  
> >  	if (!pmu->pmu_cpu_context)
> >  		return;
> >  
> >  	cpuctx = per_cpu_ptr(pmu->pmu_cpu_context, cpu);
> > -	cpuctx->ctx.pmu = pmu;
> > +	cpuctx->ctx.pmu = pmu;*/
> >  }
> 
> Confirmed; my ADL seems to work fine without all that.

Additionally; this doesn't insta crash.

---
diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index cb69ff1e6138..016072a89f8f 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -1019,10 +1019,10 @@ static int armv8pmu_set_event_filter(struct hw_perf_event *event,
 	return 0;
 }
 
-static int armv8pmu_filter_match(struct perf_event *event)
+static bool armv8pmu_filter(struct pmu *pmu, int cpu)
 {
-	unsigned long evtype = event->hw.config_base & ARMV8_PMU_EVTYPE_EVENT;
-	return evtype != ARMV8_PMUV3_PERFCTR_CHAIN;
+	struct arm_pmu *armpmu = to_arm_pmu(pmu);
+	return !cpumask_test_cpu(smp_processor_id(), &armpmu->supported_cpus);
 }
 
 static void armv8pmu_reset(void *info)
@@ -1253,7 +1253,7 @@ static int armv8_pmu_init(struct arm_pmu *cpu_pmu, char *name,
 	cpu_pmu->stop			= armv8pmu_stop;
 	cpu_pmu->reset			= armv8pmu_reset;
 	cpu_pmu->set_event_filter	= armv8pmu_set_event_filter;
-	cpu_pmu->filter_match		= armv8pmu_filter_match;
+	cpu_pmu->filter			= armv8pmu_filter;
 
 	cpu_pmu->pmu.event_idx		= armv8pmu_user_event_idx;
 
diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
index 7a2d12ad6d1f..a8f1e38c66a7 100644
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -86,6 +86,8 @@ DEFINE_STATIC_CALL_NULL(x86_pmu_swap_task_ctx, *x86_pmu.swap_task_ctx);
 DEFINE_STATIC_CALL_NULL(x86_pmu_drain_pebs,   *x86_pmu.drain_pebs);
 DEFINE_STATIC_CALL_NULL(x86_pmu_pebs_aliases, *x86_pmu.pebs_aliases);
 
+DEFINE_STATIC_CALL_NULL(x86_pmu_filter, *x86_pmu.filter);
+
 /*
  * This one is magic, it will get called even when PMU init fails (because
  * there is no PMU), in which case it should simply return NULL.
@@ -2038,6 +2040,7 @@ static void x86_pmu_static_call_update(void)
 	static_call_update(x86_pmu_pebs_aliases, x86_pmu.pebs_aliases);
 
 	static_call_update(x86_pmu_guest_get_msrs, x86_pmu.guest_get_msrs);
+	static_call_update(x86_pmu_filter, x86_pmu.filter);
 }
 
 static void _x86_pmu_read(struct perf_event *event)
@@ -2668,12 +2671,13 @@ static int x86_pmu_aux_output_match(struct perf_event *event)
 	return 0;
 }
 
-static int x86_pmu_filter_match(struct perf_event *event)
+static bool x86_pmu_filter(struct pmu *pmu, int cpu)
 {
-	if (x86_pmu.filter_match)
-		return x86_pmu.filter_match(event);
+	bool ret = false;
 
-	return 1;
+	static_call_cond(x86_pmu_filter)(pmu, cpu, &ret);
+
+	return ret;
 }
 
 static struct pmu pmu = {
@@ -2704,7 +2708,7 @@ static struct pmu pmu = {
 
 	.aux_output_match	= x86_pmu_aux_output_match,
 
-	.filter_match		= x86_pmu_filter_match,
+	.filter			= x86_pmu_filter,
 };
 
 void arch_perf_update_userpage(struct perf_event *event,
diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 768771e5e4e9..40cebd9b90a1 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -4675,12 +4675,11 @@ static int intel_pmu_aux_output_match(struct perf_event *event)
 	return is_intel_pt_event(event);
 }
 
-static int intel_pmu_filter_match(struct perf_event *event)
+static void intel_pmu_filter(struct pmu *pmu, int cpu, bool *ret)
 {
-	struct x86_hybrid_pmu *pmu = hybrid_pmu(event->pmu);
-	unsigned int cpu = smp_processor_id();
+	struct x86_hybrid_pmu *hpmu = hybrid_pmu(pmu);
 
-	return cpumask_test_cpu(cpu, &pmu->supported_cpus);
+	*ret = !cpumask_test_cpu(cpu, &hpmu->supported_cpus);
 }
 
 PMU_FORMAT_ATTR(offcore_rsp, "config1:0-63");
@@ -6348,7 +6347,7 @@ __init int intel_pmu_init(void)
 		x86_pmu.update_topdown_event = adl_update_topdown_event;
 		x86_pmu.set_topdown_event_period = adl_set_topdown_event_period;
 
-		x86_pmu.filter_match = intel_pmu_filter_match;
+		x86_pmu.filter = intel_pmu_filter;
 		x86_pmu.get_event_constraints = adl_get_event_constraints;
 		x86_pmu.hw_config = adl_hw_config;
 		x86_pmu.limit_period = spr_limit_period;
diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
index 9c835ecb232e..b3ff55fc5794 100644
--- a/arch/x86/events/perf_event.h
+++ b/arch/x86/events/perf_event.h
@@ -924,7 +924,7 @@ struct x86_pmu {
 
 	int (*aux_output_match) (struct perf_event *event);
 
-	int (*filter_match)(struct perf_event *event);
+	void (*filter)(struct pmu *pmu, int cpu, bool *ret);
 	/*
 	 * Hybrid support
 	 *
diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
index 0407a38b470a..0f9519874fde 100644
--- a/include/linux/perf/arm_pmu.h
+++ b/include/linux/perf/arm_pmu.h
@@ -99,7 +99,7 @@ struct arm_pmu {
 	void		(*stop)(struct arm_pmu *);
 	void		(*reset)(void *);
 	int		(*map_event)(struct perf_event *event);
-	int		(*filter_match)(struct perf_event *event);
+	bool		(*filter)(struct pmu *pmu, int cpu);
 	int		num_events;
 	bool		secure_access; /* 32-bit ARM only */
 #define ARMV8_PMUV3_MAX_COMMON_EVENTS		0x40
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 7847818e5397..4be3aaae89be 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -519,9 +519,10 @@ struct pmu {
 					/* optional */
 
 	/*
-	 * Filter events for PMU-specific reasons.
+	 * Skip programming this PMU on the given CPU. Typically needed for
+	 * big.LITTLE things.
 	 */
-	int (*filter_match)		(struct perf_event *event); /* optional */
+	bool (*filter)			(struct pmu *pmu, int cpu); /* optional */
 
 	/*
 	 * Check period value for PERF_EVENT_IOC_PERIOD ioctl.
diff --git a/kernel/events/core.c b/kernel/events/core.c
index c6b64a48dea6..180842ba8473 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2181,38 +2181,11 @@ static bool is_orphaned_event(struct perf_event *event)
 	return event->state == PERF_EVENT_STATE_DEAD;
 }
 
-static inline int __pmu_filter_match(struct perf_event *event)
-{
-	struct pmu *pmu = event->pmu;
-	return pmu->filter_match ? pmu->filter_match(event) : 1;
-}
-
-/*
- * Check whether we should attempt to schedule an event group based on
- * PMU-specific filtering. An event group can consist of HW and SW events,
- * potentially with a SW leader, so we must check all the filters, to
- * determine whether a group is schedulable:
- */
-static inline int pmu_filter_match(struct perf_event *event)
-{
-	struct perf_event *sibling;
-
-	if (!__pmu_filter_match(event))
-		return 0;
-
-	for_each_sibling_event(sibling, event) {
-		if (!__pmu_filter_match(sibling))
-			return 0;
-	}
-
-	return 1;
-}
-
 static inline int
 event_filter_match(struct perf_event *event)
 {
 	return (event->cpu == -1 || event->cpu == smp_processor_id()) &&
-	       perf_cgroup_match(event) && pmu_filter_match(event);
+	       perf_cgroup_match(event);
 }
 
 static void
@@ -3661,6 +3634,9 @@ static noinline int visit_groups_merge(struct perf_event_context *ctx,
 	struct perf_event **evt;
 	int ret;
 
+	if (pmu->filter && pmu->filter(pmu, cpu))
+		return 0;
+
 	if (!ctx->task) {
 		cpuctx = this_cpu_ptr(&cpu_context);
 		event_heap = (struct min_heap){

  reply	other threads:[~2022-08-24 15:06 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-13 13:47 [RFC v2] perf: Rewrite core context handling Ravi Bangoria
2022-01-17  7:18 ` [perf] f7cf7134e4: WARNING:at_kernel/events/core.c:#__pmu_ctx_sched_out kernel test robot
2022-01-31  4:43 ` [RFC v2] perf: Rewrite core context handling Ravi Bangoria
2022-06-13 14:35 ` Peter Zijlstra
2022-06-13 14:36   ` Peter Zijlstra
2022-06-13 14:38   ` Peter Zijlstra
2022-08-02  6:11     ` Ravi Bangoria
2022-08-22 15:29       ` Peter Zijlstra
2022-08-22 15:43         ` Peter Zijlstra
2022-08-22 16:37           ` Ravi Bangoria
2022-08-23  4:20             ` Ravi Bangoria
2022-08-29  3:54               ` Ravi Bangoria
2022-08-23  6:30             ` Peter Zijlstra
2022-08-29  4:00             ` Ravi Bangoria
2022-08-29 11:58               ` Peter Zijlstra
2022-08-22 16:52       ` Peter Zijlstra
2022-08-23  4:57         ` Ravi Bangoria
2022-06-13 14:41   ` Peter Zijlstra
2022-08-22 14:38     ` Ravi Bangoria
2022-06-13 14:43   ` Peter Zijlstra
2022-08-02  6:16     ` Ravi Bangoria
2022-08-23  8:57       ` Peter Zijlstra
2022-08-24  5:07         ` Ravi Bangoria
2022-08-24  7:27           ` Peter Zijlstra
2022-08-24  7:53             ` Ravi Bangoria
2022-06-13 14:55   ` Peter Zijlstra
2022-08-02  6:10     ` Ravi Bangoria
2022-08-22 16:44       ` Peter Zijlstra
2022-08-23  4:46         ` Ravi Bangoria
2022-06-17 13:36   ` Peter Zijlstra
2022-08-24 10:13     ` Peter Zijlstra
2022-06-27  4:18   ` Ravi Bangoria
2022-08-02  6:06     ` Ravi Bangoria
2022-08-24 12:15   ` Peter Zijlstra
2022-08-24 14:59     ` Peter Zijlstra [this message]
2022-08-25  5:39       ` Ravi Bangoria
2022-08-25  9:17         ` Peter Zijlstra
2022-08-25 11:03       ` Ravi Bangoria
2022-08-02  6:13 ` Ravi Bangoria
2022-08-23  7:10   ` Peter Zijlstra
2022-08-02  6:17 ` Ravi Bangoria
2022-08-23  7:26   ` Peter Zijlstra
2022-08-23 15:14     ` Ravi Bangoria
2022-08-22 14:40 ` Ravi Bangoria

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YwY8u7gx6bO+RBcg@worktop.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=acme@kernel.org \
    --cc=ak@linux.intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=alexey.budankov@linux.intel.com \
    --cc=eranian@google.com \
    --cc=frederic@kernel.org \
    --cc=irogers@google.com \
    --cc=jolsa@redhat.com \
    --cc=kim.phillips@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maddy@linux.ibm.com \
    --cc=mark.rutland@arm.com \
    --cc=megha.dey@intel.com \
    --cc=namhyung@kernel.org \
    --cc=ravi.bangoria@amd.com \
    --cc=santosh.shukla@amd.com \
    --cc=songliubraving@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox