cgroup: Swap cgroup v1 deletion and migration

When we are about to derealize a controller on v1 cgroup, we first
attempt to delete the controller cgroup and migrate afterwards. This
doesn't work in practice because populated cgroup cannot be deleted.
Furthermore, we leave out slices from migration completely, so
(un)setting a control value on them won't realize their controller
cgroup.

Rework actual realization, unit_create_cgroup() becomes
unit_update_cgroup() and make sure that controller hierarchies are
reduced when given controller cgroup ceased to be needed.

Note that with this we introduce slight deviation between v1 and v2 code
-- when a descendant unit turns off a delegated controller, we attempt
to disable it in ancestor slices. On v2 this may fail (kernel enforced,
because of child cgroups using the controller), on v1 we'll migrate
whole subtree and trim the subhierachy. (Previously, we wouldn't take
away delegated controller, however, derealization was broken anyway.)

Fixes: #14149

diff --git a/src/core/cgroup.c b/src/core/cgroup.c
index 97785680a7cd129039a79f4bf8f8df4a43413454..6750bbbe1d4af895b030df8da1f826dc646e40d2 100644 (file)
--- a/src/core/cgroup.c
+++ b/src/core/cgroup.c
@@ -1629,7 +1629,10 @@ const char *unit_get_realized_cgroup_path(Unit *u, CGroupMask mask) {
 }
 
 static const char *migrate_callback(CGroupMask mask, void *userdata) {
-        return unit_get_realized_cgroup_path(userdata, mask);
+        /* If not realized at all, migrate to root ("").
+         * It may happen if we're upgrading from older version that didn't clean up.
+         */
+        return strempty(unit_get_realized_cgroup_path(userdata, mask));
 }
 
 char *unit_default_cgroup_path(const Unit *u) {
@@ -1820,13 +1823,14 @@ int unit_pick_cgroup_path(Unit *u) {
         return 0;
 }
 
-static int unit_create_cgroup(
+static int unit_update_cgroup(
                 Unit *u,
                 CGroupMask target_mask,
                 CGroupMask enable_mask,
                 ManagerState state) {
 
-        bool created;
+        bool created, is_root_slice;
+        CGroupMask migrate_mask = 0;
         int r;
 
         assert(u);
@@ -1849,7 +1853,9 @@ static int unit_create_cgroup(
         (void) unit_watch_cgroup(u);
         (void) unit_watch_cgroup_memory(u);
 
-        /* Preserve enabled controllers in delegated units, adjust others. */
+
+        /* For v2 we preserve enabled controllers in delegated units, adjust others,
+         * for v1 we figure out which controller hierarchies need migration. */
         if (created || !u->cgroup_realized || !unit_cgroup_delegate(u)) {
                 CGroupMask result_mask = 0;
 
@@ -1860,20 +1866,30 @@ static int unit_create_cgroup(
 
                 /* Remember what's actually enabled now */
                 u->cgroup_enabled_mask = result_mask;
+
+                migrate_mask = u->cgroup_realized_mask ^ target_mask;
         }
 
         /* Keep track that this is now realized */
         u->cgroup_realized = true;
         u->cgroup_realized_mask = target_mask;
 
-        if (u->type != UNIT_SLICE && !unit_cgroup_delegate(u)) {
+        /* Migrate processes in controller hierarchies both downwards (enabling) and upwards (disabling).
+         *
+         * Unnecessary controller cgroups are trimmed (after emptied by upward migration).
+         * We perform migration also with whole slices for cases when users don't care about leave
+         * granularity. Since delegated_mask is subset of target mask, we won't trim slice subtree containing
+         * delegated units.
+         */
+        if (cg_all_unified() == 0) {
+                r = cg_migrate_v1_controllers(u->manager->cgroup_supported, migrate_mask, u->cgroup_path, migrate_callback, u);
+                if (r < 0)
+                        log_unit_warning_errno(u, r, "Failed to migrate controller cgroups from %s, ignoring: %m", u->cgroup_path);
 
-                /* Then, possibly move things over, but not if
-                 * subgroups may contain processes, which is the case
-                 * for slice and delegation units. */
-                r = cg_migrate_everywhere(u->manager->cgroup_supported, u->cgroup_path, u->cgroup_path, migrate_callback, u);
+                is_root_slice = unit_has_name(u, SPECIAL_ROOT_SLICE);
+                r = cg_trim_v1_controllers(u->manager->cgroup_supported, ~target_mask, u->cgroup_path, !is_root_slice);
                 if (r < 0)
-                        log_unit_warning_errno(u, r, "Failed to migrate cgroup from to %s, ignoring: %m", u->cgroup_path);
+                        log_unit_warning_errno(u, r, "Failed to delete controller cgroups %s, ignoring: %m", u->cgroup_path);
         }
 
         /* Set attributes */
@@ -2136,7 +2152,7 @@ static int unit_realize_cgroup_now_enable(Unit *u, ManagerState state) {
         new_target_mask = u->cgroup_realized_mask | target_mask;
         new_enable_mask = u->cgroup_enabled_mask | enable_mask;
 
-        return unit_create_cgroup(u, new_target_mask, new_enable_mask, state);
+        return unit_update_cgroup(u, new_target_mask, new_enable_mask, state);
 }
 
 /* Controllers can only be disabled depth-first, from the leaves of the
@@ -2180,7 +2196,7 @@ static int unit_realize_cgroup_now_disable(Unit *u, ManagerState state) {
                 new_target_mask = m->cgroup_realized_mask & target_mask;
                 new_enable_mask = m->cgroup_enabled_mask & enable_mask;
 
-                r = unit_create_cgroup(m, new_target_mask, new_enable_mask, state);
+                r = unit_update_cgroup(m, new_target_mask, new_enable_mask, state);
                 if (r < 0)
                         return r;
         }
@@ -2259,7 +2275,7 @@ static int unit_realize_cgroup_now(Unit *u, ManagerState state) {
         }
 
         /* Now actually deal with the cgroup we were trying to realise and set attributes */
-        r = unit_create_cgroup(u, target_mask, enable_mask, state);
+        r = unit_update_cgroup(u, target_mask, enable_mask, state);
         if (r < 0)
                 return r;
 

diff --git a/src/shared/cgroup-setup.c b/src/shared/cgroup-setup.c
index e8398cbde5ba7fdba370453b4cdfc9a5b8d9da11..cd2ba6149e3f5a583d1eb7274d1c606b5f49de24 100644 (file)
--- a/src/shared/cgroup-setup.c
+++ b/src/shared/cgroup-setup.c
@@ -608,8 +608,6 @@ int cg_create_everywhere(CGroupMask supported, CGroupMask mask, const char *path
                 n = cgroup_controller_to_string(c);
                 if (FLAGS_SET(mask, bit))
                         (void) cg_create(n, path);
-                else
-                        (void) cg_trim(n, path, true);
 
                 done |= CGROUP_MASK_EXTEND_JOINED(bit);
         }
@@ -674,29 +672,20 @@ int cg_attach_many_everywhere(CGroupMask supported, const char *path, Set* pids,
         return r;
 }
 
-int cg_migrate_everywhere(CGroupMask supported, const char *from, const char *to, cg_migrate_callback_t to_callback, void *userdata) {
+int cg_migrate_v1_controllers(CGroupMask supported, CGroupMask mask, const char *from, cg_migrate_callback_t to_callback, void *userdata) {
         CGroupController c;
         CGroupMask done;
         int r = 0, q;
 
-        if (!path_equal(from, to))  {
-                r = cg_migrate_recursive(SYSTEMD_CGROUP_CONTROLLER, from, SYSTEMD_CGROUP_CONTROLLER, to, CGROUP_REMOVE);
-                if (r < 0)
-                        return r;
-        }
-
-        q = cg_all_unified();
-        if (q < 0)
-                return q;
-        if (q > 0)
-                return r;
+        assert(to_callback);
 
         supported &= CGROUP_MASK_V1;
+        mask = CGROUP_MASK_EXTEND_JOINED(mask);
         done = 0;
 
         for (c = 0; c < _CGROUP_CONTROLLER_MAX; c++) {
                 CGroupMask bit = CGROUP_CONTROLLER_TO_MASK(c);
-                const char *p = NULL;
+                const char *to = NULL;
 
                 if (!FLAGS_SET(supported, bit))
                         continue;
@@ -704,21 +693,20 @@ int cg_migrate_everywhere(CGroupMask supported, const char *from, const char *to
                 if (FLAGS_SET(done, bit))
                         continue;
 
-                if (to_callback)
-                        p = to_callback(bit, userdata);
-                if (!p)
-                        p = to;
+                if (!FLAGS_SET(mask, bit))
+                        continue;
 
-                (void) cg_migrate_recursive_fallback(SYSTEMD_CGROUP_CONTROLLER, to, cgroup_controller_to_string(c), p, 0);
-                done |= CGROUP_MASK_EXTEND_JOINED(bit);
+                to = to_callback(bit, userdata);
+
+                /* Remember first error and try continuing */
+                q = cg_migrate_recursive_fallback(SYSTEMD_CGROUP_CONTROLLER, from, cgroup_controller_to_string(c), to, 0);
+                r = (r < 0) ? r : q;
         }
 
         return r;
 }
 
 int cg_trim_everywhere(CGroupMask supported, const char *path, bool delete_root) {
-        CGroupController c;
-        CGroupMask done;
         int r, q;
 
         r = cg_trim(SYSTEMD_CGROUP_CONTROLLER, path, delete_root);
@@ -731,7 +719,16 @@ int cg_trim_everywhere(CGroupMask supported, const char *path, bool delete_root)
         if (q > 0)
                 return r;
 
+        return cg_trim_v1_controllers(supported, _CGROUP_MASK_ALL, path, delete_root);
+}
+
+int cg_trim_v1_controllers(CGroupMask supported, CGroupMask mask, const char *path, bool delete_root) {
+        CGroupController c;
+        CGroupMask done;
+        int r = 0, q;
+
         supported &= CGROUP_MASK_V1;
+        mask = CGROUP_MASK_EXTEND_JOINED(mask);
         done = 0;
 
         for (c = 0; c < _CGROUP_CONTROLLER_MAX; c++) {
@@ -743,7 +740,11 @@ int cg_trim_everywhere(CGroupMask supported, const char *path, bool delete_root)
                 if (FLAGS_SET(done, bit))
                         continue;
 
-                (void) cg_trim(cgroup_controller_to_string(c), path, delete_root);
+                if (FLAGS_SET(mask, bit)) {
+                        /* Remember first error and try continuing */
+                        q = cg_trim(cgroup_controller_to_string(c), path, delete_root);
+                        r = (r < 0) ? r : q;
+                }
                 done |= CGROUP_MASK_EXTEND_JOINED(bit);
         }
 

diff --git a/src/shared/cgroup-setup.h b/src/shared/cgroup-setup.h
index 6e9b6857d8af31551234c30a4b19c22487b708e6..72c2b6ee6a5d7691e8bba7f6749d4f004e53fb52 100644 (file)
--- a/src/shared/cgroup-setup.h
+++ b/src/shared/cgroup-setup.h
@@ -29,6 +29,7 @@ int cg_migrate_recursive_fallback(const char *cfrom, const char *pfrom, const ch
 int cg_create_everywhere(CGroupMask supported, CGroupMask mask, const char *path);
 int cg_attach_everywhere(CGroupMask supported, const char *path, pid_t pid, cg_migrate_callback_t callback, void *userdata);
 int cg_attach_many_everywhere(CGroupMask supported, const char *path, Set* pids, cg_migrate_callback_t callback, void *userdata);
-int cg_migrate_everywhere(CGroupMask supported, const char *from, const char *to, cg_migrate_callback_t callback, void *userdata);
+int cg_migrate_v1_controllers(CGroupMask supported, CGroupMask mask, const char *from, cg_migrate_callback_t to_callback, void *userdata);
 int cg_trim_everywhere(CGroupMask supported, const char *path, bool delete_root);
+int cg_trim_v1_controllers(CGroupMask supported, CGroupMask mask, const char *path, bool delete_root);
 int cg_enable_everywhere(CGroupMask supported, CGroupMask mask, const char *p, CGroupMask *ret_result_mask);