[PATCH 0/3] dmaengine: rcar-dmac: collection of small missed fixes

[PATCH 0/3] dmaengine: rcar-dmac: collection of small missed fixes

[Wolfram Sang]

While working on this driver, I found that there were these small fixes
contributed in the past but they have not been applied yet. So, I
collected and rebased them, added review tags and minorly improved the
commit messages. Please apply.


Koichiro Den (1):
  dmaengine: rcar-dmac: Add missing dma_descriptor_unmap()

Pan Chuang (1):
  dmaengine: rcar-dmac: Remove redundant dev_err()/dev_err_probe()

Zhang Shurong (1):
  dmaengine: rcar-dmac: Fix PM usage counter imbalance

 drivers/dma/sh/rcar-dmac.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

-- 
2.53.0

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

[Wolfram Sang]

From: Zhang Shurong <zhang_shurong@foxmail.com>

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>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.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 3ad8aba5dcd4..5b54f1c4ce15 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -1070,7 +1070,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.53.0

[PATCH 2/3] dmaengine: rcar-dmac: Add missing dma_descriptor_unmap()

[Wolfram Sang]

From: Koichiro Den <den@valinux.co.jp>

Call dma_descriptor_unmap() right after dma_cookie_complete() in the
threaded IRQ completion path. Without this, streaming DMA mappings
attached to the descriptor are never released and may eventually exhaust
DMA mapping resources (e.g. IOVA with an IOMMU or bounce-buffer slots
with SWIOTLB), leading to dma_map_* failures.

Also ensure dma_descriptor_unmap() is called in rcar_dmac_chan_reinit()
(error/terminate path) to avoid the same type of leaks.

Fixes: 87244fe5abdf ("dmaengine: rcar-dmac: Add Renesas R-Car Gen2 DMA Controller (DMAC) driver")
Signed-off-by: Koichiro Den <den@valinux.co.jp>
[wsa: added Fixes tag]
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/dma/sh/rcar-dmac.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index 5b54f1c4ce15..9155bceb723b 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -844,6 +844,7 @@ static void rcar_dmac_chan_reinit(struct rcar_dmac_chan *chan)
 
 	list_for_each_entry_safe(desc, _desc, &descs, node) {
 		list_del(&desc->node);
+		dma_descriptor_unmap(&desc->async_tx);
 		rcar_dmac_desc_put(chan, desc);
 	}
 }
@@ -1654,6 +1655,7 @@ static irqreturn_t rcar_dmac_isr_channel_thread(int irq, void *dev)
 		desc = list_first_entry(&chan->desc.done, struct rcar_dmac_desc,
 					node);
 		dma_cookie_complete(&desc->async_tx);
+		dma_descriptor_unmap(&desc->async_tx);
 		list_del(&desc->node);
 
 		dmaengine_desc_get_callback(&desc->async_tx, &cb);
-- 
2.53.0

[PATCH 3/3] dmaengine: rcar-dmac: Remove redundant dev_err()/dev_err_probe()

[Wolfram Sang]

From: Pan Chuang <panchuang@vivo.com>

The devm_request_threaded_irq() now automatically logs detailed error
messages on failure. This eliminates the need for driver-specific
dev_err() and dev_err_probe() calls that previously printed generic
messages.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/dma/sh/rcar-dmac.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index 9155bceb723b..9c0c00e7cc2f 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -1797,11 +1797,8 @@ static int rcar_dmac_chan_probe(struct rcar_dmac *dmac,
 					rcar_dmac_isr_channel,
 					rcar_dmac_isr_channel_thread, 0,
 					irqname, rchan);
-	if (ret) {
-		dev_err(dmac->dev, "failed to request IRQ %u (%d)\n",
-			rchan->irq, ret);
+	if (ret)
 		return ret;
-	}
 
 	return 0;
 }
-- 
2.53.0

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

[Laurent Pinchart]

