rufetch and calcru sometimes should be called atomically together.

This patch fixes places where they should be called atomically changing
their locking requirements (both assume per-proc spinlock held) and
introducing rufetchcalc which wrappers both calls to be performed in
atomic way.

Reviewed by: jeff
Approved by: jeff (mentor)

9 files changed, 47 insertions, 23 deletions

diff --git a/sys/compat/linux/linux_misc.c b/sys/compat/linux/linux_misc.c
index e09310c..926efbc 100644
--- a/sys/compat/linux/linux_misc.c
+++ b/sys/compat/linux/linux_misc.c
@@ -670,7 +670,9 @@ linux_times(struct thread *td, struct linux_times_args *args)
 	if (args->buf != NULL) {
 		p = td->td_proc;
 		PROC_LOCK(p);
+		PROC_SLOCK(p);
 		calcru(p, &utime, &stime);
+		PROC_SUNLOCK(p);
 		calccru(p, &cutime, &cstime);
 		PROC_UNLOCK(p);
 
diff --git a/sys/compat/svr4/svr4_misc.c b/sys/compat/svr4/svr4_misc.c
index 7277622..a158fd7 100644
--- a/sys/compat/svr4/svr4_misc.c
+++ b/sys/compat/svr4/svr4_misc.c
@@ -828,7 +828,9 @@ svr4_sys_times(td, uap)
 
 	p = td->td_proc;
 	PROC_LOCK(p);
+	PROC_SLOCK(p);
 	calcru(p, &utime, &stime);
+	PROC_SUNLOCK(p);
 	calccru(p, &cutime, &cstime);
 	PROC_UNLOCK(p);
 
@@ -1241,7 +1243,9 @@ loop:
 			pid = p->p_pid;
 			status = p->p_xstat;
 			ru = p->p_ru;
+			PROC_SLOCK(p);
 			calcru(p, &ru.ru_utime, &ru.ru_stime);
+			PROC_SUNLOCK(p);
 			PROC_UNLOCK(p);
 			sx_sunlock(&proctree_lock);
 
@@ -1266,7 +1270,9 @@ loop:
 			pid = p->p_pid;
 			status = W_STOPCODE(p->p_xstat);
 			ru = p->p_ru;
+			PROC_SLOCK(p);
 			calcru(p, &ru.ru_utime, &ru.ru_stime);
+			PROC_SUNLOCK(p);
 			PROC_UNLOCK(p);
 
 		        if (((uap->options & SVR4_WNOWAIT)) == 0) {
@@ -1288,7 +1294,9 @@ loop:
 			pid = p->p_pid;
 			ru = p->p_ru;
 			status = SIGCONT;
+			PROC_SLOCK(p);
 			calcru(p, &ru.ru_utime, &ru.ru_stime);
+			PROC_SUNLOCK(p);
 			PROC_UNLOCK(p);
 
 		        if (((uap->options & SVR4_WNOWAIT)) == 0) {
diff --git a/sys/fs/procfs/procfs_status.c b/sys/fs/procfs/procfs_status.c
index b92d157..1a8148b 100644
--- a/sys/fs/procfs/procfs_status.c
+++ b/sys/fs/procfs/procfs_status.c
@@ -127,12 +127,12 @@ procfs_doprocstatus(PFS_FILL_ARGS)
 		} else
 			wmesg = "nochan";
 	}
-	PROC_SUNLOCK(p);
 
 	if (p->p_sflag & PS_INMEM) {
 		struct timeval start, ut, st;
 
 		calcru(p, &ut, &st);
+		PROC_SUNLOCK(p);
 		start = p->p_stats->p_start;
 		timevaladd(&start, &boottime);
 		sbuf_printf(sb, " %jd,%ld %jd,%ld %jd,%ld",
@@ -140,6 +140,7 @@ procfs_doprocstatus(PFS_FILL_ARGS)
 		    (intmax_t)ut.tv_sec, ut.tv_usec,
 		    (intmax_t)st.tv_sec, st.tv_usec);
 	} else {
+		PROC_SUNLOCK(p);
 		sbuf_printf(sb, " -1,-1 -1,-1 -1,-1");
 	}
 
diff --git a/sys/kern/kern_acct.c b/sys/kern/kern_acct.c
index e7409b3..269b4e4 100644
--- a/sys/kern/kern_acct.c
+++ b/sys/kern/kern_acct.c
@@ -370,8 +370,7 @@ acct_process(struct thread *td)
 	bcopy(p->p_comm, acct.ac_comm, sizeof acct.ac_comm);
 
 	/* (2) The amount of user and system time that was used */
-	rufetch(p, &ru);
-	calcru(p, &ut, &st);
+	rufetchcalc(p, &ru, &ut, &st);
 	acct.ac_utime = encode_timeval(ut);
 	acct.ac_stime = encode_timeval(st);
 
diff --git a/sys/kern/kern_exit.c b/sys/kern/kern_exit.c
index 58abd2e..7700a8f 100644
--- a/sys/kern/kern_exit.c
+++ b/sys/kern/kern_exit.c
@@ -705,16 +705,14 @@ loop:
 		nfound++;
 		PROC_SLOCK(p);
 		if (p->p_state == PRS_ZOMBIE) {
-			PROC_SUNLOCK(p);
-			
-			td->td_retval[0] = p->p_pid;
-			if (status)
-				*status = p->p_xstat;	/* convert to int */
 			if (rusage) {
 				*rusage = p->p_ru;
 				calcru(p, &rusage->ru_utime, &rusage->ru_stime);
 			}
-
+			PROC_SUNLOCK(p);
+			td->td_retval[0] = p->p_pid;
+			if (status)
+				*status = p->p_xstat;	/* convert to int */
 			PROC_LOCK(q);
 			sigqueue_take(p->p_ksi);
 			PROC_UNLOCK(q);
diff --git a/sys/kern/kern_proc.c b/sys/kern/kern_proc.c
index 7abdfcf..f9f993d 100644
--- a/sys/kern/kern_proc.c
+++ b/sys/kern/kern_proc.c
@@ -700,7 +700,9 @@ fill_kinfo_proc_only(struct proc *p, struct kinfo_proc *kp)
 	if ((p->p_sflag & PS_INMEM) && p->p_stats != NULL) {
 		kp->ki_start = p->p_stats->p_start;
 		timevaladd(&kp->ki_start, &boottime);
+		PROC_SLOCK(p);
 		calcru(p, &kp->ki_rusage.ru_utime, &kp->ki_rusage.ru_stime);
+		PROC_SUNLOCK(p);
 		calccru(p, &kp->ki_childutime, &kp->ki_childstime);
 
 		/* Some callers want child-times in a single value */
diff --git a/sys/kern/kern_resource.c b/sys/kern/kern_resource.c
index 5982066..9f47402 100644
--- a/sys/kern/kern_resource.c
+++ b/sys/kern/kern_resource.c
@@ -829,12 +829,11 @@ calccru(p, up, sp)
 void
 calcru(struct proc *p, struct timeval *up, struct timeval *sp)
 {
-	struct rusage_ext rux;
 	struct thread *td;
 	uint64_t u;
 
 	PROC_LOCK_ASSERT(p, MA_OWNED);
-	PROC_SLOCK(p);
+	PROC_SLOCK_ASSERT(p, MA_OWNED);
 	/*
 	 * If we are getting stats for the current process, then add in the
 	 * stats that this thread has accumulated in its current time slice.
@@ -847,14 +846,7 @@ calcru(struct proc *p, struct timeval *up, struct timeval *sp)
 		p->p_rux.rux_runtime += u - PCPU_GET(switchtime);
 		PCPU_SET(switchtime, u);
 	}
-	/* Work on a copy of p_rux so we can let go of p_slock */
-	rux = p->p_rux;
-	PROC_SUNLOCK(p);
-	calcru1(p, &rux, up, sp);
-	/* Update the result from the p_rux copy */
-	p->p_rux.rux_uu = rux.rux_uu;
-	p->p_rux.rux_su = rux.rux_su;
-	p->p_rux.rux_tu = rux.rux_tu;
+	calcru1(p, &p->p_rux, up, sp);
 }
 
 static void
@@ -965,8 +957,8 @@ kern_getrusage(td, who, rup)
 	switch (who) {
 
 	case RUSAGE_SELF:
-		rufetch(p, rup);
-		calcru(p, &rup->ru_utime, &rup->ru_stime);
+		rufetchcalc(p, rup, &rup->ru_utime,
+		    &rup->ru_stime);
 		break;
 
 	case RUSAGE_CHILDREN:
@@ -1039,7 +1031,8 @@ rufetch(struct proc *p, struct rusage *ru)
 {
 	struct thread *td;
 
-	PROC_SLOCK(p);
+	PROC_SLOCK_ASSERT(p, MA_OWNED);
+
 	*ru = p->p_ru;
 	if (p->p_numthreads > 0)  {
 		FOREACH_THREAD_IN_PROC(p, td) {
@@ -1049,6 +1042,21 @@ rufetch(struct proc *p, struct rusage *ru)
 			rucollect(ru, &td->td_ru);
 		}
 	}
+}
+
+/*
+ * Atomically perform a rufetch and a calcru together.
+ * Consumers, can safely assume the calcru is executed only once
+ * rufetch is completed.
+ */
+void
+rufetchcalc(struct proc *p, struct rusage *ru, struct timeval *up,
+    struct timeval *sp)
+{
+
+	PROC_SLOCK(p);
+	rufetch(p, ru);
+	calcru(p, up, sp);
 	PROC_SUNLOCK(p);
 }
 
diff --git a/sys/kern/kern_time.c b/sys/kern/kern_time.c
index 8634c8a..e220cd6 100644
--- a/sys/kern/kern_time.c
+++ b/sys/kern/kern_time.c
@@ -213,13 +213,17 @@ kern_clock_gettime(struct thread *td, clockid_t clock_id, struct timespec *ats)
 		break;
 	case CLOCK_VIRTUAL:
 		PROC_LOCK(p);
+		PROC_SLOCK(p);
 		calcru(p, &user, &sys);
+		PROC_SUNLOCK(p);
 		PROC_UNLOCK(p);
 		TIMEVAL_TO_TIMESPEC(&user, ats);
 		break;
 	case CLOCK_PROF:
 		PROC_LOCK(p);
+		PROC_SLOCK(p);
 		calcru(p, &user, &sys);
+		PROC_SUNLOCK(p);
 		PROC_UNLOCK(p);
 		timevaladd(&user, &sys);
 		TIMEVAL_TO_TIMESPEC(&user, ats);
diff --git a/sys/sys/resourcevar.h b/sys/sys/resourcevar.h
index a94ba94..ea8bbdb 100644
--- a/sys/sys/resourcevar.h
+++ b/sys/sys/resourcevar.h
@@ -123,8 +123,10 @@ rlim_t	 lim_max(struct proc *p, int which);
 void	 lim_rlimit(struct proc *p, int which, struct rlimit *rlp);
 void	 ruadd(struct rusage *ru, struct rusage_ext *rux, struct rusage *ru2,
 	    struct rusage_ext *rux2);
-void	 rufetch(struct proc *p, struct rusage *ru);
 void	 rucollect(struct rusage *ru, struct rusage *ru2);
+void	 rufetch(struct proc *p, struct rusage *ru);
+void	 rufetchcalc(struct proc *p, struct rusage *ru, struct timeval *up,
+	    struct timeval *sp);
 void	 ruxagg(struct rusage_ext *rux, struct thread *td);
 int	 suswintr(void *base, int word);
 struct uidinfo