[PATCH] dmaengine: rcar-dmac: Fix PM usage counter imbalance

[PATCH] dmaengine: rcar-dmac: Fix PM usage counter imbalance

[Zhang Shurong]

pm_runtime_get_sync will increment pm usage counter
even it failed. Forgetting to putting operation will
result in reference leak here. We fix it by replacing
it with pm_runtime_resume_and_get to keep usage counter
balanced.

Fixes: 87244fe5abdf ("dmaengine: rcar-dmac: Add Renesas R-Car Gen2 DMA Controller (DMAC) driver")
Signed-off-by: Zhang Shurong <zhang_shurong@foxmail.com>
---
 drivers/dma/sh/rcar-dmac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index 0c45ce8c74aa..c1ce3b0ae74d 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -1068,7 +1068,7 @@ static int rcar_dmac_alloc_chan_resources(struct dma_chan *chan)
 	if (ret < 0)
 		return -ENOMEM;
 
-	return pm_runtime_get_sync(chan->device->dev);
+	return pm_runtime_resume_and_get(chan->device->dev);
 }
 
 static void rcar_dmac_free_chan_resources(struct dma_chan *chan)
-- 
2.39.5

Re: [PATCH] dmaengine: rcar-dmac: Fix PM usage counter imbalance

[Geert Uytterhoeven]

Hi Zhang,

On Sun, 29 Jun 2025 at 17:57, Zhang Shurong <zhang_shurong@foxmail.com> wrote:
> pm_runtime_get_sync will increment pm usage counter
> even it failed. Forgetting to putting operation will
> result in reference leak here. We fix it by replacing
> it with pm_runtime_resume_and_get to keep usage counter
> balanced.
>
> Fixes: 87244fe5abdf ("dmaengine: rcar-dmac: Add Renesas R-Car Gen2 DMA Controller (DMAC) driver")
> Signed-off-by: Zhang Shurong <zhang_shurong@foxmail.com>

Thanks for your patch!

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

> --- a/drivers/dma/sh/rcar-dmac.c
> +++ b/drivers/dma/sh/rcar-dmac.c
> @@ -1068,7 +1068,7 @@ static int rcar_dmac_alloc_chan_resources(struct dma_chan *chan)
>         if (ret < 0)
>                 return -ENOMEM;
>
> -       return pm_runtime_get_sync(chan->device->dev);
> +       return pm_runtime_resume_and_get(chan->device->dev);

Note that there are other issues with this function: in case of failure,
none of the memory allocated before is freed.  Probably the original
author assumed none of this can really fail.

>  }
>
>  static void rcar_dmac_free_chan_resources(struct dma_chan *chan)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] dmaengine: rcar-dmac: Fix PM usage counter imbalance

[Zhang Shurong]

Hi Geert,
Thank you for reviewing my patch and providing the Reviewed-by tag. 
I appreciate you pointing this out. Testing these fixes comprehensively has been challenging 
as QEMU doesn't support the Renesas R-Car SoC, which makes it difficult to verify the complete
behavior of the driver.
I'll continue working on addressing the memory leak issue where the function doesn't free previously
allocated memory on failure.
Thanks again for your thorough review.
Best regards,
Zhang Shurong

> On Jul 2, 2025, at 18:18, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> 
> Hi Zhang,
> 
> On Sun, 29 Jun 2025 at 17:57, Zhang Shurong <zhang_shurong@foxmail.com> wrote:
>> pm_runtime_get_sync will increment pm usage counter
>> even it failed. Forgetting to putting operation will
>> result in reference leak here. We fix it by replacing
>> it with pm_runtime_resume_and_get to keep usage counter
>> balanced.
>> 
>> Fixes: 87244fe5abdf ("dmaengine: rcar-dmac: Add Renesas R-Car Gen2 DMA Controller (DMAC) driver")
>> Signed-off-by: Zhang Shurong <zhang_shurong@foxmail.com>
> 
> Thanks for your patch!
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
>> --- a/drivers/dma/sh/rcar-dmac.c
>> +++ b/drivers/dma/sh/rcar-dmac.c
>> @@ -1068,7 +1068,7 @@ static int rcar_dmac_alloc_chan_resources(struct dma_chan *chan)
>>        if (ret < 0)
>>                return -ENOMEM;
>> 
>> -       return pm_runtime_get_sync(chan->device->dev);
>> +       return pm_runtime_resume_and_get(chan->device->dev);
> 
> Note that there are other issues with this function: in case of failure,
> none of the memory allocated before is freed.  Probably the original
> author assumed none of this can really fail.
> 
>> }
>> 
>> static void rcar_dmac_free_chan_resources(struct dma_chan *chan)
> 
> Gr{oetje,eeting}s,
> 
>                        Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                -- Linus Torvalds

Re: [PATCH] dmaengine: rcar-dmac: Fix PM usage counter imbalance

[Zhang Shurong]

Hi Geert,
Thank you for reviewing my patch and providing the Reviewed-by tag. 
I appreciate you pointing this out. Testing these fixes comprehensively has been challenging 
as QEMU doesn't support the Renesas R-Car SoC, which makes it difficult to verify the complete
behavior of the driver.
I'll continue working on addressing the memory leak issue where the function doesn't free previously
allocated memory on failure.
Thanks again for your thorough review.
Best regards,
Zhang Shurong

> On Jul 2, 2025, at 18:18, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> 
> Hi Zhang,
> 
> On Sun, 29 Jun 2025 at 17:57, Zhang Shurong <zhang_shurong@foxmail.com> wrote:
>> pm_runtime_get_sync will increment pm usage counter
>> even it failed. Forgetting to putting operation will
>> result in reference leak here. We fix it by replacing
>> it with pm_runtime_resume_and_get to keep usage counter
>> balanced.
>> 
>> Fixes: 87244fe5abdf ("dmaengine: rcar-dmac: Add Renesas R-Car Gen2 DMA Controller (DMAC) driver")
>> Signed-off-by: Zhang Shurong <zhang_shurong@foxmail.com>
> 
> Thanks for your patch!
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
>> --- a/drivers/dma/sh/rcar-dmac.c
>> +++ b/drivers/dma/sh/rcar-dmac.c
>> @@ -1068,7 +1068,7 @@ static int rcar_dmac_alloc_chan_resources(struct dma_chan *chan)
>>        if (ret < 0)
>>                return -ENOMEM;
>> 
>> -       return pm_runtime_get_sync(chan->device->dev);
>> +       return pm_runtime_resume_and_get(chan->device->dev);
> 
> Note that there are other issues with this function: in case of failure,
> none of the memory allocated before is freed.  Probably the original
> author assumed none of this can really fail.
> 
>> }
>> 
>> static void rcar_dmac_free_chan_resources(struct dma_chan *chan)
> 
> Gr{oetje,eeting}s,
> 
>                        Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                -- Linus Torvalds

Re: [PATCH] dmaengine: rcar-dmac: Fix PM usage counter imbalance

[Markus Elfring]

…
> result in reference leak here. We fix it by replacing
> it with pm_runtime_resume_and_get to keep usage counter
> balanced.

You may occasionally put more than 55 characters into text lines
of such a change description.

See also:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.16-rc5#n94

Regards,
Markus