Re: [PATCH v8 1/8] dmaengine: sh: rz-dmac: Protect the driver specific lists

[Frank Li]

On Tue, Jan 20, 2026 at 03:33:23PM +0200, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
>
> The driver lists (ld_free, ld_queue) are used in
> rz_dmac_free_chan_resources(), rz_dmac_terminate_all(),
> rz_dmac_issue_pending(), and rz_dmac_irq_handler_thread(), all under
> the virtual channel lock. Take the same lock in rz_dmac_prep_slave_sg()
> and rz_dmac_prep_dma_memcpy() as well to avoid concurrency issues, since
> these functions also check whether the lists are empty and update or
> remove list entries.
>
> Fixes: 5000d37042a6 ("dmaengine: sh: Add DMAC driver for RZ/G2L SoC")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
> ---

Reviewed-by: Frank Li <Frank.Li@xxxxxxx>
>
> Changes in v8:
> - none
>
> Changes in v7:
> - none
>
> Changes in v6:
> - none
>
> Changes in v5:
> - none, this patch is new
>
>  drivers/dma/sh/rz-dmac.c | 57 ++++++++++++++++++++++------------------
>  1 file changed, 32 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> index 3dde4b006bcc..36f5fc80a17a 100644
> --- a/drivers/dma/sh/rz-dmac.c
> +++ b/drivers/dma/sh/rz-dmac.c
> @@ -10,6 +10,7 @@
>   */
>
>  #include <linux/bitfield.h>
> +#include <linux/cleanup.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/dmaengine.h>
>  #include <linux/interrupt.h>
> @@ -453,6 +454,7 @@ static int rz_dmac_alloc_chan_resources(struct dma_chan *chan)
>  		if (!desc)
>  			break;
>
> +		/* No need to lock. This is called only for the 1st client. */
>  		list_add_tail(&desc->node, &channel->ld_free);
>  		channel->descs_allocated++;
>  	}
> @@ -508,18 +510,21 @@ rz_dmac_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
>  	dev_dbg(dmac->dev, "%s channel: %d src=0x%pad dst=0x%pad len=%zu\n",
>  		__func__, channel->index, &src, &dest, len);
>
> -	if (list_empty(&channel->ld_free))
> -		return NULL;
> +	scoped_guard(spinlock_irqsave, &channel->vc.lock) {
> +		if (list_empty(&channel->ld_free))
> +			return NULL;
> +
> +		desc = list_first_entry(&channel->ld_free, struct rz_dmac_desc, node);
>
> -	desc = list_first_entry(&channel->ld_free, struct rz_dmac_desc, node);
> +		desc->type = RZ_DMAC_DESC_MEMCPY;
> +		desc->src = src;
> +		desc->dest = dest;
> +		desc->len = len;
> +		desc->direction = DMA_MEM_TO_MEM;
>
> -	desc->type = RZ_DMAC_DESC_MEMCPY;
> -	desc->src = src;
> -	desc->dest = dest;
> -	desc->len = len;
> -	desc->direction = DMA_MEM_TO_MEM;
> +		list_move_tail(channel->ld_free.next, &channel->ld_queue);
> +	}
>
> -	list_move_tail(channel->ld_free.next, &channel->ld_queue);
>  	return vchan_tx_prep(&channel->vc, &desc->vd, flags);
>  }
>
> @@ -535,27 +540,29 @@ rz_dmac_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
>  	int dma_length = 0;
>  	int i = 0;
>
> -	if (list_empty(&channel->ld_free))
> -		return NULL;
> +	scoped_guard(spinlock_irqsave, &channel->vc.lock) {
> +		if (list_empty(&channel->ld_free))
> +			return NULL;
>
> -	desc = list_first_entry(&channel->ld_free, struct rz_dmac_desc, node);
> +		desc = list_first_entry(&channel->ld_free, struct rz_dmac_desc, node);
>
> -	for_each_sg(sgl, sg, sg_len, i) {
> -		dma_length += sg_dma_len(sg);
> -	}
> +		for_each_sg(sgl, sg, sg_len, i)
> +			dma_length += sg_dma_len(sg);
>
> -	desc->type = RZ_DMAC_DESC_SLAVE_SG;
> -	desc->sg = sgl;
> -	desc->sgcount = sg_len;
> -	desc->len = dma_length;
> -	desc->direction = direction;
> +		desc->type = RZ_DMAC_DESC_SLAVE_SG;
> +		desc->sg = sgl;
> +		desc->sgcount = sg_len;
> +		desc->len = dma_length;
> +		desc->direction = direction;
>
> -	if (direction == DMA_DEV_TO_MEM)
> -		desc->src = channel->src_per_address;
> -	else
> -		desc->dest = channel->dst_per_address;
> +		if (direction == DMA_DEV_TO_MEM)
> +			desc->src = channel->src_per_address;
> +		else
> +			desc->dest = channel->dst_per_address;
> +
> +		list_move_tail(channel->ld_free.next, &channel->ld_queue);
> +	}
>
> -	list_move_tail(channel->ld_free.next, &channel->ld_queue);
>  	return vchan_tx_prep(&channel->vc, &desc->vd, flags);
>  }
>
> --
> 2.43.0
>