From f2f725e5cc950e84ebfd09bd64bc01c0ebdb6734 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 13 Feb 2018 10:50:13 +0100 Subject: [PATCH] pid1: rename unit_check_gc to unit_may_gc "check" is unclear: what is true, what is false? Let's rename to "can_gc" and revert the return value ("positive" values are easier to grok). v2: - rename from unit_can_gc to unit_may_gc --- src/core/automount.c | 13 ++++++++----- src/core/manager.c | 2 +- src/core/mount.c | 9 ++++++--- src/core/service.c | 10 +++++----- src/core/socket.c | 6 +++--- src/core/socket.h | 4 ++-- src/core/swap.c | 9 ++++++--- src/core/unit.c | 36 ++++++++++++++++++------------------ src/core/unit.h | 9 ++++----- 9 files changed, 53 insertions(+), 45 deletions(-) diff --git a/src/core/automount.c b/src/core/automount.c index c191336c0e..01a6ff806e 100644 --- a/src/core/automount.c +++ b/src/core/automount.c @@ -965,13 +965,16 @@ static const char *automount_sub_state_to_string(Unit *u) { return automount_state_to_string(AUTOMOUNT(u)->state); } -static bool automount_check_gc(Unit *u) { +static bool automount_may_gc(Unit *u) { + Unit *t; + assert(u); - if (!UNIT_TRIGGER(u)) - return false; + t = UNIT_TRIGGER(u); + if (!t) + return true; - return UNIT_VTABLE(UNIT_TRIGGER(u))->check_gc(UNIT_TRIGGER(u)); + return UNIT_VTABLE(t)->may_gc(t); } static int automount_dispatch_io(sd_event_source *s, int fd, uint32_t events, void *userdata) { @@ -1124,7 +1127,7 @@ const UnitVTable automount_vtable = { .active_state = automount_active_state, .sub_state_to_string = automount_sub_state_to_string, - .check_gc = automount_check_gc, + .may_gc = automount_may_gc, .trigger_notify = automount_trigger_notify, diff --git a/src/core/manager.c b/src/core/manager.c index 5021e00b87..c36a954e61 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -1057,7 +1057,7 @@ static void unit_gc_sweep(Unit *u, unsigned gc_marker) { if (u->in_cleanup_queue) goto bad; - if (unit_check_gc(u)) + if (!unit_may_gc(u)) goto good; u->gc_marker = gc_marker + GC_OFFSET_IN_PATH; diff --git a/src/core/mount.c b/src/core/mount.c index 914458f8e6..fa33a71d14 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -1234,12 +1234,15 @@ _pure_ static const char *mount_sub_state_to_string(Unit *u) { return mount_state_to_string(MOUNT(u)->state); } -_pure_ static bool mount_check_gc(Unit *u) { +_pure_ static bool mount_may_gc(Unit *u) { Mount *m = MOUNT(u); assert(m); - return m->from_proc_self_mountinfo; + if (m->from_proc_self_mountinfo) + return false; + + return true; } static void mount_sigchld_event(Unit *u, pid_t pid, int code, int status) { @@ -1995,7 +1998,7 @@ const UnitVTable mount_vtable = { .active_state = mount_active_state, .sub_state_to_string = mount_sub_state_to_string, - .check_gc = mount_check_gc, + .may_gc = mount_may_gc, .sigchld_event = mount_sigchld_event, diff --git a/src/core/service.c b/src/core/service.c index 37904874b5..b6106593ee 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -2833,20 +2833,20 @@ static const char *service_sub_state_to_string(Unit *u) { return service_state_to_string(SERVICE(u)->state); } -static bool service_check_gc(Unit *u) { +static bool service_may_gc(Unit *u) { Service *s = SERVICE(u); assert(s); /* Never clean up services that still have a process around, even if the service is formally dead. Note that - * unit_check_gc() already checked our cgroup for us, we just check our two additional PIDs, too, in case they + * unit_may_gc() already checked our cgroup for us, we just check our two additional PIDs, too, in case they * have moved outside of the cgroup. */ if (main_pid_good(s) > 0 || control_pid_good(s) > 0) - return true; + return false; - return false; + return true; } static int service_retry_pid_file(Service *s) { @@ -3934,7 +3934,7 @@ const UnitVTable service_vtable = { .will_restart = service_will_restart, - .check_gc = service_check_gc, + .may_gc = service_may_gc, .sigchld_event = service_sigchld_event, diff --git a/src/core/socket.c b/src/core/socket.c index 703f9f760f..f3e034d2af 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -2819,12 +2819,12 @@ SocketType socket_port_type_from_string(const char *s) { return _SOCKET_TYPE_INVALID; } -_pure_ static bool socket_check_gc(Unit *u) { +_pure_ static bool socket_may_gc(Unit *u) { Socket *s = SOCKET(u); assert(u); - return s->n_connections > 0; + return s->n_connections == 0; } static int socket_accept_do(Socket *s, int fd) { @@ -3324,7 +3324,7 @@ const UnitVTable socket_vtable = { .active_state = socket_active_state, .sub_state_to_string = socket_sub_state_to_string, - .check_gc = socket_check_gc, + .may_gc = socket_may_gc, .sigchld_event = socket_sigchld_event, diff --git a/src/core/socket.h b/src/core/socket.h index 9c528fb39c..84ec9cff08 100644 --- a/src/core/socket.h +++ b/src/core/socket.h @@ -104,8 +104,8 @@ struct Socket { DynamicCreds dynamic_creds; /* For Accept=no sockets refers to the one service we'll - activate. For Accept=yes sockets is either NULL, or filled - when the next service we spawn. */ + * activate. For Accept=yes sockets is either NULL, or filled + * to refer to the next service we spawn. */ UnitRef service; SocketState state, deserialized_state; diff --git a/src/core/swap.c b/src/core/swap.c index fffd8d4627..0af6538338 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -979,12 +979,15 @@ _pure_ static const char *swap_sub_state_to_string(Unit *u) { return swap_state_to_string(SWAP(u)->state); } -_pure_ static bool swap_check_gc(Unit *u) { +_pure_ static bool swap_may_gc(Unit *u) { Swap *s = SWAP(u); assert(s); - return s->from_proc_swaps; + if (s->from_proc_swaps) + return false; + + return true; } static void swap_sigchld_event(Unit *u, pid_t pid, int code, int status) { @@ -1506,7 +1509,7 @@ const UnitVTable swap_vtable = { .active_state = swap_active_state, .sub_state_to_string = swap_sub_state_to_string, - .check_gc = swap_check_gc, + .may_gc = swap_may_gc, .sigchld_event = swap_sigchld_event, diff --git a/src/core/unit.c b/src/core/unit.c index 9a57bcfb4b..fd2cc6a667 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -336,20 +336,21 @@ int unit_set_description(Unit *u, const char *description) { return 0; } -bool unit_check_gc(Unit *u) { +bool unit_may_gc(Unit *u) { UnitActiveState state; int r; assert(u); - /* Checks whether the unit is ready to be unloaded for garbage collection. Returns true, when the unit shall - * stay around, false if there's no reason to keep it loaded. */ + /* Checks whether the unit is ready to be unloaded for garbage collection. + * Returns true when the unit may be collected, and false if there's some + * reason to keep it loaded. */ if (u->job) - return true; + return false; if (u->nop_job) - return true; + return false; state = unit_active_state(u); @@ -359,26 +360,26 @@ bool unit_check_gc(Unit *u) { UNIT_VTABLE(u)->release_resources(u); if (u->perpetual) - return true; + return false; if (u->refs) - return true; + return false; if (sd_bus_track_count(u->bus_track) > 0) - return true; + return false; /* But we keep the unit object around for longer when it is referenced or configured to not be gc'ed */ switch (u->collect_mode) { case COLLECT_INACTIVE: if (state != UNIT_INACTIVE) - return true; + return false; break; case COLLECT_INACTIVE_OR_FAILED: if (!IN_SET(state, UNIT_INACTIVE, UNIT_FAILED)) - return true; + return false; break; @@ -394,14 +395,13 @@ bool unit_check_gc(Unit *u) { if (r < 0) log_unit_debug_errno(u, r, "Failed to determine whether cgroup %s is empty: %m", u->cgroup_path); if (r <= 0) - return true; + return false; } - if (UNIT_VTABLE(u)->check_gc) - if (UNIT_VTABLE(u)->check_gc(u)) - return true; + if (UNIT_VTABLE(u)->may_gc && !UNIT_VTABLE(u)->may_gc(u)) + return false; - return false; + return true; } void unit_add_to_load_queue(Unit *u) { @@ -431,7 +431,7 @@ void unit_add_to_gc_queue(Unit *u) { if (u->in_gc_queue || u->in_cleanup_queue) return; - if (unit_check_gc(u)) + if (!unit_may_gc(u)) return; LIST_PREPEND(gc_queue, u->manager->gc_unit_queue, u); @@ -1119,7 +1119,7 @@ void unit_dump(Unit *u, FILE *f, const char *prefix) { "%s\tActive Enter Timestamp: %s\n" "%s\tActive Exit Timestamp: %s\n" "%s\tInactive Enter Timestamp: %s\n" - "%s\tGC Check Good: %s\n" + "%s\tMay GC: %s\n" "%s\tNeed Daemon Reload: %s\n" "%s\tTransient: %s\n" "%s\tPerpetual: %s\n" @@ -1137,7 +1137,7 @@ void unit_dump(Unit *u, FILE *f, const char *prefix) { prefix, strna(format_timestamp(timestamp2, sizeof(timestamp2), u->active_enter_timestamp.realtime)), prefix, strna(format_timestamp(timestamp3, sizeof(timestamp3), u->active_exit_timestamp.realtime)), prefix, strna(format_timestamp(timestamp4, sizeof(timestamp4), u->inactive_enter_timestamp.realtime)), - prefix, yes_no(unit_check_gc(u)), + prefix, yes_no(unit_may_gc(u)), prefix, yes_no(unit_need_daemon_reload(u)), prefix, yes_no(u->transient), prefix, yes_no(u->perpetual), diff --git a/src/core/unit.h b/src/core/unit.h index 3210583050..8e3d9bf323 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -490,10 +490,9 @@ struct UnitVTable { /* Additionally to UnitActiveState determine whether unit is to be restarted. */ bool (*will_restart)(Unit *u); - /* Return true when there is reason to keep this entry around - * even nothing references it and it isn't active in any - * way */ - bool (*check_gc)(Unit *u); + /* Return false when there is a reason to prevent this unit from being gc'ed + * even though nothing references it and it isn't active in any way. */ + bool (*may_gc)(Unit *u); /* When the unit is not running and no job for it queued we shall release its runtime resources */ void (*release_resources)(Unit *u); @@ -623,7 +622,7 @@ int unit_add_exec_dependencies(Unit *u, ExecContext *c); int unit_choose_id(Unit *u, const char *name); int unit_set_description(Unit *u, const char *description); -bool unit_check_gc(Unit *u); +bool unit_may_gc(Unit *u); void unit_add_to_load_queue(Unit *u); void unit_add_to_dbus_queue(Unit *u);