From 65be7e06528a221450602bbb3963a8ae0378925b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 29 May 2018 12:19:09 +0200 Subject: [PATCH] pid1: do not reset subtree_control on already-existing units with delegation 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 | 15 ++++++++++++--- src/core/cgroup.c | 15 +++++++++++---- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index ca31dee4a7..a7214432f5 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -754,6 +754,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; @@ -2104,23 +2107,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++) { @@ -2135,7 +2144,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) { diff --git a/src/core/cgroup.c b/src/core/cgroup.c index adc3e4ab2d..0da49544e6 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -1453,6 +1453,7 @@ static int unit_create_cgroup( CGroupContext *c; int r; + bool created; assert(u); @@ -1469,14 +1470,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;