Google Git
Sign in
pigweed/third_party/github/zephyrproject-rtos/zephyr/8606fabf74c0082057b0caf6526907f660523cac^!/./kernel
commit
8606fabf74c0082057b0caf6526907f660523cac
[log]
author
Andy Ross <andrew.j.ross@intel.com>
Mon Mar 26 10:54:40 2018 -0700
committer
Anas Nashif <nashif@linux.intel.com>
Tue Apr 24 03:57:20 2018 +0530
tree
b070d19a68e43d663c02ce2a8c457d0c64683409
parent
b481d0a045900cd45cfc24100266d9ed58ce827e [diff]
kernel: Scheduler refactoring: use _reschedule_*() always

There was a somewhat promiscuous pattern in the kernel where IPC
mechanisms would do something that might effect the current thread
choice, then check _must_switch_threads() (or occasionally
__must_switch_threads -- don't ask, the distinction is being replaced
by real English words), sometimes _is_in_isr() (but not always, even
in contexts where that looks like it would be a mistake), and then
call _Swap() if everything is OK, otherwise releasing the irq_lock().
Sometimes this was done directly, sometimes via the inverted test,
sometimes (poll, heh) by doing the test when the thread state was
modified and then needlessly passing the result up the call stack to
the point of the _Swap().

And some places were just calling _reschedule_threads(), which did all
this already.

Unify all this madness.  The old _reschedule_threads() function has
split into two variants: _reschedule_yield() and
_reschedule_noyield().  The latter is the "normal" one that respects
the cooperative priority of the current thread (i.e. it won't switch
out even if there is a higher priority thread ready -- the current
thread has to pend itself first), the former is used in the handful of
places where code was doing a swap unconditionally, just to preserve
precise behavior across the refactor.  I'm not at all convinced it
should exist...

Signed-off-by: Andy Ross <andrew.j.ross@intel.com>

diff --git a/kernel/include/ksched.h b/kernel/include/ksched.h
index bc6d41e..caae854 100644
--- a/kernel/include/ksched.h
+++ b/kernel/include/ksched.h

@@ -21,13 +21,13 @@
 
 extern void _add_thread_to_ready_q(struct k_thread *thread);
 extern void _remove_thread_from_ready_q(struct k_thread *thread);
-extern void _reschedule_threads(int key);
+extern int _reschedule_noyield(int key);
+extern int _reschedule_yield(int key);
 extern void k_sched_unlock(void);
 extern void _pend_thread(struct k_thread *thread,
 			 _wait_q_t *wait_q, s32_t timeout);
 extern void _pend_current_thread(_wait_q_t *wait_q, s32_t timeout);
 extern void _move_thread_to_end_of_prio_q(struct k_thread *thread);
-extern int __must_switch_threads(void);
 extern int _is_thread_time_slicing(struct k_thread *thread);
 extern void _update_time_slice_before_swap(void);
 #ifdef _NON_OPTIMIZED_TICKS_PER_SEC
@@ -263,15 +263,6 @@
 #endif
 
 /*
- * Checks if current thread must be context-switched out. The caller must
- * already know that the execution context is a thread.
- */
-static inline int _must_switch_threads(void)
-{
-	return _is_preempt(_current) && __must_switch_threads();
-}
-
-/*
  * Called directly by other internal kernel code.
  * Exposed to applications via k_sched_lock(), which just calls this
  */

diff --git a/kernel/mailbox.c b/kernel/mailbox.c
index 1c5bb95..42bd8a0 100644
--- a/kernel/mailbox.c
+++ b/kernel/mailbox.c

@@ -219,7 +219,7 @@
 	_set_thread_return_value(sending_thread, 0);
 	_mark_thread_as_not_pending(sending_thread);
 	_ready_thread(sending_thread);
-	_reschedule_threads(key);
+	_reschedule_noyield(key);
 }
 
 /**
@@ -276,7 +276,7 @@
 			 * until the receiver consumes the message
 			 */
 			if (sending_thread->base.thread_state & _THREAD_DUMMY) {
-				_reschedule_threads(key);
+				_reschedule_noyield(key);
 				return 0;
 			}
 #endif

diff --git a/kernel/mem_slab.c b/kernel/mem_slab.c
index 63d3507..2dd8b53 100644
--- a/kernel/mem_slab.c
+++ b/kernel/mem_slab.c

