From: Mike Yuan <me@yhndnzj.com>
Date: Sat, 1 Jun 2024 23:50:09 +0000 (+0800)
Subject: core: unify reset_accounting handling
X-Git-Tag: v257-rc1~1014^2~6
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=bc347edfe075f36bada0e1e7aefd62b10fcad4a7;p=thirdparty%2Fsystemd.git

core: unify reset_accounting handling

Since the introduction of CGroupRuntime, there's no need
to call *_reset_accounting in unit_new(), hence make those
static. While at it, refrain from hardcoding default values
in cgroup_runtime_new(), but call the corresponding funcs.

This also corrects the default value of io_accounting_base.

Fixes #33482
---

diff --git a/src/core/cgroup.c b/src/core/cgroup.c
index dec47e14f75..47413e6bc37 100644
--- a/src/core/cgroup.c
+++ b/src/core/cgroup.c
@@ -4593,15 +4593,14 @@ int unit_get_tasks_current(Unit *u, uint64_t *ret) {
         return cg_get_attribute_as_uint64("pids", crt->cgroup_path, "pids.current", ret);
 }
 
-static int unit_get_cpu_usage_raw(Unit *u, nsec_t *ret) {
-        uint64_t ns;
+static int unit_get_cpu_usage_raw(const Unit *u, const CGroupRuntime *crt, nsec_t *ret) {
         int r;
 
         assert(u);
+        assert(crt);
         assert(ret);
 
-        CGroupRuntime *crt = unit_get_cgroup_runtime(u);
-        if (!crt || !crt->cgroup_path)
+        if (!crt->cgroup_path)
                 return -ENODATA;
 
         /* The root cgroup doesn't expose this information, let's get it from /proc instead */
@@ -4615,25 +4614,24 @@ static int unit_get_cpu_usage_raw(Unit *u, nsec_t *ret) {
         r = cg_all_unified();
         if (r < 0)
                 return r;
-        if (r > 0) {
-                _cleanup_free_ char *val = NULL;
-                uint64_t us;
+        if (r == 0)
+                return cg_get_attribute_as_uint64("cpuacct", crt->cgroup_path, "cpuacct.usage", ret);
 
-                r = cg_get_keyed_attribute("cpu", crt->cgroup_path, "cpu.stat", STRV_MAKE("usage_usec"), &val);
-                if (IN_SET(r, -ENOENT, -ENXIO))
-                        return -ENODATA;
-                if (r < 0)
-                        return r;
+        _cleanup_free_ char *val = NULL;
+        uint64_t us;
 
-                r = safe_atou64(val, &us);
-                if (r < 0)
-                        return r;
+        r = cg_get_keyed_attribute("cpu", crt->cgroup_path, "cpu.stat", STRV_MAKE("usage_usec"), &val);
+        if (IN_SET(r, -ENOENT, -ENXIO))
+                return -ENODATA;
+        if (r < 0)
+                return r;
 
-                ns = us * NSEC_PER_USEC;
-        } else
-                return cg_get_attribute_as_uint64("cpuacct", crt->cgroup_path, "cpuacct.usage", ret);
+        r = safe_atou64(val, &us);
+        if (r < 0)
+                return r;
+
+        *ret = us * NSEC_PER_USEC;
 
-        *ret = ns;
         return 0;
 }
 
@@ -4654,7 +4652,7 @@ int unit_get_cpu_usage(Unit *u, nsec_t *ret) {
         if (!UNIT_CGROUP_BOOL(u, cpu_accounting))
                 return -ENODATA;
 
-        r = unit_get_cpu_usage_raw(u, &ns);
+        r = unit_get_cpu_usage_raw(u, crt, &ns);
         if (r == -ENODATA && crt->cpu_usage_last != NSEC_INFINITY) {
                 /* If we can't get the CPU usage anymore (because the cgroup was already removed, for example), use our
                  * cached value. */
@@ -4771,22 +4769,27 @@ int unit_get_effective_limit(Unit *u, CGroupLimitType type, uint64_t *ret) {
         return 0;
 }
 
-static int unit_get_io_accounting_raw(Unit *u, uint64_t ret[static _CGROUP_IO_ACCOUNTING_METRIC_MAX]) {
-        static const char *const field_names[_CGROUP_IO_ACCOUNTING_METRIC_MAX] = {
+static int unit_get_io_accounting_raw(
+                const Unit *u,
+                const CGroupRuntime *crt,
+                uint64_t ret[static _CGROUP_IO_ACCOUNTING_METRIC_MAX]) {
+
+        static const char* const field_names[_CGROUP_IO_ACCOUNTING_METRIC_MAX] = {
                 [CGROUP_IO_READ_BYTES]       = "rbytes=",
                 [CGROUP_IO_WRITE_BYTES]      = "wbytes=",
                 [CGROUP_IO_READ_OPERATIONS]  = "rios=",
                 [CGROUP_IO_WRITE_OPERATIONS] = "wios=",
         };
+
         uint64_t acc[_CGROUP_IO_ACCOUNTING_METRIC_MAX] = {};
         _cleanup_free_ char *path = NULL;
         _cleanup_fclose_ FILE *f = NULL;
         int r;
 
         assert(u);
+        assert(crt);
 
-        CGroupRuntime *crt = unit_get_cgroup_runtime(u);
-        if (!crt || !crt->cgroup_path)
+        if (!crt->cgroup_path)
                 return -ENODATA;
 
         if (unit_has_host_root_cgroup(u))
@@ -4876,7 +4879,7 @@ int unit_get_io_accounting(
         if (allow_cache && crt->io_accounting_last[metric] != UINT64_MAX)
                 goto done;
 
-        r = unit_get_io_accounting_raw(u, raw);
+        r = unit_get_io_accounting_raw(u, crt, raw);
         if (r == -ENODATA && crt->io_accounting_last[metric] != UINT64_MAX)
                 goto done;
         if (r < 0)
@@ -4897,45 +4900,52 @@ done:
         return 0;
 }
 
-int unit_reset_cpu_accounting(Unit *u) {
+static int unit_reset_cpu_accounting(Unit *unit, CGroupRuntime *crt) {
         int r;
 
-        assert(u);
-
-        CGroupRuntime *crt = unit_get_cgroup_runtime(u);
-        if (!crt || !crt->cgroup_path)
-                return 0;
+        assert(crt);
 
+        crt->cpu_usage_base = 0;
         crt->cpu_usage_last = NSEC_INFINITY;
 
-        r = unit_get_cpu_usage_raw(u, &crt->cpu_usage_base);
-        if (r < 0) {
-                crt->cpu_usage_base = 0;
-                return r;
+        if (unit) {
+                r = unit_get_cpu_usage_raw(unit, crt, &crt->cpu_usage_base);
+                if (r < 0 && r != -ENODATA)
+                        return r;
         }
 
         return 0;
 }
 
-void unit_reset_memory_accounting_last(Unit *u) {
-        assert(u);
+static int unit_reset_io_accounting(Unit *unit, CGroupRuntime *crt) {
+        int r;
 
-        CGroupRuntime *crt = unit_get_cgroup_runtime(u);
-        if (!crt || !crt->cgroup_path)
-                return;
+        assert(crt);
+
+        zero(crt->io_accounting_base);
+        FOREACH_ELEMENT(i, crt->io_accounting_last)
+                *i = UINT64_MAX;
+
+        if (unit) {
+                r = unit_get_io_accounting_raw(unit, crt, crt->io_accounting_base);
+                if (r < 0 && r != -ENODATA)
+                        return r;
+        }
+
+        return 0;
+}
+
+static void cgroup_runtime_reset_memory_accounting_last(CGroupRuntime *crt) {
+        assert(crt);
 
         FOREACH_ELEMENT(i, crt->memory_accounting_last)
                 *i = UINT64_MAX;
 }
 
-int unit_reset_ip_accounting(Unit *u) {
+static int cgroup_runtime_reset_ip_accounting(CGroupRuntime *crt) {
         int r = 0;
 
-        assert(u);
-
-        CGroupRuntime *crt = unit_get_cgroup_runtime(u);
-        if (!crt || !crt->cgroup_path)
-                return 0;
+        assert(crt);
 
         if (crt->ip_accounting_ingress_map_fd >= 0)
                 RET_GATHER(r, bpf_firewall_reset_accounting(crt->ip_accounting_ingress_map_fd));
@@ -4948,46 +4958,19 @@ int unit_reset_ip_accounting(Unit *u) {
         return r;
 }
 
-void unit_reset_io_accounting_last(Unit *u) {
-        assert(u);
-
-        CGroupRuntime *crt = unit_get_cgroup_runtime(u);
-        if (!crt || !crt->cgroup_path)
-                return;
-
-        FOREACH_ARRAY(i, crt->io_accounting_last, _CGROUP_IO_ACCOUNTING_METRIC_MAX)
-                *i = UINT64_MAX;
-}
-
-int unit_reset_io_accounting(Unit *u) {
-        int r;
+int unit_reset_accounting(Unit *u) {
+        int r = 0;
 
         assert(u);
 
         CGroupRuntime *crt = unit_get_cgroup_runtime(u);
-        if (!crt || !crt->cgroup_path)
+        if (!crt)
                 return 0;
 
-        unit_reset_io_accounting_last(u);
-
-        r = unit_get_io_accounting_raw(u, crt->io_accounting_base);
-        if (r < 0) {
-                zero(crt->io_accounting_base);
-                return r;
-        }
-
-        return 0;
-}
-
-int unit_reset_accounting(Unit *u) {
-        int r = 0;
-
-        assert(u);
-
-        RET_GATHER(r, unit_reset_cpu_accounting(u));
-        RET_GATHER(r, unit_reset_io_accounting(u));
-        RET_GATHER(r, unit_reset_ip_accounting(u));
-        unit_reset_memory_accounting_last(u);
+        cgroup_runtime_reset_memory_accounting_last(crt);
+        RET_GATHER(r, unit_reset_cpu_accounting(u, crt));
+        RET_GATHER(r, unit_reset_io_accounting(u, crt));
+        RET_GATHER(r, cgroup_runtime_reset_ip_accounting(crt));
 
         return r;
 }
@@ -5211,7 +5194,7 @@ int unit_get_cpuset(Unit *u, CPUSet *cpus, const char *name) {
         return parse_cpu_set_full(v, cpus, false, NULL, NULL, 0, NULL);
 }
 
-CGroupRuntime *cgroup_runtime_new(void) {
+CGroupRuntime* cgroup_runtime_new(void) {
         _cleanup_(cgroup_runtime_freep) CGroupRuntime *crt = NULL;
 
         crt = new(CGroupRuntime, 1);
@@ -5219,8 +5202,6 @@ CGroupRuntime *cgroup_runtime_new(void) {
                 return NULL;
 
         *crt = (CGroupRuntime) {
-                .cpu_usage_last = NSEC_INFINITY,
-
                 .cgroup_control_inotify_wd = -1,
                 .cgroup_memory_inotify_wd = -1,
 
@@ -5235,19 +5216,15 @@ CGroupRuntime *cgroup_runtime_new(void) {
                 .cgroup_invalidated_mask = _CGROUP_MASK_ALL,
         };
 
-        FOREACH_ELEMENT(i, crt->memory_accounting_last)
-                *i = UINT64_MAX;
-        FOREACH_ELEMENT(i, crt->io_accounting_base)
-                *i = UINT64_MAX;
-        FOREACH_ELEMENT(i, crt->io_accounting_last)
-                *i = UINT64_MAX;
-        FOREACH_ELEMENT(i, crt->ip_accounting_extra)
-                *i = UINT64_MAX;
+        unit_reset_cpu_accounting(/* unit = */ NULL, crt);
+        unit_reset_io_accounting(/* unit = */ NULL, crt);
+        cgroup_runtime_reset_memory_accounting_last(crt);
+        assert_se(cgroup_runtime_reset_ip_accounting(crt) >= 0);
 
         return TAKE_PTR(crt);
 }
 
-CGroupRuntime *cgroup_runtime_free(CGroupRuntime *crt) {
+CGroupRuntime* cgroup_runtime_free(CGroupRuntime *crt) {
         if (!crt)
                 return NULL;
 
diff --git a/src/core/cgroup.h b/src/core/cgroup.h
index 08fccf96126..7bc849c5428 100644
--- a/src/core/cgroup.h
+++ b/src/core/cgroup.h
@@ -489,11 +489,6 @@ int unit_get_io_accounting(Unit *u, CGroupIOAccountingMetric metric, bool allow_
 int unit_get_ip_accounting(Unit *u, CGroupIPAccountingMetric metric, uint64_t *ret);
 int unit_get_effective_limit(Unit *u, CGroupLimitType type, uint64_t *ret);
 
-int unit_reset_cpu_accounting(Unit *u);
-void unit_reset_memory_accounting_last(Unit *u);
-int unit_reset_ip_accounting(Unit *u);
-void unit_reset_io_accounting_last(Unit *u);
-int unit_reset_io_accounting(Unit *u);
 int unit_reset_accounting(Unit *u);
 
 #define UNIT_CGROUP_BOOL(u, name)                               \
@@ -527,8 +522,8 @@ int unit_cgroup_freezer_action(Unit *u, FreezerAction action);
 const char* freezer_action_to_string(FreezerAction a) _const_;
 FreezerAction freezer_action_from_string(const char *s) _pure_;
 
-CGroupRuntime *cgroup_runtime_new(void);
-CGroupRuntime *cgroup_runtime_free(CGroupRuntime *crt);
+CGroupRuntime* cgroup_runtime_new(void);
+CGroupRuntime* cgroup_runtime_free(CGroupRuntime *crt);
 DEFINE_TRIVIAL_CLEANUP_FUNC(CGroupRuntime*, cgroup_runtime_free);
 
 int cgroup_runtime_serialize(Unit *u, FILE *f, FDSet *fds);
diff --git a/src/core/unit.c b/src/core/unit.c
index 9d64103aef7..f5563c1fe93 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -129,9 +129,6 @@ Unit* unit_new(Manager *m, size_t size) {
                 .burst = 16
         };
 
-        unit_reset_memory_accounting_last(u);
-        unit_reset_io_accounting_last(u);
-
         return u;
 }