From: Lennart Poettering Date: Mon, 26 Apr 2021 16:14:07 +0000 (+0200) Subject: core: add UNIT_GET_SLICE() helper X-Git-Tag: v249-rc1~155^2~13 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=12f64221b0af5a9380a840ebeb897c2cd6cff955;p=thirdparty%2Fsystemd.git core: add UNIT_GET_SLICE() helper In a later commit we intend to move the slice logic to use proper dependencies instead of a "UnitRef" object. This preparatory commit drops direct use of the slice UnitRef object for a static inline function UNIT_GET_SLICE() that is both easier to grok, and allows us to easily replace its internal implementation later on. --- diff --git a/src/core/bpf-firewall.c b/src/core/bpf-firewall.c index 02e33399c3e..2a41bffee6b 100644 --- a/src/core/bpf-firewall.c +++ b/src/core/bpf-firewall.c @@ -422,7 +422,7 @@ static int bpf_firewall_prepare_access_maps( assert(ret_ipv6_map_fd); assert(ret_has_any); - for (p = u; p; p = UNIT_DEREF(p->slice)) { + for (p = u; p; p = UNIT_GET_SLICE(p)) { CGroupContext *cc; cc = unit_get_cgroup_context(p); @@ -463,7 +463,7 @@ static int bpf_firewall_prepare_access_maps( return ipv6_map_fd; } - for (p = u; p; p = UNIT_DEREF(p->slice)) { + for (p = u; p; p = UNIT_GET_SLICE(p)) { CGroupContext *cc; cc = unit_get_cgroup_context(p); diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 92ac35af8e3..97820eb2640 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -680,7 +680,7 @@ int cgroup_add_bpf_foreign_program(CGroupContext *c, uint32_t attach_type, const if (c && c->entry##_set) \ return c->entry; \ \ - while ((u = UNIT_DEREF(u->slice))) { \ + while ((u = UNIT_GET_SLICE(u))) { \ c = unit_get_cgroup_context(u); \ if (c && c->default_##entry##_set) \ return c->default_##entry; \ @@ -1549,7 +1549,7 @@ static bool unit_get_needs_bpf_firewall(Unit *u) { return true; /* If any parent slice has an IP access list defined, it applies too */ - for (p = UNIT_DEREF(u->slice); p; p = UNIT_DEREF(p->slice)) { + for (p = UNIT_GET_SLICE(u); p; p = UNIT_GET_SLICE(p)) { c = unit_get_cgroup_context(p); if (!c) return false; @@ -1715,14 +1715,16 @@ CGroupMask unit_get_members_mask(Unit *u) { } CGroupMask unit_get_siblings_mask(Unit *u) { + Unit *slice; assert(u); /* Returns the mask of controllers all of the unit's siblings * require, i.e. the members mask of the unit's parent slice * if there is one. */ - if (UNIT_ISSET(u->slice)) - return unit_get_members_mask(UNIT_DEREF(u->slice)); + slice = UNIT_GET_SLICE(u); + if (slice) + return unit_get_members_mask(slice); return unit_get_subtree_mask(u); /* we are the top-level slice */ } @@ -1739,6 +1741,7 @@ static CGroupMask unit_get_disable_mask(Unit *u) { CGroupMask unit_get_ancestor_disable_mask(Unit *u) { CGroupMask mask; + Unit *slice; assert(u); mask = unit_get_disable_mask(u); @@ -1746,8 +1749,9 @@ CGroupMask unit_get_ancestor_disable_mask(Unit *u) { /* Returns the mask of controllers which are marked as forcibly * disabled in any ancestor unit or the unit in question. */ - if (UNIT_ISSET(u->slice)) - mask |= unit_get_ancestor_disable_mask(UNIT_DEREF(u->slice)); + slice = UNIT_GET_SLICE(u); + if (slice) + mask |= unit_get_ancestor_disable_mask(slice); return mask; } @@ -1789,13 +1793,16 @@ CGroupMask unit_get_enable_mask(Unit *u) { } void unit_invalidate_cgroup_members_masks(Unit *u) { + Unit *slice; + assert(u); /* Recurse invalidate the member masks cache all the way up the tree */ u->cgroup_members_mask_valid = false; - if (UNIT_ISSET(u->slice)) - unit_invalidate_cgroup_members_masks(UNIT_DEREF(u->slice)); + slice = UNIT_GET_SLICE(u); + if (slice) + unit_invalidate_cgroup_members_masks(slice); } const char *unit_get_realized_cgroup_path(Unit *u, CGroupMask mask) { @@ -1809,7 +1816,7 @@ const char *unit_get_realized_cgroup_path(Unit *u, CGroupMask mask) { FLAGS_SET(u->cgroup_realized_mask, mask)) return u->cgroup_path; - u = UNIT_DEREF(u->slice); + u = UNIT_GET_SLICE(u); } return NULL; @@ -1823,7 +1830,8 @@ static const char *migrate_callback(CGroupMask mask, void *userdata) { } char *unit_default_cgroup_path(const Unit *u) { - _cleanup_free_ char *escaped = NULL, *slice = NULL; + _cleanup_free_ char *escaped = NULL, *slice_path = NULL; + Unit *slice; int r; assert(u); @@ -1831,8 +1839,9 @@ char *unit_default_cgroup_path(const Unit *u) { if (unit_has_name(u, SPECIAL_ROOT_SLICE)) return strdup(u->manager->cgroup_root); - if (UNIT_ISSET(u->slice) && !unit_has_name(UNIT_DEREF(u->slice), SPECIAL_ROOT_SLICE)) { - r = cg_slice_to_path(UNIT_DEREF(u->slice)->id, &slice); + slice = UNIT_GET_SLICE(u); + if (slice && !unit_has_name(slice, SPECIAL_ROOT_SLICE)) { + r = cg_slice_to_path(slice->id, &slice_path); if (r < 0) return NULL; } @@ -1841,7 +1850,7 @@ char *unit_default_cgroup_path(const Unit *u) { if (!escaped) return NULL; - return path_join(empty_to_root(u->manager->cgroup_root), slice, escaped); + return path_join(empty_to_root(u->manager->cgroup_root), slice_path, escaped); } int unit_set_cgroup_path(Unit *u, const char *path) { @@ -2315,14 +2324,16 @@ static void unit_remove_from_cgroup_realize_queue(Unit *u) { * hierarchy downwards to the unit in question. */ static int unit_realize_cgroup_now_enable(Unit *u, ManagerState state) { CGroupMask target_mask, enable_mask, new_target_mask, new_enable_mask; + Unit *slice; int r; assert(u); /* First go deal with this unit's parent, or we won't be able to enable * any new controllers at this layer. */ - if (UNIT_ISSET(u->slice)) { - r = unit_realize_cgroup_now_enable(UNIT_DEREF(u->slice), state); + slice = UNIT_GET_SLICE(u); + if (slice) { + r = unit_realize_cgroup_now_enable(slice, state); if (r < 0) return r; } @@ -2431,6 +2442,7 @@ static int unit_realize_cgroup_now_disable(Unit *u, ManagerState state) { * Returns 0 on success and < 0 on failure. */ static int unit_realize_cgroup_now(Unit *u, ManagerState state) { CGroupMask target_mask, enable_mask; + Unit *slice; int r; assert(u); @@ -2449,8 +2461,9 @@ static int unit_realize_cgroup_now(Unit *u, ManagerState state) { return r; /* Enable controllers above us, if there are any */ - if (UNIT_ISSET(u->slice)) { - r = unit_realize_cgroup_now_enable(UNIT_DEREF(u->slice), state); + slice = UNIT_GET_SLICE(u); + if (slice) { + r = unit_realize_cgroup_now_enable(slice, state); if (r < 0) return r; } @@ -2544,10 +2557,14 @@ void unit_add_family_to_cgroup_realize_queue(Unit *u) { /* Parent comes after children */ unit_add_to_cgroup_realize_queue(u); - } while ((u = UNIT_DEREF(u->slice))); + + u = UNIT_GET_SLICE(u); + } while (u); } int unit_realize_cgroup(Unit *u) { + Unit *slice; + assert(u); if (!UNIT_HAS_CGROUP_CONTEXT(u)) @@ -2562,8 +2579,9 @@ int unit_realize_cgroup(Unit *u) { * This call will defer work on the siblings and derealized ancestors to the next event loop * iteration and synchronously creates the parent cgroups (unit_realize_cgroup_now). */ - if (UNIT_ISSET(u->slice)) - unit_add_family_to_cgroup_realize_queue(UNIT_DEREF(u->slice)); + slice = UNIT_GET_SLICE(u); + if (slice) + unit_add_family_to_cgroup_realize_queue(slice); /* And realize this one now (and apply the values) */ return unit_realize_cgroup_now(u, manager_state(u->manager)); diff --git a/src/core/slice.c b/src/core/slice.c index cb94c97b24f..5d836a11e6b 100644 --- a/src/core/slice.c +++ b/src/core/slice.c @@ -53,7 +53,7 @@ static int slice_add_parent_slice(Slice *s) { assert(s); - if (UNIT_ISSET(u->slice)) + if (UNIT_GET_SLICE(u)) return 0; r = slice_build_parent_slice(u->id, &a); @@ -101,7 +101,7 @@ static int slice_verify(Slice *s) { if (r < 0) return log_unit_error_errno(UNIT(s), r, "Failed to determine parent slice: %m"); - if (parent ? !unit_has_name(UNIT_DEREF(UNIT(s)->slice), parent) : UNIT_ISSET(UNIT(s)->slice)) + if (parent ? !unit_has_name(UNIT_GET_SLICE(UNIT(s)), parent) : !!UNIT_GET_SLICE(UNIT(s))) return log_unit_error_errno(UNIT(s), SYNTHETIC_ERRNO(ENOEXEC), "Located outside of parent slice. Refusing."); return 0; diff --git a/src/core/unit-printf.c b/src/core/unit-printf.c index ee8b0b3de82..113dd1cc8a5 100644 --- a/src/core/unit-printf.c +++ b/src/core/unit-printf.c @@ -132,18 +132,15 @@ static int specifier_cgroup_root(char specifier, const void *data, const void *u } static int specifier_cgroup_slice(char specifier, const void *data, const void *userdata, char **ret) { - const Unit *u = userdata; + const Unit *u = userdata, *slice; char *n; assert(u); bad_specifier(u, specifier); - if (UNIT_ISSET(u->slice)) { - const Unit *slice; - - slice = UNIT_DEREF(u->slice); - + slice = UNIT_GET_SLICE(u); + if (slice) { if (slice->cgroup_path) n = strdup(slice->cgroup_path); else diff --git a/src/core/unit.c b/src/core/unit.c index 9b9c6d01b90..f3eaa595708 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -615,6 +615,7 @@ static void unit_done(Unit *u) { } Unit* unit_free(Unit *u) { + Unit *slice; char *t; if (!u) @@ -662,8 +663,9 @@ Unit* unit_free(Unit *u) { /* A unit is being dropped from the tree, make sure our family is realized properly. Do this after we * detach the unit from slice tree in order to eliminate its effect on controller masks. */ - if (UNIT_ISSET(u->slice)) - unit_add_family_to_cgroup_realize_queue(UNIT_DEREF(u->slice)); + slice = UNIT_GET_SLICE(u); + if (slice) + unit_add_family_to_cgroup_realize_queue(slice); if (u->on_console) manager_unref_console(u->manager); @@ -1397,6 +1399,7 @@ int unit_add_default_target_dependency(Unit *u, Unit *target) { } static int unit_add_slice_dependencies(Unit *u) { + Unit *slice; assert(u); if (!UNIT_HAS_CGROUP_CONTEXT(u)) @@ -1407,8 +1410,9 @@ static int unit_add_slice_dependencies(Unit *u) { relationship). */ UnitDependencyMask mask = u->type == UNIT_SLICE ? UNIT_DEPENDENCY_IMPLICIT : UNIT_DEPENDENCY_FILE; - if (UNIT_ISSET(u->slice)) - return unit_add_two_dependencies(u, UNIT_AFTER, UNIT_REQUIRES, UNIT_DEREF(u->slice), true, mask); + slice = UNIT_GET_SLICE(u); + if (slice) + return unit_add_two_dependencies(u, UNIT_AFTER, UNIT_REQUIRES, slice, true, mask); if (unit_has_name(u, SPECIAL_ROOT_SLICE)) return 0; @@ -3124,11 +3128,11 @@ int unit_set_slice(Unit *u, Unit *slice) { !unit_has_name(slice, SPECIAL_ROOT_SLICE)) return -EPERM; - if (UNIT_DEREF(u->slice) == slice) + if (UNIT_GET_SLICE(u) == slice) return 0; /* Disallow slice changes if @u is already bound to cgroups */ - if (UNIT_ISSET(u->slice) && u->cgroup_realized) + if (UNIT_GET_SLICE(u) && u->cgroup_realized) return -EBUSY; unit_ref_set(&u->slice, u, slice); @@ -3142,7 +3146,7 @@ int unit_set_default_slice(Unit *u) { assert(u); - if (UNIT_ISSET(u->slice)) + if (UNIT_GET_SLICE(u)) return 0; if (u->instance) { @@ -3185,12 +3189,14 @@ int unit_set_default_slice(Unit *u) { } const char *unit_slice_name(Unit *u) { + Unit *slice; assert(u); - if (!UNIT_ISSET(u->slice)) + slice = UNIT_GET_SLICE(u); + if (!slice) return NULL; - return UNIT_DEREF(u->slice)->id; + return slice->id; } int unit_load_related_unit(Unit *u, const char *type, Unit **_found) { diff --git a/src/core/unit.h b/src/core/unit.h index fec5e7a30d4..cf110ebb55f 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -706,6 +706,11 @@ static inline Unit* UNIT_TRIGGER(Unit *u) { return unit_has_dependency(u, UNIT_ATOM_TRIGGERS, NULL); } +static inline Unit* UNIT_GET_SLICE(const Unit *u) { + assert(u); + return u->slice.target; +} + Unit* unit_new(Manager *m, size_t size); Unit* unit_free(Unit *u); DEFINE_TRIVIAL_CLEANUP_FUNC(Unit *, unit_free); diff --git a/src/test/test-cgroup-mask.c b/src/test/test-cgroup-mask.c index 3edc2754e1e..19e159b9ff1 100644 --- a/src/test/test-cgroup-mask.c +++ b/src/test/test-cgroup-mask.c @@ -70,13 +70,13 @@ static int test_cgroup_mask(void) { assert_se(manager_load_startable_unit_or_warn(m, "parent-deep.slice", NULL, &parent_deep) >= 0); assert_se(manager_load_startable_unit_or_warn(m, "nomem.slice", NULL, &nomem_parent) >= 0); assert_se(manager_load_startable_unit_or_warn(m, "nomemleaf.service", NULL, &nomem_leaf) >= 0); - assert_se(UNIT_DEREF(son->slice) == parent); - assert_se(UNIT_DEREF(daughter->slice) == parent); - assert_se(UNIT_DEREF(parent_deep->slice) == parent); - assert_se(UNIT_DEREF(grandchild->slice) == parent_deep); - assert_se(UNIT_DEREF(nomem_leaf->slice) == nomem_parent); - root = UNIT_DEREF(parent->slice); - assert_se(UNIT_DEREF(nomem_parent->slice) == root); + assert_se(UNIT_GET_SLICE(son) == parent); + assert_se(UNIT_GET_SLICE(daughter) == parent); + assert_se(UNIT_GET_SLICE(parent_deep) == parent); + assert_se(UNIT_GET_SLICE(grandchild) == parent_deep); + assert_se(UNIT_GET_SLICE(nomem_leaf) == nomem_parent); + root = UNIT_GET_SLICE(parent); + assert_se(UNIT_GET_SLICE(nomem_parent) == root); /* Verify per-unit cgroups settings. */ ASSERT_CGROUP_MASK_JOINED(unit_get_own_mask(son), CGROUP_MASK_CPU); diff --git a/src/test/test-cgroup-unit-default.c b/src/test/test-cgroup-unit-default.c index b03f6ff12ee..225d138e415 100644 --- a/src/test/test-cgroup-unit-default.c +++ b/src/test/test-cgroup-unit-default.c @@ -91,28 +91,28 @@ static int test_default_memory_low(void) { assert_se(manager_load_startable_unit_or_warn(m, "dml.slice", NULL, &dml) >= 0); assert_se(manager_load_startable_unit_or_warn(m, "dml-passthrough.slice", NULL, &dml_passthrough) >= 0); - assert_se(UNIT_DEREF(dml_passthrough->slice) == dml); + assert_se(UNIT_GET_SLICE(dml_passthrough) == dml); assert_se(manager_load_startable_unit_or_warn(m, "dml-passthrough-empty.service", NULL, &dml_passthrough_empty) >= 0); - assert_se(UNIT_DEREF(dml_passthrough_empty->slice) == dml_passthrough); + assert_se(UNIT_GET_SLICE(dml_passthrough_empty) == dml_passthrough); assert_se(manager_load_startable_unit_or_warn(m, "dml-passthrough-set-dml.service", NULL, &dml_passthrough_set_dml) >= 0); - assert_se(UNIT_DEREF(dml_passthrough_set_dml->slice) == dml_passthrough); + assert_se(UNIT_GET_SLICE(dml_passthrough_set_dml) == dml_passthrough); assert_se(manager_load_startable_unit_or_warn(m, "dml-passthrough-set-ml.service", NULL, &dml_passthrough_set_ml) >= 0); - assert_se(UNIT_DEREF(dml_passthrough_set_ml->slice) == dml_passthrough); + assert_se(UNIT_GET_SLICE(dml_passthrough_set_ml) == dml_passthrough); assert_se(manager_load_startable_unit_or_warn(m, "dml-override.slice", NULL, &dml_override) >= 0); - assert_se(UNIT_DEREF(dml_override->slice) == dml); + assert_se(UNIT_GET_SLICE(dml_override) == dml); assert_se(manager_load_startable_unit_or_warn(m, "dml-override-empty.service", NULL, &dml_override_empty) >= 0); - assert_se(UNIT_DEREF(dml_override_empty->slice) == dml_override); + assert_se(UNIT_GET_SLICE(dml_override_empty) == dml_override); assert_se(manager_load_startable_unit_or_warn(m, "dml-discard.slice", NULL, &dml_discard) >= 0); - assert_se(UNIT_DEREF(dml_discard->slice) == dml); + assert_se(UNIT_GET_SLICE(dml_discard) == dml); assert_se(manager_load_startable_unit_or_warn(m, "dml-discard-empty.service", NULL, &dml_discard_empty) >= 0); - assert_se(UNIT_DEREF(dml_discard_empty->slice) == dml_discard); + assert_se(UNIT_GET_SLICE(dml_discard_empty) == dml_discard); assert_se(manager_load_startable_unit_or_warn(m, "dml-discard-set-ml.service", NULL, &dml_discard_set_ml) >= 0); - assert_se(UNIT_DEREF(dml_discard_set_ml->slice) == dml_discard); + assert_se(UNIT_GET_SLICE(dml_discard_set_ml) == dml_discard); - root = UNIT_DEREF(dml->slice); - assert_se(!UNIT_ISSET(root->slice)); + assert_se(root = UNIT_GET_SLICE(dml)); + assert_se(!UNIT_GET_SLICE(root)); assert_se(unit_get_ancestor_memory_low(root) == CGROUP_LIMIT_MIN);