]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
core: Rework recursive freeze/thaw
authorAdrian Vovk <adrianvovk@gmail.com>
Sun, 21 Jan 2024 20:05:20 +0000 (15:05 -0500)
committerAdrian Vovk <adrianvovk@gmail.com>
Tue, 30 Jan 2024 16:18:15 +0000 (11:18 -0500)
This commit overhauls the way freeze/thaw works recursively:

First, it introduces new FreezerActions that are like the existing
FREEZE and THAW but indicate that the action was initiated by a parent
unit. We also refactored the code to pass these FreezerActions through
the whole call stack so that we can make use of them. FreezerState was
extended similarly, to be able to differentiate between a unit that's
frozen manually and a unit that's frozen because a parent is frozen.

Next, slices were changed to check recursively that all their child
units can be frozen before it attempts to freeze them. This is different
from the previous behavior, that would just check if the unit's type
supported freezing at all. This cleans up the code, and also ensures
that the behavior of slices corresponds to the unit's actual ability
to be frozen

Next, we make it so that if you FREEZE a slice, it'll PARENT_FREEZE
all of its children. Similarly, if you THAW a slice it will PARENT_THAW
its children.

Finally, we use the new states available to us to refactor the code
that actually does the cgroup freezing. The code now looks at the unit's
existing freezer state and the action being requested, and decides what
next state is most appropriate. Then it puts the unit in that state.
For instance, a RUNNING unit with a request to PARENT_FREEZE will
put the unit into the PARENT_FREEZING state. As another example, a
FROZEN unit who's parent is also FROZEN will transition to
PARENT_FROZEN in response to a request to THAW.

Fixes https://github.com/systemd/systemd/issues/30640
Fixes https://github.com/systemd/systemd/issues/15850

src/basic/unit-def.c
src/basic/unit-def.h
src/core/cgroup.c
src/core/cgroup.h
src/core/dbus-unit.c
src/core/scope.c
src/core/service.c
src/core/slice.c
src/core/unit.c
src/core/unit.h
test/units/testsuite-38.sh

