From 9b27811d49709b13f6255061089c9632572ee1ce Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 27 May 2024 15:37:51 +0200 Subject: [PATCH 1/2] machined: GC machines during runtime too One major omission in machine's logic so far was that the GC was only run at startup and on the check-idle timeout, which is really slow. Let's make this more like the GC logic in homed or logind: make sure we run it in a close-by event loop cycle. --- src/machine/machine.c | 2 ++ src/machine/machined-core.c | 52 +++++++++++++++++++++++++++++++++++++ src/machine/machined.c | 27 ++----------------- src/machine/machined.h | 5 ++++ 4 files changed, 61 insertions(+), 25 deletions(-) diff --git a/src/machine/machine.c b/src/machine/machine.c index 1ef2be27abf9e..25ecc4d197bdc 100644 --- a/src/machine/machine.c +++ b/src/machine/machine.c @@ -592,6 +592,8 @@ void machine_add_to_gc_queue(Machine *m) { LIST_PREPEND(gc_queue, m->manager->machine_gc_queue, m); m->in_gc_queue = true; + + manager_enqueue_gc(m->manager); } MachineState machine_get_state(Machine *s) { diff --git a/src/machine/machined-core.c b/src/machine/machined-core.c index ffca2094942f2..a7e1853b53533 100644 --- a/src/machine/machined-core.c +++ b/src/machine/machined-core.c @@ -104,3 +104,55 @@ int manager_find_machine_for_gid(Manager *m, gid_t gid, Machine **ret_machine, g return false; } + +void manager_gc(Manager *m, bool drop_not_started) { + Machine *machine; + + assert(m); + + while ((machine = LIST_POP(gc_queue, m->machine_gc_queue))) { + machine->in_gc_queue = false; + + /* First, if we are not closing yet, initiate stopping */ + if (machine_may_gc(machine, drop_not_started) && + machine_get_state(machine) != MACHINE_CLOSING) + machine_stop(machine); + + /* Now, the stop probably made this referenced + * again, but if it didn't, then it's time to let it + * go entirely. */ + if (machine_may_gc(machine, drop_not_started)) { + machine_finalize(machine); + machine_free(machine); + } + } +} + +static int on_deferred_gc(sd_event_source *s, void *userdata) { + manager_gc(userdata, /* drop_not_started= */ true); + return 0; +} + +void manager_enqueue_gc(Manager *m) { + int r; + + assert(m); + + if (m->deferred_gc_event_source) { + r = sd_event_source_set_enabled(m->deferred_gc_event_source, SD_EVENT_ONESHOT); + if (r < 0) + log_warning_errno(r, "Failed to enable GC event source, ignoring: %m"); + + return; + } + + r = sd_event_add_defer(m->event, &m->deferred_gc_event_source, on_deferred_gc, m); + if (r < 0) + return (void) log_warning_errno(r, "Failed to allocate GC event source, ignoring: %m"); + + r = sd_event_source_set_priority(m->deferred_gc_event_source, SD_EVENT_PRIORITY_IDLE); + if (r < 0) + log_warning_errno(r, "Failed to tweak priority of event source, ignoring: %m"); + + (void) sd_event_source_set_description(m->deferred_gc_event_source, "deferred-gc"); +} diff --git a/src/machine/machined.c b/src/machine/machined.c index 60badbf5d075b..4f2ca626e2077 100644 --- a/src/machine/machined.c +++ b/src/machine/machined.c @@ -97,6 +97,8 @@ static Manager* manager_unref(Manager *m) { sd_event_source_unref(m->nscd_cache_flush_event); #endif + sd_event_source_disable_unref(m->deferred_gc_event_source); + hashmap_free(m->polkit_registry); manager_varlink_done(m); @@ -264,29 +266,6 @@ static int manager_connect_bus(Manager *m) { return 0; } -static void manager_gc(Manager *m, bool drop_not_started) { - Machine *machine; - - assert(m); - - while ((machine = LIST_POP(gc_queue, m->machine_gc_queue))) { - machine->in_gc_queue = false; - - /* First, if we are not closing yet, initiate stopping */ - if (machine_may_gc(machine, drop_not_started) && - machine_get_state(machine) != MACHINE_CLOSING) - machine_stop(machine); - - /* Now, the stop probably made this referenced - * again, but if it didn't, then it's time to let it - * go entirely. */ - if (machine_may_gc(machine, drop_not_started)) { - machine_finalize(machine); - machine_free(machine); - } - } -} - static int manager_startup(Manager *m) { Machine *machine; int r; @@ -331,8 +310,6 @@ static bool check_idle(void *userdata) { if (!hashmap_isempty(m->polkit_registry)) return false; - manager_gc(m, true); - return hashmap_isempty(m->machines); } diff --git a/src/machine/machined.h b/src/machine/machined.h index 67abed0fd67e2..4c22382f2ad27 100644 --- a/src/machine/machined.h +++ b/src/machine/machined.h @@ -24,6 +24,8 @@ struct Manager { Hashmap *machine_units; Hashmap *machine_leaders; + sd_event_source *deferred_gc_event_source; + Hashmap *polkit_registry; Hashmap *image_cache; @@ -68,3 +70,6 @@ static inline void manager_enqueue_nscd_cache_flush(Manager *m) {} int manager_find_machine_for_uid(Manager *m, uid_t host_uid, Machine **ret_machine, uid_t *ret_internal_uid); int manager_find_machine_for_gid(Manager *m, gid_t host_gid, Machine **ret_machine, gid_t *ret_internal_gid); + +void manager_gc(Manager *m, bool drop_not_started); +void manager_enqueue_gc(Manager *m); From 1762c2c045d3a78d3cad54c6b1e5ee9624b32b00 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 27 May 2024 15:36:44 +0200 Subject: [PATCH 2/2] machined: watch leader PID's lifetime via pidfd If we have a pidfd, we might as well track the machine's leader PID's lifetime, and enqueue the machine for a GC run. (This is similar to what we are already doing for logind's session leaders) --- src/machine/machine.c | 37 +++++++++++++++++++++++++++++++++++++ src/machine/machine.h | 1 + 2 files changed, 38 insertions(+) diff --git a/src/machine/machine.c b/src/machine/machine.c index 25ecc4d197bdc..a12ea7c16bbed 100644 --- a/src/machine/machine.c +++ b/src/machine/machine.c @@ -118,6 +118,7 @@ Machine* machine_free(Machine *m) { m->manager->host_machine = NULL; } + m->leader_pidfd_event_source = sd_event_source_disable_unref(m->leader_pidfd_event_source); if (pidref_is_set(&m->leader)) { if (m->manager) (void) hashmap_remove_value(m->manager->machine_leaders, PID_TO_PTR(m->leader.pid), m); @@ -475,6 +476,38 @@ static int machine_ensure_scope(Machine *m, sd_bus_message *properties, sd_bus_e return 0; } +static int machine_dispatch_leader_pidfd(sd_event_source *s, int fd, unsigned revents, void *userdata) { + Machine *m = ASSERT_PTR(userdata); + + m->leader_pidfd_event_source = sd_event_source_disable_unref(m->leader_pidfd_event_source); + machine_add_to_gc_queue(m); + + return 0; +} + +static int machine_watch_pidfd(Machine *m) { + int r; + + assert(m); + assert(m->manager); + assert(pidref_is_set(&m->leader)); + assert(!m->leader_pidfd_event_source); + + if (m->leader.fd < 0) + return 0; + + /* If we have a pidfd for the leader, let's also track it for POLLIN, and GC the machine + * automatically if it dies */ + + r = sd_event_add_io(m->manager->event, &m->leader_pidfd_event_source, m->leader.fd, EPOLLIN, machine_dispatch_leader_pidfd, m); + if (r < 0) + return r; + + (void) sd_event_source_set_description(m->leader_pidfd_event_source, "machine-pidfd"); + + return 0; +} + int machine_start(Machine *m, sd_bus_message *properties, sd_bus_error *error) { int r; @@ -490,6 +523,10 @@ int machine_start(Machine *m, sd_bus_message *properties, sd_bus_error *error) { if (r < 0) return r; + r = machine_watch_pidfd(m); + if (r < 0) + return r; + /* Create cgroup */ r = machine_ensure_scope(m, properties, error); if (r < 0) diff --git a/src/machine/machine.h b/src/machine/machine.h index 12551025c7db9..8f1feda14befb 100644 --- a/src/machine/machine.h +++ b/src/machine/machine.h @@ -49,6 +49,7 @@ struct Machine { char *scope_job; PidRef leader; + sd_event_source *leader_pidfd_event_source; dual_timestamp timestamp;