LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 0/5] dmaengine: Fix memory leak amongs virt-dma users
@ 2015-03-27 11:35 Peter Ujfalusi
  2015-03-27 11:35 ` [PATCH 1/5] dmaengine: edma: fix memory leak when terminating running transfers Peter Ujfalusi
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: Peter Ujfalusi @ 2015-03-27 11:35 UTC (permalink / raw)
  To: vinod.koul, linux
  Cc: dan.j.williams, dmaengine, linux-kernel, linux-arm-kernel

Hi,

Due to the implementation of some of the drivers using virt-dma they leak memory
by design.
All it takes is to stop a transfer which is not yet completed, this includes
cyclic (audio) channels also.
These drivers tend to remove the vdesc->node from the virt-dma lists when they
start the transfer. In terminate all vchan_get_all_descriptors() will not find
the transfer which is still running (not in any of the lists) so the free_desc
callback will not be called on them leaving us with memory leak.

There are other drivers leaking memory IMHO, but I was not sure how to fix them:
sun6i-dma,
sa11x0-dma,
k3dma

Others are already doing something to prevent the leak by either directly
freeing the desc or by adding back the vdesc->node to a list.

Regards,
Peter
---
Peter Ujfalusi (4):
  dmaengine: omap-dma: Fix memory leak when terminating running transfer
  dmaengine: bcm2835-dma: Fix memory leak when stopping a running
    transfer
  dmaengine: hsu: Fix memory leak when stopping a running transfer
  dmaengine: moxart-dma: Fix memory leak when stopping a running
    transfer

Petr Kulhavy (1):
  dmaengine: edma: fix memory leak when terminating running transfers

 drivers/dma/bcm2835-dma.c | 1 +
 drivers/dma/edma.c        | 7 +++++++
 drivers/dma/hsu/hsu.c     | 5 ++++-
 drivers/dma/moxart-dma.c  | 4 +++-
 drivers/dma/omap-dma.c    | 1 +
 5 files changed, 16 insertions(+), 2 deletions(-)

-- 
2.3.3


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

* [PATCH 1/5] dmaengine: edma: fix memory leak when terminating running transfers
  2015-03-27 11:35 [PATCH 0/5] dmaengine: Fix memory leak amongs virt-dma users Peter Ujfalusi
@ 2015-03-27 11:35 ` Peter Ujfalusi
  2015-03-30 17:48   ` Vinod Koul
  2015-03-27 11:35 ` [PATCH 2/5] dmaengine: omap-dma: Fix memory leak when terminating running transfer Peter Ujfalusi
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Peter Ujfalusi @ 2015-03-27 11:35 UTC (permalink / raw)
  To: vinod.koul, linux
  Cc: dan.j.williams, dmaengine, linux-kernel, linux-arm-kernel,
	Petr Kulhavy, stable, linux-omap

From: Petr Kulhavy <petr@barix.com>

If edma_terminate_all() was called while a transfer was running (i.e. after
edma_execute() but before edma_callback()) the echan->edesc was not freed.

This was due to the fact that a running transfer is on none of the
vchan lists: desc_submitted, desc_issued, desc_completed (edma_execute()
removes it from the desc_issued list), so the vchan_dma_desc_free_list()
called at the end of edma_terminate_all() didn't find it and didn't free it.

This bug was found on an AM1808 based hardware (very similar to da850evm,
however using the second MMC/SD controller), where intense operations on the SD
card wasted the device 128MB RAM within a couple of days.

Peter Ujfalusi:
The issue is even more severe since it affects cyclic (audio) transfers as
well. In this case starting/stopping audio will results memory leak.

