2020-02-21 08:08:26

by Greg KH

[permalink] [raw]
Subject: [PATCH 5.4 160/344] dmaengine: imx-sdma: Fix memory leak

From: Sascha Hauer <[email protected]>

[ Upstream commit 02939cd167095f16328a1bd5cab5a90b550606df ]

The current descriptor is not on any list of the virtual DMA channel.
Once sdma_terminate_all() is called when a descriptor is currently
in flight then this one is forgotten to be freed. We have to call
vchan_terminate_vdesc() on this descriptor to re-add it to the lists.
Now that we also free the currently running descriptor we can (and
actually have to) remove the current descriptor from its list also
for the cyclic case.

Signed-off-by: Sascha Hauer <[email protected]>
Reviewed-by: Robin Gong <[email protected]>
Tested-by: Robin Gong <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
drivers/dma/imx-sdma.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index c27e206a764c3..66f1b2ac5cde4 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -760,12 +760,8 @@ static void sdma_start_desc(struct sdma_channel *sdmac)
return;
}
sdmac->desc = desc = to_sdma_desc(&vd->tx);
- /*
- * Do not delete the node in desc_issued list in cyclic mode, otherwise
- * the desc allocated will never be freed in vchan_dma_desc_free_list
- */
- if (!(sdmac->flags & IMX_DMA_SG_LOOP))
- list_del(&vd->node);
+
+ list_del(&vd->node);

sdma->channel_control[channel].base_bd_ptr = desc->bd_phys;
sdma->channel_control[channel].current_bd_ptr = desc->bd_phys;
@@ -1071,7 +1067,6 @@ static void sdma_channel_terminate_work(struct work_struct *work)

spin_lock_irqsave(&sdmac->vc.lock, flags);
vchan_get_all_descriptors(&sdmac->vc, &head);
- sdmac->desc = NULL;
spin_unlock_irqrestore(&sdmac->vc.lock, flags);
vchan_dma_desc_free_list(&sdmac->vc, &head);
sdmac->context_loaded = false;
@@ -1080,11 +1075,19 @@ static void sdma_channel_terminate_work(struct work_struct *work)
static int sdma_disable_channel_async(struct dma_chan *chan)
{
struct sdma_channel *sdmac = to_sdma_chan(chan);
+ unsigned long flags;
+
+ spin_lock_irqsave(&sdmac->vc.lock, flags);

sdma_disable_channel(chan);

- if (sdmac->desc)
+ if (sdmac->desc) {
+ vchan_terminate_vdesc(&sdmac->desc->vd);
+ sdmac->desc = NULL;
schedule_work(&sdmac->terminate_worker);
+ }
+
+ spin_unlock_irqrestore(&sdmac->vc.lock, flags);

return 0;
}
--
2.20.1




2020-02-24 13:24:34

by Andreas Tobler

[permalink] [raw]
Subject: Re: [PATCH 5.4 160/344] dmaengine: imx-sdma: Fix memory leak

Hi all,

