Re: [PATCH v9 15/16] drm/panfrost: Fix races between perfcnt and reset sequence

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Mixing goto-based error handling with scoped_guard() cleanup in panfrost_perfcnt_enable_locked() violates cleanup API semantics.
--

commit 6a11ea2bc38694e85eb36147ae0e357afc1cc799
Author: Adrián Larumbe <adrian.larumbe@collabora.com>

drm/panfrost: Fix races between perfcnt and reset sequence

This commit addresses concurrency issues between the reset sequence and
perfcnt session teardown by governing hardware access with the reset
r/w semaphore. It also expands the uAPI to notify userspace if a reset
has occurred during a perfcnt session.

> diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> index b3f71d7fd82a9..9847657179a57 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
[ ... ]
> @@ -122,54 +203,31 @@ static int panfrost_perfcnt_enable_locked(struct panfrost_device *pfdev,
>  	ret = drm_gem_vmap(&bo->base, &map);
>  	if (ret)
>  		goto err_put_mapping;
> +
>  	perfcnt->buf = map.vaddr;
> +	perfcnt->counterset = counterset;
>  
>  	panfrost_gem_internal_set_label(&bo->base, "Perfcnt sample buffer");
>  
[ ... ]
> +	scoped_guard(rwsem_read, &pfdev->reset.lock) {
> +		/*
> +		 * Clear the counters to start from a fresh state.
> +		 */
> +		gpu_write(pfdev, GPU_INT_CLEAR, GPU_IRQ_PERFCNT_SAMPLE_COMPLETED);
> +		gpu_write(pfdev, GPU_CMD, GPU_CMD_PERFCNT_CLEAR);
>  
> -	gpu_write(pfdev, GPU_PERFCNT_CFG, cfg);
> +		ret = panfrost_perfcnt_hw_enable(pfdev);
> +		if (ret)
> +			goto err_vunmap;

[Severity: Low]
Does this mix goto-based error handling with scope-based cleanup?

The kernel cleanup API guidelines state that we should never mix goto
labels and cleanup helpers (like scoped_guard) in the same function.
Jumping out of a scoped_guard block to an error label creates confusing
ownership semantics. For a given routine, the documentation recommends
converting all resources that need a goto cleanup to scope-based cleanup,
or converting none of them.

> -	if (panfrost_has_hw_issue(pfdev, HW_ISSUE_8186))
> -		gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0xffffffff);
> +		perfcnt->reset_happened = false;
> +		perfcnt->owns_as_ref = true;
> +		perfcnt->user = user;
> +	}
>  
>  	/* The BO ref is retained by the mapping. */
>  	drm_gem_object_put(&bo->base);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260912-claude-fixes-v9-0-e588feaa61ef@collabora.com?part=15