Signed-off-by: Petr Kulhavy <petr@barix.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
CC: <stable@vger.kernel.org>
CC: <linux-omap@vger.kernel.org>
---
 drivers/dma/edma.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index 741baa68365c..984c2b12dae3 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -268,6 +268,13 @@ static int edma_terminate_all(struct dma_chan *chan)
 	 */
 	if (echan->edesc) {
 		int cyclic = echan->edesc->cyclic;
+
+		/*
+		 * free the running request descriptor
+		 * since it is not in any of the vdesc lists
+		 */
+		edma_desc_free(&echan->edesc->vdesc);
+
 		echan->edesc = NULL;
 		edma_stop(echan->ch_num);
 		/* Move the cyclic channel back to default queue */
-- 
2.3.3


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

* [PATCH 2/5] dmaengine: omap-dma: Fix memory leak when terminating running transfer
  2015-03-27 11:35 [PATCH 0/5] dmaengine: Fix memory leak amongs virt-dma users Peter Ujfalusi
  2015-03-27 11:35 ` [PATCH 1/5] dmaengine: edma: fix memory leak when terminating running transfers Peter Ujfalusi
@ 2015-03-27 11:35 ` Peter Ujfalusi
  2015-03-30 17:48   ` Vinod Koul
  2015-03-27 11:35 ` [PATCH 3/5] dmaengine: bcm2835-dma: Fix memory leak when stopping a " Peter Ujfalusi
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Peter Ujfalusi @ 2015-03-27 11:35 UTC (permalink / raw)
  To: vinod.koul, linux
  Cc: dan.j.williams, dmaengine, linux-kernel, linux-arm-kernel,
	stable, linux-omap

In omap_dma_start_desc the vdesc->node is removed from the virt-dma
framework managed lists (to be precise from the desc_issued list).
If a terminate_all comes before the transfer finishes the omap_desc will
not be freed up because it is not in any of the lists and we stopped the
DMA channel so the transfer will not going to complete.
There is no special sequence for leaking memory when using cyclic (audio)
transfer: with every start and stop of a cyclic transfer the driver leaks
struct omap_desc worth of memory.

Free up the allocated memory directly in omap_dma_terminate_all() since the
framework will not going to do that for us.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
CC: <stable@vger.kernel.org>
CC: <linux-omap@vger.kernel.org>
---
 drivers/dma/omap-dma.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
index 1e646d6c8230..6a4c378ee432 100644
--- a/drivers/dma/omap-dma.c
+++ b/drivers/dma/omap-dma.c
@@ -1002,6 +1002,7 @@ static int omap_dma_terminate_all(struct dma_chan *chan)
 	 * c->desc is NULL and exit.)
 	 */
 	if (c->desc) {
+		omap_dma_desc_free(&c->desc->vd);
 		c->desc = NULL;
 		/* Avoid stopping the dma twice */
 		if (!c->paused)
-- 
2.3.3


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

* [PATCH 3/5] dmaengine: bcm2835-dma: Fix memory leak when stopping a running transfer
  2015-03-27 11:35 [PATCH 0/5] dmaengine: Fix memory leak amongs virt-dma users Peter Ujfalusi
  2015-03-27 11:35 ` [PATCH 1/5] dmaengine: edma: fix memory leak when terminating running transfers Peter Ujfalusi
  2015-03-27 11:35 ` [PATCH 2/5] dmaengine: omap-dma: Fix memory leak when terminating running transfer Peter Ujfalusi
@ 2015-03-27 11:35 ` Peter Ujfalusi
  2015-03-28  3:57   ` Stephen Warren
  2015-03-30 17:48   ` Vinod Koul
  2015-03-27 11:35 ` [PATCH 4/5] dmaengine: hsu: " Peter Ujfalusi
  2015-03-27 11:35 ` [PATCH 5/5] dmaengine: moxart-dma: " Peter Ujfalusi
  4 siblings, 2 replies; 17+ messages in thread
From: Peter Ujfalusi @ 2015-03-27 11:35 UTC (permalink / raw)
  To: vinod.koul, linux
  Cc: dan.j.williams, dmaengine, linux-kernel, linux-arm-kernel,
	Stephen Warren, Lee Jones

The vd->node is removed from the lists when the transfer started so the
vchan_get_all_descriptors() will not find it. This results memory leak.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
CC: Stephen Warren <swarren@wwwdotorg.org>
CC: Lee Jones <lee@kernel.org>
---
 drivers/dma/bcm2835-dma.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/dma/bcm2835-dma.c b/drivers/dma/bcm2835-dma.c
index 0723096fb50a..c92d6a70ccf3 100644
--- a/drivers/dma/bcm2835-dma.c
+++ b/drivers/dma/bcm2835-dma.c
@@ -475,6 +475,7 @@ static int bcm2835_dma_terminate_all(struct dma_chan *chan)
 	 * c->desc is NULL and exit.)
 	 */
 	if (c->desc) {
+		bcm2835_dma_desc_free(&c->desc->vd);
 		c->desc = NULL;
 		bcm2835_dma_abort(c->chan_base);
 
-- 
2.3.3


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

* [PATCH 4/5] dmaengine: hsu: Fix memory leak when stopping a running transfer
  2015-03-27 11:35 [PATCH 0/5] dmaengine: Fix memory leak amongs virt-dma users Peter Ujfalusi
                   ` (2 preceding siblings ...)
  2015-03-27 11:35 ` [PATCH 3/5] dmaengine: bcm2835-dma: Fix memory leak when stopping a " Peter Ujfalusi
@ 2015-03-27 11:35 ` Peter Ujfalusi
  2015-03-27 12:01   ` Andy Shevchenko
  2015-05-07 16:26   ` Andy Shevchenko
  2015-03-27 11:35 ` [PATCH 5/5] dmaengine: moxart-dma: " Peter Ujfalusi
  4 siblings, 2 replies; 17+ messages in thread
From: Peter Ujfalusi @ 2015-03-27 11:35 UTC (permalink / raw)
  To: vinod.koul, linux
  Cc: dan.j.williams, dmaengine, linux-kernel, linux-arm-kernel,
	Andy Shevchenko

The vd->node is removed from the lists when the transfer started so the
vchan_get_all_descriptors() will not find it. This results memory leak.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
CC: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/dma/hsu/hsu.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/hsu/hsu.c b/drivers/dma/hsu/hsu.c
index 683ba9b62795..d1864bda008f 100644
--- a/drivers/dma/hsu/hsu.c
+++ b/drivers/dma/hsu/hsu.c
@@ -387,7 +387,10 @@ static int hsu_dma_terminate_all(struct dma_chan *chan)
 	spin_lock_irqsave(&hsuc->vchan.lock, flags);
 
 	hsu_dma_stop_channel(hsuc);
-	hsuc->desc = NULL;
+	if (hsuc->desc) {
+		hsu_dma_desc_free(&hsuc->desc->vchan);
+		hsuc->desc = NULL;
+	}
 
 	vchan_get_all_descriptors(&hsuc->vchan, &head);
 	spin_unlock_irqrestore(&hsuc->vchan.lock, flags);
-- 
2.3.3


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

* [PATCH 5/5] dmaengine: moxart-dma: Fix memory leak when stopping a running transfer
  2015-03-27 11:35 [PATCH 0/5] dmaengine: Fix memory leak amongs virt-dma users Peter Ujfalusi
                   ` (3 preceding siblings ...)
  2015-03-27 11:35 ` [PATCH 4/5] dmaengine: hsu: " Peter Ujfalusi
@ 2015-03-27 11:35 ` Peter Ujfalusi
  2015-03-30 17:49   ` Vinod Koul
  4 siblings, 1 reply; 17+ messages in thread
From: Peter Ujfalusi @ 2015-03-27 11:35 UTC (permalink / raw)
  To: vinod.koul, linux
  Cc: dan.j.williams, dmaengine, linux-kernel, linux-arm-kernel, Jonas Jensen

The vd->node is removed from the lists when the transfer started so the
vchan_get_all_descriptors() will not find it. This results memory leak.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
CC: Jonas Jensen <jonas.jensen@gmail.com>
---
 drivers/dma/moxart-dma.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/moxart-dma.c b/drivers/dma/moxart-dma.c
index 15cab7d79525..b4634109e010 100644
--- a/drivers/dma/moxart-dma.c
+++ b/drivers/dma/moxart-dma.c
@@ -193,8 +193,10 @@ static int moxart_terminate_all(struct dma_chan *chan)
 
 	spin_lock_irqsave(&ch->vc.lock, flags);
 
-	if (ch->desc)
+	if (ch->desc) {
+		moxart_dma_desc_free(&ch->desc->vd);
 		ch->desc = NULL;
+	}
 
 	ctrl = readl(ch->base + REG_OFF_CTRL);
 	ctrl &= ~(APB_DMA_ENABLE | APB_DMA_FIN_INT_EN | APB_DMA_ERR_INT_EN);
-- 
2.3.3


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

* Re: [PATCH 4/5] dmaengine: hsu: Fix memory leak when stopping a running transfer
  2015-03-27 11:35 ` [PATCH 4/5] dmaengine: hsu: " Peter Ujfalusi
@ 2015-03-27 12:01   ` Andy Shevchenko
  2015-03-30 17:37     ` Vinod Koul
  2015-05-07 16:26   ` Andy Shevchenko
  1 sibling, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2015-03-27 12:01 UTC (permalink / raw)
  To: Peter Ujfalusi, 'Greg Kroah-Hartman', linux-serial
  Cc: vinod.koul, linux, dan.j.williams, dmaengine, linux-kernel,
	linux-arm-kernel

On Fri, 2015-03-27 at 13:35 +0200, Peter Ujfalusi wrote:
> The vd->node is removed from the lists when the transfer started so the
> vchan_get_all_descriptors() will not find it. This results memory leak.
> 

Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Though this one would go via Greg's tty tree (Cc'ed) I think.
It becomes to regular work flow after 4.1-rc1.


> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> CC: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> ---
>  drivers/dma/hsu/hsu.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/hsu/hsu.c b/drivers/dma/hsu/hsu.c
> index 683ba9b62795..d1864bda008f 100644
> --- a/drivers/dma/hsu/hsu.c
> +++ b/drivers/dma/hsu/hsu.c
> @@ -387,7 +387,10 @@ static int hsu_dma_terminate_all(struct dma_chan *chan)
>  	spin_lock_irqsave(&hsuc->vchan.lock, flags);
>  
>  	hsu_dma_stop_channel(hsuc);
> -	hsuc->desc = NULL;
> +	if (hsuc->desc) {
> +		hsu_dma_desc_free(&hsuc->desc->vchan);
> +		hsuc->desc = NULL;
> +	}
>  
>  	vchan_get_all_descriptors(&hsuc->vchan, &head);
>  	spin_unlock_irqrestore(&hsuc->vchan.lock, flags);


-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy


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

* Re: [PATCH 3/5] dmaengine: bcm2835-dma: Fix memory leak when stopping a running transfer
  2015-03-27 11:35 ` [PATCH 3/5] dmaengine: bcm2835-dma: Fix memory leak when stopping a " Peter Ujfalusi
@ 2015-03-28  3:57   ` Stephen Warren
  2015-03-28 21:50     ` Peter Ujfalusi
  2015-03-30 17:48   ` Vinod Koul
  1 sibling, 1 reply; 17+ messages in thread
From: Stephen Warren @ 2015-03-28  3:57 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: vinod.koul, linux, dan.j.williams, dmaengine, linux-kernel,
	linux-arm-kernel, Lee Jones

On 03/27/2015 05:35 AM, Peter Ujfalusi wrote:
> The vd->node is removed from the lists when the transfer started so the
> vchan_get_all_descriptors() will not find it. This results memory leak.

Acked-by: Stephen Warren <swarren@wwwdotorg.org>
(I'm just assuming the explanation makes sense and is correct; more of a
not-a-NAK so it doesn't look like a lack of response!)

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

* Re: [PATCH 3/5] dmaengine: bcm2835-dma: Fix memory leak when stopping a running transfer
  2015-03-28  3:57   ` Stephen Warren
@ 2015-03-28 21:50     ` Peter Ujfalusi
  0 siblings, 0 replies; 17+ messages in thread
From: Peter Ujfalusi @ 2015-03-28 21:50 UTC (permalink / raw)
  To: Stephen Warren
  Cc: vinod.koul, linux, dan.j.williams, dmaengine, linux-kernel,
	linux-arm-kernel, Lee Jones

Thanks,

On 03/28/2015 05:57 AM, Stephen Warren wrote:
> On 03/27/2015 05:35 AM, Peter Ujfalusi wrote:
>> The vd->node is removed from the lists when the transfer started so the
>> vchan_get_all_descriptors() will not find it. This results memory leak.
> 
> Acked-by: Stephen Warren <swarren@wwwdotorg.org>
> (I'm just assuming the explanation makes sense and is correct; more of a
> not-a-NAK so it doesn't look like a lack of response!)

I can not debug the bcm2835-dma driver, but the exact same pattern existed in
edma and omap-dma driver and they both leaked the edesc in a same predictable
fashion. I just checked the virt-dma users and sent the fixes for the ones
which had the same flow of events.

-- 
Péter

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

* Re: [PATCH 4/5] dmaengine: hsu: Fix memory leak when stopping a running transfer
  2015-03-27 12:01   ` Andy Shevchenko
@ 2015-03-30 17:37     ` Vinod Koul
  2015-04-28 11:17       ` 'Greg Kroah-Hartman'
  0 siblings, 1 reply; 17+ messages in thread
From: Vinod Koul @ 2015-03-30 17:37 UTC (permalink / raw)
  To: 'Greg Kroah-Hartman'
  Cc: Peter Ujfalusi, Andy Shevchenko, linux-serial, linux,
	dan.j.williams, dmaengine, linux-kernel, linux-arm-kernel

On Fri, Mar 27, 2015 at 02:01:58PM +0200, Andy Shevchenko wrote:
> On Fri, 2015-03-27 at 13:35 +0200, Peter Ujfalusi wrote:
> > The vd->node is removed from the lists when the transfer started so the
> > vchan_get_all_descriptors() will not find it. This results memory leak.
> > 
> 
> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Vinod Koul <vinod.koul@intel.com>

Greg,

Please apply this in your queue for next

-- 
Vinod

> 
> Though this one would go via Greg's tty tree (Cc'ed) I think.
> It becomes to regular work flow after 4.1-rc1.
> 
> 
> > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> > CC: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> > ---
> >  drivers/dma/hsu/hsu.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/dma/hsu/hsu.c b/drivers/dma/hsu/hsu.c
> > index 683ba9b62795..d1864bda008f 100644
> > --- a/drivers/dma/hsu/hsu.c
> > +++ b/drivers/dma/hsu/hsu.c
> > @@ -387,7 +387,10 @@ static int hsu_dma_terminate_all(struct dma_chan *chan)
> >  	spin_lock_irqsave(&hsuc->vchan.lock, flags);
> >  
> >  	hsu_dma_stop_channel(hsuc);
> > -	hsuc->desc = NULL;
> > +	if (hsuc->desc) {
> > +		hsu_dma_desc_free(&hsuc->desc->vchan);
> > +		hsuc->desc = NULL;
> > +	}
> >  
> >  	vchan_get_all_descriptors(&hsuc->vchan, &head);
> >  	spin_unlock_irqrestore(&hsuc->vchan.lock, flags);
> 
> 
> -- 
> Andy Shevchenko <andriy.shevchenko@intel.com>
> Intel Finland Oy
> 

