Re: [PATCH 1/2] dmaengine: dma-axi-dmac: Defer freeing DMA descriptors

[Nuno Sá]

On Thu, Mar 26, 2026 at 01:37:35PM +0000, Nuno Sá wrote:
> From: Eliza Balas <eliza.balas@xxxxxxxxxx>
> 
> This IP core can be used in architectures (like Microblaze) where DMA
> descriptors are allocated with vmalloc(). Hence, given that freeing the
> descriptors happen in softirq context, vunmpap() will BUG().
> 
> To solve the above, we setup a work item during allocation of the
> descriptors and schedule in softirq context. Hence, the actual freeing
> happens in threaded context.
> 
> Signed-off-by: Eliza Balas <eliza.balas@xxxxxxxxxx>
> Signed-off-by: Nuno Sá <nuno.sa@xxxxxxxxxx>
> ---
>  drivers/dma/dma-axi-dmac.c | 48 +++++++++++++++++++++++++++++++++-------------
>  1 file changed, 35 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/dma/dma-axi-dmac.c b/drivers/dma/dma-axi-dmac.c
> index 45c2c8e4bc45..df2668064ea2 100644
> --- a/drivers/dma/dma-axi-dmac.c
> +++ b/drivers/dma/dma-axi-dmac.c
> @@ -133,6 +133,8 @@ struct axi_dmac_desc {
>  	struct virt_dma_desc vdesc;
>  	struct axi_dmac_chan *chan;
>  
> +	struct work_struct sched_work;

Ahh, just realized that workqueue.h needs to be included. Will wait for
some feedback before v2.

- Nuno Sá

> +
>  	bool cyclic;
>  	bool cyclic_eot;
>  	bool have_partial_xfer;
> @@ -650,6 +652,26 @@ static void axi_dmac_issue_pending(struct dma_chan *c)
>  	spin_unlock_irqrestore(&chan->vchan.lock, flags);
>  }
>  
> +static void axi_dmac_free_desc(struct axi_dmac_desc *desc)
> +{
> +	struct axi_dmac *dmac = chan_to_axi_dmac(desc->chan);
> +	struct device *dev = dmac->dma_dev.dev;
> +	struct axi_dmac_hw_desc *hw = desc->sg[0].hw;
> +	dma_addr_t hw_phys = desc->sg[0].hw_phys;
> +
> +	dma_free_coherent(dev, PAGE_ALIGN(desc->num_sgs * sizeof(*hw)),
> +			  hw, hw_phys);
> +	kfree(desc);
> +}
> +
> +static void axi_dmac_free_desc_schedule_work(struct work_struct *work)
> +{
> +	struct axi_dmac_desc *desc = container_of(work, struct axi_dmac_desc,
> +						  sched_work);
> +
> +	axi_dmac_free_desc(desc);
> +}
> +
>  static struct axi_dmac_desc *
>  axi_dmac_alloc_desc(struct axi_dmac_chan *chan, unsigned int num_sgs)
>  {
> @@ -687,21 +709,18 @@ axi_dmac_alloc_desc(struct axi_dmac_chan *chan, unsigned int num_sgs)
>  	/* The last hardware descriptor will trigger an interrupt */
>  	desc->sg[num_sgs - 1].hw->flags = AXI_DMAC_HW_FLAG_LAST | AXI_DMAC_HW_FLAG_IRQ;
>  
> +	/*
> +	 * We need to setup a work item because this IP can be used on archs
> +	 * that rely on vmalloced memory for descriptors. And given that freeing
> +	 * the descriptors happens in softirq context, vunmpap() will BUG().
> +	 * Hence, setup the worker so that we can queue it and free the
> +	 * descriptor in threaded context.
> +	 */
> +	INIT_WORK(&desc->sched_work, axi_dmac_free_desc_schedule_work);
> +
>  	return desc;
>  }
>  
> -static void axi_dmac_free_desc(struct axi_dmac_desc *desc)
> -{
> -	struct axi_dmac *dmac = chan_to_axi_dmac(desc->chan);
> -	struct device *dev = dmac->dma_dev.dev;
> -	struct axi_dmac_hw_desc *hw = desc->sg[0].hw;
> -	dma_addr_t hw_phys = desc->sg[0].hw_phys;
> -
> -	dma_free_coherent(dev, PAGE_ALIGN(desc->num_sgs * sizeof(*hw)),
> -			  hw, hw_phys);
> -	kfree(desc);
> -}
> -
>  static struct axi_dmac_sg *axi_dmac_fill_linear_sg(struct axi_dmac_chan *chan,
>  	enum dma_transfer_direction direction, dma_addr_t addr,
>  	unsigned int num_periods, unsigned int period_len,
> @@ -942,7 +961,10 @@ static void axi_dmac_free_chan_resources(struct dma_chan *c)
>  
>  static void axi_dmac_desc_free(struct virt_dma_desc *vdesc)
>  {
> -	axi_dmac_free_desc(to_axi_dmac_desc(vdesc));
> +	struct axi_dmac_desc *desc = to_axi_dmac_desc(vdesc);
> +
> +	/* See the comment in axi_dmac_alloc_desc() for the why! */
> +	schedule_work(&desc->sched_work);
>  }
>  
>  static bool axi_dmac_regmap_rdwr(struct device *dev, unsigned int reg)
> 
> -- 
> 2.53.0
>