From 3850d0505fb4350dccdd63cf6129db40fe9b5bd0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 28 Aug 2015 13:38:49 +0200 Subject: build-sys: add missing Makefile symlink --- src/shared/Makefile | 1 + 1 file changed, 1 insertion(+) create mode 120000 src/shared/Makefile diff --git a/src/shared/Makefile b/src/shared/Makefile new file mode 120000 index 0000000000..d0b0e8e008 --- /dev/null +++ b/src/shared/Makefile @@ -0,0 +1 @@ +../Makefile \ No newline at end of file -- cgit v1.2.3-54-g00ecf From 3f5e811594bcdb25d3aa859cb0be06f6df3a03b9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 28 Aug 2015 16:05:32 +0200 Subject: core: don't generate stub unit file for transient units We store the properties for transient units in drop-ins anyway, and units don't have to have fragment files, hence don't bother with them, and don't create them. --- src/core/load-fragment.c | 5 +++++ src/core/unit.c | 46 ++++++++++++---------------------------------- 2 files changed, 17 insertions(+), 34 deletions(-) diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index b3bf8bdb40..721da3470e 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -3603,6 +3603,11 @@ int unit_load_fragment(Unit *u) { assert(u->load_state == UNIT_STUB); assert(u->id); + if (u->transient) { + u->load_state = UNIT_LOADED; + return 0; + } + /* First, try to find the unit under its id. We always look * for unit files in the default directories, to make it easy * to override things by placing things in /etc/systemd/system */ diff --git a/src/core/unit.c b/src/core/unit.c index 43a5ca1064..a6b56e2998 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -405,17 +405,17 @@ static void unit_remove_transient(Unit *u) { return; if (u->fragment_path) - unlink(u->fragment_path); + (void) unlink(u->fragment_path); STRV_FOREACH(i, u->dropin_paths) { _cleanup_free_ char *p = NULL; int r; - unlink(*i); + (void) unlink(*i); r = path_get_parent(*i, &p); if (r >= 0) - rmdir(p); + (void) rmdir(p); } } @@ -3317,6 +3317,8 @@ ExecRuntime *unit_get_exec_runtime(Unit *u) { } static int unit_drop_in_dir(Unit *u, UnitSetPropertiesMode mode, bool transient, char **dir) { + assert(u); + if (u->manager->running_as == MANAGER_USER) { int r; @@ -3324,9 +3326,9 @@ static int unit_drop_in_dir(Unit *u, UnitSetPropertiesMode mode, bool transient, r = user_config_home(dir); else r = user_runtime_dir(dir); - if (r == 0) return -ENOENT; + return r; } @@ -3340,8 +3342,7 @@ static int unit_drop_in_dir(Unit *u, UnitSetPropertiesMode mode, bool transient, return 0; } -static int unit_drop_in_file(Unit *u, - UnitSetPropertiesMode mode, const char *name, char **p, char **q) { +static int unit_drop_in_file(Unit *u, UnitSetPropertiesMode mode, const char *name, char **p, char **q) { _cleanup_free_ char *dir = NULL; int r; @@ -3475,40 +3476,17 @@ int unit_remove_drop_in(Unit *u, UnitSetPropertiesMode mode, const char *name) { } int unit_make_transient(Unit *u) { - int r; - assert(u); + if (!UNIT_VTABLE(u)->can_transient) + return -EOPNOTSUPP; + u->load_state = UNIT_STUB; u->load_error = 0; u->transient = true; + u->fragment_path = mfree(u->fragment_path); - free(u->fragment_path); - u->fragment_path = NULL; - - if (u->manager->running_as == MANAGER_USER) { - _cleanup_free_ char *c = NULL; - - r = user_runtime_dir(&c); - if (r < 0) - return r; - if (r == 0) - return -ENOENT; - - u->fragment_path = strjoin(c, "/", u->id, NULL); - if (!u->fragment_path) - return -ENOMEM; - - mkdir_p(c, 0755); - } else { - u->fragment_path = strappend("/run/systemd/system/", u->id); - if (!u->fragment_path) - return -ENOMEM; - - mkdir_p("/run/systemd/system", 0755); - } - - return write_string_file_atomic_label(u->fragment_path, "# Transient stub"); + return 0; } int unit_kill_context( -- cgit v1.2.3-54-g00ecf From 6513d561ce4c894ea4e29612a2ed62c8310a164f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 28 Aug 2015 17:11:37 +0200 Subject: core: use DUAL_TIMESTAMP_NULL where we can --- src/core/main.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/core/main.c b/src/core/main.c index 87b97aa883..e232be88c0 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -1391,8 +1391,7 @@ int main(int argc, char *argv[]) { /* clear the kernel timestamp, * because we are not PID 1 */ - kernel_timestamp.monotonic = 0ULL; - kernel_timestamp.realtime = 0ULL; + kernel_timestamp = DUAL_TIMESTAMP_NULL; } /* Initialize default unit */ -- cgit v1.2.3-54-g00ecf From 35b7ff80e29524cb01f881ca6d52c669970c88f1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 28 Aug 2015 17:14:59 +0200 Subject: unit: add new macros to test for unit contexts --- src/core/cgroup.c | 5 +---- src/core/dbus-unit.c | 2 +- src/core/dbus.c | 2 +- src/core/unit.c | 2 +- src/core/unit.h | 4 ++++ 5 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 6474e08bd2..c26807ba2b 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -806,12 +806,9 @@ static void unit_queue_siblings(Unit *u) { } int unit_realize_cgroup(Unit *u) { - CGroupContext *c; - assert(u); - c = unit_get_cgroup_context(u); - if (!c) + if (!UNIT_HAS_CGROUP_CONTEXT(u)) return 0; /* So, here's the deal: when realizing the cgroups for this diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index 42bb653cc1..91c31987fe 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -965,7 +965,7 @@ static int bus_unit_set_transient_property( return 1; - } else if (streq(name, "Slice") && unit_get_cgroup_context(u)) { + } else if (streq(name, "Slice") && UNIT_HAS_CGROUP_CONTEXT(u)) { const char *s; r = sd_bus_message_read(message, "s", &s); diff --git a/src/core/dbus.c b/src/core/dbus.c index d091aa5419..7ad16aa42b 100644 --- a/src/core/dbus.c +++ b/src/core/dbus.c @@ -381,7 +381,7 @@ static int bus_unit_cgroup_find(sd_bus *bus, const char *path, const char *inter if (!streq_ptr(interface, unit_dbus_interface_from_type(u->type))) return 0; - if (!unit_get_cgroup_context(u)) + if (!UNIT_HAS_CGROUP_CONTEXT(u)) return 0; *found = u; diff --git a/src/core/unit.c b/src/core/unit.c index a6b56e2998..2ad49fd50b 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -1122,7 +1122,7 @@ static int unit_add_target_dependencies(Unit *u) { static int unit_add_slice_dependencies(Unit *u) { assert(u); - if (!unit_get_cgroup_context(u)) + if (!UNIT_HAS_CGROUP_CONTEXT(u)) return 0; if (UNIT_ISSET(u->slice)) diff --git a/src/core/unit.h b/src/core/unit.h index f53b7f6da1..8da12356f8 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -439,6 +439,10 @@ extern const UnitVTable * const unit_vtable[_UNIT_TYPE_MAX]; /* For casting the various unit types into a unit */ #define UNIT(u) (&(u)->meta) +#define UNIT_HAS_EXEC_CONTEXT(u) (UNIT_VTABLE(u)->exec_context_offset > 0) +#define UNIT_HAS_CGROUP_CONTEXT(u) (UNIT_VTABLE(u)->cgroup_context_offset > 0) +#define UNIT_HAS_KILL_CONTEXT(u) (UNIT_VTABLE(u)->kill_context_offset > 0) + #define UNIT_TRIGGER(u) ((Unit*) set_first((u)->dependencies[UNIT_TRIGGERS])) DEFINE_CAST(SERVICE, Service); -- cgit v1.2.3-54-g00ecf From d79200e26ee39d4b451f95e876fc4595df51fe51 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 28 Aug 2015 17:36:39 +0200 Subject: unit: unify how we assing slices to units This adds a new call unit_set_slice(), and simplifies unit_add_default_slice(). THis should make our code a bit more robust and simpler. --- src/core/dbus-unit.c | 37 +++++++++++++++++++------------------ src/core/load-fragment.c | 22 ++++++++-------------- src/core/mount.c | 2 +- src/core/scope.c | 2 +- src/core/service.c | 2 +- src/core/socket.c | 2 +- src/core/swap.c | 2 +- src/core/unit.c | 35 +++++++++++++++++++++++++++++++---- src/core/unit.h | 3 ++- 9 files changed, 65 insertions(+), 42 deletions(-) diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index 91c31987fe..1e6291e762 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -965,38 +965,39 @@ static int bus_unit_set_transient_property( return 1; - } else if (streq(name, "Slice") && UNIT_HAS_CGROUP_CONTEXT(u)) { + } else if (streq(name, "Slice")) { + Unit *slice; const char *s; + if (!UNIT_HAS_CGROUP_CONTEXT(u)) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "The slice property is only available for units with control groups."); + if (u->type == UNIT_SLICE) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Slice may not be set for slice units."); + r = sd_bus_message_read(message, "s", &s); if (r < 0) return r; - if (!unit_name_is_valid(s, UNIT_NAME_PLAIN) || !endswith(s, ".slice")) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid slice name %s", s); + if (!unit_name_is_valid(s, UNIT_NAME_PLAIN)) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid unit name '%s'", s); - if (isempty(s)) { - if (mode != UNIT_CHECK) { - unit_ref_unset(&u->slice); - unit_remove_drop_in(u, mode, name); - } - } else { - Unit *slice; + r = manager_load_unit(u->manager, s, NULL, error, &slice); + if (r < 0) + return r; + + if (slice->type != UNIT_SLICE) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Unit name '%s' is not a slice", s); - r = manager_load_unit(u->manager, s, NULL, error, &slice); + if (mode != UNIT_CHECK) { + r = unit_set_slice(u, slice); if (r < 0) return r; - if (slice->type != UNIT_SLICE) - return -EINVAL; - - if (mode != UNIT_CHECK) { - unit_ref_set(&u->slice, slice); - unit_write_drop_in_private_format(u, mode, name, "Slice=%s\n", s); - } + unit_write_drop_in_private_format(u, mode, name, "Slice=%s\n", s); } return 1; + } else if (STR_IN_SET(name, "Requires", "RequiresOverridable", "Requisite", "RequisiteOverridable", diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 721da3470e..745291c5c6 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -2602,7 +2602,7 @@ int config_parse_unit_slice( void *userdata) { _cleanup_free_ char *k = NULL; - Unit *u = userdata, *slice; + Unit *u = userdata, *slice = NULL; int r; assert(filename); @@ -2611,29 +2611,23 @@ int config_parse_unit_slice( assert(u); r = unit_name_printf(u, rvalue, &k); - if (r < 0) - log_syntax(unit, LOG_ERR, filename, line, -r, - "Failed to resolve unit specifiers on %s. Ignoring.", rvalue); - if (!k) { - k = strdup(rvalue); - if (!k) - return log_oom(); + if (r < 0) { + log_syntax(unit, LOG_ERR, filename, line, r, "Failed to resolve unit specifiers on %s. Ignoring.", rvalue); + return 0; } r = manager_load_unit(u->manager, k, NULL, NULL, &slice); if (r < 0) { - log_syntax(unit, LOG_ERR, filename, line, -r, - "Failed to load slice unit %s. Ignoring.", k); + log_syntax(unit, LOG_ERR, filename, line, r, "Failed to load slice unit %s. Ignoring.", k); return 0; } - if (slice->type != UNIT_SLICE) { - log_syntax(unit, LOG_ERR, filename, line, EINVAL, - "Slice unit %s is not a slice. Ignoring.", k); + r = unit_set_slice(u, slice); + if (r < 0) { + log_syntax(unit, LOG_ERR, filename, line, r, "Failed to assign slice %s to unit %s. Ignoring.", slice->id, u->id); return 0; } - unit_ref_set(&u->slice, slice); return 0; } diff --git a/src/core/mount.c b/src/core/mount.c index 7e19e66a51..2b81d17b9c 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -521,7 +521,7 @@ static int mount_add_extras(Mount *m) { if (r < 0) return r; - r = unit_add_default_slice(u, &m->cgroup_context); + r = unit_set_default_slice(u); if (r < 0) return r; diff --git a/src/core/scope.c b/src/core/scope.c index bf89936153..c594ab5294 100644 --- a/src/core/scope.c +++ b/src/core/scope.c @@ -164,7 +164,7 @@ static int scope_load(Unit *u) { if (r < 0) return r; - r = unit_add_default_slice(u, &s->cgroup_context); + r = unit_set_default_slice(u); if (r < 0) return r; diff --git a/src/core/service.c b/src/core/service.c index 097e7c710c..3c4232417d 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -556,7 +556,7 @@ static int service_add_extras(Service *s) { if (r < 0) return r; - r = unit_add_default_slice(UNIT(s), &s->cgroup_context); + r = unit_set_default_slice(UNIT(s)); if (r < 0) return r; diff --git a/src/core/socket.c b/src/core/socket.c index aa65c5b7a0..1014fad626 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -345,7 +345,7 @@ static int socket_add_extras(Socket *s) { if (r < 0) return r; - r = unit_add_default_slice(u, &s->cgroup_context); + r = unit_set_default_slice(u); if (r < 0) return r; } diff --git a/src/core/swap.c b/src/core/swap.c index 349fd6f7b9..4f3ddc9f04 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -326,7 +326,7 @@ static int swap_load(Unit *u) { if (r < 0) return r; - r = unit_add_default_slice(u, &s->cgroup_context); + r = unit_set_default_slice(u); if (r < 0) return r; diff --git a/src/core/unit.c b/src/core/unit.c index 2ad49fd50b..9afb5736d7 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -2424,14 +2424,42 @@ char *unit_default_cgroup_path(Unit *u) { return strjoin(u->manager->cgroup_root, "/", escaped, NULL); } -int unit_add_default_slice(Unit *u, CGroupContext *c) { +int unit_set_slice(Unit *u, Unit *slice) { + assert(u); + assert(slice); + + /* Sets the unit slice if it has not been set before. Is extra + * careful, to only allow this for units that actually have a + * cgroup context. Also, we don't allow to set this for slices + * (since the parent slice is derived from the name). Make + * sure the unit we set is actually a slice. */ + + if (!UNIT_HAS_CGROUP_CONTEXT(u)) + return -EOPNOTSUPP; + + if (u->type == UNIT_SLICE) + return -EINVAL; + + if (slice->type != UNIT_SLICE) + return -EINVAL; + + if (UNIT_DEREF(u->slice) == slice) + return 0; + + if (UNIT_ISSET(u->slice)) + return -EBUSY; + + unit_ref_set(&u->slice, slice); + return 1; +} + +int unit_set_default_slice(Unit *u) { _cleanup_free_ char *b = NULL; const char *slice_name; Unit *slice; int r; assert(u); - assert(c); if (UNIT_ISSET(u->slice)) return 0; @@ -2471,8 +2499,7 @@ int unit_add_default_slice(Unit *u, CGroupContext *c) { if (r < 0) return r; - unit_ref_set(&u->slice, slice); - return 0; + return unit_set_slice(u, slice); } const char *unit_slice_name(Unit *u) { diff --git a/src/core/unit.h b/src/core/unit.h index 8da12356f8..bc26653247 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -494,7 +494,8 @@ int unit_load_fragment_and_dropin(Unit *u); int unit_load_fragment_and_dropin_optional(Unit *u); int unit_load(Unit *unit); -int unit_add_default_slice(Unit *u, CGroupContext *c); +int unit_set_slice(Unit *u, Unit *slice); +int unit_set_default_slice(Unit *u); const char *unit_description(Unit *u) _pure_; -- cgit v1.2.3-54-g00ecf From a1b4e6e93396c1c449aac4a2a1847033b8ce59f4 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 28 Aug 2015 18:17:14 +0200 Subject: pager: set $LESSCHARSET when we output UTF8 chars This way we can be sure that less has the same idea of the terminal as we do. This solves issues in systems that have locale uninitalized, where systemd would output UTF-8 but less wouldn't allow it and show them as control characters. --- src/shared/pager.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/shared/pager.c b/src/shared/pager.c index 85f8fa5b39..3992f9c837 100644 --- a/src/shared/pager.c +++ b/src/shared/pager.c @@ -82,7 +82,7 @@ int pager_open(bool jump_to_end) { /* In the child start the pager */ if (pager_pid == 0) { - const char* less_opts; + const char* less_opts, *less_charset; (void) reset_all_signal_handlers(); (void) reset_signal_mask(); @@ -90,6 +90,7 @@ int pager_open(bool jump_to_end) { dup2(fd[0], STDIN_FILENO); safe_close_pair(fd); + /* Initialize a good set of less options */ less_opts = getenv("SYSTEMD_LESS"); if (!less_opts) less_opts = "FRSXMK"; @@ -97,6 +98,15 @@ int pager_open(bool jump_to_end) { less_opts = strjoina(less_opts, " +G"); setenv("LESS", less_opts, 1); + /* Initialize a good charset for less. This is + * particularly important if we output UTF-8 + * characters. */ + less_charset = getenv("SYSTEMD_LESSCHARSET"); + if (!less_charset && is_locale_utf8()) + less_charset = "utf-8"; + if (less_charset) + setenv("LESSCHARSET", less_charset, 1); + /* Make sure the pager goes away when the parent dies */ if (prctl(PR_SET_PDEATHSIG, SIGTERM) < 0) _exit(EXIT_FAILURE); -- cgit v1.2.3-54-g00ecf From 9797f89bf0ec75a60066b162ab7c23dfb8e734e7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 28 Aug 2015 18:22:14 +0200 Subject: util: treat 'C' and 'POSIX' locale identical --- src/basic/util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/basic/util.c b/src/basic/util.c index a9071aa117..737f2a221c 100644 --- a/src/basic/util.c +++ b/src/basic/util.c @@ -4273,7 +4273,7 @@ bool is_locale_utf8(void) { /* Check result, but ignore the result if C was set * explicitly. */ cached_answer = - streq(set, "C") && + STR_IN_SET(set, "C", "POSIX") && !getenv("LC_ALL") && !getenv("LC_CTYPE") && !getenv("LANG"); -- cgit v1.2.3-54-g00ecf From 52f448c3fff64752649af4b8be5260c1a8a105ea Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 28 Aug 2015 18:29:02 +0200 Subject: unit: minor simplification --- src/core/unit.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core/unit.c b/src/core/unit.c index 9afb5736d7..5f602bdf5f 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -3135,17 +3135,17 @@ int unit_kill_common( int r = 0; - if (who == KILL_MAIN && main_pid <= 0) { + if (who == KILL_MAIN) { if (main_pid < 0) return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_PROCESS, "%s units have no main processes", unit_type_to_string(u->type)); - else + else if (main_pid == 0) return sd_bus_error_set_const(error, BUS_ERROR_NO_SUCH_PROCESS, "No main process to kill"); } - if (who == KILL_CONTROL && control_pid <= 0) { + if (who == KILL_CONTROL) { if (control_pid < 0) return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_PROCESS, "%s units have no control processes", unit_type_to_string(u->type)); - else + else if (control_pid == 0) return sd_bus_error_set_const(error, BUS_ERROR_NO_SUCH_PROCESS, "No control process to kill"); } -- cgit v1.2.3-54-g00ecf From 03af6492f071eb202a91eb154a7fc12edcac9087 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 28 Aug 2015 19:17:47 +0200 Subject: cgtop: show resource usage relative to cgroup root only This way the output is restricted to cgroups from a container when run in one. --- src/cgtop/cgtop.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/cgtop/cgtop.c b/src/cgtop/cgtop.c index 26b104f111..9e771d821b 100644 --- a/src/cgtop/cgtop.c +++ b/src/cgtop/cgtop.c @@ -348,21 +348,21 @@ static int refresh_one( return r; } -static int refresh(Hashmap *a, Hashmap *b, unsigned iteration) { +static int refresh(const char *root, Hashmap *a, Hashmap *b, unsigned iteration) { int r; assert(a); - r = refresh_one(SYSTEMD_CGROUP_CONTROLLER, "/", a, b, iteration, 0); + r = refresh_one(SYSTEMD_CGROUP_CONTROLLER, root, a, b, iteration, 0); if (r < 0) return r; - r = refresh_one("cpuacct", "/", a, b, iteration, 0); + r = refresh_one("cpuacct", root, a, b, iteration, 0); if (r < 0) return r; - r = refresh_one("memory", "/", a, b, iteration, 0); + r = refresh_one("memory", root, a, b, iteration, 0); if (r < 0) return r; - r = refresh_one("blkio", "/", a, b, iteration, 0); + r = refresh_one("blkio", root, a, b, iteration, 0); if (r < 0) return r; @@ -721,6 +721,7 @@ int main(int argc, char *argv[]) { unsigned iteration = 0; usec_t last_refresh = 0; bool quit = false, immediate_refresh = false; + _cleanup_free_ char *root = NULL; log_parse_environment(); log_open(); @@ -729,6 +730,12 @@ int main(int argc, char *argv[]) { if (r <= 0) goto finish; + r = cg_get_root_path(&root); + if (r < 0) { + log_error_errno(r, "Failed to get root control group path: %m"); + goto finish; + } + a = hashmap_new(&string_hash_ops); b = hashmap_new(&string_hash_ops); if (!a || !b) { @@ -751,7 +758,7 @@ int main(int argc, char *argv[]) { if (t >= last_refresh + arg_delay || immediate_refresh) { - r = refresh(a, b, iteration++); + r = refresh(root, a, b, iteration++); if (r < 0) goto finish; -- cgit v1.2.3-54-g00ecf From a6149b93afeb4e7b37e1313920ac2e0a91ff1c08 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 28 Aug 2015 19:29:03 +0200 Subject: process-util: trivial optimization --- src/basic/process-util.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/basic/process-util.c b/src/basic/process-util.c index 61f188467f..cff2d2a034 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -181,10 +181,10 @@ int is_kernel_thread(pid_t pid) { bool eof; FILE *f; - if (pid == 0) + if (pid == 0 || pid == 1) /* pid 1, and we ourselves certainly aren't a kernel thread */ return 0; - assert(pid > 0); + assert(pid > 1); p = procfs_file_alloca(pid, "cmdline"); f = fopen(p, "re"); -- cgit v1.2.3-54-g00ecf From cb88a0a4ae7e9a03edb8772b6e0d8d2fc79c9f6b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 28 Aug 2015 19:29:33 +0200 Subject: cgls: print the expressive error message we have --- src/cgls/cgls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cgls/cgls.c b/src/cgls/cgls.c index fae9398aa5..22f33f000d 100644 --- a/src/cgls/cgls.c +++ b/src/cgls/cgls.c @@ -160,7 +160,7 @@ static int get_cgroup_root(char **ret) { &error, ret); if (r < 0) - return log_error_errno(r, "Failed to query unit control group path: %m"); + return log_error_errno(r, "Failed to query unit control group path: %s", bus_error_message(&error, r)); return 0; } -- cgit v1.2.3-54-g00ecf From 41ba8b6e69ad79b6c8e603ac970720665c88a363 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 28 Aug 2015 19:31:07 +0200 Subject: cgtop: ignore kernel threads when counting tasks However, allow them to be counted in by specifying -k --- man/systemd-cgtop.xml | 8 ++++++++ src/cgtop/cgtop.c | 16 ++++++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/man/systemd-cgtop.xml b/man/systemd-cgtop.xml index e4bc22f278..14b503c3da 100644 --- a/man/systemd-cgtop.xml +++ b/man/systemd-cgtop.xml @@ -168,6 +168,14 @@ percentage. + + + + Include kernel threads when counting tasks in + control groups. By default, kernel threads are not included in + the count. + + diff --git a/src/cgtop/cgtop.c b/src/cgtop/cgtop.c index 9e771d821b..720af66fd2 100644 --- a/src/cgtop/cgtop.c +++ b/src/cgtop/cgtop.c @@ -30,6 +30,7 @@ #include "path-util.h" #include "terminal-util.h" +#include "process-util.h" #include "util.h" #include "hashmap.h" #include "cgroup-util.h" @@ -64,6 +65,7 @@ static unsigned arg_iterations = (unsigned) -1; static bool arg_batch = false; static bool arg_raw = false; static usec_t arg_delay = 1*USEC_PER_SEC; +static bool arg_kernel_threads = false; static enum { ORDER_PATH, @@ -154,8 +156,13 @@ static int process(const char *controller, const char *path, Hashmap *a, Hashmap return r; g->n_tasks = 0; - while (cg_read_pid(f, &pid) > 0) + while (cg_read_pid(f, &pid) > 0) { + + if (!arg_kernel_threads && is_kernel_thread(pid) > 0) + continue; + g->n_tasks++; + } if (g->n_tasks > 0) g->n_tasks_valid = true; @@ -565,6 +572,7 @@ static void help(void) { " -r --raw Provide raw (not human-readable) numbers\n" " --cpu=percentage Show CPU usage as percentage (default)\n" " --cpu=time Show CPU usage as time\n" + " -k Include kernel threads in task count\n" " -d --delay=DELAY Delay between updates\n" " -n --iterations=N Run for N iterations before exiting\n" " -b --batch Run in batch mode, accepting no input\n" @@ -600,7 +608,7 @@ static int parse_argv(int argc, char *argv[]) { assert(argc >= 1); assert(argv); - while ((c = getopt_long(argc, argv, "hptcmin:brd:", options, NULL)) >= 0) + while ((c = getopt_long(argc, argv, "hptcmin:brd:k", options, NULL)) >= 0) switch (c) { @@ -700,6 +708,10 @@ static int parse_argv(int argc, char *argv[]) { } break; + case 'k': + arg_kernel_threads = true; + break; + case '?': return -EINVAL; -- cgit v1.2.3-54-g00ecf From 3cb5beea0c484011fffbd50ae0aaaf71cc699eef Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sun, 30 Aug 2015 15:11:35 +0200 Subject: cgtop: recursively count cgroup member tasks When showing the number of tasks in a cgroup, recursively count tasks in child cgroups and include them in the number. This ensures that the number of tasks is cummulative the same way as memory, cpu and IO resources are. Old behaviour can be restored by passing the new --recursive=no switch. --- man/systemd-cgtop.xml | 13 ++++++- src/cgtop/cgtop.c | 96 +++++++++++++++++++++++++++++++++++++-------------- 2 files changed, 83 insertions(+), 26 deletions(-) diff --git a/man/systemd-cgtop.xml b/man/systemd-cgtop.xml index 14b503c3da..7f54b0b982 100644 --- a/man/systemd-cgtop.xml +++ b/man/systemd-cgtop.xml @@ -176,6 +176,18 @@ the count. + + + + Controls whether the number of tasks shown for + a control group shall include all tasks that are contained in + any of the child control groups as well. Takes a boolean + argument, defaults to yes. If enabled the + tasks in child control groups are included, if disabled only + the tasks in the control group itself are + counted. + + @@ -212,7 +224,6 @@ - Keys diff --git a/src/cgtop/cgtop.c b/src/cgtop/cgtop.c index 720af66fd2..040e42ea4b 100644 --- a/src/cgtop/cgtop.c +++ b/src/cgtop/cgtop.c @@ -66,6 +66,7 @@ static bool arg_batch = false; static bool arg_raw = false; static usec_t arg_delay = 1*USEC_PER_SEC; static bool arg_kernel_threads = false; +static bool arg_recursive = true; static enum { ORDER_PATH, @@ -109,7 +110,14 @@ static const char *maybe_format_bytes(char *buf, size_t l, bool is_valid, off_t return format_bytes(buf, l, t); } -static int process(const char *controller, const char *path, Hashmap *a, Hashmap *b, unsigned iteration) { +static int process( + const char *controller, + const char *path, + Hashmap *a, + Hashmap *b, + unsigned iteration, + Group **ret) { + Group *g; int r; @@ -303,6 +311,9 @@ static int process(const char *controller, const char *path, Hashmap *a, Hashmap g->io_iteration = iteration; } + if (ret) + *ret = g; + return 0; } @@ -312,9 +323,11 @@ static int refresh_one( Hashmap *a, Hashmap *b, unsigned iteration, - unsigned depth) { + unsigned depth, + Group **ret) { _cleanup_closedir_ DIR *d = NULL; + Group *ours; int r; assert(controller); @@ -324,7 +337,7 @@ static int refresh_one( if (depth > arg_depth) return 0; - r = process(controller, path, a, b, iteration); + r = process(controller, path, a, b, iteration, &ours); if (r < 0) return r; @@ -336,10 +349,13 @@ static int refresh_one( for (;;) { _cleanup_free_ char *fn = NULL, *p = NULL; + Group *child = NULL; r = cg_read_subgroup(d, &fn); - if (r <= 0) + if (r < 0) return r; + if (r == 0) + break; p = strjoin(path, "/", fn, NULL); if (!p) @@ -347,12 +363,30 @@ static int refresh_one( path_kill_slashes(p); - r = refresh_one(controller, p, a, b, iteration, depth + 1); + r = refresh_one(controller, p, a, b, iteration, depth + 1, &child); if (r < 0) return r; + + if (arg_recursive && + child && + child->n_tasks_valid && + streq(controller, SYSTEMD_CGROUP_CONTROLLER)) { + + /* Recursively sum up processes */ + + if (ours->n_tasks_valid) + ours->n_tasks += child->n_tasks; + else { + ours->n_tasks = child->n_tasks; + ours->n_tasks_valid = true; + } + } } - return r; + if (ret) + *ret = ours; + + return 1; } static int refresh(const char *root, Hashmap *a, Hashmap *b, unsigned iteration) { @@ -360,16 +394,16 @@ static int refresh(const char *root, Hashmap *a, Hashmap *b, unsigned iteration) assert(a); - r = refresh_one(SYSTEMD_CGROUP_CONTROLLER, root, a, b, iteration, 0); + r = refresh_one(SYSTEMD_CGROUP_CONTROLLER, root, a, b, iteration, 0, NULL); if (r < 0) return r; - r = refresh_one("cpuacct", root, a, b, iteration, 0); + r = refresh_one("cpuacct", root, a, b, iteration, 0, NULL); if (r < 0) return r; - r = refresh_one("memory", root, a, b, iteration, 0); + r = refresh_one("memory", root, a, b, iteration, 0, NULL); if (r < 0) return r; - r = refresh_one("blkio", root, a, b, iteration, 0); + r = refresh_one("blkio", root, a, b, iteration, 0, NULL); if (r < 0) return r; @@ -379,11 +413,11 @@ static int refresh(const char *root, Hashmap *a, Hashmap *b, unsigned iteration) static int group_compare(const void*a, const void *b) { const Group *x = *(Group**)a, *y = *(Group**)b; - if (arg_order != ORDER_TASKS) { + if (arg_order != ORDER_TASKS || arg_recursive) { /* Let's make sure that the parent is always before - * the child. Except when ordering by tasks, since - * that is actually not accumulative for all - * children. */ + * the child. Except when ordering by tasks and + * recursive summing is off, since that is actually + * not accumulative for all children. */ if (path_startswith(y->path, x->path)) return -1; @@ -573,6 +607,7 @@ static void help(void) { " --cpu=percentage Show CPU usage as percentage (default)\n" " --cpu=time Show CPU usage as time\n" " -k Include kernel threads in task count\n" + " --recursive=BOOL Sum up task count recursively\n" " -d --delay=DELAY Delay between updates\n" " -n --iterations=N Run for N iterations before exiting\n" " -b --batch Run in batch mode, accepting no input\n" @@ -587,23 +622,24 @@ static int parse_argv(int argc, char *argv[]) { ARG_DEPTH, ARG_CPU_TYPE, ARG_ORDER, + ARG_RECURSIVE, }; static const struct option options[] = { - { "help", no_argument, NULL, 'h' }, - { "version", no_argument, NULL, ARG_VERSION }, - { "delay", required_argument, NULL, 'd' }, - { "iterations", required_argument, NULL, 'n' }, - { "batch", no_argument, NULL, 'b' }, - { "raw", no_argument, NULL, 'r' }, - { "depth", required_argument, NULL, ARG_DEPTH }, - { "cpu", optional_argument, NULL, ARG_CPU_TYPE }, - { "order", required_argument, NULL, ARG_ORDER }, + { "help", no_argument, NULL, 'h' }, + { "version", no_argument, NULL, ARG_VERSION }, + { "delay", required_argument, NULL, 'd' }, + { "iterations", required_argument, NULL, 'n' }, + { "batch", no_argument, NULL, 'b' }, + { "raw", no_argument, NULL, 'r' }, + { "depth", required_argument, NULL, ARG_DEPTH }, + { "cpu", optional_argument, NULL, ARG_CPU_TYPE }, + { "order", required_argument, NULL, ARG_ORDER }, + { "recursive", required_argument, NULL, ARG_RECURSIVE }, {} }; - int c; - int r; + int c, r; assert(argc >= 1); assert(argv); @@ -712,6 +748,16 @@ static int parse_argv(int argc, char *argv[]) { arg_kernel_threads = true; break; + case ARG_RECURSIVE: + r = parse_boolean(optarg); + if (r < 0) { + log_error("Failed to parse --recursive= argument: %s", optarg); + return r; + } + + arg_recursive = r; + break; + case '?': return -EINVAL; -- cgit v1.2.3-54-g00ecf From 7fcfb7ee2f0c2562c0e102915cacbc3ec2c4b8f8 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sun, 30 Aug 2015 16:15:08 +0200 Subject: cgtop: allow toggling of --recursive= and -k at runtime --- man/systemd-cgtop.xml | 15 +++++++++++++++ src/cgtop/cgtop.c | 18 ++++++++++++++++-- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/man/systemd-cgtop.xml b/man/systemd-cgtop.xml index 7f54b0b982..fe17165544 100644 --- a/man/systemd-cgtop.xml +++ b/man/systemd-cgtop.xml @@ -277,6 +277,21 @@ respectively. + + k + + Toggle between including or excluding kernel + threads in control group task counts. + + + + r + + Toggle between recursively including or + excluding tasks in child control groups in control group task + counts. + + diff --git a/src/cgtop/cgtop.c b/src/cgtop/cgtop.c index 040e42ea4b..0791c5a2d0 100644 --- a/src/cgtop/cgtop.c +++ b/src/cgtop/cgtop.c @@ -895,6 +895,20 @@ int main(int argc, char *argv[]) { arg_cpu_type = arg_cpu_type == CPU_TIME ? CPU_PERCENT : CPU_TIME; break; + case 'k': + arg_kernel_threads = !arg_kernel_threads; + fprintf(stdout, "\nCounting kernel threads: %s.", yes_no(arg_kernel_threads)); + fflush(stdout); + sleep(1); + break; + + case 'r': + arg_recursive = !arg_recursive; + fprintf(stdout, "\nRecursive task counting: %s", yes_no(arg_recursive)); + fflush(stdout); + sleep(1); + break; + case '+': if (arg_delay < USEC_PER_SEC) arg_delay += USEC_PER_MSEC*250; @@ -923,8 +937,8 @@ int main(int argc, char *argv[]) { case 'h': fprintf(stdout, "\t<" ON "p" OFF "> By path; <" ON "t" OFF "> By tasks; <" ON "c" OFF "> By CPU; <" ON "m" OFF "> By memory; <" ON "i" OFF "> By I/O\n" - "\t<" ON "+" OFF "> Increase delay; <" ON "-" OFF "> Decrease delay; <" ON "%%" OFF "> Toggle time\n" - "\t<" ON "q" OFF "> Quit; <" ON "SPACE" OFF "> Refresh"); + "\t<" ON "+" OFF "> Inc. delay; <" ON "-" OFF "> Dec. delay; <" ON "%%" OFF "> Toggle time; <" ON "SPACE" OFF "> Refresh\n" + "\t<" ON "k" OFF "> Count kernel threads; <" ON "r" OFF "> Count recursively; <" ON "q" OFF "> Quit"); fflush(stdout); sleep(3); break; -- cgit v1.2.3-54-g00ecf From 6d3eefd28e653c42bc4a6e0e58dfd9581b5c6e0a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sun, 30 Aug 2015 16:38:52 +0200 Subject: man: document relationship between keys and switches of cgtop --- man/systemd-cgtop.xml | 71 ++++++++++++++++++++++++++++++--------------------- 1 file changed, 42 insertions(+), 29 deletions(-) diff --git a/man/systemd-cgtop.xml b/man/systemd-cgtop.xml index fe17165544..0e0ea3ba7a 100644 --- a/man/systemd-cgtop.xml +++ b/man/systemd-cgtop.xml @@ -165,7 +165,8 @@ Controls whether the CPU usage is shown as percentage or time. By default the CPU usage is shown as - percentage. + percentage. This setting may also be toggled at runtime by + pressing the % key. @@ -173,7 +174,8 @@ Include kernel threads when counting tasks in control groups. By default, kernel threads are not included in - the count. + the count. This setting may also be toggled at runtime by + pressing the k key. @@ -184,16 +186,18 @@ any of the child control groups as well. Takes a boolean argument, defaults to yes. If enabled the tasks in child control groups are included, if disabled only - the tasks in the control group itself are - counted. + the tasks in the control group itself are counted. This + setting may also be toggled at runtime by pressing the + r key. - Perform only this many iterations. A value of 0 - indicates that the program should run indefinitely. + Perform only this many iterations. A value of + 0 indicates that the program should run + indefinitely. @@ -201,10 +205,11 @@ Specify refresh delay in seconds (or if one of - ms, - us, + ms, us, min is specified as unit in this time - unit). + unit). This setting may also be increased and decreased at + runtime by pressing the + and + - keys. @@ -232,64 +237,72 @@ - h + h Shows a short help text. - SPACE + Immediately refresh output. - q + q Terminate the program. - - p - t - c - m - i + p + t + c + m + i Sort the control groups by path, number of - tasks, CPU load, memory usage, or IO load, respectively. - + tasks, CPU load, memory usage, or IO load, respectively. This + setting may also be controlled using the + command line + switch. - % + % Toggle between showing CPU time as time or - percentage. + percentage. This setting may also be controlled using the + command line switch. - + - - + + + - Increase or decrease refresh delay, - respectively. + respectively. This setting may also be controlled using the + command line + switch. - k + k Toggle between including or excluding kernel - threads in control group task counts. + threads in control group task counts. This setting may also be + controlled using the command line + switch. - r + r Toggle between recursively including or excluding tasks in child control groups in control group task - counts. + counts. This setting may also be controlled using the + command line + switch. -- cgit v1.2.3-54-g00ecf From 90990e28c9067b37cd5a8a90ed813a036fdd0bd0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sun, 30 Aug 2015 22:13:55 +0200 Subject: manager: remove ask-password fd from sd_event before closing it Otherwise we might attempt to remove a non-existing fd from epoll. --- src/core/manager.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/manager.c b/src/core/manager.c index ecea89c377..ede2a9910d 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -250,8 +250,8 @@ static int manager_dispatch_ask_password_fd(sd_event_source *source, static void manager_close_ask_password(Manager *m) { assert(m); - m->ask_password_inotify_fd = safe_close(m->ask_password_inotify_fd); m->ask_password_event_source = sd_event_source_unref(m->ask_password_event_source); + m->ask_password_inotify_fd = safe_close(m->ask_password_inotify_fd); m->have_ask_password = -EINVAL; } -- cgit v1.2.3-54-g00ecf From 324496eb25b51395a00430b28fac639502df9e3e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 31 Aug 2015 13:03:16 +0200 Subject: cgls: pretty print root cgroup path Make sure show it as "/" rather than empty string. --- src/cgls/cgls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cgls/cgls.c b/src/cgls/cgls.c index 22f33f000d..a8d910d532 100644 --- a/src/cgls/cgls.c +++ b/src/cgls/cgls.c @@ -263,7 +263,7 @@ int main(int argc, char *argv[]) { if (r < 0) goto finish; - printf("Control group %s:\n", root); + printf("Control group %s:\n", isempty(root) ? "/" : root); fflush(stdout); r = show_cgroup(SYSTEMD_CGROUP_CONTROLLER, root, NULL, 0, arg_kernel_threads, output_flags); -- cgit v1.2.3-54-g00ecf From 556c25cf8c1250cb529c9a95e3b20b6f4d19ffc2 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 31 Aug 2015 13:07:24 +0200 Subject: sd-event: improve debug message when we fail to remove and fd from an epoll Let's help users to debug issues with epoll fd removal by printing the name of the event source. --- src/libsystemd/sd-event/sd-event.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c index 0e33ced342..c419be820a 100644 --- a/src/libsystemd/sd-event/sd-event.c +++ b/src/libsystemd/sd-event/sd-event.c @@ -481,7 +481,8 @@ static void source_io_unregister(sd_event_source *s) { return; r = epoll_ctl(s->event->epoll_fd, EPOLL_CTL_DEL, s->io.fd, NULL); - assert_log(r >= 0); + if (r < 0) + log_debug_errno(errno, "Failed to remove source %s from epoll: %m", strna(s->description)); s->io.registered = false; } -- cgit v1.2.3-54-g00ecf From dcd719908229479b0b6ec14d6c1362eb82b1bbf3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 31 Aug 2015 13:29:46 +0200 Subject: cgtop: rework error handling Never report errors twice. --- src/cgtop/cgtop.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/src/cgtop/cgtop.c b/src/cgtop/cgtop.c index 0791c5a2d0..06a43d15e4 100644 --- a/src/cgtop/cgtop.c +++ b/src/cgtop/cgtop.c @@ -494,7 +494,7 @@ static int group_compare(const void*a, const void *b) { #define ON ANSI_HIGHLIGHT_ON #define OFF ANSI_HIGHLIGHT_OFF -static int display(Hashmap *a) { +static void display(Hashmap *a) { Iterator i; Group *g; Group **array; @@ -589,8 +589,6 @@ static int display(Hashmap *a) { putchar('\n'); } - - return 0; } static void help(void) { @@ -817,8 +815,10 @@ int main(int argc, char *argv[]) { if (t >= last_refresh + arg_delay || immediate_refresh) { r = refresh(root, a, b, iteration++); - if (r < 0) + if (r < 0) { + log_error_errno(r, "Failed to refresh: %m"); goto finish; + } group_hashmap_clear(b); @@ -830,9 +830,7 @@ int main(int argc, char *argv[]) { immediate_refresh = false; } - r = display(b); - if (r < 0) - goto finish; + display(b); if (arg_iterations && iteration >= arg_iterations) break; @@ -842,7 +840,7 @@ int main(int argc, char *argv[]) { fflush(stdout); if (arg_batch) - usleep(last_refresh + arg_delay - t); + (void) usleep(last_refresh + arg_delay - t); else { r = read_one_char(stdin, &key, last_refresh + arg_delay - t, NULL); if (r == -ETIMEDOUT) @@ -960,10 +958,5 @@ finish: group_hashmap_free(a); group_hashmap_free(b); - if (r < 0) { - log_error_errno(r, "Exiting with failure: %m"); - return EXIT_FAILURE; - } - - return EXIT_SUCCESS; + return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS; } -- cgit v1.2.3-54-g00ecf