Merge 63d0fe5b84 into 4f3df8c1bb

Nick's largely responsible for nerd-sniping me into fixing #34516
and did most of the testing.

NEWS

@@ -764,6 +764,9 @@ CHANGES WITH 257 in spe:
           other cases EnterNamespace= might be an suitable approach to acquire
           symbolized backtraces.)
 
+        Special thanks to Nick Owens for bringing attention to and testing
+        fixes for issue #34516.
+
         Contributions from: 12paper, A. Wilcox, Abderrahim Kitouni,
         Adrian Vovk, Alain Greppin, Allison Karlitskaya, Alyssa Ross,
         Anders Jonsson, Andika Triwidada, Andres Beltran, Anouk Ceyssens,

hwdb.d/60-sensor.hwdb

@@ -295,6 +295,10 @@ sensor:modalias:acpi:MXC6655*:dmi:*:svnCHUWIInnovationAndTechnology*:pnHi10X:*
 sensor:modalias:acpi:KIOX000A*:dmi:*:svnCHUWIInnovationAndTechnology*:pnHi10X:*
  ACCEL_MOUNT_MATRIX=0, -1, 0; -1, 0, 0; 0, 0, 1
 
+# Chuwi Hi10 X1
+sensor:modalias:acpi:NSA2513*:dmi:*:svnCHUWIInnovationAndTechnology*:pnHi10X1:*
+ ACCEL_MOUNT_MATRIX=0, 1, 0; -1, 0, 0; 0, 0, 1
+
 # Chuwi Hi10 Go
 sensor:modalias:acpi:MXC6655*:dmi:*:svnCHUWIINNOVATIONLIMITED:pnHi10Go:*
  ACCEL_MOUNT_MATRIX=-1, 0, 0; 0,-1, 0; 0, 0, 1

man/systemd-oomd.service.xml

@@ -127,6 +127,24 @@
     </variablelist>
   </refsect1>
 
+  <refsect1>
+    <title>Signals</title>
+
+    <variablelist>
+      <varlistentry>
+        <term><constant>SIGHUP</constant></term>
+
+        <listitem><para>Upon reception of the <constant>SIGHUP</constant> process signal
+        <command>systemd-oomd</command> will reload its configuration file.</para>
+
+        <xi:include href="version-info.xml" xpointer="v257"/></listitem>
+      </varlistentry>
+
+      <xi:include href="standard-options.xml" xpointer="help" />
+      <xi:include href="standard-options.xml" xpointer="version" />
+    </variablelist>
+  </refsect1>
+
   <refsect1>
     <title>See Also</title>
     <para><simplelist type="inline">

src/oom/meson.build

