core: simplify cg_[all_]unified()

cg_[all_]unified() test whether a specific controller or all controllers are on
the unified hierarchy.  While what's being asked is a simple binary question,
the callers must assume that the functions may fail any time, which
unnecessarily complicates their usages.  This complication is unnecessary.
Internally, the test result is cached anyway and there are only a few places
where the test actually needs to be performed.

This patch simplifies cg_[all_]unified().

* cg_[all_]unified() are updated to return bool.  If the result can't be
  decided, assertion failure is triggered.  Error handlings from their callers
  are dropped.

* cg_unified_flush() is updated to calculate the new result synchrnously and
  return whether it succeeded or not.  Places which need to flush the test
  result are updated to test for failure.  This ensures that all the following
  cg_[all_]unified() tests succeed.

* Places which expected possible cg_[all_]unified() failures are updated to
  call and test cg_unified_flush() before calling cg_[all_]unified().  This
  includes functions used while setting up mounts during boot and
  manager_setup_cgroup().

diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c
index 323720caef49797a6cd33c5f103f4f8a488846c5..bcb022896816450bfe101370a6d735e62c235433 100644 (file)
--- a/src/basic/cgroup-util.c
+++ b/src/basic/cgroup-util.c
@@ -210,12 +210,6 @@ int cg_rmdir(const char *controller, const char *path) {
         if (r < 0 && errno != ENOENT)
                 return -errno;
 
-        if (streq(controller, SYSTEMD_CGROUP_CONTROLLER) && cg_hybrid_unified()) {
-                r = cg_rmdir(SYSTEMD_CGROUP_CONTROLLER_LEGACY, path);
-                if (r < 0)
-                        log_warning_errno(r, "Failed to remove compat systemd cgroup %s: %m", path);
-        }
-
         return 0;
 }
 
