From: Anita Zhang Date: Tue, 10 Mar 2020 08:46:09 +0000 (-0700) Subject: core: add varlink call to get cgroup paths of units using ManagedOOM*= X-Git-Tag: v247-rc1~66^2~11 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=e30bbc90c9c2b12792894dbef877d7d96371cbda;p=thirdparty%2Fsystemd.git core: add varlink call to get cgroup paths of units using ManagedOOM*= --- diff --git a/src/basic/def.h b/src/basic/def.h index 970654a1ade..9f1f3c229c7 100644 --- a/src/basic/def.h +++ b/src/basic/def.h @@ -63,3 +63,5 @@ .un.sun_family = AF_UNIX, \ .un.sun_path = "\0/org/freedesktop/plymouthd", \ } + +#define VARLINK_ADDR_PATH_MANAGED_OOM "/run/systemd/io.system.ManagedOOM" diff --git a/src/core/core-varlink.c b/src/core/core-varlink.c index 4bb262bd93d..18219097f44 100644 --- a/src/core/core-varlink.c +++ b/src/core/core-varlink.c @@ -2,6 +2,7 @@ #include "core-varlink.h" #include "mkdir.h" +#include "strv.h" #include "user-util.h" #include "varlink.h" @@ -15,6 +16,11 @@ typedef struct LookupParameters { const char *service; } LookupParameters; +static const char* const managed_oom_mode_properties[] = { + "ManagedOOMSwap", + "ManagedOOMMemoryPressure", +}; + static int build_user_json(const char *user_name, uid_t uid, JsonVariant **ret) { assert(user_name); assert(uid_is_valid(uid)); @@ -45,6 +51,150 @@ static bool user_match_lookup_parameters(LookupParameters *p, const char *name, return true; } +static int build_managed_oom_json_array_element(Unit *u, const char *property, JsonVariant **ret_v) { + _cleanup_(json_variant_unrefp) JsonVariant *v = NULL; + CGroupContext *c; + const char *mode; + int r; + + assert(u); + assert(property); + assert(ret_v); + + if (!UNIT_VTABLE(u)->can_set_managed_oom) + return -EOPNOTSUPP; + + c = unit_get_cgroup_context(u); + if (!c) + return -EINVAL; + + if (UNIT_IS_INACTIVE_OR_FAILED(unit_active_state(u))) + /* systemd-oomd should always treat inactive units as though they didn't enable any action since they + * should not have a valid cgroup */ + mode = managed_oom_mode_to_string(MANAGED_OOM_AUTO); + else if (streq(property, "ManagedOOMSwap")) + mode = managed_oom_mode_to_string(c->moom_swap); + else if (streq(property, "ManagedOOMMemoryPressure")) + mode = managed_oom_mode_to_string(c->moom_mem_pressure); + else + return -EINVAL; + + r = json_build(&v, JSON_BUILD_OBJECT( + 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("limit", JSON_BUILD_UNSIGNED(c->moom_mem_pressure_limit)))); + + *ret_v = TAKE_PTR(v); + return r; +} + +int manager_varlink_send_managed_oom_update(Unit *u) { + _cleanup_(json_variant_unrefp) JsonVariant *arr = NULL, *v = NULL; + CGroupContext *c; + int r; + + assert(u); + + if (!UNIT_VTABLE(u)->can_set_managed_oom || !u->manager || !u->manager->managed_oom_varlink_request || !u->cgroup_path) + return 0; + + c = unit_get_cgroup_context(u); + if (!c) + return 0; + + r = json_build(&arr, JSON_BUILD_EMPTY_ARRAY); + if (r < 0) + return r; + + for (size_t i = 0; i < ELEMENTSOF(managed_oom_mode_properties); i++) { + _cleanup_(json_variant_unrefp) JsonVariant *e = NULL; + + r = build_managed_oom_json_array_element(u, managed_oom_mode_properties[i], &e); + if (r < 0) + return r; + + r = json_variant_append_array(&arr, e); + if (r < 0) + return r; + } + + r = json_build(&v, JSON_BUILD_OBJECT(JSON_BUILD_PAIR("cgroups", JSON_BUILD_VARIANT(arr)))); + if (r < 0) + return r; + + return varlink_notify(u->manager->managed_oom_varlink_request, v); +} + +static int vl_method_subscribe_managed_oom_cgroups( + Varlink *link, + JsonVariant *parameters, + VarlinkMethodFlags flags, + void *userdata) { + static const UnitType supported_unit_types[] = { UNIT_SLICE, UNIT_SERVICE, UNIT_SCOPE }; + _cleanup_(json_variant_unrefp) JsonVariant *v = NULL, *arr = NULL; + Manager *m = userdata; + int r; + + assert(link); + assert(m); + + if (json_variant_elements(parameters) > 0) + return varlink_error_invalid_parameter(link, parameters); + + /* We only take one subscriber for this method so return an error if there's already an existing one. + * This shouldn't happen since systemd-oomd is the only client of this method. */ + if (FLAGS_SET(flags, VARLINK_METHOD_MORE) && m->managed_oom_varlink_request) + return varlink_error(m->managed_oom_varlink_request, VARLINK_ERROR_SUBSCRIPTION_TAKEN, NULL); + + r = json_build(&arr, JSON_BUILD_EMPTY_ARRAY); + if (r < 0) + return r; + + for (size_t i = 0; i < ELEMENTSOF(supported_unit_types); i++) { + Unit *u; + + LIST_FOREACH(units_by_type, u, m->units_by_type[supported_unit_types[i]]) { + CGroupContext *c; + + if (UNIT_IS_INACTIVE_OR_FAILED(unit_active_state(u))) + continue; + + c = unit_get_cgroup_context(u); + if (!c) + continue; + + for (size_t j = 0; j < ELEMENTSOF(managed_oom_mode_properties); j++) { + _cleanup_(json_variant_unrefp) JsonVariant *e = NULL; + + /* For the initial varlink call we only care about units that enabled (i.e. mode is not + * set to "auto") oomd properties. */ + if (!(streq(managed_oom_mode_properties[j], "ManagedOOMSwap") && c->moom_swap == MANAGED_OOM_KILL) && + !(streq(managed_oom_mode_properties[j], "ManagedOOMMemoryPressure") && c->moom_mem_pressure == MANAGED_OOM_KILL)) + continue; + + r = build_managed_oom_json_array_element(u, managed_oom_mode_properties[j], &e); + if (r < 0) + return r; + + r = json_variant_append_array(&arr, e); + if (r < 0) + return r; + } + } + } + + r = json_build(&v, JSON_BUILD_OBJECT(JSON_BUILD_PAIR("cgroups", JSON_BUILD_VARIANT(arr)))); + if (r < 0) + return r; + + if (!FLAGS_SET(flags, VARLINK_METHOD_MORE)) + return varlink_reply(link, v); + + m->managed_oom_varlink_request = varlink_ref(link); + return varlink_notify(m->managed_oom_varlink_request, v); +} + static int vl_method_get_user_record(Varlink *link, JsonVariant *parameters, VarlinkMethodFlags flags, void *userdata) { static const JsonDispatch dispatch_table[] = { @@ -262,6 +412,17 @@ static int vl_method_get_memberships(Varlink *link, JsonVariant *parameters, Var return varlink_error(link, "io.systemd.UserDatabase.NoRecordFound", NULL); } +static void vl_disconnect(VarlinkServer *s, Varlink *link, void *userdata) { + Manager *m = userdata; + + assert(m); + assert(s); + assert(link); + + if (link == m->managed_oom_varlink_request) + m->managed_oom_varlink_request = varlink_unref(link); +} + int manager_varlink_init(Manager *m) { _cleanup_(varlink_server_unrefp) VarlinkServer *s = NULL; int r; @@ -284,16 +445,25 @@ int manager_varlink_init(Manager *m) { s, "io.systemd.UserDatabase.GetUserRecord", vl_method_get_user_record, "io.systemd.UserDatabase.GetGroupRecord", vl_method_get_group_record, - "io.systemd.UserDatabase.GetMemberships", vl_method_get_memberships); + "io.systemd.UserDatabase.GetMemberships", vl_method_get_memberships, + "io.systemd.ManagedOOM.SubscribeManagedOOMCGroups", vl_method_subscribe_managed_oom_cgroups); if (r < 0) return log_error_errno(r, "Failed to register varlink methods: %m"); + r = varlink_server_bind_disconnect(s, vl_disconnect); + if (r < 0) + return log_error_errno(r, "Failed to register varlink disconnect handler: %m"); + if (!MANAGER_IS_TEST_RUN(m)) { (void) mkdir_p_label("/run/systemd/userdb", 0755); r = varlink_server_listen_address(s, "/run/systemd/userdb/io.systemd.DynamicUser", 0666); if (r < 0) return log_error_errno(r, "Failed to bind to varlink socket: %m"); + + r = varlink_server_listen_address(s, VARLINK_ADDR_PATH_MANAGED_OOM, 0666); + if (r < 0) + return log_error_errno(r, "Failed to bind to varlink socket: %m"); } r = varlink_server_attach_event(s, m->event, SD_EVENT_PRIORITY_NORMAL); @@ -307,5 +477,11 @@ int manager_varlink_init(Manager *m) { void manager_varlink_done(Manager *m) { assert(m); + /* Send the final message if we still have a subscribe request open. */ + if (m->managed_oom_varlink_request) { + (void) varlink_error(m->managed_oom_varlink_request, VARLINK_ERROR_DISCONNECTED, NULL); + m->managed_oom_varlink_request = varlink_unref(m->managed_oom_varlink_request); + } + m->varlink_server = varlink_server_unref(m->varlink_server); } diff --git a/src/core/core-varlink.h b/src/core/core-varlink.h index 89818e2766c..0b191ae6c44 100644 --- a/src/core/core-varlink.h +++ b/src/core/core-varlink.h @@ -5,3 +5,8 @@ int manager_varlink_init(Manager *m); void manager_varlink_done(Manager *m); + +/* The manager is expected to send an update to systemd-oomd if one of the following occurs: + * - The value of ManagedOOM*= properties change + * - A unit with ManagedOOM*= properties changes unit active state */ +int manager_varlink_send_managed_oom_update(Unit *u); diff --git a/src/core/dbus-cgroup.c b/src/core/dbus-cgroup.c index 78abcbdbc8d..a39116ff0a3 100644 --- a/src/core/dbus-cgroup.c +++ b/src/core/dbus-cgroup.c @@ -8,6 +8,7 @@ #include "bus-get-properties.h" #include "cgroup-util.h" #include "cgroup.h" +#include "core-varlink.h" #include "dbus-cgroup.h" #include "dbus-util.h" #include "errno-util.h" @@ -1692,6 +1693,7 @@ int bus_cgroup_set_property( unit_write_settingf(u, flags, name, "%s=%s", name, mode); } + (void) manager_varlink_send_managed_oom_update(u); return 1; } @@ -1699,7 +1701,14 @@ int bus_cgroup_set_property( if (!UNIT_VTABLE(u)->can_set_managed_oom) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Cannot set %s for this unit type", name); - return bus_set_transient_percent(u, name, &c->moom_mem_pressure_limit, message, flags, error); + r = bus_set_transient_percent(u, name, &c->moom_mem_pressure_limit, message, flags, error); + if (r < 0) + return r; + + if (c->moom_mem_pressure == MANAGED_OOM_KILL) + (void) manager_varlink_send_managed_oom_update(u); + + return 1; } if (streq(name, "DisableControllers") || (u->transient && u->load_state == UNIT_STUB)) diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4 index f84febd9536..5b148779dc4 100644 --- a/src/core/load-fragment-gperf.gperf.m4 +++ b/src/core/load-fragment-gperf.gperf.m4 @@ -226,7 +226,7 @@ $1.IPIngressFilterPath, config_parse_ip_filter_bpf_progs, 0, $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.ManagedOOMMemoryPressureLimitPercent,config_parse_managed_oom_mem_pressure_limit,0, offsetof($1, cgroup_context.moom_mem_pressure_limit) +$1.ManagedOOMMemoryPressureLimitPercent,config_parse_managed_oom_mem_pressure_limit,0, offsetof($1, cgroup_context) $1.NetClass, config_parse_warn_compat, DISABLED_LEGACY, 0' )m4_dnl Unit.Description, config_parse_unit_string_printf, 0, offsetof(Unit, description) diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 81253239aa3..9361e930515 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -26,6 +26,7 @@ #include "capability-util.h" #include "cgroup-setup.h" #include "conf-parser.h" +#include "core-varlink.h" #include "cpu-set-util.h" #include "env-util.h" #include "errno-list.h" @@ -3823,7 +3824,7 @@ int config_parse_managed_oom_mode( const char *rvalue, void *data, void *userdata) { - + Unit *u = userdata; ManagedOOMMode *mode = data, m; UnitType t; @@ -3835,7 +3836,7 @@ int config_parse_managed_oom_mode( if (isempty(rvalue)) { *mode = MANAGED_OOM_AUTO; - return 0; + goto finish; } m = managed_oom_mode_from_string(rvalue); @@ -3843,8 +3844,10 @@ int config_parse_managed_oom_mode( log_syntax(unit, LOG_WARNING, filename, line, 0, "Invalid syntax, ignoring: %s", rvalue); return 0; } - *mode = m; + +finish: + (void) manager_varlink_send_managed_oom_update(u); return 0; } @@ -3859,8 +3862,8 @@ int config_parse_managed_oom_mem_pressure_limit( const char *rvalue, void *data, void *userdata) { - - unsigned *limit = data; + Unit *u = userdata; + CGroupContext *c = data; UnitType t; int r; @@ -3871,8 +3874,8 @@ int config_parse_managed_oom_mem_pressure_limit( return log_syntax(unit, LOG_WARNING, filename, line, 0, "%s= is not supported for this unit type, ignoring.", lvalue); if (isempty(rvalue)) { - *limit = 0; - return 0; + c->moom_mem_pressure_limit = 0; + goto finish; } r = parse_percent(rvalue); @@ -3881,7 +3884,12 @@ int config_parse_managed_oom_mem_pressure_limit( return 0; } - *limit = r; + c->moom_mem_pressure_limit = r; + +finish: + /* Only update the limit if memory pressure detection is enabled because the information is irrelevant otherwise */ + if (c->moom_mem_pressure == MANAGED_OOM_KILL) + (void) manager_varlink_send_managed_oom_update(u); return 0; } diff --git a/src/core/manager.h b/src/core/manager.h index 9e98b31c4bd..073cc74a853 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -434,6 +434,8 @@ struct Manager { bool honor_device_enumeration; VarlinkServer *varlink_server; + /* Only systemd-oomd should be using this to subscribe to changes in ManagedOOM settings */ + Varlink *managed_oom_varlink_request; }; static inline usec_t manager_default_timeout_abort_usec(Manager *m) { diff --git a/src/core/unit.c b/src/core/unit.c index 1165d4ea8b0..44b9f66e422 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -15,6 +15,7 @@ #include "bus-util.h" #include "cgroup-setup.h" #include "cgroup-util.h" +#include "core-varlink.h" #include "dbus-unit.h" #include "dbus.h" #include "dropin.h" @@ -2592,6 +2593,18 @@ void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, UnitNotifyFlag * the bus queue, so that any job change signal queued will force out the unit change signal first. */ unit_add_to_dbus_queue(u); + /* Update systemd-oomd on the property/state change */ + if (os != ns) { + /* Always send an update if the unit is going into an inactive state so systemd-oomd knows to stop + * monitoring. + * Also send an update whenever the unit goes active; this is to handle a case where an override file + * sets one of the ManagedOOM*= properties to "kill", then later removes it. systemd-oomd needs to + * know to stop monitoring when the unit changes from "kill" -> "auto" on daemon-reload, but we don't + * have the information on the property. Thus, indiscriminately send an update. */ + if (UNIT_IS_INACTIVE_OR_FAILED(ns) || ns == UNIT_ACTIVE) + (void) manager_varlink_send_managed_oom_update(u); + } + /* Update timestamps for state changes */ if (!MANAGER_IS_RELOADING(m)) { dual_timestamp_get(&u->state_change_timestamp); diff --git a/src/shared/varlink.h b/src/shared/varlink.h index 06a34b480d4..030db39b2f5 100644 --- a/src/shared/varlink.h +++ b/src/shared/varlink.h @@ -171,3 +171,4 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(VarlinkServer *, varlink_server_unref); #define VARLINK_ERROR_METHOD_NOT_FOUND "org.varlink.service.MethodNotFound" #define VARLINK_ERROR_METHOD_NOT_IMPLEMENTED "org.varlink.service.MethodNotImplemented" #define VARLINK_ERROR_INVALID_PARAMETER "org.varlink.service.InvalidParameter" +#define VARLINK_ERROR_SUBSCRIPTION_TAKEN "org.varlink.service.SubscriptionTaken"