daandemeyer / rpms / systemd

Forked from rpms/systemd 2 years ago
Clone
52b84b
From 5458256264c97eee521caf07c705f549a0f0bd55 Mon Sep 17 00:00:00 2001
52b84b
From: Lennart Poettering <lennart@poettering.net>
52b84b
Date: Thu, 9 Aug 2018 16:26:27 +0200
52b84b
Subject: [PATCH] core: rework StopWhenUnneeded= logic
52b84b
52b84b
Previously, we'd act immediately on StopWhenUnneeded= when a unit state
52b84b
changes. With this rework we'll maintain a queue instead: whenever
52b84b
there's the chance that StopWhenUneeded= might have an effect we enqueue
52b84b
the unit, and process it later when we have nothing better to do.
52b84b
52b84b
This should make the implementation a bit more reliable, as the unit notify event
52b84b
cannot immediately enqueue tons of side-effect jobs that might
52b84b
contradict each other, but we do so only in a strictly ordered fashion,
52b84b
from the main event loop.
52b84b
52b84b
This slightly changes the check when to consider a unit "unneeded".
52b84b
Previously, we'd assume that a unit in "deactivating" state could also
52b84b
be cleaned up. With this new logic we'll only consider units unneeded
52b84b
that are fully up and have no job queued. This means that whenever
52b84b
there's something pending for a unit we won't clean it up.
52b84b
52b84b
(cherry picked from commit a3c1168ac293f16d9343d248795bb4c246aaff4a)
52b84b
52b84b
Resolves: #1798046
52b84b
---
52b84b
 src/core/manager.c |  43 ++++++++++++++++
52b84b
 src/core/manager.h |   3 ++
52b84b
 src/core/unit.c    | 122 +++++++++++++++++++++++++--------------------
52b84b
 src/core/unit.h    |   7 +++
52b84b
 4 files changed, 120 insertions(+), 55 deletions(-)
52b84b
52b84b
diff --git a/src/core/manager.c b/src/core/manager.c
52b84b
index 0eae7d46fb..4c04896aaa 100644
52b84b
--- a/src/core/manager.c
52b84b
+++ b/src/core/manager.c
52b84b
@@ -1211,6 +1211,45 @@ static unsigned manager_dispatch_gc_job_queue(Manager *m) {
52b84b
         return n;
52b84b
 }
52b84b
 
