On Tue, Feb 16, 2010 at 3:20 PM, Ameya Palande <ameya.palande@xxxxxxxxx> 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@xxxxxxxxx>
> ---
> arch/arm/plat-omap/include/dspbridge/drv.h | 13 ++++++
> arch/arm/plat-omap/include/dspbridge/proc.h | 5 +-
> drivers/dsp/bridge/pmgr/dmm.c | 3 +-
> drivers/dsp/bridge/pmgr/wcd.c | 7 ++-
> drivers/dsp/bridge/rmgr/drv.c | 27 +++++++++---
> drivers/dsp/bridge/rmgr/drv_interface.c | 7 ++-
> drivers/dsp/bridge/rmgr/node.c | 5 +-
> drivers/dsp/bridge/rmgr/proc.c | 58 +++++++++++++++++++++-----
> 8 files changed, 97 insertions(+), 28 deletions(-)
>
> diff --git a/arch/arm/plat-omap/include/dspbridge/drv.h b/arch/arm/plat-omap/include/dspbridge/drv.h
> index 3069e74..6e8187d 100644
> --- a/arch/arm/plat-omap/include/dspbridge/drv.h
> +++ b/arch/arm/plat-omap/include/dspbridge/drv.h
> @@ -101,6 +101,15 @@ struct DMM_MAP_OBJECT {
> struct DMM_MAP_OBJECT *next;
> } ;
>
> +/*
> + * New structure (member of process context) used for accounting of DMM
> + * reserved memory information
> + */
> +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 */
> @@ -141,6 +150,10 @@ struct PROCESS_CONTEXT{
> /* DMM mapped memory resources */
> struct DMM_MAP_OBJECT *dmm_map_list;
>
> + /* DMM reserved memory resources */
> + struct list_head dmm_rsv_list;
> + spinlock_t dmm_rsv_list_lock;
> +
Why rsv requires a spinlock, but not map?
I guess it would be needed if somehow PROC_UnReserveMemory() was
called by user-space at the same time than bridge_close, but is that
really possible? If it is, then the same danger is present with
PROC_UnMap() unless I'm missing something.
> /* 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..558c053 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,7 @@
> * Details:
> */
> extern DSP_STATUS PROC_UnReserveMemory(DSP_HPROCESSOR hProcessor,
> - void *pRsvAddr);
> + void *pRsvAddr, struct PROCESS_CONTEXT *pr_ctxt,
> + int cleanup);
>
> #endif /* PROC_ */
> diff --git a/drivers/dsp/bridge/pmgr/dmm.c b/drivers/dsp/bridge/pmgr/dmm.c
> index d5a7275..72aee60 100644
> --- a/drivers/dsp/bridge/pmgr/dmm.c
> +++ b/drivers/dsp/bridge/pmgr/dmm.c
> @@ -386,9 +386,10 @@ DSP_STATUS DMM_ReserveMemory(struct DMM_OBJECT *hDmmMgr, u32 size,
> node->MappedSize = 0;
> /* Return the chunk's starting address */
> *pRsvAddr = rsvAddr;
> - } else
> + } else {
> /*dSP chunk of given size is not available */
> status = DSP_EMEMORY;
> + }
>
> SYNC_LeaveCS(pDmmObj->hDmmLock);
> GT_3trace(DMM_debugMask, GT_4CLASS,
> diff --git a/drivers/dsp/bridge/pmgr/wcd.c b/drivers/dsp/bridge/pmgr/wcd.c
> index beea23b..747d069 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, 1);
> }
> }
> 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, 1);
> return status;
> }
>
> diff --git a/drivers/dsp/bridge/rmgr/drv.c b/drivers/dsp/bridge/rmgr/drv.c
> index 3dc38d0..9d5c077 100644
> --- a/drivers/dsp/bridge/rmgr/drv.c
> +++ b/drivers/dsp/bridge/rmgr/drv.c
> @@ -277,25 +277,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;
> @@ -305,6 +300,24 @@ DSP_STATUS DRV_RemoveAllDMMResElements(HANDLE hPCtxt)
> kfree(pTempDMMRes2);
> }
> pCtxt->dmm_map_list = NULL;
> +
> + /* Free DMM reserved memory resources */
> + spin_lock(&pCtxt->dmm_rsv_list_lock);
> + list_for_each_entry_safe(rsv_obj, temp, &pCtxt->dmm_rsv_list, link) {
> + /*
> + * Since we have already acquired spin lock for dmm_rsv_list,
> + * we don't want to perform cleanup in PROC_UnReserveMemory.
> + */
Sure, but why did we acquire a spin lock in the first place?
> + status = PROC_UnReserveMemory(pCtxt->hProcessor,
> + (void *)rsv_obj->dsp_reserved_addr, pCtxt, 0);
If this behaved similarly to the unmap code, then 'cleanup' could be
one and there wouldn't be any need to manually remove the entries.
> + if (DSP_FAILED(status))
> + pr_err("%s: PROC_UnReserveMemory failed!"
> + " status = 0x%xn", __func__, status);
> + list_del(&rsv_obj->link);
> + kfree(rsv_obj);
> + }
> + spin_unlock(&pCtxt->dmm_rsv_list_lock);
> +
> return status;
> }
Cheers.
--
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
