dmaengine.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] dmaengine: sf-pdma: Fix an error that calls callback twice
@ 2020-08-27 13:27 Brad Kim
  0 siblings, 0 replies; 6+ messages in thread
From: Brad Kim @ 2020-08-27 13:27 UTC (permalink / raw)
  To: green.wan, koul; +Cc: dmaengine, Brad Kim

Because a callback is called twice when DMA transfer complete
the second callback may be possible to access a freed memory
if the first callback routines perform the dma_release_channel function.
So this patch serialized the callback functions

Signed-off-by: Brad Kim <brad.kim@semifive.com>
---
 drivers/dma/sf-pdma/sf-pdma.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/sf-pdma/sf-pdma.c b/drivers/dma/sf-pdma/sf-pdma.c
index 6e530dca6d9e..754994087e5f 100644
--- a/drivers/dma/sf-pdma/sf-pdma.c
+++ b/drivers/dma/sf-pdma/sf-pdma.c
@@ -295,7 +295,10 @@ static void sf_pdma_donebh_tasklet(unsigned long arg)
 	}
 	spin_unlock_irqrestore(&chan->lock, flags);
 
-	dmaengine_desc_get_callback_invoke(desc->async_tx, NULL);
+	spin_lock_irqsave(&chan->vchan.lock, flags);
+	list_del(&chan->desc->vdesc.node);
+	vchan_cookie_complete(&chan->desc->vdesc);
+	spin_unlock_irqrestore(&chan->vchan.lock, flags);
 }
 
 static void sf_pdma_errbh_tasklet(unsigned long arg)
@@ -332,8 +335,7 @@ static irqreturn_t sf_pdma_done_isr(int irq, void *dev_id)
 	residue = readq(regs->residue);
 
 	if (!residue) {
-		list_del(&chan->desc->vdesc.node);
-		vchan_cookie_complete(&chan->desc->vdesc);
+		tasklet_hi_schedule(&chan->done_tasklet);
 	} else {
 		/* submit next trascatioin if possible */
 		struct sf_pdma_desc *desc = chan->desc;
@@ -347,8 +349,6 @@ static irqreturn_t sf_pdma_done_isr(int irq, void *dev_id)
 
 	spin_unlock_irqrestore(&chan->vchan.lock, flags);
 
-	tasklet_hi_schedule(&chan->done_tasklet);
-
 	return IRQ_HANDLED;
 }
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] dmaengine: sf-pdma: Fix an error that calls callback twice
  2020-09-03 11:17 Brad Kim
@ 2020-09-11 12:10 ` Vinod Koul
  0 siblings, 0 replies; 6+ messages in thread
From: Vinod Koul @ 2020-09-11 12:10 UTC (permalink / raw)
  To: Brad Kim; +Cc: green.wan, dmaengine, Brad Kim

On 03-09-20, 20:17, Brad Kim wrote:
> Because a callback is called twice when DMA transfer complete
> the second callback may be possible to access a freed memory
> if the first callback routines perform the dma_release_channel function.
> So this patch serialized the callback functions

Applied, thanks

-- 
~Vinod

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] dmaengine: sf-pdma: Fix an error that calls callback twice
@ 2020-09-03 11:17 Brad Kim
  2020-09-11 12:10 ` Vinod Koul
  0 siblings, 1 reply; 6+ messages in thread
From: Brad Kim @ 2020-09-03 11:17 UTC (permalink / raw)
  To: vkoul, green.wan; +Cc: dmaengine, Brad Kim, Brad Kim

Because a callback is called twice when DMA transfer complete
the second callback may be possible to access a freed memory
if the first callback routines perform the dma_release_channel function.
So this patch serialized the callback functions

Signed-off-by: Brad Kim <brad.kim@semifive.com>
Tested-and-Reviewed-by: Green Wan <green.wan@sifive.com>
Signed-off-by: Brad Kim <brad.kim@sifive.com>
---
 drivers/dma/sf-pdma/sf-pdma.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/sf-pdma/sf-pdma.c b/drivers/dma/sf-pdma/sf-pdma.c
index 6e530dca6d9e..754994087e5f 100644
--- a/drivers/dma/sf-pdma/sf-pdma.c
+++ b/drivers/dma/sf-pdma/sf-pdma.c
@@ -295,7 +295,10 @@ static void sf_pdma_donebh_tasklet(unsigned long arg)
 	}
 	spin_unlock_irqrestore(&chan->lock, flags);
 
