From 613328c3e2d922283c388aafd25166e22e11a909 Mon Sep 17 00:00:00 2001 From: Anita Zhang Date: Sat, 7 Mar 2020 17:14:35 -0800 Subject: [PATCH 1/2] cgroup-util: helper to cg_get_attribute and convert to uint64_t A common pattern in the codebase is reading a cgroup memory value and converting it to a uint64_t. Let's make it a helper and refactor a few places to use it so it's more concise. --- src/basic/cgroup-util.c | 26 +++++++++++++++++++ src/basic/cgroup-util.h | 2 ++ src/basic/limits-util.c | 9 +------ src/core/cgroup.c | 57 ++++------------------------------------- 4 files changed, 34 insertions(+), 60 deletions(-) diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 489a155b48b..5df24343d1c 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -1639,6 +1639,32 @@ int cg_get_attribute(const char *controller, const char *path, const char *attri return read_one_line_file(p, ret); } +int cg_get_attribute_as_uint64(const char *controller, const char *path, const char *attribute, uint64_t *ret) { + _cleanup_free_ char *value = NULL; + uint64_t v; + int r; + + assert(ret); + + r = cg_get_attribute(controller, path, attribute, &value); + if (r == -ENOENT) + return -ENODATA; + if (r < 0) + return r; + + if (streq(value, "max")) { + *ret = CGROUP_LIMIT_MAX; + return 0; + } + + r = safe_atou64(value, &v); + if (r < 0) + return r; + + *ret = v; + return 0; +} + int cg_get_keyed_attribute( const char *controller, const char *path, diff --git a/src/basic/cgroup-util.h b/src/basic/cgroup-util.h index 300555f1aca..f5b1ffcc3d4 100644 --- a/src/basic/cgroup-util.h +++ b/src/basic/cgroup-util.h @@ -184,6 +184,8 @@ int cg_set_attribute(const char *controller, const char *path, const char *attri int cg_get_attribute(const char *controller, const char *path, const char *attribute, char **ret); int cg_get_keyed_attribute(const char *controller, const char *path, const char *attribute, char **keys, char **values); +int cg_get_attribute_as_uint64(const char *controller, const char *path, const char *attribute, uint64_t *ret); + int cg_set_access(const char *controller, const char *path, uid_t uid, gid_t gid); int cg_set_xattr(const char *controller, const char *path, const char *name, const void *value, size_t size, int flags); diff --git a/src/basic/limits-util.c b/src/basic/limits-util.c index a74d8197ea7..43f6b6f68a9 100644 --- a/src/basic/limits-util.c +++ b/src/basic/limits-util.c @@ -120,16 +120,9 @@ uint64_t system_tasks_max(void) { if (r < 0) log_debug_errno(r, "Failed to determine cgroup root path, ignoring: %m"); else { - _cleanup_free_ char *value = NULL; - - r = cg_get_attribute("pids", root, "pids.max", &value); + r = cg_get_attribute_as_uint64("pids", root, "pids.max", &b); if (r < 0) log_debug_errno(r, "Failed to read pids.max attribute of cgroup root, ignoring: %m"); - else if (!streq(value, "max")) { - r = safe_atou64(value, &b); - if (r < 0) - log_debug_errno(r, "Failed to parse pids.max attribute of cgroup root, ignoring: %m"); - } } return MIN3(TASKS_MAX, diff --git a/src/core/cgroup.c b/src/core/cgroup.c index ddd3f408174..5e4fe600a29 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -215,31 +215,12 @@ void cgroup_context_done(CGroupContext *c) { } static int unit_get_kernel_memory_limit(Unit *u, const char *file, uint64_t *ret) { - _cleanup_free_ char *raw_kval = NULL; - uint64_t kval; - int r; - assert(u); if (!u->cgroup_realized) return -EOWNERDEAD; - r = cg_get_attribute("memory", u->cgroup_path, file, &raw_kval); - if (r < 0) - return r; - - if (streq(raw_kval, "max")) { - *ret = CGROUP_LIMIT_MAX; - return 0; - } - - r = safe_atou64(raw_kval, &kval); - if (r < 0) - return r; - - *ret = kval; - - return 0; + return cg_get_attribute_as_uint64("memory", u->cgroup_path, file, ret); } static int unit_compare_memory_limit(Unit *u, const char *property_name, uint64_t *ret_unit_value, uint64_t *ret_kernel_value) { @@ -3112,7 +3093,6 @@ int manager_notify_cgroup_empty(Manager *m, const char *cgroup) { } int unit_get_memory_current(Unit *u, uint64_t *ret) { - _cleanup_free_ char *v = NULL; int r; assert(u); @@ -3134,22 +3114,11 @@ int unit_get_memory_current(Unit *u, uint64_t *ret) { r = cg_all_unified(); if (r < 0) return r; - if (r > 0) - r = cg_get_attribute("memory", u->cgroup_path, "memory.current", &v); - else - r = cg_get_attribute("memory", u->cgroup_path, "memory.usage_in_bytes", &v); - if (r == -ENOENT) - return -ENODATA; - if (r < 0) - return r; - return safe_atou64(v, ret); + return cg_get_attribute_as_uint64("memory", u->cgroup_path, r > 0 ? "memory.current" : "memory.usage_in_bytes", ret); } int unit_get_tasks_current(Unit *u, uint64_t *ret) { - _cleanup_free_ char *v = NULL; - int r; - assert(u); assert(ret); @@ -3166,17 +3135,10 @@ int unit_get_tasks_current(Unit *u, uint64_t *ret) { if ((u->cgroup_realized_mask & CGROUP_MASK_PIDS) == 0) return -ENODATA; - r = cg_get_attribute("pids", u->cgroup_path, "pids.current", &v); - if (r == -ENOENT) - return -ENODATA; - if (r < 0) - return r; - - return safe_atou64(v, ret); + return cg_get_attribute_as_uint64("pids", u->cgroup_path, "pids.current", ret); } static int unit_get_cpu_usage_raw(Unit *u, nsec_t *ret) { - _cleanup_free_ char *v = NULL; uint64_t ns; int r; @@ -3212,17 +3174,8 @@ static int unit_get_cpu_usage_raw(Unit *u, nsec_t *ret) { return r; ns = us * NSEC_PER_USEC; - } else { - r = cg_get_attribute("cpuacct", u->cgroup_path, "cpuacct.usage", &v); - if (r == -ENOENT) - return -ENODATA; - if (r < 0) - return r; - - r = safe_atou64(v, &ns); - if (r < 0) - return r; - } + } else + return cg_get_attribute_as_uint64("cpuacct", u->cgroup_path, "cpuacct.usage", ret); *ret = ns; return 0; From baa358df3247f5e9d1d8d2791b4edd886bebe539 Mon Sep 17 00:00:00 2001 From: Anita Zhang Date: Tue, 17 Mar 2020 17:47:19 -0700 Subject: [PATCH 2/2] cgroup-util: cg_get_xattr_malloc helper `cg_get_xattr_malloc` to read a cgroup xattr value and allocate space to hold said value (simple helper combining existing functions). --- src/basic/cgroup-util.c | 19 +++++++++++++++++++ src/basic/cgroup-util.h | 1 + 2 files changed, 20 insertions(+) diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 5df24343d1c..be73cfa3f0d 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -37,6 +37,7 @@ #include "strv.h" #include "unit-name.h" #include "user-util.h" +#include "xattr-util.h" static int cg_enumerate_items(const char *controller, const char *path, FILE **_f, const char *item) { _cleanup_free_ char *fs = NULL; @@ -605,6 +606,24 @@ int cg_get_xattr(const char *controller, const char *path, const char *name, voi return (int) n; } +int cg_get_xattr_malloc(const char *controller, const char *path, const char *name, char **ret) { + _cleanup_free_ char *fs = NULL; + int r; + + assert(path); + assert(name); + + r = cg_get_path(controller, path, NULL, &fs); + if (r < 0) + return r; + + r = getxattr_malloc(fs, name, ret, false); + if (r < 0) + return r; + + return r; +} + int cg_remove_xattr(const char *controller, const char *path, const char *name) { _cleanup_free_ char *fs = NULL; int r; diff --git a/src/basic/cgroup-util.h b/src/basic/cgroup-util.h index f5b1ffcc3d4..237139fad00 100644 --- a/src/basic/cgroup-util.h +++ b/src/basic/cgroup-util.h @@ -190,6 +190,7 @@ int cg_set_access(const char *controller, const char *path, uid_t uid, gid_t gid int cg_set_xattr(const char *controller, const char *path, const char *name, const void *value, size_t size, int flags); int cg_get_xattr(const char *controller, const char *path, const char *name, void *value, size_t size); +int cg_get_xattr_malloc(const char *controller, const char *path, const char *name, char **ret); int cg_remove_xattr(const char *controller, const char *path, const char *name); int cg_install_release_agent(const char *controller, const char *agent);