Re: [PATCH] accel/ivpu: Use threaded IRQ for IPC callback processing

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Firmware command queue leak and duplicate hardware initialization on the retry path in `ivpu_job_submit`.
--

commit 91623d3f3cc1b38781bd2f9f2354ef0fa3591d50
Author: Karol Wachowski <karol.wachowski@linux.intel.com>

accel/ivpu: Use threaded IRQ for IPC callback processing

Dispatching IPC callbacks from system_percpu_wq adds scheduling latency
that is neither bounded nor predictable, which hurts job completion
turnaround. Handle them from a threaded IRQ instead, and defer job
resource teardown to a dedicated unbounded workqueue.

> diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
> index 521931d1f7fca..7320c97762832 100644
> --- a/drivers/accel/ivpu/ivpu_job.c
> +++ b/drivers/accel/ivpu/ivpu_job.c
[ ... ]
> @@ -709,6 +748,14 @@ static int ivpu_job_submit(struct ivpu_job *job, u8 priority, u32 cmdq_id)
>  	}
>  
>  	ret = ivpu_cmdq_register(file_priv, cmdq);
> +	if (ret == -EBUSY && !flushed) {
> +		/* Doorbell may be held by jobs pending deferred cleanup */
> +		mutex_unlock(&file_priv->lock);
> +		mutex_unlock(&vdev->submitted_jobs_lock);
> +		flush_work(&vdev->job_destroy_work);
> +		flushed = true;
> +		goto retry;
> +	}

[Severity: High]
Does this retry loop cause duplicate firmware queue initialization and leak
resources?

If ivpu_cmdq_register() successfully calls ivpu_hws_cmdq_init() but then
ivpu_register_db() fails with -EBUSY, the function returns -EBUSY without
undoing the firmware queue initialization and leaves cmdq->db_id as 0:

drivers/accel/ivpu/ivpu_job.c:ivpu_cmdq_register() {
    ...
	if (vdev->fw->sched_mode == VPU_SCHEDULING_MODE_HW) {
		ret = ivpu_hws_cmdq_init(file_priv, cmdq, VPU_ENGINE_COMPUTE, cmdq->priority);
		if (ret)
			return ret;
	}

	ret = ivpu_register_db(file_priv, cmdq);
	if (ret)
		return ret;  // Returns -EBUSY here, db_id is 0
    ...
}

When the goto retry path is taken, ivpu_cmdq_register() is called again
with the same cmdq. Because cmdq->db_id is still 0, the early bailout is
skipped, and ivpu_hws_cmdq_init() is called a second time.

Furthermore, if the retry also fails (or is not attempted), will this
leak the firmware resource? The unregister path seems to bail early if
db_id is 0:

drivers/accel/ivpu/ivpu_job.c:ivpu_cmdq_unregister() {
	if (!cmdq->db_id)
		return 0;
    ... // ivpu_jsm_hws_destroy_cmdq() is skipped
}

>  	if (ret) {
>  		ivpu_err(vdev, "Failed to register command queue: %d\n", ret);
>  		goto err_unlock;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611055201.948726-1-karol.wachowski@linux.intel.com?part=1