All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Bartosz Golaszewski" <bartosz.golaszewski@oss.qualcomm.com>
Cc: vkoul@kernel.org, Frank.Li@kernel.org, dmaengine@vger.kernel.org
Subject: Re: [PATCH v17 03/14] dmaengine: qcom: bam_dma: convert tasklet to a BH workqueue
Date: Tue, 19 May 2026 13:44:53 +0000	[thread overview]
Message-ID: <20260519134453.89022C2BCB3@smtp.kernel.org> (raw)
In-Reply-To: <20260519-qcom-qce-cmd-descr-v17-3-53a595414b79@oss.qualcomm.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] PM usage counter leak on failure in bam_start_dma() and bam_dma_irq()
--

commit 16a499b407c35e00637c627257ad62de2a108b14
Author: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

dmaengine: qcom: bam_dma: convert tasklet to a BH workqueue

This commit replaces the legacy tasklet implementation in the BAM DMA
driver with a high-priority BH workqueue.

> diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
> index cea44833201d6..e2f16efcdb55f 100644
> --- a/drivers/dma/qcom/bam_dma.c
> +++ b/drivers/dma/qcom/bam_dma.c
[ ... ]
> @@ -893,9 +894,9 @@ static irqreturn_t bam_dma_irq(int irq, void *data)
>  
>  	srcs |= process_channel_irqs(bdev);
>  
> -	/* kick off tasklet to start next dma transfer */
> +	/* kick off the work queue to start next dma transfer */
>  	if (srcs & P_IRQ)
> -		tasklet_schedule(&bdev->task);
> +		queue_work(system_bh_highpri_wq, &bdev->work);
>  
>  	ret = pm_runtime_get_sync(bdev->dev);
>  	if (ret < 0)
>  		return IRQ_NONE;

Does this code leak the PM usage counter if pm_runtime_get_sync() returns
a negative value?

While this wasn't introduced by this patch, pm_runtime_get_sync()
unconditionally increments the usage counter before checking the state.
Returning IRQ_NONE here without dropping the counter permanently prevents
the device from entering runtime suspend again.

Could this be updated to use pm_runtime_resume_and_get() instead, which
automatically drops the usage counter on failure?

[ ... ]
> @@ -1111,14 +1112,13 @@ static void dma_tasklet(struct tasklet_struct *t)
>  		if (!list_empty(&bchan->vc.desc_issued) && !IS_BUSY(bchan))
>  			bam_start_dma(bchan);

A similar pattern is present in bam_start_dma(), which is executed by this
newly added work queue callback. It calls pm_runtime_get_sync() and returns
early on failure without dropping the usage counter.

>  	}
> -
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-qcom-qce-cmd-descr-v17-0-53a595414b79@oss.qualcomm.com?part=3

  reply	other threads:[~2026-05-19 13:44 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-19 13:17 [PATCH v17 00/14] crypto/dmaengine: qce: introduce BAM locking and use DMA for register I/O Bartosz Golaszewski
2026-05-19 13:17 ` [PATCH v17 01/14] dmaengine: constify struct dma_descriptor_metadata_ops Bartosz Golaszewski
2026-05-19 13:17 ` [PATCH v17 02/14] dmaengine: qcom: bam_dma: free interrupt before the clock in error path Bartosz Golaszewski
2026-05-19 13:48   ` sashiko-bot
2026-05-19 13:17 ` [PATCH v17 03/14] dmaengine: qcom: bam_dma: convert tasklet to a BH workqueue Bartosz Golaszewski
2026-05-19 13:44   ` sashiko-bot [this message]
2026-05-19 13:17 ` [PATCH v17 04/14] dmaengine: qcom: bam_dma: Extend the driver's device match data Bartosz Golaszewski
2026-05-19 13:17 ` [PATCH v17 05/14] dmaengine: qcom: bam_dma: Add pipe_lock_supported flag support Bartosz Golaszewski
2026-05-19 13:17 ` [PATCH v17 06/14] dmaengine: qcom: bam_dma: add support for BAM locking Bartosz Golaszewski
2026-05-19 13:54   ` sashiko-bot
2026-05-19 13:17 ` [PATCH v17 07/14] crypto: qce - Cancel work on device detach Bartosz Golaszewski
2026-05-19 13:47   ` sashiko-bot
2026-05-19 13:17 ` [PATCH v17 08/14] crypto: qce - Include algapi.h in the core.h header Bartosz Golaszewski
2026-05-19 13:17 ` [PATCH v17 09/14] crypto: qce - Remove unused ignore_buf Bartosz Golaszewski
2026-05-19 13:48   ` sashiko-bot
2026-05-19 13:17 ` [PATCH v17 10/14] crypto: qce - Simplify arguments of devm_qce_dma_request() Bartosz Golaszewski
2026-05-19 13:17 ` [PATCH v17 11/14] crypto: qce - Use existing devres APIs in devm_qce_dma_request() Bartosz Golaszewski
2026-05-19 14:00   ` sashiko-bot
2026-05-19 13:17 ` [PATCH v17 12/14] crypto: qce - Map crypto memory for DMA Bartosz Golaszewski
2026-05-19 13:52   ` sashiko-bot
2026-05-19 13:17 ` [PATCH v17 13/14] crypto: qce - Add BAM DMA support for crypto register I/O Bartosz Golaszewski
2026-05-19 13:56   ` sashiko-bot
2026-05-19 13:17 ` [PATCH v17 14/14] crypto: qce - Communicate the base physical address to the dmaengine Bartosz Golaszewski
2026-05-19 14:17   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260519134453.89022C2BCB3@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=Frank.Li@kernel.org \
    --cc=bartosz.golaszewski@oss.qualcomm.com \
    --cc=dmaengine@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.