@@ -5,6 +5,7 @@ systemd_oomd_sources = files(
         'oomd-manager.c',
         'oomd-util.c',
         'oomd.c',
+        'oomd-conf.c'
 )
 
 executables += [

src/oom/oomd-conf.c

@@ -0,0 +1,115 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+
+#include "oomd-conf.h"
+#include "conf-parser.h"
+#include "log.h"
+#include "oomd-manager.h"
+
+static int arg_swap_used_limit_permyriad = -1;
+static int arg_mem_pressure_limit_permyriad = -1;
+static usec_t arg_mem_pressure_usec = DEFAULT_MEM_PRESSURE_DURATION_USEC;
+
+static int config_parse_duration(
+                const char *unit,
+                const char *filename,
+                unsigned line,
+                const char *section,
+                unsigned section_line,
+                const char *lvalue,
+                int ltype,
+                const char *rvalue,
+                void *data,
+                void *userdata) {
+
+        usec_t usec, *duration = ASSERT_PTR(data);
+        int r;
+
+        if (isempty(rvalue)) {
+                *duration = DEFAULT_MEM_PRESSURE_DURATION_USEC;
+                return 0;
+        }
+
+        r = parse_sec(rvalue, &usec);
+        if (r < 0)
+                return log_syntax_parse_error(unit, filename, line, r, lvalue, rvalue);
+
+        if (usec == 0) {
+                /* Map zero -> default for backwards compatibility. */
+                *duration = DEFAULT_MEM_PRESSURE_DURATION_USEC;
+                return 0;
+        }
+
+        if (usec < 1 * USEC_PER_SEC || usec == USEC_INFINITY)
+                return log_syntax(
+                                unit,
+                                LOG_WARNING,
+                                filename,
+                                line,
+                                0,
+                                "%s= must be at least 1s and less than infinity, ignoring: %s",
+                                lvalue,
+                                rvalue);
+
+        *duration = usec;
+        return 0;
+}
+
+int manager_set_defaults(Manager *m) {
+        assert(m);
+
+        int r;
+
+        m->swap_used_limit_permyriad = DEFAULT_SWAP_USED_LIMIT_PERCENT * 100;
+        r = store_loadavg_fixed_point(DEFAULT_MEM_PRESSURE_LIMIT_PERCENT, 0, &m->default_mem_pressure_limit);
+        if (r < 0)
+                return r;
+        m->default_mem_pressure_duration_usec = DEFAULT_MEM_PRESSURE_DURATION_USEC;
+
+        return 0;
+}
+
+int manager_parse_config_file(Manager *m) {
+        assert(m);
+
+        unsigned long l, f;
+        int r;
+
+        static const ConfigTableItem items[] = {
+                { "OOM", "SwapUsedLimit", config_parse_permyriad, 0, &arg_swap_used_limit_permyriad },
+                { "OOM", "DefaultMemoryPressureLimit", config_parse_permyriad, 0, &arg_mem_pressure_limit_permyriad },
+                { "OOM", "DefaultMemoryPressureDurationSec", config_parse_duration, 0, &arg_mem_pressure_usec },
+                {}
+        };
+
+        r = config_parse_standard_file_with_dropins(
+                        "systemd/oomd.conf",
+                        "OOM\0",
+                        config_item_table_lookup,
+                        items,
+                        CONFIG_PARSE_WARN,
+                        /* userdata= */ NULL);
+        if (r < 0)
+                return r;
+
+        m->swap_used_limit_permyriad = arg_swap_used_limit_permyriad >= 0 ?
+                        arg_swap_used_limit_permyriad :
+                        DEFAULT_SWAP_USED_LIMIT_PERCENT * 100;
+        assert(m->swap_used_limit_permyriad <= 10000);
+
+        if (arg_mem_pressure_limit_permyriad >= 0) {
+                assert(arg_mem_pressure_limit_permyriad <= 10000);
+
+                l = arg_mem_pressure_limit_permyriad / 100;
+                f = arg_mem_pressure_limit_permyriad % 100;
+        } else {
+                l = DEFAULT_MEM_PRESSURE_LIMIT_PERCENT;
+                f = 0;
+        }
+        r = store_loadavg_fixed_point(l, f, &m->default_mem_pressure_limit);
+        if (r < 0)
+                return r;
+
+        m->default_mem_pressure_duration_usec = arg_mem_pressure_usec;
+
+        return 0;
+}

src/oom/oomd-conf.h

@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+#pragma once
+
+#include "conf-parser.h"
+#include "oomd-manager.h"
+
+int manager_set_defaults(Manager *m);
+
+int manager_parse_config_file(Manager *m);

src/oom/oomd-manager.c

@@ -4,15 +4,17 @@
 #include "sd-json.h"
 
 #include "bus-log-control-api.h"
-#include "bus-util.h"
 #include "bus-polkit.h"
+#include "bus-util.h"
 #include "cgroup-util.h"
+#include "daemon-util.h"
 #include "fd-util.h"
 #include "fileio.h"
 #include "format-util.h"
 #include "json-util.h"
 #include "memory-util.h"
 #include "memstream-util.h"
+#include "oomd-conf.h"
 #include "oomd-manager-bus.h"
 #include "oomd-manager.h"
 #include "path-util.h"
@@ -120,6 +122,7 @@ static int process_managed_oom_message(Manager *m, uid_t uid, sd_json_variant *p
                 if (r < 0 && r != -EEXIST)
                         log_debug_errno(r, "Failed to insert message, ignoring: %m");
 
+                // TODO: Given that these values are updated dynamically on every invocation, it is okay not to handle this as part of reload.
                 /* Always update the limit in case it was changed. For non-memory pressure detection the value is
                  * ignored so always updating it here is not a problem. */
                 ctx = hashmap_get(monitor_hm, empty_to_root(message.path));
@@ -648,6 +651,26 @@ Manager* manager_free(Manager *m) {
         return mfree(m);
 }
 
+static int manager_dispatch_reload_signal(sd_event_source *s, const struct signalfd_siginfo *si, void *userdata) {
+        Manager *m = ASSERT_PTR(userdata);
+        int r;
+
+        (void) notify_reloading();
+
+        r = manager_set_defaults(m);
+        if (r < 0)
+                log_warning_errno(r, "Failed to set default values on reload: %m");
+
+        r = manager_parse_config_file(m);
+        if (r < 0)
+                log_warning_errno(r, "Failed to parse config file on reload: %m");
+        else
+                log_info("Config file reloaded.");
+
+        (void) sd_notify(/* unset= */ false, NOTIFY_READY);
+        return 0;
+}
+
 int manager_new(Manager **ret) {
         _cleanup_(manager_freep) Manager *m = NULL;
         int r;
@@ -658,12 +681,24 @@ int manager_new(Manager **ret) {
         if (!m)
                 return -ENOMEM;
 
+        r = manager_set_defaults(m);
+        if (r < 0)
+                log_warning_errno(r, "Failed to set default values on reload: %m");
+
+        r = manager_parse_config_file(m);
+        if (r < 0)
+                log_warning_errno(r, "Failed to parse configuration file: %m");
+
         r = sd_event_default(&m->event);
         if (r < 0)
                 return r;
 
         (void) sd_event_set_watchdog(m->event, true);
 
+        r = sd_event_add_signal(m->event, /* ret_event_source= */ NULL, SIGHUP | SD_EVENT_SIGNAL_PROCMASK, manager_dispatch_reload_signal, m);
+        if (r < 0)
+                return r;
+
         r = sd_event_set_signal_exit(m->event, true);
         if (r < 0)
                 return r;
@@ -754,36 +789,14 @@ static int manager_varlink_init(Manager *m, int fd) {
 int manager_start(
                 Manager *m,
                 bool dry_run,
-                int swap_used_limit_permyriad,
-                int mem_pressure_limit_permyriad,
-                usec_t mem_pressure_usec,
                 int fd) {
 
-        unsigned long l, f;
         int r;
 
         assert(m);
 
         m->dry_run = dry_run;
 
-        m->swap_used_limit_permyriad = swap_used_limit_permyriad >= 0 ? swap_used_limit_permyriad : DEFAULT_SWAP_USED_LIMIT_PERCENT * 100;
-        assert(m->swap_used_limit_permyriad <= 10000);
-
-        if (mem_pressure_limit_permyriad >= 0) {
-                assert(mem_pressure_limit_permyriad <= 10000);
-
-                l = mem_pressure_limit_permyriad / 100;
-                f = mem_pressure_limit_permyriad % 100;
-        } else {
-                l = DEFAULT_MEM_PRESSURE_LIMIT_PERCENT;
-                f = 0;
-        }
-        r = store_loadavg_fixed_point(l, f, &m->default_mem_pressure_limit);
-        if (r < 0)
-                return r;
-
-        m->default_mem_pressure_duration_usec = mem_pressure_usec;
-
         r = manager_connect_bus(m);
         if (r < 0)
                 return r;

src/oom/oomd-manager.h

@@ -66,7 +66,7 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free);
 
 int manager_new(Manager **ret);
 
-int manager_start(Manager *m, bool dry_run, int swap_used_limit_permyriad, int mem_pressure_limit_permyriad, usec_t mem_pressure_usec, int fd);
+int manager_start(Manager *m, bool dry_run, int fd);
 
 int manager_get_dump_string(Manager *m, char **ret);
 

src/oom/oomd.c

@@ -11,6 +11,7 @@
 #include "fileio.h"
 #include "log.h"
 #include "main-func.h"
+#include "oomd-conf.h"
 #include "oomd-manager-bus.h"
 #include "oomd-manager.h"
 #include "parse-util.h"
@@ -19,62 +20,6 @@
 #include "signal-util.h"
 
 static bool arg_dry_run = false;
-static int arg_swap_used_limit_permyriad = -1;
-static int arg_mem_pressure_limit_permyriad = -1;
-static usec_t arg_mem_pressure_usec = DEFAULT_MEM_PRESSURE_DURATION_USEC;
-
-static int config_parse_duration(
-                const char *unit,
-                const char *filename,
-                unsigned line,
-                const char *section,
-                unsigned section_line,
-                const char *lvalue,
-                int ltype,
-                const char *rvalue,
-                void *data,
-                void *userdata) {
-
-        usec_t usec, *duration = ASSERT_PTR(data);
-        int r;
-
-        if (isempty(rvalue)) {
-                *duration = DEFAULT_MEM_PRESSURE_DURATION_USEC;
-                return 0;
-        }
-
-        r = parse_sec(rvalue, &usec);
-        if (r < 0)
-                return log_syntax_parse_error(unit, filename, line, r, lvalue, rvalue);
-
-        if (usec == 0) {
-                /* Map zero -> default for backwards compatibility. */
-                *duration = DEFAULT_MEM_PRESSURE_DURATION_USEC;
-                return 0;
-        }
-
-        if (usec < 1 * USEC_PER_SEC || usec == USEC_INFINITY)
-                return log_syntax(unit, LOG_WARNING, filename, line, 0, "%s= must be at least 1s and less than infinity, ignoring: %s", lvalue, rvalue);
-
-        *duration = usec;
-        return 0;
-}
-
-static int parse_config(void) {
-        static const ConfigTableItem items[] = {
-                { "OOM", "SwapUsedLimit",                    config_parse_permyriad, 0, &arg_swap_used_limit_permyriad    },
-                { "OOM", "DefaultMemoryPressureLimit",       config_parse_permyriad, 0, &arg_mem_pressure_limit_permyriad },
-                { "OOM", "DefaultMemoryPressureDurationSec", config_parse_duration,  0, &arg_mem_pressure_usec            },
-                {}
-        };
-
-        return config_parse_standard_file_with_dropins(
-                        "systemd/oomd.conf",
-                        "OOM\0",
-                        config_item_table_lookup, items,
-                        CONFIG_PARSE_WARN,
-                        /* userdata= */ NULL);
-}
 
 static int help(void) {
         _cleanup_free_ char *link = NULL;
@@ -166,10 +111,6 @@ static int run(int argc, char *argv[]) {
         if (r <= 0)
                 return r;
 
-        r = parse_config();
-        if (r < 0)
-                return r;
-
         /* Do some basic requirement checks for running systemd-oomd. It's not exhaustive as some of the other
          * requirements do not have a reliable means to check for in code. */
 
@@ -211,9 +152,6 @@ static int run(int argc, char *argv[]) {
         r = manager_start(
                         m,
                         arg_dry_run,
-                        arg_swap_used_limit_permyriad,
-                        arg_mem_pressure_limit_permyriad,
-                        arg_mem_pressure_usec,
                         fd);
         if (r < 0)
                 return log_error_errno(r, "Failed to start up daemon: %m");

test/units/TEST-55-OOMD.sh

@@ -264,6 +264,41 @@ EOF
     systemctl daemon-reload
 }
 
+testcase_reload() {
+    # Check if the oomd.conf drop-in config is loaded.
+    assert_in 'Swap Used Limit: 90.00%' "$(oomctl)"
+    assert_in 'Default Memory Pressure Limit: 60.00%' "$(oomctl)"
+    assert_in 'Default Memory Pressure Duration: 2s' "$(oomctl)"
+
+    # Test oomd reload
+    mkdir -p /run/systemd/oomd.conf.d/
+    {
+        echo "[OOM]"
+        echo "SwapUsedLimit=80%"
+        echo "DefaultMemoryPressureLimit=55%"
+        echo "DefaultMemoryPressureDurationSec=5s"
+    } >/run/systemd/oomd.conf.d/99-oomd-test.conf
+
+    systemctl reload systemd-oomd.service
+    assert_in 'Swap Used Limit: 80.00%' "$(oomctl)"
+    assert_in 'Default Memory Pressure Limit: 55.00%' "$(oomctl)"
+    assert_in 'Default Memory Pressure Duration: 5s' "$(oomctl)"
+
+    # Set back to default via reload
+    mkdir -p /run/systemd/oomd.conf.d/
+    {
+        echo "[OOM]"
+        echo "SwapUsedLimit=90%"
+        echo "DefaultMemoryPressureLimit=60%"
+        echo "DefaultMemoryPressureDurationSec=2s"
+    } >/run/systemd/oomd.conf.d/99-oomd-test.conf
+
+    systemctl reload systemd-oomd.service
+    assert_in 'Swap Used Limit: 90.00%' "$(oomctl)"
+    assert_in 'Default Memory Pressure Limit: 60.00%' "$(oomctl)"
+    assert_in 'Default Memory Pressure Duration: 2s' "$(oomctl)"
+}
+
 run_testcases
 
 systemd-analyze log-level info

units/systemd-oomd.service.in

@@ -53,7 +53,7 @@ RestrictSUIDSGID=yes
 SystemCallArchitectures=native
 SystemCallErrorNumber=EPERM
 SystemCallFilter=@system-service
-Type=notify
+Type=notify-reload
 User=systemd-oom
 {{SERVICE_WATCHDOG}}