[PATCHv5 0/4] DSPBRIDGE: Improved reserved and mapped resource cleanup

[PATCHv5 0/4] DSPBRIDGE: Improved reserved and mapped resource cleanup

[Ameya Palande]

This patch series splits DMM_RES_OBJECT into DMM_MAP_OBJECT and DMM_RSV_OBJECT
which are used independently for mapped and reserved memory resources
accounting. This will help in cleanup of reserved memory resources which was
not handled properly before. With these patches resource cleanup mechanism
will work perfectly in a use case where a big chunk of memory is reserved and
then lot of mappings are created inside it.

Changes since v4:
1. Replace unnecessary list_for_each_entry_safe to list_for_each_entry
   http://marc.info/?l=linux-omap&m=126645721715598&w=2
2. Comments at appropriate places to explain resource tracking objects
   insertion and removals.
   http://marc.info/?l=linux-omap&m=126649541624172&w=2

Changes since v3:
1. Improved mapped memory resource cleanup

Changes since v2:
1. Removed locking from DRV_RemoveAllDMMResElements()
2. Removed cleanup variable from PROC_UnReserveMemory()
   http://marc.info/?l=linux-omap&m=126637211831587&w=2
3. Rebased patchset on top of following commit:
   DSPBRIDGE: Remove conditional check from the InputMsg function

Changed since v1:
1. Reduced indentation
   http://marc.info/?l=linux-omap&m=126624982331523&w=2

Ameya Palande (4):
  DSPBRIDGE: Rename DMM_RES_OBJECT to DMM_MAP_OBJECT
  DSPBRIDGE: New reserved memory accounting framework
  DSPBRIDGE: Fix memory corruption in DRV_ProcFreeDMMRes
  DSPBRIDGE: Improved mapped memory cleanup

 arch/arm/plat-omap/include/dspbridge/drv.h         |   44 ++---
 arch/arm/plat-omap/include/dspbridge/proc.h        |    4 +-
 .../plat-omap/include/dspbridge/resourcecleanup.h  |   11 --
 drivers/dsp/bridge/pmgr/wcd.c                      |    7 +-
 drivers/dsp/bridge/rmgr/drv.c                      |  176 +++-----------------
 drivers/dsp/bridge/rmgr/drv_interface.c            |    5 +-
 drivers/dsp/bridge/rmgr/node.c                     |    5 +-
 drivers/dsp/bridge/rmgr/proc.c                     |  105 +++++++++---
 8 files changed, 134 insertions(+), 223 deletions(-)

[PATCH 1/4] DSPBRIDGE: Rename DMM_RES_OBJECT to DMM_MAP_OBJECT

[Ameya Palande]

In its current state DMM_RES_OBJECT is not correctly tracking reserve and
unreserve but only map and unmap. So lets rename it to DMM_MAP_OBJECT to
clarify what it is really doing!

In addition to this, this patch also does some trivial code cleanup.

Signed-off-by: Ameya Palande <ameya.palande@nokia.com>
Reviewed-by: Felipe Contreras <felipe.contreras@nokia.com>
---
 arch/arm/plat-omap/include/dspbridge/drv.h |   14 +++---
 drivers/dsp/bridge/rmgr/drv.c              |   69 ++++++++++++++--------------
 drivers/dsp/bridge/rmgr/drv_interface.c    |    2 +-
 3 files changed, 43 insertions(+), 42 deletions(-)

diff --git a/arch/arm/plat-omap/include/dspbridge/drv.h b/arch/arm/plat-omap/include/dspbridge/drv.h
index b044291..d5f5277 100644
--- a/arch/arm/plat-omap/include/dspbridge/drv.h
+++ b/arch/arm/plat-omap/include/dspbridge/drv.h
@@ -90,15 +90,15 @@ struct NODE_RES_OBJECT {
 	struct NODE_RES_OBJECT         *next;
 } ;
 
