Re: where to release proc.p_stats

John Baldwin wrote:

>On Friday 21 October 2005 04:32 pm, David Schultz wrote:
>  
>
>>On Fri, Oct 21, 2005, John Baldwin wrote:
>>    
>>
>>>On Friday 21 October 2005 09:13 am, nocool wrote:
>>>      
>>>
>>>>freebsd-hackers�ï�¼Œhello
>>>>
>>>>	Question about 5.4 kernel source code.
>>>>	I have some question about strust proc's initialize. Kernel use
>>>>proc_zone to allocate proc items and initialize them with proc_init
>>>>(sys\kern\kern_proc.c) function. In this function, we can find the
>>>>field proc.p_stats is allocated with pstats_alloc(), as
>>>>
>>>>p->p_stats = pstats_alloc();
>>>>
>>>>and pstats_alloc is realized as
>>>>
>>>>malloc(sizeof(struct pstats), M_SUBPROC, M_ZERO|M_WAITOK);
>>>>
>>>>But I can't find where this field is freed. If it will not be release,
>>>>will there be memory leakage?
>>>>        
>>>>
>>>Heh, das_at_ forgot to call pstats_free() when he did the changes.  The
>>>reason is probably because proc_fini() doesn't do anything useful because
>>>we never recycle proc structs.  We should probably at least add the
>>>operations there though for documentation purposes.  Something like this
>>>would work I think:
>>>      
>>>
>>I didn't put in the call because we never free proc structures, but
>>documenting what should happen if we ever do free them is a good
>>idea.  There's a fair amount of other cleanup that needs to happen
>>as well, which you can probably find in the CVS history.  (IIRC,
>>I'm guilty of removing the code at a time when more things depended
>>upon struct proc being type safe.  Are there any remaining reasons
>>why we can't free struct procs at this point?)
>>
>>By the way, there's no reason why we can't fold struct pstats into
>>struct proc so we don't have to allocate and free it at all.
>>It's never shared, so the extra level of indirection just adds overhead.
>>The main reason I didn't make this change earlier was to maintain binary
>>compatibility when I backported my U-area changes to -STABLE.
>>    
>>
>
>Looks like some of the functions (vm_dispose_proc() and sched_destroyproc()) 
>have vanished, so this is all that would be in there now:
>
>Index: kern_proc.c
>===================================================================
>RCS file: /usr/cvs/src/sys/kern/kern_proc.c,v
>retrieving revision 1.232
>diff -u -r1.232 kern_proc.c
>--- kern_proc.c 2 Oct 2005 23:27:56 -0000       1.232
>+++ kern_proc.c 21 Oct 2005 21:21:45 -0000
>_at__at_ -196,8 +196,17 _at__at_
> static void
> proc_fini(void *mem, int size)
> {
>+#ifdef notnow
>+       struct proc *p;
>
>+       p = (struct proc *)mem;
>+       pstats_free(p->p_stats);
>+       ksegrp_free(FIRST_KSEGRP_IN_PROC(p));
>+       thread_free(FIRST_THREAD_IN_PROC(p));
>+       mtx_destroy(&p->p_mtx);
>+#else
>        panic("proc reclaimed");
>+#endif
> }
>
> /*
>
>  
>

sched_destroyproc was removed by someone I believe because "it was not 
used".

if you were removing a proc you possibly should re introduce it.