On 21.02.20 08:39, Greg Kroah-Hartman wrote:
> From: Sascha Hauer <[email protected]>
>
> [ Upstream commit 02939cd167095f16328a1bd5cab5a90b550606df ]
>
> The current descriptor is not on any list of the virtual DMA channel.
> Once sdma_terminate_all() is called when a descriptor is currently
> in flight then this one is forgotten to be freed. We have to call
> vchan_terminate_vdesc() on this descriptor to re-add it to the lists.
> Now that we also free the currently running descriptor we can (and
> actually have to) remove the current descriptor from its list also
> for the cyclic case.
>
> Signed-off-by: Sascha Hauer <[email protected]>
> Reviewed-by: Robin Gong <[email protected]>
> Tested-by: Robin Gong <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]
> Signed-off-by: Vinod Koul <[email protected]>
> Signed-off-by: Sasha Levin <[email protected]>
> ---
> drivers/dma/imx-sdma.c | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index c27e206a764c3..66f1b2ac5cde4 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -760,12 +760,8 @@ static void sdma_start_desc(struct sdma_channel *sdmac)
> return;
> }
> sdmac->desc = desc = to_sdma_desc(&vd->tx);
> - /*
> - * Do not delete the node in desc_issued list in cyclic mode, otherwise
> - * the desc allocated will never be freed in vchan_dma_desc_free_list
> - */
> - if (!(sdmac->flags & IMX_DMA_SG_LOOP))
> - list_del(&vd->node);
> +
> + list_del(&vd->node);
>
> sdma->channel_control[channel].base_bd_ptr = desc->bd_phys;
> sdma->channel_control[channel].current_bd_ptr = desc->bd_phys;
> @@ -1071,7 +1067,6 @@ static void sdma_channel_terminate_work(struct work_struct *work)
>
> spin_lock_irqsave(&sdmac->vc.lock, flags);
> vchan_get_all_descriptors(&sdmac->vc, &head);
> - sdmac->desc = NULL;
> spin_unlock_irqrestore(&sdmac->vc.lock, flags);
> vchan_dma_desc_free_list(&sdmac->vc, &head);
> sdmac->context_loaded = false;
> @@ -1080,11 +1075,19 @@ static void sdma_channel_terminate_work(struct work_struct *work)
> static int sdma_disable_channel_async(struct dma_chan *chan)
> {
> struct sdma_channel *sdmac = to_sdma_chan(chan);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&sdmac->vc.lock, flags);
>
> sdma_disable_channel(chan);
>
> - if (sdmac->desc)
> + if (sdmac->desc) {
> + vchan_terminate_vdesc(&sdmac->desc->vd);
> + sdmac->desc = NULL;
> schedule_work(&sdmac->terminate_worker);
> + }
> +
> + spin_unlock_irqrestore(&sdmac->vc.lock, flags);
>
> return 0;
> }
>

This patch breaks our imx6 board with the attached trace. Reverting the
patch makes it boot again.
I tried also 5.6-rc3 and it booted too. A closer look into imx-sdma.c
from 5.6-rc3 showed me some details which might have to be backported as
well to make this patch work.
I tried a1ff6a07f5a3951fcac84f064a76d1ad79c10e40 and was somehow
successful. I still have one trace but the board boots now.

Any insights from the experts?
TIA,
Andreas






Attachments:
trace_uart_sdma.txt (2.03 kB)
trace_uart_sdma.txt

2020-02-24 14:59:08

by Sascha Hauer

[permalink] [raw]
Subject: Re: [PATCH 5.4 160/344] dmaengine: imx-sdma: Fix memory leak