@@ -123,15 +123,11 @@
 		_set_thread_return_value_with_data(pending_thread, 0, *mem);
 		_abort_thread_timeout(pending_thread);
 		_ready_thread(pending_thread);
-		if (_must_switch_threads()) {
-			_Swap(key);
-			return;
-		}
 	} else {
 		**(char ***)mem = slab->free_list;
 		slab->free_list = *(char **)mem;
 		slab->num_used--;
 	}
 
-	irq_unlock(key);
+	_reschedule_noyield(key);
 }

diff --git a/kernel/mempool.c b/kernel/mempool.c
index 71c706f..83b2437 100644
--- a/kernel/mempool.c
+++ b/kernel/mempool.c

@@ -112,7 +112,7 @@
 	}
 
 	if (need_sched && !_is_in_isr()) {
-		_reschedule_threads(key);
+		_reschedule_noyield(key);
 	} else {
 		irq_unlock(key);
 	}

diff --git a/kernel/msg_q.c b/kernel/msg_q.c
index 90867d0..cd9810e 100644
--- a/kernel/msg_q.c
+++ b/kernel/msg_q.c

@@ -95,10 +95,8 @@
 			_set_thread_return_value(pending_thread, 0);
 			_abort_thread_timeout(pending_thread);
 			_ready_thread(pending_thread);
-			if (!_is_in_isr() && _must_switch_threads()) {
-				_Swap(key);
-				return 0;
-			}
+			_reschedule_noyield(key);
+			return 0;
 		} else {
 			/* put message in queue */
 			memcpy(q->write_ptr, data, q->msg_size);
@@ -116,7 +114,7 @@
 		/* wait for put message success, failure, or timeout */
 		_pend_current_thread(&q->wait_q, timeout);
 		_current->base.swap_data = data;
-		return _Swap(key);
+		return _reschedule_yield(key);
 	}
 
 	irq_unlock(key);
@@ -188,10 +186,8 @@
 			_set_thread_return_value(pending_thread, 0);
 			_abort_thread_timeout(pending_thread);
 			_ready_thread(pending_thread);
-			if (!_is_in_isr() && _must_switch_threads()) {
-				_Swap(key);
-				return 0;
-			}
+			_reschedule_noyield(key);
+			return 0;
 		}
 		result = 0;
 	} else if (timeout == K_NO_WAIT) {
@@ -236,7 +232,7 @@
 	q->used_msgs = 0;
 	q->read_ptr = q->write_ptr;
 
-	_reschedule_threads(key);
+	_reschedule_noyield(key);
 }
 
 #ifdef CONFIG_USERSPACE

diff --git a/kernel/pipes.c b/kernel/pipes.c
index c8eba45..d720403 100644
--- a/kernel/pipes.c
+++ b/kernel/pipes.c

@@ -511,7 +511,7 @@
 		_sched_unlock_no_reschedule();
 		_pend_thread((struct k_thread *) &async_desc->thread,
 			     &pipe->wait_q.writers, K_FOREVER);
-		_reschedule_threads(key);
+		_reschedule_noyield(key);
 		return 0;
 	}
 #endif

diff --git a/kernel/poll.c b/kernel/poll.c
index 66e5940..7959476 100644
--- a/kernel/poll.c
+++ b/kernel/poll.c

@@ -267,11 +267,8 @@
 }
 
 /* must be called with interrupts locked */
-static int signal_poll_event(struct k_poll_event *event, u32_t state,
-			      int *must_reschedule)
+static int signal_poll_event(struct k_poll_event *event, u32_t state)
 {
-	*must_reschedule = 0;
-
 	if (!event->poller) {
 		goto ready_event;
 	}
@@ -300,26 +297,20 @@
 	}
 
 	_ready_thread(thread);
-	*must_reschedule = !_is_in_isr() && _must_switch_threads();
 
 ready_event:
 	set_event_ready(event, state);
 	return 0;
 }
 
