Adam Williamson 3620ae
From 6f202edb2c2e340523c6c0f2c0a93690eaab7a68 Mon Sep 17 00:00:00 2001
Adam Williamson 3620ae
From: Adam Williamson <awilliam@redhat.com>
Adam Williamson 3620ae
Date: Tue, 18 Feb 2020 08:44:34 -0800
Adam Williamson 3620ae
Subject: [PATCH] Revert "job: Don't mark as redundant if deps are relevant"
Adam Williamson 3620ae
Adam Williamson 3620ae
This reverts commit 097537f07a2fab3cb73aef7bc59f2a66aa93f533. It
Adam Williamson 3620ae
causes https://bugzilla.redhat.com/show_bug.cgi?id=1803293 .
Adam Williamson 3620ae
---
Adam Williamson 3620ae
 src/core/job.c         | 51 ++++++------------------------------------
Adam Williamson 3620ae
 src/core/job.h         |  3 +--
Adam Williamson 3620ae
 src/core/transaction.c |  8 +++----
Adam Williamson 3620ae
 3 files changed, 12 insertions(+), 50 deletions(-)
Adam Williamson 3620ae
Adam Williamson 3620ae
diff --git a/src/core/job.c b/src/core/job.c
Adam Williamson 3620ae
index 5982404cf0..5048a5093e 100644
Adam Williamson 3620ae
--- a/src/core/job.c
Adam Williamson 3620ae
+++ b/src/core/job.c
Adam Williamson 3620ae
@@ -383,62 +383,25 @@ JobType job_type_lookup_merge(JobType a, JobType b) {
Adam Williamson 3620ae
         return job_merging_table[(a - 1) * a / 2 + b];
Adam Williamson 3620ae
 }
Adam Williamson 3620ae
 
