Skip to content

Commit

Permalink
pid1: do not reset subtree_control on already-existing units with del…
Browse files Browse the repository at this point in the history
…egation

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'.
  • Loading branch information
keszybz authored and poettering committed Jun 11, 2018
1 parent d28b67d commit 65be7e0
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 7 deletions.
15 changes: 12 additions & 3 deletions src/basic/cgroup-util.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -2104,23 +2107,29 @@ int cg_get_keyed_attribute(

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++) {
Expand All @@ -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) {
Expand Down
15 changes: 11 additions & 4 deletions src/core/cgroup.c
Original file line number Diff line number Diff line change
Expand Up @@ -1453,6 +1453,7 @@ static int unit_create_cgroup(

CGroupContext *c;
int r;
bool created;

assert(u);

Expand All @@ -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;
Expand Down

0 comments on commit 65be7e0

Please sign in to comment.