-/* returns 1 if a reschedule must take place, 0 otherwise */
-int _handle_obj_poll_events(sys_dlist_t *events, u32_t state)
+void _handle_obj_poll_events(sys_dlist_t *events, u32_t state)
 {
 	struct k_poll_event *poll_event;
-	int must_reschedule;
 
 	poll_event = (struct k_poll_event *)sys_dlist_get(events);
-	if (!poll_event) {
-		return 0;
+	if (poll_event) {
+		(void) signal_poll_event(poll_event, state);
 	}
-
-	(void) signal_poll_event(poll_event, state, &must_reschedule);
-	return must_reschedule;
 }
 
 void k_poll_signal_init(struct k_poll_signal *signal)
@@ -333,7 +324,6 @@
 {
 	unsigned int key = irq_lock();
 	struct k_poll_event *poll_event;
-	int must_reschedule;
 
 	signal->result = result;
 	signal->signaled = 1;
@@ -344,14 +334,8 @@
 		return 0;
 	}
 
-	int rc = signal_poll_event(poll_event, K_POLL_STATE_SIGNALED,
-				    &must_reschedule);
+	int rc = signal_poll_event(poll_event, K_POLL_STATE_SIGNALED);
 
-	if (must_reschedule) {
-		(void)_Swap(key);
-	} else {
-		irq_unlock(key);
-	}
-
+	_reschedule_noyield(key);
 	return rc;
 }

diff --git a/kernel/queue.c b/kernel/queue.c
index d570a38..e960a83 100644
--- a/kernel/queue.c
+++ b/kernel/queue.c

@@ -68,13 +68,10 @@
 }
 #endif /* CONFIG_POLL */
 
-/* returns 1 if a reschedule must take place, 0 otherwise */
-static inline int handle_poll_events(struct k_queue *queue, u32_t state)
+static inline void handle_poll_events(struct k_queue *queue, u32_t state)
 {
 #ifdef CONFIG_POLL
-	return _handle_obj_poll_events(&queue->poll_events, state);
-#else
-	return 0;
+	_handle_obj_poll_events(&queue->poll_events, state);
 #endif
 }
 
@@ -88,19 +85,12 @@
 
 	if (first_pending_thread) {
 		prepare_thread_to_run(first_pending_thread, NULL);
-		if (!_is_in_isr() && _must_switch_threads()) {
-			(void)_Swap(key);
-			return;
-		}
 	}
 #else
-	if (handle_poll_events(queue, K_POLL_STATE_NOT_READY)) {
-		(void)_Swap(key);
-		return;
-	}
+	handle_poll_events(queue, K_POLL_STATE_NOT_READY);
 #endif /* !CONFIG_POLL */
 
-	irq_unlock(key);
+	_reschedule_noyield(key);
 }
 
 void k_queue_insert(struct k_queue *queue, void *prev, void *data)
@@ -113,11 +103,7 @@
 
 	if (first_pending_thread) {
 		prepare_thread_to_run(first_pending_thread, data);
-		if (!_is_in_isr() && _must_switch_threads()) {
-			(void)_Swap(key);
-			return;
-		}
-		irq_unlock(key);
+		_reschedule_noyield(key);
 		return;
 	}
 #endif /* !CONFIG_POLL */
@@ -125,13 +111,10 @@
 	sys_slist_insert(&queue->data_q, prev, data);
 
 #if defined(CONFIG_POLL)
-	if (handle_poll_events(queue, K_POLL_STATE_DATA_AVAILABLE)) {
-		(void)_Swap(key);
-		return;
-	}
+	handle_poll_events(queue, K_POLL_STATE_DATA_AVAILABLE);
 #endif /* CONFIG_POLL */
 
-	irq_unlock(key);
+	_reschedule_noyield(key);
 }
 
 void k_queue_append(struct k_queue *queue, void *data)
@@ -148,7 +131,6 @@
 {
 	__ASSERT(head && tail, "invalid head or tail");
 
-	int need_sched = 0;
 	unsigned int key = irq_lock();
 #if !defined(CONFIG_POLL)
 	struct k_thread *thread;
@@ -156,7 +138,6 @@
 	while (head && ((thread = _unpend_first_thread(&queue->wait_q)))) {
 		prepare_thread_to_run(thread, head);
 		head = *(void **)head;
-		need_sched = 1;
 	}
 
 	if (head) {
@@ -165,18 +146,10 @@
 
 #else
 	sys_slist_append_list(&queue->data_q, head, tail);
-	if (handle_poll_events(queue, K_POLL_STATE_DATA_AVAILABLE)) {
-		(void)_Swap(key);
-		return;
-	}
+	handle_poll_events(queue, K_POLL_STATE_DATA_AVAILABLE);
 #endif /* !CONFIG_POLL */
 
-	if (need_sched) {
-		_reschedule_threads(key);
-		return;
-	} else {
-		irq_unlock(key);
-	}
+	_reschedule_noyield(key);
 }
 
 void k_queue_merge_slist(struct k_queue *queue, sys_slist_t *list)

diff --git a/kernel/sched.c b/kernel/sched.c
index 34f16c2..1564e6a 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c

@@ -135,23 +135,34 @@
 #endif
 }
 
