Revert "job: Don't mark as redundant if deps are relevant"

This reverts commit 097537f07a.

At least Fedora and Debian have already reverted this at the distro
level because it causes more problems than it solves. Arch is debating
reverting it as well [0] but would strongly prefer that this happens
upstream first. Fixes #15188.

[0] https://bugs.archlinux.org/task/66458
This commit is contained in:
Dave Reisner 2020-06-11 10:34:13 -04:00 committed by Zbigniew Jędrzejewski-Szmek
parent 0389f4fa81
commit cc479760b4
3 changed files with 12 additions and 50 deletions

View file

@ -387,62 +387,25 @@ JobType job_type_lookup_merge(JobType a, JobType b) {
return job_merging_table[(a - 1) * a / 2 + b]; return job_merging_table[(a - 1) * a / 2 + b];
} }
bool job_later_link_matters(Job *j, JobType type, unsigned generation) { bool job_type_is_redundant(JobType a, UnitActiveState b) {
JobDependency *l; switch (a) {
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) {
case JOB_START: 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: 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: case JOB_VERIFY_ACTIVE:
return IN_SET(state, UNIT_ACTIVE, UNIT_RELOADING); return IN_SET(b, UNIT_ACTIVE, UNIT_RELOADING);
case JOB_RELOAD: case JOB_RELOAD:
return return
state == UNIT_RELOADING; b == UNIT_RELOADING;
case JOB_RESTART: case JOB_RESTART:
return return
state == UNIT_ACTIVATING; b == UNIT_ACTIVATING;
case JOB_NOP: case JOB_NOP:
return true; return true;

View 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); return a == job_type_lookup_merge(a, b);
} }
bool job_later_link_matters(Job *j, JobType type, unsigned generation); bool job_type_is_redundant(JobType a, UnitActiveState b) _pure_;
bool job_is_redundant(Job *j, unsigned generation);
/* Collapses a state-dependent job type into a simpler type by observing /* 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. */ * the state of the unit which it is going to be applied to. */

View file

@ -279,7 +279,7 @@ static int transaction_merge_jobs(Transaction *tr, sd_bus_error *e) {
return 0; return 0;
} }
static void transaction_drop_redundant(Transaction *tr, unsigned generation) { static void transaction_drop_redundant(Transaction *tr) {
bool again; bool again;
/* Goes through the transaction and removes all jobs of the units whose jobs are all noops. If not /* 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) LIST_FOREACH(transaction, k, j)
if (tr->anchor_job == k || 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))) { (k->unit->job && job_type_is_conflicting(k->type, k->unit->job->type))) {
keep = true; keep = true;
break; break;
@ -735,7 +735,7 @@ int transaction_activate(
transaction_minimize_impact(tr); transaction_minimize_impact(tr);
/* Third step: Drop redundant jobs */ /* Third step: Drop redundant jobs */
transaction_drop_redundant(tr, generation++); transaction_drop_redundant(tr);
for (;;) { for (;;) {
/* Fourth step: Let's remove unneeded jobs that might /* Fourth step: Let's remove unneeded jobs that might
@ -777,7 +777,7 @@ int transaction_activate(
} }
/* Eights step: Drop redundant jobs again, if the merging now allows us to drop more. */ /* 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 */ /* Ninth step: check whether we can actually apply this */
r = transaction_is_destructive(tr, mode, e); r = transaction_is_destructive(tr, mode, e);