On Thu, Sep 17, 2026 at 09:12:06AM +0200, Wolfram Sang wrote:
> From: Zhang Shurong <zhang_shurong@foxmail.com>
> 
> 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>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.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 3ad8aba5dcd4..5b54f1c4ce15 100644
> --- a/drivers/dma/sh/rcar-dmac.c
> +++ b/drivers/dma/sh/rcar-dmac.c
> @@ -1070,7 +1070,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)

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/3] dmaengine: rcar-dmac: Add missing dma_descriptor_unmap()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] dmaengine: data corruption due to premature dma_cookie_complete() before unmap
--

commit 1e9a738c62158e887202df32a8267f4bc4bfe4cd
Author: Koichiro Den <den@valinux.co.jp>

dmaengine: rcar-dmac: Add missing dma_descriptor_unmap()

This commit adds missing calls to dma_descriptor_unmap() in the threaded IRQ
completion path and the channel reinit path. This prevents streaming DMA 
mappings from exhausting mapping resources.

> diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
> index 2d2baa36feeae..016024858fe5a 100644
> --- a/drivers/dma/sh/rcar-dmac.c
> +++ b/drivers/dma/sh/rcar-dmac.c

[ ... ]

> @@ -1652,6 +1653,7 @@ static irqreturn_t rcar_dmac_isr_channel_thread(int irq, void *dev)
>  		desc = list_first_entry(&chan->desc.done, struct rcar_dmac_desc,
>  					node);
>  		dma_cookie_complete(&desc->async_tx);
> +		dma_descriptor_unmap(&desc->async_tx);

[Severity: High]
Could this specific ordering lead to data corruption? 

By calling dma_cookie_complete() before dma_descriptor_unmap() in
rcar_dmac_isr_channel_thread(), polling clients (via dma_async_is_tx_complete()
or rcar_dmac_tx_status()) checking for completion locklessly might observe
the completion too early.

If they access the destination buffer before the unmap operation finishes
cache invalidations or SWIOTLB bounce buffer copies, it could result in
reading stale data.

Would it be safer to place dma_descriptor_unmap() strictly before
dma_cookie_complete() to prevent clients from seeing stale memory?

>  		list_del(&desc->node);
>  
>  		dmaengine_desc_get_callback(&desc->async_tx, &cb);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917071208.36888-1-wsa+renesas@sang-engineering.com?part=2

Re: [PATCH 2/3] dmaengine: rcar-dmac: Add missing dma_descriptor_unmap()

[Laurent Pinchart]

On Thu, Sep 17, 2026 at 09:12:07AM +0200, Wolfram Sang wrote:
> From: Koichiro Den <den@valinux.co.jp>
> 
> Call dma_descriptor_unmap() right after dma_cookie_complete() in the
> threaded IRQ completion path. Without this, streaming DMA mappings
> attached to the descriptor are never released and may eventually exhaust
> DMA mapping resources (e.g. IOVA with an IOMMU or bounce-buffer slots
> with SWIOTLB), leading to dma_map_* failures.

That looks like a horrible API. It pushes a requirement onto drivers to
clean up something done by clients, and is completely undocumented.
Can't those calls be moved somewhere in the DMA engine core ?

> Also ensure dma_descriptor_unmap() is called in rcar_dmac_chan_reinit()
> (error/terminate path) to avoid the same type of leaks.
> 
> Fixes: 87244fe5abdf ("dmaengine: rcar-dmac: Add Renesas R-Car Gen2 DMA Controller (DMAC) driver")
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> [wsa: added Fixes tag]
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>  drivers/dma/sh/rcar-dmac.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
> index 5b54f1c4ce15..9155bceb723b 100644
> --- a/drivers/dma/sh/rcar-dmac.c
> +++ b/drivers/dma/sh/rcar-dmac.c
> @@ -844,6 +844,7 @@ static void rcar_dmac_chan_reinit(struct rcar_dmac_chan *chan)
>  
>  	list_for_each_entry_safe(desc, _desc, &descs, node) {
>  		list_del(&desc->node);
> +		dma_descriptor_unmap(&desc->async_tx);
>  		rcar_dmac_desc_put(chan, desc);
>  	}
>  }
> @@ -1654,6 +1655,7 @@ static irqreturn_t rcar_dmac_isr_channel_thread(int irq, void *dev)
>  		desc = list_first_entry(&chan->desc.done, struct rcar_dmac_desc,
>  					node);
>  		dma_cookie_complete(&desc->async_tx);
> +		dma_descriptor_unmap(&desc->async_tx);
>  		list_del(&desc->node);
>  
>  		dmaengine_desc_get_callback(&desc->async_tx, &cb);

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/3] dmaengine: rcar-dmac: Add missing dma_descriptor_unmap()

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 849 bytes --]