-/* reschedule threads if the scheduler is not locked */
-/* not callable from ISR */
-/* must be called with interrupts locked */
-void _reschedule_threads(int key)
+/* Releases the irq_lock and swaps to a higher priority thread if one
+ * is available, returning the _Swap() return value, otherwise zero.
+ * Does not swap away from a thread at a cooperative (unpreemptible)
+ * priority unless "yield" is true.
+ */
+static int resched(int key, int yield)
 {
-#ifdef CONFIG_PREEMPT_ENABLED
 	K_DEBUG("rescheduling threads\n");
 
-	if (_must_switch_threads()) {
+	if (!_is_in_isr() &&
+	    (yield || _is_preempt(_current)) &&
+	    _is_prio_higher(_get_highest_ready_prio(), _current->base.prio)) {
 		K_DEBUG("context-switching out %p\n", _current);
-		_Swap(key);
+		return _Swap(key);
 	} else {
 		irq_unlock(key);
+		return 0;
 	}
-#else
-	irq_unlock(key);
-#endif
+}
+
+int _reschedule_noyield(int key)
+{
+	return resched(key, 0);
+}
+
+int _reschedule_yield(int key)
+{
+	return resched(key, 1);
 }
 
 void k_sched_lock(void)
@@ -174,7 +185,7 @@
 	K_DEBUG("scheduler unlocked (%p:%d)\n",
 		_current, _current->base.sched_locked);
 
-	_reschedule_threads(key);
+	_reschedule_noyield(key);
 #endif
 }
 
@@ -237,43 +248,6 @@
 	_pend_thread(_current, wait_q, timeout);
 }
 
-#if defined(CONFIG_PREEMPT_ENABLED) && defined(CONFIG_KERNEL_DEBUG)
-/* debug aid */
-static void dump_ready_q(void)
-{
-	K_DEBUG("bitmaps: ");
-	for (int bitmap = 0; bitmap < K_NUM_PRIO_BITMAPS; bitmap++) {
-		K_DEBUG("%x", _ready_q.prio_bmap[bitmap]);
-	}
-	K_DEBUG("\n");
-	for (int prio = 0; prio < K_NUM_PRIORITIES; prio++) {
-		K_DEBUG("prio: %d, head: %p\n",
-			prio - _NUM_COOP_PRIO,
-			sys_dlist_peek_head(&_ready_q.q[prio]));
-	}
-}
-#endif  /* CONFIG_PREEMPT_ENABLED && CONFIG_KERNEL_DEBUG */
-
-/*
- * Check if there is a thread of higher prio than the current one. Should only
- * be called if we already know that the current thread is preemptible.
- */
-int __must_switch_threads(void)
-{
-#ifdef CONFIG_PREEMPT_ENABLED
-	K_DEBUG("current prio: %d, highest prio: %d\n",
-		_current->base.prio, _get_highest_ready_prio());
-
-#ifdef CONFIG_KERNEL_DEBUG
-	dump_ready_q();
-#endif  /* CONFIG_KERNEL_DEBUG */
-
-	return _is_prio_higher(_get_highest_ready_prio(), _current->base.prio);
-#else
-	return 0;
-#endif
-}
-
 int _impl_k_thread_priority_get(k_tid_t thread)
 {
 	return thread->base.prio;
@@ -297,7 +271,7 @@
 	int key = irq_lock();
 
 	_thread_priority_set(thread, prio);
-	_reschedule_threads(key);
+	_reschedule_noyield(key);
 }
 
 #ifdef CONFIG_USERSPACE
@@ -431,7 +405,7 @@
 	if (_is_in_isr()) {
 		irq_unlock(key);
 	} else {
-		_reschedule_threads(key);
+		_reschedule_noyield(key);
 	}
 }
 

