Rework getrusage(2) infrastructure

- the userland call is now made to PM only, and PM relays the call to
  other servers as appropriate; this is an ABI change that will
  ultimately allow us to add proper support for wait3() and the like;
  for the moment there is backward compatibility;
- the getrusage-specific kernel subcall has been removed, as it
  provided only redundant functionality, and did not provide the means
  to be extended correctly in the future - namely, allowing the kernel
  to return different values depending on whether resource usage of
  the caller (self) or its children was requested;
- VM is now told whether resource usage of the caller (self) or its
  children is requested, and it refrains from filling in wrong values
  for information it does not have;
- VM now uses the correct unit for the ru_maxrss values;
- VFS is cut out of the loop entirely, since it does not provide any
  values at the moment; a comment explains how it should be readded.

Change-Id: I27b0f488437dec3d8e784721c67b03f2f853120f

etc/system.conf

@@ -61,6 +61,7 @@ service pm
 		EXEC_NEWMEM	# 03
 		WILLEXIT	# 05
 		NOTIFY_SIG	# 39
+		GETRUSAGE	# 47
 		;
 	io	NONE;		# No I/O range allowed
 	irq	NONE;		# No IRQ allowed

minix/commands/service/parse.c

@@ -754,6 +754,7 @@ struct
 	{ "CLEARCACHE",		VM_CLEARCACHE },
 	{ "VFS_MMAP",		VM_VFS_MMAP },
 	{ "VFS_REPLY",		VM_VFS_REPLY },
+	{ "GETRUSAGE",		VM_GETRUSAGE },
 	{ "RS_PREPARE",		VM_RS_PREPARE },
 	{ NULL,			0 },
 };

minix/include/minix/callnr.h

@@ -111,7 +111,7 @@
 #define VFS_GETVFSSTAT		(VFS_BASE + 39)
 #define VFS_STATVFS1 	 	(VFS_BASE + 40)
 #define VFS_FSTATVFS1		(VFS_BASE + 41)
-#define VFS_GETRUSAGE		(VFS_BASE + 42)
+#define VFS_GETRUSAGE		(VFS_BASE + 42)		/* obsolete */
 #define VFS_SVRCTL		(VFS_BASE + 43)
 #define VFS_GCOV_FLUSH		(VFS_BASE + 44)
 #define VFS_MAPDRIVER		(VFS_BASE + 45)

minix/include/minix/com.h

@@ -332,7 +332,6 @@
 #   define GET_IDLETSC	  21	/* get cumulative idle time stamp counter */
 #   define GET_CPUINFO    23    /* get information about cpus */
 #   define GET_REGS	  24	/* get general process registers */
-#   define GET_RUSAGE	  25	/* get resource usage */
 
 /* Subfunctions for SYS_PRIVCTL */
 #define SYS_PRIV_ALLOW		1	/* Allow process to run */
@@ -765,7 +764,7 @@
 /* Basic vm calls allowed to every process. */
 #define VM_BASIC_CALLS \
     VM_BRK, VM_MMAP, VM_MUNMAP, VM_MAP_PHYS, VM_UNMAP_PHYS, VM_INFO, \
