[PATCH 0/4] dmaeninge/ste_dma40: bug fixes for 3.1

[PATCH 0/4] dmaeninge/ste_dma40: bug fixes for 3.1

[Per Forlin]

Started off fixing a memory leak due to not freed dma descriptors
in mmci. The descriptor allocated at device_prep...(), not submitted,
isn't freed at dmaeninge_terminate_all() or dmaengine_release().
While sorting this one out some other issues were found as well.
* kernel doc missing
* duplication of d40_pool_lli_free()
* free of client descriptor triggers an oops.

Per Forlin (4):
  dmaengine/ste_dma40: add missing kernel doc for pending_queue
  dmaengine/ste_dma40: remove duplicate call to d40_pool_lli_free().
  dmaengine/ste_dma40: fix Oops due to double free of client descriptor
  dmaengine/ste_dma40: fix memory leak due to prepared descriptors

 drivers/dma/ste_dma40.c |   42 +++++++++++++++++++++++++++++-------------
 1 files changed, 29 insertions(+), 13 deletions(-)

-- 
1.7.4.1

[PATCH 1/4] dmaengine/ste_dma40: add missing kernel doc for pending_queue

[Per Forlin]

Signed-off-by: Per Forlin <per.forlin@linaro.org>
---
 drivers/dma/ste_dma40.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index cd3a7c7..486b6c0 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -174,6 +174,7 @@ struct d40_base;
  * @tasklet: Tasklet that gets scheduled from interrupt context to complete a
  * transfer and call client callback.
  * @client: Cliented owned descriptor list.
+ * @pending_queue: Submitted jobs, to be issued by issue_pending()
  * @active: Active descriptor.
  * @queue: Queued jobs.
  * @dma_cfg: The client configuration of this dma channel.
-- 
1.7.4.1

[PATCH 2/4] dmaengine/ste_dma40: remove duplicate call to d40_pool_lli_free().

[Per Forlin]

d40_desc_free() already calls d40_pool_lli_free().

