kernel: sched: change to 3-way thread priority comparison

`z_is_t1_higher_prio_than_t2` was being called twice in both the
context-switch fastpath and in `z_priq_rb_lessthan`, just to
dealing with priority ties. In addition, the API was error-prone
(and too much in the fastpath to be able to assert its invarients)
- see also #32710 for a previous example of this API breaking
and returning a>b but also b>a.

Replacing this with a direct 3-way comparison `z_cmp_t1_prio_with_t2`
sidesteps most of these issues. There is still a concern that
`sgn(z_cmp_t1_prio_with_t2(a,b)) != -sgn(z_cmp_t1_prio_with_t2(b,a))`
but I don't see any way to alleviate this aside from adding an
assert to the fastpath.

Signed-off-by: James Harris <james.harris@intel.com>
This commit is contained in:
James Harris 2021-03-01 09:19:57 -08:00 committed by Anas Nashif
commit 2cd0f66515
3 changed files with 40 additions and 23 deletions

View file

@ -219,7 +219,7 @@ static inline bool z_is_prio_lower_or_equal(int prio1, int prio2)
return z_is_prio1_lower_than_or_equal_to_prio2(prio1, prio2);
}
bool z_is_t1_higher_prio_than_t2(struct k_thread *t1, struct k_thread *t2);
int32_t z_sched_prio_cmp(struct k_thread *t1, struct k_thread *t2);
static inline bool _is_valid_prio(int prio, void *entry_point)
{

View file

@ -100,15 +100,15 @@ static inline void add_event(sys_dlist_t *events, struct k_poll_event *event,
pending = (struct k_poll_event *)sys_dlist_peek_tail(events);
if ((pending == NULL) ||
z_is_t1_higher_prio_than_t2(poller_thread(pending->poller),
poller_thread(poller))) {
(z_sched_prio_cmp(poller_thread(pending->poller),
poller_thread(poller)) > 0)) {
sys_dlist_append(events, &event->_node);
return;
}
SYS_DLIST_FOR_EACH_CONTAINER(events, pending, _node) {
if (z_is_t1_higher_prio_than_t2(poller_thread(poller),
poller_thread(pending->poller))) {
if (z_sched_prio_cmp(poller_thread(poller),
poller_thread(pending->poller)) > 0) {
sys_dlist_insert(&pending->_node, &event->_node);
return;
}

View file

@ -81,30 +81,46 @@ static inline bool is_thread_dummy(struct k_thread *thread)
}
#endif
bool z_is_t1_higher_prio_than_t2(struct k_thread *thread_1,
/*
* Return value same as e.g. memcmp
* > 0 -> thread 1 priority > thread 2 priority
* = 0 -> thread 1 priority == thread 2 priority
* < 0 -> thread 1 priority < thread 2 priority
* Do not rely on the actual value returned aside from the above.
* (Again, like memcmp.)
*/
int32_t z_sched_prio_cmp(struct k_thread *thread_1,
struct k_thread *thread_2)
{
if (thread_1->base.prio < thread_2->base.prio) {
return true;
/* `prio` is <32b, so the below cannot overflow. */
int32_t b1 = thread_1->base.prio;
int32_t b2 = thread_2->base.prio;
if (b1 != b2) {
return b2 - b1;
}
#ifdef CONFIG_SCHED_DEADLINE
/* If we assume all deadlines live within the same "half" of
* the 32 bit modulus space (this is a documented API rule),
* then the latest dealine in the queue minus the earliest is
* then the latest deadline in the queue minus the earliest is
* guaranteed to be (2's complement) non-negative. We can
* leverage that to compare the values without having to check
* the current time.
*/
if (thread_1->base.prio == thread_2->base.prio) {
int32_t d1 = thread_1->base.prio_deadline;
int32_t d2 = thread_2->base.prio_deadline;
uint32_t d1 = thread_1->base.prio_deadline;
uint32_t d2 = thread_2->base.prio_deadline;
return (d2 - d1) > 0;
if (d1 != d2) {
/* Sooner deadline means higher effective priority.
* Doing the calculation with unsigned types and casting
* to signed isn't perfect, but at least reduces this
* from UB on overflow to impdef.
*/
return (int32_t) (d2 - d1);
}
#endif
return false;
return 0;
}
static ALWAYS_INLINE bool should_preempt(struct k_thread *thread,
@ -277,12 +293,10 @@ static ALWAYS_INLINE struct k_thread *next_up(void)
}
if (active) {
bool curr_wins = z_is_t1_higher_prio_than_t2(_current, thread);
bool th_wins = z_is_t1_higher_prio_than_t2(thread, _current);
bool tie = (!curr_wins && !th_wins);
int32_t cmp = z_sched_prio_cmp(_current, thread);
/* Ties only switch if state says we yielded */
if (curr_wins || (tie && !_current_cpu->swap_ok)) {
if ((cmp > 0) || ((cmp == 0) && !_current_cpu->swap_ok)) {
thread = _current;
}
@ -944,7 +958,7 @@ ALWAYS_INLINE void z_priq_dumb_add(sys_dlist_t *pq, struct k_thread *thread)
__ASSERT_NO_MSG(!z_is_idle_thread_object(thread));
SYS_DLIST_FOR_EACH_CONTAINER(pq, t, base.qnode_dlist) {
if (z_is_t1_higher_prio_than_t2(thread, t)) {
if (z_sched_prio_cmp(thread, t) > 0) {
sys_dlist_insert(&t->base.qnode_dlist,
&thread->base.qnode_dlist);
return;
@ -975,13 +989,16 @@ struct k_thread *z_priq_dumb_best(sys_dlist_t *pq)
bool z_priq_rb_lessthan(struct rbnode *a, struct rbnode *b)
{
struct k_thread *thread_a, *thread_b;
int32_t cmp;
thread_a = CONTAINER_OF(a, struct k_thread, base.qnode_rb);
thread_b = CONTAINER_OF(b, struct k_thread, base.qnode_rb);
if (z_is_t1_higher_prio_than_t2(thread_a, thread_b)) {
cmp = z_sched_prio_cmp(thread_a, thread_b);
if (cmp > 0) {
return true;
} else if (z_is_t1_higher_prio_than_t2(thread_b, thread_a)) {
} else if (cmp < 0) {
return false;
} else {
return thread_a->base.order_key < thread_b->base.order_key