Re: [patch] system_process_attributes for OpenBSD

[Timo Myyrä]

Omar Polo <op@omarpolo.com> [2021-01-01, 23:29 +0100]:

> Hello,
>
> I wanted to try proced, but it doesn't work OOTB on OpenBSD.  On emacs
> 27.1 (installed from packages) list-system-processes returns nil, but on
> emacs built from master correctly returns the list of
> pids. process-attributes unfortunately returns nil in all the cases.
>
> The attached patch (against the master branch) adds an implementation of
> system_process_attributes for OpenBSD.  First time hacking on emacs, I
> tried to follow the style but please excuse if I forgot something :)
>
> It's not possible to access processes information via sysctl, one has to
> use kvm.  I studied the sources of top and ps while working on this
> patch, and I followed what those programs do.  kvm needs to be
> initialised with kvm_open [0]: the downside is that a program shouldn't
> call two or more time kvm_open (see the BUGS section in the linked
> manpage); so I had to use that ugly hack with a static variable: there
> is a more elegant way to handle something like this?
>
> There are five stats that I'm not able to collect (cminflt, cmajflt,
> cstime, ctime, thcount), but otherwise process-attributes and proced
> seem to work just fine:
>
>     (process-attributes 66968)
>     ;; =>
>     ((args . "emacs --daemon")
>      (pmem . 0.9343402932966782)
>      (pcpu . 2.587890625)
>      (etime 0 41450 331172 251000)
>      (rss . 141820)
>      (vsize . 113460)
>      (start 24558 64860 215303 0)
>      (nice . 20)
>      (pri . 24)
>      (cutime 0 201 920000 0)
>      (time 0 376 790000 0)
>      (stime 0 36 190000 0)
>      (utime 0 340 600000 0)
>      (majflt . 5690)
>      (minflt . 3903171)
>      (tpgid . -1)
>      (sess . 66968)
>      (pgrp . 1000)
>      (ppid . 1)
>      (state . "S")
>      (comm . "emacs-27.1")
>      (group . "op")
>      (egid . 1000)
>      (user . "op")
>      (euid . 1000))
>
> I'm not sure I got the `state' flag correct, where are they documented?
>
> One last thing, I don't have a copyright assignment and I know very
> little about them.  I believe I have to sign one if this patch a chance
> to be accepted.  Can someone please fill me in on the details?
>
> Thanks,
> Omar Polo
>
> [0]: http://man.openbsd.org/kvm_open
> [1]: 
> http://cvsweb.openbsd.org/cgi-bin/cvsweb/~checkout~/src/sys/sys/sysctl.h?rev=1.213&content-type=text/plain
>
> diff --git a/configure.ac b/configure.ac
> index bf768441fe..6e8d4a54e5 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1574,6 +1574,8 @@ AC_DEFUN
>  
>    hpux*) LIBS_SYSTEM="-l:libdld.sl" ;;
>  
> +  openbsd) LIBS_SYSTEM="-lkvm" ;;
> +
>    qnxnto) LIBS_SYSTEM="-lsocket" ;;
>  
>    solaris) LIBS_SYSTEM="-lsocket -lnsl" ;;
> diff --git a/src/sysdep.c b/src/sysdep.c
> index eeb9d18494..f1e99c0b8d 100644
> --- a/src/sysdep.c
> +++ b/src/sysdep.c
> @@ -53,6 +53,11 @@
>  # include <sys/sysctl.h>
>  #endif
>  
> +#if defined __OpenBSD__
> +# include <kvm.h>
> +# include <sys/proc.h>
> +#endif
> +
>  #ifdef DARWIN_OS
>  # include <libproc.h>
>  #endif
> @@ -2972,6 +2977,14 @@ make_lisp_timeval (struct timeval t)
>    return make_lisp_time (timeval_to_timespec (t));
>  }
>  
> +#elif defined __OpenBSD__
> +
> +static Lisp_Object
> +make_lisp_timeval (long sec, long usec)
> +{
> +  return make_lisp_time(make_timespec(sec, usec * 1000));
> +}
> +
>  #endif
>  
>  #ifdef GNU_LINUX
> @@ -3661,6 +3674,188 @@ system_process_attributes (Lisp_Object pid)
>    return attrs;
>  }
>  
> +#elif defined __OpenBSD__
> +
> +Lisp_Object
> +system_process_attributes (Lisp_Object pid)
> +{
> +  static kvm_t *kd = NULL;
> +
> +  int proc_id, nentries, fscale, i;
> +  int pagesize = getpagesize ();
> +  int mib[2];
> +  size_t len;
> +  double pct;
> +  char *ttyname, **argv, *args;
> +  struct kinfo_proc *proc;
> +  struct passwd *pw;
> +  struct group *gr;
> +  struct timespec t;
> +  struct uvmexp uvmexp;
> +
> +  Lisp_Object attrs = Qnil;
> +  Lisp_Object decoded_comm, decoded_args;
> +
> +  CHECK_NUMBER (pid);
> +  CONS_TO_INTEGER (pid, int, proc_id);
> +
> +  /* hack */
> +  if (kd == NULL)
> +    {
> +      kd = kvm_open(NULL, NULL, NULL, KVM_NO_FILES, NULL);
> +      if (kd == NULL)
> +     return attrs;
> +    }
> +
> +  proc = kvm_getprocs(kd, KERN_PROC_PID, proc_id, sizeof(*proc), &nentries);
> +  if (proc == NULL)
> +    return attrs;
> +
> +  attrs = Fcons (Fcons (Qeuid, INT_TO_INTEGER (proc->p_uid)), attrs);
> +
> +  block_input ();
> +  pw = getpwuid (proc->p_uid);
> +  unblock_input ();
> +  if (pw)
> +    attrs = Fcons (Fcons (Quser, build_string(pw->pw_name)), attrs);
> +
> +  attrs = Fcons (Fcons (Qegid, INT_TO_INTEGER(proc->p_svgid)), attrs);
> +
> +  block_input ();
> +  gr = getgrgid (proc->p_svgid);
> +  unblock_input ();
> +  if (gr)
> +    attrs = Fcons (Fcons (Qgroup, build_string (gr->gr_name)), attrs);
> +
> +  AUTO_STRING (comm, proc->p_comm);
> +  decoded_comm = code_convert_string_norecord (comm, Vlocale_coding_system, 
> 0);
> +  attrs = Fcons (Fcons (Qcomm, decoded_comm), attrs);
> +
> +  {
> +    char state[2] = {'\0', '\0'};
> +    switch (proc->p_stat) {
> +    case SRUN:
> +      state[0] = 'R';
> +      break;
> +    case SSLEEP:
> +      state[0] = 'S';
> +      break;
> +    case SSTOP:
> +      state[0] = 'T';
> +      break;
> +    case SZOMB:
> +      state[0] = 'Z';
> +      break;
> +    case SDEAD:
> +      state[0] = 'D';
> +      break;
> +    }
> +    attrs = Fcons (Fcons (Qstate, build_string (state)), attrs);
> +  }
> +
> +  attrs = Fcons (Fcons (Qppid, INT_TO_INTEGER (proc->p_ppid)), attrs);
> +  attrs = Fcons (Fcons (Qpgrp, INT_TO_INTEGER (proc->p_gid)), attrs);
> +  attrs = Fcons (Fcons (Qsess, INT_TO_INTEGER (proc->p_sid)),  attrs);
> +
> +  block_input ();
> +  ttyname = proc->p_tdev == NODEV ? NULL : devname (proc->p_tdev, S_IFCHR);
> +  unblock_input ();
> +  if (ttyname)
> +    attrs = Fcons (Fcons (Qttname, build_string (ttyname)), attrs);
> +
> +  attrs = Fcons (Fcons (Qtpgid,   INT_TO_INTEGER (proc->p_tpgid)), attrs);
> +  attrs = Fcons (Fcons (Qminflt,  INT_TO_INTEGER (proc->p_uru_minflt)),
> +              attrs);
> +  attrs = Fcons (Fcons (Qmajflt,  INT_TO_INTEGER (proc->p_uru_majflt)),
> +              attrs);
> +
> +  /* FIXME: missing cminflt, cmajflt. */
> +
> +  attrs = Fcons (Fcons (Qutime, make_lisp_timeval (proc->p_uutime_sec,
> +                                                proc->p_uutime_usec)),
> +              attrs);
> +  attrs = Fcons (Fcons (Qstime, make_lisp_timeval (proc->p_ustime_sec,
> +                                                proc->p_ustime_usec)),
> +              attrs);
> +  t = timespec_add (make_timespec (proc->p_uutime_sec,
> +                                proc->p_uutime_usec * 1000),
> +                 make_timespec (proc->p_ustime_sec,
> +                                proc->p_ustime_usec * 1000));
> +  attrs = Fcons (Fcons (Qtime, make_lisp_time (t)), attrs);
> +
> +  attrs = Fcons (Fcons (Qcutime, make_lisp_timeval (proc->p_uctime_sec,
> +                                                 proc->p_uctime_usec)),
> +              attrs);
> +
> +  /* FIXME: missing cstime and thus ctime. */
> +
> +  attrs = Fcons (Fcons (Qpri,   make_fixnum (proc->p_priority)), attrs);
> +  attrs = Fcons (Fcons (Qnice,  make_fixnum (proc->p_nice)), attrs);
> +
> +  /* FIXME: missing thcount (thread count) */
> +
> +  attrs = Fcons (Fcons (Qstart, make_lisp_timeval (proc->p_ustart_sec,
> +                                                proc->p_ustart_usec)),
> +              attrs);
> +
> +  len = (proc->p_vm_tsize + proc->p_vm_dsize + proc->p_vm_ssize) * pagesize 
> >> 10;
> +  attrs = Fcons (Fcons (Qvsize, make_fixnum (len)), attrs);
> +
> +  attrs = Fcons (Fcons (Qrss,   make_fixnum (proc->p_vm_rssize * pagesize >> 
> 10)),
> +              attrs);
> +
> +  t = make_timespec (proc->p_ustart_sec,
> +                  proc->p_ustart_usec * 1000);
> +  t = timespec_sub (current_timespec (), t);
> +  attrs = Fcons (Fcons (Qetime, make_lisp_time (t)), attrs);
> +
> +  mib[0] = CTL_KERN;
> +  mib[1] = KERN_FSCALE;
> +  len = sizeof (fscale);
> +  if (sysctl (mib, 2, &fscale, &len, NULL, 0) != -1)
> +    {
> +      pct = (double)proc->p_pctcpu / fscale * 100.0;
> +      attrs = Fcons (Fcons (Qpcpu, make_float (pct)), attrs);
> +    }
> +
> +  mib[0] = CTL_VM;
> +  mib[1] = VM_UVMEXP;
> +  len = sizeof (uvmexp);
> +  if (sysctl (mib, 2, &uvmexp, &len, NULL, 0) != -1)
> +    {
> +      pct = (100.0 * (double)proc->p_vm_rssize / uvmexp.npages);
> +      attrs = Fcons (Fcons (Qpmem, make_float (pct)), attrs);
> +    }
> +
> +  /* concatenate process argv */
> +
> +  if ((argv = kvm_getargv(kd, proc, 0)) == NULL)
> +    return attrs;
> +
> +  len = 0;
> +  for (i = 0; argv[i] != NULL; ++i)
> +    len += strlen(argv[i]) + 1;
> +
> +  if ((args = calloc(1, len)) == NULL)
> +    return attrs;
> +
> +  for (i = 0; argv[i] != NULL; ++i)
> +    {
> +      strlcat(args, argv[i], len);
> +      if (argv[i+1] != NULL)
> +     strlcat(args, " ", len);
> +    }
> +
> +  AUTO_STRING (args_str, args);
> +  decoded_args = code_convert_string_norecord (args_str,
> +                                            Vlocale_coding_system, 0);
> +  attrs = Fcons (Fcons (Qargs, decoded_args), attrs);
> +
> +  free(args);
> +
> +  return attrs;
> +}
> +
>  #elif defined DARWIN_OS
>  
>  Lisp_Object