On Thu, Sep 17, 2026 at 10:38:25AM +0300, Laurent Pinchart wrote:
> On Thu, Sep 17, 2026 at 09:12:07AM +0200, Wolfram Sang wrote:
> > From: Koichiro Den <den@valinux.co.jp>
> > 
> > Call dma_descriptor_unmap() right after dma_cookie_complete() in the
> > threaded IRQ completion path. Without this, streaming DMA mappings
> > attached to the descriptor are never released and may eventually exhaust
> > DMA mapping resources (e.g. IOVA with an IOMMU or bounce-buffer slots
> > with SWIOTLB), leading to dma_map_* failures.
> 
> That looks like a horrible API. It pushes a requirement onto drivers to
> clean up something done by clients, and is completely undocumented.
> Can't those calls be moved somewhere in the DMA engine core ?

In general, I agree. No bandwidth for an API cleanup here, though. It
affects multiple drivers.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/3] dmaengine: rcar-dmac: Add missing dma_descriptor_unmap()

[Frank Li]

On Thu, Sep 17, 2026 at 09:12:07AM +0200, Wolfram Sang wrote:
> From: Koichiro Den <den@valinux.co.jp>
>
> Call dma_descriptor_unmap() right after dma_cookie_complete() in the
> threaded IRQ completion path. Without this, streaming DMA mappings
> attached to the descriptor are never released and may eventually exhaust
> DMA mapping resources (e.g. IOVA with an IOMMU or bounce-buffer slots
> with SWIOTLB), leading to dma_map_* failures.
>
> Also ensure dma_descriptor_unmap() is called in rcar_dmac_chan_reinit()
> (error/terminate path) to avoid the same type of leaks.
>
> Fixes: 87244fe5abdf ("dmaengine: rcar-dmac: Add Renesas R-Car Gen2 DMA Controller (DMAC) driver")
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> [wsa: added Fixes tag]
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>  drivers/dma/sh/rcar-dmac.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
> index 5b54f1c4ce15..9155bceb723b 100644
> --- a/drivers/dma/sh/rcar-dmac.c
> +++ b/drivers/dma/sh/rcar-dmac.c
> @@ -844,6 +844,7 @@ static void rcar_dmac_chan_reinit(struct rcar_dmac_chan *chan)
>
>  	list_for_each_entry_safe(desc, _desc, &descs, node) {
>  		list_del(&desc->node);
> +		dma_descriptor_unmap(&desc->async_tx);
>  		rcar_dmac_desc_put(chan, desc);
>  	}
>  }
> @@ -1654,6 +1655,7 @@ static irqreturn_t rcar_dmac_isr_channel_thread(int irq, void *dev)
>  		desc = list_first_entry(&chan->desc.done, struct rcar_dmac_desc,
>  					node);
>  		dma_cookie_complete(&desc->async_tx);
> +		dma_descriptor_unmap(&desc->async_tx);

Does callback function still use these data? suppose should unmap after
callback return.

Frank

>  		list_del(&desc->node);
>
>  		dmaengine_desc_get_callback(&desc->async_tx, &cb);
> --
> 2.53.0
>

Re: [PATCH 2/3] dmaengine: rcar-dmac: Add missing dma_descriptor_unmap()

[Koichiro Den]

