]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
core: add UNIT_GET_SLICE() helper
authorLennart Poettering <lennart@poettering.net>
Mon, 26 Apr 2021 16:14:07 +0000 (18:14 +0200)
committerLennart Poettering <lennart@poettering.net>
Tue, 25 May 2021 14:02:00 +0000 (16:02 +0200)
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.

src/core/bpf-firewall.c
src/core/cgroup.c
src/core/slice.c
src/core/unit-printf.c
src/core/unit.c
src/core/unit.h
src/test/test-cgroup-mask.c
src/test/test-cgroup-unit-default.c

index 02e33399c3e36615f9b0932bfbdd0e7363608221..2a41bffee6bbf993583b7bd2f56ebe0aa2415596 100644 (file)
@@ -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);
index 92ac35af8e3aabdc8099a6adb657d401059dcc1f..97820eb264034bb875199ca14c90293e380d358c 100644 (file)
@@ -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));
index cb94c97b24fcb67ce310d9b9ab86be594afa37f6..5d836a11e6b1b16349575328687a45ec5a510fea 100644 (file)
@@ -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;
index ee8b0b3de827fbf9a5e77ba9e242b0e54a693d77..113dd1cc8a5b77b9243a2764eeafa1b4a39240db 100644 (file)
@@ -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
index 9b9c6d01b90a6ed5008d219ff1e378fc9c6d4e86..f3eaa59570873c99265d193fe25fd9484a419fe2 100644 (file)
@@ -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) {
index fec5e7a30d483506f87f80fee843c85ec0468f6e..cf110ebb55f999bc1aecab592397c672e6627928 100644 (file)
@@ -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);
index 3edc2754e1e697bc8ce78afdffff936de45fac3b..19e159b9ff1c892105576a33732a962f46df7804 100644 (file)
@@ -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);
index b03f6ff12ee2808259ff814b1fe3714559c18f41..225d138e41563e281400a0c0a079fb4e0c7d2b43 100644 (file)
@@ -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);