* [PATCHv4 1/4] DSPBRIDGE: Rename DMM_RES_OBJECT to DMM_MAP_OBJECT
2010-02-17 18:05 [PATCHv4 0/4] DSPBRIDGE: Improved reserved and mapped resource cleanup Ameya Palande
@ 2010-02-17 18:05 ` Ameya Palande
2010-02-17 18:05 ` [PATCHv4 2/4] DSPBRIDGE: New reserved memory accounting framework Ameya Palande
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: Ameya Palande @ 2010-02-17 18:05 UTC (permalink / raw)
To: linux-omap; +Cc: felipe.contreras, nm, deepak.chitriki, x0095840, omar.ramirez
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
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCHv4 2/4] DSPBRIDGE: New reserved memory accounting framework
2010-02-17 18:05 [PATCHv4 0/4] DSPBRIDGE: Improved reserved and mapped resource cleanup Ameya Palande
2010-02-17 18:05 ` [PATCHv4 1/4] DSPBRIDGE: Rename DMM_RES_OBJECT to DMM_MAP_OBJECT Ameya Palande
@ 2010-02-17 18:05 ` Ameya Palande
2010-02-18 1:40 ` Guzman Lugo, Fernando
2010-02-17 18:05 ` [PATCHv4 3/4] DSPBRIDGE: Fix memory corruption in DRV_ProcFreeDMMRes Ameya Palande
2010-02-17 18:05 ` [PATCHv4 4/4] DSPBRIDGE: Improved mapped memory cleanup Ameya Palande
3 siblings, 1 reply; 11+ messages in thread
From: Ameya Palande @ 2010-02-17 18:05 UTC (permalink / raw)
To: linux-omap; +Cc: felipe.contreras, nm, deepak.chitriki, x0095840, omar.ramirez
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 | 52 +++++++++++++++++++++------
7 files changed, 73 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..6ce76cb 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,29 @@ 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;
+
+ 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 +1719,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 *temp, *rsv_obj;
GT_2trace(PROC_DebugMask, GT_ENTER,
"Entered PROC_UnReserveMemory, args:\n\t"
@@ -1720,18 +1736,32 @@ 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;
+
+ spin_lock(&pr_ctxt->dmm_rsv_lock);
+ list_for_each_entry_safe(rsv_obj, temp, &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
^ permalink raw reply related [flat|nested] 11+ messages in thread* RE: [PATCHv4 2/4] DSPBRIDGE: New reserved memory accounting framework
2010-02-17 18:05 ` [PATCHv4 2/4] DSPBRIDGE: New reserved memory accounting framework Ameya Palande
@ 2010-02-18 1:40 ` Guzman Lugo, Fernando
2010-02-18 14:48 ` Ameya Palande
0 siblings, 1 reply; 11+ messages in thread
From: Guzman Lugo, Fernando @ 2010-02-18 1:40 UTC (permalink / raw)
To: Ameya Palande, linux-omap@vger.kernel.org
Cc: felipe.contreras@nokia.com, Menon, Nishanth,
Chitriki Rudramuni, Deepak, Ramirez Luna, Omar
Hi,
>-----Original Message-----
>From: Ameya Palande [mailto:ameya.palande@nokia.com]
>Sent: Wednesday, February 17, 2010 12:06 PM
>To: linux-omap@vger.kernel.org
>Cc: felipe.contreras@nokia.com; Menon, Nishanth; Chitriki Rudramuni,
>Deepak; Guzman Lugo, Fernando; Ramirez Luna, Omar
>Subject: [PATCHv4 2/4] DSPBRIDGE: New reserved memory accounting framework
>
>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 | 52 +++++++++++++++++++++--
>----
> 7 files changed, 73 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..6ce76cb 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,29 @@ 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;
>+
>+ 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 +1719,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 *temp, *rsv_obj;
>
> GT_2trace(PROC_DebugMask, GT_ENTER,
> "Entered PROC_UnReserveMemory, args:\n\t"
>@@ -1720,18 +1736,32 @@ 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;
>+
>+ spin_lock(&pr_ctxt->dmm_rsv_lock);
>+ list_for_each_entry_safe(rsv_obj, temp, &pr_ctxt->dmm_rsv_list, link)
list_for_each_entry_safe is used when you delete a element from the list and continue but this is not the case. You should use list_for_each_entry instead, which has simpler logic and we can get ride of temp variable.
Regards,
Fernando.
>{
>+ 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
^ permalink raw reply [flat|nested] 11+ messages in thread* RE: [PATCHv4 2/4] DSPBRIDGE: New reserved memory accounting framework
2010-02-18 1:40 ` Guzman Lugo, Fernando
@ 2010-02-18 14:48 ` Ameya Palande
0 siblings, 0 replies; 11+ messages in thread
From: Ameya Palande @ 2010-02-18 14:48 UTC (permalink / raw)
To: ext Guzman Lugo, Fernando
Cc: linux-omap@vger.kernel.org, Contreras Felipe (Nokia-D/Helsinki),
Menon, Nishanth, Chitriki Rudramuni, Deepak, Ramirez Luna, Omar
Hi Fernando,
On Thu, 2010-02-18 at 02:40 +0100, ext Guzman Lugo, Fernando wrote:
> Hi,
>
> >-----Original Message-----
> >From: Ameya Palande [mailto:ameya.palande@nokia.com]
> >Sent: Wednesday, February 17, 2010 12:06 PM
> >To: linux-omap@vger.kernel.org
> >Cc: felipe.contreras@nokia.com; Menon, Nishanth; Chitriki Rudramuni,
> >Deepak; Guzman Lugo, Fernando; Ramirez Luna, Omar
> >Subject: [PATCHv4 2/4] DSPBRIDGE: New reserved memory accounting framework
> >
> >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>
> >---
> >@@ -1720,18 +1736,32 @@ 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;
> >+
> >+ spin_lock(&pr_ctxt->dmm_rsv_lock);
> >+ list_for_each_entry_safe(rsv_obj, temp, &pr_ctxt->dmm_rsv_list, link)
>
> list_for_each_entry_safe is used when you delete a element from the list and continue but this is not the case. You should use list_for_each_entry instead, which has simpler logic and we can get ride of temp variable.
Thanks! V5 should take care of this :)
Cheers,
Ameya.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCHv4 3/4] DSPBRIDGE: Fix memory corruption in DRV_ProcFreeDMMRes
2010-02-17 18:05 [PATCHv4 0/4] DSPBRIDGE: Improved reserved and mapped resource cleanup Ameya Palande
2010-02-17 18:05 ` [PATCHv4 1/4] DSPBRIDGE: Rename DMM_RES_OBJECT to DMM_MAP_OBJECT Ameya Palande
2010-02-17 18:05 ` [PATCHv4 2/4] DSPBRIDGE: New reserved memory accounting framework Ameya Palande
@ 2010-02-17 18:05 ` Ameya Palande
2010-02-17 18:05 ` [PATCHv4 4/4] DSPBRIDGE: Improved mapped memory cleanup Ameya Palande
3 siblings, 0 replies; 11+ messages in thread
From: Ameya Palande @ 2010-02-17 18:05 UTC (permalink / raw)
To: linux-omap; +Cc: felipe.contreras, nm, deepak.chitriki, x0095840, omar.ramirez
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
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCHv4 4/4] DSPBRIDGE: Improved mapped memory cleanup
2010-02-17 18:05 [PATCHv4 0/4] DSPBRIDGE: Improved reserved and mapped resource cleanup Ameya Palande
` (2 preceding siblings ...)
2010-02-17 18:05 ` [PATCHv4 3/4] DSPBRIDGE: Fix memory corruption in DRV_ProcFreeDMMRes Ameya Palande
@ 2010-02-17 18:05 ` Ameya Palande
2010-02-18 1:52 ` Guzman Lugo, Fernando
3 siblings, 1 reply; 11+ messages in thread
From: Ameya Palande @ 2010-02-17 18:05 UTC (permalink / raw)
To: linux-omap; +Cc: felipe.contreras, nm, deepak.chitriki, x0095840, omar.ramirez
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 | 33 +++--
5 files changed, 44 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 6ce76cb..7fa3a16 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,17 @@ 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;
+
+ 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;
@@ -1664,8 +1669,7 @@ DSP_STATUS PROC_UnMap(DSP_HPROCESSOR hProcessor, void *pMapAddr,
struct DMM_OBJECT *hDmmMgr;
u32 vaAlign;
u32 sizeAlign;
-
- HANDLE dmmRes;
+ struct DMM_MAP_OBJECT *temp, *map_obj;
GT_2trace(PROC_DebugMask, GT_ENTER,
"Entered PROC_UnMap, args:\n\thProcessor:"
@@ -1705,9 +1709,16 @@ 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);
+ spin_lock(&pr_ctxt->dmm_map_lock);
+ list_for_each_entry_safe(map_obj, temp, &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
^ permalink raw reply related [flat|nested] 11+ messages in thread* RE: [PATCHv4 4/4] DSPBRIDGE: Improved mapped memory cleanup
2010-02-17 18:05 ` [PATCHv4 4/4] DSPBRIDGE: Improved mapped memory cleanup Ameya Palande
@ 2010-02-18 1:52 ` Guzman Lugo, Fernando
2010-02-18 12:15 ` Ameya Palande
0 siblings, 1 reply; 11+ messages in thread
From: Guzman Lugo, Fernando @ 2010-02-18 1:52 UTC (permalink / raw)
To: Ameya Palande, linux-omap@vger.kernel.org
Cc: felipe.contreras@nokia.com, Menon, Nishanth,
Chitriki Rudramuni, Deepak, Ramirez Luna, Omar
Hi,
>-----Original Message-----
>From: Ameya Palande [mailto:ameya.palande@nokia.com]
>Sent: Wednesday, February 17, 2010 12:06 PM
>To: linux-omap@vger.kernel.org
>Cc: felipe.contreras@nokia.com; Menon, Nishanth; Chitriki Rudramuni,
>Deepak; Guzman Lugo, Fernando; Ramirez Luna, Omar
>Subject: [PATCHv4 4/4] DSPBRIDGE: Improved mapped memory cleanup
>
>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 | 33 +++--
> 5 files changed, 44 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 6ce76cb..7fa3a16 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,17 @@ 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;
>+
>+ 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);
> }
What do you think about it?
Instead of removing DRV_InsertDMMResElement make it an inline function with your code inside:
inline void DRV_InsertDMMResElement(u32 ppMapAddr)
{
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);
}
}
It could make the code more understandable about what it is actually doing.
It also applies to the functions which removes this patch.
Regards,
Fenando
>+
> func_end:
> GT_1trace(PROC_DebugMask, GT_ENTER, "Leaving PROC_Map [0x%x]",
>status);
> return status;
>@@ -1664,8 +1669,7 @@ DSP_STATUS PROC_UnMap(DSP_HPROCESSOR hProcessor, void
>*pMapAddr,
> struct DMM_OBJECT *hDmmMgr;
> u32 vaAlign;
> u32 sizeAlign;
>-
>- HANDLE dmmRes;
>+ struct DMM_MAP_OBJECT *temp, *map_obj;
>
> GT_2trace(PROC_DebugMask, GT_ENTER,
> "Entered PROC_UnMap, args:\n\thProcessor:"
>@@ -1705,9 +1709,16 @@ 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);
>+ spin_lock(&pr_ctxt->dmm_map_lock);
>+ list_for_each_entry_safe(map_obj, temp, &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
^ permalink raw reply [flat|nested] 11+ messages in thread* RE: [PATCHv4 4/4] DSPBRIDGE: Improved mapped memory cleanup
2010-02-18 1:52 ` Guzman Lugo, Fernando
@ 2010-02-18 12:15 ` Ameya Palande
2010-02-18 14:27 ` Felipe Contreras
0 siblings, 1 reply; 11+ messages in thread
From: Ameya Palande @ 2010-02-18 12:15 UTC (permalink / raw)
To: ext Guzman Lugo, Fernando
Cc: linux-omap@vger.kernel.org, Contreras Felipe (Nokia-D/Helsinki),
Menon, Nishanth, Chitriki Rudramuni, Deepak, Ramirez Luna, Omar
Hi Fernando,
On Thu, 2010-02-18 at 02:52 +0100, ext Guzman Lugo, Fernando wrote:
> Hi,
>
> >-----Original Message-----
> >From: Ameya Palande [mailto:ameya.palande@nokia.com]
> >Sent: Wednesday, February 17, 2010 12:06 PM
> >To: linux-omap@vger.kernel.org
> >Cc: felipe.contreras@nokia.com; Menon, Nishanth; Chitriki Rudramuni,
> >Deepak; Guzman Lugo, Fernando; Ramirez Luna, Omar
> >Subject: [PATCHv4 4/4] DSPBRIDGE: Improved mapped memory cleanup
> >
> >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>
> >---
> >@@ -1334,11 +1333,17 @@ 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;
> >+
> >+ 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);
> > }
>
> What do you think about it?
> Instead of removing DRV_InsertDMMResElement make it an inline function with your code inside:
Making DRV_InsertDMMResElement inline doesn't make sense since we are
not calling it multiple times.
> inline void DRV_InsertDMMResElement(u32 ppMapAddr)
> {
> 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);
> }
> }
>
> It could make the code more understandable about what it is actually doing.
> It also applies to the functions which removes this patch.
I can put a comment here to clarify what this code is doing ;)
Cheers,
Ameya.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCHv4 4/4] DSPBRIDGE: Improved mapped memory cleanup
2010-02-18 12:15 ` Ameya Palande
@ 2010-02-18 14:27 ` Felipe Contreras
2010-02-18 14:47 ` Ameya Palande
0 siblings, 1 reply; 11+ messages in thread
From: Felipe Contreras @ 2010-02-18 14:27 UTC (permalink / raw)
To: Palande Ameya (Nokia-D/Helsinki)
Cc: ext Guzman Lugo, Fernando, linux-omap@vger.kernel.org,
Menon, Nishanth, Chitriki Rudramuni, Deepak, Ramirez Luna, Omar
On Thu, Feb 18, 2010 at 01:15:55PM +0100, Ameya Palande wrote:
> On Thu, 2010-02-18 at 02:52 +0100, ext Guzman Lugo, Fernando wrote:
> > What do you think about it?
> > Instead of removing DRV_InsertDMMResElement make it an inline function with your code inside:
>
> Making DRV_InsertDMMResElement inline doesn't make sense since we are
> not calling it multiple times.
I agree with Fernando; the code would be more readable. Even more if we
rename the function to instert_map_element().
note: should be 'static inline'
> > inline void DRV_InsertDMMResElement(u32 ppMapAddr)
> > {
> > 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);
> > }
> > }
> >
> > It could make the code more understandable about what it is actually doing.
> > It also applies to the functions which removes this patch.
>
> I can put a comment here to clarify what this code is doing ;)
AFAIK in linux, self-documenting code is preferred over comments.
Cheers.
--
Felipe Contreras
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCHv4 4/4] DSPBRIDGE: Improved mapped memory cleanup
2010-02-18 14:27 ` Felipe Contreras
@ 2010-02-18 14:47 ` Ameya Palande
0 siblings, 0 replies; 11+ messages in thread
From: Ameya Palande @ 2010-02-18 14:47 UTC (permalink / raw)
To: Contreras Felipe (Nokia-D/Helsinki)
Cc: ext Guzman Lugo, Fernando, linux-omap@vger.kernel.org,
Menon, Nishanth, Chitriki Rudramuni, Deepak, Ramirez Luna, Omar
Hi Felipe,
On Thu, 2010-02-18 at 15:27 +0100, Contreras Felipe (Nokia-D/Helsinki)
wrote:
> On Thu, Feb 18, 2010 at 01:15:55PM +0100, Ameya Palande wrote:
> > On Thu, 2010-02-18 at 02:52 +0100, ext Guzman Lugo, Fernando wrote:
> > > What do you think about it?
> > > Instead of removing DRV_InsertDMMResElement make it an inline function with your code inside:
> >
> > Making DRV_InsertDMMResElement inline doesn't make sense since we are
> > not calling it multiple times.
>
> I agree with Fernando; the code would be more readable. Even more if we
> rename the function to instert_map_element().
I guess the issue is about why this code is doing it rather than the
readability of code. I have sent V5 of the patchset with comment
explaining the purpose of doing this.
If you think that by creating a new static inline function like
insert_map_element(), we can understand it better then following comment
also does the same thing:
/* Following code inserts a map element ... and explain why */
Anyways insert_map_element() function doesn't explain why it is
required :(
Cheers,
Ameya.
^ permalink raw reply [flat|nested] 11+ messages in thread