[PATCH 6/8] drm/i915/pmu: Replace closed with registered

[Matt Roper]

On Fri, Oct 11, 2024 at 03:54:28PM -0700, Lucas De Marchi wrote:
> Since i915 calls perf_pmu_register/perf_pmu_unregister, let's call the
> variable "registered" so we can flip the logic and rely on it being
> false by default. Looking at other drivers, it's also more common.
> Examples: arch/x86/events/intel/uncore.c and
> drivers/powercap/intel_rapl_common.c.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_pmu.c | 25 +++++++++++++------------
>  drivers/gpu/drm/i915/i915_pmu.h |  4 ++--
>  2 files changed, 15 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
> index 409e10d8190a8..e28c29bae2d9a 100644
> --- a/drivers/gpu/drm/i915/i915_pmu.c
> +++ b/drivers/gpu/drm/i915/i915_pmu.c
> @@ -303,7 +303,7 @@ void i915_pmu_gt_parked(struct intel_gt *gt)
>  {
>  	struct i915_pmu *pmu = &gt->i915->pmu;
>  
> -	if (pmu->closed)
> +	if (!pmu->registered)
>  		return;
>  
>  	spin_lock_irq(&pmu->lock);
> @@ -325,7 +325,7 @@ void i915_pmu_gt_unparked(struct intel_gt *gt)
>  {
>  	struct i915_pmu *pmu = &gt->i915->pmu;
>  
> -	if (pmu->closed)
> +	if (!pmu->registered)
>  		return;
>  
>  	spin_lock_irq(&pmu->lock);
> @@ -593,7 +593,7 @@ static int i915_pmu_event_init(struct perf_event *event)
>  	struct drm_i915_private *i915 = pmu_to_i915(pmu);
>  	int ret;
>  
> -	if (pmu->closed)
> +	if (!pmu->registered)
>  		return -ENODEV;
>  
>  	if (event->attr.type != event->pmu->type)
> @@ -686,7 +686,7 @@ static void i915_pmu_event_read(struct perf_event *event)
>  	struct hw_perf_event *hwc = &event->hw;
>  	u64 prev, new;
>  
> -	if (pmu->closed) {
> +	if (!pmu->registered) {
>  		event->hw.state = PERF_HES_STOPPED;
>  		return;
>  	}
> @@ -812,7 +812,7 @@ static void i915_pmu_event_start(struct perf_event *event, int flags)
>  {
>  	struct i915_pmu *pmu = event_to_pmu(event);
>  
> -	if (pmu->closed)
> +	if (!pmu->registered)
>  		return;
>  
>  	i915_pmu_enable(event);
> @@ -823,7 +823,7 @@ static void i915_pmu_event_stop(struct perf_event *event, int flags)
>  {
>  	struct i915_pmu *pmu = event_to_pmu(event);
>  
> -	if (pmu->closed)
> +	if (!pmu->registered)
>  		goto out;
>  
>  	if (flags & PERF_EF_UPDATE)
> @@ -839,7 +839,7 @@ static int i915_pmu_event_add(struct perf_event *event, int flags)
>  {
>  	struct i915_pmu *pmu = event_to_pmu(event);
>  
> -	if (pmu->closed)
> +	if (!pmu->registered)
>  		return -ENODEV;
>  
>  	if (flags & PERF_EF_START)
> @@ -1186,7 +1186,7 @@ static int i915_pmu_cpu_offline(unsigned int cpu, struct hlist_node *node)
>  	 * Unregistering an instance generates a CPU offline event which we must
>  	 * ignore to avoid incorrectly modifying the shared i915_pmu_cpumask.
>  	 */
> -	if (pmu->closed)
> +	if (!pmu->registered)
>  		return 0;
>  
>  	if (cpumask_test_and_clear_cpu(cpu, &i915_pmu_cpumask)) {
> @@ -1256,8 +1256,6 @@ void i915_pmu_register(struct drm_i915_private *i915)
>  	};
>  	int ret = -ENOMEM;
>  
> -	pmu->closed = true;
> -
>  	if (GRAPHICS_VER(i915) <= 2) {
>  		drm_info(&i915->drm, "PMU not supported for this GPU.");
>  		return;
> @@ -1316,7 +1314,7 @@ void i915_pmu_register(struct drm_i915_private *i915)
>  	if (drmm_add_action(&i915->drm, free_pmu, pmu))
>  		goto err_unreg;
>  
> -	pmu->closed = false;
> +	pmu->registered = true;
>  
>  	return;
>  
> @@ -1337,12 +1335,15 @@ void i915_pmu_unregister(struct drm_i915_private *i915)
>  {
>  	struct i915_pmu *pmu = &i915->pmu;
>  
> +	if (!pmu->registered)
> +		return;
> +
>  	/*
>  	 * "Disconnect" the PMU callbacks - since all are atomic synchronize_rcu
>  	 * ensures all currently executing ones will have exited before we
>  	 * proceed with unregistration.
>  	 */
> -	pmu->closed = true;
> +	pmu->registered = false;
>  	synchronize_rcu();
>  
>  	hrtimer_cancel(&pmu->timer);
> diff --git a/drivers/gpu/drm/i915/i915_pmu.h b/drivers/gpu/drm/i915/i915_pmu.h
> index 41af038c37388..3c1cf594954d9 100644
> --- a/drivers/gpu/drm/i915/i915_pmu.h
> +++ b/drivers/gpu/drm/i915/i915_pmu.h
> @@ -68,9 +68,9 @@ struct i915_pmu {
>  	 */
>  	struct pmu base;
>  	/**
> -	 * @closed: i915 is unregistering.
> +	 * @closed: PMU is registered and not in the unregistering process.

Missing a name change in the kerneldoc here.

Otherwise,

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

>  	 */
> -	bool closed;
> +	bool registered;
>  	/**
>  	 * @name: Name as registered with perf core.
>  	 */
> -- 
> 2.47.0
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation