]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
cgroup: Add root slice to cgroup realization queue
authorMichal Koutný <mkoutny@suse.com>
Fri, 1 May 2020 12:00:42 +0000 (14:00 +0200)
committerMichal Koutný <mkoutny@suse.com>
Tue, 28 Jul 2020 13:49:24 +0000 (15:49 +0200)
When we're disabling controller on a direct child of root cgroup, we
forgot to add root slice into cgroup realization queue, which prevented
proper disabling of the controller (on unified hierarchy).

The mechanism relying on "bounce from bottom and propagate up" in
unit_create_cgroup doesn't work on unified hierarchy (leaves needn't be
enabled). Drop it as we rely on the ancestors to be queued -- that's now
intentional but was artifact of combining the two patches:

cb5e3bc37d ("cgroup: Don't explicitly check for member in UNIT_BEFORE") v240~78
65f6b6bdcb ("core: fix re-realization of cgroup siblings") v245-rc1~153^2

Fixes: #14917
src/core/cgroup.c

index f146f650559c4461d9ab83693cb68cde71b5646f..97785680a7cd129039a79f4bf8f8df4a43413454 100644 (file)
@@ -1858,23 +1858,6 @@ static int unit_create_cgroup(
                 if (r < 0)
                         log_unit_warning_errno(u, r, "Failed to enable/disable controllers on cgroup %s, ignoring: %m", u->cgroup_path);
 
-                /* If we just turned off a controller, this might release the controller for our parent too, let's
-                 * enqueue the parent for re-realization in that case again. */
-                if (UNIT_ISSET(u->slice)) {
-                        CGroupMask turned_off;
-
-                        turned_off = (u->cgroup_realized ? u->cgroup_enabled_mask & ~result_mask : 0);
-                        if (turned_off != 0) {
-                                Unit *parent;
-
-                                /* Force the parent to propagate the enable mask to the kernel again, by invalidating
-                                 * the controller we just turned off. */
-
-                                for (parent = UNIT_DEREF(u->slice); parent; parent = UNIT_DEREF(parent->slice))
-                                        unit_invalidate_cgroup(parent, turned_off);
-                        }
-                }
-
                 /* Remember what's actually enabled now */
                 u->cgroup_enabled_mask = result_mask;
         }
@@ -2317,8 +2300,7 @@ unsigned manager_dispatch_cgroup_realize_queue(Manager *m) {
 static void unit_add_siblings_to_cgroup_realize_queue(Unit *u) {
         Unit *slice;
 
-        /* This adds the siblings of the specified unit and the siblings of all parent units to the cgroup
-         * queue. (But neither the specified unit itself nor the parents.)
+        /* This adds the path from the specified unit to root slice to the queue and siblings at each level.
          *
          * Propagation of realization "side-ways" (i.e. towards siblings) is relevant on cgroup-v1 where
          * scheduling becomes very weird if two units that own processes reside in the same slice, but one is
@@ -2334,7 +2316,6 @@ static void unit_add_siblings_to_cgroup_realize_queue(Unit *u) {
                 void *v;
 
                 HASHMAP_FOREACH_KEY(v, m, slice->dependencies[UNIT_BEFORE], i) {
-
                         /* Skip units that have a dependency on the slice but aren't actually in it. */
                         if (UNIT_DEREF(m->slice) != slice)
                                 continue;
@@ -2360,6 +2341,9 @@ static void unit_add_siblings_to_cgroup_realize_queue(Unit *u) {
 
                 u = slice;
         }
+
+        /* Root slice comes last */
+        unit_add_to_cgroup_realize_queue(u);
 }
 
 int unit_realize_cgroup(Unit *u) {
@@ -2377,9 +2361,10 @@ int unit_realize_cgroup(Unit *u) {
          * get as much resources as all our group together. This call
          * will synchronously create the parent cgroups, but will
          * defer work on the siblings to the next event loop
-         * iteration. */
+         * iteration. When removing a realized controller, it may become unnecessary in ancestors,
+         * so we also ensure deferred bottom up (de)realization of ancestors.
+         */
 
-        /* Add all sibling slices to the cgroup queue. */
         unit_add_siblings_to_cgroup_realize_queue(u);
 
         /* And realize this one now (and apply the values) */