-    VM_GETRUSAGE
+    VM_GETRUSAGE /* VM_GETRUSAGE is to be removed from this list ASAP */
 
 /*===========================================================================*
  *                Messages for IPC server				     *

minix/include/minix/ipc.h

@@ -774,13 +774,6 @@ typedef struct {
 } mess_lc_vfs_readwrite;
 _ASSERT_MSG_SIZE(mess_lc_vfs_readwrite);
 
-typedef struct {
-	vir_bytes addr;
-
-	uint8_t padding[52];
-} mess_lc_vfs_rusage;
-_ASSERT_MSG_SIZE(mess_lc_vfs_rusage);
-
 typedef struct {
 	uint32_t nfds;
 	fd_set *readfds;
@@ -854,13 +847,6 @@ typedef struct {
 } mess_lc_vm_getphys;
 _ASSERT_MSG_SIZE(mess_lc_vm_getphys);
 
-typedef struct {
-	vir_bytes addr;
-
-	uint8_t padding[52];
-} mess_lc_vm_rusage;
-_ASSERT_MSG_SIZE(mess_lc_vm_rusage);
-
 typedef struct {
 	endpoint_t	forwhom;
 	void		*addr;
@@ -1368,6 +1354,15 @@ typedef struct {
 } mess_lsys_vm_query_exit;
 _ASSERT_MSG_SIZE(mess_lsys_vm_query_exit);
 
+typedef struct {
+	endpoint_t	endpt;
+	vir_bytes	addr;
+	int		children;
+
+	uint8_t		padding[44];
+} mess_lsys_vm_rusage;
+_ASSERT_MSG_SIZE(mess_lsys_vm_rusage);
+
 typedef struct {
 	endpoint_t	ep;
 	void		*vaddr;
@@ -2117,7 +2112,6 @@ typedef struct noxfer_message {
 		mess_lc_vfs_pipe2	m_lc_vfs_pipe2;
 		mess_lc_vfs_readlink	m_lc_vfs_readlink;
 		mess_lc_vfs_readwrite	m_lc_vfs_readwrite;
-		mess_lc_vfs_rusage	m_lc_vfs_rusage;
 		mess_lc_vfs_select	m_lc_vfs_select;
 		mess_lc_vfs_stat	m_lc_vfs_stat;
 		mess_lc_vfs_statvfs1	m_lc_vfs_statvfs1;
@@ -2126,7 +2120,6 @@ typedef struct noxfer_message {
 		mess_lc_vfs_umount	m_lc_vfs_umount;
 		mess_lc_vm_brk		m_lc_vm_brk;
 		mess_lc_vm_getphys	m_lc_vm_getphys;
-		mess_lc_vm_rusage	m_lc_vm_rusage;
 		mess_lc_vm_shm_unmap	m_lc_vm_shm_unmap;
 		mess_lchardriver_vfs_reply m_lchardriver_vfs_reply;
 		mess_lchardriver_vfs_sel1 m_lchardriver_vfs_sel1;
@@ -2183,6 +2176,7 @@ typedef struct noxfer_message {
 		mess_lsys_vm_info	m_lsys_vm_info;
 		mess_lsys_vm_map_phys	m_lsys_vm_map_phys;
 		mess_lsys_vm_query_exit	m_lsys_vm_query_exit;
+		mess_lsys_vm_rusage	m_lsys_vm_rusage;
 		mess_lsys_vm_unmap_phys	m_lsys_vm_unmap_phys;
 		mess_lsys_vm_update	m_lsys_vm_update;
 		mess_lsys_vm_vmremap	m_lsys_vm_vmremap;

minix/include/minix/syslib.h

@@ -190,7 +190,6 @@ int sys_diagctl(int ctl, char *arg1, int arg2);
 #define sys_getpriv(dst, nr)	sys_getinfo(GET_PRIV, dst, 0,0, nr)
 #define sys_getidletsc(dst)	sys_getinfo(GET_IDLETSC, dst, 0,0,0)
 #define sys_getregs(dst,nr)	sys_getinfo(GET_REGS, dst, 0,0, nr)
-#define sys_getrusage(dst, nr)  sys_getinfo(GET_RUSAGE, dst, 0,0, nr)
 int sys_getinfo(int request, void *val_ptr, int val_len, void *val_ptr2,
 	int val_len2);
 int sys_whoami(endpoint_t *ep, char *name, int namelen, int

minix/include/minix/vm.h

@@ -8,6 +8,7 @@
 
 int vm_exit(endpoint_t ep);
 int vm_fork(endpoint_t ep, int slotno, endpoint_t *child_ep);
+int vm_getrusage(endpoint_t endpt, void *addr, int children);
 int vm_willexit(endpoint_t ep);
 int vm_adddma(endpoint_t proc_e, phys_bytes start, phys_bytes size);
 int vm_deldma(endpoint_t proc_e, phys_bytes start, phys_bytes size);

minix/kernel/system/do_getinfo.c

@@ -189,31 +189,6 @@ int do_getinfo(struct proc * caller, message * m_ptr)
         src_vir = (vir_bytes) &idl->p_cycles;
         break;
     }
-    case GET_RUSAGE: {
-	struct proc *target = NULL;
-	int target_slot = 0;
-	u64_t usec;
-        nr_e = (m_ptr->m_lsys_krn_sys_getinfo.val_len2_e == SELF) ?
-            caller->p_endpoint : m_ptr->m_lsys_krn_sys_getinfo.val_len2_e;
-
-	if (!isokendpt(nr_e, &target_slot))
-		return EINVAL;
-
-	target = proc_addr(target_slot);
-	if (isemptyp(target))
-		return EINVAL;
-
-	length = sizeof(r_usage);
-	memset(&r_usage, 0, sizeof(r_usage));
-	usec = target->p_user_time * 1000000 / system_hz;
-	r_usage.ru_utime.tv_sec = usec / 1000000;
-	r_usage.ru_utime.tv_usec = usec % 1000000;
-	usec = target->p_sys_time * 1000000 / system_hz;
-	r_usage.ru_stime.tv_sec = usec / 1000000;
-	r_usage.ru_stime.tv_usec = usec % 1000000;
-	src_vir = (vir_bytes) &r_usage;
-	break;
-    }
     default:
 	printf("do_getinfo: invalid request %d\n",
 		m_ptr->m_lsys_krn_sys_getinfo.request);

minix/lib/libc/sys/getrusage.c

@@ -6,34 +6,14 @@
 #include <unistd.h>
 #include <sys/resource.h>
 
-int getrusage(int who, struct rusage *r_usage)
+int
+getrusage(int who, struct rusage * r_usage)
 {
-	int rc;
 	message m;
 
 	memset(&m, 0, sizeof(m));
 	m.m_lc_pm_rusage.who = who;
 	m.m_lc_pm_rusage.addr = (vir_bytes)r_usage;
 
-	if (r_usage == NULL) {
+	return _syscall(PM_PROC_NR, PM_GETRUSAGE, &m);
-		errno = EFAULT;
-		return -1;
-	}
-	if (who != RUSAGE_SELF && who != RUSAGE_CHILDREN) {
-		errno = EINVAL;
-		return -1;
-	}
-
-	memset(r_usage, 0, sizeof(struct rusage));
-	if ((rc = _syscall(PM_PROC_NR, PM_GETRUSAGE, &m)) < 0)
-		return rc;
-
-	memset(&m, 0, sizeof(m));
-	m.m_lc_vfs_rusage.addr = (vir_bytes)r_usage;
-	if ((rc = _syscall(VFS_PROC_NR, VFS_GETRUSAGE, &m)) < 0)
-		return rc;
-
-	memset(&m, 0, sizeof(m));
-	m.m_lc_vm_rusage.addr = (vir_bytes)r_usage;
-	return _syscall(VM_PROC_NR, VM_GETRUSAGE, &m);
 }

minix/lib/libsys/Makefile

@@ -94,6 +94,7 @@ SRCS+=  \
 	vm_cache.c \
 	vm_exit.c \
 	vm_fork.c \
+	vm_getrusage.c \
 	vm_info.c \
 	vm_map_phys.c \
 	vm_memctl.c \

minix/lib/libsys/vm_getrusage.c

@@ -0,0 +1,17 @@
+
+#include "syslib.h"
+#include <string.h>
+#include <minix/vm.h>
+
+int
+vm_getrusage(endpoint_t endpt, void * addr, int children)
+{
+	message m;
+
+	memset(&m, 0, sizeof(m));
+	m.m_lsys_vm_rusage.endpt = endpt;
+	m.m_lsys_vm_rusage.addr = (vir_bytes)addr;
+	m.m_lsys_vm_rusage.children = children;
+
+	return _taskcall(VM_PROC_NR, VM_GETRUSAGE, &m);
+}

minix/servers/pm/misc.c

@@ -7,6 +7,7 @@
  *   do_getepinfo: get the pid/uid/gid of a process given its endpoint
  *   do_getsetpriority: get/set process priority
  *   do_svrctl: process manager control
+ *   do_getrusage: obtain process resource usage information
  */
 
 #include "pm.h"