Hi Omar,

Are you sure you need kvm? I'm under impression its frowned upon to
access that from userland.

I tried once to fix proced as well but didn't finish the status function
and this has been bit rotting on my disk since. I don't recall what state
this is but this uses sysctl to get the pid list so it could be merged
with your diff to avoid kvm use?

Timo

diff --git a/src/sysdep.c b/src/sysdep.c
index eeb9d18494..16fa4e6f69 100644
--- a/src/sysdep.c
+++ b/src/sysdep.c
@@ -82,6 +82,15 @@ #define STDERR_FILENO fileno(GetStdHandle(STD_ERROR_HANDLE))
 #endif /* WINDOWSNT */
 
 #include <sys/types.h>
+#if defined DARWIN_OS || defined __FreeBSD__ || defined __OpenBSD__
+# include <sys/sysctl.h>
+#endif
+
+#ifdef __OpenBSD__
+# include <math.h>
+# include <sys/proc.h>
+#endif
+
 #include <sys/stat.h>
 #include <errno.h>
 
@@ -2946,6 +2955,42 @@ list_system_processes (void)
   return  proclist;
 }
 
+#elif defined __OpenBSD__
+
+Lisp_Object
+list_system_processes (void)
+{
+  int mib[] = {CTL_KERN, KERN_PROC, 0, 0, sizeof (struct kinfo_proc), 0};
+  size_t len;
+  struct kinfo_proc *procs;
+  size_t i;
+
+  Lisp_Object proclist = Qnil;
+
+  if (sysctl (mib, 6, NULL, &len, NULL, 0) == -1)
+    {
+      return proclist;
+    }
+
+  procs = xmalloc (len);
+  mib[5] = (int)(len = sizeof (struct kinfo_proc));
+  if (sysctl (mib, 6, procs, &len, NULL, 0) == -1)
+    {
+      xfree (procs);
+      return proclist;
+    }
+
+  len /= sizeof (struct kinfo_proc);
+  for (i = 0; i < len; i++)
+    {
+      proclist = Fcons (INT_TO_INTEGER (procs[i].p_pid), proclist);
+    }
+
+  xfree (procs);
+
+  return  proclist;
+}
+
 /* The WINDOWSNT implementation is in w32.c.
    The MSDOS implementation is in dosfns.c.  */
 #elif !defined (WINDOWSNT) && !defined (MSDOS)
