Date: Mon, 20 Sep 2004 14:10:58 -0400 From: John Baldwin <jhb@FreeBSD.org> To: David Schultz <das@FreeBSD.org> Cc: cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/kern kern_proc.c kern_switch.c src/sys/sys sched.h src/sys/vm vm_glue.c Message-ID: <200409201410.58648.jhb@FreeBSD.org> In-Reply-To: <20040920001317.GA2829@VARK.MIT.EDU> References: <200409191834.i8JIYHXU089517@repoman.freebsd.org> <414E0DCA.5090601@elischer.org> <20040920001317.GA2829@VARK.MIT.EDU>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sunday 19 September 2004 08:13 pm, David Schultz wrote:
> On Sun, Sep 19, 2004, Julian Elischer wrote:
> > David Schultz wrote:
> > >das 2004-09-19 18:34:17 UTC
> > >
> > > FreeBSD src repository
> > >
> > > Modified files:
> > > sys/kern kern_proc.c kern_switch.c
> > > sys/sys sched.h
> > > sys/vm vm_glue.c
> > > Log:
> > > The zone from which proc structures are allocated is marked
> > > UMA_ZONE_NOFREE to guarantee type stability, so proc_fini() should
> > > never be called. Move an assertion from proc_fini() to proc_dtor()
> > > and garbage-collect the rest of the unreachable code. I have retained
> > > vm_proc_dispose(), since I consider its disuse a bug.
> >
> > well we do aim to one day remove the requirement for UMA_ZONE_NOFREE.
> > In fact I have a gague feeling we mayhave already done so. I think it
> > had to do with what page tables the kernel ran on after a thread went
> > away. Peter may have a better memory as to why that was required.
>
> We are clearly not there yet for things like p_vmspace. Look at
> sys_process.c or procfs, for instance. But I don't mind backing
> this out if someone is imminently preparing to remove the
> requirement.
>
> By the way, it would be great if someone who understands it better
> could edit the comments in sys/proc.h to reflect reality,
> particularly with respect to the locking model. Some of it seems
> to have rotted. For example, (l) and (m) are unused and p_stats
> looks like it should be (c + j).
Here's the current diff in my jhb_proc branch (which has stats and rusage done
(woo, big win there I know :-p)):
--- //depot/projects/smpng/sys/sys/proc.h 2004/09/13 18:26:18
+++ //depot/user/jhb/proc/sys/proc.h 2004/09/13 18:41:29
@@ -271,13 +271,12 @@
int td_pinned; /* (k) Temporary cpu pin count. */
struct kse_thr_mailbox *td_mailbox; /* (*) Userland mailbox address. */
struct ucred *td_ucred; /* (k) Reference to credentials. */
- struct thread *td_standin; /* (*) Use this for an upcall. */
- u_int td_prticks; /* (*) Profclock hits in sys for user */
- struct kse_upcall *td_upcall; /* (*) Upcall structure. */
- u_int64_t td_sticks; /* (j) Statclock hits in system mode. */
- u_int td_uuticks; /* (*) Statclock in user, for UTS. */
- u_int td_usticks; /* (*) Statclock in kernel, for UTS. */
- int td_intrval; /* (*) Return value of TDF_INTERRUPT. */
+ struct thread *td_standin; /* (k + a) Use this for an upcall. */
+ struct kse_upcall *td_upcall; /* (k + j) Upcall structure. */
+ u_int64_t td_sticks; /* (k) Statclock hits in system mode. */
+ u_int td_uuticks; /* (k) Statclock in user, for UTS. */
+ u_int td_usticks; /* (k) Statclock in kernel, for UTS. */
+ int td_intrval; /* (j) Return value of TDF_INTERRUPT. */
sigset_t td_oldsigmask; /* (k) Saved mask from pre sigpause. */
sigset_t td_sigmask; /* (c) Current signal mask. */
sigset_t td_siglist; /* (c) Sigs arrived, not delivered. */
@@ -530,9 +529,9 @@
u_int p_swtime; /* (j) Time swapped in or out. */
struct itimerval p_realtimer; /* (c) Alarm timer. */
struct bintime p_runtime; /* (j) Real time. */
- u_int64_t p_uu; /* (j) Previous user time in usec. */
- u_int64_t p_su; /* (j) Previous system time in usec. */
- u_int64_t p_iu; /* (j) Previous intr time in usec. */
+ u_int64_t p_uu; /* (c) Previous user time in usec. */
+ u_int64_t p_su; /* (c) Previous system time in usec. */
+ u_int64_t p_iu; /* (c) Previous intr time in usec. */
u_int64_t p_uticks; /* (j) Statclock hits in user mode. */
u_int64_t p_sticks; /* (j) Statclock hits in system mode. */
u_int64_t p_iticks; /* (j) Statclock hits in intr. */
--- //depot/projects/smpng/sys/sys/resource.h 2004/06/23 20:40:08
+++ //depot/user/jhb/proc/sys/resource.h 2004/06/24 19:35:42
@@ -49,29 +49,34 @@
/*
* Resource utilization information.
+ *
+ * Locking key:
+ * c - locked by proc mtx
+ * j - locked by sched_lock mtx
+ * n - not locked, lazy
*/
#define RUSAGE_SELF 0
#define RUSAGE_CHILDREN -1
struct rusage {
- struct timeval ru_utime; /* user time used */
- struct timeval ru_stime; /* system time used */
- long ru_maxrss; /* max resident set size */
+ struct timeval ru_utime; /* (n) user time used */
+ struct timeval ru_stime; /* (n) system time used */
+ long ru_maxrss; /* (j) max resident set size */
#define ru_first ru_ixrss
- long ru_ixrss; /* integral shared memory size */
- long ru_idrss; /* integral unshared data " */
- long ru_isrss; /* integral unshared stack " */
- long ru_minflt; /* page reclaims */
- long ru_majflt; /* page faults */
- long ru_nswap; /* swaps */
- long ru_inblock; /* block input operations */
- long ru_oublock; /* block output operations */
- long ru_msgsnd; /* messages sent */
- long ru_msgrcv; /* messages received */
- long ru_nsignals; /* signals received */
- long ru_nvcsw; /* voluntary context switches */
- long ru_nivcsw; /* involuntary " */
+ long ru_ixrss; /* (j) integral shared memory size */
+ long ru_idrss; /* (j) integral unshared data " */
+ long ru_isrss; /* (j) integral unshared stack " */
+ long ru_minflt; /* (c) page reclaims */
+ long ru_majflt; /* (c) page faults */
+ long ru_nswap; /* (c + j) swaps */
+ long ru_inblock; /* (n) block input operations */
+ long ru_oublock; /* (n) block output operations */
+ long ru_msgsnd; /* (n) messages sent */
+ long ru_msgrcv; /* (n) messages received */
+ long ru_nsignals; /* (c) signals received */
+ long ru_nvcsw; /* (j) voluntary context switches */
+ long ru_nivcsw; /* (j) involuntary " */
#define ru_last ru_nivcsw
};
Note that the actual p_stats pointer is (b) as correctly annotated in
sys/proc.h. I think I've actually already committed the locking for the
pstats structure. I'm not sure why you think procfs or ptrace() requires
type stable procs since you have to lookup the proc by pid first before you
can try to deference the pointer. The ptrace code at least is careful to
PHOLD() before it ever releases the proc lock it gets from pfind(). It may
be that wait1() isn't handing that edge case well (maybe it doesn't check
p_lock before doing uma_zfree()) but if that is the case that can be easily
fixed. Are there any other ares you are concerned about?
--
John Baldwin <jhb@FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve" = http://www.FreeBSD.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200409201410.58648.jhb>