From 2aa57a655004c8f9fba48eeb71aa9a31fdf3c94b Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Fri, 24 Nov 2017 19:48:38 +0100
Subject: [PATCH] cgroup: when dispatching the cgroup realization queue, check
 again if we shall actually realize

We add units to the cgroup realization queue when propagating realizing
requests to sibling units, and when invalidating cgroup settings because
some cgroup setting changed. In the time between where we add the unit
to the queue until the cgroup is actually dispatched the unit's state
might have changed however, so that the unit doesn't actually need to be
realized anymore, for example because the unit went down. To handle
that, check the unit state again, if realization makes sense.

Redundant realization is usually not a problem, except when the unit is
not actually running, hence check exactly for that.
---
 src/core/cgroup.c | 43 ++++++++++++++++++++++++++++---------------
 1 file changed, 28 insertions(+), 15 deletions(-)

diff --git a/src/core/cgroup.c b/src/core/cgroup.c
index 8ae63e8615..51c52ccabe 100644
--- a/src/core/cgroup.c
+++ b/src/core/cgroup.c
@@ -1505,6 +1505,27 @@ static bool unit_has_mask_realized(
                  (!needs_bpf && u->cgroup_bpf_state == UNIT_CGROUP_BPF_OFF));
 }
 
+static void unit_add_to_cgroup_realize_queue(Unit *u) {
+        assert(u);
+
+        if (u->in_cgroup_realize_queue)
+                return;
+
+        LIST_PREPEND(cgroup_realize_queue, u->manager->cgroup_realize_queue, u);
+        u->in_cgroup_realize_queue = true;
+}
+
+static void unit_remove_from_cgroup_realize_queue(Unit *u) {
+        assert(u);
+
+        if (!u->in_cgroup_realize_queue)
+                return;
+
+        LIST_REMOVE(cgroup_realize_queue, u->manager->cgroup_realize_queue, u);
+        u->in_cgroup_realize_queue = false;
+}
+
+
 /* Check if necessary controllers and attributes for a unit are in place.
  *
  * If so, do nothing.
@@ -1518,10 +1539,7 @@ static int unit_realize_cgroup_now(Unit *u, ManagerState state) {
 
         assert(u);
 
-        if (u->in_cgroup_realize_queue) {
-                LIST_REMOVE(cgroup_realize_queue, u->manager->cgroup_realize_queue, u);
-                u->in_cgroup_realize_queue = false;
-        }
+        unit_remove_from_cgroup_realize_queue(u);
 
         target_mask = unit_get_target_mask(u);
         enable_mask = unit_get_enable_mask(u);
@@ -1554,16 +1572,6 @@ static int unit_realize_cgroup_now(Unit *u, ManagerState state) {
         return 0;
 }
 
-static void unit_add_to_cgroup_realize_queue(Unit *u) {
-        assert(u);
-
-        if (u->in_cgroup_realize_queue)
-                return;
-
-        LIST_PREPEND(cgroup_realize_queue, u->manager->cgroup_realize_queue, u);
-        u->in_cgroup_realize_queue = true;
-}
-
 unsigned manager_dispatch_cgroup_realize_queue(Manager *m) {
         ManagerState state;
         unsigned n = 0;
@@ -1577,6 +1585,12 @@ unsigned manager_dispatch_cgroup_realize_queue(Manager *m) {
         while ((i = m->cgroup_realize_queue)) {
                 assert(i->in_cgroup_realize_queue);
 
+                if (UNIT_IS_INACTIVE_OR_FAILED(unit_active_state(i))) {
+                        /* Maybe things changed, and the unit is not actually active anymore? */
+                        unit_remove_from_cgroup_realize_queue(i);
+                        continue;
+                }
+
                 r = unit_realize_cgroup_now(i, state);
                 if (r < 0)
                         log_warning_errno(r, "Failed to realize cgroups for queued unit %s, ignoring: %m", i->id);
@@ -2353,7 +2367,6 @@ int unit_get_ip_accounting(
         fd = IN_SET(metric, CGROUP_IP_INGRESS_BYTES, CGROUP_IP_INGRESS_PACKETS) ?
                 u->ip_accounting_ingress_map_fd :
                 u->ip_accounting_egress_map_fd;
-
         if (fd < 0)
                 return -ENODATA;