Adam Williamson 3620ae
-bool job_later_link_matters(Job *j, JobType type, unsigned generation) {
Adam Williamson 3620ae
-        JobDependency *l;
Adam Williamson 3620ae
-
Adam Williamson 3620ae
-        assert(j);
Adam Williamson 3620ae
-
Adam Williamson 3620ae
-        j->generation = generation;
Adam Williamson 3620ae
-
Adam Williamson 3620ae
-        LIST_FOREACH(subject, l, j->subject_list) {
Adam Williamson 3620ae
-                UnitActiveState state = _UNIT_ACTIVE_STATE_INVALID;
Adam Williamson 3620ae
-
Adam Williamson 3620ae
-                /* Have we seen this before? */
Adam Williamson 3620ae
-                if (l->object->generation == generation)
Adam Williamson 3620ae
-                        continue;
Adam Williamson 3620ae
-
Adam Williamson 3620ae
-                state = unit_active_state(l->object->unit);
Adam Williamson 3620ae
-                switch (type) {
Adam Williamson 3620ae
-
Adam Williamson 3620ae
-                case JOB_START:
Adam Williamson 3620ae
-                        return IN_SET(state, UNIT_INACTIVE, UNIT_FAILED) ||
Adam Williamson 3620ae
-                               job_later_link_matters(l->object, type, generation);
Adam Williamson 3620ae
-
Adam Williamson 3620ae
-                case JOB_STOP:
Adam Williamson 3620ae
-                        return IN_SET(state, UNIT_ACTIVE, UNIT_RELOADING) ||
Adam Williamson 3620ae
-                               job_later_link_matters(l->object, type, generation);
Adam Williamson 3620ae
-
Adam Williamson 3620ae
-                default:
Adam Williamson 3620ae
-                        assert_not_reached("Invalid job type");
Adam Williamson 3620ae
-                }
Adam Williamson 3620ae
-        }
Adam Williamson 3620ae
-
Adam Williamson 3620ae
-        return false;
Adam Williamson 3620ae
-}
Adam Williamson 3620ae
-
Adam Williamson 3620ae
-bool job_is_redundant(Job *j, unsigned generation) {
Adam Williamson 3620ae
-
Adam Williamson 3620ae
-        assert(j);
Adam Williamson 3620ae
-
Adam Williamson 3620ae
-        UnitActiveState state = unit_active_state(j->unit);
Adam Williamson 3620ae
-        switch (j->type) {
Adam Williamson 3620ae
+bool job_type_is_redundant(JobType a, UnitActiveState b) {
Adam Williamson 3620ae
+        switch (a) {
Adam Williamson 3620ae
 
Adam Williamson 3620ae
         case JOB_START:
Adam Williamson 3620ae
-                return IN_SET(state, UNIT_ACTIVE, UNIT_RELOADING) && !job_later_link_matters(j, JOB_START, generation);
Adam Williamson 3620ae
+                return IN_SET(b, UNIT_ACTIVE, UNIT_RELOADING);
Adam Williamson 3620ae
 
Adam Williamson 3620ae
         case JOB_STOP:
Adam Williamson 3620ae
-                return IN_SET(state, UNIT_INACTIVE, UNIT_FAILED) && !job_later_link_matters(j, JOB_STOP, generation);
Adam Williamson 3620ae
+                return IN_SET(b, UNIT_INACTIVE, UNIT_FAILED);
Adam Williamson 3620ae
 
Adam Williamson 3620ae
         case JOB_VERIFY_ACTIVE:
Adam Williamson 3620ae
-                return IN_SET(state, UNIT_ACTIVE, UNIT_RELOADING);
Adam Williamson 3620ae
+                return IN_SET(b, UNIT_ACTIVE, UNIT_RELOADING);
Adam Williamson 3620ae
 
Adam Williamson 3620ae
         case JOB_RELOAD:
Adam Williamson 3620ae
                 return
Adam Williamson 3620ae
-                        state == UNIT_RELOADING;
Adam Williamson 3620ae
+                        b == UNIT_RELOADING;
Adam Williamson 3620ae
 
Adam Williamson 3620ae
         case JOB_RESTART:
Adam Williamson 3620ae
                 return
Adam Williamson 3620ae
-                        state == UNIT_ACTIVATING;
Adam Williamson 3620ae
+                        b == UNIT_ACTIVATING;
Adam Williamson 3620ae
 
Adam Williamson 3620ae
         case JOB_NOP:
Adam Williamson 3620ae
                 return true;
Adam Williamson 3620ae
diff --git a/src/core/job.h b/src/core/job.h
Adam Williamson 3620ae
index 02b057ee06..03ad640618 100644
Adam Williamson 3620ae
--- a/src/core/job.h
Adam Williamson 3620ae
+++ b/src/core/job.h
Adam Williamson 3620ae
@@ -196,8 +196,7 @@ _pure_ static inline bool job_type_is_superset(JobType a, JobType b) {
Adam Williamson 3620ae
         return a == job_type_lookup_merge(a, b);
Adam Williamson 3620ae
 }
Adam Williamson 3620ae
 
Adam Williamson 3620ae
-bool job_later_link_matters(Job *j, JobType type, unsigned generation);
Adam Williamson 3620ae
-bool job_is_redundant(Job *j, unsigned generation);
Adam Williamson 3620ae
+bool job_type_is_redundant(JobType a, UnitActiveState b) _pure_;
Adam Williamson 3620ae
 
Adam Williamson 3620ae
 /* Collapses a state-dependent job type into a simpler type by observing
Adam Williamson 3620ae
  * the state of the unit which it is going to be applied to. */
Adam Williamson 3620ae
diff --git a/src/core/transaction.c b/src/core/transaction.c
Adam Williamson 3620ae
index 8d67f9ce1a..a0ea0f0489 100644
Adam Williamson 3620ae
--- a/src/core/transaction.c
Adam Williamson 3620ae
+++ b/src/core/transaction.c
Adam Williamson 3620ae
@@ -279,7 +279,7 @@ static int transaction_merge_jobs(Transaction *tr, sd_bus_error *e) {
Adam Williamson 3620ae
         return 0;
Adam Williamson 3620ae
 }
Adam Williamson 3620ae
 
Adam Williamson 3620ae
-static void transaction_drop_redundant(Transaction *tr, unsigned generation) {
Adam Williamson 3620ae
+static void transaction_drop_redundant(Transaction *tr) {
Adam Williamson 3620ae
         bool again;
Adam Williamson 3620ae
 
Adam Williamson 3620ae
         /* Goes through the transaction and removes all jobs of the units whose jobs are all noops. If not
Adam Williamson 3620ae
@@ -299,7 +299,7 @@ static void transaction_drop_redundant(Transaction *tr, unsigned generation) {
Adam Williamson 3620ae
 
Adam Williamson 3620ae
                         LIST_FOREACH(transaction, k, j)
Adam Williamson 3620ae
                                 if (tr->anchor_job == k ||
Adam Williamson 3620ae
-                                    !job_is_redundant(k, generation) ||
Adam Williamson 3620ae
+                                    !job_type_is_redundant(k->type, unit_active_state(k->unit)) ||
Adam Williamson 3620ae
                                     (k->unit->job && job_type_is_conflicting(k->type, k->unit->job->type))) {
Adam Williamson 3620ae
                                         keep = true;
Adam Williamson 3620ae
                                         break;
Adam Williamson 3620ae
@@ -730,7 +730,7 @@ int transaction_activate(
Adam Williamson 3620ae
                 transaction_minimize_impact(tr);
Adam Williamson 3620ae
 
Adam Williamson 3620ae
         /* Third step: Drop redundant jobs */
Adam Williamson 3620ae
-        transaction_drop_redundant(tr, generation++);
Adam Williamson 3620ae
+        transaction_drop_redundant(tr);
Adam Williamson 3620ae
 
Adam Williamson 3620ae
         for (;;) {
Adam Williamson 3620ae
                 /* Fourth step: Let's remove unneeded jobs that might
Adam Williamson 3620ae
@@ -772,7 +772,7 @@ int transaction_activate(
Adam Williamson 3620ae
         }
Adam Williamson 3620ae
 
Adam Williamson 3620ae
         /* Eights step: Drop redundant jobs again, if the merging now allows us to drop more. */
Adam Williamson 3620ae
-        transaction_drop_redundant(tr, generation++);
Adam Williamson 3620ae
+        transaction_drop_redundant(tr);
Adam Williamson 3620ae
 
Adam Williamson 3620ae
         /* Ninth step: check whether we can actually apply this */
Adam Williamson 3620ae
         r = transaction_is_destructive(tr, mode, e);
Adam Williamson 3620ae
-- 
Adam Williamson 3620ae
2.25.0
Adam Williamson 3620ae