Re: [PATCH v2] dmaengine: qcom_hidma: Fix runtime PM leak in hidma_issue_task()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] dmaengine: qcom_hidma: stranded descriptors and PM leak in
  hidma_issue_task()
--

commit 5c9f6221932ae0db0413c36b36c0997430c109f5
Author: Wentao Liang <vulab@iscas.ac.cn>

dmaengine: qcom_hidma: Fix runtime PM leak in hidma_issue_task()

This commit replaces pm_runtime_get_sync() with pm_runtime_resume_and_get()
in hidma_issue_task() and returns early without calling hidma_ll_start()
when runtime resume fails.

> diff --git a/drivers/dma/qcom/hidma.c b/drivers/dma/qcom/hidma.c
> index 1349066b1544d..c49cf622e5fb5 100644
> --- a/drivers/dma/qcom/hidma.c
> +++ b/drivers/dma/qcom/hidma.c
> @@ -219,7 +219,8 @@ static void hidma_issue_task(struct tasklet_struct *t)
>  {
>  	struct hidma_dev *dmadev = from_tasklet(dmadev, t, task);
>  
> -	pm_runtime_get_sync(dmadev->ddev.dev);
> +	if (pm_runtime_resume_and_get(dmadev->ddev.dev) < 0)
> +		return;
>  	hidma_ll_start(dmadev->lldev);
>  }

[Severity: Medium]
Does returning early here leave queued descriptors stranded on
mchan->active and still leak the runtime PM reference taken in
hidma_issue_pending()?

Before scheduling dmadev->task, hidma_issue_pending() already queues the
descriptors into dmadev->lldev and increments the runtime PM usage count:

drivers/dma/qcom/hidma.c:hidma_issue_pending() {
	spin_lock_irqsave(&mchan->lock, flags);
	list_for_each_entry_safe(qdesc, next, &mchan->queued, node) {
		hidma_ll_queue_request(dmadev->lldev, qdesc->tre_ch);
		list_move_tail(&qdesc->node, &mchan->active);
	}
	...
	/* PM will be released in hidma_callback function. */
	status = pm_runtime_get(dmadev->ddev.dev);
	if (status < 0)
		tasklet_schedule(&dmadev->task);
	else
		hidma_ll_start(dmadev->lldev);
}

pm_runtime_get() increments dev->power.usage_count unconditionally, even
when it returns a negative error such as -EINPROGRESS or -EACCES.

When pm_runtime_resume_and_get() fails in hidma_issue_task(), it only drops
the second reference that it took itself, leaving the reference from
pm_runtime_get() elevated.  Skipping hidma_ll_start() without aborting the
queued TREs also leaves those descriptors stuck on mchan->active and
lldev->pending_tre_list, so hidma_callback() never runs to complete the
transfers or call pm_runtime_put_autosuspend().

When pm_runtime_resume_and_get() succeeds, both hidma_issue_pending() and
hidma_issue_task() have incremented usage_count, while hidma_callback()
only drops one reference.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/179395457228.2120679.13809646473624751961@iscas.ac.cn?part=1