-	dmaengine_desc_get_callback_invoke(desc->async_tx, NULL);
+	spin_lock_irqsave(&chan->vchan.lock, flags);
+	list_del(&chan->desc->vdesc.node);
+	vchan_cookie_complete(&chan->desc->vdesc);
+	spin_unlock_irqrestore(&chan->vchan.lock, flags);
 }
 
 static void sf_pdma_errbh_tasklet(unsigned long arg)
@@ -332,8 +335,7 @@ static irqreturn_t sf_pdma_done_isr(int irq, void *dev_id)
 	residue = readq(regs->residue);
 
 	if (!residue) {
-		list_del(&chan->desc->vdesc.node);
-		vchan_cookie_complete(&chan->desc->vdesc);
+		tasklet_hi_schedule(&chan->done_tasklet);
 	} else {
 		/* submit next trascatioin if possible */
 		struct sf_pdma_desc *desc = chan->desc;
@@ -347,8 +349,6 @@ static irqreturn_t sf_pdma_done_isr(int irq, void *dev_id)
 
 	spin_unlock_irqrestore(&chan->vchan.lock, flags);
 
-	tasklet_hi_schedule(&chan->done_tasklet);
-
 	return IRQ_HANDLED;
 }
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] dmaengine: sf-pdma: Fix an error that calls callback twice
  2020-08-27 13:33 Brad Kim
       [not found] ` <CAJivOr4iT-GZ_CcAHRPp9rjZbOaBXcX6j4KGQH=XuGoArZSnsg@mail.gmail.com>
@ 2020-09-03  6:22 ` Vinod Koul
  1 sibling, 0 replies; 6+ messages in thread
From: Vinod Koul @ 2020-09-03  6:22 UTC (permalink / raw)
  To: Brad Kim; +Cc: green.wan, dmaengine, Brad Kim

Hello Brad,

On 27-08-20, 22:33, Brad Kim wrote:
> Because a callback is called twice when DMA transfer complete
> the second callback may be possible to access a freed memory
> if the first callback routines perform the dma_release_channel function.
> So this patch serialized the callback functions
> 
> Signed-off-by: Brad Kim <brad.kim@semifive.com>

The patch came from Brad Kim <brad.kim@sifive.com> but was s-o-b brad.kim@semifive.com

I am not sure which one is correct but DCO clearly states that patch
author Brad Kim <brad.kim@sifive.com> should sign off! You can add
additional sign offs but that one is mandatory!

Thanks

-- 
~Vinod

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] dmaengine: sf-pdma: Fix an error that calls callback twice
       [not found] ` <CAJivOr4iT-GZ_CcAHRPp9rjZbOaBXcX6j4KGQH=XuGoArZSnsg@mail.gmail.com>
@ 2020-09-01  3:04   ` Green Wan
  0 siblings, 0 replies; 6+ messages in thread
From: Green Wan @ 2020-09-01  3:04 UTC (permalink / raw)
  To: Brad Kim; +Cc: Vinod Koul, dmaengine, Brad Kim

Looks good to me.

Tested-and-Reviewed-by: Green Wan <green.wan@sifive.com>


