Merge pull request #18553 from Werkov/cgroup-user-instance-controllers

Make (user) instance aware of delegated cgroup controllers

src/basic/cgroup-util.c

@@ -527,41 +527,22 @@ int cg_get_path(const char *controller, const char *path, const char *suffix, ch
         return 0;
 }
 
-static int controller_is_accessible(const char *controller) {
+static int controller_is_v1_accessible(const char *root, const char *controller) {
-        int r;
+        const char *cpath, *dn;
 
         assert(controller);
 
-        /* Checks whether a specific controller is accessible,
+        dn = controller_to_dirname(controller);
-         * i.e. its hierarchy mounted. In the unified hierarchy all
+        cpath = strjoina("/sys/fs/cgroup/", dn);
-         * controllers are considered accessible, except for the named
+        if (root)
-         * hierarchies */
+                /* Also check that:
+                 * - possible subcgroup is created at root,
+                 * - we can modify the hierarchy.
+                 * "Leak" cpath on stack */
+                cpath = strjoina(cpath, root, "/cgroup.procs");
 
-        if (!cg_controller_is_valid(controller))
+        if (laccess(cpath, root ? W_OK : F_OK) < 0)
-                return -EINVAL;
+                return -errno;
-
-        r = cg_all_unified();
-        if (r < 0)
-                return r;
-        if (r > 0) {
-                /* We don't support named hierarchies if we are using
-                 * the unified hierarchy. */
-
-                if (streq(controller, SYSTEMD_CGROUP_CONTROLLER))
-                        return 0;
-
-                if (startswith(controller, "name="))
-                        return -EOPNOTSUPP;
-
-        } else {
-                const char *cc, *dn;
-
-                dn = controller_to_dirname(controller);
-                cc = strjoina("/sys/fs/cgroup/", dn);
-
-                if (laccess(cc, F_OK) < 0)
-                        return -errno;
-        }
 
         return 0;
 }
@@ -572,10 +553,23 @@ int cg_get_path_and_check(const char *controller, const char *path, const char *
         assert(controller);
         assert(fs);
 
-        /* Check if the specified controller is actually accessible */
+        if (!cg_controller_is_valid(controller))
-        r = controller_is_accessible(controller);
+                return -EINVAL;
+
+        r = cg_all_unified();
         if (r < 0)
                 return r;
+        if (r > 0) {
+                /* In the unified hierarchy all controllers are considered accessible,
+                 * except for the named hierarchies */
+                if (startswith(controller, "name="))
+                        return -EOPNOTSUPP;
+        } else {
+                /* Check if the specified controller is actually accessible */
+                r = controller_is_v1_accessible(NULL, controller);
+                if (r < 0)
+                        return r;
+        }
 
         return cg_get_path(controller, path, suffix, fs);
 }
@@ -1892,7 +1886,7 @@ int cg_mask_from_string(const char *value, CGroupMask *ret) {
         return 0;
 }
 
-int cg_mask_supported(CGroupMask *ret) {
+int cg_mask_supported_subtree(const char *root, CGroupMask *ret) {
         CGroupMask mask;
         int r;
 
@@ -1904,15 +1898,11 @@ int cg_mask_supported(CGroupMask *ret) {
         if (r < 0)
                 return r;
         if (r > 0) {
-                _cleanup_free_ char *root = NULL, *controllers = NULL, *path = NULL;
+                _cleanup_free_ char *controllers = NULL, *path = NULL;
 
                 /* In the unified hierarchy we can read the supported and accessible controllers from
                  * the top-level cgroup attribute */
 
-                r = cg_get_root_path(&root);
-                if (r < 0)
-                        return r;
-
                 r = cg_get_path(SYSTEMD_CGROUP_CONTROLLER, root, "cgroup.controllers", &path);
                 if (r < 0)
                         return r;
@@ -1931,7 +1921,7 @@ int cg_mask_supported(CGroupMask *ret) {
         } else {
                 CGroupController c;
 
-                /* In the legacy hierarchy, we check which hierarchies are mounted. */
+                /* In the legacy hierarchy, we check which hierarchies are accessible. */
 
                 mask = 0;
                 for (c = 0; c < _CGROUP_CONTROLLER_MAX; c++) {
@@ -1942,7 +1932,7 @@ int cg_mask_supported(CGroupMask *ret) {
                                 continue;
 
                         n = cgroup_controller_to_string(c);
-                        if (controller_is_accessible(n) >= 0)
+                        if (controller_is_v1_accessible(root, n) >= 0)
                                 mask |= bit;
                 }
         }
@@ -1951,6 +1941,17 @@ int cg_mask_supported(CGroupMask *ret) {
         return 0;
 }
 
+int cg_mask_supported(CGroupMask *ret) {
+        _cleanup_free_ char *root = NULL;
+        int r;
+
+        r = cg_get_root_path(&root);
+        if (r < 0)
+                return r;
+
+        return cg_mask_supported_subtree(root, ret);
+}
+
 int cg_kernel_controllers(Set **ret) {
         _cleanup_set_free_free_ Set *controllers = NULL;
         _cleanup_fclose_ FILE *f = NULL;

src/basic/cgroup-util.h

@@ -260,6 +260,7 @@ int cg_slice_to_path(const char *unit, char **ret);
 typedef const char* (*cg_migrate_callback_t)(CGroupMask mask, void *userdata);
 
 int cg_mask_supported(CGroupMask *ret);
+int cg_mask_supported_subtree(const char *root, CGroupMask *ret);
 int cg_mask_from_string(const char *s, CGroupMask *ret);
 int cg_mask_to_string(CGroupMask mask, char **ret);
 

src/core/cgroup.c

@@ -1880,10 +1880,6 @@ int unit_pick_cgroup_path(Unit *u) {
         return 0;
 }
 
-static int cg_v1_errno_to_log_level(int r) {
-        return r == -EROFS ? LOG_DEBUG : LOG_WARNING;
-}
-
 static int unit_update_cgroup(
                 Unit *u,
                 CGroupMask target_mask,
@@ -1941,30 +1937,16 @@ static int unit_update_cgroup(
          * We perform migration also with whole slices for cases when users don't care about leave
          * granularity. Since delegated_mask is subset of target mask, we won't trim slice subtree containing
          * delegated units.
-         *
-         * If we're in an nspawn container and using legacy cgroups, the controller hierarchies are mounted
-         * read-only into the container. We skip migration/trim in this scenario since it would fail
-         * regardless with noisy "Read-only filesystem" warnings.
          */
         if (cg_all_unified() == 0) {
                 r = cg_migrate_v1_controllers(u->manager->cgroup_supported, migrate_mask, u->cgroup_path, migrate_callback, u);
                 if (r < 0)
-                        log_unit_full_errno(
+                        log_unit_warning_errno(u, r, "Failed to migrate controller cgroups from %s, ignoring: %m", u->cgroup_path);
-                                u,
-                                cg_v1_errno_to_log_level(r),
-                                r,
-                                "Failed to migrate controller cgroups from %s, ignoring: %m",
-                                u->cgroup_path);
 
                 is_root_slice = unit_has_name(u, SPECIAL_ROOT_SLICE);
                 r = cg_trim_v1_controllers(u->manager->cgroup_supported, ~target_mask, u->cgroup_path, !is_root_slice);
                 if (r < 0)
-                        log_unit_full_errno(
+                        log_unit_warning_errno(u, r, "Failed to delete controller cgroups %s, ignoring: %m", u->cgroup_path);
-                                u,
-                                cg_v1_errno_to_log_level(r),
-                                r,
-                                "Failed to delete controller cgroups %s, ignoring: %m",
-                                u->cgroup_path);
         }
 
         /* Set attributes */
@@ -3151,7 +3133,7 @@ int manager_setup_cgroup(Manager *m) {
                 (void) cg_set_attribute("memory", "/", "memory.use_hierarchy", "1");
 
         /* 8. Figure out which controllers are supported */
-        r = cg_mask_supported(&m->cgroup_supported);
+        r = cg_mask_supported_subtree(m->cgroup_root, &m->cgroup_supported);
         if (r < 0)
                 return log_error_errno(r, "Failed to determine supported controllers: %m");