From a1fe14bc333c7b2b9886a7cdd7d78677e0a01413 Mon Sep 17 00:00:00 2001 From: Attilio Rao Date: Sat, 9 Jun 2007 21:48:44 +0000 Subject: [PATCH] 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) --- sys/compat/linux/linux_misc.c | 2 ++ sys/compat/svr4/svr4_misc.c | 8 ++++++++ sys/fs/procfs/procfs_status.c | 3 ++- sys/kern/kern_acct.c | 3 +-- sys/kern/kern_exit.c | 10 ++++------ sys/kern/kern_proc.c | 2 ++ sys/kern/kern_resource.c | 34 +++++++++++++++++++++------------- sys/kern/kern_time.c | 4 ++++ sys/sys/resourcevar.h | 4 +++- 9 files changed, 47 insertions(+), 23 deletions(-) diff --git a/sys/compat/linux/linux_misc.c b/sys/compat/linux/linux_misc.c index e09310c7aad4..926efbc1f55c 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 727762221376..a158fd7cf2cd 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 @@ svr4_sys_waitsys(td, uap) 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 @@ svr4_sys_waitsys(td, uap) 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 @@ svr4_sys_waitsys(td, uap) 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 b92d1577ae08..1a8148b6107c 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 e7409b33bdc0..269b4e481398 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 58abd2e79148..7700a8f9271a 100644 --- a/sys/kern/kern_exit.c +++ b/sys/kern/kern_exit.c @@ -705,16 +705,14 @@ kern_wait(struct thread *td, pid_t pid, int *status, int options, 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 7abdfcf90f84..f9f993d85aa8 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 5982066029f3..9f47402807f0 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 8634c8a68d85..e220cd6e5a4a 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 a94ba94c13d2..ea8bbdbe0040 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