From owner-freebsd-current@FreeBSD.ORG Tue Oct 25 17:41:27 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 56DA316A41F; Tue, 25 Oct 2005 17:41:27 +0000 (GMT) (envelope-from jhb@freebsd.org) Received: from mv.twc.weather.com (mv.twc.weather.com [65.212.71.225]) by mx1.FreeBSD.org (Postfix) with ESMTP id D7ACA43D7B; Tue, 25 Oct 2005 17:41:19 +0000 (GMT) (envelope-from jhb@freebsd.org) Received: from [10.50.41.234] (Not Verified[10.50.41.234]) by mv.twc.weather.com with NetIQ MailMarshal (v6, 0, 3, 8) id ; Tue, 25 Oct 2005 13:57:48 -0400 From: John Baldwin To: Julian Elischer Date: Tue, 25 Oct 2005 13:41:20 -0400 User-Agent: KMail/1.8.2 References: <20051021131329.A16FC126E@smtp.263.net> <200510211728.32476.jhb@freebsd.org> <435965EE.7070504@elischer.org> In-Reply-To: <435965EE.7070504@elischer.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline Message-Id: <200510251341.22621.jhb@freebsd.org> 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: Tue, 25 Oct 2005 17:41:27 -0000 On Friday 21 October 2005 06:04 pm, Julian Elischer wrote: > 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=EF=BF=BD=C3=AF=EF=BF=BD=C2=BC=C5=92hello > >>>> > >>>> 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 =3D 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 a= dd > >>> the operations there though for documentation purposes. Something li= ke > >>> 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 the= re > > now: > > > >Index: kern_proc.c > >=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > >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 =3D (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. I actually looked in the CVS history to find out if vm_dispose_proc() and=20 sched_destroyproc() should come back and I don't think they need to. =2D-=20 John Baldwin <>< http://www.FreeBSD.org/~jhb/ "Power Users Use the Power to Serve" =3D http://www.FreeBSD.org