Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754141AbdIHKOv (ORCPT ); Fri, 8 Sep 2017 06:14:51 -0400 Received: from mail-lf0-f66.google.com ([209.85.215.66]:38706 "EHLO mail-lf0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751894AbdIHKOt (ORCPT ); Fri, 8 Sep 2017 06:14:49 -0400 X-Google-Smtp-Source: AOwi7QAGevkc8WHM6sGd7gihnzy74JiEn8kBSLhyKcdrOssME9zPVohlE7SO9EX8jYx5a7WQ2ONWgw== From: Alexander Kochetkov To: Dan Williams , Vinod Koul , dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org Cc: Alexander Kochetkov Subject: [PATCH] dmaengine: pl330: fix descriptor allocation fail Date: Fri, 8 Sep 2017 13:00:26 +0300 Message-Id: <1504864826-14202-1-git-send-email-al.kochet@gmail.com> X-Mailer: git-send-email 1.7.9.5 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3544 Lines: 116 Thread A calls pl330_get_desc() to get descriptor. If DMAC descriptor pool is empty pl330_get_desc() allocates new descriptor using add_desc() and then get newly allocated descriptor using pluck_desc(). It is possible that another concurrent thread B calls pluck_desc() and catch newly allocated descriptor. In that case descriptor allocation for thread A will fail with: kernel: dma-pl330 20078000.dma-controller: pl330_get_desc:2469 ALERT! The commit fix the issue by calling _add_desc() to allocate new descriptor to the local on stack pool and than get it from local pool. So the issue described will nether happen. Signed-off-by: Alexander Kochetkov --- drivers/dma/pl330.c | 44 +++++++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c index f37f497..0e7f6c9 100644 --- a/drivers/dma/pl330.c +++ b/drivers/dma/pl330.c @@ -2417,7 +2417,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; @@ -2427,27 +2428,33 @@ 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 int add_desc(struct pl330_dmac *pl330, gfp_t flg, int count) +{ + return _add_desc(&pl330->desc_pool, &pl330->pool_lock, flg, count); +} + +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); @@ -2456,11 +2463,16 @@ 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; } +static struct dma_pl330_desc *pluck_desc(struct pl330_dmac *pl330) +{ + return _pluck_desc(&pl330->desc_pool, &pl330->pool_lock); +} + static struct dma_pl330_desc *pl330_get_desc(struct dma_pl330_chan *pch) { struct pl330_dmac *pl330 = pch->dmac; @@ -2472,16 +2484,14 @@ static struct dma_pl330_desc *pl330_get_desc(struct dma_pl330_chan *pch) /* 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 */ -- 1.7.9.5