Hi Ameya, > -----Original Message----- > From: Ameya Palande [mailto:ameya.palande@xxxxxxxxx] > Sent: Wednesday, August 05, 2009 4:52 PM > To: Ramos Falcon, Ernesto > Cc: linux-omap@xxxxxxxxxxxxxxx; Doyu Hiroshi (Nokia-D/Helsinki); Guzman > Lugo, Fernando; Ramirez Luna, Omar; Tereshonkov Roman (Nokia-D/Helsinki); > Moogi, Suyog > Subject: Re: [PATCH 2/3] DSPBRIDGE: Move resource cleanup to > bridge_release > > Hi Ernesto, > > ext Ramos Falcon, Ernesto wrote: > > Hi, > > > > We have detected a use case where if an application creates a child > process using fork call, and then the child and father processes call > DSPProcessor_Attach() and create a new process context with new tgid; when > the processes are terminated, only the last process calls bridge_release > cleaning only the resources in the father process, leaving the child > resources unreleased. > > > > One solution we have seen is to perform goes through the entire process > context list, clean up all the resources for all terminated processes or > in "zombie" state, as below, > > > > DRV_GetProcCtxtList(&pCtxtclosed, (struct DRV_OBJECT *)hDrvObject); > > while (pCtxtclosed != NULL) { > > printk("pCtxtclosed->pid = %d\n",pCtxtclosed->pid); > > tsk = find_task_by_pid(pCtxtclosed->pid); > > > > if ((tsk == NULL) || (tsk->exit_state == EXIT_ZOMBIE)) { > > > > GT_1trace(driverTrace, GT_5CLASS, > > "***Task structure not existing for " > > "process***%d\n", pCtxtclosed->pid); > > DRV_RemoveAllResources(pCtxtclosed); > > if (pCtxtclosed->hProcessor != NULL) { > > PROC_Detach > > (pCtxtclosed->hProcessor); > > } > > pTmp = pCtxtclosed->next; > > DRV_RemoveProcContext((struct DRV_OBJECT *)hDrvObject, > > pCtxtclosed, > > (void *)pCtxtclosed->pid); > > } else { > > pTmp = pCtxtclosed->next; > > } > > pCtxtclosed = pTmp; > > } > > > > Please let me know your comments. > > > > /Ernesto > > Good point :) > > I would like to simplify this use case ;) > > If we call DSPProcessor_Attach() twice in the same process and kill the > process, > then it will leak memory for 1st instance of PROCESSOR object. > > When we call open() on /dev/DspBridge a new PROCESS_CONTEXT is allocated, > and it > should be allocated **only once** in bridge_open() unlike in > NODE_Allocate() and > PROC_Attach(). PROCESS_CONTEXT tracks all the resources allocated on > behalf of > an open file handle(and not the process / thread). When this handle is > closed > all these resources should be freed in bridge_release(). Accountability of > resources should be done using PROCESS_CONTEXT and **not pid (which will > be > different for different thread) / tgid (which will be different for parent > and > child). > > Above problem occurs because PROCESS_CONTEXT by design tracks only one > PROCESSOR > object which gets freed in bridge_release(). > > Let me know your comments on this, and then we can proceed to fix this > issue. > > Cheers, > Ameya. You're right; I think using the PROCESS_CONTEXT to track the resources would resolve the issue. Also, with his approach we don't need to create a new context in the PROC_Attach /NODE_Allocate. We can solve the issue by implementing a counter to track the number of calls to the PROC_Attach/Detach, so in that way we create a process handle only for the first time, and for the subsequent calls we need to return the existing handle. In the other hand PROC_Detach would be executed for the last call to this function. I don't know yet how we would access or if there is an easy way to get the private_data as if get the pid using the "current", though. /Ernesto -- 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