@@ -2959,7 +3004,7 @@ list_system_processes (void)
 #endif /* !defined (WINDOWSNT) */
 
 
-#if defined __FreeBSD__ || defined DARWIN_OS
+#if defined __FreeBSD__ || defined DARWIN_OS || defined __OpenBSD__
 
 static struct timespec
 timeval_to_timespec (struct timeval t)
@@ -3661,6 +3706,170 @@ system_process_attributes (Lisp_Object pid)
   return attrs;
 }
 
+#elif defined __OpenBSD__
+
+Lisp_Object
+system_process_attributes (Lisp_Object pid)
+{
+  int proc_id;
+  int pagesize;
+  unsigned long npages;
+  int fscale;
+  struct passwd *pw;
+  struct group  *gr;
+  char *ttyname;
+  size_t len;
+  char args[MAXPATHLEN];
+  struct timespec t, now;
+
+  int mib[4] = {CTL_KERN, KERN_PROC, KERN_PROC_PID};
+  struct kinfo_proc proc;
+  size_t proclen = sizeof proc;
+
+  Lisp_Object attrs = Qnil;
+  Lisp_Object decoded_comm;
+
+  CHECK_NUMBER (pid);
+  CONS_TO_INTEGER (pid, int, proc_id);
+  mib[3] = proc_id;
+
+  if (sysctl (mib, 4, &proc, &proclen, NULL, 0) != 0 || proclen == 0)
+    return attrs;
+
+  attrs = Fcons (Fcons (Qeuid, INT_TO_INTEGER (proc.p_uid)), attrs);
+
+  block_input ();
+  pw = getpwuid (proc.p_uid);
+  unblock_input ();
+  if (pw)
+    attrs = Fcons (Fcons (Quser, build_string (pw->pw_name)), attrs);
+
+  attrs = Fcons (Fcons (Qegid, INT_TO_INTEGER (proc.p_svgid)), attrs);
+
+  block_input ();
+  gr = getgrgid (proc.p_svgid);
+  unblock_input ();
+  if (gr)
+    attrs = Fcons (Fcons (Qgroup, build_string (gr->gr_name)), attrs);
+
+  AUTO_STRING (comm, proc.p_comm);
+  decoded_comm = code_convert_string_norecord (comm, Vlocale_coding_system, 0);
+
+  attrs = Fcons (Fcons (Qcomm, decoded_comm), attrs);
+  {
+    char state[2] = {'\0', '\0'};
+    switch (proc.p_stat)
+      {
+      case SRUN:
+       state[0] = 'R';
+       break;
+
+      case SSLEEP:
+       state[0] = 'S';
+       break;
+
+      case SZOMB:
+       state[0] = 'Z';
+       break;
+
+      case SSTOP:
+       state[0] = 'T';
+       break;
+
+      case SIDL:
+       state[0] = 'I';
+       break;
+      }
+    attrs = Fcons (Fcons (Qstate, build_string (state)), attrs);
+  }
+
+  attrs = Fcons (Fcons (Qppid, INT_TO_INTEGER (proc.p_ppid)), attrs);
+  attrs = Fcons (Fcons (Qpgrp, INT_TO_INTEGER (proc.p__pgid)), attrs);
+  attrs = Fcons (Fcons (Qsess, INT_TO_INTEGER (proc.p_sid)),  attrs);
+
+  block_input ();
+  ttyname = proc.p_tdev == NODEV ? NULL : devname (proc.p_tdev, S_IFCHR);
+  unblock_input ();
+  if (ttyname)
+    attrs = Fcons (Fcons (Qtty, build_string (ttyname)), attrs);
+
+  attrs = Fcons (Fcons (Qtpgid,   INT_TO_INTEGER (proc.p_tpgid)), attrs);
+
+  attrs = Fcons (Fcons (Qminflt, INT_TO_INTEGER (proc.p_uru_minflt)), attrs);
+  attrs = Fcons (Fcons (Qmajflt, INT_TO_INTEGER (proc.p_uru_majflt)), attrs);
+
+  struct timespec p_uutime, p_ustime, p_start;
+  p_uutime.tv_sec = proc.p_uutime_sec;
+  p_uutime.tv_nsec = proc.p_uutime_usec * 1000;
+  p_ustime.tv_sec = proc.p_ustime_sec;
+  p_ustime.tv_nsec = proc.p_ustime_usec * 1000;
+  p_start.tv_sec = proc.p_ustart_sec;
+  p_start.tv_nsec = proc.p_ustart_usec * 1000;
+  attrs = Fcons (Fcons (Qutime, make_lisp_timeval (make_timeval (p_uutime))), 
attrs);
+  attrs = Fcons (Fcons (Qstime, make_lisp_timeval (make_timeval (p_ustime))), 
attrs);
+  t = timespec_add (p_uutime, p_ustime);
+  attrs = Fcons (Fcons (Qtime, make_lisp_time (t)), attrs);
+
+  attrs = Fcons (Fcons (Qpri,   make_fixnum (proc.p_priority)), attrs);
+  attrs = Fcons (Fcons (Qnice,  make_fixnum (proc.p_nice)), attrs);
+  // XXX: was ki_start,
+  attrs = Fcons (Fcons (Qstart, make_lisp_timeval (make_timeval(p_start))), 
attrs);
+  attrs = Fcons (Fcons (Qvsize, make_fixnum (proc.p_vm_map_size >> 10)), 
attrs);
+
+  pagesize = sysconf (_SC_PAGESIZE);
+  if (pagesize == -1)
+      return attrs;
+
+  attrs = Fcons (Fcons (Qrss,   make_fixnum (proc.p_vm_rssize * pagesize >> 
10)), attrs);
+
+  now = current_timespec ();
+  t = timespec_sub (now, p_start);
+  attrs = Fcons (Fcons (Qetime, make_lisp_time (t)), attrs);
+
+  /* XXX: OpenBSD swtime is not used?
+  len = sizeof fscale;
+  int kern_mib = {CTL_KERN, KERN_FSCALE};
+  if (sysctl (kern_mib, 2, &fscale, &len, NULL, 0) == 0)
+    {
+      double pcpu;
+      fixpt_t ccpu;
+      len = sizeof ccpu;
+      if (sysctlbyname ("kern.ccpu", &ccpu, &len, NULL, 0) == 0)
+       {
+          // XXX: p_swtime always zero on OpenBSD
+         pcpu = (100.0 * proc.p_pctcpu / fscale
+                 / (1 - exp (proc.p_swtime * log ((double) ccpu / fscale))));
+         attrs = Fcons (Fcons (Qpcpu, INT_TO_INTEGER (pcpu)), attrs);
+       }
+    }
+  */
+
+  double pmem = (proc.p_flag & PS_INEXEC
+                 ? 100.0 * proc.p_vm_rssize / proc.p_rlim_rss_cur
+                 : 0);
+  attrs = Fcons (Fcons (Qpmem, INT_TO_INTEGER (pmem)), attrs);
+
+  mib[2] = KERN_PROC_ARGS;
+  len = MAXPATHLEN;
+  if (sysctl (mib, 4, args, &len, NULL, 0) == 0 && len != 0)
+    {
+      int i;
+      for (i = 0; i < len; i++)
+       {
+         if (! args[i] && i < len - 1)
+           args[i] = ' ';
+       }
+
+      AUTO_STRING (comm, args);
+      decoded_comm = code_convert_string_norecord (comm,
+                                                  Vlocale_coding_system, 0);
+
+      attrs = Fcons (Fcons (Qargs, decoded_comm), attrs);
+    }
+
+  return attrs;
+}
+
 #elif defined DARWIN_OS
 
 Lisp_Object