lkml.org 
[lkml]   [2022]   [Jun]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 3/8] dmaengine: hisilicon: Add multi-thread support for a DMA channel
From

在 2022/6/27 14:51, - 写道:
> -----Original Message-----
> From: Vinod Koul [mailto:vkoul@kernel.org]
> Sent: Monday, June 27, 2022 2:21 PM
> To: haijie <haijie1@huawei.com>
> Cc: Wangzhou (B) <wangzhou1@hisilicon.com>; dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 3/8] dmaengine: hisilicon: Add multi-thread support for a DMA channel
>
> On 25-06-22, 15:44, Jie Hai wrote:
>> When we get a DMA channel and try to use it in multiple threads it
>> will cause oops and hanging the system.
>>
>> % echo 100 > /sys/module/dmatest/parameters/threads_per_chan
>> % echo 100 > /sys/module/dmatest/parameters/iterations
>> % echo 1 > /sys/module/dmatest/parameters/run
>> [383493.327077] Unable to handle kernel paging request at virtual
>> address dead000000000108
>> [383493.335103] Mem abort info:
>> [383493.335103] ESR = 0x96000044
>> [383493.335105] EC = 0x25: DABT (current EL), IL = 32 bits
>> [383493.335107] SET = 0, FnV = 0
>> [383493.335108] EA = 0, S1PTW = 0
>> [383493.335109] FSC = 0x04: level 0 translation fault
>> [383493.335110] Data abort info:
>> [383493.335111] ISV = 0, ISS = 0x00000044
>> [383493.364739] CM = 0, WnR = 1
>> [383493.367793] [dead000000000108] address between user and kernel
>> address ranges
>> [383493.375021] Internal error: Oops: 96000044 [#1] PREEMPT SMP
>> [383493.437574] CPU: 63 PID: 27895 Comm: dma0chan0-copy2 Kdump:
>> loaded Tainted: GO 5.17.0-rc4+ #2
>> [383493.457851] pstate: 204000c9 (nzCv daIF +PAN -UAO -TCO -DIT
>> -SSBS BTYPE=--)
>> [383493.465331] pc : vchan_tx_submit+0x64/0xa0 [383493.469957] lr :
>> vchan_tx_submit+0x34/0xa0
>>
>> This happens because of data race. Each thread rewrite channels's
>> descriptor as soon as device_issue_pending is called. It leads to the
>> situation that the driver thinks that it uses the right descriptor in
>> interrupt handler while channels's descriptor has been changed by
>> other thread.
>>
>> With current fixes channels's descriptor changes it's value only when
>> it has been used. A new descriptor is acquired from
>> vc->desc_issued queue that is already filled with descriptors
>> that are ready to be sent. Threads have no direct access to DMA
>> channel descriptor. Now it is just possible to queue a descriptor for
>> further processing.
>>
>> Fixes: e9f08b65250d ("dmaengine: hisilicon: Add Kunpeng DMA engine
>> support")
>> Signed-off-by: Jie Hai <haijie1@huawei.com>
>> ---
>> drivers/dma/hisi_dma.c | 6 ++----
>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/dma/hisi_dma.c b/drivers/dma/hisi_dma.c index
>> 0a0f8a4d168a..0385419be8d5 100644
>> --- a/drivers/dma/hisi_dma.c
>> +++ b/drivers/dma/hisi_dma.c
>> @@ -271,7 +271,6 @@ static void hisi_dma_start_transfer(struct
>> hisi_dma_chan *chan)
>>
>> vd = vchan_next_desc(&chan->vc);
>> if (!vd) {
>> - dev_err(&hdma_dev->pdev->dev, "no issued task!\n");
> how is this a fix?

> With current fixes, hisi_dma_start_transfer may be called twice for one desc.

> If channel's desc is NULL, When hisi_dma_issue_pending
>> chan->desc = NULL;
>> return;
>> }
>> @@ -303,7 +302,7 @@ static void hisi_dma_issue_pending(struct dma_chan
>> *c)
>>
>> spin_lock_irqsave(&chan->vc.lock, flags);
>>
>> - if (vchan_issue_pending(&chan->vc))
>> + if (vchan_issue_pending(&chan->vc) && !chan->desc)
> This looks good
>
>> hisi_dma_start_transfer(chan);
>>
>> spin_unlock_irqrestore(&chan->vc.lock, flags); @@ -442,11 +441,10 @@
>> static irqreturn_t hisi_dma_irq(int irq, void *data)
>> chan->qp_num, chan->cq_head);
>> if (FIELD_GET(STATUS_MASK, cqe->w0) == STATUS_SUCC) {
>> vchan_cookie_complete(&desc->vd);
>> + hisi_dma_start_transfer(chan);
> Why should this fix the error reported?
>
>> } else {
>> dev_err(&hdma_dev->pdev->dev, "task error!\n");
>> }
>> -
>> - chan->desc = NULL;
>> }
>>
>> spin_unlock(&chan->vc.lock);
>> --
>> 2.33.0
> --
> ~Vinod
> .

\
 
 \ /
  Last update: 2022-06-27 15:18    [W:0.193 / U:0.204 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site