Revert "job: Don't mark as redundant if deps are relevant"
authorMichael Biebl <biebl@debian.org>
Thu, 12 Mar 2020 12:37:08 +0000 (13:37 +0100)
committerMichael Biebl <biebl@debian.org>
Sat, 18 Apr 2020 18:41:18 +0000 (19:41 +0100)
This reverts commit 097537f07a2fab3cb73aef7bc59f2a66aa93f533.

See https://github.com/systemd/systemd/issues/15091

Closes: #953670
Gbp-Pq: Topic debian
Gbp-Pq: Name Revert-job-Don-t-mark-as-redundant-if-deps-are-relevant.patch

src/core/job.c
src/core/job.h
src/core/transaction.c

index 9fe30359df36dfa4e367bb318f7d0099c19cf089..8610496109090f9cd261e2d6da3c11d07f1b2cc4 100644 (file)
@@ -383,62 +383,25 @@ JobType job_type_lookup_merge(JobType a, JobType b) {
         return job_merging_table[(a - 1) * a / 2 + b];
 }
 
-bool job_later_link_matters(Job *j, JobType type, unsigned generation) {
-        JobDependency *l;
-
-        assert(j);
-
-        j->generation = generation;
-
-        LIST_FOREACH(subject, l, j->subject_list) {
-                UnitActiveState state = _UNIT_ACTIVE_STATE_INVALID;
-
-                /* Have we seen this before? */
-                if (l->object->generation == generation)
-                        continue;
-
-                state = unit_active_state(l->object->unit);
-                switch (type) {
-
-                case JOB_START:
-                        return IN_SET(state, UNIT_INACTIVE, UNIT_FAILED) ||
-                               job_later_link_matters(l->object, type, generation);
-
-                case JOB_STOP:
-                        return IN_SET(state, UNIT_ACTIVE, UNIT_RELOADING) ||
-                               job_later_link_matters(l->object, type, generation);
-
-                default:
-                        assert_not_reached("Invalid job type");
-                }
-        }
-
-        return false;
-}
-
-bool job_is_redundant(Job *j, unsigned generation) {
-
-        assert(j);
-
-        UnitActiveState state = unit_active_state(j->unit);
-        switch (j->type) {
+bool job_type_is_redundant(JobType a, UnitActiveState b) {
+        switch (a) {
 
         case JOB_START:
-                return IN_SET(state, UNIT_ACTIVE, UNIT_RELOADING) && !job_later_link_matters(j, JOB_START, generation);
+                return IN_SET(b, UNIT_ACTIVE, UNIT_RELOADING);
 
         case JOB_STOP:
-                return IN_SET(state, UNIT_INACTIVE, UNIT_FAILED) && !job_later_link_matters(j, JOB_STOP, generation);
+                return IN_SET(b, UNIT_INACTIVE, UNIT_FAILED);
 
         case JOB_VERIFY_ACTIVE:
-                return IN_SET(state, UNIT_ACTIVE, UNIT_RELOADING);
+                return IN_SET(b, UNIT_ACTIVE, UNIT_RELOADING);
 
         case JOB_RELOAD:
                 return
-                        state == UNIT_RELOADING;
+                        b == UNIT_RELOADING;
 
         case JOB_RESTART:
                 return
-                        state == UNIT_ACTIVATING;
+                        b == UNIT_ACTIVATING;
 
         case JOB_NOP:
                 return true;
index 02b057ee0641709d7c26c3a3ea331628c4ed3536..03ad640618e7bf047f9139a2d9df59fa4134dd76 100644 (file)
@@ -196,8 +196,7 @@ _pure_ static inline bool job_type_is_superset(JobType a, JobType b) {
         return a == job_type_lookup_merge(a, b);
 }
 
-bool job_later_link_matters(Job *j, JobType type, unsigned generation);
-bool job_is_redundant(Job *j, unsigned generation);
+bool job_type_is_redundant(JobType a, UnitActiveState b) _pure_;
 
 /* Collapses a state-dependent job type into a simpler type by observing
  * the state of the unit which it is going to be applied to. */
index 49f43e03278c39d4e31a2b07616262330af1ca93..6dc4e95bebc45aa7af0f5e9f9b77538438d85d04 100644 (file)
@@ -279,7 +279,7 @@ static int transaction_merge_jobs(Transaction *tr, sd_bus_error *e) {
         return 0;
 }
 
-static void transaction_drop_redundant(Transaction *tr, unsigned generation) {
+static void transaction_drop_redundant(Transaction *tr) {
         bool again;
 
         /* Goes through the transaction and removes all jobs of the units whose jobs are all noops. If not
@@ -299,7 +299,7 @@ static void transaction_drop_redundant(Transaction *tr, unsigned generation) {
 
                         LIST_FOREACH(transaction, k, j)
                                 if (tr->anchor_job == k ||
-                                    !job_is_redundant(k, generation) ||
+                                    !job_type_is_redundant(k->type, unit_active_state(k->unit)) ||
                                     (k->unit->job && job_type_is_conflicting(k->type, k->unit->job->type))) {
                                         keep = true;
                                         break;
@@ -732,7 +732,7 @@ int transaction_activate(
                 transaction_minimize_impact(tr);
 
         /* Third step: Drop redundant jobs */
-        transaction_drop_redundant(tr, generation++);
+        transaction_drop_redundant(tr);
 
         for (;;) {
                 /* Fourth step: Let's remove unneeded jobs that might
@@ -774,7 +774,7 @@ int transaction_activate(
         }
 
         /* Eights step: Drop redundant jobs again, if the merging now allows us to drop more. */
-        transaction_drop_redundant(tr, generation++);
+        transaction_drop_redundant(tr);
 
         /* Ninth step: check whether we can actually apply this */
         r = transaction_is_destructive(tr, mode, e);