-/* New structure (member of process context) abstracts DMM resource info */
-struct DMM_RES_OBJECT {
+/* Abstracts DMM resource info */
+struct DMM_MAP_OBJECT {
 	s32            dmmAllocated; /* DMM status */
 	u32           ulMpuAddr;
 	u32           ulDSPAddr;
 	u32           ulDSPResAddr;
-	u32           dmmSize;
+	u32           size;
 	HANDLE          hProcessor;
-	struct DMM_RES_OBJECT  *next;
+	struct DMM_MAP_OBJECT  *next;
 } ;
 
 /* New structure (member of process context) abstracts DMM resource info */
@@ -139,9 +139,9 @@ struct PROCESS_CONTEXT{
 	struct NODE_RES_OBJECT *pNodeList;
 	struct mutex node_mutex;
 
-	/* DMM resources */
-	struct DMM_RES_OBJECT *pDMMList;
-	struct mutex dmm_mutex;
+	/* DMM mapped memory resources */
+	struct DMM_MAP_OBJECT *dmm_map_list;
+	struct mutex dmm_map_mutex;
 
 	/* DSP Heap resources */
 	struct DSPHEAP_RES_OBJECT *pDSPHEAPList;
diff --git a/drivers/dsp/bridge/rmgr/drv.c b/drivers/dsp/bridge/rmgr/drv.c
index e8b2c58..9b513e2 100644
--- a/drivers/dsp/bridge/rmgr/drv.c
+++ b/drivers/dsp/bridge/rmgr/drv.c
@@ -198,37 +198,37 @@ static DSP_STATUS DRV_ProcFreeNodeRes(HANDLE hPCtxt)
 DSP_STATUS DRV_InsertDMMResElement(HANDLE hDMMRes, HANDLE hPCtxt)
 {
 	struct PROCESS_CONTEXT *pCtxt = (struct PROCESS_CONTEXT *)hPCtxt;
-	struct DMM_RES_OBJECT **pDMMRes = (struct DMM_RES_OBJECT **)hDMMRes;
+	struct DMM_MAP_OBJECT **pDMMRes = (struct DMM_MAP_OBJECT **)hDMMRes;
 	DSP_STATUS	status = DSP_SOK;
-	struct DMM_RES_OBJECT *pTempDMMRes = NULL;
+	struct DMM_MAP_OBJECT *pTempDMMRes = NULL;
 
-	*pDMMRes = (struct DMM_RES_OBJECT *)
-		    MEM_Calloc(1 * sizeof(struct DMM_RES_OBJECT), MEM_PAGED);
+	*pDMMRes = (struct DMM_MAP_OBJECT *)
+		    MEM_Calloc(1 * sizeof(struct DMM_MAP_OBJECT), MEM_PAGED);
 	GT_0trace(curTrace, GT_ENTER, "DRV_InsertDMMResElement: 1");
 	if (*pDMMRes == NULL) {
 		GT_0trace(curTrace, GT_5CLASS, "DRV_InsertDMMResElement: 2");
 		status = DSP_EHANDLE;
 	}
 	if (DSP_SUCCEEDED(status)) {
-		if (mutex_lock_interruptible(&pCtxt->dmm_mutex)) {
+		if (mutex_lock_interruptible(&pCtxt->dmm_map_mutex)) {
 			kfree(*pDMMRes);
 			return DSP_EFAIL;
 		}
 
-		if (pCtxt->pDMMList != NULL) {
+		if (pCtxt->dmm_map_list) {
 			GT_0trace(curTrace, GT_5CLASS,
 				 "DRV_InsertDMMResElement: 3");
-			pTempDMMRes = pCtxt->pDMMList;
-			while (pTempDMMRes->next != NULL)
+			pTempDMMRes = pCtxt->dmm_map_list;
+			while (pTempDMMRes->next)
 				pTempDMMRes = pTempDMMRes->next;
 
 			pTempDMMRes->next = *pDMMRes;
 		} else {
-			pCtxt->pDMMList = *pDMMRes;
+			pCtxt->dmm_map_list = *pDMMRes;
 			GT_0trace(curTrace, GT_5CLASS,
 				 "DRV_InsertDMMResElement: 4");
 		}
-		mutex_unlock(&pCtxt->dmm_mutex);
+		mutex_unlock(&pCtxt->dmm_map_mutex);
 	}
 	GT_0trace(curTrace, GT_ENTER, "DRV_InsertDMMResElement: 5");
 	return status;
@@ -239,15 +239,15 @@ DSP_STATUS DRV_InsertDMMResElement(HANDLE hDMMRes, HANDLE hPCtxt)
 DSP_STATUS DRV_RemoveDMMResElement(HANDLE hDMMRes, HANDLE hPCtxt)
 {
 	struct PROCESS_CONTEXT *pCtxt = (struct PROCESS_CONTEXT *)hPCtxt;
-	struct DMM_RES_OBJECT *pDMMRes = (struct DMM_RES_OBJECT *)hDMMRes;
-	struct DMM_RES_OBJECT *pTempDMMRes = NULL;
+	struct DMM_MAP_OBJECT *pDMMRes = (struct DMM_MAP_OBJECT *)hDMMRes;
+	struct DMM_MAP_OBJECT *pTempDMMRes = NULL;
 	DSP_STATUS status = DSP_SOK;
 
-	if (mutex_lock_interruptible(&pCtxt->dmm_mutex))
+	if (mutex_lock_interruptible(&pCtxt->dmm_map_mutex))
 		return DSP_EFAIL;
-	pTempDMMRes = pCtxt->pDMMList;
-	if (pCtxt->pDMMList == pDMMRes) {
-		pCtxt->pDMMList = pDMMRes->next;
+	pTempDMMRes = pCtxt->dmm_map_list;
+	if (pCtxt->dmm_map_list == pDMMRes) {
+		pCtxt->dmm_map_list = pDMMRes->next;
 	} else {
 		while (pTempDMMRes && pTempDMMRes->next != pDMMRes)
 			pTempDMMRes = pTempDMMRes->next;
@@ -256,7 +256,7 @@ DSP_STATUS DRV_RemoveDMMResElement(HANDLE hDMMRes, HANDLE hPCtxt)
 		else
 			pTempDMMRes->next = pDMMRes->next;
 	}
-	mutex_unlock(&pCtxt->dmm_mutex);
+	mutex_unlock(&pCtxt->dmm_map_mutex);
 	kfree(pDMMRes);
 	return status;
 }
@@ -266,14 +266,14 @@ DSP_STATUS DRV_UpdateDMMResElement(HANDLE hDMMRes, u32 pMpuAddr, u32 ulSize,
 				  u32 pReqAddr, u32 pMapAddr,
 				  HANDLE hProcessor)
 {
-	struct DMM_RES_OBJECT *pDMMRes = (struct DMM_RES_OBJECT *)hDMMRes;
+	struct DMM_MAP_OBJECT *pDMMRes = (struct DMM_MAP_OBJECT *)hDMMRes;
 	DSP_STATUS status = DSP_SOK;
 
 	DBC_Assert(hDMMRes != NULL);
 	pDMMRes->ulMpuAddr = pMpuAddr;
 	pDMMRes->ulDSPAddr = pMapAddr;
 	pDMMRes->ulDSPResAddr = pReqAddr;
-	pDMMRes->dmmSize = ulSize;
+	pDMMRes->size = ulSize;
 	pDMMRes->hProcessor = hProcessor;
 	pDMMRes->dmmAllocated = 1;
 
@@ -285,11 +285,11 @@ DSP_STATUS  DRV_ProcFreeDMMRes(HANDLE hPCtxt)
 {
 	struct PROCESS_CONTEXT *pCtxt = (struct PROCESS_CONTEXT *)hPCtxt;
 	DSP_STATUS status = DSP_SOK;
-	struct DMM_RES_OBJECT *pDMMList = pCtxt->pDMMList;
-	struct DMM_RES_OBJECT *pDMMRes = NULL;
+	struct DMM_MAP_OBJECT *pDMMList = pCtxt->dmm_map_list;
+	struct DMM_MAP_OBJECT *pDMMRes = NULL;
 
 	GT_0trace(curTrace, GT_ENTER, "\nDRV_ProcFreeDMMRes: 1\n");
-	while (pDMMList != NULL) {
+	while (pDMMList) {
 		pDMMRes = pDMMList;
 		pDMMList = pDMMList->next;
 		if (pDMMRes->dmmAllocated) {
@@ -315,32 +315,32 @@ DSP_STATUS DRV_RemoveAllDMMResElements(HANDLE hPCtxt)
 {
 	struct PROCESS_CONTEXT *pCtxt = (struct PROCESS_CONTEXT *)hPCtxt;
 	DSP_STATUS status = DSP_SOK;
-	struct DMM_RES_OBJECT *pTempDMMRes2 = NULL;
-	struct DMM_RES_OBJECT *pTempDMMRes = NULL;
+	struct DMM_MAP_OBJECT *pTempDMMRes2 = NULL;
+	struct DMM_MAP_OBJECT *pTempDMMRes = NULL;
 
 	DRV_ProcFreeDMMRes(pCtxt);
-	pTempDMMRes = pCtxt->pDMMList;
+	pTempDMMRes = pCtxt->dmm_map_list;
 	while (pTempDMMRes != NULL) {
 		pTempDMMRes2 = pTempDMMRes;
 		pTempDMMRes = pTempDMMRes->next;
 		kfree(pTempDMMRes2);
 	}
-	pCtxt->pDMMList = NULL;
+	pCtxt->dmm_map_list = NULL;
 	return status;
 }
 
 DSP_STATUS DRV_GetDMMResElement(u32 pMapAddr, HANDLE hDMMRes, HANDLE hPCtxt)
 {
 	struct PROCESS_CONTEXT *pCtxt = (struct PROCESS_CONTEXT *)hPCtxt;
-	struct DMM_RES_OBJECT **pDMMRes = (struct DMM_RES_OBJECT **)hDMMRes;
+	struct DMM_MAP_OBJECT **pDMMRes = (struct DMM_MAP_OBJECT **)hDMMRes;
 	DSP_STATUS status = DSP_SOK;
-	struct DMM_RES_OBJECT *pTempDMM = NULL;
+	struct DMM_MAP_OBJECT *pTempDMM = NULL;
 
-	if (mutex_lock_interruptible(&pCtxt->dmm_mutex))
+	if (mutex_lock_interruptible(&pCtxt->dmm_map_mutex))
 		return DSP_EFAIL;
 
-	pTempDMM = pCtxt->pDMMList;
-	while ((pTempDMM != NULL) && (pTempDMM->ulDSPAddr != pMapAddr)) {
+	pTempDMM = pCtxt->dmm_map_list;
+	while (pTempDMM && (pTempDMM->ulDSPAddr != pMapAddr)) {
 		GT_3trace(curTrace, GT_ENTER,
 			 "DRV_GetDMMResElement: 2 pTempDMM:%x "
 			 "pTempDMM->ulDSPAddr:%x pMapAddr:%x\n", pTempDMM,
@@ -348,14 +348,15 @@ DSP_STATUS DRV_GetDMMResElement(u32 pMapAddr, HANDLE hDMMRes, HANDLE hPCtxt)
 		pTempDMM = pTempDMM->next;
 	}
 
-	mutex_unlock(&pCtxt->dmm_mutex);
+	mutex_unlock(&pCtxt->dmm_map_mutex);
 
-	if (pTempDMM != NULL) {
+	if (pTempDMM) {
 		GT_0trace(curTrace, GT_ENTER, "DRV_GetDMMResElement: 3");
 		*pDMMRes = pTempDMM;
 	} else {
 		status = DSP_ENOTFOUND;
-	} GT_0trace(curTrace, GT_ENTER, "DRV_GetDMMResElement: 4");
+		GT_0trace(curTrace, GT_ENTER, "DRV_GetDMMResElement: 4");
+	}
 	return status;
 }
 
diff --git a/drivers/dsp/bridge/rmgr/drv_interface.c b/drivers/dsp/bridge/rmgr/drv_interface.c
index f083ba7..e6a7eb7 100644
--- a/drivers/dsp/bridge/rmgr/drv_interface.c
+++ b/drivers/dsp/bridge/rmgr/drv_interface.c
@@ -499,7 +499,7 @@ static int bridge_open(struct inode *ip, struct file *filp)
 	pr_ctxt = MEM_Calloc(sizeof(struct PROCESS_CONTEXT), MEM_PAGED);
 	if (pr_ctxt) {
 		pr_ctxt->resState = PROC_RES_ALLOCATED;
-		mutex_init(&pr_ctxt->dmm_mutex);
+		mutex_init(&pr_ctxt->dmm_map_mutex);
 		mutex_init(&pr_ctxt->node_mutex);
 		mutex_init(&pr_ctxt->strm_mutex);
 	} else {
-- 
1.6.3.3

[PATCH 2/4] DSPBRIDGE: New reserved memory accounting framework

[Ameya Palande]

DSP_RSV_OBJECT is introduced to track reserved memory accounting information.
This will allow us to do proper cleanup for memory reserved using
PROC_ReserveMemory().

Signed-off-by: Ameya Palande <ameya.palande@nokia.com>
---
 arch/arm/plat-omap/include/dspbridge/drv.h  |   10 ++++
 arch/arm/plat-omap/include/dspbridge/proc.h |    4 +-
 drivers/dsp/bridge/pmgr/wcd.c               |    7 ++-
 drivers/dsp/bridge/rmgr/drv.c               |   18 +++++---
 drivers/dsp/bridge/rmgr/drv_interface.c     |    2 +
 drivers/dsp/bridge/rmgr/node.c              |    5 +-
 drivers/dsp/bridge/rmgr/proc.c              |   62 ++++++++++++++++++++++-----
 7 files changed, 83 insertions(+), 25 deletions(-)

diff --git a/arch/arm/plat-omap/include/dspbridge/drv.h b/arch/arm/plat-omap/include/dspbridge/drv.h
index d5f5277..f7d0e3e 100644
--- a/arch/arm/plat-omap/include/dspbridge/drv.h
+++ b/arch/arm/plat-omap/include/dspbridge/drv.h
@@ -101,6 +101,12 @@ struct DMM_MAP_OBJECT {
 	struct DMM_MAP_OBJECT  *next;
 } ;
 
+/* Used for DMM reserved memory accounting */
+struct DMM_RSV_OBJECT {
+	struct	list_head	link;
+	u32	dsp_reserved_addr;
+};
+
 /* New structure (member of process context) abstracts DMM resource info */
 struct DSPHEAP_RES_OBJECT {
 	s32            heapAllocated; /* DMM status */
@@ -143,6 +149,10 @@ struct PROCESS_CONTEXT{
 	struct DMM_MAP_OBJECT *dmm_map_list;
 	struct mutex dmm_map_mutex;
 
+	/* DMM reserved memory resources */
+	struct list_head dmm_rsv_list;
+	spinlock_t dmm_rsv_lock;
+
 	/* DSP Heap resources */
 	struct DSPHEAP_RES_OBJECT *pDSPHEAPList;
 
diff --git a/arch/arm/plat-omap/include/dspbridge/proc.h b/arch/arm/plat-omap/include/dspbridge/proc.h
index 8dbdaac..1ffe763 100644
--- a/arch/arm/plat-omap/include/dspbridge/proc.h
+++ b/arch/arm/plat-omap/include/dspbridge/proc.h
@@ -560,7 +560,7 @@
  *  Details:
  */
 	extern DSP_STATUS PROC_ReserveMemory(DSP_HPROCESSOR hProcessor,
-					     u32 ulSize, void **ppRsvAddr);
+		u32 ulSize, void **ppRsvAddr, struct PROCESS_CONTEXT *pr_ctxt);
 
 /*
  *  ======== PROC_UnMap ========
@@ -604,6 +604,6 @@
  *  Details:
  */
 	extern DSP_STATUS PROC_UnReserveMemory(DSP_HPROCESSOR hProcessor,
-					       void *pRsvAddr);
+			void *pRsvAddr, struct PROCESS_CONTEXT *pr_ctxt);
 
 #endif				/* PROC_ */
diff --git a/drivers/dsp/bridge/pmgr/wcd.c b/drivers/dsp/bridge/pmgr/wcd.c
index beea23b..1ef606e 100644
--- a/drivers/dsp/bridge/pmgr/wcd.c
+++ b/drivers/dsp/bridge/pmgr/wcd.c
@@ -1054,12 +1054,13 @@ u32 PROCWRAP_ReserveMemory(union Trapped_Args *args, void *pr_ctxt)
 
 	GT_0trace(WCD_debugMask, GT_ENTER, "PROCWRAP_ReserveMemory: entered\n");
 	status = PROC_ReserveMemory(args->ARGS_PROC_RSVMEM.hProcessor,
-				   args->ARGS_PROC_RSVMEM.ulSize, &pRsvAddr);
+				   args->ARGS_PROC_RSVMEM.ulSize, &pRsvAddr,
+				   pr_ctxt);
 	if (DSP_SUCCEEDED(status)) {
 		if (put_user(pRsvAddr, args->ARGS_PROC_RSVMEM.ppRsvAddr)) {
 			status = DSP_EINVALIDARG;
 			PROC_UnReserveMemory(args->ARGS_PROC_RSVMEM.hProcessor,
-				pRsvAddr);
+				pRsvAddr, pr_ctxt);
 		}
 	}
 	return status;
@@ -1100,7 +1101,7 @@ u32 PROCWRAP_UnReserveMemory(union Trapped_Args *args, void *pr_ctxt)
 	GT_0trace(WCD_debugMask, GT_ENTER,
 		 "PROCWRAP_UnReserveMemory: entered\n");
 	status = PROC_UnReserveMemory(args->ARGS_PROC_UNRSVMEM.hProcessor,
-				     args->ARGS_PROC_UNRSVMEM.pRsvAddr);
+			     args->ARGS_PROC_UNRSVMEM.pRsvAddr, pr_ctxt);
 	return status;
 }
 
diff --git a/drivers/dsp/bridge/rmgr/drv.c b/drivers/dsp/bridge/rmgr/drv.c
index 9b513e2..12ba7e0 100644
--- a/drivers/dsp/bridge/rmgr/drv.c
+++ b/drivers/dsp/bridge/rmgr/drv.c
@@ -298,25 +298,20 @@ DSP_STATUS  DRV_ProcFreeDMMRes(HANDLE hPCtxt)
 			if (DSP_FAILED(status))
 				pr_debug("%s: PROC_UnMap failed! status ="
 						" 0x%xn", __func__, status);
-			status = PROC_UnReserveMemory(pDMMRes->hProcessor,
-				 (void *)pDMMRes->ulDSPResAddr);
-			if (DSP_FAILED(status))
-				pr_debug("%s: PROC_UnReserveMemory failed!"
-					" status = 0x%xn", __func__, status);
 			pDMMRes->dmmAllocated = 0;
 		}
 	}
 	return status;
 }
 
-/* Release all DMM resources and its context
-* This is called from .bridge_release. */
+/* Release all Mapped and Reserved DMM resources */
 DSP_STATUS DRV_RemoveAllDMMResElements(HANDLE hPCtxt)
 {
 	struct PROCESS_CONTEXT *pCtxt = (struct PROCESS_CONTEXT *)hPCtxt;
 	DSP_STATUS status = DSP_SOK;
 	struct DMM_MAP_OBJECT *pTempDMMRes2 = NULL;
 	struct DMM_MAP_OBJECT *pTempDMMRes = NULL;
+	struct DMM_RSV_OBJECT *temp, *rsv_obj;
 
 	DRV_ProcFreeDMMRes(pCtxt);
 	pTempDMMRes = pCtxt->dmm_map_list;
@@ -326,6 +321,15 @@ DSP_STATUS DRV_RemoveAllDMMResElements(HANDLE hPCtxt)
 		kfree(pTempDMMRes2);
 	}
 	pCtxt->dmm_map_list = NULL;
+
+	/* Free DMM reserved memory resources */
+	list_for_each_entry_safe(rsv_obj, temp, &pCtxt->dmm_rsv_list, link) {
+		status = PROC_UnReserveMemory(pCtxt->hProcessor,
+				(void *)rsv_obj->dsp_reserved_addr, pCtxt);
+		if (DSP_FAILED(status))
+			pr_err("%s: PROC_UnReserveMemory failed!"
+					" status = 0x%xn", __func__, status);
+	}
 	return status;
 }
 
diff --git a/drivers/dsp/bridge/rmgr/drv_interface.c b/drivers/dsp/bridge/rmgr/drv_interface.c
index e6a7eb7..80b8c7e 100644
--- a/drivers/dsp/bridge/rmgr/drv_interface.c
+++ b/drivers/dsp/bridge/rmgr/drv_interface.c
@@ -500,6 +500,8 @@ static int bridge_open(struct inode *ip, struct file *filp)
 	if (pr_ctxt) {
 		pr_ctxt->resState = PROC_RES_ALLOCATED;
 		mutex_init(&pr_ctxt->dmm_map_mutex);
+		spin_lock_init(&pr_ctxt->dmm_rsv_lock);
+		INIT_LIST_HEAD(&pr_ctxt->dmm_rsv_list);
 		mutex_init(&pr_ctxt->node_mutex);
 		mutex_init(&pr_ctxt->strm_mutex);
 	} else {
diff --git a/drivers/dsp/bridge/rmgr/node.c b/drivers/dsp/bridge/rmgr/node.c
index b60d1ed..17b07ed 100644
--- a/drivers/dsp/bridge/rmgr/node.c
+++ b/drivers/dsp/bridge/rmgr/node.c
@@ -454,7 +454,7 @@ DSP_STATUS NODE_Allocate(struct PROC_OBJECT *hProcessor,
 	status = PROC_ReserveMemory(hProcessor,
 			pNode->createArgs.asa.taskArgs.uHeapSize + PAGE_SIZE,
 			(void **)&(pNode->createArgs.asa.taskArgs.
-				uDSPHeapResAddr));
+				uDSPHeapResAddr), pr_ctxt);
 	if (DSP_FAILED(status)) {
 		GT_1trace(NODE_debugMask, GT_5CLASS,
 			 "NODE_Allocate:Failed to reserve "
@@ -2726,7 +2726,8 @@ static void DeleteNode(struct NODE_OBJECT *hNode,
 					 " Status = 0x%x\n", (u32)status);
 			}
 			status = PROC_UnReserveMemory(hNode->hProcessor,
-					(void *)taskArgs.uDSPHeapResAddr);
+					(void *)taskArgs.uDSPHeapResAddr,
+					pr_ctxt);
 			if (DSP_SUCCEEDED(status)) {
 				GT_0trace(NODE_debugMask, GT_5CLASS,
 					 "DSPProcessor_UnReserveMemory "
diff --git a/drivers/dsp/bridge/rmgr/proc.c b/drivers/dsp/bridge/rmgr/proc.c
index 6c0173a..b8e0334 100644
--- a/drivers/dsp/bridge/rmgr/proc.c
+++ b/drivers/dsp/bridge/rmgr/proc.c
@@ -1431,11 +1431,12 @@ func_end:
  *      Reserve a virtually contiguous region of DSP address space.
  */
 DSP_STATUS PROC_ReserveMemory(DSP_HPROCESSOR hProcessor, u32 ulSize,
-			     void **ppRsvAddr)
+		     void **ppRsvAddr, struct PROCESS_CONTEXT *pr_ctxt)
 {
 	struct DMM_OBJECT *hDmmMgr;
 	DSP_STATUS status = DSP_SOK;
 	struct PROC_OBJECT *pProcObject = (struct PROC_OBJECT *)hProcessor;
+	struct DMM_RSV_OBJECT *rsv_obj;
 
 	GT_3trace(PROC_DebugMask, GT_ENTER,
 		 "Entered PROC_ReserveMemory, args:\n\t"
@@ -1447,16 +1448,34 @@ DSP_STATUS PROC_ReserveMemory(DSP_HPROCESSOR hProcessor, u32 ulSize,
 			 "InValid Processor Handle \n");
 		goto func_end;
 	}
+
 	status = DMM_GetHandle(pProcObject, &hDmmMgr);
 	if (DSP_FAILED(status)) {
 		GT_1trace(PROC_DebugMask, GT_7CLASS, "PROC_ReserveMemory: "
 			 "Failed to get DMM Mgr handle: 0x%x\n", status);
-	} else
-		status = DMM_ReserveMemory(hDmmMgr, ulSize, (u32 *)ppRsvAddr);
+		goto func_end;
+	}
+
+	status = DMM_ReserveMemory(hDmmMgr, ulSize, (u32 *)ppRsvAddr);
+	if (status != DSP_SOK)
+		goto func_end;
 
+	/*
+	 * A successful reserve should be followed by insertion of rsv_obj
+	 * into dmm_rsv_list, so that reserved memory resource tracking
+	 * remains uptodate
+	 */
+	rsv_obj = kmalloc(sizeof(struct DMM_RSV_OBJECT), GFP_KERNEL);
+	if (rsv_obj) {
+		rsv_obj->dsp_reserved_addr = (u32) *ppRsvAddr;
+		spin_lock(&pr_ctxt->dmm_rsv_lock);
+		list_add(&rsv_obj->link, &pr_ctxt->dmm_rsv_list);
+		spin_unlock(&pr_ctxt->dmm_rsv_lock);
+	}
+
+func_end:
 	GT_1trace(PROC_DebugMask, GT_ENTER, "Leaving PROC_ReserveMemory [0x%x]",
 		 status);
-func_end:
 	return status;
 }
 
@@ -1705,11 +1724,13 @@ func_end:
  *  Purpose:
  *      Frees a previously reserved region of DSP address space.
  */
-DSP_STATUS PROC_UnReserveMemory(DSP_HPROCESSOR hProcessor, void *pRsvAddr)
+DSP_STATUS PROC_UnReserveMemory(DSP_HPROCESSOR hProcessor, void *pRsvAddr,
+		struct PROCESS_CONTEXT *pr_ctxt)
 {
 	struct DMM_OBJECT *hDmmMgr;
 	DSP_STATUS status = DSP_SOK;
 	struct PROC_OBJECT *pProcObject = (struct PROC_OBJECT *)hProcessor;
+	struct DMM_RSV_OBJECT *rsv_obj;
 
 	GT_2trace(PROC_DebugMask, GT_ENTER,
 		 "Entered PROC_UnReserveMemory, args:\n\t"
@@ -1720,18 +1741,37 @@ DSP_STATUS PROC_UnReserveMemory(DSP_HPROCESSOR hProcessor, void *pRsvAddr)
 			 "InValid Processor Handle \n");
 		goto func_end;
 	}
+
 	status = DMM_GetHandle(pProcObject, &hDmmMgr);
-	if (DSP_FAILED(status))
+	if (DSP_FAILED(status)) {
 		GT_1trace(PROC_DebugMask, GT_7CLASS,
 			 "PROC_UnReserveMemory: Failed to get DMM Mgr "
 			 "handle: 0x%x\n", status);
-	else
-		status = DMM_UnReserveMemory(hDmmMgr, (u32) pRsvAddr);
+		goto func_end;
+	}
+
+	status = DMM_UnReserveMemory(hDmmMgr, (u32) pRsvAddr);
+	if (status != DSP_SOK)
+		goto func_end;
+
+	/*
+	 * A successful unreserve should be followed by removal of rsv_obj
+	 * from dmm_rsv_list, so that reserved memory resource tracking
+	 * remains uptodate
+	 */
+	spin_lock(&pr_ctxt->dmm_rsv_lock);
+	list_for_each_entry(rsv_obj, &pr_ctxt->dmm_rsv_list, link) {
+		if (rsv_obj->dsp_reserved_addr == (u32)pRsvAddr) {
+			list_del(&rsv_obj->link);
+			kfree(rsv_obj);
+			break;
+		}
+	}
+	spin_unlock(&pr_ctxt->dmm_rsv_lock);
 
-	GT_1trace(PROC_DebugMask, GT_ENTER,
-		 "Leaving PROC_UnReserveMemory [0x%x]",
-		 status);
 func_end:
+	GT_1trace(PROC_DebugMask, GT_ENTER,
+		 "Leaving PROC_UnReserveMemory [0x%x]", status);
 	return status;
 }
 
-- 
1.6.3.3

[PATCH 3/4] DSPBRIDGE: Fix memory corruption in DRV_ProcFreeDMMRes

[Ameya Palande]

This patch fixes following issues:

1. pDMMRes was dereferenced and modified when it was already freed by
   PROC_Ummap(). This results in memory corruption.

2. Instead of passing ulDSPAddr, ulDSPResAddr was passed to PROC_UnMap()
   which will not retrieve correct DMMRes element.

Signed-off-by: Ameya Palande <ameya.palande@nokia.com>
Signed-off-by: Felipe Contreras <felipe.contreras@nokia.com>
---
 drivers/dsp/bridge/rmgr/drv.c |   12 ++----------
 1 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/dsp/bridge/rmgr/drv.c b/drivers/dsp/bridge/rmgr/drv.c
index 12ba7e0..b88b5a3 100644
--- a/drivers/dsp/bridge/rmgr/drv.c
+++ b/drivers/dsp/bridge/rmgr/drv.c
@@ -293,12 +293,12 @@ DSP_STATUS  DRV_ProcFreeDMMRes(HANDLE hPCtxt)
 		pDMMRes = pDMMList;
 		pDMMList = pDMMList->next;
 		if (pDMMRes->dmmAllocated) {
+			/* PROC_UnMap will free pDMMRes pointer */
 			status = PROC_UnMap(pDMMRes->hProcessor,
-				 (void *)pDMMRes->ulDSPResAddr, pCtxt);
+				 (void *)pDMMRes->ulDSPAddr, pCtxt);
 			if (DSP_FAILED(status))
 				pr_debug("%s: PROC_UnMap failed! status ="
 						" 0x%xn", __func__, status);
-			pDMMRes->dmmAllocated = 0;
 		}
 	}
 	return status;
@@ -309,17 +309,9 @@ DSP_STATUS DRV_RemoveAllDMMResElements(HANDLE hPCtxt)
 {
 	struct PROCESS_CONTEXT *pCtxt = (struct PROCESS_CONTEXT *)hPCtxt;
 	DSP_STATUS status = DSP_SOK;
-	struct DMM_MAP_OBJECT *pTempDMMRes2 = NULL;
-	struct DMM_MAP_OBJECT *pTempDMMRes = NULL;
 	struct DMM_RSV_OBJECT *temp, *rsv_obj;
 
 	DRV_ProcFreeDMMRes(pCtxt);
-	pTempDMMRes = pCtxt->dmm_map_list;
-	while (pTempDMMRes != NULL) {
-		pTempDMMRes2 = pTempDMMRes;
-		pTempDMMRes = pTempDMMRes->next;
-		kfree(pTempDMMRes2);
-	}
 	pCtxt->dmm_map_list = NULL;
 
 	/* Free DMM reserved memory resources */
-- 
1.6.3.3

[PATCH 4/4] DSPBRIDGE: Improved mapped memory cleanup

[Ameya Palande]

This patch improves current mapped memory cleanup mechanism by using linux
native list implementation. As a side effect we also get following benefits:

1. Unnecessary data members in DMM_MAP_OBJECT are removed which results in
   memory saving.

2. Following functions are removed as they are not needed anymore:
   DRV_ProcFreeDMMRes()
   DRV_UpdateDMMResElement()
   DRV_InsertDMMResElement()
   DRV_GetDMMResElement()
   DRV_RemoveDMMResElement()

Signed-off-by: Ameya Palande <ameya.palande@nokia.com>
---
 arch/arm/plat-omap/include/dspbridge/drv.h         |   30 +---
 .../plat-omap/include/dspbridge/resourcecleanup.h  |   11 --
 drivers/dsp/bridge/rmgr/drv.c                      |  161 ++------------------
 drivers/dsp/bridge/rmgr/drv_interface.c            |    3 +-
 drivers/dsp/bridge/rmgr/proc.c                     |   43 ++++--
 5 files changed, 54 insertions(+), 194 deletions(-)

diff --git a/arch/arm/plat-omap/include/dspbridge/drv.h b/arch/arm/plat-omap/include/dspbridge/drv.h
index f7d0e3e..854923a 100644
--- a/arch/arm/plat-omap/include/dspbridge/drv.h
+++ b/arch/arm/plat-omap/include/dspbridge/drv.h
@@ -90,16 +90,11 @@ struct NODE_RES_OBJECT {
 	struct NODE_RES_OBJECT         *next;
 } ;
 
-/* Abstracts DMM resource info */
+/* Used for DMM mapped memory accounting */
 struct DMM_MAP_OBJECT {
-	s32            dmmAllocated; /* DMM status */
-	u32           ulMpuAddr;
-	u32           ulDSPAddr;
-	u32           ulDSPResAddr;
-	u32           size;
-	HANDLE          hProcessor;
-	struct DMM_MAP_OBJECT  *next;
-} ;
+	struct	list_head	link;
+	u32	dsp_addr;
+};
 
 /* Used for DMM reserved memory accounting */
 struct DMM_RSV_OBJECT {
@@ -146,8 +141,8 @@ struct PROCESS_CONTEXT{
 	struct mutex node_mutex;
 
 	/* DMM mapped memory resources */
-	struct DMM_MAP_OBJECT *dmm_map_list;
-	struct mutex dmm_map_mutex;
+	struct list_head dmm_map_list;
+	spinlock_t dmm_map_lock;
 
 	/* DMM reserved memory resources */
 	struct list_head dmm_rsv_list;
@@ -398,17 +393,4 @@ struct PROCESS_CONTEXT{
 	extern DSP_STATUS DRV_ReleaseResources(IN u32 dwContext,
 					       struct DRV_OBJECT *hDrvObject);
 
-/*
- *  ======== DRV_ProcFreeDMMRes ========
- *  Purpose:
- *       Actual DMM De-Allocation.
- *  Parameters:
- *      hPCtxt:      Path to the driver Registry Key.
- *  Returns:
- *      DSP_SOK if success;
- */
-
-
-	extern DSP_STATUS  DRV_ProcFreeDMMRes(HANDLE hPCtxt);
-
 #endif				/* DRV_ */
diff --git a/arch/arm/plat-omap/include/dspbridge/resourcecleanup.h b/arch/arm/plat-omap/include/dspbridge/resourcecleanup.h
index ffbcf5e..2bb756a 100644
--- a/arch/arm/plat-omap/include/dspbridge/resourcecleanup.h
+++ b/arch/arm/plat-omap/include/dspbridge/resourcecleanup.h
@@ -48,17 +48,6 @@ extern DSP_STATUS DRV_RemoveNodeResElement(HANDLE nodeRes, HANDLE status);
 
 extern void DRV_ProcNodeUpdateStatus(HANDLE hNodeRes, s32 status);
 
-extern DSP_STATUS DRV_UpdateDMMResElement(HANDLE dmmRes, u32 pMpuAddr,
-					  u32 ulSize, u32 pReqAddr,
-					  u32 ppMapAddr, HANDLE hProcesso);
-
-extern DSP_STATUS DRV_InsertDMMResElement(HANDLE dmmRes, HANDLE pCtxt);
-
-extern DSP_STATUS DRV_GetDMMResElement(u32 pMapAddr, HANDLE dmmRes,
-				       HANDLE pCtxt);
-
-extern DSP_STATUS DRV_RemoveDMMResElement(HANDLE dmmRes, HANDLE pCtxt);
-
 extern DSP_STATUS DRV_ProcUpdateSTRMRes(u32 uNumBufs, HANDLE STRMRes);
 
 extern DSP_STATUS DRV_ProcInsertSTRMResElement(HANDLE hStrm, HANDLE STRMRes,
diff --git a/drivers/dsp/bridge/rmgr/drv.c b/drivers/dsp/bridge/rmgr/drv.c
index b88b5a3..bb6c246 100644
--- a/drivers/dsp/bridge/rmgr/drv.c
+++ b/drivers/dsp/bridge/rmgr/drv.c
@@ -193,129 +193,27 @@ static DSP_STATUS DRV_ProcFreeNodeRes(HANDLE hPCtxt)
 	return status;
 }
 
-/* Allocate the DMM resource element
-* This is called from Proc_Map. after the actual resource is allocated */
-DSP_STATUS DRV_InsertDMMResElement(HANDLE hDMMRes, HANDLE hPCtxt)
-{
-	struct PROCESS_CONTEXT *pCtxt = (struct PROCESS_CONTEXT *)hPCtxt;
-	struct DMM_MAP_OBJECT **pDMMRes = (struct DMM_MAP_OBJECT **)hDMMRes;
-	DSP_STATUS	status = DSP_SOK;
-	struct DMM_MAP_OBJECT *pTempDMMRes = NULL;
-
-	*pDMMRes = (struct DMM_MAP_OBJECT *)
-		    MEM_Calloc(1 * sizeof(struct DMM_MAP_OBJECT), MEM_PAGED);
-	GT_0trace(curTrace, GT_ENTER, "DRV_InsertDMMResElement: 1");
-	if (*pDMMRes == NULL) {
-		GT_0trace(curTrace, GT_5CLASS, "DRV_InsertDMMResElement: 2");
-		status = DSP_EHANDLE;
-	}
-	if (DSP_SUCCEEDED(status)) {
-		if (mutex_lock_interruptible(&pCtxt->dmm_map_mutex)) {
-			kfree(*pDMMRes);
-			return DSP_EFAIL;
-		}
-
-		if (pCtxt->dmm_map_list) {
-			GT_0trace(curTrace, GT_5CLASS,
-				 "DRV_InsertDMMResElement: 3");
-			pTempDMMRes = pCtxt->dmm_map_list;
-			while (pTempDMMRes->next)
-				pTempDMMRes = pTempDMMRes->next;
-
-			pTempDMMRes->next = *pDMMRes;
-		} else {
-			pCtxt->dmm_map_list = *pDMMRes;
-			GT_0trace(curTrace, GT_5CLASS,
-				 "DRV_InsertDMMResElement: 4");
-		}
-		mutex_unlock(&pCtxt->dmm_map_mutex);
-	}
-	GT_0trace(curTrace, GT_ENTER, "DRV_InsertDMMResElement: 5");
-	return status;
-}
-
-/* Release DMM resource element context
-* This is called from Proc_UnMap. after the actual resource is freed */
-DSP_STATUS DRV_RemoveDMMResElement(HANDLE hDMMRes, HANDLE hPCtxt)
-{
-	struct PROCESS_CONTEXT *pCtxt = (struct PROCESS_CONTEXT *)hPCtxt;
-	struct DMM_MAP_OBJECT *pDMMRes = (struct DMM_MAP_OBJECT *)hDMMRes;
-	struct DMM_MAP_OBJECT *pTempDMMRes = NULL;
-	DSP_STATUS status = DSP_SOK;
-
-	if (mutex_lock_interruptible(&pCtxt->dmm_map_mutex))
-		return DSP_EFAIL;
-	pTempDMMRes = pCtxt->dmm_map_list;
-	if (pCtxt->dmm_map_list == pDMMRes) {
-		pCtxt->dmm_map_list = pDMMRes->next;
-	} else {
-		while (pTempDMMRes && pTempDMMRes->next != pDMMRes)
-			pTempDMMRes = pTempDMMRes->next;
-		if (!pTempDMMRes)
-			status = DSP_ENOTFOUND;
-		else
-			pTempDMMRes->next = pDMMRes->next;
-	}
-	mutex_unlock(&pCtxt->dmm_map_mutex);
-	kfree(pDMMRes);
-	return status;
-}
-
-/* Update DMM resource status */
-DSP_STATUS DRV_UpdateDMMResElement(HANDLE hDMMRes, u32 pMpuAddr, u32 ulSize,
-				  u32 pReqAddr, u32 pMapAddr,
-				  HANDLE hProcessor)
-{
-	struct DMM_MAP_OBJECT *pDMMRes = (struct DMM_MAP_OBJECT *)hDMMRes;
-	DSP_STATUS status = DSP_SOK;
-
-	DBC_Assert(hDMMRes != NULL);
-	pDMMRes->ulMpuAddr = pMpuAddr;
-	pDMMRes->ulDSPAddr = pMapAddr;
-	pDMMRes->ulDSPResAddr = pReqAddr;
-	pDMMRes->size = ulSize;
-	pDMMRes->hProcessor = hProcessor;
-	pDMMRes->dmmAllocated = 1;
-
-	return status;
-}
-
-/* Actual DMM De-Allocation */
-DSP_STATUS  DRV_ProcFreeDMMRes(HANDLE hPCtxt)
-{
-	struct PROCESS_CONTEXT *pCtxt = (struct PROCESS_CONTEXT *)hPCtxt;
-	DSP_STATUS status = DSP_SOK;
-	struct DMM_MAP_OBJECT *pDMMList = pCtxt->dmm_map_list;
-	struct DMM_MAP_OBJECT *pDMMRes = NULL;
-
-	GT_0trace(curTrace, GT_ENTER, "\nDRV_ProcFreeDMMRes: 1\n");
-	while (pDMMList) {
-		pDMMRes = pDMMList;
-		pDMMList = pDMMList->next;
-		if (pDMMRes->dmmAllocated) {
-			/* PROC_UnMap will free pDMMRes pointer */
-			status = PROC_UnMap(pDMMRes->hProcessor,
-				 (void *)pDMMRes->ulDSPAddr, pCtxt);
-			if (DSP_FAILED(status))
-				pr_debug("%s: PROC_UnMap failed! status ="
-						" 0x%xn", __func__, status);
-		}
-	}
-	return status;
-}
-
 /* Release all Mapped and Reserved DMM resources */
 DSP_STATUS DRV_RemoveAllDMMResElements(HANDLE hPCtxt)
 {
 	struct PROCESS_CONTEXT *pCtxt = (struct PROCESS_CONTEXT *)hPCtxt;
 	DSP_STATUS status = DSP_SOK;
-	struct DMM_RSV_OBJECT *temp, *rsv_obj;
-
-	DRV_ProcFreeDMMRes(pCtxt);
-	pCtxt->dmm_map_list = NULL;
+	struct DMM_MAP_OBJECT *temp_map, *map_obj;
+	struct DMM_RSV_OBJECT *temp_rsv, *rsv_obj;
+
+	/* Free DMM mapped memory resources */
+	list_for_each_entry_safe(map_obj, temp_map, &pCtxt->dmm_map_list,
+			link) {
+		status = PROC_UnMap(pCtxt->hProcessor,
+				(void *)map_obj->dsp_addr, pCtxt);
+		if (DSP_FAILED(status))
+			pr_err("%s: PROC_UnMap failed!"
+					" status = 0x%xn", __func__, status);
+	}
 
 	/* Free DMM reserved memory resources */
-	list_for_each_entry_safe(rsv_obj, temp, &pCtxt->dmm_rsv_list, link) {
+	list_for_each_entry_safe(rsv_obj, temp_rsv, &pCtxt->dmm_rsv_list,
+			link) {
 		status = PROC_UnReserveMemory(pCtxt->hProcessor,
 				(void *)rsv_obj->dsp_reserved_addr, pCtxt);
 		if (DSP_FAILED(status))
@@ -325,37 +223,6 @@ DSP_STATUS DRV_RemoveAllDMMResElements(HANDLE hPCtxt)
 	return status;
 }
 
-DSP_STATUS DRV_GetDMMResElement(u32 pMapAddr, HANDLE hDMMRes, HANDLE hPCtxt)
-{
-	struct PROCESS_CONTEXT *pCtxt = (struct PROCESS_CONTEXT *)hPCtxt;
-	struct DMM_MAP_OBJECT **pDMMRes = (struct DMM_MAP_OBJECT **)hDMMRes;
-	DSP_STATUS status = DSP_SOK;
-	struct DMM_MAP_OBJECT *pTempDMM = NULL;
-
-	if (mutex_lock_interruptible(&pCtxt->dmm_map_mutex))
-		return DSP_EFAIL;
-
-	pTempDMM = pCtxt->dmm_map_list;
-	while (pTempDMM && (pTempDMM->ulDSPAddr != pMapAddr)) {
-		GT_3trace(curTrace, GT_ENTER,
-			 "DRV_GetDMMResElement: 2 pTempDMM:%x "
-			 "pTempDMM->ulDSPAddr:%x pMapAddr:%x\n", pTempDMM,
-			 pTempDMM->ulDSPAddr, pMapAddr);
-		pTempDMM = pTempDMM->next;
-	}
-
-	mutex_unlock(&pCtxt->dmm_map_mutex);
-
-	if (pTempDMM) {
-		GT_0trace(curTrace, GT_ENTER, "DRV_GetDMMResElement: 3");
-		*pDMMRes = pTempDMM;
-	} else {
-		status = DSP_ENOTFOUND;
-		GT_0trace(curTrace, GT_ENTER, "DRV_GetDMMResElement: 4");
-	}
-	return status;
-}
-
 /* Update Node allocation status */
 void DRV_ProcNodeUpdateStatus(HANDLE hNodeRes, s32 status)
 {
diff --git a/drivers/dsp/bridge/rmgr/drv_interface.c b/drivers/dsp/bridge/rmgr/drv_interface.c
index 80b8c7e..800ed61 100644
--- a/drivers/dsp/bridge/rmgr/drv_interface.c
+++ b/drivers/dsp/bridge/rmgr/drv_interface.c
@@ -499,7 +499,8 @@ static int bridge_open(struct inode *ip, struct file *filp)
 	pr_ctxt = MEM_Calloc(sizeof(struct PROCESS_CONTEXT), MEM_PAGED);
 	if (pr_ctxt) {
 		pr_ctxt->resState = PROC_RES_ALLOCATED;
-		mutex_init(&pr_ctxt->dmm_map_mutex);
+		spin_lock_init(&pr_ctxt->dmm_map_lock);
+		INIT_LIST_HEAD(&pr_ctxt->dmm_map_list);
 		spin_lock_init(&pr_ctxt->dmm_rsv_lock);
 		INIT_LIST_HEAD(&pr_ctxt->dmm_rsv_list);
 		mutex_init(&pr_ctxt->node_mutex);
diff --git a/drivers/dsp/bridge/rmgr/proc.c b/drivers/dsp/bridge/rmgr/proc.c
index b8e0334..5ddfff6 100644
--- a/drivers/dsp/bridge/rmgr/proc.c
+++ b/drivers/dsp/bridge/rmgr/proc.c
@@ -1286,8 +1286,7 @@ DSP_STATUS PROC_Map(DSP_HPROCESSOR hProcessor, void *pMpuAddr, u32 ulSize,
 	u32 sizeAlign;
 	DSP_STATUS status = DSP_SOK;
 	struct PROC_OBJECT *pProcObject = (struct PROC_OBJECT *)hProcessor;
-
-       HANDLE        dmmRes;
+	struct DMM_MAP_OBJECT *map_obj;
 
 	GT_6trace(PROC_DebugMask, GT_ENTER, "Entered PROC_Map, args:\n\t"
 		 "hProcessor %x, pMpuAddr %x, ulSize %x, pReqAddr %x, "
@@ -1334,11 +1333,22 @@ DSP_STATUS PROC_Map(DSP_HPROCESSOR hProcessor, void *pMpuAddr, u32 ulSize,
 	}
 	(void)SYNC_LeaveCS(hProcLock);
 
-	if (DSP_SUCCEEDED(status)) {
-		DRV_InsertDMMResElement(&dmmRes, pr_ctxt);
-		DRV_UpdateDMMResElement(dmmRes, (u32)pMpuAddr, ulSize,
-				(u32)pReqAddr, (u32)*ppMapAddr, hProcessor);
+	if (DSP_FAILED(status))
+		goto func_end;
+
+	/*
+	 * A successful map should be followed by insertion of map_obj
+	 * into dmm_map_list, so that mapped memory resource tracking
+	 * remains uptodate
+	 */
+	map_obj = kmalloc(sizeof(struct DMM_MAP_OBJECT), GFP_KERNEL);
+	if (map_obj) {
+		map_obj->dsp_addr = (u32)*ppMapAddr;
+		spin_lock(&pr_ctxt->dmm_map_lock);
+		list_add(&map_obj->link, &pr_ctxt->dmm_map_list);
+		spin_unlock(&pr_ctxt->dmm_map_lock);
 	}
+
 func_end:
 	GT_1trace(PROC_DebugMask, GT_ENTER, "Leaving PROC_Map [0x%x]", status);
 	return status;
@@ -1669,8 +1679,7 @@ DSP_STATUS PROC_UnMap(DSP_HPROCESSOR hProcessor, void *pMapAddr,
 	struct DMM_OBJECT *hDmmMgr;
 	u32 vaAlign;
 	u32 sizeAlign;
-
-	HANDLE	      dmmRes;
+	struct DMM_MAP_OBJECT *map_obj;
 
 	GT_2trace(PROC_DebugMask, GT_ENTER,
 		 "Entered PROC_UnMap, args:\n\thProcessor:"
@@ -1710,9 +1719,21 @@ DSP_STATUS PROC_UnMap(DSP_HPROCESSOR hProcessor, void *pMapAddr,
 	if (DSP_FAILED(status))
 		goto func_end;
 
-	if (DRV_GetDMMResElement((u32)pMapAddr, &dmmRes, pr_ctxt)
-							!= DSP_ENOTFOUND)
-		DRV_RemoveDMMResElement(dmmRes, pr_ctxt);
+	/*
+	 * A successful unmap should be followed by removal of map_obj
+	 * from dmm_map_list, so that mapped memory resource tracking
+	 * remains uptodate
+	 */
+	spin_lock(&pr_ctxt->dmm_map_lock);
+	list_for_each_entry(map_obj, &pr_ctxt->dmm_map_list, link) {
+		if (map_obj->dsp_addr == (u32)pMapAddr) {
+			list_del(&map_obj->link);
+			kfree(map_obj);
+			break;
+		}
+	}
+	spin_unlock(&pr_ctxt->dmm_map_lock);
+
 func_end:
 	GT_1trace(PROC_DebugMask, GT_ENTER,
 		 "Leaving PROC_UnMap [0x%x]", status);
-- 
1.6.3.3

Re: [PATCHv5 0/4] DSPBRIDGE: Improved reserved and mapped resource cleanup

[Felipe Contreras]

On Thu, Feb 18, 2010 at 03:40:49PM +0100, Ameya Palande wrote:
> This patch series splits DMM_RES_OBJECT into DMM_MAP_OBJECT and DMM_RSV_OBJECT
> which are used independently for mapped and reserved memory resources
> accounting. This will help in cleanup of reserved memory resources which was
> not handled properly before. With these patches resource cleanup mechanism
> will work perfectly in a use case where a big chunk of memory is reserved and
> then lot of mappings are created inside it.
> 
> Changes since v4:
> 1. Replace unnecessary list_for_each_entry_safe to list_for_each_entry
>    http://marc.info/?l=linux-omap&m=126645721715598&w=2
> 2. Comments at appropriate places to explain resource tracking objects
>    insertion and removals.
>    http://marc.info/?l=linux-omap&m=126649541624172&w=2
> 
> Changes since v3:
> 1. Improved mapped memory resource cleanup
> 
> Changes since v2:
> 1. Removed locking from DRV_RemoveAllDMMResElements()
> 2. Removed cleanup variable from PROC_UnReserveMemory()
>    http://marc.info/?l=linux-omap&m=126637211831587&w=2
> 3. Rebased patchset on top of following commit:
>    DSPBRIDGE: Remove conditional check from the InputMsg function
> 
> Changed since v1:
> 1. Reduced indentation
>    http://marc.info/?l=linux-omap&m=126624982331523&w=2
> 
> Ameya Palande (4):
>   DSPBRIDGE: Rename DMM_RES_OBJECT to DMM_MAP_OBJECT
>   DSPBRIDGE: New reserved memory accounting framework
>   DSPBRIDGE: Fix memory corruption in DRV_ProcFreeDMMRes
>   DSPBRIDGE: Improved mapped memory cleanup

I'm not entirely happy with this order, I generally prefer:
 1) critical fixes (fix memory corruption)
 2) reorganization
 3) new feature (fix res object leakage)

This way the critical fixes can be merged first, while the rest of the
patches are worked on. And reorganization usually don't have much
problem going in.

Currently in your patch #2 the spinlocks protection is not symmetrical
between map and res.

But in general they look ok:
Reviwed-by: Felipe Contreras <felipe.contreras@nokia.com>

-- 
Felipe Contreras

RE: [PATCHv5 0/4] DSPBRIDGE: Improved reserved and mapped resource cleanup

[Guzman Lugo, Fernando]

Hi,

>-----Original Message-----
>From: Ameya Palande [mailto:ameya.palande@nokia.com]
>Sent: Thursday, February 18, 2010 8:41 AM
>To: linux-omap@vger.kernel.org
>Cc: felipe.contreras@nokia.com; Menon, Nishanth; Chitriki Rudramuni,
>Deepak; Guzman Lugo, Fernando; Ramirez Luna, Omar
>Subject: [PATCHv5 0/4] DSPBRIDGE: Improved reserved and mapped resource
>cleanup
>
>This patch series splits DMM_RES_OBJECT into DMM_MAP_OBJECT and
>DMM_RSV_OBJECT
>which are used independently for mapped and reserved memory resources
>accounting. This will help in cleanup of reserved memory resources which
>was
>not handled properly before. With these patches resource cleanup mechanism
>will work perfectly in a use case where a big chunk of memory is reserved
>and
>then lot of mappings are created inside it.
>
>Changes since v4:
>1. Replace unnecessary list_for_each_entry_safe to list_for_each_entry
>   http://marc.info/?l=linux-omap&m=126645721715598&w=2
>2. Comments at appropriate places to explain resource tracking objects
>   insertion and removals.
>   http://marc.info/?l=linux-omap&m=126649541624172&w=2
>
>Changes since v3:
>1. Improved mapped memory resource cleanup
>
>Changes since v2:
>1. Removed locking from DRV_RemoveAllDMMResElements()
>2. Removed cleanup variable from PROC_UnReserveMemory()
>   http://marc.info/?l=linux-omap&m=126637211831587&w=2
>3. Rebased patchset on top of following commit:
>   DSPBRIDGE: Remove conditional check from the InputMsg function
>
>Changed since v1:
>1. Reduced indentation
>   http://marc.info/?l=linux-omap&m=126624982331523&w=2
>
>Ameya Palande (4):
>  DSPBRIDGE: Rename DMM_RES_OBJECT to DMM_MAP_OBJECT
>  DSPBRIDGE: New reserved memory accounting framework
>  DSPBRIDGE: Fix memory corruption in DRV_ProcFreeDMMRes
>  DSPBRIDGE: Improved mapped memory cleanup
>
> arch/arm/plat-omap/include/dspbridge/drv.h         |   44 ++---
> arch/arm/plat-omap/include/dspbridge/proc.h        |    4 +-
> .../plat-omap/include/dspbridge/resourcecleanup.h  |   11 --
> drivers/dsp/bridge/pmgr/wcd.c                      |    7 +-
> drivers/dsp/bridge/rmgr/drv.c                      |  176 +++-------------
>----
> drivers/dsp/bridge/rmgr/drv_interface.c            |    5 +-
> drivers/dsp/bridge/rmgr/node.c                     |    5 +-
> drivers/dsp/bridge/rmgr/proc.c                     |  105 +++++++++---
> 8 files changed, 134 insertions(+), 223 deletions(-)


Just for the comment in "DSPBRIDGE: Improved mapped memory cleanup", now there is a comment that explain what the code does, however I still think that the resource cleanup related code should be in only one file (for example drv.h) to have a more organized code. Others things look good!

Acked-by: Fernando Guzman Lugo <x0095840@ti.com>

Re: [PATCH 1/4] DSPBRIDGE: Rename DMM_RES_OBJECT to DMM_MAP_OBJECT

[Omar Ramirez Luna]

On 2/18/2010 8:40 AM, Ameya Palande wrote:
> In its current state DMM_RES_OBJECT is not correctly tracking reserve and
> unreserve but only map and unmap. So lets rename it to DMM_MAP_OBJECT to
> clarify what it is really doing!
>
> In addition to this, this patch also does some trivial code cleanup.
>
> Signed-off-by: Ameya Palande<ameya.palande@nokia.com>
> Reviewed-by: Felipe Contreras<felipe.contreras@nokia.com>
> ---
>   arch/arm/plat-omap/include/dspbridge/drv.h |   14 +++---
>   drivers/dsp/bridge/rmgr/drv.c              |   69 ++++++++++++++--------------
>   drivers/dsp/bridge/rmgr/drv_interface.c    |    2 +-
>   3 files changed, 43 insertions(+), 42 deletions(-)
>

Pushed to dspbridge

- omar

Re: [PATCH 2/4] DSPBRIDGE: New reserved memory accounting framework

[Omar Ramirez Luna]

On 2/18/2010 8:40 AM, Ameya Palande wrote:
> DSP_RSV_OBJECT is introduced to track reserved memory accounting information.
> This will allow us to do proper cleanup for memory reserved using
> PROC_ReserveMemory().
>
> Signed-off-by: Ameya Palande<ameya.palande@nokia.com>
> ---
>   arch/arm/plat-omap/include/dspbridge/drv.h  |   10 ++++
>   arch/arm/plat-omap/include/dspbridge/proc.h |    4 +-
>   drivers/dsp/bridge/pmgr/wcd.c               |    7 ++-
>   drivers/dsp/bridge/rmgr/drv.c               |   18 +++++---
>   drivers/dsp/bridge/rmgr/drv_interface.c     |    2 +
>   drivers/dsp/bridge/rmgr/node.c              |    5 +-
>   drivers/dsp/bridge/rmgr/proc.c              |   62 ++++++++++++++++++++++-----
>   7 files changed, 83 insertions(+), 25 deletions(-)
>

Pushed to dspbridge

- omar

Re: [PATCH 3/4] DSPBRIDGE: Fix memory corruption in DRV_ProcFreeDMMRes

[Omar Ramirez Luna]

On 2/18/2010 8:40 AM, Ameya Palande wrote:
> This patch fixes following issues:
>
> 1. pDMMRes was dereferenced and modified when it was already freed by
>     PROC_Ummap(). This results in memory corruption.
>
> 2. Instead of passing ulDSPAddr, ulDSPResAddr was passed to PROC_UnMap()
>     which will not retrieve correct DMMRes element.
>
> Signed-off-by: Ameya Palande<ameya.palande@nokia.com>
> Signed-off-by: Felipe Contreras<felipe.contreras@nokia.com>
> ---
>   drivers/dsp/bridge/rmgr/drv.c |   12 ++----------
>   1 files changed, 2 insertions(+), 10 deletions(-)
>

Pushed to dspbridge

- omar

Re: [PATCH 4/4] DSPBRIDGE: Improved mapped memory cleanup

[Omar Ramirez Luna]

On 2/18/2010 8:40 AM, Ameya Palande wrote:
> This patch improves current mapped memory cleanup mechanism by using linux
> native list implementation. As a side effect we also get following benefits:
>
> 1. Unnecessary data members in DMM_MAP_OBJECT are removed which results in
>     memory saving.
>
> 2. Following functions are removed as they are not needed anymore:
>     DRV_ProcFreeDMMRes()
>     DRV_UpdateDMMResElement()
>     DRV_InsertDMMResElement()
>     DRV_GetDMMResElement()
>     DRV_RemoveDMMResElement()
>
> Signed-off-by: Ameya Palande<ameya.palande@nokia.com>
> ---
>   arch/arm/plat-omap/include/dspbridge/drv.h         |   30 +---
>   .../plat-omap/include/dspbridge/resourcecleanup.h  |   11 --
>   drivers/dsp/bridge/rmgr/drv.c                      |  161 ++------------------
>   drivers/dsp/bridge/rmgr/drv_interface.c            |    3 +-
>   drivers/dsp/bridge/rmgr/proc.c                     |   43 ++++--
>   5 files changed, 54 insertions(+), 194 deletions(-)
>

Pushed to dspbridge

- omar