On Mon, Feb 24, 2020 at 01:24:04PM +0000, Andreas Tobler wrote:
> Hi all,
>
> On 21.02.20 08:39, Greg Kroah-Hartman wrote:
> > From: Sascha Hauer <[email protected]>
> >
> > [ Upstream commit 02939cd167095f16328a1bd5cab5a90b550606df ]
> >
> > The current descriptor is not on any list of the virtual DMA channel.
> > Once sdma_terminate_all() is called when a descriptor is currently
> > in flight then this one is forgotten to be freed. We have to call
> > vchan_terminate_vdesc() on this descriptor to re-add it to the lists.
> > Now that we also free the currently running descriptor we can (and
> > actually have to) remove the current descriptor from its list also
> > for the cyclic case.
> >
> > Signed-off-by: Sascha Hauer <[email protected]>
> > Reviewed-by: Robin Gong <[email protected]>
> > Tested-by: Robin Gong <[email protected]>
> > Link: https://lore.kernel.org/r/[email protected]
> > Signed-off-by: Vinod Koul <[email protected]>
> > Signed-off-by: Sasha Levin <[email protected]>
> > ---
> > drivers/dma/imx-sdma.c | 19 +++++++++++--------
> > 1 file changed, 11 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> > index c27e206a764c3..66f1b2ac5cde4 100644
> > --- a/drivers/dma/imx-sdma.c
> > +++ b/drivers/dma/imx-sdma.c
> > @@ -760,12 +760,8 @@ static void sdma_start_desc(struct sdma_channel *sdmac)
> > return;
> > }
> > sdmac->desc = desc = to_sdma_desc(&vd->tx);
> > - /*
> > - * Do not delete the node in desc_issued list in cyclic mode, otherwise
> > - * the desc allocated will never be freed in vchan_dma_desc_free_list
> > - */
> > - if (!(sdmac->flags & IMX_DMA_SG_LOOP))
> > - list_del(&vd->node);
> > +
> > + list_del(&vd->node);
> >
> > sdma->channel_control[channel].base_bd_ptr = desc->bd_phys;
> > sdma->channel_control[channel].current_bd_ptr = desc->bd_phys;
> > @@ -1071,7 +1067,6 @@ static void sdma_channel_terminate_work(struct work_struct *work)
> >
> > spin_lock_irqsave(&sdmac->vc.lock, flags);
> > vchan_get_all_descriptors(&sdmac->vc, &head);
> > - sdmac->desc = NULL;
> > spin_unlock_irqrestore(&sdmac->vc.lock, flags);
> > vchan_dma_desc_free_list(&sdmac->vc, &head);
> > sdmac->context_loaded = false;
> > @@ -1080,11 +1075,19 @@ static void sdma_channel_terminate_work(struct work_struct *work)
> > static int sdma_disable_channel_async(struct dma_chan *chan)
> > {
> > struct sdma_channel *sdmac = to_sdma_chan(chan);
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&sdmac->vc.lock, flags);
> >
> > sdma_disable_channel(chan);
> >
> > - if (sdmac->desc)
> > + if (sdmac->desc) {
> > + vchan_terminate_vdesc(&sdmac->desc->vd);
> > + sdmac->desc = NULL;
> > schedule_work(&sdmac->terminate_worker);
> > + }
> > +
> > + spin_unlock_irqrestore(&sdmac->vc.lock, flags);
> >
> > return 0;
> > }
> >
>
> This patch breaks our imx6 board with the attached trace. Reverting the
> patch makes it boot again.
> I tried also 5.6-rc3 and it booted too. A closer look into imx-sdma.c
> from 5.6-rc3 showed me some details which might have to be backported as
> well to make this patch work.
> I tried a1ff6a07f5a3951fcac84f064a76d1ad79c10e40 and was somehow
> successful. I still have one trace but the board boots now.
>
> Any insights from the experts?

This series should be applied as a whole or not, only 7/9 is optional.

It seems I have to avoid the trigger word "fix" in my commit messages or
make sure these patches won't apply without their dependencies :-/

Sascha


--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

2020-02-24 21:24:57

by Sasha Levin

[permalink] [raw]
Subject: Re: [PATCH 5.4 160/344] dmaengine: imx-sdma: Fix memory leak

