From e588710311ee5bece284871d613418831d56f2bd Mon Sep 17 00:00:00 2001 From: Alexander Kochetkov Date: Wed, 4 Oct 2017 14:37:23 +0300 Subject: dmaengine: pl330: fix descriptor allocation fail If two concurrent threads call pl330_get_desc() when DMAC descriptor pool is empty it is possible that allocation for one of threads will fail with message: kernel: dma-pl330 20078000.dma-controller: pl330_get_desc:2469 ALERT! Here how that can happen. Thread A calls pl330_get_desc() to get descriptor. If DMAC descriptor pool is empty pl330_get_desc() allocates new descriptor on shared pool using add_desc() and then get newly allocated descriptor using pluck_desc(). At the same time thread B calls pluck_desc() and take newly allocated descriptor. In that case descriptor allocation for thread A will fail. Using on-stack pool for new descriptor allow avoid the issue described. The patch modify pl330_get_desc() to use on-stack pool for allocation new descriptors. Signed-off-by: Alexander Kochetkov Tested-by: Marek Szyprowski Signed-off-by: Vinod Koul --- drivers/dma/pl330.c | 39 ++++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-) (limited to 'drivers/dma/pl330.c') diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c index f122c2a7b9f0..d7327fd5f445 100644 --- a/drivers/dma/pl330.c +++ b/drivers/dma/pl330.c @@ -2390,7 +2390,8 @@ static inline void _init_desc(struct dma_pl330_desc *desc) } /* Returns the number of descriptors added to the DMAC pool */ -static int add_desc(struct pl330_dmac *pl330, gfp_t flg, int count) +static int add_desc(struct list_head *pool, spinlock_t *lock, + gfp_t flg, int count) { struct dma_pl330_desc *desc; unsigned long flags; @@ -2400,27 +2401,28 @@ static int add_desc(struct pl330_dmac *pl330, gfp_t flg, int count) if (!desc) return 0; - spin_lock_irqsave(&pl330->pool_lock, flags); + spin_lock_irqsave(lock, flags); for (i = 0; i < count; i++) { _init_desc(&desc[i]); - list_add_tail(&desc[i].node, &pl330->desc_pool); + list_add_tail(&desc[i].node, pool); } - spin_unlock_irqrestore(&pl330->pool_lock, flags); + spin_unlock_irqrestore(lock, flags); return count; } -static struct dma_pl330_desc *pluck_desc(struct pl330_dmac *pl330) +static struct dma_pl330_desc *pluck_desc(struct list_head *pool, + spinlock_t *lock) { struct dma_pl330_desc *desc = NULL; unsigned long flags; - spin_lock_irqsave(&pl330->pool_lock, flags); + spin_lock_irqsave(lock, flags); - if (!list_empty(&pl330->desc_pool)) { - desc = list_entry(pl330->desc_pool.next, + if (!list_empty(pool)) { + desc = list_entry(pool->next, struct dma_pl330_desc, node); list_del_init(&desc->node); @@ -2429,7 +2431,7 @@ static struct dma_pl330_desc *pluck_desc(struct pl330_dmac *pl330) desc->txd.callback = NULL; } - spin_unlock_irqrestore(&pl330->pool_lock, flags); + spin_unlock_irqrestore(lock, flags); return desc; } @@ -2441,20 +2443,18 @@ static struct dma_pl330_desc *pl330_get_desc(struct dma_pl330_chan *pch) struct dma_pl330_desc *desc; /* Pluck one desc from the pool of DMAC */ - desc = pluck_desc(pl330); + desc = pluck_desc(&pl330->desc_pool, &pl330->pool_lock); /* If the DMAC pool is empty, alloc new */ if (!desc) { - if (!add_desc(pl330, GFP_ATOMIC, 1)) - return NULL; + DEFINE_SPINLOCK(lock); + LIST_HEAD(pool); - /* Try again */ - desc = pluck_desc(pl330); - if (!desc) { - dev_err(pch->dmac->ddma.dev, - "%s:%d ALERT!\n", __func__, __LINE__); + if (!add_desc(&pool, &lock, GFP_ATOMIC, 1)) return NULL; - } + + desc = pluck_desc(&pool, &lock); + WARN_ON(!desc || !list_empty(&pool)); } /* Initialize the descriptor */ @@ -2868,7 +2868,8 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id) spin_lock_init(&pl330->pool_lock); /* Create a descriptor pool of default size */ - if (!add_desc(pl330, GFP_KERNEL, NR_DEFAULT_DESC)) + if (!add_desc(&pl330->desc_pool, &pl330->pool_lock, + GFP_KERNEL, NR_DEFAULT_DESC)) dev_warn(&adev->dev, "unable to allocate desc\n"); INIT_LIST_HEAD(&pd->channels); -- cgit v1.2.3-59-g8ed1b