On Thu, Sep 17, 2026 at 11:39:28AM -0500, Frank Li wrote:
> On Thu, Sep 17, 2026 at 09:12:07AM +0200, Wolfram Sang wrote:
> > From: Koichiro Den <den@valinux.co.jp>
> >
> > Call dma_descriptor_unmap() right after dma_cookie_complete() in the
> > threaded IRQ completion path. Without this, streaming DMA mappings
> > attached to the descriptor are never released and may eventually exhaust
> > DMA mapping resources (e.g. IOVA with an IOMMU or bounce-buffer slots
> > with SWIOTLB), leading to dma_map_* failures.
> >
> > Also ensure dma_descriptor_unmap() is called in rcar_dmac_chan_reinit()
> > (error/terminate path) to avoid the same type of leaks.
> >
> > Fixes: 87244fe5abdf ("dmaengine: rcar-dmac: Add Renesas R-Car Gen2 DMA Controller (DMAC) driver")
> > Signed-off-by: Koichiro Den <den@valinux.co.jp>
> > [wsa: added Fixes tag]
> > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > ---
> >  drivers/dma/sh/rcar-dmac.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
> > index 5b54f1c4ce15..9155bceb723b 100644
> > --- a/drivers/dma/sh/rcar-dmac.c
> > +++ b/drivers/dma/sh/rcar-dmac.c
> > @@ -844,6 +844,7 @@ static void rcar_dmac_chan_reinit(struct rcar_dmac_chan *chan)
> >
> >  	list_for_each_entry_safe(desc, _desc, &descs, node) {
> >  		list_del(&desc->node);
> > +		dma_descriptor_unmap(&desc->async_tx);
> >  		rcar_dmac_desc_put(chan, desc);
> >  	}
> >  }
> > @@ -1654,6 +1655,7 @@ static irqreturn_t rcar_dmac_isr_channel_thread(int irq, void *dev)
> >  		desc = list_first_entry(&chan->desc.done, struct rcar_dmac_desc,
> >  					node);
> >  		dma_cookie_complete(&desc->async_tx);
> > +		dma_descriptor_unmap(&desc->async_tx);
> 
> Does callback function still use these data? suppose should unmap after
> callback return.

Unmapping doesn't free the buffer, so I don't think accessing the buffer in the
callback requires that order. Correct me if I misinterpret your point.


P.S. Sashiko's concern about completing the cookie before unmapping seems valid,
but other drivers use that order too. Also, with drivers that can automatically
start DMA in tx_submit(), the following race also seems possible:

      API user                                 dmaengine driver
      -------------------------------------    -----------------------------
  #1  unmap = dmaengine_get_unmap_data(...)
  #2  dma_set_unmap(tx, unmap)
  #3  async_tx_submit(...)
  #4                                           dma_descriptor_unmap(tx)
  #5                                           invoke callback
  #6  dmaengine_unmap_put(unmap);

Here, #4 only drops the refcount from 2 to 1. With swiotlb, the actual unmap and
copyback can happen later at #6, so the callback in #5 may see stale destination
data. All in all, I have no idea how to sort all these concerns out cleanly, at
least for now. (I hope I'm missing something here and the existing code is
actually fine, but in any case, I personally feel the unmap infrastructure could
use some cleanup.)

Thanks,
Koichiro

> 
> Frank
> 
> >  		list_del(&desc->node);
> >
> >  		dmaengine_desc_get_callback(&desc->async_tx, &cb);
> > --
> > 2.53.0
> >

Re: [PATCH 2/3] dmaengine: rcar-dmac: Add missing dma_descriptor_unmap()

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 935 bytes --]


> > +		dma_descriptor_unmap(&desc->async_tx);
>
> Does callback function still use these data? suppose should unmap after
> callback return.

Actually, the opposite is true. Drivers have been fixed to use this
order. One example:

commit 9b335978f7081cd4fe264709599a18073e12fee2
Author: Dave Jiang <dave.jiang@intel.com>
Date:   Mon Jul 25 10:33:57 2016 -0700

    dmaengine: fsldma: move unmap to before callback

    Completion callback should happen after dma_descriptor_unmap() has
    happened. This allow the cache invalidate to happen and ensure that
    the data accessed by the upper layer is in memory that was from DMA
    rather than stale data. On some architecture this is done by the
    hardware, however we should make the code consistent to not cause
    confusion.

    Signed-off-by: Dave Jiang <dave.jiang@intel.com>
    Acked-by: Li Yang <leoyang.li@nxp.com>
    Signed-off-by: Vinod Koul <vinod.koul@intel.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/3] dmaengine: rcar-dmac: Add missing dma_descriptor_unmap()

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 417 bytes --]