On Mon, Feb 24, 2020 at 03:57:18PM +0100, Sascha Hauer wrote:
>On Mon, Feb 24, 2020 at 01:24:04PM +0000, Andreas Tobler wrote:
>> Hi all,
>>
>> On 21.02.20 08:39, Greg Kroah-Hartman wrote:
>> > From: Sascha Hauer <[email protected]>
>> >
>> > [ Upstream commit 02939cd167095f16328a1bd5cab5a90b550606df ]
>> >
>> > The current descriptor is not on any list of the virtual DMA channel.
>> > Once sdma_terminate_all() is called when a descriptor is currently
>> > in flight then this one is forgotten to be freed. We have to call
>> > vchan_terminate_vdesc() on this descriptor to re-add it to the lists.
>> > Now that we also free the currently running descriptor we can (and
>> > actually have to) remove the current descriptor from its list also
>> > for the cyclic case.
>> >
>> > Signed-off-by: Sascha Hauer <[email protected]>
>> > Reviewed-by: Robin Gong <[email protected]>
>> > Tested-by: Robin Gong <[email protected]>
>> > Link: https://lore.kernel.org/r/[email protected]
>> > Signed-off-by: Vinod Koul <[email protected]>
>> > Signed-off-by: Sasha Levin <[email protected]>
>> > ---
>> > drivers/dma/imx-sdma.c | 19 +++++++++++--------
>> > 1 file changed, 11 insertions(+), 8 deletions(-)
>> >
>> > diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
>> > index c27e206a764c3..66f1b2ac5cde4 100644
>> > --- a/drivers/dma/imx-sdma.c
>> > +++ b/drivers/dma/imx-sdma.c
>> > @@ -760,12 +760,8 @@ static void sdma_start_desc(struct sdma_channel *sdmac)
>> > return;
>> > }
>> > sdmac->desc = desc = to_sdma_desc(&vd->tx);
>> > - /*
>> > - * Do not delete the node in desc_issued list in cyclic mode, otherwise
>> > - * the desc allocated will never be freed in vchan_dma_desc_free_list
>> > - */
>> > - if (!(sdmac->flags & IMX_DMA_SG_LOOP))
>> > - list_del(&vd->node);
>> > +
>> > + list_del(&vd->node);
>> >
>> > sdma->channel_control[channel].base_bd_ptr = desc->bd_phys;
>> > sdma->channel_control[channel].current_bd_ptr = desc->bd_phys;
>> > @@ -1071,7 +1067,6 @@ static void sdma_channel_terminate_work(struct work_struct *work)
>> >
>> > spin_lock_irqsave(&sdmac->vc.lock, flags);
>> > vchan_get_all_descriptors(&sdmac->vc, &head);
>> > - sdmac->desc = NULL;
>> > spin_unlock_irqrestore(&sdmac->vc.lock, flags);
>> > vchan_dma_desc_free_list(&sdmac->vc, &head);
>> > sdmac->context_loaded = false;
>> > @@ -1080,11 +1075,19 @@ static void sdma_channel_terminate_work(struct work_struct *work)
>> > static int sdma_disable_channel_async(struct dma_chan *chan)
>> > {
>> > struct sdma_channel *sdmac = to_sdma_chan(chan);
>> > + unsigned long flags;
>> > +
>> > + spin_lock_irqsave(&sdmac->vc.lock, flags);
>> >
>> > sdma_disable_channel(chan);
>> >
>> > - if (sdmac->desc)
>> > + if (sdmac->desc) {
>> > + vchan_terminate_vdesc(&sdmac->desc->vd);
>> > + sdmac->desc = NULL;
>> > schedule_work(&sdmac->terminate_worker);
>> > + }
>> > +
>> > + spin_unlock_irqrestore(&sdmac->vc.lock, flags);
>> >
>> > return 0;
>> > }
>> >
>>
>> This patch breaks our imx6 board with the attached trace. Reverting the
>> patch makes it boot again.
>> I tried also 5.6-rc3 and it booted too. A closer look into imx-sdma.c
>> from 5.6-rc3 showed me some details which might have to be backported as
>> well to make this patch work.
>> I tried a1ff6a07f5a3951fcac84f064a76d1ad79c10e40 and was somehow
>> successful. I still have one trace but the board boots now.
>>
>> Any insights from the experts?
>
>This series should be applied as a whole or not, only 7/9 is optional.
>
>It seems I have to avoid the trigger word "fix" in my commit messages or
>make sure these patches won't apply without their dependencies :-/

Or you could just tag the dependencies so that we could take all of them
as well? We have a nice "Depends-on:" tag that makes it easy.

As with everything in life, you want to communicate more effectively
rather than not communicate at all.

--
Thanks,
Sasha

2020-02-26 06:51:21

by Sascha Hauer

[permalink] [raw]
Subject: Re: [PATCH 5.4 160/344] dmaengine: imx-sdma: Fix memory leak