@@ -404,28 +405,57 @@ int do_svrctl(void)
 /*===========================================================================*
  *				do_getrusage				     *
  *===========================================================================*/
-int do_getrusage()
+int
+do_getrusage(void)
 {
-	int res = 0;
+	clock_t user_time, sys_time;
-	clock_t user_time = 0;
-	clock_t sys_time = 0;
 	struct rusage r_usage;
 	u64_t usec;
+	int r, children;
+
 	if (m_in.m_lc_pm_rusage.who != RUSAGE_SELF &&
 	    m_in.m_lc_pm_rusage.who != RUSAGE_CHILDREN)
 		return EINVAL;
-	if ((res = sys_getrusage(&r_usage, who_e)) < 0)
-		return res;
 
-	if (m_in.m_lc_pm_rusage.who == RUSAGE_CHILDREN) {
+	/*
-		usec = mp->mp_child_utime * 1000000 / sys_hz();
+	 * TODO: first relay the call to VFS.  As is, VFS does not have any
-		r_usage.ru_utime.tv_sec = usec / 1000000;
+	 * fields it can fill with meaningful values, but this may change in
-		r_usage.ru_utime.tv_usec = usec % 1000000;
+	 * the future.  In that case, PM would first have to use the tell_vfs()
-		usec = mp->mp_child_stime * 1000000 / sys_hz();
+	 * system to get those values from VFS, and do the rest here upon
-		r_usage.ru_stime.tv_sec = usec / 1000000;
+	 * getting the response.
-		r_usage.ru_stime.tv_usec = usec % 1000000;
+	 */
+
+	memset(&r_usage, 0, sizeof(r_usage));
+
+	children = (m_in.m_lc_pm_rusage.who == RUSAGE_CHILDREN);
+
+	/*
+	 * Get system times.  For RUSAGE_SELF, get the times for the calling
+	 * process from the kernel.  For RUSAGE_CHILDREN, we already have the
+	 * values we should return right here.
+	 */
+	if (!children) {
+		if ((r = sys_times(who_e, &user_time, &sys_time, NULL,
+		    NULL)) != OK)
+			return r;
+	} else {
+		user_time = mp->mp_child_utime;
+		sys_time = mp->mp_child_stime;
 	}
 
+	/* In both cases, convert from clock ticks to microseconds. */
+	usec = user_time * 1000000 / sys_hz();
+	r_usage.ru_utime.tv_sec = usec / 1000000;
+	r_usage.ru_utime.tv_usec = usec % 1000000;
+	usec = sys_time * 1000000 / sys_hz();
+	r_usage.ru_stime.tv_sec = usec / 1000000;
+	r_usage.ru_stime.tv_usec = usec % 1000000;
+
+	/* Get additional fields from VM. */
+	if ((r = vm_getrusage(who_e, &r_usage, children)) != OK)
+		return r;
+
+	/* Finally copy the structure to the caller. */
 	return sys_datacopy(SELF, (vir_bytes)&r_usage, who_e,
-		m_in.m_lc_pm_rusage.addr, (vir_bytes) sizeof(r_usage));
+	    m_in.m_lc_pm_rusage.addr, (vir_bytes)sizeof(r_usage));
 }

