[patch] system_process_attributes for OpenBSD

[patch] system_process_attributes for OpenBSD

[Omar Polo]

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

Re: [patch] system_process_attributes for OpenBSD

[Lars Ingebrigtsen]

Omar Polo <op@omarpolo.com> writes:

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

Great!  Looks good to me, and I've tried the patch here on an OpenBSD
system, and it seems to work well.

> 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?

Sure; here's the rationale behind the assignments:

https://www.gnu.org/licenses/why-assign.en.html

And here's the form to get started:


Please email the following information to assign@gnu.org, and we
will send you the assignment form for your past and future changes.

Please use your full legal name (in ASCII characters) as the subject
line of the message.
----------------------------------------------------------------------
REQUEST: SEND FORM FOR PAST AND FUTURE CHANGES

[What is the name of the program or package you're contributing to?]
Emacs

[Did you copy any files or text written by someone else in these changes?
Even if that material is free software, we need to know about it.]

[Do you have an employer who might have a basis to claim to own
your changes?  Do you attend a school which might make such a claim?]

[For the copyright registration, what country are you a citizen of?]

[What year were you born?]

[Please write your email address here.]

[Please write your postal address here.]

[Which files have you changed so far, and which new files have you written
so far?]

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

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,

Why can't the sysctl be used to access process info? I was under
impression that using kvm directly is not recommended method and users
should be using sysctl to access the info?

Here's my old WIP diff adding the proced info, I never finished
debugging the attributes function but I didn't use kvm.

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

Re: [patch] system_process_attributes for OpenBSD

[Omar Polo]

Timo Myyrä <timo.myyra@bittivirhe.fi> writes:

> Omar Polo <op@omarpolo.com> [2021-01-01, 23:29 +0100]:
>
>[snip]
>
> Hi Omar,
>
> Are you sure you need kvm? I'm under impression its frowned upon to
> access that from userland.

Thanks for sharing!  I haven't followed closely the development of
OpenBSD, but at least on -CURRENT trying to get a struct kinfo_proc for
a specific pid via sysctl results in errors (both from sysctl(8) and
sysctl(3)):

    $ sysctl kern.proc
    sysctl: use ps to view kern.proc information
    $ sysctl kern.proc 75056  # a valid pid
    sysctl: use ps to view kern.proc information
    sysctl: top level name 75056 in 75056 is invalid

    $ cat <<EOF > pid.c
    #include <sys/types.h>
    #include <sys/sysctl.h>
    #include <err.h>
    #include <stdlib.h>

    int
    main(void)
    {
            pid_t pid = 75056; /* valid pid */
            int mib[4] = { CTL_KERN, KERN_PROC, KERN_PROC_PID, pid };
            struct kinfo_proc proc;
            size_t len = sizeof(proc);

            if (sysctl(mib, 4, &proc, &len, NULL, 0) != 0 || len == 0)
                    err(1, "sysctl");
            return 0;
    }
    EOF
    $ cc pid.c -o pid && ./pid
    pid: sysctl: Invalid argument
    $ ps 75056
    75056 ??  S        0:38.85 emacs --daemon (emacs-27.1)

This plus the fact that both top and ps in base uses kvm were the
rationale for my choice.

Regarding list_system_processes, I still have to check the differences,
but the current version on master is fundamentally equivalent to yours
(modulo a bunch of #ifdefs for freebsd and macos), so I guess it was
merged at some point?  Here on emacs-27.1 (list-system-processes)
returns nil, but on emacs compiled from master it works correctly.

Re: [patch] system_process_attributes for OpenBSD

[Timo Myyrä]

Omar Polo <op@omarpolo.com> [2021-01-02, 11:36 +0100]:

> Timo Myyrä <timo.myyra@bittivirhe.fi> writes:
>
>> Omar Polo <op@omarpolo.com> [2021-01-01, 23:29 +0100]:
>>
>>[snip]
>>
>> Hi Omar,
>>
>> Are you sure you need kvm? I'm under impression its frowned upon to
>> access that from userland.
>
> Thanks for sharing!  I haven't followed closely the development of
> OpenBSD, but at least on -CURRENT trying to get a struct kinfo_proc for
> a specific pid via sysctl results in errors (both from sysctl(8) and
> sysctl(3)):
>
>     $ sysctl kern.proc
>     sysctl: use ps to view kern.proc information
>     $ sysctl kern.proc 75056  # a valid pid
>     sysctl: use ps to view kern.proc information
>     sysctl: top level name 75056 in 75056 is invalid
>
>     $ cat <<EOF > pid.c
>     #include <sys/types.h>
>     #include <sys/sysctl.h>
>     #include <err.h>
>     #include <stdlib.h>
>
>     int
>     main(void)
>     {
>             pid_t pid = 75056; /* valid pid */
>             int mib[4] = { CTL_KERN, KERN_PROC, KERN_PROC_PID, pid };
>             struct kinfo_proc proc;
>             size_t len = sizeof(proc);
>
>             if (sysctl(mib, 4, &proc, &len, NULL, 0) != 0 || len == 0)
>                     err(1, "sysctl");
>             return 0;
>     }
>     EOF
>     $ cc pid.c -o pid && ./pid
>     pid: sysctl: Invalid argument
>     $ ps 75056
>     75056 ??  S        0:38.85 emacs --daemon (emacs-27.1)
>
> This plus the fact that both top and ps in base uses kvm were the
> rationale for my choice.
>
> Regarding list_system_processes, I still have to check the differences,
> but the current version on master is fundamentally equivalent to yours
> (modulo a bunch of #ifdefs for freebsd and macos), so I guess it was
> merged at some point?  Here on emacs-27.1 (list-system-processes)
> returns nil, but on emacs compiled from master it works correctly.

Yeah, forgot to finish this so its been a while. Seems that the process
listing is already in emacs so that can be discarded.

But I got following sample program working without using kvm.

#include <sys/types.h>
#include <sys/sysctl.h>
#include <err.h>
#include <stdlib.h>
#include <stdio.h>

int
main(int argc, char **argv) {
  if (argc < 2) {
    fprintf(stderr, "Must give PID number as an argument!\n");
    exit(1);
  }
  pid_t pid = atoi(argv[1]); /* valid pid */
  int mib[6] = { CTL_KERN, KERN_PROC, KERN_PROC_PID, pid, sizeof(struct kinfo_proc), 1};
  struct kinfo_proc proc;
  size_t len = sizeof(proc);

  if (sysctl(mib, 6, &proc, &len, NULL, 0) != 0)
    err(1, "sysctl");
  printf("pid belongs to command: %s\n", proc.p_comm);
  return 0;
}

I'd say above would be prefered as there would be no need to add more
libraries. I recall the sysctl(8) is intentionally a bit limited but the
sysctl(2) should give full access.

Timo

Re: [patch] system_process_attributes for OpenBSD

[Omar Polo]

Timo Myyrä <timo.myyra@bittivirhe.fi> writes:

> [snip]
>
> Yeah, forgot to finish this so its been a while. Seems that the process
> listing is already in emacs so that can be discarded.
>
> But I got following sample program working without using kvm.
>
> #include <sys/types.h>
> #include <sys/sysctl.h>
> #include <err.h>
> #include <stdlib.h>
> #include <stdio.h>
>
> int
> main(int argc, char **argv) {
>   if (argc < 2) {
>     fprintf(stderr, "Must give PID number as an argument!\n");
>     exit(1);
>   }
>   pid_t pid = atoi(argv[1]); /* valid pid */
>   int mib[6] = { CTL_KERN, KERN_PROC, KERN_PROC_PID, pid, sizeof(struct kinfo_proc), 1};
>   struct kinfo_proc proc;
>   size_t len = sizeof(proc);
>
>   if (sysctl(mib, 6, &proc, &len, NULL, 0) != 0)
>     err(1, "sysctl");
>   printf("pid belongs to command: %s\n", proc.p_comm);
>   return 0;
> }
>
> I'd say above would be prefered as there would be no need to add more
> libraries. I recall the sysctl(8) is intentionally a bit limited but the
> sysctl(2) should give full access.
>
> Timo

Yeah, you're right!  No wonders I got "invalid argument" from sysctl(2),
I was using the wrong mib!  I'll update the patch ditching kvm in favour
of sysctl.  Other than avoid linking to kvm, this allows me to drop that
ugly static variable for the kvm handler.

Thanks!

Omar Polo

Re: [patch] system_process_attributes for OpenBSD

[Omar Polo]

Second attempt.  Now with 100% less linkages and allocations!

Lars: thanks for testing and providing me the form.  I sent it and I'm
waiting the response.

To recap, in the original patch I used kvm(3), because I thought sysctl
wasn't an option.  Timo pointed out that sysctl actually provides those
info; I was only using the wrong mib.

The patch below uses sysctl(2) to get processes attributes.  There are
five stats that I don't know how to collect (cminflt, cmajflt, cstime,
ctime and thcount).  Everything else seem consistent with top(1).

This new patch is equivalent to the first (just a rename from proc->xxx
to proc.xxx), with the only exception being how args are concatenated.
Before, kvm managed the memory for us, but with sysctl we have to
provide the buffer by ourselves.  I avoided to dynamically allocate the
args string and instead reused the existing storage; this is similar to
the freebsd code, but if it seems ugly I can replace that with
calloc+strlcat like in the previous patch.

Cheers!

Omar Polo


diff 5ef30fbf11c3ee0ff1c30061dab554b9bd3ce841 /home/op/build/emacs
blob - 29c88f5308e47611e425f89a916eca578c86e09f
file + src/sysdep.c
--- src/sysdep.c
+++ src/sysdep.c
@@ -53,6 +53,10 @@ along with GNU Emacs.  If not, see <https://www.gnu.or
 # include <sys/sysctl.h>
 #endif
 
+#if defined __OpenBSD__
+# include <sys/proc.h>
+#endif
+
 #ifdef DARWIN_OS
 # include <libproc.h>
 #endif
@@ -2984,6 +2988,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
@@ -3673,6 +3685,189 @@ system_process_attributes (Lisp_Object pid)
   return attrs;
 }
 
+#elif defined __OpenBSD__
+
+Lisp_Object
+system_process_attributes (Lisp_Object pid)
+{
+  int proc_id, nentries, fscale, i;
+  int pagesize = getpagesize ();
+  int mib[6];
+  size_t len;
+  double pct;
+  char *ttyname, args[ARG_MAX];
+  struct kinfo_proc proc;
+  struct passwd *pw;
+  struct group *gr;
+  struct timespec t;
+  struct uvmexp uvmexp;
+
+  Lisp_Object attrs = Qnil;
+  Lisp_Object decoded_comm;
+
+  CHECK_NUMBER (pid);
+  CONS_TO_INTEGER (pid, int, proc_id);
+
+  len = sizeof proc;
+  mib[0] = CTL_KERN;
+  mib[1] = KERN_PROC;
+  mib[2] = KERN_PROC_PID;
+  mib[3] = proc_id;
+  mib[4] = len;
+  mib[5] = 1;
+  if (sysctl (mib, 6, &proc, &len, NULL, 0) != 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 SIDL:
+      state[0] = 'I';
+      break;
+    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);
+
+  len = sizeof (fscale);
+  mib[0] = CTL_KERN;
+  mib[1] = KERN_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);
+    }
+
+  len = sizeof (uvmexp);
+  mib[0] = CTL_VM;
+  mib[1] = VM_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);
+    }
+
+  len = sizeof args;
+  mib[0] = CTL_KERN;
+  mib[1] = KERN_PROC_ARGS;
+  mib[2] = proc_id;
+  mib[3] = KERN_PROC_ARGV;
+  if (sysctl (mib, 4, &args, &len, NULL, 0) == 0 && len != 0)
+    {
+      char **argv = (char**)args;
+
+      /* concatenate argv reusing the existing storage storage.
+	 sysctl(8) guarantees that "the buffer pointed to by oldp is
+	 filled with an array of char pointers followed by the strings
+	 themselves." */
+      for (i = 0; argv[i] != NULL; ++i)
+	{
+	  if (argv[i+1] != NULL)
+	    {
+	      len = strlen (argv[i]);
+	      argv[i][len] = ' ';
+	    }
+	}
+
+      AUTO_STRING (comm, *argv);
+      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

Re: [patch] system_process_attributes for OpenBSD

[Lars Ingebrigtsen]

Omar Polo <op@omarpolo.com> writes:

> Second attempt.  Now with 100% less linkages and allocations!
>
> Lars: thanks for testing and providing me the form.  I sent it and I'm
> waiting the response.

I haven't tested this version, but skimming the patch, it looks good to
me.  We'll be notified when the paperwork goes through, but to ensure
that this gets applied, could you remind us, too?  You can also use
`M-x submit-emacs-patch' in Emacs 28 to put it in the issue tracker;
that should ensure that it's not lost.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: [patch] system_process_attributes for OpenBSD

[Omar Polo]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Omar Polo <op@omarpolo.com> writes:
>
>> Second attempt.  Now with 100% less linkages and allocations!
>>
>> Lars: thanks for testing and providing me the form.  I sent it and I'm
>> waiting the response.
>
> I haven't tested this version, but skimming the patch, it looks good to
> me.  We'll be notified when the paperwork goes through, but to ensure
> that this gets applied, could you remind us, too?  You can also use
> `M-x submit-emacs-patch' in Emacs 28 to put it in the issue tracker;
> that should ensure that it's not lost.

I didn't knew about submit-emacs-patch.  I'll do that once the request
gets applied.

Thanks!