On Mon, Feb 24, 2020 at 04:23:19PM -0500, Sasha Levin wrote:
> On Mon, Feb 24, 2020 at 03:57:18PM +0100, Sascha Hauer wrote:
> > On Mon, Feb 24, 2020 at 01:24:04PM +0000, Andreas Tobler wrote:
> > > Hi all,
> > >
> > > On 21.02.20 08:39, Greg Kroah-Hartman wrote:
> > > > From: Sascha Hauer <[email protected]>
> > > >
> > > > [ Upstream commit 02939cd167095f16328a1bd5cab5a90b550606df ]
> > > >
> > > > The current descriptor is not on any list of the virtual DMA channel.
> > > > Once sdma_terminate_all() is called when a descriptor is currently
> > > > in flight then this one is forgotten to be freed. We have to call
> > > > vchan_terminate_vdesc() on this descriptor to re-add it to the lists.
> > > > Now that we also free the currently running descriptor we can (and
> > > > actually have to) remove the current descriptor from its list also
> > > > for the cyclic case.
> > > >
> > > > Signed-off-by: Sascha Hauer <[email protected]>
> > > > Reviewed-by: Robin Gong <[email protected]>
> > > > Tested-by: Robin Gong <[email protected]>
> > > > Link: https://lore.kernel.org/r/[email protected]
> > > > Signed-off-by: Vinod Koul <[email protected]>
> > > > Signed-off-by: Sasha Levin <[email protected]>
> > > > ---
> > > > drivers/dma/imx-sdma.c | 19 +++++++++++--------
> > > > 1 file changed, 11 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> > > > index c27e206a764c3..66f1b2ac5cde4 100644
> > > > --- a/drivers/dma/imx-sdma.c
> > > > +++ b/drivers/dma/imx-sdma.c
> > > > @@ -760,12 +760,8 @@ static void sdma_start_desc(struct sdma_channel *sdmac)
> > > > return;
> > > > }
> > > > sdmac->desc = desc = to_sdma_desc(&vd->tx);
> > > > - /*
> > > > - * Do not delete the node in desc_issued list in cyclic mode, otherwise
> > > > - * the desc allocated will never be freed in vchan_dma_desc_free_list
> > > > - */
> > > > - if (!(sdmac->flags & IMX_DMA_SG_LOOP))
> > > > - list_del(&vd->node);
> > > > +
> > > > + list_del(&vd->node);
> > > >
> > > > sdma->channel_control[channel].base_bd_ptr = desc->bd_phys;
> > > > sdma->channel_control[channel].current_bd_ptr = desc->bd_phys;
> > > > @@ -1071,7 +1067,6 @@ static void sdma_channel_terminate_work(struct work_struct *work)
> > > >
> > > > spin_lock_irqsave(&sdmac->vc.lock, flags);
> > > > vchan_get_all_descriptors(&sdmac->vc, &head);
> > > > - sdmac->desc = NULL;
> > > > spin_unlock_irqrestore(&sdmac->vc.lock, flags);
> > > > vchan_dma_desc_free_list(&sdmac->vc, &head);
> > > > sdmac->context_loaded = false;
> > > > @@ -1080,11 +1075,19 @@ static void sdma_channel_terminate_work(struct work_struct *work)
> > > > static int sdma_disable_channel_async(struct dma_chan *chan)
> > > > {
> > > > struct sdma_channel *sdmac = to_sdma_chan(chan);
> > > > + unsigned long flags;
> > > > +
> > > > + spin_lock_irqsave(&sdmac->vc.lock, flags);
> > > >
> > > > sdma_disable_channel(chan);
> > > >
> > > > - if (sdmac->desc)
> > > > + if (sdmac->desc) {
> > > > + vchan_terminate_vdesc(&sdmac->desc->vd);
> > > > + sdmac->desc = NULL;
> > > > schedule_work(&sdmac->terminate_worker);
> > > > + }
> > > > +
> > > > + spin_unlock_irqrestore(&sdmac->vc.lock, flags);
> > > >
> > > > return 0;
> > > > }
> > > >
> > >
> > > This patch breaks our imx6 board with the attached trace. Reverting the
> > > patch makes it boot again.
> > > I tried also 5.6-rc3 and it booted too. A closer look into imx-sdma.c
> > > from 5.6-rc3 showed me some details which might have to be backported as
> > > well to make this patch work.
> > > I tried a1ff6a07f5a3951fcac84f064a76d1ad79c10e40 and was somehow
> > > successful. I still have one trace but the board boots now.
> > >
> > > Any insights from the experts?
> >
> > This series should be applied as a whole or not, only 7/9 is optional.
> >
> > It seems I have to avoid the trigger word "fix" in my commit messages or
> > make sure these patches won't apply without their dependencies :-/
>
> Or you could just tag the dependencies so that we could take all of them
> as well? We have a nice "Depends-on:" tag that makes it easy.
>
> As with everything in life, you want to communicate more effectively
> rather than not communicate at all.

Speaking of which, if you want people to use that "Depends-on:" tag you
should spread the word about it. It was used for 30 commits in the whole
Kernel history and Documentation/ doesn't mention it at all.

Anyway, this helps only for patches from which I actually know the
dependencies. I knew them this time, because the whole series only had
the purpose of making ground for the patch. Often enough I don't know
them putting a patch onto a kernel just because it applies cleanly
doesn't seem like a good idea to me.

