Re: [PATCH 2/6] dmaengine: virt-dma: Add missing locking around list operations

[Peter Ujfalusi <peter.ujfalusi@ti.com>]

On 10/12/2019 14.33, Sascha Hauer wrote:
> All list operations are protected by &vc->lock. As vchan_vdesc_fini()
> is called unlocked add the missing locking around the list operations.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/dma/virt-dma.h | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/dma/virt-dma.h b/drivers/dma/virt-dma.h
> index e213137b6bc1..e9f5250fbe4d 100644
> --- a/drivers/dma/virt-dma.h
> +++ b/drivers/dma/virt-dma.h
> @@ -113,10 +113,15 @@ static inline void vchan_vdesc_fini(struct virt_dma_desc *vd)
>  {
>  	struct virt_dma_chan *vc = to_virt_chan(vd->tx.chan);
>  
> -	if (dmaengine_desc_test_reuse(&vd->tx))
> +	if (dmaengine_desc_test_reuse(&vd->tx)) {
> +		unsigned long flags;
> +
> +		spin_lock_irqsave(&vc->lock, flags);
>  		list_add(&vd->node, &vc->desc_allocated);
> -	else
> +		spin_unlock_irqrestore(&vc->lock, flags);
> +	} else {

If we add:
		list_del(&vd->node);

here, then the list_del() can be removed from vchan_complete() before
vchan_vdesc_fini() is called and as a plus the
vchan_dma_desc_free_list() can be as simple as:

list_for_each_entry_safe(vd, _vd, head, node)
	vchan_vdesc_fini(vd);

But only if we don't care about the debug print in there, if we care
then the vchan_synchronize() could use the very same simple loop to free
up only the descriptors from the desc_terminated list.


>  		vc->desc_free(vd);
> +	}
>  }
>  
>  /**
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki