The current implementation does not return the low 32 bits of
k_uptime_get() as suggested by it's documentation; it returns the number
of milliseconds represented by the low 32-bits of the underlying system
clock. The truncation before translation results in discontinuities at
every point where the system clock increments bit 33.
Reimplement it using the full-precision value, and update the
documentation to note that this variant has little value for
long-running applications.
Closes#18739.
Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
The `next_timeout()` function used to call the `elapsed()` function
directly in the `MAX` macro call. This caused the `elapsed()` function
to be executed twice, with possible different results, if the system
clock incremented its value in a meantime.
As a result, the whole `MAX(0, to->dticks - elapsed()` expresion could
return an incorrect value of -1, which represents the K_FOREVER timeout.
This led to a stall in devices running tickless kernel (as observed on
nRF52840).
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
This is an oddball API. It's untested. In fact testing its proper
behavior requires very elaborate automation (you need a device outside
the Zephyr hardware to measure real world time, and a mechanism for
getting the device into and out of idle without using the timer
driver). And this makes for needless difficulty managing code
coverage metrics.
It was always just a hint anyway. Mark the old API deprecated and
replace it with a kconfig tunable. The effect of that is just to
change the timeout value passed to the timer driver, where we can
manage code coverage metrics more easily (only one driver cares to
actually support this feature anyway).
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
Move internal and architecture specific headers from include/drivers to
subfolder for timer:
include/drivers/timer
Signed-off-by: Anas Nashif <anas.nashif@intel.com>
If the system sets its clock frequency at runtime, this is
stored in a variable that can't be directly read by user
mode. For this case only, add a system call to fetch its
value and modify the definition of
sys_clock_hw_cycles_per_sec() to use it.
Since this is now a system call, store in a temporary variable
inside z_ms_to_ticks(). The syscall overhead only applies
when called from user mode, other contexts are completely
inlined.
Added stub syscall header for mocking framework, to get rid
of inclusion errors.
Fixes: #16238
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
Controlling expression of if and iteration statements must have a
boolean type.
MISRA-C rule 14.4
Signed-off-by: Flavio Ceolin <flavio.ceolin@intel.com>
Update reserved function names starting with one underscore, replacing
them as follows:
'_k_' with 'z_'
'_K_' with 'Z_'
'_handler_' with 'z_handl_'
'_Cstart' with 'z_cstart'
'_Swap' with 'z_swap'
This renaming is done on both global and those static function names
in kernel/include and include/. Other static function names in kernel/
are renamed by removing the leading underscore. Other function names
not starting with any prefix listed above are renamed starting with
a 'z_' or 'Z_' prefix.
Function names starting with two or three leading underscores are not
automatcally renamed since these names will collide with the variants
with two or three leading underscores.
Various generator scripts have also been updated as well as perf,
linker and usb files. These are
drivers/serial/uart_handlers.c
include/linker/kobject-text.ld
kernel/include/syscall_handler.h
scripts/gen_kobject_list.py
scripts/gen_syscall_header.py
Signed-off-by: Patrik Flykt <patrik.flykt@intel.com>
In some circumstances (e.g., a tickless kernel), k_timer_remaining_get()
would not account for time passed that didn't involve clock interrupts.
This adds a simple fix for that, and adds a test case. In addition, the
return value of k_timer_remaining_get() is clamped at 0 in the case of
overdue timers and the API description is adjusted to reflect this.
Fixes: #13353
Signed-off-by: Charles E. Youse <charles.youse@intel.com>
There are issues using lowercase min and max macros when compiling a C++
application with a third-party toolchain such as GNU ARM Embedded when
using some STL headers i.e. <chrono>.
This is because there are actual C++ functions called min and max
defined in some of the STL headers and these macros interfere with them.
By changing the macros to UPPERCASE, which is consistent with almost all
other pre-processor macros this naming conflict is avoided.
All files that use these macros have been updated.
Signed-off-by: Carlos Stuart <carlosstuart1970@gmail.com>
The sys_dlist_insert_*() functions had a behavior where a NULL
argument for the insertion position to sys_dlist_insert_after/before()
was interpreted as "the end of the list". We never used that
convention (except in one spot internal to dlist.h which was not
itself used anywhere), and of course already have an API for appending
and prepending to a list.
In practice this was a performance disaster. The NULL check is
virtually never provable statically by the compiler, so that test and
branch is present always. And worse, the check and call to another
function was pushing this beyond the complexity limit for gcc to
inline a function (at -Os optimization anyway), forcing us to use
function calls for what should be a ~8 instruction sequence. The
upshot is that dlist insertions were 2-3x slower than they needed to
be.
Deprecate these older APIs and introduce a new sys_dlist_insert() call
which can be much better optimized.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
The fix in commit e664c78b82 ("kernel/timeout: Fix recursive
spinlock in z_set_timeout_expiry()") missed a spot that had also been
introduced with recent locking work. The new
_get_next_timeout_expiry() implementation takes its own lock, which is
recursive when called from z_clock_announce(). Fix by calling the
wrapped implementation instead.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
Whether a timeout is linked into the timeout queue can be determined
from the corresponding sys_dnode_t linked state. This removes the need
to use a special flag value in dticks to determine that the timeout is
inactive.
Update _abort_timeout to return an error code, rather than the flag
value, when the timeout to be aborted was not active.
Remove the _INACTIVE flag value, and replace its external uses with an
internal API function that checks whether a timeout is inactive.
Signed-off-by: Peter A. Bigot <pab@pabigot.com>
The z_set_timeout_expiry() function was added in part to simply the
locking strategy, but it missed a case where a function it was calling
was re-locking the same spinlock. It "works"[1] in uniprocessor
environments, but can be a deadlock in SMP.
Fix this by moving the meat of the function to an unlocked utility,
use that locally, and turn the entry point into one that does locking.
Actually this only gets called from idle now, which is a use case that
will go away when TICKLESS_IDLE is removed as a separate feature (once
you know all timeouts are set tickless, you don't need to set it from
the idle entry at all).
Discovered via lucky inspection.
[1] It doesn't work. It releases the lock prematurely at the end of
the inner block. But in practice this wasn't discovered.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
The logic in z_set_timeout_expiry() missed the case where the ticks
argument could be zero (or lower), which can happen naturally due to
timing/interrupt slop. In those circumstances, it would still try to
reset a timer that was "about to expire at the next tick", which would
run afoul of the drivers' internal decisions about how soon a timer
interrupt could be set, and then get pushed out to the next tick.
Explicitly detect this as an "imminent" predicate to make the logic
clearer.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
The use of dticks == INACTIVE to tell whether or not a timeout was
already in the list was insufficient. There is a time period between
the moment a timeout is removed from the list and the end of its
handler where it is not in the list, yet its list node pointers still
point into it. Doing things like aborting a thread while that is true
(which can be asynchronous too!) would corrupt the list even though
all the operations on it were "atomic".
Set the timeout node pointers to nulls atomically when removed, and
check for double-remove conditions (which, again, might be perfectly
OK).
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
This loop was structured badly, as a while(true) with multiple "exit
if" cases in the body. It was bad enough that I genuinely fooled
myself into rewriting it, having convinced myself there was a bug in
it when there wasn't.
So keep the rewritten loop which expresses the iteration in a more
invariant way (i.e. "while we have an element to expire" and not "test
if we have to exit the loop"). Shorter and easier. Also makes the
locking clearer as we can simply release the lock around the callback
in a natural/obvious way.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
MISRA-C says all declarations of an object or function must use the
same name and qualifiers.
MISRA-C rule 8.3
Signed-off-by: Flavio Ceolin <flavio.ceolin@intel.com>
System must not set the clock expiry via backdoor as it may
effect in unbound time drift of all scheduled timeouts.
Fixes: #11502
Signed-off-by: Pawel Dunaj <pawel.dunaj@nordicsemi.no>
The call to z_clock_set_timeout() was being made outside the timeout
lock, which can race against other contexts setting sooner-expiring
timeouts.
Also add a long comment to one spot (timeslicing) where this call is
made outside the timeout spinlock (inside the scheduler lock) and why
this is OK.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
Duh: "remove()" is a POSIX symbol, and on at least some platforms
stdio.h can be included here out of platform headers causing a name
collision.
Fixes #10669's direct issue, though the broader issue of how to choose
names for statics remains controversial.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
Our funny convention holds that passing ticks==1 to _add_timeout()
means "at the next tick". But that means that 1, 0, and all negative
numbers are expected to behave the same. In ticked mode, that's fine
because it will, after all, expire at the next tick.
But in tickless, the next announcement may be for several ticks, and
that zero will appear to expire "before" the next tick in the
consumption loop.
Make sure all "next tick" expirations look the same.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
When fetching the next timeout to expire, the value is relative to the
last announced tick, so you subtract the timer-provided elapsed time
to get the true delta from "now". When adding a new timeout, you
*have* a value relative to now, so you compute the delta vs. the last
announced tick by adding the elapsed() time. Duh.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
This was wrong in subtle ways. In tickless mode it's possible to get
an announcement for multiple ticks at a time and have multiple
callbacks to execute that were technically scheduled at different
times. We want to fix the current tick at the value represented by
the currently-executing callback's EXPIRATION (even if we missed it!),
so that any new timeouts it sets (c.f. a k_timer period) happen at the
right point, in phase with the expected series. In single-tick mode
the code ends up the same always, so the bug wasn't visible.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
The previous comment correctly and carefully explained why the 64 bit
value in curr_tick doesn't require locking when reading only the low
32 bits.
It completely missed the fact that the calculation of elapsed time and
the read of curr_tick ABSOLUTELY DO require locking, because the
former is expressed in terms of the latter. This was always bug, even
in the old code, but never witnessed because we ran so little software
in tickless mode.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
Now that the API has been fixed up, replace the existing timeout queue
with a much smaller version. The basic algorithm is unchanged:
timeouts are stored in a sorted dlist with each node nolding a delta
time from the previous node in the list; the announce call just walks
this list pulling off the heads as needed. Advantages:
* Properly spinlocked and SMP-aware. The earlier timer implementation
relied on only CPU 0 doing timeout work, and on an irq_lock() being
taken before entry (something that was violated in a few spots).
Now any CPU can wake up for an event (or all of them) and everything
works correctly.
* The *_thread_timeout() API is now expressible as a clean wrapping
(just one liners) around the lower-level interface based on function
pointer callbacks. As a result the timeout objects no longer need
to store backpointers to the thread and wait_q and have shrunk by
33%.
* MUCH smaller, to the tune of hundreds of lines of code removed.
* Future proof, in that all operations on the queue are now fronted by
just two entry points (_add_timeout() and z_clock_announce()) which
can easily be augmented with fancier data structures.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
Peter Bigot