Re: [PATCH v6 3/3] drm/imagination: Release pm references in case of error

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/pvr: misplaced pm_ref get clobbers submission error
- [Medium] drm/pvr: misapplied PM reference error handling is dead code
--

commit 5439537ea7866cb6e72eceafcbe08fbac4918034
Author: Alexandru Dadu <alexandru.dadu@imgtec.com>

drm/imagination: Release pm references in case of error
    
This commit attempts to add early release of power references in case of
errors during job submission.

> diff --git a/drivers/gpu/drm/imagination/pvr_queue.c b/drivers/gpu/drm/imagination/pvr_queue.c
> index b58c0887cafe5..1b013791897dd 100644
> --- a/drivers/gpu/drm/imagination/pvr_queue.c
> +++ b/drivers/gpu/drm/imagination/pvr_queue.c

[ ... ]

> @@ -817,9 +817,19 @@ static struct dma_fence *pvr_queue_run_job(struct drm_sched_job *sched_job)
>  		trace_pvr_job_submit_fw(job);
>  	}
>  
> +	err = pvr_job_get_pm_ref(job);
> +

[Severity: High]
Does this assignment overwrite the err variable containing the result of the
KCCB kick?

If pvr_cccb_send_kccb_kick() or pvr_cccb_send_kccb_combined_kick() returned
an error, pvr_job_get_pm_ref() will return 0 since the reference was already
acquired at the start of pvr_queue_run_job(). This causes the hardware
submission error to be lost, and the scheduler will receive a success
indication instead of an error.

>  	if (WARN_ON(err))
>  		goto err_release;
>  
> +	if (job->paired_job) {
> +		err = pvr_job_get_pm_ref(job->paired_job);
> +		if (WARN_ON(err)) {
> +			pvr_job_release_pm_ref(job);
> +			goto err_release;

[Severity: Medium]
Is this error handling logic unreachable?

Because the PM reference for job->paired_job was already acquired at the
beginning of the function, pvr_job_get_pm_ref() will return 0, making the
WARN_ON(err) condition unreachable.

If this abort path were somehow reached after the job was successfully
submitted to the hardware, jumping to err_release would inappropriately call
pvr_kccb_release_slot(). Could this corrupt KCCB slot accounting since the
hardware kick already occurred?

> +		}
> +	}
> +
>  	return dma_fence_get(job->done_fence);
>  
>  err_release:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924-b4-upstream-propagate-all-errors-from-kccb-cmd-submission-code-v6-0-d9d5375665ed@imgtec.com?part=3