minix/servers/vfs/misc.c

@@ -966,16 +966,10 @@ void panic_hook(void)
  *===========================================================================*/
 int do_getrusage(void)
 {
-	int res;
+	/* Obsolete vfs_getrusage(2) call from userland. The getrusage call is
-	struct rusage r_usage;
+	 * now fully handled by PM, and for any future fields that should be
-
+	 * supplied by VFS, VFS should be queried by PM rather than by the user
-	if ((res = sys_datacopy_wrapper(who_e, m_in.m_lc_vfs_rusage.addr, SELF,
+	 * program directly.  TODO: remove this call after the next release.
-		(vir_bytes) &r_usage, (vir_bytes) sizeof(r_usage))) < 0)
+	 */
-		return res;
+	return OK;
-
-	r_usage.ru_inblock = 0;
-	r_usage.ru_oublock = 0;
-
-	return sys_datacopy_wrapper(SELF, (vir_bytes) &r_usage, who_e,
-		m_in.m_lc_vfs_rusage.addr, (phys_bytes) sizeof(r_usage));
 }

minix/servers/vfs/table.c

@@ -57,7 +57,7 @@ int (* const call_vec[NR_VFS_CALLS])(void) = {
 	CALL(VFS_GETVFSSTAT)	= do_getvfsstat,	/* getvfsstat(2) */
 	CALL(VFS_STATVFS1)	= do_statvfs,		/* statvfs(2) */
 	CALL(VFS_FSTATVFS1)	= do_fstatvfs,		/* fstatvfs(2) */
-	CALL(VFS_GETRUSAGE)	= do_getrusage,		/* getrusage(2) */
+	CALL(VFS_GETRUSAGE)	= do_getrusage,		/* (obsolete) */
 	CALL(VFS_SVRCTL)	= do_svrctl,		/* svrctl(2) */
 	CALL(VFS_GCOV_FLUSH)	= do_gcov_flush,	/* gcov_flush(2) */
 	CALL(VFS_MAPDRIVER)	= do_mapdriver,		/* mapdriver(2) */

minix/servers/vm/utility.c

@@ -429,21 +429,47 @@ int do_getrusage(message *m)
 	int res, slot;
 	struct vmproc *vmp;
 	struct rusage r_usage;
-	if ((res = vm_isokendpt(m->m_source, &slot)) != OK)
+
+	/* If the request is not from PM, it is coming directly from userland.
+	 * This is an obsolete construction. In the future, userland programs
+	 * should no longer be allowed to call vm_getrusage(2) directly at all.
+	 * For backward compatibility, we simply return success for now.
+	 */
+	if (m->m_source != PM_PROC_NR)
+		return OK;
+
+	/* Get the process for which resource usage is requested. */
+	if ((res = vm_isokendpt(m->m_lsys_vm_rusage.endpt, &slot)) != OK)
 		return ESRCH;
 
 	vmp = &vmproc[slot];
 
-	if ((res = sys_datacopy(m->m_source, m->m_lc_vm_rusage.addr,
+	/* We are going to change only a few fields, so copy in the rusage
+	 * structure first. The structure is still in PM's address space at
+	 * this point, so use the message source.
+	 */
+	if ((res = sys_datacopy(m->m_source, m->m_lsys_vm_rusage.addr,
 		SELF, (vir_bytes) &r_usage, (vir_bytes) sizeof(r_usage))) < 0)
 		return res;
 
-	r_usage.ru_maxrss = vmp->vm_total_max;
+	if (!m->m_lsys_vm_rusage.children) {
+		r_usage.ru_maxrss = vmp->vm_total_max / 1024L; /* unit is KB */
 		r_usage.ru_minflt = vmp->vm_minor_page_fault;
 		r_usage.ru_majflt = vmp->vm_major_page_fault;
+	} else {
+		/* XXX TODO: return the fields for terminated, waited-for
+		 * children of the given process. We currently do not have this
+		 * information! In the future, rather than teaching VM about
+		 * the process hierarchy, PM should probably tell VM at process
+		 * exit time which other process should inherit its resource
+		 * usage fields. For now, we assume PM clears the fields before
+		 * making this call, so we don't zero the fields explicitly.
+		 */
+	}
 
+	/* Copy out the resulting structure back to PM. */
 	return sys_datacopy(SELF, (vir_bytes) &r_usage, m->m_source,
-		m->m_lc_vm_rusage.addr, (vir_bytes) sizeof(r_usage));
+		m->m_lsys_vm_rusage.addr, (vir_bytes) sizeof(r_usage));
 }
 
 /*===========================================================================*

minix/tests/test75.c

@@ -72,7 +72,6 @@ main(int argc, char *argv[])
 		e(1);
 		exit(1);
 	}
-	CHECK_NOT_ZERO_FIELD(r_usage2, ru_maxrss);
 	if ((child = fork()) == 0) {
 		/*
 		 * We cannot do this part of the test in the parent, since
@@ -100,7 +99,6 @@ main(int argc, char *argv[])
 			exit(1);
 		}
 		CHECK_NOT_ZERO_FIELD(r_usage3, ru_utime.tv_sec);
-		CHECK_NOT_ZERO_FIELD(r_usage3, ru_maxrss);
 	}
 	quit();
 

minix/usr.bin/trace/service/pm.c

@@ -1240,6 +1240,12 @@ pm_getrusage_in(struct trace_proc * proc, const message * m_out,
 		put_struct_timeval(proc, "ru_stime", PF_LOCADDR,
 		    (vir_bytes)&buf.ru_stime);
 
+		if (verbose > 0) {
+			put_value(proc, "ru_maxrss", "%ld", buf.ru_maxrss);
+			put_value(proc, "ru_minflt", "%ld", buf.ru_minflt);
+			put_value(proc, "ru_majflt", "%ld", buf.ru_majflt);
+		}
+
 		put_close_struct(proc, verbose > 0);
 	}
 	put_equals(proc);

minix/usr.bin/trace/service/vfs.c

@@ -1327,35 +1327,6 @@ vfs_fstatvfs1_out(struct trace_proc * proc, const message * m_out)
 	return CT_NOTDONE;
 }
 
-static int
-vfs_getrusage_out(struct trace_proc * __unused proc,
-	const message * __unused m_out)
-{
-
-	return CT_NOTDONE;
-}
-
-static void
-vfs_getrusage_in(struct trace_proc * proc, const message * m_out,
-	const message * __unused m_in, int failed)
-{
-	struct rusage buf;
-
-	/* Inline; we will certainly not be reusing this anywhere else. */
-	if (put_open_struct(proc, "rusage", failed,
-	    m_out->m_lc_vfs_rusage.addr, &buf, sizeof(buf))) {
-		/* Reason for hiding these two better: they're always zero. */
-		if (verbose > 1) {
-			put_value(proc, "ru_inblock", "%ld", buf.ru_inblock);
-			put_value(proc, "ru_oublock", "%ld", buf.ru_oublock);
-		}
-
-		put_close_struct(proc, verbose > 1);
-	}
-	put_equals(proc);
-	put_result(proc);
-}
-
 static int
 vfs_svrctl_out(struct trace_proc * proc, const message * m_out)
 {
@@ -1437,8 +1408,6 @@ static const struct call_handler vfs_map[] = {
 	    vfs_statvfs1_in),
 	VFS_CALL(FSTATVFS1) = HANDLER("fstatvfs1", vfs_fstatvfs1_out,
 	    vfs_statvfs1_in),
-	VFS_CALL(GETRUSAGE) = HANDLER("vfs_getrusage", vfs_getrusage_out,
-	    vfs_getrusage_in),
 	VFS_CALL(SVRCTL) = HANDLER("vfs_svrctl", vfs_svrctl_out,
 	    vfs_svrctl_in),
 	VFS_CALL(GCOV_FLUSH) = HANDLER("gcov_flush", vfs_gcov_flush_out,

minix/usr.bin/trace/service/vm.c

@@ -88,43 +88,12 @@ vm_munmap_out(struct trace_proc * proc, const message * m_out)
 	return CT_DONE;
 }
 
-static int
-vm_getrusage_out(struct trace_proc * __unused proc,
-	const message * __unused m_out)
-{
-
-	return CT_NOTDONE;
-}
-
-static void
-vm_getrusage_in(struct trace_proc * proc, const message * m_out,
-	const message * __unused m_in, int failed)
-{
-	struct rusage buf;
-
-	/* Inline; we will certainly not be reusing this anywhere else. */
-	if (put_open_struct(proc, "rusage", failed,
-	    m_out->m_lc_vm_rusage.addr, &buf, sizeof(buf))) {
-		if (verbose > 0) {
-			put_value(proc, "ru_maxrss", "%ld", buf.ru_maxrss);
-			put_value(proc, "ru_minflt", "%ld", buf.ru_minflt);
-			put_value(proc, "ru_majflt", "%ld", buf.ru_majflt);
-		}
-
-		put_close_struct(proc, verbose > 0);
-	}
-	put_equals(proc);
-	put_result(proc);
-}
-
 #define VM_CALL(c) [((VM_ ## c) - VM_RQ_BASE)]
 
 static const struct call_handler vm_map[] = {
 	VM_CALL(BRK) = HANDLER("brk", vm_brk_out, default_in),
 	VM_CALL(MMAP) = HANDLER("mmap", vm_mmap_out, vm_mmap_in),
 	VM_CALL(MUNMAP) = HANDLER("munmap", vm_munmap_out, default_in),
-	VM_CALL(GETRUSAGE) = HANDLER("vm_getrusage", vm_getrusage_out,
-	    vm_getrusage_in),
 };
 
 const struct calls vm_calls = {