[PATCH RESEND v2] dma: cppi41: only allocate descriptor memory once

2013-12-04 Thread Daniel Mack
cdd-cd and cdd-descs_phys are allocated DESCS_AREAS times from
init_descs() and freed as often from purge_descs(). This leads to both
memory leaks and double-frees.

Fix this by pulling the calls to dma_{alloc,free}_coherent() out of the
loops.

While at it, remove the intermediate variable mem_decs (I guess it was
only there to make the code comply to the 80-chars CodingSytle rule).

Signed-off-by: Daniel Mack zon...@gmail.com
---
 drivers/dma/cppi41.c | 28 ++--
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index c29dacf..ca908c6 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -713,19 +713,15 @@ err:
 
 static void purge_descs(struct device *dev, struct cppi41_dd *cdd)
 {
-   unsigned int mem_decs;
int i;
 
-   mem_decs = ALLOC_DECS_NUM * sizeof(struct cppi41_desc);
-
for (i = 0; i  DESCS_AREAS; i++) {
-
cppi_writel(0, cdd-qmgr_mem + QMGR_MEMBASE(i));
cppi_writel(0, cdd-qmgr_mem + QMGR_MEMCTRL(i));
-
-   dma_free_coherent(dev, mem_decs, cdd-cd,
-   cdd-descs_phys);
}
+
+   dma_free_coherent(dev, ALLOC_DECS_NUM * sizeof(struct cppi41_desc),
+ cdd-cd, cdd-descs_phys);
 }
 
 static void disable_sched(struct cppi41_dd *cdd)
@@ -747,8 +743,7 @@ static void deinit_cppi41(struct device *dev, struct 
cppi41_dd *cdd)
 
 static int init_descs(struct device *dev, struct cppi41_dd *cdd)
 {
-   unsigned int desc_size;
-   unsigned int mem_decs;
+   unsigned int desc_size = sizeof(struct cppi41_desc);
int i;
u32 reg;
u32 idx;
@@ -757,28 +752,25 @@ static int init_descs(struct device *dev, struct 
cppi41_dd *cdd)
(sizeof(struct cppi41_desc) - 1));
BUILD_BUG_ON(sizeof(struct cppi41_desc)  32);
BUILD_BUG_ON(ALLOC_DECS_NUM  32);
+   BUILD_BUG_ON(DESCS_AREAS != 1);
 
-   desc_size = sizeof(struct cppi41_desc);
-   mem_decs = ALLOC_DECS_NUM * desc_size;
+   cdd-cd = dma_alloc_coherent(dev, ALLOC_DECS_NUM * desc_size,
+cdd-descs_phys, GFP_KERNEL);
+   if (!cdd-cd)
+   return -ENOMEM;
 
idx = 0;
for (i = 0; i  DESCS_AREAS; i++) {
-
reg = idx  QMGR_MEMCTRL_IDX_SH;
reg |= (ilog2(desc_size) - 5)  QMGR_MEMCTRL_DESC_SH;
reg |= ilog2(ALLOC_DECS_NUM) - 5;
 
-   BUILD_BUG_ON(DESCS_AREAS != 1);
-   cdd-cd = dma_alloc_coherent(dev, mem_decs,
-   cdd-descs_phys, GFP_KERNEL);
-   if (!cdd-cd)
-   return -ENOMEM;
-
cppi_writel(cdd-descs_phys, cdd-qmgr_mem + QMGR_MEMBASE(i));
cppi_writel(reg, cdd-qmgr_mem + QMGR_MEMCTRL(i));
 
idx += ALLOC_DECS_NUM;
}
+
return 0;
 }
 
-- 
1.8.4.2

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH RESEND v2] dma: cppi41: only allocate descriptor memory once

2013-12-04 Thread Sebastian Andrzej Siewior
* Daniel Mack | 2013-12-04 18:22:24 [+0100]:

cdd-cd and cdd-descs_phys are allocated DESCS_AREAS times from
init_descs() and freed as often from purge_descs(). This leads to both
memory leaks and double-frees.

Fix this by pulling the calls to dma_{alloc,free}_coherent() out of the
loops.

While at it, remove the intermediate variable mem_decs (I guess it was
only there to make the code comply to the 80-chars CodingSytle rule).

Signed-off-by: Daniel Mack zon...@gmail.com
We talked about this. I prefer to remove the loop and DESCS_AREAS
instead of putting build bug on != 1. If someone decides to use 2 (what
I doubt) I will still want to call dma_alloc_coherent twice for to small
regions (instead one huge).

I will provide a patch some time for this unless you decide to do so…

Sebastian
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH RESEND v2] dma: cppi41: only allocate descriptor memory once

2013-12-04 Thread Daniel Mack
On 12/04/2013 07:58 PM, Sebastian Andrzej Siewior wrote:
 * Daniel Mack | 2013-12-04 18:22:24 [+0100]:
 
 cdd-cd and cdd-descs_phys are allocated DESCS_AREAS times from
 init_descs() and freed as often from purge_descs(). This leads to both
 memory leaks and double-frees.

 Fix this by pulling the calls to dma_{alloc,free}_coherent() out of the
 loops.

 While at it, remove the intermediate variable mem_decs (I guess it was
 only there to make the code comply to the 80-chars CodingSytle rule).

 Signed-off-by: Daniel Mack zon...@gmail.com
 We talked about this. I prefer to remove the loop and DESCS_AREAS
 instead of putting build bug on != 1. If someone decides to use 2 (what
 I doubt) I will still want to call dma_alloc_coherent twice for to small
 regions (instead one huge).
 
 I will provide a patch some time for this unless you decide to do so…

Alright. Thanks then :)


Daniel

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH RESEND v2] dma: cppi41: only allocate descriptor memory once

2013-12-04 Thread Daniel Mack
On 12/04/2013 07:58 PM, Sebastian Andrzej Siewior wrote:
 * Daniel Mack | 2013-12-04 18:22:24 [+0100]:
 
 cdd-cd and cdd-descs_phys are allocated DESCS_AREAS times from
 init_descs() and freed as often from purge_descs(). This leads to both
 memory leaks and double-frees.

 Fix this by pulling the calls to dma_{alloc,free}_coherent() out of the
 loops.

 While at it, remove the intermediate variable mem_decs (I guess it was
 only there to make the code comply to the 80-chars CodingSytle rule).

 Signed-off-by: Daniel Mack zon...@gmail.com
 We talked about this. I prefer to remove the loop and DESCS_AREAS
 instead of putting build bug on != 1. If someone decides to use 2 (what
 I doubt) I will still want to call dma_alloc_coherent twice for to small
 regions (instead one huge).
 
 I will provide a patch some time for this unless you decide to do so…

Sorry - what I wanted to say is: Ok, I'll cook up a patch, thanks :)



Daniel

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html