index 908c0cd03f20f6c6c69569aba08a6c6a5709990a..9bc4920998e3c82a98e5e18d7da26c25673da041 100644 (file)
@@ -117,14 +117,33 @@ static const char* const unit_active_state_table[_UNIT_ACTIVE_STATE_MAX] = {
 DEFINE_STRING_TABLE_LOOKUP(unit_active_state, UnitActiveState);
 
 static const char* const freezer_state_table[_FREEZER_STATE_MAX] = {
-        [FREEZER_RUNNING]  = "running",
-        [FREEZER_FREEZING] = "freezing",
-        [FREEZER_FROZEN]   = "frozen",
-        [FREEZER_THAWING]  = "thawing",
+        [FREEZER_RUNNING]            = "running",
+        [FREEZER_FREEZING]           = "freezing",
+        [FREEZER_FREEZING_BY_PARENT] = "freezing-by-parent",
+        [FREEZER_FROZEN]             = "frozen",
+        [FREEZER_FROZEN_BY_PARENT]   = "frozen-by-parent",
+        [FREEZER_THAWING]            = "thawing",
 };
 
 DEFINE_STRING_TABLE_LOOKUP(freezer_state, FreezerState);
 
+/* Maps in-progress freezer states to the corresponding finished state */
+static const FreezerState freezer_state_finish_table[_FREEZER_STATE_MAX] = {
+        [FREEZER_FREEZING]           = FREEZER_FROZEN,
+        [FREEZER_FREEZING_BY_PARENT] = FREEZER_FROZEN_BY_PARENT,
+        [FREEZER_THAWING]            = FREEZER_RUNNING,
+
+        /* Finished states trivially map to themselves */
+        [FREEZER_RUNNING]            = FREEZER_RUNNING,
+        [FREEZER_FROZEN]             = FREEZER_FROZEN,
+        [FREEZER_FROZEN_BY_PARENT]   = FREEZER_FROZEN_BY_PARENT,
+};
+
+FreezerState freezer_state_finish(FreezerState state) {
+        assert(state >= 0 && state < _FREEZER_STATE_MAX);
+        return freezer_state_finish_table[state];
+}
+
 static const char* const unit_marker_table[_UNIT_MARKER_MAX] = {
         [UNIT_MARKER_NEEDS_RELOAD]  = "needs-reload",
         [UNIT_MARKER_NEEDS_RESTART] = "needs-restart",
index 6627da5614c24b6a40916afd7d864679af986ed2..8e73e286da125b4c9a5cc509e5553b36230f8eb1 100644 (file)
@@ -53,8 +53,10 @@ typedef enum UnitActiveState {
 
 typedef enum FreezerState {
         FREEZER_RUNNING,
-        FREEZER_FREEZING,
+        FREEZER_FREEZING, /* freezing due to user request */
         FREEZER_FROZEN,
+        FREEZER_FREEZING_BY_PARENT, /* freezing as a result of parent slice freezing */
+        FREEZER_FROZEN_BY_PARENT,
         FREEZER_THAWING,
         _FREEZER_STATE_MAX,
         _FREEZER_STATE_INVALID = -EINVAL,
@@ -297,6 +299,7 @@ UnitActiveState unit_active_state_from_string(const char *s) _pure_;
 
 const char *freezer_state_to_string(FreezerState i) _const_;
 FreezerState freezer_state_from_string(const char *s) _pure_;
+FreezerState freezer_state_finish(FreezerState i) _const_;
 
 const char *unit_marker_to_string(UnitMarker m) _const_;
 UnitMarker unit_marker_from_string(const char *s) _pure_;
index e7585c3c0189467dc749e971023a7b91a6f8f377..5520118026d39cff924bd8397252b691c5a1de66 100644 (file)
@@ -3897,8 +3897,10 @@ static int unit_check_cgroup_events(Unit *u) {
                         unit_add_to_cgroup_empty_queue(u);
         }
 
-        /* Disregard freezer state changes due to operations not initiated by us */
-        if (values[1] && IN_SET(u->freezer_state, FREEZER_FREEZING, FREEZER_THAWING)) {
+        /* Disregard freezer state changes due to operations not initiated by us.
+         * See: https://github.com/systemd/systemd/pull/13512/files#r416469963 and
+         *      https://github.com/systemd/systemd/pull/13512#issuecomment-573007207 */
+        if (values[1] && IN_SET(u->freezer_state, FREEZER_FREEZING, FREEZER_FREEZING_BY_PARENT, FREEZER_THAWING)) {
                 if (streq(values[1], "0"))
                         unit_thawed(u);
                 else
@@ -4891,66 +4893,87 @@ void manager_invalidate_startup_units(Manager *m) {
                 unit_invalidate_cgroup(u, CGROUP_MASK_CPU|CGROUP_MASK_IO|CGROUP_MASK_BLKIO|CGROUP_MASK_CPUSET);
 }
 
-int unit_cgroup_freezer_action(Unit *u, FreezerAction action) {
-        _cleanup_free_ char *path = NULL;
-        FreezerState target, kernel = _FREEZER_STATE_INVALID;
-        int r, ret;
+static int unit_cgroup_freezer_kernel_state(Unit *u, FreezerState *ret) {
+        _cleanup_free_ char *val = NULL;
+        FreezerState s;
+        int r;
 
         assert(u);
-        assert(IN_SET(action, FREEZER_FREEZE, FREEZER_THAW));
+        assert(ret);
 
-        if (!cg_freezer_supported())
-                return 0;
+        r = cg_get_keyed_attribute(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path, "cgroup.events",
+                                   STRV_MAKE("frozen"), &val);
+        if (IN_SET(r, -ENOENT, -ENXIO))
+                return -ENODATA;
+        if (r < 0)
+                return r;
 
-        /* Ignore all requests to thaw init.scope or -.slice and reject all requests to freeze them */
-        if (unit_has_name(u, SPECIAL_ROOT_SLICE) || unit_has_name(u, SPECIAL_INIT_SCOPE))
-                return action == FREEZER_FREEZE ? -EPERM : 0;
+        if (streq(val, "0"))
+                s = FREEZER_RUNNING;
+        else if (streq(val, "1"))
+                s = FREEZER_FROZEN;
+        else {
+                log_unit_debug_errno(u, SYNTHETIC_ERRNO(EINVAL), "Unexpected cgroup frozen state: %s", val);
+                s = _FREEZER_STATE_INVALID;
+        }
 
-        if (!u->cgroup_realized)
-                return -EBUSY;
+        *ret = s;
+        return 0;
+}
 
-        if (action == FREEZER_THAW) {
-                Unit *slice = UNIT_GET_SLICE(u);
+int unit_cgroup_freezer_action(Unit *u, FreezerAction action) {
+        _cleanup_free_ char *path = NULL;
+        FreezerState target, current, next;
+        int r;
 
-                if (slice) {
-                        r = unit_cgroup_freezer_action(slice, FREEZER_THAW);
-                        if (r < 0)
-                                return log_unit_error_errno(u, r, "Failed to thaw slice %s of unit: %m", slice->id);
-                }
-        }
+        assert(u);
+        assert(IN_SET(action, FREEZER_FREEZE, FREEZER_PARENT_FREEZE,
+                              FREEZER_THAW, FREEZER_PARENT_THAW));
+
+        if (!cg_freezer_supported() || !u->cgroup_realized)
+                return 0;
 
-        target = action == FREEZER_FREEZE ? FREEZER_FROZEN : FREEZER_RUNNING;
+        unit_next_freezer_state(u, action, &next, &target);
 
-        r = unit_freezer_state_kernel(u, &kernel);
+        r = unit_cgroup_freezer_kernel_state(u, &current);
         if (r < 0)
-                log_unit_debug_errno(u, r, "Failed to obtain cgroup freezer state: %m");
+                return r;
 
-        if (target == kernel) {
-                u->freezer_state = target;
-                if (action == FREEZER_FREEZE)
-                        return 0;
-                ret = 0;
-        } else
-                ret = 1;
+        if (current == target)
+                next = freezer_state_finish(next);
+        else if (IN_SET(next, FREEZER_FROZEN, FREEZER_FROZEN_BY_PARENT, FREEZER_RUNNING)) {
+                /* We're transitioning into a finished state, which implies that the cgroup's
+                 * current state already matches the target and thus we'd return 0. But, reality
+                 * shows otherwise. This indicates that our freezer_state tracking has diverged
+                 * from the real state of the cgroup, which can happen if someone meddles with the
+                 * cgroup from underneath us. This really shouldn't happen during normal operation,
+                 * though. So, let's warn about it and fix up the state to be valid */
+
+                log_unit_warning(u, "Unit wants to transition to %s freezer state but cgroup is unexpectedly %s, fixing up.",
+                                 freezer_state_to_string(next), freezer_state_to_string(current) ?: "(invalid)");
+
+                if (next == FREEZER_FROZEN)
+                        next = FREEZER_FREEZING;
+                else if (next == FREEZER_FROZEN_BY_PARENT)
+                        next = FREEZER_FREEZING_BY_PARENT;
+                else if (next == FREEZER_RUNNING)
+                        next = FREEZER_THAWING;
+        }
 
         r = cg_get_path(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path, "cgroup.freeze", &path);
         if (r < 0)
                 return r;
 
-        log_unit_debug(u, "%s unit.", action == FREEZER_FREEZE ? "Freezing" : "Thawing");
-
-        if (target != kernel) {
-                if (action == FREEZER_FREEZE)
-                        u->freezer_state = FREEZER_FREEZING;
-                else
-                        u->freezer_state = FREEZER_THAWING;
-        }
+        log_unit_debug(u, "Unit freezer state was %s, now %s.",
+                       freezer_state_to_string(u->freezer_state),
+                       freezer_state_to_string(next));
 
-        r = write_string_file(path, one_zero(action == FREEZER_FREEZE), WRITE_STRING_FILE_DISABLE_BUFFER);
+        r = write_string_file(path, one_zero(target == FREEZER_FROZEN), WRITE_STRING_FILE_DISABLE_BUFFER);
         if (r < 0)
                 return r;
 
-        return ret;
+        u->freezer_state = next;
+        return target != current;
 }
 
 int unit_get_cpuset(Unit *u, CPUSet *cpus, const char *name) {
@@ -4989,17 +5012,10 @@ static const char* const cgroup_device_policy_table[_CGROUP_DEVICE_POLICY_MAX] =
 
 DEFINE_STRING_TABLE_LOOKUP(cgroup_device_policy, CGroupDevicePolicy);
 
-static const char* const freezer_action_table[_FREEZER_ACTION_MAX] = {
-        [FREEZER_FREEZE] = "freeze",
-        [FREEZER_THAW] = "thaw",
-};
-
-DEFINE_STRING_TABLE_LOOKUP(freezer_action, FreezerAction);
-
 static const char* const cgroup_pressure_watch_table[_CGROUP_PRESSURE_WATCH_MAX] = {
-        [CGROUP_PRESSURE_WATCH_OFF] = "off",
+        [CGROUP_PRESSURE_WATCH_OFF]  = "off",
         [CGROUP_PRESSURE_WATCH_AUTO] = "auto",
-        [CGROUP_PRESSURE_WATCH_ON] = "on",
+        [CGROUP_PRESSURE_WATCH_ON]   = "on",
         [CGROUP_PRESSURE_WATCH_SKIP] = "skip",
 };
 
index c56979de01fd331564b2af6716b2ae3aa5111892..841792805ce9f68c23199326daf496b837044457 100644 (file)
@@ -53,7 +53,9 @@ typedef enum CGroupDevicePolicy {
 
 typedef enum FreezerAction {
         FREEZER_FREEZE,
+        FREEZER_PARENT_FREEZE,
         FREEZER_THAW,
+        FREEZER_PARENT_THAW,
 
         _FREEZER_ACTION_MAX,
         _FREEZER_ACTION_INVALID = -EINVAL,
@@ -433,9 +435,6 @@ bool unit_cgroup_delegate(Unit *u);
 int unit_get_cpuset(Unit *u, CPUSet *cpus, const char *name);
 int unit_cgroup_freezer_action(Unit *u, FreezerAction action);
 
-const char* freezer_action_to_string(FreezerAction a) _const_;
-FreezerAction freezer_action_from_string(const char *s) _pure_;
-
 const char* cgroup_pressure_watch_to_string(CGroupPressureWatch a) _const_;
 CGroupPressureWatch cgroup_pressure_watch_from_string(const char *s) _pure_;
 
index 7aa44b2393e445ec83d050473bad4c249e5fe8da..6234a638d193f17046a388d2b833f1debb8e2d2d 100644 (file)
@@ -723,7 +723,6 @@ int bus_unit_method_clean(sd_bus_message *message, void *userdata, sd_bus_error
 
 static int bus_unit_method_freezer_generic(sd_bus_message *message, void *userdata, sd_bus_error *error, FreezerAction action) {
         const char* perm;
-        int (*method)(Unit*);
         Unit *u = ASSERT_PTR(userdata);
         bool reply_no_delay = false;
         int r;
@@ -731,13 +730,7 @@ static int bus_unit_method_freezer_generic(sd_bus_message *message, void *userda
         assert(message);
         assert(IN_SET(action, FREEZER_FREEZE, FREEZER_THAW));
 
-        if (action == FREEZER_FREEZE) {
-                perm = "stop";
-                method = unit_freeze;
-        } else {
-                perm = "start";
-                method = unit_thaw;
-        }
+        perm = action == FREEZER_FREEZE ? "stop" : "start";
 
         r = mac_selinux_unit_access_check(u, message, perm, error);
         if (r < 0)
@@ -754,15 +747,17 @@ static int bus_unit_method_freezer_generic(sd_bus_message *message, void *userda
         if (r == 0)
                 return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */
 
-        r = method(u);
+        r = unit_freezer_action(u, action);
         if (r == -EOPNOTSUPP)
-                return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, "Unit '%s' does not support freezing.", u->id);
+                return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, "Unit does not support freeze/thaw.");
         if (r == -EBUSY)
                 return sd_bus_error_set(error, BUS_ERROR_UNIT_BUSY, "Unit has a pending job.");
         if (r == -EHOSTDOWN)
                 return sd_bus_error_set(error, BUS_ERROR_UNIT_INACTIVE, "Unit is inactive.");
         if (r == -EALREADY)
-                return sd_bus_error_setf(error, SD_BUS_ERROR_FAILED, "Previously requested freezer operation for unit '%s' is still in progress.", u->id);
+                return sd_bus_error_setf(error, SD_BUS_ERROR_FAILED, "Previously requested freezer operation for unit is still in progress.");
+        if (r == -ECHILD)
+                return sd_bus_error_setf(error, SD_BUS_ERROR_FAILED, "Unit is frozen by a parent slice.");
         if (r < 0)
                 return r;
         if (r == 0)
index e4c27da91d23541122cef739219ce36eb8677c86..3a671d5733857451d9baccda07899b0cff00aea8 100644 (file)
@@ -804,8 +804,7 @@ const UnitVTable scope_vtable = {
         .start = scope_start,
         .stop = scope_stop,
 
-        .freeze = unit_freeze_vtable_common,
-        .thaw = unit_thaw_vtable_common,
+        .freezer_action = unit_cgroup_freezer_action,
 
         .get_timeout = scope_get_timeout,
 
index aacfe0d57e855a8c2268f9e9122b79d9c79adea5..087ce0d03dbe5110f28c558f4b699b74ef25e157 100644 (file)
@@ -5160,8 +5160,7 @@ const UnitVTable service_vtable = {
         .clean = service_clean,
         .can_clean = service_can_clean,
 
-        .freeze = unit_freeze_vtable_common,
-        .thaw = unit_thaw_vtable_common,
+        .freezer_action = unit_cgroup_freezer_action,
 
         .serialize = service_serialize,
         .deserialize_item = service_deserialize_item,
index fb4f23c10690385aabecdcb11ebc13cd66fa8704..e9b2683912c61cf011e8de13e27a0430f32622a6 100644 (file)
@@ -347,46 +347,47 @@ static void slice_enumerate_perpetual(Manager *m) {
                 (void) slice_make_perpetual(m, SPECIAL_SYSTEM_SLICE, NULL);
 }
 
-static bool slice_freezer_action_supported_by_children(Unit *s) {
+static bool slice_can_freeze(Unit *s) {
         Unit *member;
 
         assert(s);
 
-        UNIT_FOREACH_DEPENDENCY(member, s, UNIT_ATOM_SLICE_OF) {
-
-                if (member->type == UNIT_SLICE &&
-                    !slice_freezer_action_supported_by_children(member))
+        UNIT_FOREACH_DEPENDENCY(member, s, UNIT_ATOM_SLICE_OF)
+                if (!unit_can_freeze(member))
                         return false;
-
-                if (!UNIT_VTABLE(member)->freeze)
-                        return false;
-        }
-
         return true;
 }
 
 static int slice_freezer_action(Unit *s, FreezerAction action) {
+        FreezerAction child_action;
         Unit *member;
         int r;
 
         assert(s);
-        assert(IN_SET(action, FREEZER_FREEZE, FREEZER_THAW));
-
-        if (action == FREEZER_FREEZE && !slice_freezer_action_supported_by_children(s)) {
+        assert(IN_SET(action, FREEZER_FREEZE, FREEZER_PARENT_FREEZE,
+                      FREEZER_THAW, FREEZER_PARENT_THAW));
+
+        if (action == FREEZER_FREEZE && !slice_can_freeze(s)) {
+                /* We're intentionally only checking for FREEZER_FREEZE here and ignoring the
+                 * _BY_PARENT variant. If we're being frozen by parent, that means someone has
+                 * already checked if we can be frozen further up the call stack. No point to
+                 * redo that work */
                 log_unit_warning(s, "Requested freezer operation is not supported by all children of the slice");
                 return 0;
         }
 
-        UNIT_FOREACH_DEPENDENCY(member, s, UNIT_ATOM_SLICE_OF) {
-                if (!member->cgroup_realized)
-                        continue;
+        if (action == FREEZER_FREEZE)
+                child_action = FREEZER_PARENT_FREEZE;
+        else if (action == FREEZER_THAW)
+                child_action = FREEZER_PARENT_THAW;
+        else
+                child_action = action;
 
-                if (action == FREEZER_FREEZE)
-                        r = UNIT_VTABLE(member)->freeze(member);
-                else if (UNIT_VTABLE(member)->thaw)
-                        r = UNIT_VTABLE(member)->thaw(member);
+        UNIT_FOREACH_DEPENDENCY(member, s, UNIT_ATOM_SLICE_OF) {
+                if (UNIT_VTABLE(member)->freezer_action)
+                        r = UNIT_VTABLE(member)->freezer_action(member, child_action);
                 else
-                        /* Thawing is requested but no corresponding method is available, ignore. */
+                        /* Only thawing will reach here, since freezing checks for a method in can_freeze */
                         r = 0;
                 if (r < 0)
                         return r;
@@ -395,24 +396,6 @@ static int slice_freezer_action(Unit *s, FreezerAction action) {
         return unit_cgroup_freezer_action(s, action);
 }
 
-static int slice_freeze(Unit *s) {
-        assert(s);
-
-        return slice_freezer_action(s, FREEZER_FREEZE);
-}
-
-static int slice_thaw(Unit *s) {
-        assert(s);
-
-        return slice_freezer_action(s, FREEZER_THAW);
-}
-
-static bool slice_can_freeze(Unit *s) {
-        assert(s);
-
-        return slice_freezer_action_supported_by_children(s);
-}
-
 const UnitVTable slice_vtable = {
         .object_size = sizeof(Slice),
         .cgroup_context_offset = offsetof(Slice, cgroup_context),
@@ -436,8 +419,7 @@ const UnitVTable slice_vtable = {
         .start = slice_start,
         .stop = slice_stop,
 
-        .freeze = slice_freeze,
-        .thaw = slice_thaw,
+        .freezer_action = slice_freezer_action,
         .can_freeze = slice_can_freeze,
 
         .serialize = slice_serialize,
index d9e67067425811765c829db3c9326fc0104c252a..b4634e0a86a854a9c48ada146b4bce609c23d7f6 100644 (file)
@@ -905,32 +905,6 @@ FreezerState unit_freezer_state(Unit *u) {
         return u->freezer_state;
 }
 
-int unit_freezer_state_kernel(Unit *u, FreezerState *ret) {
-        char *values[1] = {};
-        int r;
-
-        assert(u);
-
-        r = cg_get_keyed_attribute(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path, "cgroup.events",
-                                   STRV_MAKE("frozen"), values);
-        if (r < 0)
-                return r;
-
-        r = _FREEZER_STATE_INVALID;
-
-        if (values[0])  {
-                if (streq(values[0], "0"))
-                        r = FREEZER_RUNNING;
-                else if (streq(values[0], "1"))
-                        r = FREEZER_FROZEN;
-        }
-
-        free(values[0]);
-        *ret = r;
-
-        return 0;
-}
-
 UnitActiveState unit_active_state(Unit *u) {
         assert(u);
 
@@ -6147,19 +6121,98 @@ bool unit_can_isolate_refuse_manual(Unit *u) {
         return unit_can_isolate(u) && !u->refuse_manual_start;
 }
 
+void unit_next_freezer_state(Unit *u, FreezerAction action, FreezerState *ret, FreezerState *ret_target) {
+        Unit *slice;
+        FreezerState curr, parent, next, tgt;
+
+        assert(u);
+        assert(IN_SET(action, FREEZER_FREEZE, FREEZER_PARENT_FREEZE,
+                              FREEZER_THAW, FREEZER_PARENT_THAW));
+        assert(ret);
+        assert(ret_target);
+
+        /* This function determintes the correct freezer state transitions for a unit
+         * given the action being requested. It returns the next state, and also the "target",
+         * which is either FREEZER_FROZEN or FREEZER_RUNNING, depending on what actual state we
+         * ultimately want to achieve. */
+
+         curr = u->freezer_state;
+         slice = UNIT_GET_SLICE(u);
+         if (slice)
+                parent = slice->freezer_state;
+         else
+                parent = FREEZER_RUNNING;
+
+        if (action == FREEZER_FREEZE) {
+                /* We always "promote" a freeze initiated by parent into a normal freeze */
+                if (IN_SET(curr, FREEZER_FROZEN, FREEZER_FROZEN_BY_PARENT))
+                        next = FREEZER_FROZEN;
+                else
+                        next = FREEZER_FREEZING;
+        } else if (action == FREEZER_THAW) {
+                /* Thawing is the most complicated operation here, because we can't thaw a unit
+                 * if its parent is frozen. So we instead "demote" a normal freeze into a freeze
+                 * initiated by parent if the parent is frozen */
+                if (IN_SET(curr, FREEZER_RUNNING, FREEZER_THAWING, FREEZER_FREEZING_BY_PARENT, FREEZER_FROZEN_BY_PARENT))
+                        next = curr;
+                else if (curr == FREEZER_FREEZING) {
+                        if (IN_SET(parent, FREEZER_RUNNING, FREEZER_THAWING))
+                                next = FREEZER_THAWING;
+                        else
+                                next = FREEZER_FREEZING_BY_PARENT;
+                } else {
+                        assert(curr == FREEZER_FROZEN);
+                        if (IN_SET(parent, FREEZER_RUNNING, FREEZER_THAWING))
+                                next = FREEZER_THAWING;
+                        else
+                                next = FREEZER_FROZEN_BY_PARENT;
+                }
+        } else if (action == FREEZER_PARENT_FREEZE) {
+                /* We need to avoid accidentally demoting units frozen manually */
+                if (IN_SET(curr, FREEZER_FREEZING, FREEZER_FROZEN, FREEZER_FROZEN_BY_PARENT))
+                        next = curr;
+                else
+                        next = FREEZER_FREEZING_BY_PARENT;
+        } else {
+                assert(action == FREEZER_PARENT_THAW);
+
+                /* We don't want to thaw units from a parent if they were frozen
+                 * manually, so for such units this action is a no-op */
+                if (IN_SET(curr, FREEZER_RUNNING, FREEZER_FREEZING, FREEZER_FROZEN))
+                        next = curr;
+                else
+                        next = FREEZER_THAWING;
+        }
+
+        tgt = freezer_state_finish(next);
+        if (tgt == FREEZER_FROZEN_BY_PARENT)
+                tgt = FREEZER_FROZEN;
+        assert(IN_SET(tgt, FREEZER_RUNNING, FREEZER_FROZEN));
+
+        *ret = next;
+        *ret_target = tgt;
+}
+
 bool unit_can_freeze(Unit *u) {
         assert(u);
 
+        if (unit_has_name(u, SPECIAL_ROOT_SLICE) || unit_has_name(u, SPECIAL_INIT_SCOPE))
+                return false;
+
         if (UNIT_VTABLE(u)->can_freeze)
                 return UNIT_VTABLE(u)->can_freeze(u);
 
-        return UNIT_VTABLE(u)->freeze;
+        return UNIT_VTABLE(u)->freezer_action;
 }
 
 void unit_frozen(Unit *u) {
         assert(u);
 
-        u->freezer_state = FREEZER_FROZEN;
+        u->freezer_state = u->freezer_state == FREEZER_FREEZING_BY_PARENT
+                           ? FREEZER_FROZEN_BY_PARENT
+                           : FREEZER_FROZEN;
+
+        log_unit_debug(u, "Unit now %s.", freezer_state_to_string(u->freezer_state));
 
         bus_unit_send_pending_freezer_message(u, false);
 }
@@ -6169,19 +6222,19 @@ void unit_thawed(Unit *u) {
 
         u->freezer_state = FREEZER_RUNNING;
 
+        log_unit_debug(u, "Unit thawed.");
+
         bus_unit_send_pending_freezer_message(u, false);
 }
 
-static int unit_freezer_action(Unit *u, FreezerAction action) {
+int unit_freezer_action(Unit *u, FreezerAction action) {
         UnitActiveState s;
-        int (*method)(Unit*);
         int r;
 
         assert(u);
         assert(IN_SET(action, FREEZER_FREEZE, FREEZER_THAW));
 
-        method = action == FREEZER_FREEZE ? UNIT_VTABLE(u)->freeze : UNIT_VTABLE(u)->thaw;
-        if (!method || !cg_freezer_supported())
+        if (!cg_freezer_supported() || !unit_can_freeze(u))
                 return -EOPNOTSUPP;
 
         if (u->job)
@@ -6194,36 +6247,21 @@ static int unit_freezer_action(Unit *u, FreezerAction action) {
         if (s != UNIT_ACTIVE)
                 return -EHOSTDOWN;
 
-        if ((IN_SET(u->freezer_state, FREEZER_FREEZING, FREEZER_THAWING) && action == FREEZER_FREEZE) ||
-            (u->freezer_state == FREEZER_THAWING && action == FREEZER_THAW))
+        if (action == FREEZER_FREEZE && IN_SET(u->freezer_state, FREEZER_FREEZING, FREEZER_FREEZING_BY_PARENT))
+                return -EALREADY;
+        if (action == FREEZER_THAW && u->freezer_state == FREEZER_THAWING)
                 return -EALREADY;
+        if (action == FREEZER_THAW && IN_SET(u->freezer_state, FREEZER_FREEZING_BY_PARENT, FREEZER_FROZEN_BY_PARENT))
+                return -ECHILD;
 
-        r = method(u);
+        r = UNIT_VTABLE(u)->freezer_action(u, action);
         if (r <= 0)
                 return r;
 
-        assert(IN_SET(u->freezer_state, FREEZER_FREEZING, FREEZER_THAWING));
-
+        assert(IN_SET(u->freezer_state, FREEZER_FREEZING, FREEZER_FREEZING_BY_PARENT, FREEZER_THAWING));
         return 1;
 }
 
-int unit_freeze(Unit *u) {
-        return unit_freezer_action(u, FREEZER_FREEZE);
-}
-
-int unit_thaw(Unit *u) {
-        return unit_freezer_action(u, FREEZER_THAW);
-}
-
-/* Wrappers around low-level cgroup freezer operations common for service and scope units */
-int unit_freeze_vtable_common(Unit *u) {
-        return unit_cgroup_freezer_action(u, FREEZER_FREEZE);
-}
-
-int unit_thaw_vtable_common(Unit *u) {
-        return unit_cgroup_freezer_action(u, FREEZER_THAW);
-}
-
 Condition *unit_find_failed_condition(Unit *u) {
         Condition *failed_trigger = NULL;
         bool has_succeeded_trigger = false;
index fdaa3b8a8ef42cbecbf8ec77c8fbfbd2a27d2599..ae22c112b7c1cbf083ecf47fc7c1f924f846d084 100644 (file)
@@ -642,9 +642,9 @@ typedef struct UnitVTable {
         /* Clear out the various runtime/state/cache/logs/configuration data */
         int (*clean)(Unit *u, ExecCleanMask m);
 
-        /* Freeze the unit */
-        int (*freeze)(Unit *u);
-        int (*thaw)(Unit *u);
+        /* Freeze or thaw the unit. Returns > 0 to indicate that the request will be handled asynchronously; unit_frozen
+         * or unit_thawed should be called once the operation is done. Returns 0 if done successfully, or < 0 on error. */
+        int (*freezer_action)(Unit *u, FreezerAction a);
         bool (*can_freeze)(Unit *u);
 
         /* Return which kind of data can be cleaned */
@@ -912,7 +912,6 @@ bool unit_has_name(const Unit *u, const char *name);
 
 UnitActiveState unit_active_state(Unit *u);
 FreezerState unit_freezer_state(Unit *u);
-int unit_freezer_state_kernel(Unit *u, FreezerState *ret);
 
 const char* unit_sub_state_to_string(Unit *u);
 
@@ -1095,15 +1094,11 @@ bool unit_can_stop_refuse_manual(Unit *u);
 bool unit_can_isolate_refuse_manual(Unit *u);
 
 bool unit_can_freeze(Unit *u);
-int unit_freeze(Unit *u);
+int unit_freezer_action(Unit *u, FreezerAction action);
+void unit_next_freezer_state(Unit *u, FreezerAction a, FreezerState *ret, FreezerState *ret_tgt);
 void unit_frozen(Unit *u);
-
-int unit_thaw(Unit *u);
 void unit_thawed(Unit *u);
 
-int unit_freeze_vtable_common(Unit *u);
-int unit_thaw_vtable_common(Unit *u);
-
 Condition *unit_find_failed_condition(Unit *u);
 
 int unit_arm_timer(Unit *u, sd_event_source **source, bool relative, usec_t usec, sd_event_time_handler_t handler);
index 5fc87fca7b0a1b2206e7b262cd21a6d3a49ed3fc..0737b9c199b1399d0c731adbcd98229a8a93d750 100755 (executable)
@@ -105,7 +105,14 @@ check_freezer_state() {
 }
 
 check_cgroup_state() {
-    grep -q "frozen $2" /sys/fs/cgroup/system.slice/"$1"/cgroup.events
+    # foo.unit -> /system.slice/foo.unit/
+    # foo.slice/ -> /foo.slice/./
+    # foo.slice/foo.unit -> /foo.slice/foo.unit/
+    local slice unit
+    unit="${1##*/}"
+    slice="${1%"$unit"}"
+    slice="${slice%/}"
+    grep -q "frozen $2" /sys/fs/cgroup/"${slice:-system.slice}"/"${unit:-.}"/cgroup.events
 }
 
 testcase_dbus_api() {
@@ -224,20 +231,94 @@ testcase_recursive() {
 
     echo "Test recursive freezing:"
 
-    echo -n "  - freeze: "
+    echo -n "  - freeze/thaw parent: "
     systemctl freeze "$slice"
-    check_freezer_state "${slice}" "frozen"
-    check_freezer_state "${unit}" "frozen"
-    grep -q "frozen 1" /sys/fs/cgroup/"${slice}"/cgroup.events
-    grep -q "frozen 1" /sys/fs/cgroup/"${slice}"/"${unit}"/cgroup.events
+    check_freezer_state "$slice" "frozen"
+    check_freezer_state "$unit" "frozen-by-parent"
+    check_cgroup_state "$slice/" 1
+    check_cgroup_state "$slice/$unit" 1
+    systemctl thaw "$slice"
+    check_freezer_state "$slice" "running"
+    check_freezer_state "$unit" "running"
+    check_cgroup_state "$slice/" 0
+    check_cgroup_state "$slice/$unit" 0
     echo "[ OK ]"
 
-    echo -n "  - thaw: "
+    echo -n "  - child freeze/thaw during frozen parent: "
+    systemctl freeze "$slice"
+    check_freezer_state "$slice" "frozen"
+    check_freezer_state "$unit" "frozen-by-parent"
+    check_cgroup_state "$slice/" 1
+    check_cgroup_state "$slice/$unit" 1
+    systemctl freeze "$unit"
+    check_freezer_state "$slice" "frozen"
+    check_freezer_state "$unit" "frozen"
+    check_cgroup_state "$slice/" 1
+    check_cgroup_state "$slice/$unit" 1
+    systemctl thaw "$unit"
+    check_freezer_state "$slice" "frozen"
+    check_freezer_state "$unit" "frozen-by-parent"
+    check_cgroup_state "$slice/" 1
+    check_cgroup_state "$slice/$unit" 1
     systemctl thaw "$slice"
-    check_freezer_state "${unit}" "running"
-    check_freezer_state "${slice}" "running"
-    grep -q "frozen 0" /sys/fs/cgroup/"${slice}"/cgroup.events
-    grep -q "frozen 0" /sys/fs/cgroup/"${slice}"/"${unit}"/cgroup.events
+    check_freezer_state "$slice" "running"
+    check_freezer_state "$unit" "running"
+    check_cgroup_state "$slice/" 0
+    check_cgroup_state "$slice/$unit" 0
+    echo "[ OK ]"
+
+    echo -n "  - pre-frozen child not thawed by parent: "
+    systemctl freeze "$unit"
+    check_freezer_state "$slice" "running"
+    check_freezer_state "$unit" "frozen"
+    check_cgroup_state "$slice/" 0
+    check_cgroup_state "$slice/$unit" 1
+    systemctl freeze "$slice"
+    check_freezer_state "$slice" "frozen"
+    check_freezer_state "$unit" "frozen"
+    check_cgroup_state "$slice/" 1
+    check_cgroup_state "$slice/$unit" 1
+    systemctl thaw "$slice"
+    check_freezer_state "$slice" "running"
+    check_freezer_state "$unit" "frozen"
+    check_cgroup_state "$slice/" 0
+    check_cgroup_state "$slice/$unit" 1
+    echo "[ OK ]"
+
+    echo -n "  - pre-frozen child demoted and thawed by parent: "
+    systemctl freeze "$slice"
+    check_freezer_state "$slice" "frozen"
+    check_freezer_state "$unit" "frozen"
+    check_cgroup_state "$slice/" 1
+    check_cgroup_state "$slice/$unit" 1
+    systemctl thaw "$unit"
+    check_freezer_state "$slice" "frozen"
+    check_freezer_state "$unit" "frozen-by-parent"
+    check_cgroup_state "$slice/" 1
+    check_cgroup_state "$slice/$unit" 1
+    systemctl thaw "$slice"
+    check_freezer_state "$slice" "running"
+    check_freezer_state "$unit" "running"
+    check_cgroup_state "$slice/" 0
+    check_cgroup_state "$slice/$unit" 0
+    echo "[ OK ]"
+
+    echo -n "  - child promoted and not thawed by parent: "
+    systemctl freeze "$slice"
+    check_freezer_state "$slice" "frozen"
+    check_freezer_state "$unit" "frozen-by-parent"
+    check_cgroup_state "$slice/" 1
+    check_cgroup_state "$slice/$unit" 1
+    systemctl freeze "$unit"
+    check_freezer_state "$slice" "frozen"
+    check_freezer_state "$unit" "frozen"
+    check_cgroup_state "$slice/" 1
+    check_cgroup_state "$slice/$unit" 1
+    systemctl thaw "$slice"
+    check_freezer_state "$slice" "running"
+    check_freezer_state "$unit" "frozen"
+    check_cgroup_state "$slice/" 0
+    check_cgroup_state "$slice/$unit" 1
     echo "[ OK ]"
 
     systemctl stop "$unit"
@@ -255,36 +336,36 @@ testcase_preserve_state() {
     echo "Test that freezer state is preserved when recursive freezing is initiated from outside (e.g. by manager up the tree):"
 
     echo -n "  - freeze from outside: "
-    echo 1 >/sys/fs/cgroup/"${slice}"/cgroup.freeze
+    echo 1 >/sys/fs/cgroup/"$slice"/cgroup.freeze
     # Give kernel some time to freeze the slice
     sleep 1
 
     # Our state should not be affected
-    check_freezer_state "${slice}" "running"
-    check_freezer_state "${unit}" "running"
+    check_freezer_state "$slice" "running"
+    check_freezer_state "$unit" "running"
 
     # However actual kernel state should be frozen
-    grep -q "frozen 1" /sys/fs/cgroup/"${slice}"/cgroup.events
-    grep -q "frozen 1" /sys/fs/cgroup/"${slice}"/"${unit}"/cgroup.events
+    check_cgroup_state "$slice/" 1
+    check_cgroup_state "$slice/$unit" 1
     echo "[ OK ]"
 
     echo -n "  - thaw from outside: "
-    echo 0 >/sys/fs/cgroup/"${slice}"/cgroup.freeze
+    echo 0 >/sys/fs/cgroup/"$slice"/cgroup.freeze
     sleep 1
 
-    check_freezer_state "${unit}" "running"
-    check_freezer_state "${slice}" "running"
-    grep -q "frozen 0" /sys/fs/cgroup/"${slice}"/cgroup.events
-    grep -q "frozen 0" /sys/fs/cgroup/"${slice}"/"${unit}"/cgroup.events
+    check_freezer_state "$unit" "running"
+    check_freezer_state "$slice" "running"
+    check_cgroup_state "$slice/" 0
+    check_cgroup_state "$slice/$unit" 0
     echo "[ OK ]"
 
     echo -n "  - thaw from outside while inner service is frozen: "
     systemctl freeze "$unit"
-    check_freezer_state "${unit}" "frozen"
-    echo 1 >/sys/fs/cgroup/"${slice}"/cgroup.freeze
-    echo 0 >/sys/fs/cgroup/"${slice}"/cgroup.freeze
-    check_freezer_state "${slice}" "running"
-    check_freezer_state "${unit}" "frozen"
+    check_freezer_state "$unit" "frozen"
+    echo 1 >/sys/fs/cgroup/"$slice"/cgroup.freeze
+    echo 0 >/sys/fs/cgroup/"$slice"/cgroup.freeze
+    check_freezer_state "$slice" "running"
+    check_freezer_state "$unit" "frozen"
     echo "[ OK ]"
 
     systemctl stop "$unit"