-- 

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

* Re: [PATCH 1/5] dmaengine: edma: fix memory leak when terminating running transfers
  2015-03-27 11:35 ` [PATCH 1/5] dmaengine: edma: fix memory leak when terminating running transfers Peter Ujfalusi
@ 2015-03-30 17:48   ` Vinod Koul
  0 siblings, 0 replies; 17+ messages in thread
From: Vinod Koul @ 2015-03-30 17:48 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: linux, dan.j.williams, dmaengine, linux-kernel, linux-arm-kernel,
	Petr Kulhavy, stable, linux-omap

On Fri, Mar 27, 2015 at 01:35:51PM +0200, Peter Ujfalusi wrote:
> From: Petr Kulhavy <petr@barix.com>
> 
> If edma_terminate_all() was called while a transfer was running (i.e. after
> edma_execute() but before edma_callback()) the echan->edesc was not freed.
> 
> This was due to the fact that a running transfer is on none of the
> vchan lists: desc_submitted, desc_issued, desc_completed (edma_execute()
> removes it from the desc_issued list), so the vchan_dma_desc_free_list()
> called at the end of edma_terminate_all() didn't find it and didn't free it.
> 
> This bug was found on an AM1808 based hardware (very similar to da850evm,
> however using the second MMC/SD controller), where intense operations on the SD
> card wasted the device 128MB RAM within a couple of days.
> 
> Peter Ujfalusi:
> The issue is even more severe since it affects cyclic (audio) transfers as
> well. In this case starting/stopping audio will results memory leak.
> 
> Signed-off-by: Petr Kulhavy <petr@barix.com>
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> CC: <stable@vger.kernel.org>
> CC: <linux-omap@vger.kernel.org>
Applied, thanks

