From 2f9e75564e04ed3e6f7aacb264ea69f5582050ca Mon Sep 17 00:00:00 2001

From: Anita Zhang <the.anitazha@gmail.com>

Date: Tue, 19 Oct 2021 18:00:58 -0700

Subject: [PATCH] Revert "core: convert Slice= into a proper dependency (and

 add a back dependency)"

This reverts commit d219a2b07cc5dc8ffd5010f08561fab2780d8616.

---

 man/org.freedesktop.systemd1.xml |  6 -----

 src/basic/unit-def.c             |  2 --

 src/basic/unit-def.h             |  4 ----

 src/core/cgroup.c                | 23 ++++++++++++++----

 src/core/dbus-unit.c             |  3 +--

 src/core/load-fragment.c         |  2 +-

 src/core/slice.c                 | 21 +++++++++++++----

 src/core/unit-dependency-atom.c  |  8 -------

 src/core/unit-dependency-atom.h  |  4 +---

 src/core/unit.c                  | 40 +++++++++-----------------------

 src/core/unit.h                  |  7 ++++--

 11 files changed, 54 insertions(+), 66 deletions(-)

diff --git a/man/org.freedesktop.systemd1.xml b/man/org.freedesktop.systemd1.xml

index c14c5b6601..c87feea201 100644

--- a/man/org.freedesktop.systemd1.xml

+++ b/man/org.freedesktop.systemd1.xml

@@ -1650,8 +1650,6 @@ 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 = ['...', ...];

