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>
Similar to the suspend refactoring earlier, this really nees to be
done in an atomic block. There were two confirmable races here,
though it's not completely clear either was being hit in practice:
1. The bit operations in z_mark_thread_as_started() aren't atomic so
it needs to be protected.
2. The intermediate state in z_ready_thread() could result in a dead
or suspended thread being added to the ready queue if another
context tried a simultaneous abort or suspend.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
Kernel wait_q's and the thread pended_on backpointer are scheduler
state and need to be modified under the scheduler lock. There was one
spot in pend() where they were not.
Also unpack z_remove_thread_from_ready_q() into an unsynchronized
utility so that it can be called by this process in a single lock
block.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
Under SMP, when a thread is marked aborting, this thread may still
be running on another CPU. However, if there is only one thread
available to run, this thread may be selected to run again due to
next_up() not checking for the aborting state. Moreover, when
there is no IPI to signal to others k_thread_abort() being called,
the k_thread_abort() target thread is marked dead after a new
thread is selected to run. This causes the original thread calling
k_thread_abort() to mistaken that target thread is no longer
running and returns.
Note that, with working IPI, z_sched_ipi() is called as an ISR
to mark the target thread dead. A new thread is then selected to
run, so that the target thread would not be selected due to it
being dead.
This moves the code to mark thread dead into next_up(), where
the next best thread is selected, and the current thread being
swapped out. z_sched_ipi() now becomes an empty function, and
calls to it are removed.
Signed-off-by: Daniel Leung <daniel.leung@intel.com>
On SMP, there is an inherent race when swapping: the old thread adds
itself back to the run queue before calling into the arch layer to do
the context switch. The former is properly synchronized under the
scheduler lock, and the later operates with interrupts locally
disabled. But until somewhere in the middle of arch_switch(), the old
thread (that is in the run queue!) does not have complete saved state
that can be restored.
So it's possible for another CPU to grab a thread before it is saved
and try to restore its unsaved register contents (which are garbage --
typically whatever state it had at the last interrupt).
Fix this by leveraging the "swapped_from" pointer already passed to
arch_switch() as a synchronization primitive. When the switch
implementation writes the new handle value, we know the switch is
complete. Then we can wait for that in z_swap() and at interrupt
exit.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
It's possible for a thread to abort itself simultaneously with an
external abort from another thread. In fact in our test suite this is
a common thing, as ztest will abort its own spawend threads at the end
of a test, as they tend to be exiting on their own.
When that happens, the thread marks itself DEAD and does all its
scheduler bookeeping, but it is STILL RUNNING on its own stack until
it makes its way to its final swap. The external context would see
that "dead" metadata and return from k_thread_abort(), allowing the
next test to reuse and spawn the same thread struct while the old
context was still running. Obviously that's bad.
Unfortunately, this is impossible to address completely without
modifying every SMP architecture to add a API-visible hook to every
swap that signals completion. In practice the best we can do is add a
delay. But note the optimization: almost always, the scheduler IPI
catches the running thread and kills it from interrupt context
(i.e. on a different stack). When that happens, we know that the
interrupted thread will never be resumed (because it's dead) and can
elide the delay. We only pay the cost when we actually detect a race.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
These two spots were calling z_sched_ipi() (the IPI handler run under
the ISR, which is a noop here because obviously the current thread
isn't DEAD) and not arch_sched_ipi() (which triggers an IPI on other
CPUs to inform them of scheduling state changes), presumably because
of a typo.
Apparently we don't have tests for k_wakeup() and
k_thread_priority_set() that are sensitive to latency in SMP
contexts...
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
Improve positioning of tracing calls. Avoid multiple calls and missing
events because of complex logix. Trace the event where things happen
really.
Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Historically, these routines were placed in thread.c and would use the
scheduler via exported, synchronized functions (e.g. "remove from
ready queue"). But those steps were very fine grained, and there were
races where the thread could be seen by other contexts (in particular
under SMP) in an intermediate state. It's not completely clear to me
that any of these were fatal bugs, but it's very hard to prove they
weren't.
At best, this is fragile. Move the z_thread_single_suspend/abort()
functions into the scheduler and do the scheduler logic in a single
critical section.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
We have been using thread, th and t for thread variables making the code
less readable, especially when we use t for timeouts and other time
related variables. Just use thread where possible and keep things
consistent.
Signed-off-by: Anas Nashif <anas.nashif@intel.com>
SPIN_VALIDATE is, as it was previously, enabled per default when having
less than 4 CPUs and either having no flash or a flash size greater than
32kB.
Small targets, which needs to have asserts enabled, can chose to have
the spinlock validation enabled or not and thereby decide whether the
overhead added is acceptable or not.
Signed-off-by: Danny Oerndrup <daor@demant.com>
Fix a gap where k_sleep(K_FOREVER) could execute a code path that
would not verify that the call was not from interrupt context.
Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
When a MetaIRQ preempts a cooperative thread, that thread would be
added back to the generic run queue. When the MetaIRQ is done, the
highest priority thread will be selected to run, which may obviously
be a cooperative thread of a higher priority than the one that was
preempted.
But that's wrong, because the original thread was promised that it
would NOT be preempted until it reached a scheduling point on its own
(that's the whole point of a cooperative thread, of course).
We need to track the thread that got preempted (one per CPU) and
return to it instead of whatever else the scheduler might have found.
Fixes#20255
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
Add assert when negative (except K_FOREVER) is passed as timeout.
Add negative timeout correction to 0.
Signed-off-by: Krzysztof Chruscinski <krzysztof.chruscinski@nordicsemi.no>
Flavio Ceolin
Krzysztof Chruscinski
Daniel Leung
Andrew Boie
Anas Nashif
Andy Ross
Watson Zeng
Enjia Mai
Kumar Gala
Oleg Zhurakivskyy
Carles Cufi
Danny Oerndrup
Peter Bigot