On Tue, Sep 1, 2020 at 9:43 AM Green Wan <green.wan@sifive.com> wrote:
>
> Looks good to me.
>
> Tested-and-Reviewed-by: Green Wan <green.wan@sifive.com>
>
> On Thu, Aug 27, 2020 at 9:33 PM Brad Kim <brad.kim@sifive.com> wrote:
>>
>> Because a callback is called twice when DMA transfer complete
>> the second callback may be possible to access a freed memory
>> if the first callback routines perform the dma_release_channel function.
>> So this patch serialized the callback functions
>>
>> Signed-off-by: Brad Kim <brad.kim@semifive.com>
>> ---
>>  drivers/dma/sf-pdma/sf-pdma.c | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/dma/sf-pdma/sf-pdma.c b/drivers/dma/sf-pdma/sf-pdma.c
>> index 6e530dca6d9e..754994087e5f 100644
>> --- a/drivers/dma/sf-pdma/sf-pdma.c
>> +++ b/drivers/dma/sf-pdma/sf-pdma.c
>> @@ -295,7 +295,10 @@ static void sf_pdma_donebh_tasklet(unsigned long arg)
>>         }
>>         spin_unlock_irqrestore(&chan->lock, flags);
>>
>> -       dmaengine_desc_get_callback_invoke(desc->async_tx, NULL);
>> +       spin_lock_irqsave(&chan->vchan.lock, flags);
>> +       list_del(&chan->desc->vdesc.node);
>> +       vchan_cookie_complete(&chan->desc->vdesc);
>> +       spin_unlock_irqrestore(&chan->vchan.lock, flags);
>>  }
>>
>>  static void sf_pdma_errbh_tasklet(unsigned long arg)
>> @@ -332,8 +335,7 @@ static irqreturn_t sf_pdma_done_isr(int irq, void *dev_id)
>>         residue = readq(regs->residue);
>>
>>         if (!residue) {
>> -               list_del(&chan->desc->vdesc.node);
>> -               vchan_cookie_complete(&chan->desc->vdesc);
>> +               tasklet_hi_schedule(&chan->done_tasklet);
>>         } else {
>>                 /* submit next trascatioin if possible */
>>                 struct sf_pdma_desc *desc = chan->desc;
>> @@ -347,8 +349,6 @@ static irqreturn_t sf_pdma_done_isr(int irq, void *dev_id)
>>
>>         spin_unlock_irqrestore(&chan->vchan.lock, flags);
>>
>> -       tasklet_hi_schedule(&chan->done_tasklet);
>> -
>>         return IRQ_HANDLED;
>>  }
>>
>> --
>> 2.17.1
>>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] dmaengine: sf-pdma: Fix an error that calls callback twice
@ 2020-08-27 13:33 Brad Kim
       [not found] ` <CAJivOr4iT-GZ_CcAHRPp9rjZbOaBXcX6j4KGQH=XuGoArZSnsg@mail.gmail.com>
  2020-09-03  6:22 ` Vinod Koul
  0 siblings, 2 replies; 6+ messages in thread
From: Brad Kim @ 2020-08-27 13:33 UTC (permalink / raw)
  To: green.wan, vkoul; +Cc: dmaengine, Brad Kim

Because a callback is called twice when DMA transfer complete
the second callback may be possible to access a freed memory
if the first callback routines perform the dma_release_channel function.
So this patch serialized the callback functions

Signed-off-by: Brad Kim <brad.kim@semifive.com>
---
 drivers/dma/sf-pdma/sf-pdma.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/sf-pdma/sf-pdma.c b/drivers/dma/sf-pdma/sf-pdma.c
index 6e530dca6d9e..754994087e5f 100644
--- a/drivers/dma/sf-pdma/sf-pdma.c
+++ b/drivers/dma/sf-pdma/sf-pdma.c
@@ -295,7 +295,10 @@ static void sf_pdma_donebh_tasklet(unsigned long arg)
 	}
 	spin_unlock_irqrestore(&chan->lock, flags);
 
-	dmaengine_desc_get_callback_invoke(desc->async_tx, NULL);
+	spin_lock_irqsave(&chan->vchan.lock, flags);
+	list_del(&chan->desc->vdesc.node);
+	vchan_cookie_complete(&chan->desc->vdesc);
+	spin_unlock_irqrestore(&chan->vchan.lock, flags);
 }
 
 static void sf_pdma_errbh_tasklet(unsigned long arg)
@@ -332,8 +335,7 @@ static irqreturn_t sf_pdma_done_isr(int irq, void *dev_id)
 	residue = readq(regs->residue);
 
 	if (!residue) {
-		list_del(&chan->desc->vdesc.node);
-		vchan_cookie_complete(&chan->desc->vdesc);
+		tasklet_hi_schedule(&chan->done_tasklet);
 	} else {
 		/* submit next trascatioin if possible */
 		struct sf_pdma_desc *desc = chan->desc;
@@ -347,8 +349,6 @@ static irqreturn_t sf_pdma_done_isr(int irq, void *dev_id)
 
 	spin_unlock_irqrestore(&chan->vchan.lock, flags);
 
-	tasklet_hi_schedule(&chan->done_tasklet);
-
 	return IRQ_HANDLED;
 }
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-09-11 12:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-27 13:27 [PATCH] dmaengine: sf-pdma: Fix an error that calls callback twice Brad Kim
2020-08-27 13:33 Brad Kim
     [not found] ` <CAJivOr4iT-GZ_CcAHRPp9rjZbOaBXcX6j4KGQH=XuGoArZSnsg@mail.gmail.com>
2020-09-01  3:04   ` Green Wan
2020-09-03  6:22 ` Vinod Koul
2020-09-03 11:17 Brad Kim
2020-09-11 12:10 ` Vinod Koul

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).