From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.6 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 05EE7C38BF9 for ; Mon, 24 Feb 2020 21:23:24 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id D196F21927 for ; Mon, 24 Feb 2020 21:23:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1582579403; bh=XnkYOmulZ8/lTR2roVqGnBGPNTBbKp8TG7sz+XwMTDk=; h=Date:From:To:Cc:Subject:References:In-Reply-To:List-ID:From; b=IFt54IMh22Xk012z556FIr5NpxuOrRxqZ5SFJj8h1QwwXdQJeuMKbKEVyN+AclbyO 1lpB6MnTDoSmrJQ6c7AIDakqNjVvoPlolHLrcD3MOQ25WfdXy83iSmOIh+TPruaaG7 Gjr1ROdpbSP4mzL46iHB6Ng6PUz2LepNa2W2RTCY= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727910AbgBXVXW (ORCPT ); Mon, 24 Feb 2020 16:23:22 -0500 Received: from mail.kernel.org ([198.145.29.99]:38002 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727322AbgBXVXW (ORCPT ); Mon, 24 Feb 2020 16:23:22 -0500 Received: from localhost (c-73-47-72-35.hsd1.nh.comcast.net [73.47.72.35]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 1758C218AC; Mon, 24 Feb 2020 21:23:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1582579401; bh=XnkYOmulZ8/lTR2roVqGnBGPNTBbKp8TG7sz+XwMTDk=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=YEBbehGBXvp3CPzembrHbTI7ud/HJbKPdfdL/5pEC9w0esFKdekF03AbMqJimpl3v GyNxtEab/KQ7pvDrxIQLOiihaSlLV3ofzr6lxx3lXS8nfHcuHYlg/ojkeQRf93SlxT WILXyi0OKIOmyLdmAOyEDENKwyzUFskZ0DxFhC4c= Date: Mon, 24 Feb 2020 16:23:19 -0500 From: Sasha Levin To: Sascha Hauer Cc: Andreas Tobler , Greg Kroah-Hartman , "linux-kernel@vger.kernel.org" , "stable@vger.kernel.org" , Robin Gong , Vinod Koul Subject: Re: [PATCH 5.4 160/344] dmaengine: imx-sdma: Fix memory leak Message-ID: <20200224212319.GE26320@sasha-vm> References: <20200221072349.335551332@linuxfoundation.org> <20200221072403.369335694@linuxfoundation.org> <1429291b-77c5-41aa-8dee-8858eba6d138@onway.ch> <20200224145718.GD3335@pengutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: <20200224145718.GD3335@pengutronix.de> User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >> > >> > [ 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 >> > Reviewed-by: Robin Gong >> > Tested-by: Robin Gong >> > Link: https://lore.kernel.org/r/20191216105328.15198-10-s.hauer@pengutronix.de >> > Signed-off-by: Vinod Koul >> > Signed-off-by: Sasha Levin >> > --- >> > 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