core: convert Slice= into a proper dependency (and add a back dependency)

The slice a unit is assigned to is currently a UnitRef reference. Let's
turn it into a proper dependency, to simplify and clean up code a bit.
Now that new dep types are cheaper, deps should generally be preferable
over everything else, if the concept applies.

This brings one major benefit: we often have to iterate through all unit
a slice contains. So far we iterated through all Before= dependencies of
the slice unit to achieve that, filtering out unrelated units, and
taking benefit of the fact that slice units are implicitly ordered
Before= the units they contain. By making Slice= a proper dependency,
and having an accompanying SliceOf= dependency type, this is much
simpler and nicer as we can directly enumerate the units a slice
contains.

The forward dependency is actually called InSlice internally, since we
already used the UNIT_SLICE name as UnitType field. However, since we
don't intend to expose the dependency to users as dep anyway (we already
have the regular Slice D-Bus property for this) this shouldn't matter.
The SliceOf= implicit dependency type (the erverse of Slice=/InSlice=)
is exported over the bus, to make things a bit nicer to debug and
discoverable.

diff --git a/man/org.freedesktop.systemd1.xml b/man/org.freedesktop.systemd1.xml
index 3d6f1a39b3be35573b75acda7b0be4cb79f381ed..de724126405bb7d9f89f03024aae0f696528cb50 100644 (file)
--- a/man/org.freedesktop.systemd1.xml
+++ b/man/org.freedesktop.systemd1.xml
@@ -1640,6 +1640,8 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice {
       @org.freedesktop.DBus.Property.EmitsChangedSignal("const")
       readonly as JoinsNamespaceOf = ['...', ...];
       @org.freedesktop.DBus.Property.EmitsChangedSignal("const")
+      readonly as SliceOf = ['...', ...];
+      @org.freedesktop.DBus.Property.EmitsChangedSignal("const")
       readonly as RequiresMountsFor = ['...', ...];
       @org.freedesktop.DBus.Property.EmitsChangedSignal("const")
       readonly as Documentation = ['...', ...];
@@ -1775,6 +1777,8 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice {
 
     <!--property JoinsNamespaceOf is not documented!-->
 
+    <!--property SliceOf is not documented!-->
+
     <!--property FreezerState is not documented!-->
 
     <!--property DropInPaths is not documented!-->
@@ -1911,6 +1915,8 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice {
 
     <variablelist class="dbus-property" generated="True" extra-ref="JoinsNamespaceOf"/>
 
+    <variablelist class="dbus-property" generated="True" extra-ref="SliceOf"/>
+
     <variablelist class="dbus-property" generated="True" extra-ref="RequiresMountsFor"/>
 
     <variablelist class="dbus-property" generated="True" extra-ref="Documentation"/>

diff --git a/src/basic/unit-def.c b/src/basic/unit-def.c
index 5cfabca83fb08fcae548ce83943483c1b564ef66..56516203178503ab39ed92668781ee7f311766cf 100644 (file)
--- a/src/basic/unit-def.c
+++ b/src/basic/unit-def.c
@@ -282,6 +282,8 @@ static const char* const unit_dependency_table[_UNIT_DEPENDENCY_MAX] = {
         [UNIT_JOINS_NAMESPACE_OF] = "JoinsNamespaceOf",
         [UNIT_REFERENCES] = "References",
         [UNIT_REFERENCED_BY] = "ReferencedBy",
+        [UNIT_IN_SLICE] = "InSlice",
+        [UNIT_SLICE_OF] = "SliceOf",
 };
 
 DEFINE_STRING_TABLE_LOOKUP(unit_dependency, UnitDependency);

diff --git a/src/basic/unit-def.h b/src/basic/unit-def.h
index c30135f4d6c73c5a467ddd98d834bedbc7c8c285..7d76576de10c255db95d0e497e7a96c14c70b1db 100644 (file)
--- a/src/basic/unit-def.h
+++ b/src/basic/unit-def.h
@@ -245,6 +245,10 @@ typedef enum UnitDependency {
         UNIT_REFERENCES,              /* Inverse of 'references' is 'referenced_by' */
         UNIT_REFERENCED_BY,
 
+        /* Slice= */
+        UNIT_IN_SLICE,
+        UNIT_SLICE_OF,
+
         _UNIT_DEPENDENCY_MAX,
         _UNIT_DEPENDENCY_INVALID = -EINVAL,
 } UnitDependency;

diff --git a/src/core/cgroup.c b/src/core/cgroup.c
index 97820eb264034bb875199ca14c90293e380d358c..5b00faa6f6297f6483025348e77f0261e4a7c7a6 100644 (file)
--- a/src/core/cgroup.c
+++ b/src/core/cgroup.c
@@ -1702,12 +1702,8 @@ CGroupMask unit_get_members_mask(Unit *u) {
         if (u->type == UNIT_SLICE) {
                 Unit *member;
 
-                UNIT_FOREACH_DEPENDENCY(member, u, UNIT_ATOM_BEFORE) {
-                        if (UNIT_DEREF(member->slice) != u)
-                                continue;
-
+                UNIT_FOREACH_DEPENDENCY(member, u, UNIT_ATOM_SLICE_OF)
                         u->cgroup_members_mask |= unit_get_subtree_mask(member); /* note that this calls ourselves again, for the children */
-                }
         }
 
         u->cgroup_members_mask_valid = true;
@@ -2362,13 +2358,10 @@ static int unit_realize_cgroup_now_disable(Unit *u, ManagerState state) {
         if (u->type != UNIT_SLICE)
                 return 0;
 
-        UNIT_FOREACH_DEPENDENCY(m, u, UNIT_ATOM_BEFORE) {
+        UNIT_FOREACH_DEPENDENCY(m, u, UNIT_ATOM_SLICE_OF) {
                 CGroupMask target_mask, enable_mask, new_target_mask, new_enable_mask;
                 int r;
 
-                if (UNIT_DEREF(m->slice) != u)
-                        continue;
-
                 /* The cgroup for this unit might not actually be fully realised yet, in which case it isn't
                  * holding any controllers open anyway. */
                 if (!m->cgroup_realized)
@@ -2530,11 +2523,7 @@ void unit_add_family_to_cgroup_realize_queue(Unit *u) {
                 /* Children of u likely changed when we're called */
                 u->cgroup_members_mask_valid = false;
 
-                UNIT_FOREACH_DEPENDENCY(m, u, UNIT_ATOM_BEFORE) {
-
-                        /* Skip units that have a dependency on the slice but aren't actually in it. */
-                        if (UNIT_DEREF(m->slice) != u)
-                                continue;
+                UNIT_FOREACH_DEPENDENCY(m, u, UNIT_ATOM_SLICE_OF) {
 
                         /* No point in doing cgroup application for units without active processes. */
                         if (UNIT_IS_INACTIVE_OR_FAILED(unit_active_state(m)))
@@ -3799,12 +3788,8 @@ void unit_invalidate_cgroup_bpf(Unit *u) {
         if (u->type == UNIT_SLICE) {
                 Unit *member;
 
-                UNIT_FOREACH_DEPENDENCY(member, u, UNIT_ATOM_BEFORE) {
-                        if (UNIT_DEREF(member->slice) != u)
-                                continue;
-
+                UNIT_FOREACH_DEPENDENCY(member, u, UNIT_ATOM_SLICE_OF)
                         unit_invalidate_cgroup_bpf(member);
-                }
         }
 }
 

diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c
index e8818c919c038b48225d146fa341a3af26f39722..a1278dd6e9865184e880fda8a9ea74d65ad12847 100644 (file)
--- a/src/core/dbus-unit.c
+++ b/src/core/dbus-unit.c
@@ -870,6 +870,7 @@ const sd_bus_vtable bus_unit_vtable[] = {
         SD_BUS_PROPERTY("PropagatesReloadTo", "as", property_get_dependencies, 0, SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("ReloadPropagatedFrom", "as", property_get_dependencies, 0, SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("JoinsNamespaceOf", "as", property_get_dependencies, 0, SD_BUS_VTABLE_PROPERTY_CONST),
+        SD_BUS_PROPERTY("SliceOf", "as", property_get_dependencies, 0, SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("RequiresMountsFor", "as", property_get_requires_mounts_for, offsetof(Unit, requires_mounts_for), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("Documentation", "as", NULL, offsetof(Unit, documentation), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("Description", "s", property_get_description, 0, SD_BUS_VTABLE_PROPERTY_CONST),
@@ -2237,7 +2238,7 @@ static int bus_unit_set_transient_property(
                         return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Unit name '%s' is not a slice", s);
 
                 if (!UNIT_WRITE_FLAGS_NOOP(flags)) {
-                        r = unit_set_slice(u, slice);
+                        r = unit_set_slice(u, slice, UNIT_DEPENDENCY_FILE);
                         if (r < 0)
                                 return r;
 

diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index cd373145426726027e1e79c25637c3494b316280..72378a408276daf6abc3ac590d3d956c39f258f2 100644 (file)
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -3574,7 +3574,7 @@ int config_parse_unit_slice(
                 return 0;
         }
 
-        r = unit_set_slice(u, slice);
+        r = unit_set_slice(u, slice, UNIT_DEPENDENCY_FILE);
         if (r < 0) {
                 log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to assign slice %s to unit %s, ignoring: %m", slice->id, u->id);
                 return 0;

diff --git a/src/core/slice.c b/src/core/slice.c
index 5d836a11e6b1b16349575328687a45ec5a510fea..595f704a17a43a3ebe8ed8616ccf3c5d12092980 100644 (file)
--- a/src/core/slice.c
+++ b/src/core/slice.c
@@ -47,7 +47,7 @@ static void slice_set_state(Slice *t, SliceState state) {
 }
 
 static int slice_add_parent_slice(Slice *s) {
-        Unit *u = UNIT(s), *parent;
+        Unit *u = UNIT(s);
         _cleanup_free_ char *a = NULL;
         int r;
 
@@ -60,12 +60,7 @@ static int slice_add_parent_slice(Slice *s) {
         if (r <= 0) /* 0 means root slice */
                 return r;
 
-        r = manager_load_unit(u->manager, a, NULL, NULL, &parent);
-        if (r < 0)
-                return r;
-
-        unit_ref_set(&u->slice, u, parent);
-        return 0;
+        return unit_add_dependency_by_name(u, UNIT_IN_SLICE, a, true, UNIT_DEPENDENCY_IMPLICIT);
 }
 
 static int slice_add_default_dependencies(Slice *s) {
@@ -346,14 +341,11 @@ static void slice_enumerate_perpetual(Manager *m) {
 
 static bool slice_freezer_action_supported_by_children(Unit *s) {
         Unit *member;
+        int r;
 
         assert(s);
 
-        UNIT_FOREACH_DEPENDENCY(member, s, UNIT_ATOM_BEFORE) {
-                int r;
-
-                if (UNIT_DEREF(member->slice) != s)
-                        continue;
+        UNIT_FOREACH_DEPENDENCY(member, s, UNIT_ATOM_SLICE_OF) {
 
                 if (member->type == UNIT_SLICE) {
                         r = slice_freezer_action_supported_by_children(member);
@@ -380,10 +372,7 @@ static int slice_freezer_action(Unit *s, FreezerAction action) {
                 return 0;
         }
 
-        UNIT_FOREACH_DEPENDENCY(member, s, UNIT_ATOM_BEFORE) {
-                if (UNIT_DEREF(member->slice) != s)
-                        continue;
-
+        UNIT_FOREACH_DEPENDENCY(member, s, UNIT_ATOM_SLICE_OF) {
                 if (action == FREEZER_FREEZE)
                         r = UNIT_VTABLE(member)->freeze(member);
                 else

diff --git a/src/core/unit-dependency-atom.c b/src/core/unit-dependency-atom.c
index 4b012ed265a1cad4e195f624cc5441011221dc73..cfd5acd33b47fec1deed46a6d0721c181e54cc69 100644 (file)
--- a/src/core/unit-dependency-atom.c
+++ b/src/core/unit-dependency-atom.c
@@ -78,6 +78,8 @@ static const UnitDependencyAtom atom_map[_UNIT_DEPENDENCY_MAX] = {
         [UNIT_JOINS_NAMESPACE_OF]     = UNIT_ATOM_JOINS_NAMESPACE_OF,
         [UNIT_REFERENCES]             = UNIT_ATOM_REFERENCES,
         [UNIT_REFERENCED_BY]          = UNIT_ATOM_REFERENCED_BY,
+        [UNIT_IN_SLICE]               = UNIT_ATOM_IN_SLICE,
+        [UNIT_SLICE_OF]               = UNIT_ATOM_SLICE_OF,
 
         /* These are dependency types without effect on our state engine. We maintain them only to make
          * things discoverable/debuggable as they are the inverse dependencies to some of the above. As they
@@ -190,6 +192,12 @@ UnitDependency unit_dependency_from_unique_atom(UnitDependencyAtom atom) {
         case UNIT_ATOM_REFERENCED_BY:
                 return UNIT_REFERENCED_BY;
 
+        case UNIT_ATOM_IN_SLICE:
+                return UNIT_IN_SLICE;
+
+        case UNIT_ATOM_SLICE_OF:
+                return UNIT_SLICE_OF;
+
         default:
                 return _UNIT_DEPENDENCY_INVALID;
         }

diff --git a/src/core/unit-dependency-atom.h b/src/core/unit-dependency-atom.h
index d04d6134a0c21c310edc23e62289f00ba951fee7..01482215118378fb463e10d04eb290d7406db535 100644 (file)
--- a/src/core/unit-dependency-atom.h
+++ b/src/core/unit-dependency-atom.h
@@ -69,7 +69,9 @@ typedef enum UnitDependencyAtom {
         UNIT_ATOM_JOINS_NAMESPACE_OF                  = UINT64_C(1) << 29,
         UNIT_ATOM_REFERENCES                          = UINT64_C(1) << 30,
         UNIT_ATOM_REFERENCED_BY                       = UINT64_C(1) << 31,
-        _UNIT_DEPENDENCY_ATOM_MAX                     = (UINT64_C(1) << 32) - 1,
+        UNIT_ATOM_IN_SLICE                            = UINT64_C(1) << 32,
+        UNIT_ATOM_SLICE_OF                            = UINT64_C(1) << 33,
+        _UNIT_DEPENDENCY_ATOM_MAX                     = (UINT64_C(1) << 34) - 1,
         _UNIT_DEPENDENCY_ATOM_INVALID                 = -EINVAL,
 } UnitDependencyAtom;
 

diff --git a/src/core/unit.c b/src/core/unit.c
index f3eaa59570873c99265d193fe25fd9484a419fe2..c94f4e75236db11d85552a7401dc6e8d344e6f43 100644 (file)
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -659,11 +659,10 @@ Unit* unit_free(Unit *u) {
                 job_free(j);
         }
 
-        unit_clear_dependencies(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. */
         slice = UNIT_GET_SLICE(u);
+        unit_clear_dependencies(u);
         if (slice)
                 unit_add_family_to_cgroup_realize_queue(slice);
 
@@ -689,7 +688,6 @@ Unit* unit_free(Unit *u) {
 
         unit_unwatch_all_pids(u);
 
-        unit_ref_unset(&u->slice);
         while (u->refs_by_target)
                 unit_ref_unset(u->refs_by_target);
 
@@ -2881,6 +2879,8 @@ int unit_add_dependency(
                 [UNIT_PROPAGATES_RELOAD_TO] = UNIT_RELOAD_PROPAGATED_FROM,
                 [UNIT_RELOAD_PROPAGATED_FROM] = UNIT_PROPAGATES_RELOAD_TO,
                 [UNIT_JOINS_NAMESPACE_OF] = UNIT_JOINS_NAMESPACE_OF, /* symmetric! 👓 */
+                [UNIT_IN_SLICE] = UNIT_SLICE_OF,
+                [UNIT_SLICE_OF] = UNIT_IN_SLICE,
         };
         Unit *original_u = u, *original_other = other;
         UnitDependencyAtom a;
@@ -2924,6 +2924,21 @@ int unit_add_dependency(
                 return log_unit_error_errno(u, SYNTHETIC_ERRNO(EINVAL),
                                             "Requested dependency TriggeredBy=%s refused (%s units cannot trigger other units).", other->id, unit_type_to_string(other->type));
 
+        if (FLAGS_SET(a, UNIT_ATOM_IN_SLICE) && other->type != UNIT_SLICE)
+                return log_unit_error_errno(u, SYNTHETIC_ERRNO(EINVAL),
+                                            "Requested dependency Slice=%s refused (%s is not a slice unit).", other->id, other->id);
+        if (FLAGS_SET(a, UNIT_ATOM_SLICE_OF) && u->type != UNIT_SLICE)
+                return log_unit_error_errno(u, SYNTHETIC_ERRNO(EINVAL),
+                                            "Requested dependency SliceOf=%s refused (%s is not a slice unit).", other->id, u->id);
+
+        if (FLAGS_SET(a, UNIT_ATOM_IN_SLICE) && !UNIT_HAS_CGROUP_CONTEXT(u))
+                return log_unit_error_errno(u, SYNTHETIC_ERRNO(EINVAL),
+                                            "Requested dependency Slice=%s refused (%s is not a cgroup unit).", other->id, u->id);
+
+        if (FLAGS_SET(a, UNIT_ATOM_SLICE_OF) && !UNIT_HAS_CGROUP_CONTEXT(other))
+                return log_unit_error_errno(u, SYNTHETIC_ERRNO(EINVAL),
+                                            "Requested dependency SliceOf=%s refused (%s is not a cgroup unit).", other->id, other->id);
+
         r = unit_add_dependency_hashmap(&u->dependencies, d, other, mask, 0);
         if (r < 0)
                 return r;
@@ -3102,15 +3117,15 @@ reset:
         return r;
 }
 
-int unit_set_slice(Unit *u, Unit *slice) {
+int unit_set_slice(Unit *u, Unit *slice, UnitDependencyMask mask) {
+        int r;
+
         assert(u);
         assert(slice);
 
-        /* Sets the unit slice if it has not been set before. Is extra
-         * careful, to only allow this for units that actually have a
-         * cgroup context. Also, we don't allow to set this for slices
-         * (since the parent slice is derived from the name). Make
-         * sure the unit we set is actually a slice. */
+        /* Sets the unit slice if it has not been set before. Is extra careful, to only allow this for units
+         * that actually have a cgroup context. Also, we don't allow to set this for slices (since the parent
+         * slice is derived from the name). Make sure the unit we set is actually a slice. */
 
         if (!UNIT_HAS_CGROUP_CONTEXT(u))
                 return -EOPNOTSUPP;
@@ -3135,7 +3150,10 @@ int unit_set_slice(Unit *u, Unit *slice) {
         if (UNIT_GET_SLICE(u) && u->cgroup_realized)
                 return -EBUSY;
 
-        unit_ref_set(&u->slice, u, slice);
+        r = unit_add_dependency(u, UNIT_IN_SLICE, slice, true, mask);
+        if (r < 0)
+                return r;
+
         return 1;
 }
 
@@ -3185,7 +3203,7 @@ int unit_set_default_slice(Unit *u) {
         if (r < 0)
                 return r;
 
-        return unit_set_slice(u, slice);
+        return unit_set_slice(u, slice, UNIT_DEPENDENCY_FILE);
 }
 
 const char *unit_slice_name(Unit *u) {

diff --git a/src/core/unit.h b/src/core/unit.h
index cf110ebb55f999bc1aecab592397c672e6627928..6b3f48596f14f37559bab937734567f24d803a3d 100644 (file)
--- a/src/core/unit.h
+++ b/src/core/unit.h
@@ -202,8 +202,6 @@ typedef struct Unit {
         dual_timestamp active_exit_timestamp;
         dual_timestamp inactive_enter_timestamp;
 
-        UnitRef slice;
-
         /* Per type list */
         LIST_FIELDS(Unit, units_by_type);
 
@@ -707,8 +705,7 @@ static inline Unit* UNIT_TRIGGER(Unit *u) {
 }
 
 static inline Unit* UNIT_GET_SLICE(const Unit *u) {
-        assert(u);
-        return u->slice.target;
+        return unit_has_dependency(u, UNIT_ATOM_IN_SLICE, NULL);
 }
 
 Unit* unit_new(Manager *m, size_t size);
@@ -751,7 +748,7 @@ Unit *unit_follow_merge(Unit *u) _pure_;
 int unit_load_fragment_and_dropin(Unit *u, bool fragment_required);
 int unit_load(Unit *unit);
 
-int unit_set_slice(Unit *u, Unit *slice);
+int unit_set_slice(Unit *u, Unit *slice, UnitDependencyMask mask);
 int unit_set_default_slice(Unit *u);
 
 const char *unit_description(Unit *u) _pure_;