@@ -1799,8 +1797,6 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice {

-    

-

@@ -1949,8 +1945,6 @@ 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 2667e61dc4..23e4825bd7 100644

--- a/src/basic/unit-def.c

+++ b/src/basic/unit-def.c

@@ -289,8 +289,6 @@ 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 08651efa57..e72452702a 100644

--- a/src/basic/unit-def.h

+++ b/src/basic/unit-def.h

@@ -254,10 +254,6 @@ 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 5c07aa71d1..9033bea169 100644

--- a/src/core/cgroup.c

+++ b/src/core/cgroup.c

@@ -1706,8 +1706,12 @@ CGroupMask unit_get_members_mask(Unit *u) {

         if (u->type == UNIT_SLICE) {

                 Unit *member;

-                UNIT_FOREACH_DEPENDENCY(member, u, UNIT_ATOM_SLICE_OF)

+                UNIT_FOREACH_DEPENDENCY(member, u, UNIT_ATOM_BEFORE) {

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

@@ -2365,10 +2369,13 @@ 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_SLICE_OF) {

+        UNIT_FOREACH_DEPENDENCY(m, u, UNIT_ATOM_BEFORE) {

                 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,7 +2537,11 @@ 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_SLICE_OF) {

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

                         /* No point in doing cgroup application for units without active processes. */

                         if (UNIT_IS_INACTIVE_OR_FAILED(unit_active_state(m)))

@@ -3869,8 +3880,12 @@ void unit_invalidate_cgroup_bpf(Unit *u) {

         if (u->type == UNIT_SLICE) {

                 Unit *member;

-                UNIT_FOREACH_DEPENDENCY(member, u, UNIT_ATOM_SLICE_OF)

+                UNIT_FOREACH_DEPENDENCY(member, u, UNIT_ATOM_BEFORE) {

+                        if (UNIT_DEREF(member->slice) != u)

+                                continue;

+

                         unit_invalidate_cgroup_bpf(member);

+                }

         }

 }

diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c

index 124b9e496b..f861988bd8 100644

--- a/src/core/dbus-unit.c

+++ b/src/core/dbus-unit.c

@@ -875,7 +875,6 @@ const sd_bus_vtable bus_unit_vtable[] = {

         SD_BUS_PROPERTY("PropagatesStopTo", "as", property_get_dependencies, 0, SD_BUS_VTABLE_PROPERTY_CONST),

         SD_BUS_PROPERTY("StopPropagatedFrom", "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),

@@ -2273,7 +2272,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, UNIT_DEPENDENCY_FILE);

+                        r = unit_set_slice(u, slice);

                         if (r < 0)

                                 return r;

diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c

index 8fb3c378ee..a068fdf313 100644

--- a/src/core/load-fragment.c

+++ b/src/core/load-fragment.c

@@ -3575,7 +3575,7 @@ int config_parse_unit_slice(

                 return 0;

         }

-        r = unit_set_slice(u, slice, UNIT_DEPENDENCY_FILE);

+        r = unit_set_slice(u, slice);

         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 2e43c00119..009c98b434 100644

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

+        Unit *u = UNIT(s), *parent;

         _cleanup_free_ char *a = NULL;

         int r;

@@ -60,7 +60,12 @@ static int slice_add_parent_slice(Slice *s) {

         if (r <= 0) /* 0 means root slice */

                 return r;

-        return unit_add_dependency_by_name(u, UNIT_IN_SLICE, a, true, UNIT_DEPENDENCY_IMPLICIT);

+        r = manager_load_unit(u->manager, a, NULL, NULL, &parent);

+        if (r < 0)

+                return r;

+

+        unit_ref_set(&u->slice, u, parent);

+        return 0;

 }

 static int slice_add_default_dependencies(Slice *s) {

@@ -349,11 +354,14 @@ 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_SLICE_OF) {

+        UNIT_FOREACH_DEPENDENCY(member, s, UNIT_ATOM_BEFORE) {

+                int r;

+

+                if (UNIT_DEREF(member->slice) != s)

+                        continue;

                 if (member->type == UNIT_SLICE) {

                         r = slice_freezer_action_supported_by_children(member);

@@ -380,7 +388,10 @@ static int slice_freezer_action(Unit *s, FreezerAction action) {

                 return 0;

         }

-        UNIT_FOREACH_DEPENDENCY(member, s, UNIT_ATOM_SLICE_OF) {

+        UNIT_FOREACH_DEPENDENCY(member, s, UNIT_ATOM_BEFORE) {

+                if (UNIT_DEREF(member->slice) != s)

+                        continue;

+

                 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 761835828a..9a8e4615f1 100644

--- a/src/core/unit-dependency-atom.c

+++ b/src/core/unit-dependency-atom.c

@@ -93,8 +93,6 @@ 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

@@ -228,12 +226,6 @@ 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 c5f8f5d400..f79f1b732a 100644

--- a/src/core/unit-dependency-atom.h

+++ b/src/core/unit-dependency-atom.h

@@ -77,9 +77,7 @@ 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_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_MAX                     = (UINT64_C(1) << 32) - 1,

         _UNIT_DEPENDENCY_ATOM_INVALID                 = -EINVAL,

 } UnitDependencyAtom;

diff --git a/src/core/unit.c b/src/core/unit.c

index 38d3eb703f..48810a2ba9 100644

--- a/src/core/unit.c

+++ b/src/core/unit.c

@@ -692,10 +692,11 @@ 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);

@@ -721,6 +722,7 @@ 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);

@@ -3032,8 +3034,6 @@ int unit_add_dependency(

                 [UNIT_JOINS_NAMESPACE_OF] = UNIT_JOINS_NAMESPACE_OF, /* symmetric! 👓 */

                 [UNIT_REFERENCES] = UNIT_REFERENCED_BY,

                 [UNIT_REFERENCED_BY] = UNIT_REFERENCES,

-                [UNIT_IN_SLICE] = UNIT_SLICE_OF,

-                [UNIT_SLICE_OF] = UNIT_IN_SLICE,

         };

         Unit *original_u = u, *original_other = other;

         UnitDependencyAtom a;

@@ -3077,21 +3077,6 @@ 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;

@@ -3270,15 +3255,15 @@ reset:

         return r;

 }

-int unit_set_slice(Unit *u, Unit *slice, UnitDependencyMask mask) {

-        int r;

-

+int unit_set_slice(Unit *u, Unit *slice) {

         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;

@@ -3303,10 +3288,7 @@ int unit_set_slice(Unit *u, Unit *slice, UnitDependencyMask mask) {

         if (UNIT_GET_SLICE(u) && u->cgroup_realized)

                 return -EBUSY;

-        r = unit_add_dependency(u, UNIT_IN_SLICE, slice, true, mask);

-        if (r < 0)

-                return r;

-

+        unit_ref_set(&u->slice, u, slice);

         return 1;

 }

@@ -3356,7 +3338,7 @@ int unit_set_default_slice(Unit *u) {

         if (r < 0)

                 return r;

-        return unit_set_slice(u, slice, UNIT_DEPENDENCY_FILE);

+        return unit_set_slice(u, slice);

 }

 const char *unit_slice_name(Unit *u) {

diff --git a/src/core/unit.h b/src/core/unit.h

index 759104ffa7..7a8bfd0568 100644

--- a/src/core/unit.h

+++ b/src/core/unit.h

@@ -202,6 +202,8 @@ typedef struct Unit {

         dual_timestamp active_exit_timestamp;

         dual_timestamp inactive_enter_timestamp;

+        UnitRef slice;

+

         /* Per type list */

         LIST_FIELDS(Unit, units_by_type);

@@ -716,7 +718,8 @@ static inline Unit* UNIT_TRIGGER(Unit *u) {

 }

 static inline Unit* UNIT_GET_SLICE(const Unit *u) {

-        return unit_has_dependency(u, UNIT_ATOM_IN_SLICE, NULL);

+        assert(u);

+        return u->slice.target;

 }

 Unit* unit_new(Manager *m, size_t size);

@@ -761,7 +764,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, UnitDependencyMask mask);

+int unit_set_slice(Unit *u, Unit *slice);

 int unit_set_default_slice(Unit *u);

 const char *unit_description(Unit *u) _pure_;

-- 

2.31.1