Fix nice handling for FreeBSD

The nice handling on FreeBSD was broken for multiple reasons: - Idletime and Realtime scheduled processes were not properly detected and insane values were reported back - There was no indicator for processes that were scheduled in idletime or realtime - A sentinel for unknown process niceness has been added such that an indication is shown when we were unable to compute the value. This now adds a platform specific field for the scheduling type which one can add to interpret the niceness of a process better. Signed-off-by: Nico Sonack <[email protected]>
htop-dev · Aug 1, 2024 · 11bf6a7 · 11bf6a7
1 parent 8edd449
commit 11bf6a7
Show file tree

Hide file tree

Showing 6 changed files with 86 additions and 12 deletions.
diff --git a/Process.c b/Process.c
@@ -649,10 +649,15 @@ void Process_writeField(const Process* this, RichString* str, RowField field) {
    case M_RESIDENT: Row_printKBytes(str, this->m_resident, coloring); return;
    case M_VIRT: Row_printKBytes(str, this->m_virt, coloring); return;
    case NICE:
-      xSnprintf(buffer, n, "%3ld ", this->nice);
-      attr = this->nice < 0 ? CRT_colors[PROCESS_HIGH_PRIORITY]
-         : this->nice > 0 ? CRT_colors[PROCESS_LOW_PRIORITY]
-         : CRT_colors[PROCESS_SHADOW];
+      if (this->nice == PROCESS_NICE_UNKNOWN) {
+         xSnprintf(buffer, n, "N/A ");
+         attr = CRT_colors[PROCESS_SHADOW];
+      } else {
+         xSnprintf(buffer, n, "%3ld ", this->nice);
+         attr = this->nice < 0 ? CRT_colors[PROCESS_HIGH_PRIORITY]
+            : this->nice > 0 ? CRT_colors[PROCESS_LOW_PRIORITY]
+            : CRT_colors[PROCESS_SHADOW];
+      }
       break;
    case NLWP:
       if (this->nlwp == 1)

diff --git a/Process.h b/Process.h
@@ -8,6 +8,7 @@ Released under the GNU GPLv2+, see the COPYING file
 in the source distribution for its full text.
 */
 
+#include <limits.h>
 #include <stdbool.h>
 #include <stdint.h>
 #include <sys/types.h>
@@ -24,6 +25,9 @@ in the source distribution for its full text.
 
 #define DEFAULT_HIGHLIGHT_SECS 5
 
+/* Sentinel value for an unknown niceness in Process.nice */
+#define PROCESS_NICE_UNKNOWN (-LONG_MAX)
+
 typedef enum Tristate_ {
    TRI_INITIAL = 0,
    TRI_OFF = -1,

diff --git a/freebsd/FreeBSDProcess.c b/freebsd/FreeBSDProcess.c
@@ -55,6 +55,7 @@ const ProcessFieldData Process_fields[LAST_PROCESSFIELD] = {
 #endif
    [JID] = { .name = "JID", .title = "JID", .description = "Jail prison ID", .flags = 0, .pidColumn = true, },
    [JAIL] = { .name = "JAIL", .title = "JAIL        ", .description = "Jail prison name", .flags = 0, },
+   [SCHEDCLASS] = { .name = "SCHEDCLASS", .title = "SC", .description = "Scheduling Class (Timesharing, Realtime, Idletime)", .flags = 0, },
    [EMULATION] = { .name = "EMULATION", .title = "EMULATION        ", .description = "System call emulation environment (ABI)", .flags = 0, },
 };
 
@@ -73,22 +74,41 @@ void Process_delete(Object* cast) {
    free(this);
 }
 
+static const char FreeBSD_schedclassChars[MAX_SCHEDCLASS] = {
+   [SCHEDCLASS_UNKNOWN] = '?',     // Something went wrong or the base system has a new scheduling class
+   [SCHEDCLASS_INTR_THREAD] = '-', // interrupt thread, these have special handling of priority
+   [SCHEDCLASS_IDLE] = 'i',        // idletime scheduling
+   [SCHEDCLASS_TIMESHARE] = ' ',   // timesharing process scheduling (regular processes are timeshared)
+   [SCHEDCLASS_REALTIME] = 'r',    // realtime scheduling
+};
+
 static void FreeBSDProcess_rowWriteField(const Row* super, RichString* str, ProcessField field) {
    const FreeBSDProcess* fp = (const FreeBSDProcess*) super;
 
    char buffer[256]; buffer[255] = '\0';
+   char sched_class;
    int attr = CRT_colors[DEFAULT_COLOR];
    size_t n = sizeof(buffer) - 1;
 
    switch (field) {
    // add FreeBSD-specific fields here
    case JID: xSnprintf(buffer, n, "%*d ", Process_pidDigits, fp->jid); break;
+
    case JAIL:
       Row_printLeftAlignedField(str, attr, fp->jname ? fp->jname : "N/A", 11);
       return;
+
    case EMULATION:
       Row_printLeftAlignedField(str, attr, fp->emul ? fp->emul : "N/A", 16);
       return;
+
+   case SCHEDCLASS:
+      assert(0 <= fp->sched_class && fp->sched_class < ARRAYSIZE(FreeBSD_schedclassChars));
+      sched_class = FreeBSD_schedclassChars[fp->sched_class];
+      assert(sched_class);
+      xSnprintf(buffer, n, " %c", sched_class);
+      break;
+
    default:
       Process_writeField(&fp->super, str, field);
       return;
@@ -109,6 +129,8 @@ static int FreeBSDProcess_compareByKey(const Process* v1, const Process* v2, Pro
       return SPACESHIP_NULLSTR(p1->jname, p2->jname);
    case EMULATION:
       return SPACESHIP_NULLSTR(p1->emul, p2->emul);
+   case SCHEDCLASS:
+      return SPACESHIP_NUMBER(p1->sched_class, p2->sched_class);
    default:
       return Process_compareByKey_Base(v1, v2, key);
    }

diff --git a/freebsd/FreeBSDProcess.h b/freebsd/FreeBSDProcess.h
@@ -13,12 +13,23 @@ in the source distribution for its full text.
 #include "Process.h"
 #include "Machine.h"
 
+typedef enum {
+   SCHEDCLASS_UNKNOWN = 0,
+
+   SCHEDCLASS_INTR_THREAD, /* interrupt thread */
+   SCHEDCLASS_REALTIME,
+   SCHEDCLASS_TIMESHARE, /* Regular scheduling */
+   SCHEDCLASS_IDLE,
+
+   MAX_SCHEDCLASS,
+} FreeBSDSchedClass;
 
 typedef struct FreeBSDProcess_ {
    Process super;
    int   jid;
    char* jname;
    char* emul;
+   FreeBSDSchedClass sched_class;
 } FreeBSDProcess;
 
 extern const ProcessClass FreeBSDProcess_class;

diff --git a/freebsd/FreeBSDProcessTable.c b/freebsd/FreeBSDProcessTable.c
@@ -248,14 +248,45 @@ void ProcessTable_goThroughEntries(ProcessTable* super) {
 
       proc->priority = kproc->ki_pri.pri_level - PZERO;
 
-      if (String_eq("intr", kproc->ki_comm) && (kproc->ki_flag & P_SYSTEM)) {
-         proc->nice = 0; //@etosan: intr kernel process (not thread) has weird nice value
-      } else if (kproc->ki_pri.pri_class == PRI_TIMESHARE) {
-         proc->nice = kproc->ki_nice - NZERO;
-      } else if (PRI_IS_REALTIME(kproc->ki_pri.pri_class)) {
-         proc->nice = PRIO_MIN - 1 - (PRI_MAX_REALTIME - kproc->ki_pri.pri_level);
-      } else {
-         proc->nice = PRIO_MAX + 1 + kproc->ki_pri.pri_level - PRI_MIN_IDLE;
+      switch (PRI_BASE(kproc->ki_pri.pri_class)) {
+         /* Handling of the below is explained in the FreeBSD base system in:
+          * /usr/src/usr.bin/top/machine.c (function format_nice) */
+         case PRI_ITHD:
+            fp->sched_class = SCHEDCLASS_INTR_THREAD;
+            proc->nice = 0;
+            break;
+
+         case PRI_REALTIME:
+            fp->sched_class = SCHEDCLASS_REALTIME;
+
+            /* Different for KPROCs and user procs */
+            if (kproc->ki_flag & P_KPROC) {
+               proc->nice = kproc->ki_pri.pri_native - PRI_MIN_REALTIME;
+            } else {
+               proc->nice = kproc->ki_pri.pri_user - PRI_MIN_REALTIME;
+            }
+            break;
+
+         case PRI_IDLE:
+            fp->sched_class = SCHEDCLASS_IDLE;
+
+            /* Different for KPROCs and user procs */
+            if (kproc->ki_flag & P_KPROC) {
+               proc->nice = kproc->ki_pri.pri_native - PRI_MIN_IDLE;
+            } else {
+               proc->nice = kproc->ki_pri.pri_user - PRI_MIN_IDLE;
+            }
+            break;
+
+         case PRI_TIMESHARE:
+            fp->sched_class = SCHEDCLASS_TIMESHARE;
+            proc->nice = kproc->ki_nice - NZERO;
+            break;
+
+         default:
+            fp->sched_class = SCHEDCLASS_UNKNOWN;
+            proc->nice = PROCESS_NICE_UNKNOWN;
+            break;
       }
 
       /* Taken from: https://github.com/freebsd/freebsd-src/blob/1ad2d87778970582854082bcedd2df0394fd4933/sys/sys/proc.h#L851 */

diff --git a/freebsd/ProcessField.h b/freebsd/ProcessField.h
@@ -12,6 +12,7 @@ in the source distribution for its full text.
    JID = 100,                    \
    JAIL = 101,                   \
    EMULATION = 102,              \
+   SCHEDCLASS = 103,             \
                                  \
    DUMMY_BUMP_FIELD = CWD,       \
    // End of list