Hi Koichiro-san,

> actually fine, but in any case, I personally feel the unmap infrastructure could
> use some cleanup.)

I share this thought, and I guess, Laurent expressed this, too. This is
a separate task to tackle, though. I still think your original patch
here makes the situation better by making sure the cache gets
invalidated and should be applied. Or am I missing something?

Happy hacking,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/3] dmaengine: rcar-dmac: Add missing dma_descriptor_unmap()

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 792 bytes --]


> I share this thought, and I guess, Laurent expressed this, too. This is
> a separate task to tackle, though. I still think your original patch
> here makes the situation better by making sure the cache gets
> invalidated and should be applied. Or am I missing something?

Okay, I got now that Sashiko's comment was related to cookie-completion
and not the callback. For me, its comment also makes sense because it is
basically the same argument which the commits I quoted to Frank used:
ensure cache completion. This is not only good before the callback but
also before cookie completion. Or am I missing something?

This is easy to fix. If we can agree on switching unmapping before
cookie completion, I can update this patch and then send a series to fix
other drivers, too.

Opinions?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/3] dmaengine: rcar-dmac: Add missing dma_descriptor_unmap()

[Koichiro Den]

On Sun, Sep 27, 2026 at 03:11:26PM +0200, Wolfram Sang wrote:
> 
> > I share this thought, and I guess, Laurent expressed this, too. This is
> > a separate task to tackle, though. I still think your original patch
> > here makes the situation better by making sure the cache gets
> > invalidated and should be applied. Or am I missing something?
> 
> Okay, I got now that Sashiko's comment was related to cookie-completion
> and not the callback. For me, its comment also makes sense because it is
> basically the same argument which the commits I quoted to Frank used:
> ensure cache completion. This is not only good before the callback but
> also before cookie completion. Or am I missing something?
> 
> This is easy to fix. If we can agree on switching unmapping before
> cookie completion, I can update this patch and then send a series to fix
> other drivers, too.

I agree with changing {cookie -> unmap}** to {unmap -> cookie} in this patch.
It looks like an improvement, I don't see any obvious downside to doing so.

Many drivers do {cookie -> unmap}, so it may be worth making the same change
there too. I haven't found a specific reason for that order in the history I
checked, but I'd also like to hear Frank's thoughts.

Thanks for handling this!

** cookie = dma_cookie_complete()
   unmap  = dma_descriptor_unmap()

Best regards,
Koichiro

> 
> Opinions?
> 

Re: [PATCH 2/3] dmaengine: rcar-dmac: Add missing dma_descriptor_unmap()

[Vinod Koul]

