From d9d3f05def0057cc064a05f8cbfb5f398779cd8c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 17 Feb 2021 17:51:27 +0100 Subject: [PATCH] core: use our usual UINT32_MAX scaling for OOMD limits So far OOMD limits used permyriads, as an upgrade from the original percent. The rest of our codebase typically scales stuff relative to UINT32_MAX. Let's clean this up, an make sure this happens here too. This is particularly relevant, as this is exposed in unit files and API, and before we mark this stable we should get the APIs right. --- man/org.freedesktop.systemd1.xml | 36 +++++++++++++-------------- src/core/cgroup.c | 4 +-- src/core/cgroup.h | 2 +- src/core/core-varlink.c | 2 +- src/core/dbus-cgroup.c | 13 +++++----- src/core/load-fragment-gperf.gperf.m4 | 2 +- src/core/load-fragment.c | 3 ++- src/oom/oomd-manager.c | 12 ++++++--- src/shared/bus-unit-util.c | 7 ++---- 9 files changed, 42 insertions(+), 39 deletions(-) diff --git a/man/org.freedesktop.systemd1.xml b/man/org.freedesktop.systemd1.xml index 2da0ff0579..8c370ba9a4 100644 --- a/man/org.freedesktop.systemd1.xml +++ b/man/org.freedesktop.systemd1.xml @@ -2469,7 +2469,7 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly s ManagedOOMMemoryPressure = '...'; @org.freedesktop.DBus.Property.EmitsChangedSignal("false") - readonly u ManagedOOMMemoryPressureLimitPermyriad = ...; + readonly u ManagedOOMMemoryPressureLimit = ...; @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly s ManagedOOMPreference = '...'; @org.freedesktop.DBus.Property.EmitsChangedSignal("const") @@ -2994,7 +2994,7 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { - + @@ -3560,7 +3560,7 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { - + @@ -4229,7 +4229,7 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2esocket { @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly s ManagedOOMMemoryPressure = '...'; @org.freedesktop.DBus.Property.EmitsChangedSignal("false") - readonly u ManagedOOMMemoryPressureLimitPermyriad = ...; + readonly u ManagedOOMMemoryPressureLimit = ...; @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly s ManagedOOMPreference = '...'; @org.freedesktop.DBus.Property.EmitsChangedSignal("const") @@ -4782,7 +4782,7 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2esocket { - + @@ -5346,7 +5346,7 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2esocket { - + @@ -5928,7 +5928,7 @@ node /org/freedesktop/systemd1/unit/home_2emount { @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly s ManagedOOMMemoryPressure = '...'; @org.freedesktop.DBus.Property.EmitsChangedSignal("false") - readonly u ManagedOOMMemoryPressureLimitPermyriad = ...; + readonly u ManagedOOMMemoryPressureLimit = ...; @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly s ManagedOOMPreference = '...'; @org.freedesktop.DBus.Property.EmitsChangedSignal("const") @@ -6409,7 +6409,7 @@ node /org/freedesktop/systemd1/unit/home_2emount { - + @@ -6891,7 +6891,7 @@ node /org/freedesktop/systemd1/unit/home_2emount { - + @@ -7594,7 +7594,7 @@ node /org/freedesktop/systemd1/unit/dev_2dsda3_2eswap { @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly s ManagedOOMMemoryPressure = '...'; @org.freedesktop.DBus.Property.EmitsChangedSignal("false") - readonly u ManagedOOMMemoryPressureLimitPermyriad = ...; + readonly u ManagedOOMMemoryPressureLimit = ...; @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly s ManagedOOMPreference = '...'; @org.freedesktop.DBus.Property.EmitsChangedSignal("const") @@ -8061,7 +8061,7 @@ node /org/freedesktop/systemd1/unit/dev_2dsda3_2eswap { - + @@ -8529,7 +8529,7 @@ node /org/freedesktop/systemd1/unit/dev_2dsda3_2eswap { - + @@ -9085,7 +9085,7 @@ node /org/freedesktop/systemd1/unit/system_2eslice { @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly s ManagedOOMMemoryPressure = '...'; @org.freedesktop.DBus.Property.EmitsChangedSignal("false") - readonly u ManagedOOMMemoryPressureLimitPermyriad = ...; + readonly u ManagedOOMMemoryPressureLimit = ...; @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly s ManagedOOMPreference = '...'; }; @@ -9222,7 +9222,7 @@ node /org/freedesktop/systemd1/unit/system_2eslice { - + @@ -9364,7 +9364,7 @@ node /org/freedesktop/systemd1/unit/system_2eslice { - + @@ -9526,7 +9526,7 @@ node /org/freedesktop/systemd1/unit/session_2d1_2escope { @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly s ManagedOOMMemoryPressure = '...'; @org.freedesktop.DBus.Property.EmitsChangedSignal("false") - readonly u ManagedOOMMemoryPressureLimitPermyriad = ...; + readonly u ManagedOOMMemoryPressureLimit = ...; @org.freedesktop.DBus.Property.EmitsChangedSignal("false") readonly s ManagedOOMPreference = '...'; @org.freedesktop.DBus.Property.EmitsChangedSignal("const") @@ -9679,7 +9679,7 @@ node /org/freedesktop/systemd1/unit/session_2d1_2escope { - + @@ -9847,7 +9847,7 @@ node /org/freedesktop/systemd1/unit/session_2d1_2escope { - + diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 644bf50e7d..1f74cb393f 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -419,7 +419,7 @@ void cgroup_context_dump(Unit *u, FILE* f, const char *prefix) { "%sDelegate: %s\n" "%sManagedOOMSwap: %s\n" "%sManagedOOMMemoryPressure: %s\n" - "%sManagedOOMMemoryPressureLimit: %" PRIu32 ".%02" PRIu32 "%%\n" + "%sManagedOOMMemoryPressureLimit: " PERMYRIAD_AS_PERCENT_FORMAT_STR "\n" "%sManagedOOMPreference: %s%%\n", prefix, yes_no(c->cpu_accounting), prefix, yes_no(c->io_accounting), @@ -453,7 +453,7 @@ void cgroup_context_dump(Unit *u, FILE* f, const char *prefix) { prefix, yes_no(c->delegate), prefix, managed_oom_mode_to_string(c->moom_swap), prefix, managed_oom_mode_to_string(c->moom_mem_pressure), - prefix, c->moom_mem_pressure_limit_permyriad / 100, c->moom_mem_pressure_limit_permyriad % 100, + prefix, PERMYRIAD_AS_PERCENT_FORMAT_VAL(UINT32_SCALE_TO_PERMYRIAD(c->moom_mem_pressure_limit)), prefix, managed_oom_preference_to_string(c->moom_preference)); if (c->delegate) { diff --git a/src/core/cgroup.h b/src/core/cgroup.h index 8183551420..fa79ba1523 100644 --- a/src/core/cgroup.h +++ b/src/core/cgroup.h @@ -163,7 +163,7 @@ struct CGroupContext { /* Settings for systemd-oomd */ ManagedOOMMode moom_swap; ManagedOOMMode moom_mem_pressure; - uint32_t moom_mem_pressure_limit_permyriad; + uint32_t moom_mem_pressure_limit; /* Normalized to 2^32-1 == 100% */ ManagedOOMPreference moom_preference; }; diff --git a/src/core/core-varlink.c b/src/core/core-varlink.c index df542e82d1..d695106658 100644 --- a/src/core/core-varlink.c +++ b/src/core/core-varlink.c @@ -83,7 +83,7 @@ static int build_managed_oom_json_array_element(Unit *u, const char *property, J JSON_BUILD_PAIR("mode", JSON_BUILD_STRING(mode)), JSON_BUILD_PAIR("path", JSON_BUILD_STRING(u->cgroup_path)), JSON_BUILD_PAIR("property", JSON_BUILD_STRING(property)), - JSON_BUILD_PAIR_CONDITION(use_limit, "limit", JSON_BUILD_UNSIGNED(c->moom_mem_pressure_limit_permyriad)))); + JSON_BUILD_PAIR_CONDITION(use_limit, "limit", JSON_BUILD_UNSIGNED(c->moom_mem_pressure_limit)))); } int manager_varlink_send_managed_oom_update(Unit *u) { diff --git a/src/core/dbus-cgroup.c b/src/core/dbus-cgroup.c index 5bf9d7f4a8..04d2ba34f3 100644 --- a/src/core/dbus-cgroup.c +++ b/src/core/dbus-cgroup.c @@ -396,7 +396,7 @@ const sd_bus_vtable bus_cgroup_vtable[] = { SD_BUS_PROPERTY("DisableControllers", "as", property_get_cgroup_mask, offsetof(CGroupContext, disable_controllers), 0), SD_BUS_PROPERTY("ManagedOOMSwap", "s", property_get_managed_oom_mode, offsetof(CGroupContext, moom_swap), 0), SD_BUS_PROPERTY("ManagedOOMMemoryPressure", "s", property_get_managed_oom_mode, offsetof(CGroupContext, moom_mem_pressure), 0), - SD_BUS_PROPERTY("ManagedOOMMemoryPressureLimitPermyriad", "u", NULL, offsetof(CGroupContext, moom_mem_pressure_limit_permyriad), 0), + SD_BUS_PROPERTY("ManagedOOMMemoryPressureLimit", "u", NULL, offsetof(CGroupContext, moom_mem_pressure_limit), 0), SD_BUS_PROPERTY("ManagedOOMPreference", "s", property_get_managed_oom_preference, offsetof(CGroupContext, moom_preference), 0), SD_BUS_VTABLE_END }; @@ -1699,7 +1699,7 @@ int bus_cgroup_set_property( return 1; } - if (streq(name, "ManagedOOMMemoryPressureLimitPermyriad")) { + if (streq(name, "ManagedOOMMemoryPressureLimit")) { uint32_t v; if (!UNIT_VTABLE(u)->can_set_managed_oom) @@ -1709,12 +1709,11 @@ int bus_cgroup_set_property( if (r < 0) return r; - if (v > 10000) - return -ERANGE; - if (!UNIT_WRITE_FLAGS_NOOP(flags)) { - c->moom_mem_pressure_limit_permyriad = v; - unit_write_settingf(u, flags, name, "ManagedOOMMemoryPressureLimit=%" PRIu32 ".%02" PRIu32 "%%", v / 100, v % 100); + c->moom_mem_pressure_limit = v; + unit_write_settingf(u, flags, name, + "ManagedOOMMemoryPressureLimit=" PERMYRIAD_AS_PERCENT_FORMAT_STR, + PERMYRIAD_AS_PERCENT_FORMAT_VAL(UINT32_SCALE_TO_PERMYRIAD(v))); } if (c->moom_mem_pressure == MANAGED_OOM_KILL) diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4 index b0460e2764..6ed6b07db2 100644 --- a/src/core/load-fragment-gperf.gperf.m4 +++ b/src/core/load-fragment-gperf.gperf.m4 @@ -229,7 +229,7 @@ $1.IPIngressFilterPath, config_parse_ip_filter_bpf_progs, $1.IPEgressFilterPath, config_parse_ip_filter_bpf_progs, 0, offsetof($1, cgroup_context.ip_filters_egress) $1.ManagedOOMSwap, config_parse_managed_oom_mode, 0, offsetof($1, cgroup_context.moom_swap) $1.ManagedOOMMemoryPressure, config_parse_managed_oom_mode, 0, offsetof($1, cgroup_context.moom_mem_pressure) -$1.ManagedOOMMemoryPressureLimit, config_parse_managed_oom_mem_pressure_limit, 0, offsetof($1, cgroup_context.moom_mem_pressure_limit_permyriad) +$1.ManagedOOMMemoryPressureLimit, config_parse_managed_oom_mem_pressure_limit, 0, offsetof($1, cgroup_context.moom_mem_pressure_limit) $1.ManagedOOMPreference, config_parse_managed_oom_preference, 0, offsetof($1, cgroup_context.moom_preference) $1.NetClass, config_parse_warn_compat, DISABLED_LEGACY, 0' )m4_dnl diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index c56c2ffc61..b9fc450ac7 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -3901,7 +3901,8 @@ int config_parse_managed_oom_mem_pressure_limit( return 0; } - *limit = r; + /* Normalize to 2^32-1 == 100% */ + *limit = UINT32_SCALE_FROM_PERMYRIAD(r); return 0; } diff --git a/src/oom/oomd-manager.c b/src/oom/oomd-manager.c index bdf41807b2..65c0bfac61 100644 --- a/src/oom/oomd-manager.c +++ b/src/oom/oomd-manager.c @@ -10,6 +10,7 @@ #include "oomd-manager-bus.h" #include "oomd-manager.h" #include "path-util.h" +#include "percent-util.h" typedef struct ManagedOOMReply { ManagedOOMMode mode; @@ -100,10 +101,15 @@ static int process_managed_oom_reply( limit = m->default_mem_pressure_limit; if (streq(reply.property, "ManagedOOMMemoryPressure")) { - if (reply.limit > 10000) + if (reply.limit > UINT32_MAX) /* out of range */ continue; - else if (reply.limit != 0) { - ret = store_loadavg_fixed_point((unsigned long) reply.limit / 100, (unsigned long) reply.limit % 100, &limit); + if (reply.limit != 0) { + int permyriad = UINT32_SCALE_TO_PERMYRIAD(reply.limit); + + ret = store_loadavg_fixed_point( + (unsigned long) permyriad / 100, + (unsigned long) permyriad % 100, + &limit); if (ret < 0) continue; } diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index 27c8eb915f..83130db2fa 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -441,15 +441,12 @@ static int bus_append_cgroup_property(sd_bus_message *m, const char *field, cons return bus_append_string(m, field, eq); if (STR_IN_SET(field, "ManagedOOMMemoryPressureLimit")) { - char *n; - r = parse_permyriad(eq); if (r < 0) return log_error_errno(r, "Failed to parse %s value: %s", field, eq); - n = strjoina(field, "Permyriad"); - - r = sd_bus_message_append(m, "(sv)", n, "u", (uint32_t) r); + /* Pass around scaled to 2^32-1 == 100% */ + r = sd_bus_message_append(m, "(sv)", field, "u", UINT32_SCALE_FROM_PERMYRIAD(r)); if (r < 0) return bus_log_create_error(r); -- 2.25.1