lkml.org 
[lkml]   [2020]   [Feb]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [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 <s.hauer@pengutronix.de>
    > >
    > > [ 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 <s.hauer@pengutronix.de>
    > > Reviewed-by: Robin Gong <yibin.gong@nxp.com>
    > > Tested-by: Robin Gong <yibin.gong@nxp.com>
    > > Link: https://lore.kernel.org/r/20191216105328.15198-10-s.hauer@pengutronix.de
    > > Signed-off-by: Vinod Koul <vkoul@kernel.org>
    > > Signed-off-by: Sasha Levin <sashal@kernel.org>
    > > ---
    > > 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

    \
     
     \ /
      Last update: 2020-02-27 10:50    [W:4.131 / U:0.068 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site