On 18-09-26, 22:07, Koichiro Den wrote:
> On Thu, Sep 17, 2026 at 11:39:28AM -0500, Frank Li wrote:
> > On Thu, Sep 17, 2026 at 09:12:07AM +0200, Wolfram Sang wrote:
> > > From: Koichiro Den <den@valinux.co.jp>
> > >
> > > Call dma_descriptor_unmap() right after dma_cookie_complete() in the
> > > threaded IRQ completion path. Without this, streaming DMA mappings
> > > attached to the descriptor are never released and may eventually exhaust
> > > DMA mapping resources (e.g. IOVA with an IOMMU or bounce-buffer slots
> > > with SWIOTLB), leading to dma_map_* failures.
> > >
> > > Also ensure dma_descriptor_unmap() is called in rcar_dmac_chan_reinit()
> > > (error/terminate path) to avoid the same type of leaks.
> > >
> > > Fixes: 87244fe5abdf ("dmaengine: rcar-dmac: Add Renesas R-Car Gen2 DMA Controller (DMAC) driver")
> > > Signed-off-by: Koichiro Den <den@valinux.co.jp>
> > > [wsa: added Fixes tag]
> > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > > ---
> > >  drivers/dma/sh/rcar-dmac.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
> > > index 5b54f1c4ce15..9155bceb723b 100644
> > > --- a/drivers/dma/sh/rcar-dmac.c
> > > +++ b/drivers/dma/sh/rcar-dmac.c
> > > @@ -844,6 +844,7 @@ static void rcar_dmac_chan_reinit(struct rcar_dmac_chan *chan)
> > >
> > >  	list_for_each_entry_safe(desc, _desc, &descs, node) {
> > >  		list_del(&desc->node);
> > > +		dma_descriptor_unmap(&desc->async_tx);
> > >  		rcar_dmac_desc_put(chan, desc);
> > >  	}
> > >  }
> > > @@ -1654,6 +1655,7 @@ static irqreturn_t rcar_dmac_isr_channel_thread(int irq, void *dev)
> > >  		desc = list_first_entry(&chan->desc.done, struct rcar_dmac_desc,
> > >  					node);
> > >  		dma_cookie_complete(&desc->async_tx);
> > > +		dma_descriptor_unmap(&desc->async_tx);
> > 
> > Does callback function still use these data? suppose should unmap after
> > callback return.
> 
> Unmapping doesn't free the buffer, so I don't think accessing the buffer in the
> callback requires that order. Correct me if I misinterpret your point.

Yep, we are not freeing, we are unmapping from dma. This can be and should be
done here.
-- 
~Vinod

Re: [PATCH 2/3] dmaengine: rcar-dmac: Add missing dma_descriptor_unmap()

[Vinod Koul]

On 29-09-26, 13:13, Koichiro Den wrote:
> On Sun, Sep 27, 2026 at 03:11:26PM +0200, Wolfram Sang wrote:
> > 
> > > I share this thought, and I guess, Laurent expressed this, too. This is
> > > a separate task to tackle, though. I still think your original patch
> > > here makes the situation better by making sure the cache gets
> > > invalidated and should be applied. Or am I missing something?
> > 
> > Okay, I got now that Sashiko's comment was related to cookie-completion
> > and not the callback. For me, its comment also makes sense because it is
> > basically the same argument which the commits I quoted to Frank used:
> > ensure cache completion. This is not only good before the callback but
> > also before cookie completion. Or am I missing something?
> > 
> > This is easy to fix. If we can agree on switching unmapping before
> > cookie completion, I can update this patch and then send a series to fix
> > other drivers, too.
> 
> I agree with changing {cookie -> unmap}** to {unmap -> cookie} in this patch.
> It looks like an improvement, I don't see any obvious downside to doing so.
> 
> Many drivers do {cookie -> unmap}, so it may be worth making the same change
> there too. I haven't found a specific reason for that order in the history I
> checked, but I'd also like to hear Frank's thoughts.

I cna chime in. I think Shashiko is correct here. We should ideally
unmap first and then call complete. This would ensure anyone seeing the
completion would get the right data buffer and chances of stale data are
eliminated.

So Wolfram, can you please reverse the order.
Also, lets document this.

Thanks
-- 
~Vinod

Re: [PATCH 2/3] dmaengine: rcar-dmac: Add missing dma_descriptor_unmap()

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1036 bytes --]


> > > This is easy to fix. If we can agree on switching unmapping before
> > > cookie completion, I can update this patch and then send a series to fix
> > > other drivers, too.
> > 
> > I agree with changing {cookie -> unmap}** to {unmap -> cookie} in this patch.
> > It looks like an improvement, I don't see any obvious downside to doing so.
> > 
> > Many drivers do {cookie -> unmap}, so it may be worth making the same change
> > there too. I haven't found a specific reason for that order in the history I
> > checked, but I'd also like to hear Frank's thoughts.
> 
> I cna chime in. I think Shashiko is correct here. We should ideally
> unmap first and then call complete. This would ensure anyone seeing the
> completion would get the right data buffer and chances of stale data are
> eliminated.
> 
> So Wolfram, can you please reverse the order.
> Also, lets document this.

Thanks for chiming in, Vinod. It seems we have consensus now, so I will
work on such a series next week. Thank you, everyone!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]