Signed-off-by: Per Forlin <per.forlin@linaro.org>
---
 drivers/dma/ste_dma40.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 486b6c0..37388d1 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -478,7 +478,6 @@ static struct d40_desc *d40_desc_get(struct d40_chan *d40c)
 
 		list_for_each_entry_safe(d, _d, &d40c->client, node)
 			if (async_tx_test_ack(&d->txd)) {
-				d40_pool_lli_free(d40c, d);
 				d40_desc_remove(d);
 				desc = d;
 				memset(desc, 0, sizeof(*desc));
@@ -1209,7 +1208,6 @@ static void dma_tasklet(unsigned long data)
 
 	if (!d40d->cyclic) {
 		if (async_tx_test_ack(&d40d->txd)) {
-			d40_pool_lli_free(d40c, d40d);
 			d40_desc_remove(d40d);
 			d40_desc_free(d40c, d40d);
 		} else {
@@ -1606,7 +1604,6 @@ static int d40_free_dma(struct d40_chan *d40c)
 	/* Release client owned descriptors */
 	if (!list_empty(&d40c->client))
 		list_for_each_entry_safe(d, _d, &d40c->client, node) {
-			d40_pool_lli_free(d40c, d);
 			d40_desc_remove(d);
 			d40_desc_free(d40c, d);
 		}
-- 
1.7.4.1

[PATCH 3/4] dmaengine/ste_dma40: fix Oops due to double free of client descriptor

[Per Forlin]

The client list may exist in two lists at the same time. This makes free
fail since the same desc is freed multiple times. Remove desc from
client list when adding it to the pending queue. Move free of client owned
descriptors from free_dma() to terminate_all().

Unable to handle kernel paging request at virtual address 00100104
pgd = dea8c000
[00100104] *pgd=1ea62831, *pte=00000000, *ppte=00000000
Internal error: Oops: 817 [#1] PREEMPT SMP
Modules linked in:
CPU: 0    Not tainted  (3.1.0-rc3+ #58)
PC is at d40_free_chan_resources+0x64/0x330

Signed-off-by: Per Forlin <per.forlin@linaro.org>
---
 drivers/dma/ste_dma40.c |   22 ++++++++++++----------
 1 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 37388d1..92ec0a2 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -644,8 +644,11 @@ static struct d40_desc *d40_first_active_get(struct d40_chan *d40c)
 	return d;
 }
 
+/* remove desc from current queue and add it to the pending_queue */
 static void d40_desc_queue(struct d40_chan *d40c, struct d40_desc *desc)
 {
+	d40_desc_remove(desc);
+	desc->is_in_client_list = false;
 	list_add_tail(&desc->node, &d40c->pending_queue);
 }
 
@@ -803,6 +806,7 @@ done:
 static void d40_term_all(struct d40_chan *d40c)
 {
 	struct d40_desc *d40d;
+	struct d40_desc *_d;
 
 	/* Release active descriptors */
 	while ((d40d = d40_first_active_get(d40c))) {
@@ -822,6 +826,14 @@ static void d40_term_all(struct d40_chan *d40c)
 		d40_desc_free(d40c, d40d);
 	}
 
+	/* Release client owned descriptors */
+	if (!list_empty(&d40c->client))
+		list_for_each_entry_safe(d40d, _d, &d40c->client, node) {
+			d40_desc_remove(d40d);
+			d40_desc_free(d40c, d40d);
+		}
+
+
 	d40c->pending_tx = 0;
 	d40c->busy = false;
 }
@@ -1594,20 +1606,10 @@ static int d40_free_dma(struct d40_chan *d40c)
 	u32 event;
 	struct d40_phy_res *phy = d40c->phy_chan;
 	bool is_src;
-	struct d40_desc *d;
-	struct d40_desc *_d;
-
 
 	/* Terminate all queued and active transfers */
 	d40_term_all(d40c);
 
-	/* Release client owned descriptors */
-	if (!list_empty(&d40c->client))
-		list_for_each_entry_safe(d, _d, &d40c->client, node) {
-			d40_desc_remove(d);
-			d40_desc_free(d40c, d);
-		}
-
 	if (phy == NULL) {
 		chan_err(d40c, "phy == null\n");
 		return -EINVAL;
-- 
1.7.4.1

[PATCH 4/4] dmaengine/ste_dma40: fix memory leak due to prepared descriptors

[Per Forlin]

Prepared descriptors that are not submitted will not be freed. Add
prepared descriptor to a list to be able to release them upon
dmaengine_terminate_all().

Signed-off-by: Per Forlin <per.forlin@linaro.org>
---
 drivers/dma/ste_dma40.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 92ec0a2..467e4dc 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -177,6 +177,7 @@ struct d40_base;
  * @pending_queue: Submitted jobs, to be issued by issue_pending()
  * @active: Active descriptor.
  * @queue: Queued jobs.
+ * @prepare_queue: Prepared jobs.
  * @dma_cfg: The client configuration of this dma channel.
  * @configured: whether the dma_cfg configuration is valid
  * @base: Pointer to the device instance struct.
@@ -204,6 +205,7 @@ struct d40_chan {
 	struct list_head		 pending_queue;
 	struct list_head		 active;
 	struct list_head		 queue;
+	struct list_head		 prepare_queue;
 	struct stedma40_chan_cfg	 dma_cfg;
 	bool				 configured;
 	struct d40_base			*base;
@@ -833,6 +835,13 @@ static void d40_term_all(struct d40_chan *d40c)
 			d40_desc_free(d40c, d40d);
 		}
 
+	/* Release descriptors in prepare queue */
+	if (!list_empty(&d40c->prepare_queue))
+		list_for_each_entry_safe(d40d, _d,
+					 &d40c->prepare_queue, node) {
+			d40_desc_remove(d40d);
+			d40_desc_free(d40c, d40d);
+		}
 
 	d40c->pending_tx = 0;
 	d40c->busy = false;
@@ -1911,6 +1920,12 @@ d40_prep_sg(struct dma_chan *dchan, struct scatterlist *sg_src,
 		goto err;
 	}
 
+	/*
+	 * add descriptor to the prepare queue in order to be able
+	 * to free them later in terminate_all
+	 */
+	list_add_tail(&desc->node, &chan->prepare_queue);
+
 	spin_unlock_irqrestore(&chan->lock, flags);
 
 	return &desc->txd;
@@ -2400,6 +2415,7 @@ static void __init d40_chan_init(struct d40_base *base, struct dma_device *dma,
 		INIT_LIST_HEAD(&d40c->queue);
 		INIT_LIST_HEAD(&d40c->pending_queue);
 		INIT_LIST_HEAD(&d40c->client);
+		INIT_LIST_HEAD(&d40c->prepare_queue);
 
 		tasklet_init(&d40c->tasklet, dma_tasklet,
 			     (unsigned long) d40c);
-- 
1.7.4.1

Re: [PATCH 0/4] dmaeninge/ste_dma40: bug fixes for 3.1

[Linus Walleij]

On Mon, Aug 29, 2011 at 1:33 PM, Per Forlin <per.forlin@linaro.org> wrote:

> Started off fixing a memory leak due to not freed dma descriptors
> in mmci. The descriptor allocated at device_prep...(), not submitted,
> isn't freed at dmaeninge_terminate_all() or dmaengine_release().
> While sorting this one out some other issues were found as well.
> * kernel doc missing
> * duplication of d40_pool_lli_free()
> * free of client descriptor triggers an oops.

Acked-by: Linus Walleij <linus.walleij@linaro.org>

For all patches, thanks Per!
Linus Walleij

Re: [PATCH 0/4] dmaeninge/ste_dma40: bug fixes for 3.1

[Vinod Koul]

On Mon, 2011-08-29 at 13:33 +0200, Per Forlin wrote:
> Started off fixing a memory leak due to not freed dma descriptors
> in mmci. The descriptor allocated at device_prep...(), not submitted,
> isn't freed at dmaeninge_terminate_all() or dmaengine_release().
> While sorting this one out some other issues were found as well.
> * kernel doc missing
> * duplication of d40_pool_lli_free()
> * free of client descriptor triggers an oops.
> 
> Per Forlin (4):
>   dmaengine/ste_dma40: add missing kernel doc for pending_queue
>   dmaengine/ste_dma40: remove duplicate call to d40_pool_lli_free().
>   dmaengine/ste_dma40: fix Oops due to double free of client descriptor
>   dmaengine/ste_dma40: fix memory leak due to prepared descriptors
> 
>  drivers/dma/ste_dma40.c |   42 +++++++++++++++++++++++++++++-------------
>  1 files changed, 29 insertions(+), 13 deletions(-)
> 
Applied, Thanks

-- 
~Vinod