From owner-freebsd-current@FreeBSD.ORG Fri Oct 21 22:04:32 2005 Return-Path: X-Original-To: freebsd-current@freebsd.org Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 7594A16A42F; Fri, 21 Oct 2005 22:04:32 +0000 (GMT) (envelope-from julian@elischer.org) Received: from a50.ironport.com (a50.ironport.com [63.251.108.112]) by mx1.FreeBSD.org (Postfix) with ESMTP id 26FAC43D45; Fri, 21 Oct 2005 22:04:32 +0000 (GMT) (envelope-from julian@elischer.org) Received: from unknown (HELO [10.251.23.117]) ([10.251.23.117]) by a50.ironport.com with ESMTP; 21 Oct 2005 15:04:33 -0700 X-IronPort-Anti-Spam-Filtered: true Message-ID: <435965EE.7070504@elischer.org> Date: Fri, 21 Oct 2005 15:04:30 -0700 From: Julian Elischer User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7.11) Gecko/20050727 X-Accept-Language: en-us, en MIME-Version: 1.0 To: John Baldwin References: <20051021131329.A16FC126E@smtp.263.net> <200510211239.35190.jhb@freebsd.org> <20051021203207.GA26616@VARK.MIT.EDU> <200510211728.32476.jhb@freebsd.org> In-Reply-To: <200510211728.32476.jhb@freebsd.org> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit Cc: nocool , freebsd-hackers@freebsd.org, David Schultz , freebsd-current , delphij Subject: Re: where to release proc.p_stats X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 21 Oct 2005 22:04:32 -0000 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@ 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 >@@ -196,8 +196,17 @@ > 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.