From 5fe74e893c7939a360dc4eb75dbf3f540526c968 Mon Sep 17 00:00:00 2001 From: Szabolcs Fruhwald Date: Wed, 20 Feb 2019 12:38:50 -0800 Subject: [PATCH] cgtop: Fix processing of controllers other than CPU After debugging the issue with gdb, I found that the following change 94ddb08 "cgtop: Still try to get CPU statistics if controller-free" has introduced a bug, which prevents process(..) method processing memory and io controllers when cpu_accounting_is_cheap() is true. The obvious fix is to move this branch to be the last one, keeping the intended behavior of the above change, without having a negative effect on the other controllers. Fixes #11773 [systemd-cgtop no longer shows memory (and io) usage] --- src/cgtop/cgtop.c | 130 +++++++++++++++++++++++++++--------------------------- 1 file changed, 65 insertions(+), 65 deletions(-) diff --git a/src/cgtop/cgtop.c b/src/cgtop/cgtop.c index b3bda30..ab3b979 100644 --- a/src/cgtop/cgtop.c +++ b/src/cgtop/cgtop.c @@ -223,71 +223,6 @@ static int process( if (g->n_tasks > 0) g->n_tasks_valid = true; - } else if (STR_IN_SET(controller, "cpu", "cpuacct") || cpu_accounting_is_cheap()) { - _cleanup_free_ char *p = NULL, *v = NULL; - uint64_t new_usage; - nsec_t timestamp; - - if (is_root_cgroup(path)) { - r = procfs_cpu_get_usage(&new_usage); - if (r < 0) - return r; - } else if (all_unified) { - _cleanup_free_ char *val = NULL; - - if (!streq(controller, "cpu")) - return 0; - - r = cg_get_keyed_attribute("cpu", path, "cpu.stat", STRV_MAKE("usage_usec"), &val); - if (IN_SET(r, -ENOENT, -ENXIO)) - return 0; - if (r < 0) - return r; - - r = safe_atou64(val, &new_usage); - if (r < 0) - return r; - - new_usage *= NSEC_PER_USEC; - } else { - if (!streq(controller, "cpuacct")) - return 0; - - r = cg_get_path(controller, path, "cpuacct.usage", &p); - if (r < 0) - return r; - - r = read_one_line_file(p, &v); - if (r == -ENOENT) - return 0; - if (r < 0) - return r; - - r = safe_atou64(v, &new_usage); - if (r < 0) - return r; - } - - timestamp = now_nsec(CLOCK_MONOTONIC); - - if (g->cpu_iteration == iteration - 1 && - (nsec_t) new_usage > g->cpu_usage) { - - nsec_t x, y; - - x = timestamp - g->cpu_timestamp; - if (x < 1) - x = 1; - - y = (nsec_t) new_usage - g->cpu_usage; - g->cpu_fraction = (double) y / (double) x; - g->cpu_valid = true; - } - - g->cpu_usage = (nsec_t) new_usage; - g->cpu_timestamp = timestamp; - g->cpu_iteration = iteration; - } else if (streq(controller, "memory")) { if (is_root_cgroup(path)) { @@ -411,6 +346,71 @@ static int process( g->io_output = wr; g->io_timestamp = timestamp; g->io_iteration = iteration; + } else if (STR_IN_SET(controller, "cpu", "cpuacct") || cpu_accounting_is_cheap()) { + _cleanup_free_ char *p = NULL, *v = NULL; + uint64_t new_usage; + nsec_t timestamp; + + if (is_root_cgroup(path)) { + r = procfs_cpu_get_usage(&new_usage); + if (r < 0) + return r; + } else if (all_unified) { + _cleanup_free_ char *val = NULL; + + if (!streq(controller, "cpu")) + return 0; + + r = cg_get_keyed_attribute("cpu", path, "cpu.stat", STRV_MAKE("usage_usec"), &val); + if (IN_SET(r, -ENOENT, -ENXIO)) + return 0; + if (r < 0) + return r; + + r = safe_atou64(val, &new_usage); + if (r < 0) + return r; + + new_usage *= NSEC_PER_USEC; + } else { + if (!streq(controller, "cpuacct")) + return 0; + + r = cg_get_path(controller, path, "cpuacct.usage", &p); + if (r < 0) + return r; + + r = read_one_line_file(p, &v); + if (r == -ENOENT) + return 0; + if (r < 0) + return r; + + r = safe_atou64(v, &new_usage); + if (r < 0) + return r; + } + + timestamp = now_nsec(CLOCK_MONOTONIC); + + if (g->cpu_iteration == iteration - 1 && + (nsec_t) new_usage > g->cpu_usage) { + + nsec_t x, y; + + x = timestamp - g->cpu_timestamp; + if (x < 1) + x = 1; + + y = (nsec_t) new_usage - g->cpu_usage; + g->cpu_fraction = (double) y / (double) x; + g->cpu_valid = true; + } + + g->cpu_usage = (nsec_t) new_usage; + g->cpu_timestamp = timestamp; + g->cpu_iteration = iteration; + } if (ret) -- 2.7.4