[PATCH] RUSAGE_THREAD
Alexander Krizhanovsky
ak at natsys-lab.com
Mon May 3 16:14:59 UTC 2010
Kostik,
thank you very much for the review!
Kostik Belousov wrote:
> On Mon, Apr 19, 2010 at 12:46:48AM +0000, Alexander Krizhanovsky wrote:
>
>> Hi all!
>>
>> This patch implements per-thread rusage statistic (like RUSAGE_THREAD in
>> Linux and RUSAGE_LWP in OpenSolaris).
>>
>> Unfortunately, we have to acquire a number of locks to read/update
>> system and user times for current thread rusage information because it's
>> also used for whole process statistic and needs to be zeroed.
>>
>> Any comments are very appreciated.
>>
>> It's tested against 8.0-RELEASE. Appropriate PR is submitted.
>>
>> --
>> Alexander Krizhanovsky
>> NatSys Lab. (http://natsys-lab.com)
>> tel: +7 (916) 717-3899, +7 (499) 747-6304
>> e-mail: ak at natsys-lab.com
>>
>>
> I think this should be somewhat modified before commit.
>
> First, please separate patch into two pieces. One would introduce
> ruxagg_tlock() helper and use it in existing locations instead of
> three-liners. Possibly, add K&R->ANSI C kern_getrusage definition
> change.
>
> Second would actually add RUSAGE_THREAD. There, please follow
> the style(9), in particular, do not initialize the local variables
> at the declaration, instead, use explicit initialization in the code.
>
Done. I have also introduced third patch for casting microseconds to
timeval and replaced a few appropriate places in whole kernel code.
patch-rusage-thread-03052010.txt is incremental for
patch-ruxagg_tlock-03052010.txt, which is by turn incremental for
patch-usec2timeval-03052010.txt.
I have made following change for calcru1():
- if ((int64_t)tu < 0) {
- /* XXX: this should be an assert /phk */
- printf("calcru: negative runtime of %jd usec for pid %d
(%s)\n",
- (intmax_t)tu, p->p_pid, p->p_comm);
- tu = ruxp->rux_tu;
- }
+ KASSERT((int64_t)tu < 0, ("calcru: negative runtime of %jd usec "
+ "for pid %d (%s)\n",
+ (intmax_t)tu, p->p_pid, p->p_comm));
tu can become negative at about 300 thousand years of uptime, so this
condition seems quite unrealistic and indeed should be replaced by an
assertion. However I didn't understand why it isn't done so far and the
comment only was added. Did I miss something?
> Should calctru() share the code with calcru1() ? I am esp. concerned
> with sanity check like negative runtime etc. Possibly, this code
> from calcru1() should be separated into function used from both
> calcru1() and calctru().
>
> Man page for getrusage(2) should be updated.
>
It's added to patch-rusage-thread-03052010.txt.
In the end - shoud I raise a new PR (the original one is kern/145813)?
> Thanks for the submission.
>
>
>> --- sys/sys/resource.h.orig 2009-10-25 01:10:29.000000000 +0000
>> +++ sys/sys/resource.h 2010-04-11 23:31:14.000000000 +0000
>> @@ -56,6 +56,7 @@
>>
>> #define RUSAGE_SELF 0
>> #define RUSAGE_CHILDREN -1
>> +#define RUSAGE_THREAD 1
>>
>> struct rusage {
>> struct timeval ru_utime; /* user time used */
>> --- sys/kern/kern_resource.c.orig 2009-10-25 01:10:29.000000000 +0000
>> +++ sys/kern/kern_resource.c 2010-04-18 23:49:04.000000000 +0000
>> @@ -76,6 +76,7 @@ static void calcru1(struct proc *p, stru
>> struct timeval *up, struct timeval *sp);
>> static int donice(struct thread *td, struct proc *chgp, int n);
>> static struct uidinfo *uilookup(uid_t uid);
>> +static void ruxagg_tlock(struct proc *p, struct thread *td);
>>
>> /*
>> * Resource controls and accounting.
>> @@ -623,9 +624,7 @@ lim_cb(void *arg)
>> return;
>> PROC_SLOCK(p);
>> FOREACH_THREAD_IN_PROC(p, td) {
>> - thread_lock(td);
>> - ruxagg(&p->p_rux, td);
>> - thread_unlock(td);
>> + ruxagg_tlock(p, td);
>> }
>> PROC_SUNLOCK(p);
>> if (p->p_rux.rux_runtime > p->p_cpulimit * cpu_tickrate()) {
>> @@ -836,9 +835,7 @@ calcru(struct proc *p, struct timeval *u
>> FOREACH_THREAD_IN_PROC(p, td) {
>> if (td->td_incruntime == 0)
>> continue;
>> - thread_lock(td);
>> - ruxagg(&p->p_rux, td);
>> - thread_unlock(td);
>> + ruxagg_tlock(p, td);
>> }
>> calcru1(p, &p->p_rux, up, sp);
>> }
>> @@ -918,6 +915,29 @@ calcru1(struct proc *p, struct rusage_ex
>> sp->tv_usec = su % 1000000;
>> }
>>
>> +static void
>> +calctru(struct thread *td)
>> +{
>> + u_int64_t tu = cputick2usec(td->td_incruntime);
>> + u_int64_t ut = td->td_uticks;
>> + u_int64_t it = td->td_iticks;
>> + u_int64_t st = td->td_sticks;
>> + u_int64_t tt, uu, su;
>> +
>> + tt = ut + st + it;
>> + if (!tt) {
>> + /* Avoid divide by zero */
>> + st = 1;
>> + tt = 1;
>> + }
>> + uu = td->td_ru.ru_utime.tv_usec + (ut * tu) / tt;
>> + su = td->td_ru.ru_stime.tv_usec + (st * tu) / tt;
>> + td->td_ru.ru_utime.tv_sec += uu / 1000000;
>> + td->td_ru.ru_utime.tv_usec = uu % 1000000;
>> + td->td_ru.ru_stime.tv_sec += su / 1000000;
>> + td->td_ru.ru_stime.tv_usec = su % 1000000;
>> +}
>> +
>> #ifndef _SYS_SYSPROTO_H_
>> struct getrusage_args {
>> int who;
>> @@ -939,10 +959,7 @@ getrusage(td, uap)
>> }
>>
>> int
>> -kern_getrusage(td, who, rup)
>> - struct thread *td;
>> - int who;
>> - struct rusage *rup;
>> +kern_getrusage(struct thread *td, int who, struct rusage *rup)
>> {
>> struct proc *p;
>> int error;
>> @@ -961,6 +978,13 @@ kern_getrusage(td, who, rup)
>> calccru(p, &rup->ru_utime, &rup->ru_stime);
>> break;
>>
>> + case RUSAGE_THREAD:
>> + PROC_SLOCK(p);
>> + ruxagg_tlock(p, td);
>> + PROC_SUNLOCK(p);
>> + *rup = td->td_ru;
>> + break;
>> +
>> default:
>> error = EINVAL;
>> }
>> @@ -1010,12 +1034,24 @@ ruxagg(struct rusage_ext *rux, struct th
>> rux->rux_uticks += td->td_uticks;
>> rux->rux_sticks += td->td_sticks;
>> rux->rux_iticks += td->td_iticks;
>> +
>> + /* update thread rusage before ticks counters cleaning */
>> + calctru(td);
>> +
>> td->td_incruntime = 0;
>> td->td_uticks = 0;
>> td->td_iticks = 0;
>> td->td_sticks = 0;
>> }
>>
>> +static void
>> +ruxagg_tlock(struct proc *p, struct thread *td)
>> +{
>> + thread_lock(td);
>> + ruxagg(&p->p_rux, td);
>> + thread_unlock(td);
>> +}
>> +
>> /*
>> * Update the rusage_ext structure and fetch a valid aggregate rusage
>> * for proc p if storage for one is supplied.
>> @@ -1030,9 +1066,7 @@ rufetch(struct proc *p, struct rusage *r
>> *ru = p->p_ru;
>> if (p->p_numthreads > 0) {
>> FOREACH_THREAD_IN_PROC(p, td) {
>> - thread_lock(td);
>> - ruxagg(&p->p_rux, td);
>> - thread_unlock(td);
>> + ruxagg_tlock(p, td);
>> rucollect(ru, &td->td_ru);
>> }
>> }
>>
>
>
>> _______________________________________________
>> freebsd-hackers at freebsd.org mailing list
>> http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
>> To unsubscribe, send any mail to "freebsd-hackers-unsubscribe at freebsd.org"
>>
--
Alexander Krizhanovsky
NatSys Lab. (http://natsys-lab.com)
tel: +7 (916) 717-3899, +7 (499) 747-6304
e-mail: ak at natsys-lab.com
-------------- next part --------------
--- src/sys/sys/resource.h.orig 2010-05-02 16:37:26.537274709 +0000
+++ src/sys/sys/resource.h 2010-05-03 19:45:46.357159359 +0000
@@ -56,6 +56,7 @@
#define RUSAGE_SELF 0
#define RUSAGE_CHILDREN -1
+#define RUSAGE_THREAD 1
struct rusage {
struct timeval ru_utime; /* user time used */
--- src/sys/kern/kern_resource.c.orig 2010-05-03 19:42:08.474513380 +0000
+++ src/sys/kern/kern_resource.c 2010-05-03 19:45:46.358157251 +0000
@@ -840,29 +840,35 @@ calcru(struct proc *p, struct timeval *u
calcru1(p, &p->p_rux, up, sp);
}
+static __inline uint64_t
+calcru_adjust_time(uint64_t it, uint64_t ut, uint64_t *st)
+{
+ uint64_t tt;
+
+ tt = it + ut + *st;
+ if (tt == 0) {
+ /* Avoid divide by zero. */
+ *st = 1;
+ tt = 1;
+ }
+ return (tt);
+}
+
static void
calcru1(struct proc *p, struct rusage_ext *ruxp, struct timeval *up,
struct timeval *sp)
{
/* {user, system, interrupt, total} {ticks, usec}: */
- u_int64_t ut, uu, st, su, it, tt, tu;
+ uint64_t ut, uu, st, su, it, tt, tu;
ut = ruxp->rux_uticks;
st = ruxp->rux_sticks;
it = ruxp->rux_iticks;
- tt = ut + st + it;
- if (tt == 0) {
- /* Avoid divide by zero */
- st = 1;
- tt = 1;
- }
+ tt = calcru_adjust_time(it, ut, &st);
tu = cputick2usec(ruxp->rux_runtime);
- if ((int64_t)tu < 0) {
- /* XXX: this should be an assert /phk */
- printf("calcru: negative runtime of %jd usec for pid %d (%s)\n",
- (intmax_t)tu, p->p_pid, p->p_comm);
- tu = ruxp->rux_tu;
- }
+ KASSERT((int64_t)tu < 0, ("calcru: negative runtime of %jd usec "
+ "for pid %d (%s)\n",
+ (intmax_t)tu, p->p_pid, p->p_comm));
if (tu >= ruxp->rux_tu) {
/*
@@ -913,6 +919,25 @@ calcru1(struct proc *p, struct rusage_ex
usec2timeval(su, sp);
}
+static void
+calctru(struct thread *td)
+{
+ uint64_t ut, uu, st, su, it, tt, tu;
+
+ ut = td->td_uticks;
+ it = td->td_iticks;
+ st = td->td_sticks;
+ tt = calcru_adjust_time(it, ut, &st);
+ tu = cputick2usec(td->td_incruntime);
+ KASSERT((int64_t)tu < 0, ("calctru: negative runtime of %jd usec "
+ "for tid %d\n", (intmax_t)tu, td->td_tid));
+
+ uu = td->td_ru.ru_utime.tv_usec + (ut * tu) / tt;
+ su = td->td_ru.ru_stime.tv_usec + (st * tu) / tt;
+ usec2timeval(&td->td_ru.ru_utime, uu);
+ usec2timeval(&td->td_ru.ru_stime, su);
+}
+
#ifndef _SYS_SYSPROTO_H_
struct getrusage_args {
int who;
@@ -953,6 +978,13 @@ kern_getrusage(struct thread *td, int wh
calccru(p, &rup->ru_utime, &rup->ru_stime);
break;
+ case RUSAGE_THREAD:
+ PROC_SLOCK(p);
+ ruxagg_tlock(p, td);
+ PROC_SUNLOCK(p);
+ *rup = td->td_ru;
+ break;
+
default:
error = EINVAL;
}
@@ -1002,6 +1034,10 @@ ruxagg(struct rusage_ext *rux, struct th
rux->rux_uticks += td->td_uticks;
rux->rux_sticks += td->td_sticks;
rux->rux_iticks += td->td_iticks;
+
+ /* Update thread rusage before ticks counters cleaning. */
+ calctru(td);
+
td->td_incruntime = 0;
td->td_uticks = 0;
td->td_iticks = 0;
--- src/lib/libc/sys/getrusage.2.orig 2009-10-25 01:10:29.000000000 +0000
+++ src/lib/libc/sys/getrusage.2 2010-05-03 19:45:46.359155143 +0000
@@ -42,6 +42,7 @@
.In sys/resource.h
.Fd "#define RUSAGE_SELF 0"
.Fd "#define RUSAGE_CHILDREN -1"
+.Fd "#define RUSAGE_THREAD 1"
.Ft int
.Fn getrusage "int who" "struct rusage *rusage"
.Sh DESCRIPTION
@@ -49,11 +50,13 @@ The
.Fn getrusage
system call
returns information describing the resources utilized by the current
-process, or all its terminated child processes.
-The
+process, the current thread or all terminated children of the
+current process.
+The corresponding
.Fa who
argument is either
-.Dv RUSAGE_SELF
+.Dv RUSAGE_SELF ,
+.Dv RUSAGE_THREAD
or
.Dv RUSAGE_CHILDREN .
The buffer to which
-------------- next part --------------
--- src/sys/kern/kern_resource.c.orig 2010-05-03 19:31:45.051576945 +0000
+++ src/sys/kern/kern_resource.c 2010-05-03 19:42:08.474513380 +0000
@@ -76,6 +76,7 @@ static void calcru1(struct proc *p, stru
struct timeval *up, struct timeval *sp);
static int donice(struct thread *td, struct proc *chgp, int n);
static struct uidinfo *uilookup(uid_t uid);
+static void ruxagg_tlock(struct proc *p, struct thread *td);
/*
* Resource controls and accounting.
@@ -623,9 +624,7 @@ lim_cb(void *arg)
return;
PROC_SLOCK(p);
FOREACH_THREAD_IN_PROC(p, td) {
- thread_lock(td);
- ruxagg(&p->p_rux, td);
- thread_unlock(td);
+ ruxagg_tlock(p, td);
}
PROC_SUNLOCK(p);
if (p->p_rux.rux_runtime > p->p_cpulimit * cpu_tickrate()) {
@@ -836,9 +835,7 @@ calcru(struct proc *p, struct timeval *u
FOREACH_THREAD_IN_PROC(p, td) {
if (td->td_incruntime == 0)
continue;
- thread_lock(td);
- ruxagg(&p->p_rux, td);
- thread_unlock(td);
+ ruxagg_tlock(p, td);
}
calcru1(p, &p->p_rux, up, sp);
}
@@ -937,10 +934,7 @@ getrusage(td, uap)
}
int
-kern_getrusage(td, who, rup)
- struct thread *td;
- int who;
- struct rusage *rup;
+kern_getrusage(struct thread *td, int who, struct rusage *rup)
{
struct proc *p;
int error;
@@ -1014,6 +1008,15 @@ ruxagg(struct rusage_ext *rux, struct th
td->td_sticks = 0;
}
+static void
+ruxagg_tlock(struct proc *p, struct thread *td)
+{
+
+ thread_lock(td);
+ ruxagg(&p->p_rux, td);
+ thread_unlock(td);
+}
+
/*
* Update the rusage_ext structure and fetch a valid aggregate rusage
* for proc p if storage for one is supplied.
@@ -1028,9 +1031,7 @@ rufetch(struct proc *p, struct rusage *r
*ru = p->p_ru;
if (p->p_numthreads > 0) {
FOREACH_THREAD_IN_PROC(p, td) {
- thread_lock(td);
- ruxagg(&p->p_rux, td);
- thread_unlock(td);
+ ruxagg_tlock(p, td);
rucollect(ru, &td->td_ru);
}
}
-------------- next part --------------
--- src/sys/sys/time.h.orig 2009-10-25 01:10:29.000000000 +0000
+++ src/sys/sys/time.h 2010-05-03 19:31:45.008535442 +0000
@@ -178,6 +178,14 @@ timeval2bintime(const struct timeval *tv
/* timevaladd and timevalsub are not inlined */
+static __inline void
+usec2timeval(uint64_t msec, struct timeval *tv)
+{
+
+ tv->tv_sec = msec / 1000000;
+ tv->tv_usec = msec % 1000000;
+}
+
#endif /* _KERNEL */
#ifndef _KERNEL /* NetBSD/OpenBSD compatible interfaces */
--- src/sys/dev/sound/midi/sequencer.c.orig 2009-10-25 01:10:29.000000000 +0000
+++ src/sys/dev/sound/midi/sequencer.c 2010-05-03 19:31:45.037557566 +0000
@@ -65,6 +65,7 @@ __FBSDID("$FreeBSD: src/sys/dev/sound/mi
#include <sys/kthread.h>
#include <sys/unistd.h>
#include <sys/selinfo.h>
+#include <sys/time.h>
#ifdef HAVE_KERNEL_OPTION_HEADERS
#include "opt_snd.h"
@@ -364,8 +365,7 @@ timer_wait(struct seq_softc *t, int tick
i = ticks * 60ull * 1000000ull / (t->tempo * t->timerbase);
- when.tv_sec = i / 1000000;
- when.tv_usec = i % 1000000;
+ usec2timeval(i, &when);
#if 0
printf("timer_wait tempo %d timerbase %d ticks %d abs %d u_sec %llu\n",
--- src/sys/kern/kern_ntptime.c.orig 2009-10-25 01:10:29.000000000 +0000
+++ src/sys/kern/kern_ntptime.c 2010-05-03 19:31:45.048574050 +0000
@@ -952,8 +952,7 @@ kern_adjtime(struct thread *td, struct t
mtx_lock(&Giant);
if (olddelta) {
- atv.tv_sec = time_adjtime / 1000000;
- atv.tv_usec = time_adjtime % 1000000;
+ usec2timeval(time_adjtime, &atv);
if (atv.tv_usec < 0) {
atv.tv_usec += 1000000;
atv.tv_sec--;
--- src/sys/kern/kern_resource.c.orig 2009-10-25 01:10:29.000000000 +0000
+++ src/sys/kern/kern_resource.c 2010-05-03 19:31:45.051576945 +0000
@@ -912,10 +912,8 @@ calcru1(struct proc *p, struct rusage_ex
ruxp->rux_su = su;
ruxp->rux_tu = tu;
- up->tv_sec = uu / 1000000;
- up->tv_usec = uu % 1000000;
- sp->tv_sec = su / 1000000;
- sp->tv_usec = su % 1000000;
+ usec2timeval(uu, up);
+ usec2timeval(su, sp);
}
#ifndef _SYS_SYSPROTO_H_
--- src/sys/netinet/sctp_timer.c.orig 2009-10-25 01:10:29.000000000 +0000
+++ src/sys/netinet/sctp_timer.c 2010-05-03 19:31:45.058544591 +0000
@@ -50,6 +50,7 @@ __FBSDID("$FreeBSD: src/sys/netinet/sctp
#include <netinet/sctp.h>
#include <netinet/sctp_uio.h>
#include <netinet/udp.h>
+#include <sys/time.h>
void
@@ -75,8 +76,7 @@ sctp_early_fr_timer(struct sctp_inpcb *i
cur_rtt = SCTP_BASE_SYSCTL(sctp_early_fr_msec);
}
cur_rtt *= 1000;
- tv.tv_sec = cur_rtt / 1000000;
- tv.tv_usec = cur_rtt % 1000000;
+ usec2timeval(cur_rtt, &tv);
min_wait = now;
timevalsub(&min_wait, &tv);
if (min_wait.tv_sec < 0 || min_wait.tv_usec < 0) {
More information about the freebsd-hackers
mailing list