From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933594AbdJQL0q (ORCPT ); Tue, 17 Oct 2017 07:26:46 -0400 Received: from mailout1.w1.samsung.com ([210.118.77.11]:41432 "EHLO mailout1.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932612AbdJQL0h (ORCPT ); Tue, 17 Oct 2017 07:26:37 -0400 X-AuditID: cbfec7f1-f793a6d00000326b-aa-59e5e8e9ba9b Subject: Re: [PATCH v2 1/2] dmaengine: pl330: fix descriptor allocation fail To: Alexander Kochetkov , dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org Cc: Dan Williams , Vinod Koul , Krzysztof Kozlowski From: Marek Szyprowski Message-id: <7c3c9213-f4e0-327b-cc5c-e883079f1cb0@samsung.com> Date: Tue, 17 Oct 2017 13:26:31 +0200 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-version: 1.0 In-reply-to: <1507117044-3456-2-git-send-email-al.kochet@gmail.com> Content-type: text/plain; charset="utf-8"; format="flowed" Content-transfer-encoding: 7bit Content-language: en-US X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprEKsWRmVeSWpSXmKPExsWy7djP87ovXzyNNDh1QsFiRb+TxfSpFxgt Vk/9y2px/vwGdovLu+awWbzs28/iwOaxc9Zddo/Fe14yeWxa1cnm8XmTXABLFJdNSmpOZllq kb5dAlfGqv4+5oItyhXLvyxkbmBsleli5OSQEDCR6Dt4kh3CFpO4cG89WxcjF4eQwFJGicPr 9zBCOJ8ZJY6e6mWC6dh49CxUYhmjxORLi1lAEkICzxklFmyTArGFBXwktj5oZQaxRQQyJO7u bgWrYRYol7h87S/YIDYBQ4mut11sIDavgJ3Es3uPwGwWAVWJ5u3PwHpFBWIkVi/ewQJRIyjx Y/I9MJtTwFni4c2zzBAzrSSe/WtlhbDlJTaveQsVF5dobr3JAnKohMABNom2i1+gPnCReLBr CguELSzx6vgWqP9lJDo7DkLV9DNKNLVqQ9gzGCXOveWFsK0lDh+/CLWMT2LStulAyziA4rwS HW1CECUeEktarjNC2I4SJ2Z/ZYIE1nlgKO5axTqBUX4Wkn9mIflhFpIfZiH5YQEjyypGkdTS 4tz01GIjveLE3OLSvHS95PzcTYzAlHL63/GPOxjfn7A6xCjAwajEw3vgyJNIIdbEsuLK3EOM EhzMSiK8u249jRTiTUmsrEotyo8vKs1JLT7EKM3BoiTOaxvVFikkkJ5YkpqdmlqQWgSTZeLg lGpgvH7g/Kp3Nc2xFucMV3y7L3TwxHG9HrPcyX7XNsz7+iw2bmMnV3qI1tz0Y/cT361YqneS q27N7+BPe9rkKy5cmt+/MbR7e4JYtTDr51rhL5+798n+2ttZscPp8VLnHfUqiZ3LT50VD762 9bq/+tTCbWcMtgtmL3v7S+vOLqGFumvMqpc1BzVHL1ViKc5INNRiLipOBAAKDEV0JQMAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrBLMWRmVeSWpSXmKPExsVy+t/xa7ovXjyNNDjz0NRiRb+TxfSpFxgt Vk/9y2px/vwGdovLu+awWbzs28/iwOaxc9Zddo/Fe14yeWxa1cnm8XmTXABLFJdNSmpOZllq kb5dAlfGqv4+5oItyhXLvyxkbmBsleli5OSQEDCR2Hj0LCOELSZx4d56NhBbSGAJo8T8rZVd jFxA9nNGiTfzf7GAJIQFfCS2PmhlBrFFBDIkzqxawgpiMwuUS0w+N4kVouE8o8S0zo3sIAk2 AUOJrrddYFN5Bewknt17BGazCKhKNG9/BjZIVCBGYuKDi4wQNYISPybfA1vGKeAs8fDmWWaI BWYSX14ehlomL7F5zVuouLhEc+tNlgmMgrOQtM9C0jILScssJC0LGFlWMYqklhbnpucWG+oV J+YWl+al6yXn525iBIb/tmM/N+9gvLQx+BCjAAejEg9vxqEnkUKsiWXFlbmHGCU4mJVEeHfd ehopxJuSWFmVWpQfX1Sak1p8iFGag0VJnLd3z+pIIYH0xJLU7NTUgtQimCwTB6dUA2O/IPvW HdbKHz0vSrD4blv3epPlm/Cj/62msN+9xPWzv+qV7+RbHmt+Cx3+JhG//hYri11pH4MhqxP3 zoNT5nKL9U9+ti91wT/Th/2pr9VDtnzZ6Hyf7fW9mGOHjr7Q5q29eDFQjkXmmmXHvltc/5Su WgrbOjWdvrKWtzPAfhbLVsnGWZ2HZjYqsRRnJBpqMRcVJwIA0mqASHsCAAA= X-CMS-MailID: 20171017112632eucas1p145f5d2a599583c5d85e564fa946c903e X-Msg-Generator: CA X-Sender-IP: 182.198.249.179 X-Local-Sender: =?UTF-8?B?TWFyZWsgU3p5cHJvd3NraRtTUlBPTC1LZXJuZWwgKFRQKRs=?= =?UTF-8?B?7IK87ISx7KCE7J6QG1NlbmlvciBTb2Z0d2FyZSBFbmdpbmVlcg==?= X-Global-Sender: =?UTF-8?B?TWFyZWsgU3p5cHJvd3NraRtTUlBPTC1LZXJuZWwgKFRQKRtT?= =?UTF-8?B?YW1zdW5nIEVsZWN0cm9uaWNzG1NlbmlvciBTb2Z0d2FyZSBFbmdpbmVlcg==?= X-Sender-Code: =?UTF-8?B?QzEwG0VIURtDMTBDRDAyQ0QwMjczOTI=?= CMS-TYPE: 201P X-CMS-RootMailID: 20171004113733epcas2p2f20bac3551453826ac42867a0f597b08 X-RootMTR: 20171004113733epcas2p2f20bac3551453826ac42867a0f597b08 References: <1507117044-3456-1-git-send-email-al.kochet@gmail.com> <1507117044-3456-2-git-send-email-al.kochet@gmail.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Alexander, It took me a while to test this patch, I've been busy with other stuff... On 2017-10-04 13:37, Alexander Kochetkov wrote: > 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 > --- > drivers/dma/pl330.c | 39 ++++++++++++++++++++------------------- > 1 file changed, 20 insertions(+), 19 deletions(-) > > diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c > index b19ee04..deec4a4 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 @@ static int __maybe_unused pl330_resume(struct device *dev) > 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); Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland