diff options
author | Michal Koutný <mkoutny@suse.com> | 2019-06-06 23:27:20 +0200 |
---|---|---|
committer | Michal Koutný <xm.koutny@gmail.com> | 2019-06-26 00:00:43 +0200 |
commit | e602f15282bcb413857e5ed06b0b02985cea1cbe (patch) | |
tree | 9d24a3dc629c72ca0e664ca074305a753d47c6f4 /src/core | |
parent | 33fe9e3fd054e70a1f9e5af359bfe3dd8435ce67 (diff) | |
download | systemd-e602f15282bcb413857e5ed06b0b02985cea1cbe.tar.gz |
core: Extract job ordering logic
The job ordering logic is spread at multiple places of the code, making
it hard to maintain and also a bit to understand. The actual execution
order of two jobs always depends on their types and the ordering
contraint between their units. Extract this logic to a new function
job_compare. The second change is simplification of the order
evaluation, JOB_STOP takes always precedence (as documented), unless two
units are both stopping, then the ordering constraint is taken into
account.
Diffstat (limited to 'src/core')
-rw-r--r-- | src/core/job.c | 161 | ||||
-rw-r--r-- | src/core/job.h | 2 |
2 files changed, 76 insertions, 87 deletions
diff --git a/src/core/job.c b/src/core/job.c index 81f5f9cb72..780f92c4d2 100644 --- a/src/core/job.c +++ b/src/core/job.c @@ -477,36 +477,14 @@ static bool job_is_runnable(Job *j) { if (j->type == JOB_NOP) return true; - if (IN_SET(j->type, JOB_START, JOB_VERIFY_ACTIVE, JOB_RELOAD)) { - /* Immediate result is that the job is or might be - * started. In this case let's wait for the - * dependencies, regardless whether they are - * starting or stopping something. */ - - HASHMAP_FOREACH_KEY(v, other, j->unit->dependencies[UNIT_AFTER], i) - if (other->job) - return false; - } - - /* Also, if something else is being stopped and we should - * change state after it, then let's wait. */ + HASHMAP_FOREACH_KEY(v, other, j->unit->dependencies[UNIT_AFTER], i) + if (other->job && job_compare(j, other->job, UNIT_AFTER) > 0) + return false; HASHMAP_FOREACH_KEY(v, other, j->unit->dependencies[UNIT_BEFORE], i) - if (other->job && - IN_SET(other->job->type, JOB_STOP, JOB_RESTART)) + if (other->job && job_compare(j, other->job, UNIT_BEFORE) > 0) return false; - /* This means that for a service a and a service b where b - * shall be started after a: - * - * start a + start b → 1st step start a, 2nd step start b - * start a + stop b → 1st step stop b, 2nd step start a - * stop a + start b → 1st step stop a, 2nd step start b - * stop a + stop b → 1st step stop b, 2nd step stop a - * - * This has the side effect that restarts are properly - * synchronized too. */ - return true; } @@ -1455,46 +1433,14 @@ bool job_may_gc(Job *j) { if (j->type == JOB_NOP) return false; - /* If a job is ordered after ours, and is to be started, then it needs to wait for us, regardless if we stop or - * start, hence let's not GC in that case. */ - HASHMAP_FOREACH_KEY(v, other, j->unit->dependencies[UNIT_BEFORE], i) { - if (!other->job) - continue; - - if (other->job->ignore_order) - continue; - - if (IN_SET(other->job->type, JOB_START, JOB_VERIFY_ACTIVE, JOB_RELOAD)) + /* The logic is inverse to job_is_runnable, we cannot GC as long as we block any job. */ + HASHMAP_FOREACH_KEY(v, other, j->unit->dependencies[UNIT_BEFORE], i) + if (other->job && job_compare(j, other->job, UNIT_BEFORE) < 0) return false; - } - - /* If we are going down, but something else is ordered After= us, then it needs to wait for us */ - if (IN_SET(j->type, JOB_STOP, JOB_RESTART)) - HASHMAP_FOREACH_KEY(v, other, j->unit->dependencies[UNIT_AFTER], i) { - if (!other->job) - continue; - - if (other->job->ignore_order) - continue; + HASHMAP_FOREACH_KEY(v, other, j->unit->dependencies[UNIT_AFTER], i) + if (other->job && job_compare(j, other->job, UNIT_AFTER) < 0) return false; - } - - /* The logic above is kinda the inverse of the job_is_runnable() logic. Specifically, if the job "we" is - * ordered before the job "other": - * - * we start + other start → stay - * we start + other stop → gc - * we stop + other start → stay - * we stop + other stop → gc - * - * "we" are ordered after "other": - * - * we start + other start → gc - * we start + other stop → gc - * we stop + other start → stay - * we stop + other stop → stay - */ return true; } @@ -1512,7 +1458,7 @@ void job_add_to_gc_queue(Job *j) { j->in_gc_queue = true; } -static int job_compare(Job * const *a, Job * const *b) { +static int job_compare_id(Job * const *a, Job * const *b) { return CMP((*a)->id, (*b)->id); } @@ -1521,7 +1467,7 @@ static size_t sort_job_list(Job **list, size_t n) { size_t a, b; /* Order by numeric IDs */ - typesafe_qsort(list, n, job_compare); + typesafe_qsort(list, n, job_compare_id); /* Filter out duplicates */ for (a = 0, b = 0; a < n; a++) { @@ -1552,23 +1498,21 @@ int job_get_before(Job *j, Job*** ret) { return 0; } - if (IN_SET(j->type, JOB_START, JOB_VERIFY_ACTIVE, JOB_RELOAD)) { - - HASHMAP_FOREACH_KEY(v, other, j->unit->dependencies[UNIT_AFTER], i) { - if (!other->job) - continue; + HASHMAP_FOREACH_KEY(v, other, j->unit->dependencies[UNIT_AFTER], i) { + if (!other->job) + continue; + if (job_compare(j, other->job, UNIT_AFTER) <= 0) + continue; - if (!GREEDY_REALLOC(list, n_allocated, n+1)) - return -ENOMEM; - list[n++] = other->job; - } + if (!GREEDY_REALLOC(list, n_allocated, n+1)) + return -ENOMEM; + list[n++] = other->job; } HASHMAP_FOREACH_KEY(v, other, j->unit->dependencies[UNIT_BEFORE], i) { if (!other->job) continue; - - if (!IN_SET(other->job->type, JOB_STOP, JOB_RESTART)) + if (job_compare(j, other->job, UNIT_BEFORE) <= 0) continue; if (!GREEDY_REALLOC(list, n_allocated, n+1)) @@ -1602,7 +1546,7 @@ int job_get_after(Job *j, Job*** ret) { if (other->job->ignore_order) continue; - if (!IN_SET(other->job->type, JOB_START, JOB_VERIFY_ACTIVE, JOB_RELOAD)) + if (job_compare(j, other->job, UNIT_BEFORE) >= 0) continue; if (!GREEDY_REALLOC(list, n_allocated, n+1)) @@ -1610,19 +1554,20 @@ int job_get_after(Job *j, Job*** ret) { list[n++] = other->job; } - if (IN_SET(j->type, JOB_STOP, JOB_RESTART)) { - HASHMAP_FOREACH_KEY(v, other, j->unit->dependencies[UNIT_AFTER], i) { - if (!other->job) - continue; + HASHMAP_FOREACH_KEY(v, other, j->unit->dependencies[UNIT_AFTER], i) { + if (!other->job) + continue; + + if (other->job->ignore_order) + continue; - if (other->job->ignore_order) - continue; + if (job_compare(j, other->job, UNIT_AFTER) >= 0) + continue; - if (!GREEDY_REALLOC(list, n_allocated, n+1)) - return -ENOMEM; - list[n++] = other->job; - } + if (!GREEDY_REALLOC(list, n_allocated, n+1)) + return -ENOMEM; + list[n++] = other->job; } n = sort_job_list(list, n); @@ -1692,3 +1637,45 @@ const char* job_type_to_access_method(JobType t) { else return "reload"; } + +/* + * assume_dep assumed dependency between units (a is before/after b) + * + * Returns + * 0 jobs are independent, + * >0 a should run after b, + * <0 a should run before b, + * + * The logic means that for a service a and a service b where b.After=a: + * + * start a + start b → 1st step start a, 2nd step start b + * start a + stop b → 1st step stop b, 2nd step start a + * stop a + start b → 1st step stop a, 2nd step start b + * stop a + stop b → 1st step stop b, 2nd step stop a + * + * This has the side effect that restarts are properly + * synchronized too. + */ +int job_compare(Job *a, Job *b, UnitDependency assume_dep) { + assert(a->type < _JOB_TYPE_MAX_IN_TRANSACTION); + assert(b->type < _JOB_TYPE_MAX_IN_TRANSACTION); + assert(IN_SET(assume_dep, UNIT_AFTER, UNIT_BEFORE)); + + /* Trivial cases first */ + if (a->type == JOB_NOP || b->type == JOB_NOP) + return 0; + + if (a->ignore_order || b->ignore_order) + return 0; + + if (assume_dep == UNIT_AFTER) + return -job_compare(b, a, UNIT_BEFORE); + + /* Let's make it simple, JOB_STOP goes always first (in case both ua and ub stop, + * then ub's stop goes first anyway). + * JOB_RESTART is JOB_STOP in disguise (before it is patched to JOB_START). */ + if (IN_SET(b->type, JOB_STOP, JOB_RESTART)) + return 1; + else + return -1; +} diff --git a/src/core/job.h b/src/core/job.h index 0f15cbf821..9c79c873d1 100644 --- a/src/core/job.h +++ b/src/core/job.h @@ -237,3 +237,5 @@ const char* job_result_to_string(JobResult t) _const_; JobResult job_result_from_string(const char *s) _pure_; const char* job_type_to_access_method(JobType t); + +int job_compare(Job *a, Job *b, UnitDependency assume_dep); |