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.

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"/>

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);

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;

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) {
+                UNIT_FOREACH_DEPENDENCY(member, u, UNIT_ATOM_SLICE_OF)
-                        if (UNIT_DEREF(member->slice) != u)
-                                continue;
-
                         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) {
+                UNIT_FOREACH_DEPENDENCY(m, u, UNIT_ATOM_SLICE_OF) {
-
-                        /* Skip units that have a dependency on the slice but aren't actually in it. */
-                        if (UNIT_DEREF(m->slice) != u)
-                                continue;
 
                         /* 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) {
+                UNIT_FOREACH_DEPENDENCY(member, u, UNIT_ATOM_SLICE_OF)
-                        if (UNIT_DEREF(member->slice) != u)
-                                continue;
-
                         unit_invalidate_cgroup_bpf(member);
-                }
         }
 }
 

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;
 

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;

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);
+        return unit_add_dependency_by_name(u, UNIT_IN_SLICE, a, true, UNIT_DEPENDENCY_IMPLICIT);
-        if (r < 0)
-                return r;
-
-        unit_ref_set(&u->slice, u, parent);
-        return 0;
 }
 
 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) {
+        UNIT_FOREACH_DEPENDENCY(member, s, UNIT_ATOM_SLICE_OF) {
-                int r;
-
-                if (UNIT_DEREF(member->slice) != s)
-                        continue;
 
                 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) {
+        UNIT_FOREACH_DEPENDENCY(member, s, UNIT_ATOM_SLICE_OF) {
-                if (UNIT_DEREF(member->slice) != s)
-                        continue;
-
                 if (action == FREEZER_FREEZE)
                         r = UNIT_VTABLE(member)->freeze(member);
                 else

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;
         }

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;
 

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
+        /* Sets the unit slice if it has not been set before. Is extra careful, to only allow this for units
-         * careful, to only allow this for units that actually have a
+         * that actually have a cgroup context. Also, we don't allow to set this for slices (since the parent
-         * cgroup context. Also, we don't allow to set this for slices
+         * slice is derived from the name). Make sure the unit we set is actually a slice. */
-         * (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) {

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 unit_has_dependency(u, UNIT_ATOM_IN_SLICE, NULL);
-        return u->slice.target;
 }
 
 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_;