systemd/0001-Revert-job-Don-t-mark-as-redundant-if-deps-are-relev.patch

156 lines
6.0 KiB
Diff
Raw Normal View History

From a9906d1f4aeeaa39a2d57563d23cb7cdd9283bf8 Mon Sep 17 00:00:00 2001
From: Franck Bui <fbui@suse.com>
Date: Wed, 18 Mar 2020 16:18:46 +0100
Subject: [PATCH 1/1] Revert "job: Don't mark as redundant if deps are
relevant"
This reverts commit 097537f07a2fab3cb73aef7bc59f2a66aa93f533, which involves a
significant behavior change which at least impacts plymouth [1] and some of the
services shipped by systemd (systemd-vconsole-setup.service).
Of course some other units shipped by other packages might rely on the old
behavior [2], which makes me wonder why this patch wasn't simply reverted until
the situation gets clarified, at least that what the author of the change
thinks too [3].
[1] https://bugzilla.redhat.com/show_bug.cgi?id=1807771
[2] https://github.com/systemd/systemd/issues/15091#issuecomment-598238061
[3] https://github.com/systemd/systemd/pull/14086#issuecomment-598600479
---
src/core/job.c | 51 +++++++-------------------------------------------
src/core/job.h | 3 +--
src/core/transaction.c | 8 ++++----
3 files changed, 12 insertions(+), 50 deletions(-)
diff --git a/src/core/job.c b/src/core/job.c
index 9fe30359df..8610496109 100644
--- a/src/core/job.c
+++ b/src/core/job.c
@@ -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;
diff --git a/src/core/job.h b/src/core/job.h
index 02b057ee06..03ad640618 100644
--- a/src/core/job.h
+++ b/src/core/job.h
@@ -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. */
diff --git a/src/core/transaction.c b/src/core/transaction.c
index 49f43e0327..6dc4e95beb 100644
--- a/src/core/transaction.c
+++ b/src/core/transaction.c
@@ -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);
--
2.16.4