On Fri, Dec 06, 2024 at 02:42:14PM +0100, Mark Kettenis wrote:
> > Date: Fri, 6 Dec 2024 16:13:59 +1000
> > From: David Gwynne <david@gwynne.id.au>
> > 
> > this repurposes the memory usage fields in struct rusage to give the
> > current memory usage info when using getrusage(RUSAGE_SELF). this
> > is the same as what we can see in tools like top.
> > 
> > currently these fields are 0. they're documented as being max and some
> > integral values i dont understand. having the current values is better
> > than nothing.
> > 
> > im already using getrusage to collect metrics in some home grown
> > software, having current memory usage nicely complements the cpu
> > usage stats. some of these things run out of memory, so understanding
> > the trend over time will be useful.
> > 
> > fwiw, netbsd also uses the current values to populate these fields, but
> > didnt update their doco.
> > 
> > ok?
> 
> I don't think this is a good idea.  The getrusage(2) interface has
> never been about capturing "current" values, but always about
> "accumulated" values.

Not sure about that:

RUSAGE_SELF      Resources used by the current process.

RUSAGE_THREAD    Resources used by the current thread.

This is all about the current process / thread. So I think returning
values similar to the ones reported via sysctl / kvm_getprocs is something
we should aim for. At least in these two cases.

RUSAGE_CHILDREN on the other hand is all about "accumulated" values.
 
> I guess maxrss is a bit of an odd one out here but the maximum is
> still a value that is determined over the entire run.  I guess at
> least here the "current" value can be seen as an approximation of the
> true maximum.  But the maximum would be much more interesting as that
> really is the relevant thing to measure to determine how much memory
> you really need for your workload.  Is there a reason why you can't
> use sysctl to measure the "current" values?

I also expressed my worry about changing the meaning of ru_maxrss.
I think that should indeed always return the maximum rss seen and not
a current value.
 
> The 4.3BSD definition of struct rusage has spread widely to other
> OSes.  Changing its interpretation just on OpenBSD isn't going to help
> people trying to write portable code.

Right now ru_ixrss, ru_idrss and ru_isrss are always 0 on OpenBSD. So I
think there is room for improvement even for portable code.
 
> > Index: lib/libc/sys/getrusage.2
> > ===================================================================
> > RCS file: /cvs/src/lib/libc/sys/getrusage.2,v
> > diff -u -p -r1.18 getrusage.2
> > --- lib/libc/sys/getrusage.2	17 Jul 2024 13:29:05 -0000	1.18
> > +++ lib/libc/sys/getrusage.2	6 Dec 2024 05:21:51 -0000
> > @@ -64,10 +64,10 @@ the following structure:
> >  struct rusage {
> >          struct timeval ru_utime; /* user time used */
> >          struct timeval ru_stime; /* system time used */
> > -        long ru_maxrss;          /* max resident set size */
> > -        long ru_ixrss;           /* integral shared text memory size */
> > -        long ru_idrss;           /* integral unshared data size */
> > -        long ru_isrss;           /* integral unshared stack size */
> > +        long ru_maxrss;          /* resident set size */
> > +        long ru_ixrss;           /* shared text memory size */
> > +        long ru_idrss;           /* unshared data size */
> > +        long ru_isrss;           /* unshared stack size */
> >          long ru_minflt;          /* page reclaims */
> >          long ru_majflt;          /* page faults */
> >          long ru_nswap;           /* swaps */
> > @@ -89,22 +89,18 @@ the total amount of time spent executing
> >  the total amount of time spent in the system executing on behalf
> >  of the process(es).
> >  .It Fa ru_maxrss
> > -the maximum resident set size utilized (in kilobytes).
> > +the resident set size utilized (in kilobytes).
> >  .It Fa ru_ixrss
> > -an
> > -.Dq integral
> > -value indicating the amount of memory used
> > +a value indicating the amount of memory used
> >  by the text segment
> >  that was also shared among other processes.
> > -This value is expressed in units of kilobytes * ticks-of-execution.
> > +This value is expressed in units of kilobytes.
> >  .It Fa ru_idrss
> > -an integral value of the amount of unshared memory residing in the
> > -data segment of a process (expressed in units of
> > -kilobytes * ticks-of-execution).
> > +a value of the amount of unshared memory residing in the
> > +data segment of a process (expressed in units of kilobytes).
> >  .It Fa ru_isrss
> > -an integral value of the amount of unshared memory residing in the
> > -stack segment of a process (expressed in units of
> > -kilobytes * ticks-of-execution).
> > +a value of the amount of unshared memory residing in the
> > +stack segment of a process (expressed in units of kilobytes).
> >  .It Fa ru_minflt
> >  the number of page faults serviced without any I/O activity; here
> >  I/O activity is avoided by
> > Index: sys/sys/resource.h
> > ===================================================================
> > RCS file: /cvs/src/sys/sys/resource.h,v
> > diff -u -p -r1.14 resource.h
> > --- sys/sys/resource.h	25 Oct 2013 04:42:48 -0000	1.14
> > +++ sys/sys/resource.h	6 Dec 2024 05:21:51 -0000
> > @@ -58,11 +58,11 @@
> >  struct	rusage {
> >  	struct timeval ru_utime;	/* user time used */
> >  	struct timeval ru_stime;	/* system time used */
> > -	long	ru_maxrss;		/* max resident set size */
> > +	long	ru_maxrss;		/* resident set size */
> >  #define	ru_first	ru_ixrss
> > -	long	ru_ixrss;		/* integral shared text memory size */
> > -	long	ru_idrss;		/* integral unshared data " */
> > -	long	ru_isrss;		/* integral unshared stack " */
> > +	long	ru_ixrss;		/* shared text memory size */
> > +	long	ru_idrss;		/* unshared data " */
> > +	long	ru_isrss;		/* unshared stack " */
> >  	long	ru_minflt;		/* page reclaims */
> >  	long	ru_majflt;		/* page faults */
> >  	long	ru_nswap;		/* swaps */
> > Index: sys/kern/kern_resource.c
> > ===================================================================
> > RCS file: /cvs/src/sys/kern/kern_resource.c,v
> > diff -u -p -r1.93 kern_resource.c
> > --- sys/kern/kern_resource.c	10 Nov 2024 06:45:36 -0000	1.93
> > +++ sys/kern/kern_resource.c	6 Dec 2024 05:21:51 -0000
> > @@ -542,6 +542,18 @@ sys_getrusage(struct proc *p, void *v, r
> >  	return (error);
> >  }
> >  
> > +static void
> > +ruspace(struct rusage *rup, struct proc *p)
> > +{
> > +	struct vmspace *vm = p->p_vmspace;
> > +
> > +	/* XXX these are all current values, not max or integrals */
> > +	rup->ru_maxrss = vm_resident_count(vm) << (PAGE_SHIFT - 10);
> > +	rup->ru_ixrss = vm->vm_tsize << (PAGE_SHIFT - 10);
> > +	rup->ru_idrss = vm->vm_dused << (PAGE_SHIFT - 10); /* vm_dsize too? */
> > +	rup->ru_isrss = vm->vm_ssize << (PAGE_SHIFT - 10);
> > +}
> > +
> >  int
> >  dogetrusage(struct proc *p, int who, struct rusage *rup)
> >  {
> > @@ -567,6 +579,7 @@ dogetrusage(struct proc *p, int who, str
> >  		}
> >  
> >  		calcru(&tu, &rup->ru_utime, &rup->ru_stime, NULL);
> > +		ruspace(rup, p);
> >  		break;
> >  
> >  	case RUSAGE_THREAD:
> > 
> > 
> 

-- 
:wq Claudio