NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

re: kern/45539: add support for getrusage(2) memory size statistics



> >Synopsis:       add support for getrusage(2) memory size statistics

thanks for looking at this.  i've wished it worked for ages.

this is from statclock():

> +     /*
> +      * If the CPU is currently scheduled to a non-idle process, then charge
> +      * that process with the appropriate VM resource utilization for a tick.
> +      *
> +      * Assume that the current process has been running the entire last
> +      * tick, and account for VM use regardless of whether in user mode or
> +      * system mode (XXX or interrupt mode?).
> +      *
> +      * rusage VM stats are expressed in kilobytes * ticks-of-execution.
> +      */
> +     /* based on code from 4.3BSD kern_clock.c and from FreeBSD */
> +
> +# define pg2kb(n)    (((n) * PAGE_SIZE) / 1024)
> +
> +     if (p != NULL) {
> +             struct vmspace *vm = p->p_vmspace;
> +             long rss;
> +
> +             /*
> +              * XXX is p_stmutex currently enough to protect updates to
> +              * p_stats too?
> +              */
> +             p->p_stats->p_ru.ru_idrss += pg2kb(vm->vm_dsize); /* unshared 
> data */
> +             p->p_stats->p_ru.ru_isrss += pg2kb(vm->vm_ssize); /* unshared 
> stack */
> +             p->p_stats->p_ru.ru_ixrss += pg2kb(vm->vm_tsize); /* "shared" 
> text? */

this is the wrong way to get these values.  the += will
accumulate over time, 'hz' times a second, depend on the target.
this is probably why you have wrong values.

i don't think statclock() is the right place to do it.  i would
simply grab them when filling in the rusage as normal.  (but this
makes me question what these values actually mean.  the member
names are "ru_i?rss".  is the "rss" supposed to imply "resident
set size" here, to?  if so, i don't know that we have any way of
getting that info.)

> +             rss = pg2kb(vm_resident_count(vm));
> +             if (rss > p->p_stats->p_ru.ru_maxrss)
> +                     p->p_stats->p_ru.ru_maxrss = rss;
> +     }

initially, i didn't like the look of this one either.
vm_resident_count() can be expensive (but i don't think it is
on any current netbsd ports) but i'm not sure that pushing the
cost of it anywhere else would help.  eg, pushing it into lwp
switch would end up being more expensive to maintain on a system
with very active but switching lwp's.


.mrg.


Home | Main Index | Thread Index | Old Index