systemd-239-82.7
Resolves: RHEL-9322
This commit is contained in:
parent
f11fa5b08f
commit
88755142d8
@ -0,0 +1,31 @@
|
||||
From 0b05aee0750c68bfd18bda777a3860c26cf154fc Mon Sep 17 00:00:00 2001
|
||||
From: Lennart Poettering <lennart@poettering.net>
|
||||
Date: Wed, 24 Oct 2018 17:30:46 +0200
|
||||
Subject: [PATCH] cgroup-util: add mask definitions for sets of controllers
|
||||
supported by cgroupsv1 vs. cgroupsv2
|
||||
|
||||
(cherry picked from commit 4edd65e4cff40158dded65c010b8c3e0b5ff5519)
|
||||
|
||||
Related: RHEL-9322
|
||||
---
|
||||
src/basic/cgroup-util.h | 7 +++++++
|
||||
1 file changed, 7 insertions(+)
|
||||
|
||||
diff --git a/src/basic/cgroup-util.h b/src/basic/cgroup-util.h
|
||||
index 65d2dbd4b6..1b0f53e8b8 100644
|
||||
--- a/src/basic/cgroup-util.h
|
||||
+++ b/src/basic/cgroup-util.h
|
||||
@@ -43,6 +43,13 @@ typedef enum CGroupMask {
|
||||
CGROUP_MASK_MEMORY = CGROUP_CONTROLLER_TO_MASK(CGROUP_CONTROLLER_MEMORY),
|
||||
CGROUP_MASK_DEVICES = CGROUP_CONTROLLER_TO_MASK(CGROUP_CONTROLLER_DEVICES),
|
||||
CGROUP_MASK_PIDS = CGROUP_CONTROLLER_TO_MASK(CGROUP_CONTROLLER_PIDS),
|
||||
+
|
||||
+ /* All real cgroupv1 controllers */
|
||||
+ CGROUP_MASK_V1 = CGROUP_MASK_CPU|CGROUP_MASK_CPUACCT|CGROUP_MASK_BLKIO|CGROUP_MASK_MEMORY|CGROUP_MASK_DEVICES|CGROUP_MASK_PIDS,
|
||||
+
|
||||
+ /* All real cgroupv2 controllers */
|
||||
+ CGROUP_MASK_V2 = CGROUP_MASK_CPU|CGROUP_MASK_IO|CGROUP_MASK_MEMORY|CGROUP_MASK_PIDS,
|
||||
+
|
||||
_CGROUP_MASK_ALL = CGROUP_CONTROLLER_TO_MASK(_CGROUP_CONTROLLER_MAX) - 1
|
||||
} CGroupMask;
|
||||
|
||||
51
1034-cgroup-dump-delegation-mask-too.patch
Normal file
51
1034-cgroup-dump-delegation-mask-too.patch
Normal file
@ -0,0 +1,51 @@
|
||||
From fb30cb6cb667dc97afeadb4e0ac6ad9134f2d30a Mon Sep 17 00:00:00 2001
|
||||
From: Lennart Poettering <lennart@poettering.net>
|
||||
Date: Wed, 21 Nov 2018 17:48:41 +0100
|
||||
Subject: [PATCH] cgroup: dump delegation mask too
|
||||
|
||||
(cherry picked from commit 0adf88b68ce71b49009d731ac6d96d9d59c4f2a9)
|
||||
|
||||
Related: RHEL-9322
|
||||
---
|
||||
src/core/unit.c | 10 ++++++++++
|
||||
1 file changed, 10 insertions(+)
|
||||
|
||||
diff --git a/src/core/unit.c b/src/core/unit.c
|
||||
index aedc1d806f..15c5bdf2a2 100644
|
||||
--- a/src/core/unit.c
|
||||
+++ b/src/core/unit.c
|
||||
@@ -1194,17 +1194,20 @@ void unit_dump(Unit *u, FILE *f, const char *prefix) {
|
||||
(void) cg_mask_to_string(u->cgroup_realized_mask, &s);
|
||||
fprintf(f, "%s\tCGroup realized mask: %s\n", prefix, strnull(s));
|
||||
}
|
||||
+
|
||||
if (u->cgroup_enabled_mask != 0) {
|
||||
_cleanup_free_ char *s = NULL;
|
||||
(void) cg_mask_to_string(u->cgroup_enabled_mask, &s);
|
||||
fprintf(f, "%s\tCGroup enabled mask: %s\n", prefix, strnull(s));
|
||||
}
|
||||
+
|
||||
m = unit_get_own_mask(u);
|
||||
if (m != 0) {
|
||||
_cleanup_free_ char *s = NULL;
|
||||
(void) cg_mask_to_string(m, &s);
|
||||
fprintf(f, "%s\tCGroup own mask: %s\n", prefix, strnull(s));
|
||||
}
|
||||
+
|
||||
m = unit_get_members_mask(u);
|
||||
if (m != 0) {
|
||||
_cleanup_free_ char *s = NULL;
|
||||
@@ -1212,6 +1215,13 @@ void unit_dump(Unit *u, FILE *f, const char *prefix) {
|
||||
fprintf(f, "%s\tCGroup members mask: %s\n", prefix, strnull(s));
|
||||
}
|
||||
|
||||
+ m = unit_get_delegate_mask(u);
|
||||
+ if (m != 0) {
|
||||
+ _cleanup_free_ char *s = NULL;
|
||||
+ (void) cg_mask_to_string(m, &s);
|
||||
+ fprintf(f, "%s\tCGroup delegate mask: %s\n", prefix, strnull(s));
|
||||
+ }
|
||||
+
|
||||
SET_FOREACH(t, u->names, i)
|
||||
fprintf(f, "%s\tName: %s\n", prefix, t);
|
||||
|
||||
@ -0,0 +1,32 @@
|
||||
From 3bd48135a82129199e99d212304cee4e0045302c Mon Sep 17 00:00:00 2001
|
||||
From: Lennart Poettering <lennart@poettering.net>
|
||||
Date: Wed, 21 Nov 2018 18:25:37 +0100
|
||||
Subject: [PATCH] cgroup: units that aren't loaded properly should not result
|
||||
in cgroup controllers being pulled in
|
||||
|
||||
This shouldn't make much difference in real life, but is a bit cleaner.
|
||||
|
||||
(cherry picked from commit 442ce7759c668bf9857eff13a90b0cfa6be8d426)
|
||||
|
||||
Related: RHEL-9322
|
||||
---
|
||||
src/core/cgroup.c | 6 +++++-
|
||||
1 file changed, 5 insertions(+), 1 deletion(-)
|
||||
|
||||
diff --git a/src/core/cgroup.c b/src/core/cgroup.c
|
||||
index 0c8a66edd1..331c97d288 100644
|
||||
--- a/src/core/cgroup.c
|
||||
+++ b/src/core/cgroup.c
|
||||
@@ -1308,7 +1308,11 @@ static CGroupMask unit_get_cgroup_mask(Unit *u) {
|
||||
CGroupMask unit_get_own_mask(Unit *u) {
|
||||
CGroupContext *c;
|
||||
|
||||
- /* Returns the mask of controllers the unit needs for itself */
|
||||
+ /* Returns the mask of controllers the unit needs for itself. If a unit is not properly loaded, return an empty
|
||||
+ * mask, as we shouldn't reflect it in the cgroup hierarchy then. */
|
||||
+
|
||||
+ if (u->load_state != UNIT_LOADED)
|
||||
+ return 0;
|
||||
|
||||
c = unit_get_cgroup_context(u);
|
||||
if (!c)
|
||||
249
1036-cgroup-be-more-careful-with-which-controllers-we-can.patch
Normal file
249
1036-cgroup-be-more-careful-with-which-controllers-we-can.patch
Normal file
@ -0,0 +1,249 @@
|
||||
From 38e4b2bb1bea9b727424ee9ba30b8e1e2988a0b8 Mon Sep 17 00:00:00 2001
|
||||
From: Lennart Poettering <lennart@poettering.net>
|
||||
Date: Thu, 22 Nov 2018 21:45:33 +0100
|
||||
Subject: [PATCH] cgroup: be more careful with which controllers we can
|
||||
enable/disable on a cgroup
|
||||
|
||||
This changes cg_enable_everywhere() to return which controllers are
|
||||
enabled for the specified cgroup. This information is then used to
|
||||
correctly track the enablement mask currently in effect for a unit.
|
||||
Moreover, when we try to turn off a controller, and this works, then
|
||||
this is indicates that the parent unit might succesfully turn it off
|
||||
now, too as our unit might have kept it busy.
|
||||
|
||||
So far, when realizing cgroups, i.e. when syncing up the kernel
|
||||
representation of relevant cgroups with our own idea we would strictly
|
||||
work from the root to the leaves. This is generally a good approach, as
|
||||
when controllers are enabled this has to happen in root-to-leaves order.
|
||||
However, when controllers are disabled this has to happen in the
|
||||
opposite order: in leaves-to-root order (this is because controllers can
|
||||
only be enabled in a child if it is already enabled in the parent, and
|
||||
if it shall be disabled in the parent then it has to be disabled in the
|
||||
child first, otherwise it is considered busy when it is attempted to
|
||||
remove it in the parent).
|
||||
|
||||
To make things complicated when invalidating a unit's cgroup membershup
|
||||
systemd can actually turn off some controllers previously turned on at
|
||||
the very same time as it turns on other controllers previously turned
|
||||
off. In such a case we have to work up leaves-to-root *and*
|
||||
root-to-leaves right after each other. With this patch this is
|
||||
implemented: we still generally operate root-to-leaves, but as soon as
|
||||
we noticed we successfully turned off a controller previously turned on
|
||||
for a cgroup we'll re-enqueue the cgroup realization for all parents of
|
||||
a unit, thus implementing leaves-to-root where necessary.
|
||||
|
||||
(cherry picked from commit 27adcc973771a998433635672e2eee0a4489b8a4)
|
||||
|
||||
Related: RHEL-9322
|
||||
---
|
||||
src/basic/cgroup-util.c | 58 ++++++++++++++++++++++++++++++++++++--
|
||||
src/basic/cgroup-util.h | 2 +-
|
||||
src/core/cgroup.c | 31 ++++++++++++++++----
|
||||
src/core/cgroup.h | 3 +-
|
||||
src/nspawn/nspawn-cgroup.c | 2 +-
|
||||
5 files changed, 84 insertions(+), 12 deletions(-)
|
||||
|
||||
diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c
|
||||
index 14abe6e014..f0aca25a00 100644
|
||||
--- a/src/basic/cgroup-util.c
|
||||
+++ b/src/basic/cgroup-util.c
|
||||
@@ -2573,22 +2573,45 @@ int cg_unified_flush(void) {
|
||||
return cg_unified_update();
|
||||
}
|
||||
|
||||
-int cg_enable_everywhere(CGroupMask supported, CGroupMask mask, const char *p) {
|
||||
+int cg_enable_everywhere(
|
||||
+ CGroupMask supported,
|
||||
+ CGroupMask mask,
|
||||
+ const char *p,
|
||||
+ CGroupMask *ret_result_mask) {
|
||||
+
|
||||
_cleanup_fclose_ FILE *f = NULL;
|
||||
_cleanup_free_ char *fs = NULL;
|
||||
CGroupController c;
|
||||
+ CGroupMask ret = 0;
|
||||
int r;
|
||||
|
||||
assert(p);
|
||||
|
||||
- if (supported == 0)
|
||||
+ if (supported == 0) {
|
||||
+ if (ret_result_mask)
|
||||
+ *ret_result_mask = 0;
|
||||
return 0;
|
||||
+ }
|
||||
|
||||
r = cg_all_unified();
|
||||
if (r < 0)
|
||||
return r;
|
||||
- if (r == 0) /* on the legacy hiearchy there's no joining of controllers defined */
|
||||
+ if (r == 0) {
|
||||
+ /* On the legacy hiearchy there's no concept of "enabling" controllers in cgroups defined. Let's claim
|
||||
+ * complete success right away. (If you wonder why we return the full mask here, rather than zero: the
|
||||
+ * caller tends to use the returned mask later on to compare if all controllers where properly joined,
|
||||
+ * and if not requeues realization. This use is the primary purpose of the return value, hence let's
|
||||
+ * minimize surprises here and reduce triggers for re-realization by always saying we fully
|
||||
+ * succeeded.) */
|
||||
+ if (ret_result_mask)
|
||||
+ *ret_result_mask = mask & supported & CGROUP_MASK_V2; /* If you wonder why we mask this with
|
||||
+ * CGROUP_MASK_V2: The 'supported' mask
|
||||
+ * might contain pure-V1 or BPF
|
||||
+ * controllers, and we never want to
|
||||
+ * claim that we could enable those with
|
||||
+ * cgroup.subtree_control */
|
||||
return 0;
|
||||
+ }
|
||||
|
||||
r = cg_get_path(SYSTEMD_CGROUP_CONTROLLER, p, "cgroup.subtree_control", &fs);
|
||||
if (r < 0)
|
||||
@@ -2620,10 +2643,39 @@ int cg_enable_everywhere(CGroupMask supported, CGroupMask mask, const char *p) {
|
||||
if (r < 0) {
|
||||
log_debug_errno(r, "Failed to enable controller %s for %s (%s): %m", n, p, fs);
|
||||
clearerr(f);
|
||||
+
|
||||
+ /* If we can't turn off a controller, leave it on in the reported resulting mask. This
|
||||
+ * happens for example when we attempt to turn off a controller up in the tree that is
|
||||
+ * used down in the tree. */
|
||||
+ if (!FLAGS_SET(mask, bit) && r == -EBUSY) /* You might wonder why we check for EBUSY
|
||||
+ * only here, and not follow the same logic
|
||||
+ * for other errors such as EINVAL or
|
||||
+ * EOPNOTSUPP or anything else. That's
|
||||
+ * because EBUSY indicates that the
|
||||
+ * controllers is currently enabled and
|
||||
+ * cannot be disabled because something down
|
||||
+ * the hierarchy is still using it. Any other
|
||||
+ * error most likely means something like "I
|
||||
+ * never heard of this controller" or
|
||||
+ * similar. In the former case it's hence
|
||||
+ * safe to assume the controller is still on
|
||||
+ * after the failed operation, while in the
|
||||
+ * latter case it's safer to assume the
|
||||
+ * controller is unknown and hence certainly
|
||||
+ * not enabled. */
|
||||
+ ret |= bit;
|
||||
+ } else {
|
||||
+ /* Otherwise, if we managed to turn on a controller, set the bit reflecting that. */
|
||||
+ if (FLAGS_SET(mask, bit))
|
||||
+ ret |= bit;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+ /* Let's return the precise set of controllers now enabled for the cgroup. */
|
||||
+ if (ret_result_mask)
|
||||
+ *ret_result_mask = ret;
|
||||
+
|
||||
return 0;
|
||||
}
|
||||
|
||||
diff --git a/src/basic/cgroup-util.h b/src/basic/cgroup-util.h
|
||||
index 1b0f53e8b8..0ce63f98e8 100644
|
||||
--- a/src/basic/cgroup-util.h
|
||||
+++ b/src/basic/cgroup-util.h
|
||||
@@ -248,7 +248,7 @@ int cg_attach_everywhere(CGroupMask supported, const char *path, pid_t pid, cg_m
|
||||
int cg_attach_many_everywhere(CGroupMask supported, const char *path, Set* pids, cg_migrate_callback_t callback, void *userdata);
|
||||
int cg_migrate_everywhere(CGroupMask supported, const char *from, const char *to, cg_migrate_callback_t callback, void *userdata);
|
||||
int cg_trim_everywhere(CGroupMask supported, const char *path, bool delete_root);
|
||||
-int cg_enable_everywhere(CGroupMask supported, CGroupMask mask, const char *p);
|
||||
+int cg_enable_everywhere(CGroupMask supported, CGroupMask mask, const char *p, CGroupMask *ret_result_mask);
|
||||
|
||||
int cg_mask_supported(CGroupMask *ret);
|
||||
int cg_mask_from_string(const char *s, CGroupMask *ret);
|
||||
diff --git a/src/core/cgroup.c b/src/core/cgroup.c
|
||||
index 331c97d288..93c0920c54 100644
|
||||
--- a/src/core/cgroup.c
|
||||
+++ b/src/core/cgroup.c
|
||||
@@ -1660,8 +1660,8 @@ static int unit_create_cgroup(
|
||||
bool needs_bpf) {
|
||||
|
||||
CGroupContext *c;
|
||||
- int r;
|
||||
bool created;
|
||||
+ int r;
|
||||
|
||||
assert(u);
|
||||
|
||||
@@ -1685,18 +1685,37 @@ static int unit_create_cgroup(
|
||||
|
||||
/* Preserve enabled controllers in delegated units, adjust others. */
|
||||
if (created || !unit_cgroup_delegate(u)) {
|
||||
+ CGroupMask result_mask = 0;
|
||||
|
||||
/* Enable all controllers we need */
|
||||
- r = cg_enable_everywhere(u->manager->cgroup_supported, enable_mask, u->cgroup_path);
|
||||
+ r = cg_enable_everywhere(u->manager->cgroup_supported, enable_mask, u->cgroup_path, &result_mask);
|
||||
if (r < 0)
|
||||
- log_unit_warning_errno(u, r, "Failed to enable controllers on cgroup %s, ignoring: %m",
|
||||
- u->cgroup_path);
|
||||
+ log_unit_warning_errno(u, r, "Failed to enable/disable controllers on cgroup %s, ignoring: %m", u->cgroup_path);
|
||||
+
|
||||
+ /* If we just turned off a controller, this might release the controller for our parent too, let's
|
||||
+ * enqueue the parent for re-realization in that case again. */
|
||||
+ if (UNIT_ISSET(u->slice)) {
|
||||
+ CGroupMask turned_off;
|
||||
+
|
||||
+ turned_off = (u->cgroup_realized ? u->cgroup_enabled_mask & ~result_mask : 0);
|
||||
+ if (turned_off != 0) {
|
||||
+ Unit *parent;
|
||||
+
|
||||
+ /* Force the parent to propagate the enable mask to the kernel again, by invalidating
|
||||
+ * the controller we just turned off. */
|
||||
+
|
||||
+ for (parent = UNIT_DEREF(u->slice); parent; parent = UNIT_DEREF(parent->slice))
|
||||
+ unit_invalidate_cgroup(parent, turned_off);
|
||||
+ }
|
||||
+ }
|
||||
+
|
||||
+ /* Remember what's actually enabled now */
|
||||
+ u->cgroup_enabled_mask = result_mask;
|
||||
}
|
||||
|
||||
/* Keep track that this is now realized */
|
||||
u->cgroup_realized = true;
|
||||
u->cgroup_realized_mask = target_mask;
|
||||
- u->cgroup_enabled_mask = enable_mask;
|
||||
u->cgroup_bpf_state = needs_bpf ? UNIT_CGROUP_BPF_ON : UNIT_CGROUP_BPF_OFF;
|
||||
|
||||
if (u->type != UNIT_SLICE && !unit_cgroup_delegate(u)) {
|
||||
@@ -1885,7 +1904,7 @@ static bool unit_has_mask_realized(
|
||||
(!needs_bpf && u->cgroup_bpf_state == UNIT_CGROUP_BPF_OFF));
|
||||
}
|
||||
|
||||
-static void unit_add_to_cgroup_realize_queue(Unit *u) {
|
||||
+void unit_add_to_cgroup_realize_queue(Unit *u) {
|
||||
assert(u);
|
||||
|
||||
if (u->in_cgroup_realize_queue)
|
||||
diff --git a/src/core/cgroup.h b/src/core/cgroup.h
|
||||
index 36ea77fdc5..535e328ab6 100644
|
||||
--- a/src/core/cgroup.h
|
||||
+++ b/src/core/cgroup.h
|
||||
@@ -175,7 +175,6 @@ CGroupMask unit_get_delegate_mask(Unit *u);
|
||||
CGroupMask unit_get_members_mask(Unit *u);
|
||||
CGroupMask unit_get_siblings_mask(Unit *u);
|
||||
CGroupMask unit_get_subtree_mask(Unit *u);
|
||||
-
|
||||
CGroupMask unit_get_target_mask(Unit *u);
|
||||
CGroupMask unit_get_enable_mask(Unit *u);
|
||||
|
||||
@@ -183,6 +182,8 @@ bool unit_get_needs_bpf(Unit *u);
|
||||
|
||||
void unit_update_cgroup_members_masks(Unit *u);
|
||||
|
||||
+void unit_add_to_cgroup_realize_queue(Unit *u);
|
||||
+
|
||||
const char *unit_get_realized_cgroup_path(Unit *u, CGroupMask mask);
|
||||
char *unit_default_cgroup_path(Unit *u);
|
||||
int unit_set_cgroup_path(Unit *u, const char *path);
|
||||
diff --git a/src/nspawn/nspawn-cgroup.c b/src/nspawn/nspawn-cgroup.c
|
||||
index a231622e29..975427aa31 100644
|
||||
--- a/src/nspawn/nspawn-cgroup.c
|
||||
+++ b/src/nspawn/nspawn-cgroup.c
|
||||
@@ -185,6 +185,6 @@ int create_subcgroup(pid_t pid, bool keep_unit, CGroupUnified unified_requested)
|
||||
}
|
||||
|
||||
/* Try to enable as many controllers as possible for the new payload. */
|
||||
- (void) cg_enable_everywhere(supported, supported, cgroup);
|
||||
+ (void) cg_enable_everywhere(supported, supported, cgroup, NULL);
|
||||
return 0;
|
||||
}
|
||||
@ -0,0 +1,35 @@
|
||||
From 70d03cffc01a77df5441e8c62e07c010e16882b2 Mon Sep 17 00:00:00 2001
|
||||
From: Lennart Poettering <lennart@poettering.net>
|
||||
Date: Fri, 23 Nov 2018 01:03:18 +0100
|
||||
Subject: [PATCH] cgroup: extend reasons when we realize the enable mask
|
||||
|
||||
After creating a cgroup we need to initialize its
|
||||
"cgroup.subtree_control" file with the controllers its children want to
|
||||
use. Currently we do so whenever the mkdir() on the cgroup succeeded,
|
||||
i.e. when we know the cgroup is "fresh". Let's update the condition
|
||||
slightly that we also do so when internally we assume a cgroup doesn't
|
||||
exist yet, even if it already does (maybe left-over from a previous
|
||||
run).
|
||||
|
||||
This shouldn't change anything IRL but make things a bit more robust.
|
||||
|
||||
(cherry picked from commit 1fd3a10c38a0be4fc42ae94cf9f8401003187b80)
|
||||
|
||||
Related: RHEL-9322
|
||||
---
|
||||
src/core/cgroup.c | 2 +-
|
||||
1 file changed, 1 insertion(+), 1 deletion(-)
|
||||
|
||||
diff --git a/src/core/cgroup.c b/src/core/cgroup.c
|
||||
index 93c0920c54..bf829407b8 100644
|
||||
--- a/src/core/cgroup.c
|
||||
+++ b/src/core/cgroup.c
|
||||
@@ -1684,7 +1684,7 @@ static int unit_create_cgroup(
|
||||
(void) unit_watch_cgroup(u);
|
||||
|
||||
/* Preserve enabled controllers in delegated units, adjust others. */
|
||||
- if (created || !unit_cgroup_delegate(u)) {
|
||||
+ if (created || !u->cgroup_realized || !unit_cgroup_delegate(u)) {
|
||||
CGroupMask result_mask = 0;
|
||||
|
||||
/* Enable all controllers we need */
|
||||
228
1038-cgroup-drastically-simplify-caching-of-cgroups-membe.patch
Normal file
228
1038-cgroup-drastically-simplify-caching-of-cgroups-membe.patch
Normal file
@ -0,0 +1,228 @@
|
||||
From 18d667eb7895b47dad3b4144b69e6e9e3afe4994 Mon Sep 17 00:00:00 2001
|
||||
From: Lennart Poettering <lennart@poettering.net>
|
||||
Date: Fri, 23 Nov 2018 01:07:34 +0100
|
||||
Subject: [PATCH] cgroup: drastically simplify caching of cgroups members mask
|
||||
|
||||
Previously we tried to be smart: when a new unit appeared and it only
|
||||
added controllers to the cgroup mask we'd update the cached members mask
|
||||
in all parents by ORing in the controller flags in their cached values.
|
||||
Unfortunately this was quite broken, as we missed some conditions when
|
||||
this cache had to be reset (for example, when a unit got unloaded),
|
||||
moreover the optimization doesn't work when a controller is removed
|
||||
anyway (as in that case there's no other way for the parent to iterate
|
||||
though all children if any other, remaining child unit still needs it).
|
||||
Hence, let's simplify the logic substantially: instead of updating the
|
||||
cache on the right events (which we didn't get right), let's simply
|
||||
invalidate the cache, and generate it lazily when we encounter it later.
|
||||
This should actually result in better behaviour as we don't have to
|
||||
calculate the new members mask for a whole subtree whever we have the
|
||||
suspicion something changed, but can delay it to the point where we
|
||||
actually need the members mask.
|
||||
|
||||
This allows us to simplify things quite a bit, which is good, since
|
||||
validating this cache for correctness is hard enough.
|
||||
|
||||
Fixes: #9512
|
||||
(cherry picked from commit 5af8805872809e6de4cc4d9495cb1a904772ab4e)
|
||||
|
||||
Resolves: RHEL-9322
|
||||
---
|
||||
src/core/cgroup.c | 49 +++++------------------------------------
|
||||
src/core/cgroup.h | 1 +
|
||||
src/core/dbus-mount.c | 2 +-
|
||||
src/core/dbus-scope.c | 2 +-
|
||||
src/core/dbus-service.c | 2 +-
|
||||
src/core/dbus-slice.c | 2 +-
|
||||
src/core/dbus-socket.c | 2 +-
|
||||
src/core/dbus-swap.c | 2 +-
|
||||
src/core/unit.c | 3 ++-
|
||||
src/core/unit.h | 2 --
|
||||
10 files changed, 14 insertions(+), 53 deletions(-)
|
||||
|
||||
diff --git a/src/core/cgroup.c b/src/core/cgroup.c
|
||||
index bf829407b8..3276c876ec 100644
|
||||
--- a/src/core/cgroup.c
|
||||
+++ b/src/core/cgroup.c
|
||||
@@ -1454,53 +1454,14 @@ bool unit_get_needs_bpf(Unit *u) {
|
||||
return false;
|
||||
}
|
||||
|
||||
-/* Recurse from a unit up through its containing slices, propagating
|
||||
- * mask bits upward. A unit is also member of itself. */
|
||||
-void unit_update_cgroup_members_masks(Unit *u) {
|
||||
- CGroupMask m;
|
||||
- bool more;
|
||||
-
|
||||
+void unit_invalidate_cgroup_members_masks(Unit *u) {
|
||||
assert(u);
|
||||
|
||||
- /* Calculate subtree mask */
|
||||
- m = unit_get_subtree_mask(u);
|
||||
-
|
||||
- /* See if anything changed from the previous invocation. If
|
||||
- * not, we're done. */
|
||||
- if (u->cgroup_subtree_mask_valid && m == u->cgroup_subtree_mask)
|
||||
- return;
|
||||
-
|
||||
- more =
|
||||
- u->cgroup_subtree_mask_valid &&
|
||||
- ((m & ~u->cgroup_subtree_mask) != 0) &&
|
||||
- ((~m & u->cgroup_subtree_mask) == 0);
|
||||
-
|
||||
- u->cgroup_subtree_mask = m;
|
||||
- u->cgroup_subtree_mask_valid = true;
|
||||
-
|
||||
- if (UNIT_ISSET(u->slice)) {
|
||||
- Unit *s = UNIT_DEREF(u->slice);
|
||||
-
|
||||
- if (more)
|
||||
- /* There's more set now than before. We
|
||||
- * propagate the new mask to the parent's mask
|
||||
- * (not caring if it actually was valid or
|
||||
- * not). */
|
||||
-
|
||||
- s->cgroup_members_mask |= m;
|
||||
-
|
||||
- else
|
||||
- /* There's less set now than before (or we
|
||||
- * don't know), we need to recalculate
|
||||
- * everything, so let's invalidate the
|
||||
- * parent's members mask */
|
||||
+ /* Recurse invalidate the member masks cache all the way up the tree */
|
||||
+ u->cgroup_members_mask_valid = false;
|
||||
|
||||
- s->cgroup_members_mask_valid = false;
|
||||
-
|
||||
- /* And now make sure that this change also hits our
|
||||
- * grandparents */
|
||||
- unit_update_cgroup_members_masks(s);
|
||||
- }
|
||||
+ if (UNIT_ISSET(u->slice))
|
||||
+ unit_invalidate_cgroup_members_masks(UNIT_DEREF(u->slice));
|
||||
}
|
||||
|
||||
const char *unit_get_realized_cgroup_path(Unit *u, CGroupMask mask) {
|
||||
diff --git a/src/core/cgroup.h b/src/core/cgroup.h
|
||||
index 535e328ab6..2ec8dd8fba 100644
|
||||
--- a/src/core/cgroup.h
|
||||
+++ b/src/core/cgroup.h
|
||||
@@ -180,6 +180,7 @@ CGroupMask unit_get_enable_mask(Unit *u);
|
||||
|
||||
bool unit_get_needs_bpf(Unit *u);
|
||||
|
||||
+void unit_invalidate_cgroup_members_masks(Unit *u);
|
||||
void unit_update_cgroup_members_masks(Unit *u);
|
||||
|
||||
void unit_add_to_cgroup_realize_queue(Unit *u);
|
||||
diff --git a/src/core/dbus-mount.c b/src/core/dbus-mount.c
|
||||
index a089b37e04..345494fd64 100644
|
||||
--- a/src/core/dbus-mount.c
|
||||
+++ b/src/core/dbus-mount.c
|
||||
@@ -190,7 +190,7 @@ int bus_mount_set_property(
|
||||
int bus_mount_commit_properties(Unit *u) {
|
||||
assert(u);
|
||||
|
||||
- unit_update_cgroup_members_masks(u);
|
||||
+ unit_invalidate_cgroup_members_masks(u);
|
||||
unit_realize_cgroup(u);
|
||||
|
||||
return 0;
|
||||
diff --git a/src/core/dbus-scope.c b/src/core/dbus-scope.c
|
||||
index 534302d188..8504352cbf 100644
|
||||
--- a/src/core/dbus-scope.c
|
||||
+++ b/src/core/dbus-scope.c
|
||||
@@ -192,7 +192,7 @@ int bus_scope_set_property(
|
||||
int bus_scope_commit_properties(Unit *u) {
|
||||
assert(u);
|
||||
|
||||
- unit_update_cgroup_members_masks(u);
|
||||
+ unit_invalidate_cgroup_members_masks(u);
|
||||
unit_realize_cgroup(u);
|
||||
|
||||
return 0;
|
||||
diff --git a/src/core/dbus-service.c b/src/core/dbus-service.c
|
||||
index 5f768a77c8..6db583492b 100644
|
||||
--- a/src/core/dbus-service.c
|
||||
+++ b/src/core/dbus-service.c
|
||||
@@ -393,7 +393,7 @@ int bus_service_set_property(
|
||||
int bus_service_commit_properties(Unit *u) {
|
||||
assert(u);
|
||||
|
||||
- unit_update_cgroup_members_masks(u);
|
||||
+ unit_invalidate_cgroup_members_masks(u);
|
||||
unit_realize_cgroup(u);
|
||||
|
||||
return 0;
|
||||
diff --git a/src/core/dbus-slice.c b/src/core/dbus-slice.c
|
||||
index 722a5688a5..effd5fa5d7 100644
|
||||
--- a/src/core/dbus-slice.c
|
||||
+++ b/src/core/dbus-slice.c
|
||||
@@ -28,7 +28,7 @@ int bus_slice_set_property(
|
||||
int bus_slice_commit_properties(Unit *u) {
|
||||
assert(u);
|
||||
|
||||
- unit_update_cgroup_members_masks(u);
|
||||
+ unit_invalidate_cgroup_members_masks(u);
|
||||
unit_realize_cgroup(u);
|
||||
|
||||
return 0;
|
||||
diff --git a/src/core/dbus-socket.c b/src/core/dbus-socket.c
|
||||
index 17494b80c8..dc74c82714 100644
|
||||
--- a/src/core/dbus-socket.c
|
||||
+++ b/src/core/dbus-socket.c
|
||||
@@ -469,7 +469,7 @@ int bus_socket_set_property(
|
||||
int bus_socket_commit_properties(Unit *u) {
|
||||
assert(u);
|
||||
|
||||
- unit_update_cgroup_members_masks(u);
|
||||
+ unit_invalidate_cgroup_members_masks(u);
|
||||
unit_realize_cgroup(u);
|
||||
|
||||
return 0;
|
||||
diff --git a/src/core/dbus-swap.c b/src/core/dbus-swap.c
|
||||
index b272d10113..353fa20132 100644
|
||||
--- a/src/core/dbus-swap.c
|
||||
+++ b/src/core/dbus-swap.c
|
||||
@@ -63,7 +63,7 @@ int bus_swap_set_property(
|
||||
int bus_swap_commit_properties(Unit *u) {
|
||||
assert(u);
|
||||
|
||||
- unit_update_cgroup_members_masks(u);
|
||||
+ unit_invalidate_cgroup_members_masks(u);
|
||||
unit_realize_cgroup(u);
|
||||
|
||||
return 0;
|
||||
diff --git a/src/core/unit.c b/src/core/unit.c
|
||||
index 15c5bdf2a2..a7f610eca8 100644
|
||||
--- a/src/core/unit.c
|
||||
+++ b/src/core/unit.c
|
||||
@@ -1583,7 +1583,8 @@ int unit_load(Unit *u) {
|
||||
if (u->job_running_timeout != USEC_INFINITY && u->job_running_timeout > u->job_timeout)
|
||||
log_unit_warning(u, "JobRunningTimeoutSec= is greater than JobTimeoutSec=, it has no effect.");
|
||||
|
||||
- unit_update_cgroup_members_masks(u);
|
||||
+ /* We finished loading, let's ensure our parents recalculate the members mask */
|
||||
+ unit_invalidate_cgroup_members_masks(u);
|
||||
}
|
||||
|
||||
assert((u->load_state != UNIT_MERGED) == !u->merged_into);
|
||||
diff --git a/src/core/unit.h b/src/core/unit.h
|
||||
index b8b914711f..e2dd7949e5 100644
|
||||
--- a/src/core/unit.h
|
||||
+++ b/src/core/unit.h
|
||||
@@ -265,7 +265,6 @@ typedef struct Unit {
|
||||
char *cgroup_path;
|
||||
CGroupMask cgroup_realized_mask;
|
||||
CGroupMask cgroup_enabled_mask;
|
||||
- CGroupMask cgroup_subtree_mask;
|
||||
CGroupMask cgroup_members_mask;
|
||||
int cgroup_inotify_wd;
|
||||
|
||||
@@ -341,7 +340,6 @@ typedef struct Unit {
|
||||
|
||||
bool cgroup_realized:1;
|
||||
bool cgroup_members_mask_valid:1;
|
||||
- bool cgroup_subtree_mask_valid:1;
|
||||
|
||||
UnitCGroupBPFState cgroup_bpf_state:2;
|
||||
|
||||
@ -0,0 +1,36 @@
|
||||
From 8b64f8944a9e48ca07c2fb086457675031814b46 Mon Sep 17 00:00:00 2001
|
||||
From: Lennart Poettering <lennart@poettering.net>
|
||||
Date: Fri, 23 Nov 2018 01:13:47 +0100
|
||||
Subject: [PATCH] cgroup: when we unload a unit, also update all its parent's
|
||||
members mask
|
||||
|
||||
This way we can corectly ensure that when a unit that requires some
|
||||
controller goes away, we propagate the removal of it all the way up, so
|
||||
that the controller is turned off in all the parents too.
|
||||
|
||||
(cherry picked from commit b8b6f321044ab085358de91a1f72a7d86593dfda)
|
||||
|
||||
Related: RHEL-9322
|
||||
---
|
||||
src/core/unit.c | 8 ++++++++
|
||||
1 file changed, 8 insertions(+)
|
||||
|
||||
diff --git a/src/core/unit.c b/src/core/unit.c
|
||||
index a7f610eca8..70e1d68ea4 100644
|
||||
--- a/src/core/unit.c
|
||||
+++ b/src/core/unit.c
|
||||
@@ -578,6 +578,14 @@ void unit_free(Unit *u) {
|
||||
if (!u)
|
||||
return;
|
||||
|
||||
+ if (UNIT_ISSET(u->slice)) {
|
||||
+ /* A unit is being dropped from the tree, make sure our parent slice recalculates the member mask */
|
||||
+ unit_invalidate_cgroup_members_masks(UNIT_DEREF(u->slice));
|
||||
+
|
||||
+ /* And make sure the parent is realized again, updating cgroup memberships */
|
||||
+ unit_add_to_cgroup_realize_queue(UNIT_DEREF(u->slice));
|
||||
+ }
|
||||
+
|
||||
u->transient_file = safe_fclose(u->transient_file);
|
||||
|
||||
if (!MANAGER_IS_RELOADING(u->manager))
|
||||
@ -0,0 +1,47 @@
|
||||
From a3ce5bf1ff531f29c329b3298f3ef69d99e3cbed Mon Sep 17 00:00:00 2001
|
||||
From: Lennart Poettering <lennart@poettering.net>
|
||||
Date: Fri, 23 Nov 2018 01:15:19 +0100
|
||||
Subject: [PATCH] test: extend testcase to ensure controller membership doesn't
|
||||
regress
|
||||
|
||||
(cherry picked from commit 43738e001e64ba0ec6522e0c35b67a006abf10e9)
|
||||
|
||||
Related: RHEL-9322
|
||||
---
|
||||
test/TEST-19-DELEGATE/testsuite.sh | 21 +++++++++++++++++++--
|
||||
1 file changed, 19 insertions(+), 2 deletions(-)
|
||||
|
||||
diff --git a/test/TEST-19-DELEGATE/testsuite.sh b/test/TEST-19-DELEGATE/testsuite.sh
|
||||
index c4c948cc11..8a3e9eb5fb 100755
|
||||
--- a/test/TEST-19-DELEGATE/testsuite.sh
|
||||
+++ b/test/TEST-19-DELEGATE/testsuite.sh
|
||||
@@ -21,10 +21,27 @@ if grep -q cgroup2 /proc/filesystems ; then
|
||||
-w /sys/fs/cgroup/system.slice/test0.service/cgroup.subtree_control
|
||||
|
||||
systemd-run --wait --unit=test1.service -p "DynamicUser=1" -p "Delegate=memory pids" \
|
||||
- grep memory /sys/fs/cgroup/system.slice/test1.service/cgroup.controllers
|
||||
+ grep -q memory /sys/fs/cgroup/system.slice/test1.service/cgroup.controllers
|
||||
|
||||
systemd-run --wait --unit=test2.service -p "DynamicUser=1" -p "Delegate=memory pids" \
|
||||
- grep pids /sys/fs/cgroup/system.slice/test2.service/cgroup.controllers
|
||||
+ grep -q pids /sys/fs/cgroup/system.slice/test2.service/cgroup.controllers
|
||||
+
|
||||
+ # "io" is not among the controllers enabled by default for all units, verify that
|
||||
+ grep -qv io /sys/fs/cgroup/system.slice/cgroup.controllers
|
||||
+
|
||||
+ # Run a service with "io" enabled, and verify it works
|
||||
+ systemd-run --wait --unit=test3.service -p "IOAccounting=yes" -p "Slice=system-foo-bar-baz.slice" \
|
||||
+ grep -q io /sys/fs/cgroup/system.slice/system-foo.slice/system-foo-bar.slice/system-foo-bar-baz.slice/test3.service/cgroup.controllers
|
||||
+
|
||||
+ # We want to check if "io" is removed again from the controllers
|
||||
+ # list. However, PID 1 (rightfully) does this asynchronously. In order
|
||||
+ # to force synchronization on this, let's start a short-lived service
|
||||
+ # which requires PID 1 to refresh the cgroup tree, so that we can
|
||||
+ # verify that this all works.
|
||||
+ systemd-run --wait --unit=test4.service true
|
||||
+
|
||||
+ # And now check again, "io" should have vanished
|
||||
+ grep -qv io /sys/fs/cgroup/system.slice/cgroup.controllers
|
||||
|
||||
# Check that unprivileged delegation works for scopes
|
||||
test_scope_unpriv_delegation
|
||||
20
systemd.spec
20
systemd.spec
@ -13,7 +13,7 @@
|
||||
Name: systemd
|
||||
Url: http://www.freedesktop.org/wiki/Software/systemd
|
||||
Version: 239
|
||||
Release: 82%{?dist}.6
|
||||
Release: 82%{?dist}.7
|
||||
# For a breakdown of the licensing, see README
|
||||
License: LGPLv2+ and MIT and GPLv2+
|
||||
Summary: System and Service Manager
|
||||
@ -1082,6 +1082,14 @@ Patch1029: 1029-core-do-not-disconnect-from-bus-when-failed-to-insta.patch
|
||||
Patch1030: 1030-sd-bus-bus-track-use-install_callback-in-sd_bus_trac.patch
|
||||
Patch1031: 1031-core-manager-restore-bus-track-deserialization-clean.patch
|
||||
Patch1032: 1032-core-manager-drop-duplicate-bus-track-deserializatio.patch
|
||||
Patch1033: 1033-cgroup-util-add-mask-definitions-for-sets-of-control.patch
|
||||
Patch1034: 1034-cgroup-dump-delegation-mask-too.patch
|
||||
Patch1035: 1035-cgroup-units-that-aren-t-loaded-properly-should-not-.patch
|
||||
Patch1036: 1036-cgroup-be-more-careful-with-which-controllers-we-can.patch
|
||||
Patch1037: 1037-cgroup-extend-reasons-when-we-realize-the-enable-mas.patch
|
||||
Patch1038: 1038-cgroup-drastically-simplify-caching-of-cgroups-membe.patch
|
||||
Patch1039: 1039-cgroup-when-we-unload-a-unit-also-update-all-its-par.patch
|
||||
Patch1040: 1040-test-extend-testcase-to-ensure-controller-membership.patch
|
||||
|
||||
%ifarch %{ix86} x86_64 aarch64
|
||||
%global have_gnu_efi 1
|
||||
@ -1708,6 +1716,16 @@ fi
|
||||
%files tests -f .file-list-tests
|
||||
|
||||
%changelog
|
||||
* Fri Aug 29 2025 systemd maintenance team <systemd-maint@redhat.com> - 239-82.7
|
||||
- cgroup-util: add mask definitions for sets of controllers supported by cgroupsv1 vs. cgroupsv2 (RHEL-9322)
|
||||
- cgroup: dump delegation mask too (RHEL-9322)
|
||||
- cgroup: units that aren't loaded properly should not result in cgroup controllers being pulled in (RHEL-9322)
|
||||
- cgroup: be more careful with which controllers we can enable/disable on a cgroup (RHEL-9322)
|
||||
- cgroup: extend reasons when we realize the enable mask (RHEL-9322)
|
||||
- cgroup: drastically simplify caching of cgroups members mask (RHEL-9322)
|
||||
- cgroup: when we unload a unit, also update all its parent's members mask (RHEL-9322)
|
||||
- test: extend testcase to ensure controller membership doesn't regress (RHEL-9322)
|
||||
|
||||
* Mon Aug 25 2025 systemd maintenance team <systemd-maint@redhat.com> - 239-82.6
|
||||
- dbus: stash the subscriber list when we disconenct from the bus (RHEL-75081)
|
||||
- manager: s/deserialized_subscribed/subscribed_as_strv (RHEL-75081)
|
||||
|
||||
Loading…
Reference in New Issue
Block a user