Index | Thread | Search

From:
Martin Pieuchot <mpi@grenadille.net>
Subject:
Re: getrusage() memory stats
To:
David Gwynne <david@gwynne.id.au>
Cc:
tech@openbsd.org
Date:
Fri, 6 Dec 2024 12:42:05 +0100

Download raw body.

Thread
On 06/12/24(Fri) 16:13, David Gwynne wrote:
> 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.

I like the move.

If we change the value to be non-integral we also have to adapt
acct_process().

I'd also like to be sure that ports are ready or being taken care of.

I wonder if we should talk about "physical pages" rather than "resident
set".  I also find the use of "residing" ambiguous if we use "resident"
for actual physical pages in use.  What about "belonging" or "allocated
as part of"?

> 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).

the amount of physical memory utilized

>  .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 */

I wouldn't put an XXX here.  Let's simply document that it's nowadays
more useful to aggregate in userland.

> +	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? */

I believe we want `vm_dsize' that's what fork and the core dump
machinery look at.

> +	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:
>