It was possible with pathological timing (see below) for the scheduler
to pick a cycle of threads on each CPU and enter the context switch
path on all of them simultaneously.
Example:
* CPU0 is idle, CPU1 is running thread A
* CPU1 makes high priority thread B runnable
* CPU1 reaches a schedule point (or returns from an interrupt) and
decides to run thread B instead
* CPU0 simultaneously takes its IPI and returns, selecting thread A
Now both CPUs enter wait_for_switch() to spin, waiting for the context
switch code on the other thread to finish and mark the thread
runnable. So we have a deadlock, each CPU is spinning waiting for the
other!
Actually, in practice this seems not to happen on existing hardware
platforms, it's only exercisable in emulation. The reason is that the
hardware IPI time is much faster than the software paths required to
reach a schedule point or interrupt exit, so CPU1 always selects the
newly scheduled thread and no deadlock appears. I tried for a bit to
make this happen with a cycle of three threads, but it's complicated
to get right and I still couldn't get the timing to hit correctly. In
qemu, though, the IPI is implemented as a Unix signal sent to the
thread running the other CPU, which is far slower and opens the window
to see this happen.
The solution is simple enough: don't store the _current thread in the
run queue until we are on the tail end of the context switch path,
after wait_for_switch() and going to reach the end in guaranteed time.
Note that this requires changing a little logic to handle the yield
case: because we can no longer rely on _current's position in the run
queue to suppress it, we need to do the priority comparison directly
based on the existing "swap_ok" flag (which has always meant
"yielded", and maybe should be renamed).
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
The QUEUED state flag was managed separately from the run queue
insertion/deletion, and the logic (while AFAICT perfectly correct) was
tangled in a few places trying to keep them in sync. Put the
management of both behind a queue_thread()/dequeue_thread() API for
clarity. The ALWAYS_INLINE usage seems to be working to get the
compiler to condense the resulting multiple assignments. No behavior
change.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
The "null out the switch handle and put it back" code in the swap
implementation is a holdover from some defensive coding (not wanting
to break the case where we picked our current thread), but it hides a
subtle SMP race: when that field goes NULL, another CPU that may have
selected that thread (which is to say, our current thread) as its next
to run will be spinning on that to detect when the field goes
non-NULL. So it will get the signal to move on when we revert the
value, when clearly we are still running on the stack!
In practice this was found on x86 which poisons the switch context
such that it crashes instantly.
Instead, be firm about state and always set the switch handle of a
currently running thread to NULL immediately before it starts running:
right before entering arch_switch() and symmetrically on the interrupt
exit path.
Fixes#28105
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
Some legacy spots in our IPC layer (legally) pass a NULL wait queue to
pend(). Allow this in the coherence assertion.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
The two calls to unpend a thread from a wait queue were inexplicably*
unsynchronized, as James Harris discovered. Rework them to call the
lowest level primities so we can wrap the process inside the scheduler
lock.
Fixes#32136
* I took a brief look. What seems to have happened here is that these
were originally synchronized via an implicit from an outer caller
(remember the original Uniprocessor irq_lock() API is a recursive
lock), and they were mostly implemented in terms of middle-level
calls that were themselves locked. So those got ported over to the
newer spinlock but the outer wrapper layer got forgotten.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
1. Exclude the CODE UNREACHABLE line while generating coverage report.
2. Exclude the memory domain deprecated API when calculating code
coverage.
Signed-off-by: Enjia Mai <enjiax.mai@intel.com>
Needing to check the current cycle time (which involves a spinlock and
register read on most architectures) is wasteful in the scheduler
priority predicate, which is a hot path. If we "burn" one bit of
precision (and document the rule), we can do the comparison without
knowing the current time.
2^31 cycles is still far longer than a live deadline thread in any
legitimate realtime app should ever live before being scheduled.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
Ticks should be assigned directly to timeout value in case of
CONFIG_LEGACY_TIMEOUT_API=y, just as they were before referenced patch.
Fixes: 7a815d5d99 ("kernel: sched: Use k_ticks_t in z_tick_sleep")
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
z_tick_sleep was using int32_t what could cause a possible overflow
when converting from k_ticks_t.
Signed-off-by: Flavio Ceolin <flavio.ceolin@intel.com>
Most of kernel files where declaring os module without providing
log level. Because of that default log level was used instead of
CONFIG_KERNEL_LOG_LEVEL.
Signed-off-by: Krzysztof Chruscinski <krzysztof.chruscinski@nordicsemi.no>
Since the tracing of thread being switched in/out has the same
instrumentation points, we can roll the tracing function calls
into the one for thread stats gathering functions.
This avoids duplicating code to call another function.
Signed-off-by: Daniel Leung <daniel.leung@intel.com>
For compatibility layers like CMSIS where thread objects
are drawn from a pool, provide a context pointer to the
exited thread object so it may be freed.
This is somewhat obscure and has no supporting APIs or
overview documentation and should be considered a private
kernel feature. Applications should really be using
k_thread_join() instead.
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
Zephyr SMP kernels need to be able to run on architectures with
incoherent caches. Naive implementation of synchronization on such
architectures requires extensive cache flushing (e.g. flush+invalidate
everything on every spin lock operation, flush on every unlock!) and
is a performance problem.
Instead, many of these systems will have access to separate "coherent"
(usually uncached) and "incoherent" regions of memory. Where this is
available, place all writable data sections by default into the
coherent region. An "__incoherent" attribute flag is defined for data
regions that are known to be CPU-local and which should use the cache.
By default, this is used for stack memory.
Stack memory will be incoherent by default, as by definition it is
local to its current thread. This requires special cache management
on context switch, so an arch API has been added for that.
Also, when enabled, add assertions to strategic places to ensure that
shared kernel data is indeed coherent. We check thread objects, the
_kernel struct, waitq's, timeouts and spinlocks. In practice almost
all kernel synchronization is built on top of these structures, and
any shared data structs will contain at least one of them.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
Signed-off-by: Anas Nashif <anas.nashif@intel.com>
When threads exited we were leaving dangling references to
them in the domain's mem_domain_q.
z_thread_single_abort() now calls into the memory domain
code via z_mem_domain_exit_thread() to take it off.
The thread setup code now invokes z_mem_domain_init_thread(),
avoiding extra checks in k_mem_domain_add_thread(), we know
the object isn't currently a member of a doamin.
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
k_thread_create() works as expected on both uninitialized memory,
or threads that have completely exited.
However, horrible and difficult to comprehend things can happen if a
thread object is already being used by the kernel and
k_thread_create() is called on it.
Historically this has been a problem with test cases trying to be
parsimonious with thread objects and not properly cleaning up
after themselves. Add an assertion for this which should catch
both the illegal creation of a thread already active, or threads
racing to create the same thread object.
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
Fixes races where threads on another CPU are joining the
exiting thread, since it could still be running when
the joiners wake up on a different CPU.
Fixes problems where the thread object is still being
used by the kernel when the fn_abort() function is called,
preventing the thread object from being recycled or
freed back to a slab pool.
Fixes a race where a thread is aborted from one CPU while
it self-aborts on another CPU, that was currently worked
around with a busy-wait.
Precedent for doing this comes from FreeRTOS, which also
performs final thread cleanup in the idle thread.
Some logic in z_thread_single_abort() rearranged such that
when we release sched_spinlock, the thread object pointer
is never dereferenced by the kernel again; join waiters
or fn_abort() logic may free it immediately.
An assertion added to z_thread_single_abort() to ensure
it never gets called with thread == _current outside of an ISR.
Some logic has been added to ensure z_thread_single_abort()
tasks don't run more than once.
Fixes: #26486
Related to: #23063#23062
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
sys_trace_thread_abort and z_thread_monitor_exit in
z_thread_single_abort also need to be protected by
sched_spinlock, otherwise when after the spinlock
release, if there is an pending interrupt, it will cause an
sched in the interrrupt exit, and those trace and monitor
function will never reach.
Signed-off-by: Watson Zeng <zhiwei@synopsys.com>
We are checking thread->base members like thread_state and prio
and making decisions based on it, hold the sched_spinlock to
avoid potential concurrency problems if these members are modified
on another CPU or nested interrupt.
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
We need to check if a thread is runnable at all before we
contemplate putting it on the end of the priority queue,
it might not be on the queue at all if it was suspended.
Replaces the less comprehensive check to see if the thread
was pending a timeout.
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
We are checking and modifying members of thread->base
(in particular it's waitq and thread_state) which are
nominally protected by sched_spinlock. Hold it while
doing this to avoid concurrent changes on another CPU
or ISR preeemption.
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
* Move switched_in into the arch context switch assembly code,
which will correctly record the switched_in information.
* Add switched_in/switched_out for context switch in irq exit.
Signed-off-by: Watson Zeng <zhiwei@synopsys.com>
The same code was being copypasted in k_thread_abort()
implementations, just move into z_thread_single_abort().
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
Check if next ready thread is same as current thread before calling
z_swap.
This avoids calling swap to just go back to the original thread.
Original code: thread 0x20000118 switches out and then in again...
>> 0x20000118 gives semaphore(signal): 0x20000104 (count: 0)
>> thread ready: 0x20000118
>> 0x20000118 switched out
>> 0x20000118 switched in
>> end call to k_sem_give
>> 0x20000118 takes semaphore(wait): 0x200000f4 (count: 0)
>> thread pend: 0x20000118
>> 0x20000118 switched out
>> 0x200001d0 switched in
with this patch:
>> 0x200001d0 gives semaphore(signal): 0x200000f4 (count: 0)
>> thread ready: 0x200001d0
>> end call to k_sem_give
>> 0x200001d0 takes semaphore(wait): 0x20000104 (count: 0)
>> thread pend: 0x200001d0
>> 0x200001d0 switched out
>> 0x20000118 switched in
>> end call to k_sem_take
The above is output from tracing with a custom format used for
debugging.
Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Add one another test case for testing both arch_curr_cpu() and
arch_sched_ipi() architecture layer interface.
Signed-off-by: Enjia Mai <enjiax.mai@intel.com>
If k_thread_join() was passed with an actual timeout value,
and not K_FOREVER, the blocking thread was not being properly
woken up when the target thread exits. The timeout itself
was never aborted, causing the joining thread to remain
un-scheduled until the timeout expires.
Amend the k_thread_join() test cases to check that the join
completed before the provided timeout period expired.
Fixes: #24744
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
Add a CONFIG_TIMEOUT_64BIT kconfig that, when selected, makes the
k_ticks_t used in timeout computations pervasively 64 bit. This will
allow much longer timeouts and much faster (i.e. more precise) tick
rates. It also enables the use of absolute (not delta) timeouts in an
upcoming commit.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
Add a k_timeout_t type, and use it everywhere that kernel API
functions were accepting a millisecond timeout argument. Instead of
forcing milliseconds everywhere (which are often not integrally
representable as system ticks), do the conversion to ticks at the
point where the timeout is created. This avoids an extra unit
conversion in some application code, and allows us to express the
timeout in units other than milliseconds to achieve greater precision.
The existing K_MSEC() et. al. macros now return initializers for a
k_timeout_t.
The K_NO_WAIT and K_FOREVER constants have now become k_timeout_t
values, which means they cannot be operated on as integers.
Applications which have their own APIs that need to inspect these
vs. user-provided timeouts can now use a K_TIMEOUT_EQ() predicate to
test for equality.
Timer drivers, which receive an integer tick count in ther
z_clock_set_timeout() functions, now use the integer-valued
K_TICKS_FOREVER constant instead of K_FOREVER.
For the initial release, to preserve source compatibility, a
CONFIG_LEGACY_TIMEOUT_API kconfig is provided. When true, the
k_timeout_t will remain a compatible 32 bit value that will work with
any legacy Zephyr application.
Some subsystems present timeout (or timeout-like) values to their own
users as APIs that would re-use the kernel's own constants and
conventions. These will require some minor design work to adapt to
the new scheme (in most cases just using k_timeout_t directly in their
own API), and they have not been changed in this patch, instead
selecting CONFIG_LEGACY_TIMEOUT_API via kconfig. These subsystems
include: CAN Bus, the Microbit display driver, I2S, LoRa modem
drivers, the UART Async API, Video hardware drivers, the console
subsystem, and the network buffer abstraction.
k_sleep() now takes a k_timeout_t argument, with a k_msleep() variant
provided that works identically to the original API.
Most of the changes here are just type/configuration management and
documentation, but there are logic changes in mempool, where a loop
that used a timeout numerically has been reworked using a new
z_timeout_end_calc() predicate. Also in queue.c, a (when POLL was
enabled) a similar loop was needlessly used to try to retry the
k_poll() call after a spurious failure. But k_poll() does not fail
spuriously, so the loop was removed.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
Replace all occurences of BUILD_ASSERT_MSG() with BUILD_ASSERT()
as a result of merging BUILD_ASSERT() and BUILD_ASSERT_MSG().
Signed-off-by: Oleg Zhurakivskyy <oleg.zhurakivskyy@intel.com>
Don't generate a Z_OOPS() if k_thread_abort() is called on a
thread that isn't running. Just return to the caller instead,
much like how k_thread_join() functions.
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
This reverts commit 8739517107.
Pull Request #23437 was merged by mistake with an invalid manifest.
Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>
Replace all occurences of BUILD_ASSERT_MSG() with BUILD_ASSERT()
as a result of merging BUILD_ASSERT() and BUILD_ASSERT_MSG().
Signed-off-by: Oleg Zhurakivskyy <oleg.zhurakivskyy@intel.com>
Private type, internal to the kernel, not directly associated
with any k_object_* APIs. Is the return value of z_object_find().
Rename to struct z_object.
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
This logic should be using the sched_lock and not its own
separate lock for these two functions.
Some simplications were made; z_thread_single_resume and
z_thread_single_suspend were only used in one place, and there was
some redundant logic for whether to reschedule in the suspend case.
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
This assertion, if built in, allows users threads to crash
the kernel in a critical section by passing a negative timeout
value, creating a DoS attack vector.
Remove this assertion, immediately below it there's a check
which just resets it to 0 anyway.
Fixes: #22999
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
Use of the _current_cpu pointer cannot be done safely in a preemptible
context. If a thread is preempted and migrates to another CPU, the
old CPU record will be wrong.
Add a validation assert to the expression that catches incorrect
usages, and fix up the spots where it was wrong (most important being
a few uses of _current outside of locks, and the arch_is_in_isr()
implementation).
Note that the resulting _current expression now requires locking and
is going to be somewhat slower. Longer term it's going to be better
to augment the arch API to allow SMP architectures to implement a
faster "get current thread pointer" action than this default.
Note also that this change means that "_current" is no longer
expressible as an lvalue (long ago, it was just a static variable), so
the places where it gets assigned now assign to _current_cpu->current
instead.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
These got dropped by an earlier patch, but are required on SMP systems
so synchronously notify other CPUs of changed scheduler state.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
This got clobbered by commit adac4cbafa in what I think was a rebase
mistake. Without it, on SMP systems it's possible to select a new
_current thread and try to return into it before another CPU has
actually finished switching away from it.
Interestingly: the frequency with which this bug got caught once it
was reintroduced was much, much higher than it was when it was fixed
the first time due to the instruction pointer poisoning introduced in
the interrim. Incompletely saved threads now have deliberately broken
state when assertions are enabled and will panic synchronously.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
