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@xxxxxxxxx>
Reviewed-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
> ---
> 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
>
--
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation
