From owner-svn-src-head@FreeBSD.ORG Sun May 2 15:52:49 2010 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 324391065672; Sun, 2 May 2010 15:52:49 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from mail.zoral.com.ua (mx0.zoral.com.ua [91.193.166.200]) by mx1.freebsd.org (Postfix) with ESMTP id 8E0CD8FC0C; Sun, 2 May 2010 15:52:48 +0000 (UTC) Received: from deviant.kiev.zoral.com.ua (root@deviant.kiev.zoral.com.ua [10.1.1.148]) by mail.zoral.com.ua (8.14.2/8.14.2) with ESMTP id o42FqiUd013282 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sun, 2 May 2010 18:52:44 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: from deviant.kiev.zoral.com.ua (kostik@localhost [127.0.0.1]) by deviant.kiev.zoral.com.ua (8.14.4/8.14.4) with ESMTP id o42Fqi2o056410; Sun, 2 May 2010 18:52:44 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by deviant.kiev.zoral.com.ua (8.14.4/8.14.4/Submit) id o42FqiwI056409; Sun, 2 May 2010 18:52:44 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: deviant.kiev.zoral.com.ua: kostik set sender to kostikbel@gmail.com using -f Date: Sun, 2 May 2010 18:52:44 +0300 From: Kostik Belousov To: Attilio Rao Message-ID: <20100502155244.GB50864@deviant.kiev.zoral.com.ua> References: <201005011446.o41EkIa6051907@svn.freebsd.org> <20100501151339.GZ2391@deviant.kiev.zoral.com.ua> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="xgyAXRrhYN0wYx8y" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.3i X-Virus-Scanned: clamav-milter 0.95.2 at skuns.kiev.zoral.com.ua X-Virus-Status: Clean X-Spam-Status: No, score=-2.5 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_50, DNS_FROM_OPENWHOIS autolearn=no version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on skuns.kiev.zoral.com.ua Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r207468 - head/sys/kern X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 02 May 2010 15:52:49 -0000 --xgyAXRrhYN0wYx8y Content-Type: text/plain; charset=koi8-r Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, May 02, 2010 at 03:48:50PM +0200, Attilio Rao wrote: > 2010/5/1 Kostik Belousov : > > On Sat, May 01, 2010 at 04:47:36PM +0200, Attilio Rao wrote: > >> 2010/5/1 Konstantin Belousov : > >> > Author: kib > >> > Date: Sat May =9A1 14:46:17 2010 > >> > New Revision: 207468 > >> > URL: http://svn.freebsd.org/changeset/base/207468 > >> > > >> > Log: > >> > =9AExtract thread_lock()/ruxagg()/thread_unlock() fragment into util= ity > >> > =9Afunction ruxagg_tlock(). > >> > =9AConvert the definition of kern_getrusage() to ANSI C. > >> > > >> > >> I would have preferred a different naming for this, as the well known > >> _locked version we have of many functions. > > > > But this is not the case there, because I did not renamed ruxagg(). > > It would be ruxagg()->ruxagg_unlocked() and ruxagg_tlock()->ruxagg(). >=20 > Yes, this is exactly what I wanted to happen. >=20 > > My biggest question with the patch is I am not sure whether to apply > > the same checks for tu in calctru() as it is done for tu in calcru1(). > > Any suggestions ? >=20 > I think that the checks may be present (the process-scope one is just > an aggregate of the threads' one, thus the same conditions apply. Ok, then the easiest way is to rewrite the patch. I removed your comments for previous version since they are no longer relevant. I have to add rusage_ext to struct thread, that would complicate the MFC. And, looking at the whole picture, I do not understand how do we handle the exiting threads. It seems that only user/system runtime is getting collected to the process rusage. The other values, like i/o counters, signals, ctx switches etc are thrown out. diff --git a/lib/libc/sys/getrusage.2 b/lib/libc/sys/getrusage.2 index bdf5d45..423503f 100644 --- a/lib/libc/sys/getrusage.2 +++ b/lib/libc/sys/getrusage.2 @@ -28,7 +28,7 @@ .\" @(#)getrusage.2 8.1 (Berkeley) 6/4/93 .\" $FreeBSD$ .\" -.Dd June 4, 1993 +.Dd May 1, 2010 .Dt GETRUSAGE 2 .Os .Sh NAME @@ -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,12 @@ The .Fn getrusage system call returns information describing the resources utilized by the current -process, or all its terminated child processes. +thread, the current process, or all its terminated child processes. The .Fa who argument is either -.Dv RUSAGE_SELF +.Dv RUSAGE_THREAD , +.Dv RUSAGE_SELF , or .Dv RUSAGE_CHILDREN . The buffer to which @@ -175,6 +177,10 @@ The .Fn getrusage system call appeared in .Bx 4.2 . +The +.Dv RUSAGE_THREAD +facility first appeared in +.Fx 8.1 . .Sh BUGS There is no way to obtain information about a child process that has not yet terminated. diff --git a/sys/kern/kern_resource.c b/sys/kern/kern_resource.c index a3ed75d..0bc78d0 100644 --- a/sys/kern/kern_resource.c +++ b/sys/kern/kern_resource.c @@ -76,7 +76,7 @@ static void calcru1(struct proc *p, struct rusage_ext *ru= xp, 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); +static void ruxagg(struct proc *p, struct thread *td); =20 /* * Resource controls and accounting. @@ -630,7 +630,7 @@ lim_cb(void *arg) return; PROC_SLOCK(p); FOREACH_THREAD_IN_PROC(p, td) { - ruxagg_tlock(p, td); + ruxagg(p, td); } PROC_SUNLOCK(p); if (p->p_rux.rux_runtime > p->p_cpulimit * cpu_tickrate()) { @@ -841,7 +841,7 @@ calcru(struct proc *p, struct timeval *up, struct timev= al *sp) FOREACH_THREAD_IN_PROC(p, td) { if (td->td_incruntime =3D=3D 0) continue; - ruxagg_tlock(p, td); + ruxagg(p, td); } calcru1(p, &p->p_rux, up, sp); } @@ -961,6 +961,16 @@ kern_getrusage(struct thread *td, int who, struct rusa= ge *rup) calccru(p, &rup->ru_utime, &rup->ru_stime); break; =20 + case RUSAGE_THREAD: + PROC_SLOCK(p); + ruxagg(p, td); + PROC_SUNLOCK(p); + thread_lock(td); + *rup =3D td->td_ru; + calcru1(p, &td->td_rux, &rup->ru_utime, &rup->ru_stime); + thread_unlock(td); + break; + default: error =3D EINVAL; } @@ -1001,7 +1011,7 @@ ruadd(struct rusage *ru, struct rusage_ext *rux, stru= ct rusage *ru2, * Aggregate tick counts into the proc's rusage_ext. */ void -ruxagg(struct rusage_ext *rux, struct thread *td) +ruxagg_locked(struct rusage_ext *rux, struct thread *td) { =20 THREAD_LOCK_ASSERT(td, MA_OWNED); @@ -1010,18 +1020,19 @@ ruxagg(struct rusage_ext *rux, struct thread *td) rux->rux_uticks +=3D td->td_uticks; rux->rux_sticks +=3D td->td_sticks; rux->rux_iticks +=3D td->td_iticks; - td->td_incruntime =3D 0; - td->td_uticks =3D 0; - td->td_iticks =3D 0; - td->td_sticks =3D 0; } =20 static void -ruxagg_tlock(struct proc *p, struct thread *td) +ruxagg(struct proc *p, struct thread *td) { =20 thread_lock(td); - ruxagg(&p->p_rux, td); + ruxagg_locked(&p->p_rux, td); + ruxagg_locked(&td->td_rux, td); + td->td_incruntime =3D 0; + td->td_uticks =3D 0; + td->td_iticks =3D 0; + td->td_sticks =3D 0; thread_unlock(td); } =20 @@ -1039,7 +1050,7 @@ rufetch(struct proc *p, struct rusage *ru) *ru =3D p->p_ru; if (p->p_numthreads > 0) { FOREACH_THREAD_IN_PROC(p, td) { - ruxagg_tlock(p, td); + ruxagg(p, td); rucollect(ru, &td->td_ru); } } diff --git a/sys/kern/kern_thread.c b/sys/kern/kern_thread.c index 9be4c2f..b32c584 100644 --- a/sys/kern/kern_thread.c +++ b/sys/kern/kern_thread.c @@ -432,7 +432,7 @@ thread_exit(void) PROC_UNLOCK(p); thread_lock(td); /* Save our tick information with both the thread and proc locked */ - ruxagg(&p->p_rux, td); + ruxagg_locked(&p->p_rux, td); PROC_SUNLOCK(p); td->td_state =3D TDS_INACTIVE; #ifdef WITNESS diff --git a/sys/sys/proc.h b/sys/sys/proc.h index fb31cfc..d2ff3df 100644 --- a/sys/sys/proc.h +++ b/sys/sys/proc.h @@ -173,6 +173,26 @@ struct kdtrace_thread; struct cpuset; =20 /* + * XXX: Does this belong in resource.h or resourcevar.h instead? + * Resource usage extension. The times in rusage structs in the kernel are + * never up to date. The actual times are kept as runtimes and tick counts + * (with control info in the "previous" times), and are converted when + * userland asks for rusage info. Backwards compatibility prevents putting + * this directly in the user-visible rusage struct. + * + * Locking: (cj) means (j) for p_rux and (c) for p_crux. + */ +struct rusage_ext { + u_int64_t rux_runtime; /* (cj) Real time. */ + u_int64_t rux_uticks; /* (cj) Statclock hits in user mode. */ + u_int64_t rux_sticks; /* (cj) Statclock hits in sys mode. */ + u_int64_t rux_iticks; /* (cj) Statclock hits in intr mode. */ + u_int64_t rux_uu; /* (c) Previous user time in usec. */ + u_int64_t rux_su; /* (c) Previous sys time in usec. */ + u_int64_t rux_tu; /* (c) Previous total time in usec. */ +}; + +/* * Kernel runnable context (thread). * This is what is put to sleep and reactivated. * Thread context. Processes may have multiple threads. @@ -219,7 +239,8 @@ struct thread { u_int td_estcpu; /* (t) estimated cpu utilization */ int td_slptick; /* (t) Time at sleep. */ int td_blktick; /* (t) Time spent blocked. */ - struct rusage td_ru; /* (t) rusage information */ + struct rusage td_ru; /* (t) rusage information. */ + struct rusage_ext td_rux; /* (t) Internal rusage information. */ uint64_t td_incruntime; /* (t) Cpu ticks to transfer to proc. */ uint64_t td_runtime; /* (t) How many cpu ticks we've run. */ u_int td_pticks; /* (t) Statclock hits for profiling */ @@ -426,26 +447,6 @@ do { \ #define TD_SET_CAN_RUN(td) (td)->td_state =3D TDS_CAN_RUN =20 /* - * XXX: Does this belong in resource.h or resourcevar.h instead? - * Resource usage extension. The times in rusage structs in the kernel are - * never up to date. The actual times are kept as runtimes and tick counts - * (with control info in the "previous" times), and are converted when - * userland asks for rusage info. Backwards compatibility prevents putting - * this directly in the user-visible rusage struct. - * - * Locking: (cj) means (j) for p_rux and (c) for p_crux. - */ -struct rusage_ext { - u_int64_t rux_runtime; /* (cj) Real time. */ - u_int64_t rux_uticks; /* (cj) Statclock hits in user mode. */ - u_int64_t rux_sticks; /* (cj) Statclock hits in sys mode. */ - u_int64_t rux_iticks; /* (cj) Statclock hits in intr mode. */ - u_int64_t rux_uu; /* (c) Previous user time in usec. */ - u_int64_t rux_su; /* (c) Previous sys time in usec. */ - u_int64_t rux_tu; /* (c) Previous total time in usec. */ -}; - -/* * Process structure. */ struct proc { diff --git a/sys/sys/resource.h b/sys/sys/resource.h index 9af96af..e703744 100644 --- a/sys/sys/resource.h +++ b/sys/sys/resource.h @@ -56,6 +56,7 @@ =20 #define RUSAGE_SELF 0 #define RUSAGE_CHILDREN -1 +#define RUSAGE_THREAD 1 =20 struct rusage { struct timeval ru_utime; /* user time used */ diff --git a/sys/sys/resourcevar.h b/sys/sys/resourcevar.h index 21728aa..95a9b49 100644 --- a/sys/sys/resourcevar.h +++ b/sys/sys/resourcevar.h @@ -131,7 +131,7 @@ 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); +void ruxagg_locked(struct rusage_ext *rux, struct thread *td); int suswintr(void *base, int word); struct uidinfo *uifind(uid_t uid); --xgyAXRrhYN0wYx8y Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (FreeBSD) iEYEARECAAYFAkvdn8oACgkQC3+MBN1Mb4hJbQCg0sO2vR/sh5TiY5QKuw1h+MZU pU4AoL2eFRvvw3cSKzap4nWcRdt6AMVk =4slr -----END PGP SIGNATURE----- --xgyAXRrhYN0wYx8y--