52b84b
+static unsigned manager_dispatch_stop_when_unneeded_queue(Manager *m) {
52b84b
+        unsigned n = 0;
52b84b
+        Unit *u;
52b84b
+        int r;
52b84b
+
52b84b
+        assert(m);
52b84b
+
52b84b
+        while ((u = m->stop_when_unneeded_queue)) {
52b84b
+                _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
52b84b
+                assert(m->stop_when_unneeded_queue);
52b84b
+
52b84b
+                assert(u->in_stop_when_unneeded_queue);
52b84b
+                LIST_REMOVE(stop_when_unneeded_queue, m->stop_when_unneeded_queue, u);
52b84b
+                u->in_stop_when_unneeded_queue = false;
52b84b
+
52b84b
+                n++;
52b84b
+
52b84b
+                if (!unit_is_unneeded(u))
52b84b
+                        continue;
52b84b
+
52b84b
+                log_unit_debug(u, "Unit is not needed anymore.");
52b84b
+
52b84b
+                /* If stopping a unit fails continuously we might enter a stop loop here, hence stop acting on the
52b84b
+                 * service being unnecessary after a while. */
52b84b
+
52b84b
+                if (!ratelimit_below(&u->auto_stop_ratelimit)) {
52b84b
+                        log_unit_warning(u, "Unit not needed anymore, but not stopping since we tried this too often recently.");
52b84b
+                        continue;
52b84b
+                }
52b84b
+
52b84b
+                /* Ok, nobody needs us anymore. Sniff. Then let's commit suicide */
52b84b
+                r = manager_add_job(u->manager, JOB_STOP, u, JOB_FAIL, &error, NULL);
52b84b
+                if (r < 0)
52b84b
+                        log_unit_warning_errno(u, r, "Failed to enqueue stop job, ignoring: %s", bus_error_message(&error, r));
52b84b
+        }
52b84b
+
52b84b
+        return n;
52b84b
+}
52b84b
+
52b84b
 static void manager_clear_jobs_and_units(Manager *m) {
52b84b
         Unit *u;
52b84b
 
52b84b
@@ -1228,6 +1267,7 @@ static void manager_clear_jobs_and_units(Manager *m) {
52b84b
         assert(!m->cleanup_queue);
52b84b
         assert(!m->gc_unit_queue);
52b84b
         assert(!m->gc_job_queue);
52b84b
+        assert(!m->stop_when_unneeded_queue);
52b84b
 
52b84b
         assert(hashmap_isempty(m->jobs));
52b84b
         assert(hashmap_isempty(m->units));
52b84b
@@ -2824,6 +2864,9 @@ int manager_loop(Manager *m) {
52b84b
                 if (manager_dispatch_cgroup_realize_queue(m) > 0)
52b84b
                         continue;
52b84b
 
52b84b
+                if (manager_dispatch_stop_when_unneeded_queue(m) > 0)
52b84b
+                        continue;
52b84b
+
52b84b
                 if (manager_dispatch_dbus_queue(m) > 0)
52b84b
                         continue;
52b84b
 
52b84b
diff --git a/src/core/manager.h b/src/core/manager.h
52b84b
index fa47952d24..40568d3c8b 100644
52b84b
--- a/src/core/manager.h
52b84b
+++ b/src/core/manager.h
52b84b
@@ -130,6 +130,9 @@ struct Manager {
52b84b
         /* Target units whose default target dependencies haven't been set yet */
52b84b
         LIST_HEAD(Unit, target_deps_queue);
52b84b
 
52b84b
+        /* Units that might be subject to StopWhenUnneeded= clean-up */
52b84b
+        LIST_HEAD(Unit, stop_when_unneeded_queue);
52b84b
+
52b84b
         sd_event *event;
52b84b
 
52b84b
         /* This maps PIDs we care about to units that are interested in. We allow multiple units to he interested in
52b84b
diff --git a/src/core/unit.c b/src/core/unit.c
52b84b
index e1f5e6f7bd..40f138d25c 100644
52b84b
--- a/src/core/unit.c
52b84b
+++ b/src/core/unit.c
52b84b
@@ -438,6 +438,22 @@ void unit_add_to_dbus_queue(Unit *u) {
52b84b
         u->in_dbus_queue = true;
52b84b
 }
52b84b
 
52b84b
+void unit_add_to_stop_when_unneeded_queue(Unit *u) {
52b84b
+        assert(u);
52b84b
+
52b84b
+        if (u->in_stop_when_unneeded_queue)
52b84b
+                return;
52b84b
+
52b84b
+        if (!u->stop_when_unneeded)
52b84b
+                return;
52b84b
+
52b84b
+        if (!UNIT_IS_ACTIVE_OR_RELOADING(unit_active_state(u)))
52b84b
+                return;
52b84b
+
52b84b
+        LIST_PREPEND(stop_when_unneeded_queue, u->manager->stop_when_unneeded_queue, u);
52b84b
+        u->in_stop_when_unneeded_queue = true;
52b84b
+}
52b84b
+
52b84b
 static void bidi_set_free(Unit *u, Hashmap *h) {
52b84b
         Unit *other;
52b84b
         Iterator i;
52b84b
@@ -634,6 +650,9 @@ void unit_free(Unit *u) {
52b84b
         if (u->in_target_deps_queue)
52b84b
                 LIST_REMOVE(target_deps_queue, u->manager->target_deps_queue, u);
52b84b
 
52b84b
+        if (u->in_stop_when_unneeded_queue)
52b84b
+                LIST_REMOVE(stop_when_unneeded_queue, u->manager->stop_when_unneeded_queue, u);
52b84b
+
52b84b
         safe_close(u->ip_accounting_ingress_map_fd);
52b84b
         safe_close(u->ip_accounting_egress_map_fd);
52b84b
 
52b84b
@@ -1950,55 +1969,71 @@ bool unit_can_reload(Unit *u) {
52b84b
         return UNIT_VTABLE(u)->reload;
52b84b
 }
52b84b
 
52b84b
-static void unit_check_unneeded(Unit *u) {
52b84b
-
52b84b
-        _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
52b84b
-
52b84b
-        static const UnitDependency needed_dependencies[] = {
52b84b
+bool unit_is_unneeded(Unit *u) {
52b84b
+        static const UnitDependency deps[] = {
52b84b
                 UNIT_REQUIRED_BY,
52b84b
                 UNIT_REQUISITE_OF,
52b84b
                 UNIT_WANTED_BY,
52b84b
                 UNIT_BOUND_BY,
52b84b
         };
52b84b
-
52b84b
-        unsigned j;
52b84b
-        int r;
52b84b
+        size_t j;
52b84b
 
52b84b
         assert(u);
52b84b
 
52b84b
-        /* If this service shall be shut down when unneeded then do
52b84b
-         * so. */
52b84b
-
52b84b
         if (!u->stop_when_unneeded)
52b84b
-                return;
52b84b
+                return false;
52b84b
 
52b84b
-        if (!UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(u)))
52b84b
-                return;
52b84b
+        /* Don't clean up while the unit is transitioning or is even inactive. */
52b84b
+        if (!UNIT_IS_ACTIVE_OR_RELOADING(unit_active_state(u)))
52b84b
+                return false;
52b84b
+        if (u->job)
52b84b
+                return false;
52b84b
 
52b84b
-        for (j = 0; j < ELEMENTSOF(needed_dependencies); j++) {
52b84b
+        for (j = 0; j < ELEMENTSOF(deps); j++) {
52b84b
                 Unit *other;
52b84b
                 Iterator i;
52b84b
                 void *v;
52b84b
 
52b84b
-                HASHMAP_FOREACH_KEY(v, other, u->dependencies[needed_dependencies[j]], i)
52b84b
-                        if (unit_active_or_pending(other) || unit_will_restart(other))
52b84b
-                                return;
52b84b
-        }
52b84b
+                /* If a dependending unit has a job queued, or is active (or in transitioning), or is marked for
52b84b
+                 * restart, then don't clean this one up. */
52b84b
 
52b84b
-        /* If stopping a unit fails continuously we might enter a stop
52b84b
-         * loop here, hence stop acting on the service being
52b84b
-         * unnecessary after a while. */
52b84b
-        if (!ratelimit_below(&u->auto_stop_ratelimit)) {
52b84b
-                log_unit_warning(u, "Unit not needed anymore, but not stopping since we tried this too often recently.");
52b84b
-                return;
52b84b
+                HASHMAP_FOREACH_KEY(v, other, u->dependencies[deps[j]], i) {
52b84b
+                        if (u->job)
52b84b
+                                return false;
52b84b
+
52b84b
+                        if (!UNIT_IS_INACTIVE_OR_FAILED(unit_active_state(other)))
52b84b
+                                return false;
52b84b
+
52b84b
+                        if (unit_will_restart(other))
52b84b
+                                return false;
52b84b
+                }
52b84b
         }
52b84b
 
52b84b
-        log_unit_info(u, "Unit not needed anymore. Stopping.");
52b84b
+        return true;
52b84b
+}
52b84b
 
52b84b
-        /* Ok, nobody needs us anymore. Sniff. Then let's commit suicide */
52b84b
-        r = manager_add_job(u->manager, JOB_STOP, u, JOB_FAIL, &error, NULL);
52b84b
-        if (r < 0)
52b84b
-                log_unit_warning_errno(u, r, "Failed to enqueue stop job, ignoring: %s", bus_error_message(&error, r));
52b84b
+static void check_unneeded_dependencies(Unit *u) {
52b84b
+
52b84b
+        static const UnitDependency deps[] = {
52b84b
+                UNIT_REQUIRES,
52b84b
+                UNIT_REQUISITE,
52b84b
+                UNIT_WANTS,
52b84b
+                UNIT_BINDS_TO,
52b84b
+        };
52b84b
+        size_t j;
52b84b
+
52b84b
+        assert(u);
52b84b
+
52b84b
+        /* Add all units this unit depends on to the queue that processes StopWhenUnneeded= behaviour. */
52b84b
+
52b84b
+        for (j = 0; j < ELEMENTSOF(deps); j++) {
52b84b
+                Unit *other;
52b84b
+                Iterator i;
52b84b
+                void *v;
52b84b
+
52b84b
+                HASHMAP_FOREACH_KEY(v, other, u->dependencies[deps[j]], i)
52b84b
+                        unit_add_to_stop_when_unneeded_queue(other);
52b84b
+        }
52b84b
 }
52b84b
 
52b84b
 static void unit_check_binds_to(Unit *u) {
52b84b
@@ -2098,29 +2133,6 @@ static void retroactively_stop_dependencies(Unit *u) {
52b84b
                         manager_add_job(u->manager, JOB_STOP, other, JOB_REPLACE, NULL, NULL);
52b84b
 }
52b84b
 
52b84b
-static void check_unneeded_dependencies(Unit *u) {
52b84b
-        Unit *other;
52b84b
-        Iterator i;
52b84b
-        void *v;
52b84b
-
52b84b
-        assert(u);
52b84b
-        assert(UNIT_IS_INACTIVE_OR_DEACTIVATING(unit_active_state(u)));
52b84b
-
52b84b
-        /* Garbage collect services that might not be needed anymore, if enabled */
52b84b
-        HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_REQUIRES], i)
52b84b
-                if (!UNIT_IS_INACTIVE_OR_DEACTIVATING(unit_active_state(other)))
52b84b
-                        unit_check_unneeded(other);
52b84b
-        HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_WANTS], i)
52b84b
-                if (!UNIT_IS_INACTIVE_OR_DEACTIVATING(unit_active_state(other)))
52b84b
-                        unit_check_unneeded(other);
52b84b
-        HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_REQUISITE], i)
52b84b
-                if (!UNIT_IS_INACTIVE_OR_DEACTIVATING(unit_active_state(other)))
52b84b
-                        unit_check_unneeded(other);
52b84b
-        HASHMAP_FOREACH_KEY(v, other, u->dependencies[UNIT_BINDS_TO], i)
52b84b
-                if (!UNIT_IS_INACTIVE_OR_DEACTIVATING(unit_active_state(other)))
52b84b
-                        unit_check_unneeded(other);
52b84b
-}
52b84b
-
52b84b
 void unit_start_on_failure(Unit *u) {
52b84b
         Unit *other;
52b84b
         Iterator i;
52b84b
@@ -2423,7 +2435,7 @@ void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, UnitNotifyFlag
52b84b
                 }
