From 112b69860f2cb6059c49404abbfb4e3f22603cfc Mon Sep 17 00:00:00 2001 From: Matt Whitlock Date: Sun, 21 Aug 2022 09:55:04 -0400 Subject: [PATCH] start-stop-daemon, supervise-daemon: set autogroup nicelevel As described in "Why nice levels are a placebo and have been for a very long time, and no one seems to have noticed"[1], the Linux kernel in its default configuration on many Linux distributions autogroups tasks by session ID and "fairly" allocates CPU time among such autogroups. The nice levels of tasks within each autogroup are only relative to other tasks within the same autogroup. Effectively, this means that the traditional nice level is rendered moot for tools like start-stop-daemon and supervise-daemon, which start each daemon in its own session and thus in its own autogroup. Linux does provide a means to change the niceness of autogroups relative to each other, so let's have start-stop- daemon and supervise-daemon make use of this feature where available so that -N,--nicelevel/SSD_NICELEVEL will actually do what the user intends. On systems where autogroups are not supported or are disabled, this commit introduces no change in behavior. Note that the setsid() call in the child process of start-stop-daemon is moved to much earlier. This is necessary so that the new process will be assigned to a new autogroup before the autogroup nicelevel is set. To avoid inadvertently acquiring /dev/tty as the controlling terminal of the new session after setsid() has given up the controlling terminal inherited from the parent process, tty_fd is opened before the call to setsid(). [1] https://www.reddit.com/r/linux/comments/d7hx2c/why_nice_levels_are_a_placebo_and_have_been_for_a/ This fixes #542. --- src/start-stop-daemon/start-stop-daemon.c | 13 ++++++++++++- src/supervise-daemon/supervise-daemon.c | 8 ++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/start-stop-daemon/start-stop-daemon.c b/src/start-stop-daemon/start-stop-daemon.c index 9117f92f..e439388f 100644 --- a/src/start-stop-daemon/start-stop-daemon.c +++ b/src/start-stop-daemon/start-stop-daemon.c @@ -878,11 +878,23 @@ int main(int argc, char **argv) devnull_fd = open("/dev/null", O_RDWR); + /* Must call setsid() before setting autogroup nicelevel + * but after opening tty_fd. */ + setsid(); + if (nicelevel != INT_MIN) { if (setpriority(PRIO_PROCESS, mypid, nicelevel) == -1) eerrorx("%s: setpriority %d: %s", applet, nicelevel, strerror(errno)); + /* Open in "r+" mode to avoid creating if non-existent. */ + fp = fopen("/proc/self/autogroup", "r+"); + if (fp) { + fprintf(fp, "%d\n", nicelevel); + fclose(fp); + } else if (errno != ENOENT) + eerrorx("%s: autogroup nice %d: %s", applet, + nicelevel, strerror(errno)); } if (ionicec != -1 && @@ -1118,7 +1130,6 @@ int main(int argc, char **argv) eerrorx("Failed to set scheduler parameters: %s", strerror(errno)); } - setsid(); execvp(exec, argv); #ifdef HAVE_PAM if (changeuser != NULL && pamr == PAM_SUCCESS) diff --git a/src/supervise-daemon/supervise-daemon.c b/src/supervise-daemon/supervise-daemon.c index 58946716..68490ad4 100644 --- a/src/supervise-daemon/supervise-daemon.c +++ b/src/supervise-daemon/supervise-daemon.c @@ -391,6 +391,14 @@ static void child_process(char *exec, char **argv) if (setpriority(PRIO_PROCESS, getpid(), nicelevel) == -1) eerrorx("%s: setpriority %d: %s", applet, nicelevel, strerror(errno)); + /* Open in "r+" mode to avoid creating if non-existent. */ + fp = fopen("/proc/self/autogroup", "r+"); + if (fp) { + fprintf(fp, "%d\n", nicelevel); + fclose(fp); + } else if (errno != ENOENT) + eerrorx("%s: autogroup nice %d: %s", applet, + nicelevel, strerror(errno)); } if (ionicec != -1 && ioprio_set(1, getpid(), ionicec | ioniced) == -1)