Date: Sun, 2 May 2010 15:48:50 +0200 From: Attilio Rao <attilio@freebsd.org> To: Kostik Belousov <kostikbel@gmail.com> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r207468 - head/sys/kern Message-ID: <AANLkTilBLCn6cPqLn2MdBrri0Yg9STRskT9csa4123-Z@mail.gmail.com> In-Reply-To: <20100501151339.GZ2391@deviant.kiev.zoral.com.ua> References: <201005011446.o41EkIa6051907@svn.freebsd.org> <AANLkTin7fGpDpQPfAp0o599b4wc6KpJw_a3qwC4taWbG@mail.gmail.com> <20100501151339.GZ2391@deviant.kiev.zoral.com.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
2010/5/1 Kostik Belousov <kostikbel@gmail.com>: > On Sat, May 01, 2010 at 04:47:36PM +0200, Attilio Rao wrote: >> 2010/5/1 Konstantin Belousov <kib@freebsd.org>: >> > Author: kib >> > Date: Sat May =C2=A01 14:46:17 2010 >> > New Revision: 207468 >> > URL: http://svn.freebsd.org/changeset/base/207468 >> > >> > Log: >> > =C2=A0Extract thread_lock()/ruxagg()/thread_unlock() fragment into uti= lity >> > =C2=A0function ruxagg_tlock(). >> > =C2=A0Convert 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(). Yes, this is exactly what I wanted to happen. > 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 ? 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. > 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 @@ > =C2=A0.\" =C2=A0 =C2=A0 @(#)getrusage.2 =C2=A0 =C2=A0 =C2=A0 =C2=A08.1 (B= erkeley) 6/4/93 > =C2=A0.\" $FreeBSD$ > =C2=A0.\" > -.Dd June 4, 1993 > +.Dd May 1, 2010 > =C2=A0.Dt GETRUSAGE 2 > =C2=A0.Os > =C2=A0.Sh NAME > @@ -42,6 +42,7 @@ > =C2=A0.In sys/resource.h > =C2=A0.Fd "#define =C2=A0 RUSAGE_SELF =C2=A0 =C2=A0 =C2=A00" > =C2=A0.Fd "#define =C2=A0 RUSAGE_CHILDREN -1" > +.Fd "#define =C2=A0 RUSAGE_THREAD =C2=A0 1" > =C2=A0.Ft int > =C2=A0.Fn getrusage "int who" "struct rusage *rusage" > =C2=A0.Sh DESCRIPTION > @@ -49,11 +50,12 @@ The > =C2=A0.Fn getrusage > =C2=A0system call > =C2=A0returns information describing the resources utilized by the curren= t > -process, or all its terminated child processes. > +thread, the current process, or all its terminated child processes. > =C2=A0The > =C2=A0.Fa who > =C2=A0argument is either > -.Dv RUSAGE_SELF > +.Dv RUSAGE_THREAD , > +.Dv RUSAGE_SELF , > =C2=A0or > =C2=A0.Dv RUSAGE_CHILDREN . > =C2=A0The buffer to which > @@ -175,6 +177,10 @@ The > =C2=A0.Fn getrusage > =C2=A0system call appeared in > =C2=A0.Bx 4.2 . > +The > +.Dv RUSAGE_THREAD > +facility first appeared in > +.Fx 8.1 . > =C2=A0.Sh BUGS > =C2=A0There is no way to obtain information about a child process > =C2=A0that has not yet terminated. > diff --git a/sys/kern/kern_resource.c b/sys/kern/kern_resource.c > index a3ed75d..8e7fdb6 100644 > --- a/sys/kern/kern_resource.c > +++ b/sys/kern/kern_resource.c > @@ -921,6 +921,31 @@ calcru1(struct proc *p, struct rusage_ext *ruxp, str= uct timeval *up, > =C2=A0 =C2=A0 =C2=A0 =C2=A0sp->tv_usec =3D su % 1000000; > =C2=A0} > > +static void > +calctru(struct thread *td) > +{ > + =C2=A0 =C2=A0 =C2=A0 /* {user, system, interrupt, total} {ticks, usec}:= */ > + =C2=A0 =C2=A0 =C2=A0 u_int64_t ut, uu, st, su, it, tt, tu; > + > + =C2=A0 =C2=A0 =C2=A0 tu =3D cputick2usec(td->td_incruntime); > + =C2=A0 =C2=A0 =C2=A0 ut =3D td->td_uticks; > + =C2=A0 =C2=A0 =C2=A0 it =3D td->td_iticks; > + =C2=A0 =C2=A0 =C2=A0 st =3D td->td_sticks; > + > + =C2=A0 =C2=A0 =C2=A0 tt =3D ut + st + it; > + =C2=A0 =C2=A0 =C2=A0 if (tt =3D=3D 0) { > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Avoid divide by zer= o */ > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 st =3D 1; > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 tt =3D 1; > + =C2=A0 =C2=A0 =C2=A0 } > + =C2=A0 =C2=A0 =C2=A0 uu =3D td->td_ru.ru_utime.tv_usec + (ut * tu) / tt= ; > + =C2=A0 =C2=A0 =C2=A0 su =3D td->td_ru.ru_stime.tv_usec + (st * tu) / tt= ; > + =C2=A0 =C2=A0 =C2=A0 td->td_ru.ru_utime.tv_sec +=3D uu / 1000000; > + =C2=A0 =C2=A0 =C2=A0 td->td_ru.ru_utime.tv_usec =3D uu % 1000000; > + =C2=A0 =C2=A0 =C2=A0 td->td_ru.ru_stime.tv_sec +=3D su / 1000000; > + =C2=A0 =C2=A0 =C2=A0 td->td_ru.ru_stime.tv_usec =3D su % 1000000; > +} > + > =C2=A0#ifndef _SYS_SYSPROTO_H_ > =C2=A0struct getrusage_args { > =C2=A0 =C2=A0 =C2=A0 =C2=A0int =C2=A0 =C2=A0 who; The comment on the top of calctru() might be removed. > @@ -961,6 +986,13 @@ kern_getrusage(struct thread *td, int who, struct ru= sage *rup) > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0calccru(p, &rup->r= u_utime, &rup->ru_stime); > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0break; > > + =C2=A0 =C2=A0 =C2=A0 case RUSAGE_THREAD: > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 PROC_SLOCK(p); > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ruxagg_tlock(p, td); > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 PROC_SUNLOCK(p); > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 *rup =3D td->td_ru; > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 break; > + > =C2=A0 =C2=A0 =C2=A0 =C2=A0default: > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0error =3D EINVAL; > =C2=A0 =C2=A0 =C2=A0 =C2=A0} > @@ -1010,6 +1042,12 @@ ruxagg(struct rusage_ext *rux, struct thread *td) > =C2=A0 =C2=A0 =C2=A0 =C2=A0rux->rux_uticks +=3D td->td_uticks; > =C2=A0 =C2=A0 =C2=A0 =C2=A0rux->rux_sticks +=3D td->td_sticks; > =C2=A0 =C2=A0 =C2=A0 =C2=A0rux->rux_iticks +=3D td->td_iticks; > + > + =C2=A0 =C2=A0 =C2=A0 /* > + =C2=A0 =C2=A0 =C2=A0 =C2=A0* Update thread rusage before ticks counters= cleaning. > + =C2=A0 =C2=A0 =C2=A0 =C2=A0*/ > + =C2=A0 =C2=A0 =C2=A0 calctru(td); > + > =C2=A0 =C2=A0 =C2=A0 =C2=A0td->td_incruntime =3D 0; > =C2=A0 =C2=A0 =C2=A0 =C2=A0td->td_uticks =3D 0; > =C2=A0 =C2=A0 =C2=A0 =C2=A0td->td_iticks =3D 0; The comment might be single-line and the extra white line after calctru() is not due. Thanks, Attilio --=20 Peace can only be achieved by understanding - A. Einstein
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTilBLCn6cPqLn2MdBrri0Yg9STRskT9csa4123-Z>