Sascha

--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

2020-02-27 09:49:57

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 5.4 160/344] dmaengine: imx-sdma: Fix memory leak

On Mon, Feb 24, 2020 at 01:24:04PM +0000, Andreas Tobler wrote:
> Hi all,
>
> On 21.02.20 08:39, Greg Kroah-Hartman wrote:
> > From: Sascha Hauer <[email protected]>
> >
> > [ Upstream commit 02939cd167095f16328a1bd5cab5a90b550606df ]
> >
> > The current descriptor is not on any list of the virtual DMA channel.
> > Once sdma_terminate_all() is called when a descriptor is currently
> > in flight then this one is forgotten to be freed. We have to call
> > vchan_terminate_vdesc() on this descriptor to re-add it to the lists.
> > Now that we also free the currently running descriptor we can (and
> > actually have to) remove the current descriptor from its list also
> > for the cyclic case.
> >
> > Signed-off-by: Sascha Hauer <[email protected]>
> > Reviewed-by: Robin Gong <[email protected]>
> > Tested-by: Robin Gong <[email protected]>
> > Link: https://lore.kernel.org/r/[email protected]
> > Signed-off-by: Vinod Koul <[email protected]>
> > Signed-off-by: Sasha Levin <[email protected]>
> > ---
> > drivers/dma/imx-sdma.c | 19 +++++++++++--------
> > 1 file changed, 11 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> > index c27e206a764c3..66f1b2ac5cde4 100644
> > --- a/drivers/dma/imx-sdma.c
> > +++ b/drivers/dma/imx-sdma.c
> > @@ -760,12 +760,8 @@ static void sdma_start_desc(struct sdma_channel *sdmac)
> > return;
> > }
> > sdmac->desc = desc = to_sdma_desc(&vd->tx);
> > - /*
> > - * Do not delete the node in desc_issued list in cyclic mode, otherwise
> > - * the desc allocated will never be freed in vchan_dma_desc_free_list
> > - */
> > - if (!(sdmac->flags & IMX_DMA_SG_LOOP))
> > - list_del(&vd->node);
> > +
> > + list_del(&vd->node);
> >
> > sdma->channel_control[channel].base_bd_ptr = desc->bd_phys;
> > sdma->channel_control[channel].current_bd_ptr = desc->bd_phys;
> > @@ -1071,7 +1067,6 @@ static void sdma_channel_terminate_work(struct work_struct *work)
> >
> > spin_lock_irqsave(&sdmac->vc.lock, flags);
> > vchan_get_all_descriptors(&sdmac->vc, &head);
> > - sdmac->desc = NULL;
> > spin_unlock_irqrestore(&sdmac->vc.lock, flags);
> > vchan_dma_desc_free_list(&sdmac->vc, &head);
> > sdmac->context_loaded = false;
> > @@ -1080,11 +1075,19 @@ static void sdma_channel_terminate_work(struct work_struct *work)
> > static int sdma_disable_channel_async(struct dma_chan *chan)
> > {
> > struct sdma_channel *sdmac = to_sdma_chan(chan);
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&sdmac->vc.lock, flags);
> >
> > sdma_disable_channel(chan);
> >
> > - if (sdmac->desc)
> > + if (sdmac->desc) {
> > + vchan_terminate_vdesc(&sdmac->desc->vd);
> > + sdmac->desc = NULL;
> > schedule_work(&sdmac->terminate_worker);
> > + }
> > +
> > + spin_unlock_irqrestore(&sdmac->vc.lock, flags);
> >
> > return 0;
> > }
> >
>
> This patch breaks our imx6 board with the attached trace. Reverting the
> patch makes it boot again.
> I tried also 5.6-rc3 and it booted too. A closer look into imx-sdma.c
> from 5.6-rc3 showed me some details which might have to be backported as
> well to make this patch work.
> I tried a1ff6a07f5a3951fcac84f064a76d1ad79c10e40 and was somehow
> successful. I still have one trace but the board boots now.
>
> Any insights from the experts?

I've now reverted it from all of the 3 stable trees it endeded up in.

thanks,

greg k-h