* [PATCH] dmaengine: imx-dma: fix wrong callback invoke
@ 2019-01-15 17:15 Leonid Iziumtsev
2019-01-20 10:52 ` Vinod Koul
2019-02-04 7:06 ` Vinod Koul
0 siblings, 2 replies; 4+ messages in thread
From: Leonid Iziumtsev @ 2019-01-15 17:15 UTC (permalink / raw)
To: dmaengine, Dan Williams, Vinod Koul, linux-kernel, m.grzeschik
Cc: Leonid Iziumtsev
Once the "ld_queue" list is not empty, next descriptor will migrate
into "ld_active" list. The "desc" variable will be overwritten
during that transition. And later the dmaengine_desc_get_callback_invoke()
will use it as an argument. As result we invoke wrong callback.
That behaviour was in place since:
commit fcaaba6c7136 ("dmaengine: imx-dma: fix callback path in tasklet").
But after commit 4cd13c21b207 ("softirq: Let ksoftirqd do its job")
things got worse, since possible delay between tasklet_schedule()
from DMA irq handler and actual tasklet function execution got bigger.
And that gave more time for new DMA request to be submitted and
to be put into "ld_queue" list.
It has been noticed that DMA issue is causing problems for "mxc-mmc"
driver. While stressing the system with heavy network traffic and
writing/reading to/from sd card simultaneously the timeout may happen:
10013000.sdhci: mxcmci_watchdog: read time out (status = 0x30004900)
That often lead to file system corruption.
Signed-off-by: Leonid Iziumtsev <leonid.iziumtsev@gmail.com>
---
drivers/dma/imx-dma.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/imx-dma.c b/drivers/dma/imx-dma.c
index c2fff3f6c9ca..4a09af3cd546 100644
--- a/drivers/dma/imx-dma.c
+++ b/drivers/dma/imx-dma.c
@@ -618,7 +618,7 @@ static void imxdma_tasklet(unsigned long data)
{
struct imxdma_channel *imxdmac = (void *)data;
struct imxdma_engine *imxdma = imxdmac->imxdma;
- struct imxdma_desc *desc;
+ struct imxdma_desc *desc, *next_desc;
unsigned long flags;
spin_lock_irqsave(&imxdma->lock, flags);
@@ -648,10 +648,10 @@ static void imxdma_tasklet(unsigned long data)
list_move_tail(imxdmac->ld_active.next, &imxdmac->ld_free);
if (!list_empty(&imxdmac->ld_queue)) {
- desc = list_first_entry(&imxdmac->ld_queue, struct imxdma_desc,
- node);
+ next_desc = list_first_entry(&imxdmac->ld_queue,
+ struct imxdma_desc, node);
list_move_tail(imxdmac->ld_queue.next, &imxdmac->ld_active);
- if (imxdma_xfer_desc(desc) < 0)
+ if (imxdma_xfer_desc(next_desc) < 0)
dev_warn(imxdma->dev, "%s: channel: %d couldn't xfer desc\n",
__func__, imxdmac->channel);
}
--
2.11.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] dmaengine: imx-dma: fix wrong callback invoke
2019-01-15 17:15 [PATCH] dmaengine: imx-dma: fix wrong callback invoke Leonid Iziumtsev
@ 2019-01-20 10:52 ` Vinod Koul
2019-01-23 11:43 ` Fabio Estevam
2019-02-04 7:06 ` Vinod Koul
1 sibling, 1 reply; 4+ messages in thread
From: Vinod Koul @ 2019-01-20 10:52 UTC (permalink / raw)
To: Leonid Iziumtsev, Fabio Estevam
Cc: dmaengine, Dan Williams, linux-kernel, m.grzeschik
On 15-01-19, 17:15, Leonid Iziumtsev wrote:
> Once the "ld_queue" list is not empty, next descriptor will migrate
> into "ld_active" list. The "desc" variable will be overwritten
> during that transition. And later the dmaengine_desc_get_callback_invoke()
> will use it as an argument. As result we invoke wrong callback.
>
> That behaviour was in place since:
> commit fcaaba6c7136 ("dmaengine: imx-dma: fix callback path in tasklet").
> But after commit 4cd13c21b207 ("softirq: Let ksoftirqd do its job")
> things got worse, since possible delay between tasklet_schedule()
> from DMA irq handler and actual tasklet function execution got bigger.
> And that gave more time for new DMA request to be submitted and
> to be put into "ld_queue" list.
>
> It has been noticed that DMA issue is causing problems for "mxc-mmc"
> driver. While stressing the system with heavy network traffic and
> writing/reading to/from sd card simultaneously the timeout may happen:
>
> 10013000.sdhci: mxcmci_watchdog: read time out (status = 0x30004900)
>
> That often lead to file system corruption.
This looks reasonable to me and I think should go to stable as well.
Fabio can we get some testing done on this patch
>
> Signed-off-by: Leonid Iziumtsev <leonid.iziumtsev@gmail.com>
> ---
> drivers/dma/imx-dma.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dma/imx-dma.c b/drivers/dma/imx-dma.c
> index c2fff3f6c9ca..4a09af3cd546 100644
> --- a/drivers/dma/imx-dma.c
> +++ b/drivers/dma/imx-dma.c
> @@ -618,7 +618,7 @@ static void imxdma_tasklet(unsigned long data)
> {
> struct imxdma_channel *imxdmac = (void *)data;
> struct imxdma_engine *imxdma = imxdmac->imxdma;
> - struct imxdma_desc *desc;
> + struct imxdma_desc *desc, *next_desc;
> unsigned long flags;
>
> spin_lock_irqsave(&imxdma->lock, flags);
> @@ -648,10 +648,10 @@ static void imxdma_tasklet(unsigned long data)
> list_move_tail(imxdmac->ld_active.next, &imxdmac->ld_free);
>
> if (!list_empty(&imxdmac->ld_queue)) {
> - desc = list_first_entry(&imxdmac->ld_queue, struct imxdma_desc,
> - node);
> + next_desc = list_first_entry(&imxdmac->ld_queue,
> + struct imxdma_desc, node);
> list_move_tail(imxdmac->ld_queue.next, &imxdmac->ld_active);
> - if (imxdma_xfer_desc(desc) < 0)
> + if (imxdma_xfer_desc(next_desc) < 0)
> dev_warn(imxdma->dev, "%s: channel: %d couldn't xfer desc\n",
> __func__, imxdmac->channel);
> }
> --
> 2.11.0
--
~Vinod
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] dmaengine: imx-dma: fix wrong callback invoke
2019-01-20 10:52 ` Vinod Koul
@ 2019-01-23 11:43 ` Fabio Estevam
0 siblings, 0 replies; 4+ messages in thread
From: Fabio Estevam @ 2019-01-23 11:43 UTC (permalink / raw)
To: Vinod Koul
Cc: Leonid Iziumtsev, dmaengine, Dan Williams, linux-kernel,
Michael Grzeschik
Hi Vinod,
On Sun, Jan 20, 2019 at 8:54 AM Vinod Koul <vkoul@kernel.org> wrote:
> This looks reasonable to me and I think should go to stable as well.
> Fabio can we get some testing done on this patch
I currently don't have access to a mx25pdk board. Will probably get
access to it next week.
Patch looks good though.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] dmaengine: imx-dma: fix wrong callback invoke
2019-01-15 17:15 [PATCH] dmaengine: imx-dma: fix wrong callback invoke Leonid Iziumtsev
2019-01-20 10:52 ` Vinod Koul
@ 2019-02-04 7:06 ` Vinod Koul
1 sibling, 0 replies; 4+ messages in thread
From: Vinod Koul @ 2019-02-04 7:06 UTC (permalink / raw)
To: Leonid Iziumtsev; +Cc: dmaengine, Dan Williams, linux-kernel, m.grzeschik
On 15-01-19, 17:15, Leonid Iziumtsev wrote:
> Once the "ld_queue" list is not empty, next descriptor will migrate
> into "ld_active" list. The "desc" variable will be overwritten
> during that transition. And later the dmaengine_desc_get_callback_invoke()
> will use it as an argument. As result we invoke wrong callback.
>
> That behaviour was in place since:
> commit fcaaba6c7136 ("dmaengine: imx-dma: fix callback path in tasklet").
> But after commit 4cd13c21b207 ("softirq: Let ksoftirqd do its job")
> things got worse, since possible delay between tasklet_schedule()
> from DMA irq handler and actual tasklet function execution got bigger.
> And that gave more time for new DMA request to be submitted and
> to be put into "ld_queue" list.
>
> It has been noticed that DMA issue is causing problems for "mxc-mmc"
> driver. While stressing the system with heavy network traffic and
> writing/reading to/from sd card simultaneously the timeout may happen:
>
> 10013000.sdhci: mxcmci_watchdog: read time out (status = 0x30004900)
>
> That often lead to file system corruption.
Applied and tagged to stable, thanks
--
~Vinod
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-02-04 7:07 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-15 17:15 [PATCH] dmaengine: imx-dma: fix wrong callback invoke Leonid Iziumtsev
2019-01-20 10:52 ` Vinod Koul
2019-01-23 11:43 ` Fabio Estevam
2019-02-04 7:06 ` 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).