cvs commit: src/sys/kern kern_proc.c kern_switch.c src/sys/sys sched.h src/sys/vm vm_glue.c

[John Baldwin]

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 at FreeBSD.org>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve"  =  http://www.FreeBSD.org