chiark / gitweb /
pid1: do not reset subtree_control on already-existing units with delegation
authorZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Tue, 29 May 2018 10:19:09 +0000 (12:19 +0200)
committerSven Eden <yamakuzure@gmx.net>
Fri, 24 Aug 2018 14:47:08 +0000 (16:47 +0200)
Fixes #8364.

Reproducer:
$ sudo systemd-run -t -p Delegate=yes bash
# mkdir /sys/fs/cgroup/system.slice/run-u6958.service/supervisor
# echo $$ > /sys/fs/cgroup/system.slice/run-u6958.service/supervisor/cgroup.procs
# echo +memory > /sys/fs/cgroup/system.slice/run-u6958.service/cgroup.subtree_control
# cat /sys/fs/cgroup/system.slice/run-u6958.service/cgroup.subtree_control
memory
# systemctl daemon-reload
# cat /sys/fs/cgroup/system.slice/run-u6958.service/cgroup.subtree_control
(empty)

With patch, the last command shows 'memory'.

src/basic/cgroup-util.c
src/core/cgroup.c

index 63117afb103c92650df6195d8484686b11887575..156118b4a675729429621db7779bbd3e9da1b4fb 100644 (file)
@@ -760,6 +760,9 @@ int cg_trim(const char *controller, const char *path, bool delete_root) {
         return r;
 }
 
+/* Create a cgroup in the hierarchy of controller.
+ * Returns 0 if the group already existed, 1 on success, negative otherwise.
+ */
 int cg_create(const char *controller, const char *path) {
         _cleanup_free_ char *fs = NULL;
         int r;
@@ -2216,23 +2219,29 @@ done:
 
 int cg_create_everywhere(CGroupMask supported, CGroupMask mask, const char *path) {
         CGroupController c;
+        bool created;
         int r;
 
         /* This one will create a cgroup in our private tree, but also
          * duplicate it in the trees specified in mask, and remove it
-         * in all others */
+         * in all others.
+         *
+         * Returns 0 if the group already existed in the systemd hierarchy,
+         * 1 on success, negative otherwise.
+         */
 
         /* First create the cgroup in our own hierarchy. */
         r = cg_create(SYSTEMD_CGROUP_CONTROLLER, path);
         if (r < 0)
                 return r;
+        created = !!r;
 
         /* If we are in the unified hierarchy, we are done now */
         r = cg_all_unified();
         if (r < 0)
                 return r;
         if (r > 0)
-                return 0;
+                return created;
 
         /* Otherwise, do the same in the other hierarchies */
         for (c = 0; c < _CGROUP_CONTROLLER_MAX; c++) {
@@ -2247,7 +2256,7 @@ int cg_create_everywhere(CGroupMask supported, CGroupMask mask, const char *path
                         (void) cg_trim(n, path, true);
         }
 
-        return 0;
+        return created;
 }
 
 int cg_attach_everywhere(CGroupMask supported, const char *path, pid_t pid, cg_migrate_callback_t path_callback, void *userdata) {
index f6fd107ffa359522d8cf8be177b4ed25d2f5b46b..0a77d3ca82a1ad19887ddd99103e02dba6b1135b 100644 (file)
@@ -1454,6 +1454,7 @@ static int unit_create_cgroup(
 
         CGroupContext *c;
         int r;
+        bool created;
 
         assert(u);
 
@@ -1470,14 +1471,20 @@ static int unit_create_cgroup(
         r = cg_create_everywhere(u->manager->cgroup_supported, target_mask, u->cgroup_path);
         if (r < 0)
                 return log_unit_error_errno(u, r, "Failed to create cgroup %s: %m", u->cgroup_path);
+        created = !!r;
 
         /* Start watching it */
         (void) unit_watch_cgroup(u);
 
-        /* Enable all controllers we need */
-        r = cg_enable_everywhere(u->manager->cgroup_supported, enable_mask, u->cgroup_path);
-        if (r < 0)
-                log_unit_warning_errno(u, r, "Failed to enable controllers on cgroup %s, ignoring: %m", u->cgroup_path);
+        /* Preserve enabled controllers in delegated units, adjust others. */
+        if (created || !unit_cgroup_delegate(u)) {
+
+                /* Enable all controllers we need */
+                r = cg_enable_everywhere(u->manager->cgroup_supported, enable_mask, u->cgroup_path);
+                if (r < 0)
+                        log_unit_warning_errno(u, r, "Failed to enable controllers on cgroup %s, ignoring: %m",
+                                               u->cgroup_path);
+        }
 
         /* Keep track that this is now realized */
         u->cgroup_realized = true;