diff mbox series

[8/8] drm/i915/pmu: Release open events when unregistering

Message ID 20241011225430.1219345-9-lucas.demarchi@intel.com (mailing list archive)
State New, archived
Headers show
Series Fix i915 pmu on bind/unbind | expand

Commit Message

Lucas De Marchi Oct. 11, 2024, 10:54 p.m. UTC 
When unregistering the PMU, disable the active events. This
allows userspace to see the change and possibly react on it, like
reopening the fd.

With perf-stat, "<not counted>" starts to be printed:

	2.007597571        500,909,500 ns   i915_0000_03_00.0/rc6-residency/
	2.508569383                  0 ns   i915_0000_03_00.0/rc6-residency/
	3.009405012      <not counted> ns   i915_0000_03_00.0/rc6-residency/

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/i915_pmu.c | 35 ++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/i915_pmu.h |  6 ++++++
 2 files changed, 40 insertions(+), 1 deletion(-)

Comments

Matt Roper Oct. 30, 2024, 12:20 a.m. UTC | #1 
On Fri, Oct 11, 2024 at 03:54:30PM -0700, Lucas De Marchi wrote:
> When unregistering the PMU, disable the active events. This
> allows userspace to see the change and possibly react on it, like
> reopening the fd.
> 
> With perf-stat, "<not counted>" starts to be printed:
> 
> 	2.007597571        500,909,500 ns   i915_0000_03_00.0/rc6-residency/
> 	2.508569383                  0 ns   i915_0000_03_00.0/rc6-residency/
> 	3.009405012      <not counted> ns   i915_0000_03_00.0/rc6-residency/
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_pmu.c | 35 ++++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/i915/i915_pmu.h |  6 ++++++
>  2 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
> index 2f8004ade6d36..1466464c119df 100644
> --- a/drivers/gpu/drm/i915/i915_pmu.c
> +++ b/drivers/gpu/drm/i915/i915_pmu.c
> @@ -515,6 +515,18 @@ static enum hrtimer_restart i915_sample(struct hrtimer *hrtimer)
>  	return HRTIMER_RESTART;
>  }
>  
> +static void i915_pmu_event_destroy(struct perf_event *event)
> +{
> +	struct i915_pmu *pmu = event_to_pmu(event);
> +	unsigned long idx;
> +	struct perf_event *e;
> +
> +	xa_for_each(&pmu->active_events, idx, e)
> +		if (e == event) {
> +			xa_erase(&pmu->active_events, idx);
> +			break;
> +		}
> +}
>  
>  static int
>  engine_event_status(struct intel_engine_cs *engine,
> @@ -539,7 +551,6 @@ static int
>  config_status(struct drm_i915_private *i915, u64 config)
>  {
>  	struct intel_gt *gt = to_gt(i915);
> -
>  	unsigned int gt_id = config_gt_id(config);
>  	unsigned int max_gt_id = HAS_EXTRA_GT_LIST(i915) ? 1 : 0;
>  
> @@ -591,6 +602,7 @@ static int i915_pmu_event_init(struct perf_event *event)
>  {
>  	struct i915_pmu *pmu = event_to_pmu(event);
>  	struct drm_i915_private *i915 = pmu_to_i915(pmu);
> +	u32 event_id;
>  	int ret;
>  
>  	if (!pmu->registered)
> @@ -620,6 +632,13 @@ static int i915_pmu_event_init(struct perf_event *event)
>  	if (ret)
>  		return ret;
>  
> +	ret = xa_alloc(&pmu->active_events, &event_id, event,
> +		       xa_limit_32b, GFP_KERNEL);
> +	if (ret)
> +		return ret;
> +
> +	event->destroy = i915_pmu_event_destroy;
> +
>  	return 0;
>  }
>  
> @@ -1263,6 +1282,7 @@ void i915_pmu_register(struct drm_i915_private *i915)
>  
>  	spin_lock_init(&pmu->lock);
>  	hrtimer_init(&pmu->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> +	xa_init_flags(&pmu->active_events, XA_FLAGS_ALLOC);
>  	pmu->timer.function = i915_sample;
>  	pmu->cpuhp.cpu = -1;
>  	init_rc6(pmu);
> @@ -1331,6 +1351,17 @@ void i915_pmu_register(struct drm_i915_private *i915)
>  	drm_notice(&i915->drm, "Failed to register PMU!\n");
>  }
>  
> +static void release_active_events(struct i915_pmu *pmu)
> +{
> +	struct perf_event *event;
> +	unsigned long idx;
> +
> +	xa_for_each(&pmu->active_events, idx, event) {
> +		xa_erase(&pmu->active_events, idx);
> +		perf_event_disable(event);
> +	}
> +}
> +
>  void i915_pmu_unregister(struct drm_i915_private *i915)
>  {
>  	struct i915_pmu *pmu = &i915->pmu;
> @@ -1341,6 +1372,8 @@ void i915_pmu_unregister(struct drm_i915_private *i915)
>  	/* Disconnect the PMU callbacks */
>  	pmu->registered = false;
>  
> +	release_active_events(pmu);
> +
>  	hrtimer_cancel(&pmu->timer);
>  
>  	i915_pmu_unregister_cpuhp_state(pmu);
> diff --git a/drivers/gpu/drm/i915/i915_pmu.h b/drivers/gpu/drm/i915/i915_pmu.h
> index 3c1cf594954d9..dd4cac95d032b 100644
> --- a/drivers/gpu/drm/i915/i915_pmu.h
> +++ b/drivers/gpu/drm/i915/i915_pmu.h
> @@ -10,6 +10,7 @@
>  #include <linux/hrtimer.h>
>  #include <linux/perf_event.h>
>  #include <linux/spinlock_types.h>
> +#include <linux/xarray.h>
>  #include <uapi/drm/i915_drm.h>
>  
>  struct drm_i915_private;
> @@ -152,6 +153,11 @@ struct i915_pmu {
>  	 * @pmu_attr: Memory block holding device attributes.
>  	 */
>  	void *pmu_attr;
> +
> +	/**
> +	 * @active_events: Events currently open by perf.
> +	 */
> +	struct xarray active_events;
>  };
>  
>  #ifdef CONFIG_PERF_EVENTS
> -- 
> 2.47.0
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 2f8004ade6d36..1466464c119df 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -515,6 +515,18 @@  static enum hrtimer_restart i915_sample(struct hrtimer *hrtimer)
 	return HRTIMER_RESTART;
 }
 
+static void i915_pmu_event_destroy(struct perf_event *event)
+{
+	struct i915_pmu *pmu = event_to_pmu(event);
+	unsigned long idx;
+	struct perf_event *e;
+
+	xa_for_each(&pmu->active_events, idx, e)
+		if (e == event) {
+			xa_erase(&pmu->active_events, idx);
+			break;
+		}
+}
 
 static int
 engine_event_status(struct intel_engine_cs *engine,
@@ -539,7 +551,6 @@  static int
 config_status(struct drm_i915_private *i915, u64 config)
 {
 	struct intel_gt *gt = to_gt(i915);
-
 	unsigned int gt_id = config_gt_id(config);
 	unsigned int max_gt_id = HAS_EXTRA_GT_LIST(i915) ? 1 : 0;
 
@@ -591,6 +602,7 @@  static int i915_pmu_event_init(struct perf_event *event)
 {
 	struct i915_pmu *pmu = event_to_pmu(event);
 	struct drm_i915_private *i915 = pmu_to_i915(pmu);
+	u32 event_id;
 	int ret;
 
 	if (!pmu->registered)
@@ -620,6 +632,13 @@  static int i915_pmu_event_init(struct perf_event *event)
 	if (ret)
 		return ret;
 
+	ret = xa_alloc(&pmu->active_events, &event_id, event,
+		       xa_limit_32b, GFP_KERNEL);
+	if (ret)
+		return ret;
+
+	event->destroy = i915_pmu_event_destroy;
+
 	return 0;
 }
 
@@ -1263,6 +1282,7 @@  void i915_pmu_register(struct drm_i915_private *i915)
 
 	spin_lock_init(&pmu->lock);
 	hrtimer_init(&pmu->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	xa_init_flags(&pmu->active_events, XA_FLAGS_ALLOC);
 	pmu->timer.function = i915_sample;
 	pmu->cpuhp.cpu = -1;
 	init_rc6(pmu);
@@ -1331,6 +1351,17 @@  void i915_pmu_register(struct drm_i915_private *i915)
 	drm_notice(&i915->drm, "Failed to register PMU!\n");
 }
 
+static void release_active_events(struct i915_pmu *pmu)
+{
+	struct perf_event *event;
+	unsigned long idx;
+
+	xa_for_each(&pmu->active_events, idx, event) {
+		xa_erase(&pmu->active_events, idx);
+		perf_event_disable(event);
+	}
+}
+
 void i915_pmu_unregister(struct drm_i915_private *i915)
 {
 	struct i915_pmu *pmu = &i915->pmu;
@@ -1341,6 +1372,8 @@  void i915_pmu_unregister(struct drm_i915_private *i915)
 	/* Disconnect the PMU callbacks */
 	pmu->registered = false;
 
+	release_active_events(pmu);
+
 	hrtimer_cancel(&pmu->timer);
 
 	i915_pmu_unregister_cpuhp_state(pmu);
diff --git a/drivers/gpu/drm/i915/i915_pmu.h b/drivers/gpu/drm/i915/i915_pmu.h
index 3c1cf594954d9..dd4cac95d032b 100644
--- a/drivers/gpu/drm/i915/i915_pmu.h
+++ b/drivers/gpu/drm/i915/i915_pmu.h
@@ -10,6 +10,7 @@ 
 #include <linux/hrtimer.h>
 #include <linux/perf_event.h>
 #include <linux/spinlock_types.h>
+#include <linux/xarray.h>
 #include <uapi/drm/i915_drm.h>
 
 struct drm_i915_private;
@@ -152,6 +153,11 @@  struct i915_pmu {
 	 * @pmu_attr: Memory block holding device attributes.
 	 */
 	void *pmu_attr;
+
+	/**
+	 * @active_events: Events currently open by perf.
+	 */
+	struct xarray active_events;
 };
 
 #ifdef CONFIG_PERF_EVENTS