@@ -554,13 +548,6 @@ static const char *controller_to_dirname(const char *controller) {
          * just cuts off the name= prefixed used for named
          * hierarchies, if it is specified. */
 
-        if (streq(controller, SYSTEMD_CGROUP_CONTROLLER)) {
-                if (cg_hybrid_unified())
-                        controller = SYSTEMD_CGROUP_CONTROLLER_HYBRID;
-                else
-                        controller = SYSTEMD_CGROUP_CONTROLLER_LEGACY;
-        }
-
         e = startswith(controller, "name=");
         if (e)
                 return e;
@@ -719,7 +706,7 @@ static int trim_cb(const char *path, const struct stat *sb, int typeflag, struct
 
 int cg_trim(const char *controller, const char *path, bool delete_root) {
         _cleanup_free_ char *fs = NULL;
-        int r = 0, q;
+        int r = 0;
 
         assert(path);
 
@@ -742,12 +729,6 @@ int cg_trim(const char *controller, const char *path, bool delete_root) {
                         return -errno;
         }
 
-        if (streq(controller, SYSTEMD_CGROUP_CONTROLLER) && cg_hybrid_unified()) {
-                q = cg_trim(SYSTEMD_CGROUP_CONTROLLER_LEGACY, path, delete_root);
-                if (q < 0)
-                        log_warning_errno(q, "Failed to trim compat systemd cgroup %s: %m", path);
-        }
-
         return r;
 }
 
@@ -771,12 +752,6 @@ int cg_create(const char *controller, const char *path) {
                 return -errno;
         }
 
-        if (streq(controller, SYSTEMD_CGROUP_CONTROLLER) && cg_hybrid_unified()) {
-                r = cg_create(SYSTEMD_CGROUP_CONTROLLER_LEGACY, path);
-                if (r < 0)
-                        log_warning_errno(r, "Failed to create compat systemd cgroup %s: %m", path);
-        }
-
         return 1;
 }
 
@@ -814,17 +789,7 @@ int cg_attach(const char *controller, const char *path, pid_t pid) {
 
         xsprintf(c, PID_FMT "\n", pid);
 
-        r = write_string_file(fs, c, 0);
-        if (r < 0)
-                return r;
-
-        if (streq(controller, SYSTEMD_CGROUP_CONTROLLER) && cg_hybrid_unified()) {
-                r = cg_attach(SYSTEMD_CGROUP_CONTROLLER_LEGACY, path, pid);
-                if (r < 0)
-                        log_warning_errno(r, "Failed to attach %d to compat systemd cgroup %s: %m", pid, path);
-        }
-
-        return 0;
+        return write_string_file(fs, c, 0);
 }
 
 int cg_attach_fallback(const char *controller, const char *path, pid_t pid) {
@@ -874,17 +839,7 @@ int cg_set_group_access(
         if (r < 0)
                 return r;
 
-        r = chmod_and_chown(fs, mode, uid, gid);
-        if (r < 0)
-                return r;
-
-        if (streq(controller, SYSTEMD_CGROUP_CONTROLLER) && cg_hybrid_unified()) {
-                r = cg_set_group_access(SYSTEMD_CGROUP_CONTROLLER_LEGACY, path, mode, uid, gid);
-                if (r < 0)
-                        log_warning_errno(r, "Failed to set group access on compat systemd cgroup %s: %m", path);
-        }
-
-        return 0;
+        return chmod_and_chown(fs, mode, uid, gid);
 }
 
 int cg_set_task_access(
@@ -913,18 +868,13 @@ int cg_set_task_access(
         if (r < 0)
                 return r;
 
-        if (!cg_unified(controller)) {
-                /* Compatibility, Always keep values for "tasks" in sync with
-                 * "cgroup.procs" */
-                if (cg_get_path(controller, path, "tasks", &procs) >= 0)
-                        (void) chmod_and_chown(procs, mode, uid, gid);
-        }
+        if (cg_unified(controller))
+                return 0;
 
-        if (streq(controller, SYSTEMD_CGROUP_CONTROLLER) && cg_hybrid_unified()) {
-                r = cg_set_task_access(SYSTEMD_CGROUP_CONTROLLER_LEGACY, path, mode, uid, gid);
-                if (r < 0)
-                        log_warning_errno(r, "Failed to set task access on compat systemd cgroup %s: %m", path);
-        }
+        /* Compatibility, Always keep values for "tasks" in sync with
+         * "cgroup.procs" */
+        if (cg_get_path(controller, path, "tasks", &procs) >= 0)
+                (void) chmod_and_chown(procs, mode, uid, gid);
 
         return 0;
 }
@@ -970,7 +920,7 @@ int cg_get_xattr(const char *controller, const char *path, const char *name, voi
 int cg_pid_get_path(const char *controller, pid_t pid, char **path) {
         _cleanup_fclose_ FILE *f = NULL;
         char line[LINE_MAX];
-        const char *fs, *controller_str;
+        const char *fs;
         size_t cs = 0;
         bool unified;
 
@@ -984,14 +934,8 @@ int cg_pid_get_path(const char *controller, pid_t pid, char **path) {
                 controller = SYSTEMD_CGROUP_CONTROLLER;
 
         unified = cg_unified(controller);
-        if (!unified) {
-                if (streq(controller, SYSTEMD_CGROUP_CONTROLLER))
-                        controller_str = SYSTEMD_CGROUP_CONTROLLER_LEGACY;
-                else
-                        controller_str = controller;
-
-                cs = strlen(controller_str);
-        }
+        if (!unified)
+                cs = strlen(controller);
 
         fs = procfs_file_alloca(pid, "cgroup");
         log_debug_elogind("Searching for PID %u in \"%s\" (controller \"%s\")",
@@ -1030,7 +974,7 @@ int cg_pid_get_path(const char *controller, pid_t pid, char **path) {
 
                         *e = 0;
                         FOREACH_WORD_SEPARATOR(word, k, l, ",", state) {
-                                if (k == cs && memcmp(word, controller_str, cs) == 0) {
+                                if (k == cs && memcmp(word, controller, cs) == 0) {
                                         found = true;
                                         break;
                                 }
@@ -1916,9 +1860,6 @@ bool cg_controller_is_valid(const char *p) {
         if (!p)
                 return false;
 
-        if (streq(p, SYSTEMD_CGROUP_CONTROLLER))
-                return true;
-
         s = startswith(p, "name=");
         if (s)
                 p = s;
@@ -2358,16 +2299,11 @@ static int cg_update_unified(void) {
         if (F_TYPE_EQUAL(fs.f_type, CGROUP2_SUPER_MAGIC))
                 unified_cache = CGROUP_UNIFIED_ALL;
         else if (F_TYPE_EQUAL(fs.f_type, TMPFS_MAGIC)) {
-                if (statfs("/sys/fs/cgroup/unified/", &fs) == 0 &&
-                    F_TYPE_EQUAL(fs.f_type, CGROUP2_SUPER_MAGIC))
-                        unified_cache = CGROUP_UNIFIED_SYSTEMD;
-                else {
-                        if (statfs("/sys/fs/cgroup/systemd/", &fs) < 0)
-                                return -errno;
-                        if (!F_TYPE_EQUAL(fs.f_type, CGROUP_SUPER_MAGIC))
-                                return -ENOMEDIUM;
-                        unified_cache = CGROUP_UNIFIED_NONE;
-                }
+                if (statfs("/sys/fs/cgroup/systemd/", &fs) < 0)
+                        return -errno;
+
+                unified_cache = F_TYPE_EQUAL(fs.f_type, CGROUP2_SUPER_MAGIC) ?
+                        CGROUP_UNIFIED_SYSTEMD : CGROUP_UNIFIED_NONE;
         } else
                 return -ENOMEDIUM;
 #else
@@ -2398,13 +2334,6 @@ bool cg_all_unified(void) {
 }
 
 #if 0 /// UNNEEDED by elogind
-bool cg_hybrid_unified(void) {
-
-        assert(cg_update_unified() >= 0);
-
-        return unified_cache == CGROUP_UNIFIED_SYSTEMD;
-}
-
 int cg_unified_flush(void) {
         unified_cache = CGROUP_UNIFIED_UNKNOWN;
 
@@ -2509,6 +2438,11 @@ bool cg_is_legacy_wanted(void) {
         /* The meaning of the kernel option is reversed wrt. to the return value
          * of this function, hence the negation. */
         return (wanted = r > 0 ? !b : false);
+        return (wanted = r > 0 ? b : false);
+}
+
+bool cg_is_legacy_systemd_controller_wanted(void) {
+        return cg_is_legacy_wanted() && !cg_is_unified_systemd_controller_wanted();
 }
 #endif // 0
 

diff --git a/src/basic/cgroup-util.h b/src/basic/cgroup-util.h
index f78f2355bc1717056dde63212af9012be91a3014..5fbea23d05406f231778ab83db3c48e146bfea16 100644 (file)
--- a/src/basic/cgroup-util.h
+++ b/src/basic/cgroup-util.h
@@ -255,7 +255,6 @@ bool cg_ns_supported(void);
 
 #if 0 /// UNNEEDED by elogind
 bool cg_all_unified(void);
-bool cg_hybrid_unified(void);
 bool cg_unified(const char *controller);
 int cg_unified_flush(void);
 

diff --git a/src/core/cgroup.c b/src/core/cgroup.c
index 5ee5bd79555b36cd10cb6619ba60f8ef6fa9e9ac..e306b8bceef179c59503ef700b9524ccc68b18d3 100644 (file)
--- a/src/core/cgroup.c
+++ b/src/core/cgroup.c
@@ -288,14 +288,24 @@ static int lookup_block_device(const char *p, dev_t *dev) {
 static int whitelist_device(const char *path, const char *node, const char *acc) {
         char buf[2+DECIMAL_STR_MAX(dev_t)*2+2+4];
         struct stat st;
+        bool ignore_notfound;
         int r;
 
         assert(path);
         assert(acc);
 
+        if (node[0] == '-') {
+                /* Non-existent paths starting with "-" must be silently ignored */
+                node++;
+                ignore_notfound = true;
+        } else
+                ignore_notfound = false;
+
         if (stat(node, &st) < 0) {
-                log_warning("Couldn't stat device %s", node);
-                return -errno;
+                if (errno == ENOENT && ignore_notfound)
+                        return 0;
+
+                return log_warning_errno(errno, "Couldn't stat device %s: %m", node);
         }
 
         if (!S_ISCHR(st.st_mode) && !S_ISBLK(st.st_mode)) {
@@ -669,7 +679,7 @@ static void cgroup_context_apply(Unit *u, CGroupMask mask, ManagerState state) {
                 bool has_weight = cgroup_context_has_cpu_weight(c);
                 bool has_shares = cgroup_context_has_cpu_shares(c);
 
-                if (cg_all_unified() > 0) {
+                if (cg_all_unified()) {
                         uint64_t weight;
 
                         if (has_weight)
@@ -849,7 +859,7 @@ static void cgroup_context_apply(Unit *u, CGroupMask mask, ManagerState state) {
         }
 
         if ((mask & CGROUP_MASK_MEMORY) && !is_root) {
-                if (cg_all_unified() > 0) {
+                if (cg_all_unified()) {
                         uint64_t max;
                         uint64_t swap_max = CGROUP_LIMIT_MAX;
 
@@ -916,8 +926,8 @@ static void cgroup_context_apply(Unit *u, CGroupMask mask, ManagerState state) {
                                 "/dev/pts/ptmx\0" "rw\0" /* /dev/pts/ptmx may not be duplicated, but accessed */
                                 /* Allow /run/elogind/inaccessible/{chr,blk} devices for mapping InaccessiblePaths */
                                 /* Allow /run/systemd/inaccessible/{chr,blk} devices for mapping InaccessiblePaths */
-                                "/run/systemd/inaccessible/chr\0" "rwm\0"
-                                "/run/systemd/inaccessible/blk\0" "rwm\0";
+                                "-/run/systemd/inaccessible/chr\0" "rwm\0"
+                                "-/run/systemd/inaccessible/blk\0" "rwm\0";
 
                         const char *x, *y;
 
@@ -1025,7 +1035,7 @@ CGroupMask unit_get_own_mask(Unit *u) {
                 e = unit_get_exec_context(u);
                 if (!e ||
                     exec_context_maintains_privileges(e) ||
-                    cg_all_unified() > 0)
+                    cg_all_unified())
                         return _CGROUP_MASK_ALL;
         }
 
@@ -1252,10 +1262,7 @@ int unit_watch_cgroup(Unit *u) {
                 return 0;
 
         /* Only applies to the unified hierarchy */
-        r = cg_unified(SYSTEMD_CGROUP_CONTROLLER);
-        if (r < 0)
-                return log_unit_error_errno(u, r, "Failed detect whether the unified hierarchy is used: %m");
-        if (r == 0)
+        if (!cg_unified(SYSTEMD_CGROUP_CONTROLLER))
                 return 0;
 
         /* Don't watch the root slice, it's pointless. */
@@ -1675,7 +1682,7 @@ int unit_watch_all_pids(Unit *u) {
         if (!u->cgroup_path)
                 return -ENOENT;
 
-        if (cg_unified(SYSTEMD_CGROUP_CONTROLLER) > 0) /* On unified we can use proper notifications */
+        if (cg_unified(SYSTEMD_CGROUP_CONTROLLER)) /* On unified we can use proper notifications */
                 return 0;
 
         return unit_watch_pids_in_path(u, u->cgroup_path);
@@ -1749,7 +1756,7 @@ static int on_cgroup_inotify_event(sd_event_source *s, int fd, uint32_t revents,
 int manager_setup_cgroup(Manager *m) {
         _cleanup_free_ char *path = NULL;
         CGroupController c;
-        int r, all_unified, systemd_unified;
+        int r;
         char *e;
 
         assert(m);
@@ -1790,16 +1797,13 @@ int manager_setup_cgroup(Manager *m) {
         if (r < 0)
                 return log_error_errno(r, "Cannot find cgroup mount point: %m");
 
-        all_unified = cg_all_unified();
-        systemd_unified = cg_unified(SYSTEMD_CGROUP_CONTROLLER);
-
-        if (all_unified < 0 || systemd_unified < 0)
-                return log_error_errno(all_unified < 0 ? all_unified : systemd_unified,
-                                       "Couldn't determine if we are running in the unified hierarchy: %m");
+        r = cg_unified_flush();
+        if (r < 0)
+                return log_error_errno(r, "Couldn't determine if we are running in the unified hierarchy: %m");
 
-        if (all_unified > 0)
+        if (cg_all_unified())
                 log_debug("Unified cgroup hierarchy is located at %s.", path);
-        else if (systemd_unified > 0)
+        else if (cg_unified(SYSTEMD_CGROUP_CONTROLLER))
                 log_debug("Unified cgroup hierarchy is located at %s. Controllers are on legacy hierarchies.", path);
         else
                 log_debug("Using cgroup controller " SYSTEMD_CGROUP_CONTROLLER ". File system hierarchy is at %s.", path);
@@ -1808,7 +1812,7 @@ int manager_setup_cgroup(Manager *m) {
                 const char *scope_path;
 
                 /* 3. Install agent */
-                if (systemd_unified) {
+                if (cg_unified(SYSTEMD_CGROUP_CONTROLLER)) {
 
                         /* In the unified hierarchy we can get
                          * cgroup empty notifications via inotify. */
@@ -1886,7 +1890,7 @@ int manager_setup_cgroup(Manager *m) {
                         return log_error_errno(errno, "Failed to open pin file: %m");
 
                 /* 6.  Always enable hierarchical support if it exists... */
-                if (!all_unified)
+                if (!cg_all_unified())
                         (void) cg_set_attribute("memory", "/", "memory.use_hierarchy", "1");
         }
 
@@ -2037,7 +2041,7 @@ int unit_get_memory_current(Unit *u, uint64_t *ret) {
         if ((u->cgroup_realized_mask & CGROUP_MASK_MEMORY) == 0)
                 return -ENODATA;
 
-        if (cg_all_unified() <= 0)
+        if (!cg_all_unified())
                 r = cg_get_attribute("memory", u->cgroup_path, "memory.usage_in_bytes", &v);
         else
                 r = cg_get_attribute("memory", u->cgroup_path, "memory.current", &v);
@@ -2082,7 +2086,7 @@ static int unit_get_cpu_usage_raw(Unit *u, nsec_t *ret) {
         if (!u->cgroup_path)
                 return -ENODATA;
 
-        if (cg_all_unified() > 0) {
+        if (cg_all_unified()) {
                 const char *keys[] = { "usage_usec", NULL };
                 _cleanup_free_ char *val = NULL;
                 uint64_t us;