-- 
~Vinod

> ---
>  drivers/dma/edma.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
> index 741baa68365c..984c2b12dae3 100644
> --- a/drivers/dma/edma.c
> +++ b/drivers/dma/edma.c
> @@ -268,6 +268,13 @@ static int edma_terminate_all(struct dma_chan *chan)
>  	 */
>  	if (echan->edesc) {
>  		int cyclic = echan->edesc->cyclic;
> +
> +		/*
> +		 * free the running request descriptor
> +		 * since it is not in any of the vdesc lists
> +		 */
> +		edma_desc_free(&echan->edesc->vdesc);
> +
>  		echan->edesc = NULL;
>  		edma_stop(echan->ch_num);
>  		/* Move the cyclic channel back to default queue */
> -- 
> 2.3.3
> 

-- 

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

* Re: [PATCH 2/5] dmaengine: omap-dma: Fix memory leak when terminating running transfer
  2015-03-27 11:35 ` [PATCH 2/5] dmaengine: omap-dma: Fix memory leak when terminating running transfer Peter Ujfalusi
@ 2015-03-30 17:48   ` Vinod Koul
  0 siblings, 0 replies; 17+ messages in thread
From: Vinod Koul @ 2015-03-30 17:48 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: linux, dan.j.williams, dmaengine, linux-kernel, linux-arm-kernel,
	stable, linux-omap

On Fri, Mar 27, 2015 at 01:35:52PM +0200, Peter Ujfalusi wrote:
> In omap_dma_start_desc the vdesc->node is removed from the virt-dma
> framework managed lists (to be precise from the desc_issued list).
> If a terminate_all comes before the transfer finishes the omap_desc will
> not be freed up because it is not in any of the lists and we stopped the
> DMA channel so the transfer will not going to complete.
> There is no special sequence for leaking memory when using cyclic (audio)
> transfer: with every start and stop of a cyclic transfer the driver leaks
> struct omap_desc worth of memory.
> 
> Free up the allocated memory directly in omap_dma_terminate_all() since the
> framework will not going to do that for us.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> CC: <stable@vger.kernel.org>
> CC: <linux-omap@vger.kernel.org>
Applied, thanks

-- 
~Vinod

> ---
>  drivers/dma/omap-dma.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
> index 1e646d6c8230..6a4c378ee432 100644
> --- a/drivers/dma/omap-dma.c
> +++ b/drivers/dma/omap-dma.c
> @@ -1002,6 +1002,7 @@ static int omap_dma_terminate_all(struct dma_chan *chan)
>  	 * c->desc is NULL and exit.)
>  	 */
>  	if (c->desc) {
> +		omap_dma_desc_free(&c->desc->vd);
>  		c->desc = NULL;
>  		/* Avoid stopping the dma twice */
>  		if (!c->paused)
> -- 
> 2.3.3
> 

-- 

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

* Re: [PATCH 3/5] dmaengine: bcm2835-dma: Fix memory leak when stopping a running transfer
  2015-03-27 11:35 ` [PATCH 3/5] dmaengine: bcm2835-dma: Fix memory leak when stopping a " Peter Ujfalusi
  2015-03-28  3:57   ` Stephen Warren
@ 2015-03-30 17:48   ` Vinod Koul
  1 sibling, 0 replies; 17+ messages in thread
From: Vinod Koul @ 2015-03-30 17:48 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: linux, dan.j.williams, dmaengine, linux-kernel, linux-arm-kernel,
	Stephen Warren, Lee Jones

On Fri, Mar 27, 2015 at 01:35:53PM +0200, Peter Ujfalusi wrote:
> The vd->node is removed from the lists when the transfer started so the
> vchan_get_all_descriptors() will not find it. This results memory leak.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> CC: Stephen Warren <swarren@wwwdotorg.org>
> CC: Lee Jones <lee@kernel.org>
Applied, thanks

-- 
~Vinod

> ---
>  drivers/dma/bcm2835-dma.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/dma/bcm2835-dma.c b/drivers/dma/bcm2835-dma.c
> index 0723096fb50a..c92d6a70ccf3 100644
> --- a/drivers/dma/bcm2835-dma.c
> +++ b/drivers/dma/bcm2835-dma.c
> @@ -475,6 +475,7 @@ static int bcm2835_dma_terminate_all(struct dma_chan *chan)
>  	 * c->desc is NULL and exit.)
>  	 */
>  	if (c->desc) {
> +		bcm2835_dma_desc_free(&c->desc->vd);
>  		c->desc = NULL;
>  		bcm2835_dma_abort(c->chan_base);
>  
> -- 
> 2.3.3
> 

-- 

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

* Re: [PATCH 5/5] dmaengine: moxart-dma: Fix memory leak when stopping a running transfer
  2015-03-27 11:35 ` [PATCH 5/5] dmaengine: moxart-dma: " Peter Ujfalusi
@ 2015-03-30 17:49   ` Vinod Koul
  0 siblings, 0 replies; 17+ messages in thread
From: Vinod Koul @ 2015-03-30 17:49 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: linux, dan.j.williams, dmaengine, linux-kernel, linux-arm-kernel,
	Jonas Jensen

On Fri, Mar 27, 2015 at 01:35:55PM +0200, Peter Ujfalusi wrote:
> The vd->node is removed from the lists when the transfer started so the
> vchan_get_all_descriptors() will not find it. This results memory leak.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> CC: Jonas Jensen <jonas.jensen@gmail.com>
Applied, thanks

-- 
~Vinod

> ---
>  drivers/dma/moxart-dma.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/moxart-dma.c b/drivers/dma/moxart-dma.c
> index 15cab7d79525..b4634109e010 100644
> --- a/drivers/dma/moxart-dma.c
> +++ b/drivers/dma/moxart-dma.c
> @@ -193,8 +193,10 @@ static int moxart_terminate_all(struct dma_chan *chan)
>  
>  	spin_lock_irqsave(&ch->vc.lock, flags);
>  
> -	if (ch->desc)
> +	if (ch->desc) {
> +		moxart_dma_desc_free(&ch->desc->vd);
>  		ch->desc = NULL;
> +	}
>  
>  	ctrl = readl(ch->base + REG_OFF_CTRL);
>  	ctrl &= ~(APB_DMA_ENABLE | APB_DMA_FIN_INT_EN | APB_DMA_ERR_INT_EN);
> -- 
> 2.3.3
> 

-- 

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

* Re: [PATCH 4/5] dmaengine: hsu: Fix memory leak when stopping a running transfer
  2015-03-30 17:37     ` Vinod Koul
@ 2015-04-28 11:17       ` 'Greg Kroah-Hartman'
  2015-05-04 11:04         ` Vinod Koul
  0 siblings, 1 reply; 17+ messages in thread
From: 'Greg Kroah-Hartman' @ 2015-04-28 11:17 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Peter Ujfalusi, Andy Shevchenko, linux-serial, linux,
	dan.j.williams, dmaengine, linux-kernel, linux-arm-kernel

On Mon, Mar 30, 2015 at 11:07:32PM +0530, Vinod Koul wrote:
> On Fri, Mar 27, 2015 at 02:01:58PM +0200, Andy Shevchenko wrote:
> > On Fri, 2015-03-27 at 13:35 +0200, Peter Ujfalusi wrote:
> > > The vd->node is removed from the lists when the transfer started so the
> > > vchan_get_all_descriptors() will not find it. This results memory leak.
> > > 
> > 
> > Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Acked-by: Vinod Koul <vinod.koul@intel.com>
> 
> Greg,
> 
> Please apply this in your queue for next

Apply what?  Can someone resend this in a format I can apply it in?

thanks,

gregk -h

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

* Re: [PATCH 4/5] dmaengine: hsu: Fix memory leak when stopping a running transfer
  2015-04-28 11:17       ` 'Greg Kroah-Hartman'
@ 2015-05-04 11:04         ` Vinod Koul
  0 siblings, 0 replies; 17+ messages in thread
From: Vinod Koul @ 2015-05-04 11:04 UTC (permalink / raw)
  To: 'Greg Kroah-Hartman'
  Cc: Peter Ujfalusi, Andy Shevchenko, linux-serial, linux,
	dan.j.williams, dmaengine, linux-kernel, linux-arm-kernel

On Tue, Apr 28, 2015 at 01:17:08PM +0200, 'Greg Kroah-Hartman' wrote:
> On Mon, Mar 30, 2015 at 11:07:32PM +0530, Vinod Koul wrote:
> > On Fri, Mar 27, 2015 at 02:01:58PM +0200, Andy Shevchenko wrote:
> > > On Fri, 2015-03-27 at 13:35 +0200, Peter Ujfalusi wrote:
> > > > The vd->node is removed from the lists when the transfer started so the
> > > > vchan_get_all_descriptors() will not find it. This results memory leak.
> > > > 
> > > 
> > > Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Acked-by: Vinod Koul <vinod.koul@intel.com>
> > 
> > Greg,
> > 
> > Please apply this in your queue for next
> 
> Apply what?  Can someone resend this in a format I can apply it in?

Ah sorry Greg, I thought you were cced in the orignal patch as well, so..

Peter, can you resend, now that this is in my next I will apply it

-- 
~Vinod

> 
> thanks,
> 
> gregk -h

-- 

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

* Re: [PATCH 4/5] dmaengine: hsu: Fix memory leak when stopping a running transfer
  2015-03-27 11:35 ` [PATCH 4/5] dmaengine: hsu: " Peter Ujfalusi
  2015-03-27 12:01   ` Andy Shevchenko
@ 2015-05-07 16:26   ` Andy Shevchenko
  1 sibling, 0 replies; 17+ messages in thread
From: Andy Shevchenko @ 2015-05-07 16:26 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: vinod.koul, linux, dan.j.williams, dmaengine, linux-kernel,
	linux-arm-kernel

On Fri, 2015-03-27 at 13:35 +0200, Peter Ujfalusi wrote:
> The vd->node is removed from the lists when the transfer started so the
> vchan_get_all_descriptors() will not find it. This results memory leak.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> CC: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/dma/hsu/hsu.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/hsu/hsu.c b/drivers/dma/hsu/hsu.c
> index 683ba9b62795..d1864bda008f 100644
> --- a/drivers/dma/hsu/hsu.c
> +++ b/drivers/dma/hsu/hsu.c
> @@ -387,7 +387,10 @@ static int hsu_dma_terminate_all(struct dma_chan *chan)
>  	spin_lock_irqsave(&hsuc->vchan.lock, flags);
>  
>  	hsu_dma_stop_channel(hsuc);
> -	hsuc->desc = NULL;
> +	if (hsuc->desc) {
> +		hsu_dma_desc_free(&hsuc->desc->vchan);

By the way, it should be 'vdesc);' at the end.

> +		hsuc->desc = NULL;
> +	}
>  
>  	vchan_get_all_descriptors(&hsuc->vchan, &head);
>  	spin_unlock_irqrestore(&hsuc->vchan.lock, flags);


-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy


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

end of thread, other threads:[~2015-05-07 16:26 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-27 11:35 [PATCH 0/5] dmaengine: Fix memory leak amongs virt-dma users Peter Ujfalusi
2015-03-27 11:35 ` [PATCH 1/5] dmaengine: edma: fix memory leak when terminating running transfers Peter Ujfalusi
2015-03-30 17:48   ` Vinod Koul
2015-03-27 11:35 ` [PATCH 2/5] dmaengine: omap-dma: Fix memory leak when terminating running transfer Peter Ujfalusi
2015-03-30 17:48   ` Vinod Koul
2015-03-27 11:35 ` [PATCH 3/5] dmaengine: bcm2835-dma: Fix memory leak when stopping a " Peter Ujfalusi
2015-03-28  3:57   ` Stephen Warren
2015-03-28 21:50     ` Peter Ujfalusi
2015-03-30 17:48   ` Vinod Koul
2015-03-27 11:35 ` [PATCH 4/5] dmaengine: hsu: " Peter Ujfalusi
2015-03-27 12:01   ` Andy Shevchenko
2015-03-30 17:37     ` Vinod Koul
2015-04-28 11:17       ` 'Greg Kroah-Hartman'
2015-05-04 11:04         ` Vinod Koul
2015-05-07 16:26   ` Andy Shevchenko
2015-03-27 11:35 ` [PATCH 5/5] dmaengine: moxart-dma: " Peter Ujfalusi
2015-03-30 17:49   ` 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).