52b84b
 
52b84b
                 /* stop unneeded units regardless if going down was expected or not */
52b84b
-                if (UNIT_IS_INACTIVE_OR_DEACTIVATING(ns))
52b84b
+                if (UNIT_IS_INACTIVE_OR_FAILED(ns))
52b84b
                         check_unneeded_dependencies(u);
52b84b
 
52b84b
                 if (ns != os && ns == UNIT_FAILED) {
52b84b
@@ -2483,7 +2495,7 @@ void unit_notify(Unit *u, UnitActiveState os, UnitActiveState ns, UnitNotifyFlag
52b84b
 
52b84b
         if (!MANAGER_IS_RELOADING(u->manager)) {
52b84b
                 /* Maybe we finished startup and are now ready for being stopped because unneeded? */
52b84b
-                unit_check_unneeded(u);
52b84b
+                unit_add_to_stop_when_unneeded_queue(u);
52b84b
 
52b84b
                 /* Maybe we finished startup, but something we needed has vanished? Let's die then. (This happens when
52b84b
                  * something BindsTo= to a Type=oneshot unit, as these units go directly from starting to inactive,
52b84b
diff --git a/src/core/unit.h b/src/core/unit.h
52b84b
index 99755823eb..595ee88d43 100644
52b84b
--- a/src/core/unit.h
52b84b
+++ b/src/core/unit.h
52b84b
@@ -212,6 +212,9 @@ typedef struct Unit {
52b84b
         /* Target dependencies queue */
52b84b
         LIST_FIELDS(Unit, target_deps_queue);
52b84b
 
52b84b
+        /* Queue of units with StopWhenUnneeded set that shell be checked for clean-up. */
52b84b
+        LIST_FIELDS(Unit, stop_when_unneeded_queue);
52b84b
+
52b84b
         /* PIDs we keep an eye on. Note that a unit might have many
52b84b
          * more, but these are the ones we care enough about to
52b84b
          * process SIGCHLD for */
52b84b
@@ -322,6 +325,7 @@ typedef struct Unit {
52b84b
         bool in_cgroup_realize_queue:1;
52b84b
         bool in_cgroup_empty_queue:1;
52b84b
         bool in_target_deps_queue:1;
52b84b
+        bool in_stop_when_unneeded_queue:1;
52b84b
 
52b84b
         bool sent_dbus_new_signal:1;
52b84b
 
52b84b
@@ -615,6 +619,7 @@ void unit_add_to_dbus_queue(Unit *u);
52b84b
 void unit_add_to_cleanup_queue(Unit *u);
52b84b
 void unit_add_to_gc_queue(Unit *u);
52b84b
 void unit_add_to_target_deps_queue(Unit *u);
52b84b
+void unit_add_to_stop_when_unneeded_queue(Unit *u);
52b84b
 
52b84b
 int unit_merge(Unit *u, Unit *other);
52b84b
 int unit_merge_by_name(Unit *u, const char *other);
52b84b
@@ -751,6 +756,8 @@ bool unit_type_supported(UnitType t);
52b84b
 
52b84b
 bool unit_is_pristine(Unit *u);
52b84b
 
52b84b
+bool unit_is_unneeded(Unit *u);
52b84b
+
52b84b
 pid_t unit_control_pid(Unit *u);
52b84b
 pid_t unit_main_pid(Unit *u);
52b84b