diff --git a/kernel/sem.c b/kernel/sem.c
index 4436868..cddd348 100644
--- a/kernel/sem.c
+++ b/kernel/sem.c

@@ -82,15 +82,10 @@
 }
 #endif
 
-/* returns 1 if a reschedule must take place, 0 otherwise */
-static inline int handle_poll_events(struct k_sem *sem)
+static inline void handle_poll_events(struct k_sem *sem)
 {
 #ifdef CONFIG_POLL
-	u32_t state = K_POLL_STATE_SEM_AVAILABLE;
-
-	return _handle_obj_poll_events(&sem->poll_events, state);
-#else
-	return 0;
+	_handle_obj_poll_events(&sem->poll_events, K_POLL_STATE_SEM_AVAILABLE);
 #endif
 }
 
@@ -99,20 +94,18 @@
 	sem->count += (sem->count != sem->limit);
 }
 
-/* returns 1 if _Swap() will need to be invoked, 0 otherwise */
-static int do_sem_give(struct k_sem *sem)
+static void do_sem_give(struct k_sem *sem)
 {
 	struct k_thread *thread = _unpend_first_thread(&sem->wait_q);
 
-	if (!thread) {
+	if (thread) {
+		(void)_abort_thread_timeout(thread);
+		_ready_thread(thread);
+		_set_thread_return_value(thread, 0);
+	} else {
 		increment_count_up_to_limit(sem);
-		return handle_poll_events(sem);
+		handle_poll_events(sem);
 	}
-	(void)_abort_thread_timeout(thread);
-	_ready_thread(thread);
-	_set_thread_return_value(thread, 0);
-
-	return !_is_in_isr() && _must_switch_threads();
 }
 
 /*
@@ -140,15 +133,10 @@
 
 void _impl_k_sem_give(struct k_sem *sem)
 {
-	unsigned int key;
+	unsigned int key = irq_lock();
 
-	key = irq_lock();
-
-	if (do_sem_give(sem)) {
-		_Swap(key);
-	} else {
-		irq_unlock(key);
-	}
+	do_sem_give(sem);
+	_reschedule_noyield(key);
 }
 
 #ifdef CONFIG_USERSPACE

diff --git a/kernel/stack.c b/kernel/stack.c
index 1d0c608..2288a8f 100644
--- a/kernel/stack.c
+++ b/kernel/stack.c

@@ -86,17 +86,14 @@
 
 		_set_thread_return_value_with_data(first_pending_thread,
 						   0, (void *)data);
-
-		if (!_is_in_isr() && _must_switch_threads()) {
-			(void)_Swap(key);
-			return;
-		}
+		_reschedule_noyield(key);
+		return;
 	} else {
 		*(stack->next) = data;
 		stack->next++;
+		irq_unlock(key);
 	}
 
-	irq_unlock(key);
 }
 
 #ifdef CONFIG_USERSPACE

diff --git a/kernel/thread.c b/kernel/thread.c
index 1f77fbc..69d8709 100644
--- a/kernel/thread.c
+++ b/kernel/thread.c

@@ -228,7 +228,7 @@
 
 	_mark_thread_as_started(thread);
 	_ready_thread(thread);
-	_reschedule_threads(key);
+	_reschedule_noyield(key);
 }
 
 #ifdef CONFIG_USERSPACE
@@ -484,7 +484,7 @@
 
 	_k_thread_single_resume(thread);
 
-	_reschedule_threads(key);
+	_reschedule_noyield(key);
 }
 
 #ifdef CONFIG_USERSPACE

diff --git a/kernel/thread_abort.c b/kernel/thread_abort.c
index be6485e..b0b9f31 100644
--- a/kernel/thread_abort.c
+++ b/kernel/thread_abort.c

@@ -46,7 +46,7 @@
 		}
 
 		/* The abort handler might have altered the ready queue. */
-		_reschedule_threads(key);
+		_reschedule_noyield(key);
 	}
 }
 #endif

diff --git a/kernel/timer.c b/kernel/timer.c
index e4e5526..cab55a3 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c

@@ -173,7 +173,7 @@
 	if (_is_in_isr()) {
 		irq_unlock(key);
 	} else {
-		_reschedule_threads(key);
+		_reschedule_noyield(key);
 	}
 }