These functions, for good design reason, take a locking key to
atomically release along with the context swtich. But there's still a
common pattern in code to do a switch unconditionally by passing
irq_lock() directly. On SMP that's a little hurtful as it spams the
global lock. Provide an _unlocked() variant for
_Swap/_reschedule/_pend_curr for simplicity and efficiency.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
Just like with _Swap(), we need two variants of these utilities which
can atomically release a lock and context switch. The naming shifts
(for byte count reasons) to _reschedule/_pend_curr, and both have an
_irqlock variant which takes the traditional locking.
Just refactoring. No logic changes.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
We want a _Swap() variant that can atomically release/restore a
spinlock state in addition to the legacy irqlock. The function as it
was is now named "_Swap_irqlock()", while _Swap() now refers to a
spinlock and takes two arguments. The former will be going away once
existing users (not that many! Swap() is an internal API, and the
long port away from legacy irqlocking is going to be happening mostly
in drivers) are ported to spinlocks.
Obviously on uniprocessor setups, these produce identical code. But
SMP requires that the correct API be used to maintain the global lock.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
* Newlib now defines a special z_newlib_partition containing
all globals relevant to newlib. Most of these are in libc.a
with a heap tracking variable in newlib's hooks.
* Both C libraries now expose a k_mem_partition containing the
bounds of the malloc heap arena. Threads that want to use
libc malloc() will need to add this to their memory domain.
* z_newlib_get_heap_bounds has been removed, in favor of the
memory partition for the heap arena
* ztest now includes the C library partitions in its memory
domain.
* The mem_alloc test now runs in user mode to prove that this
all works for both C libraries.
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
GCC 6.2.0 is making frustratingly poor inlining decisions with some of
these routines, resulting in an awful lot of runtime calls for code
that is only ever expanded once or twice within the file.
Treat with targetted ALWAYS_INLINE's to force the issue. The
scheduler code is a hot path.
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 call to _arch_switch is a giant screaming sign inviting optimizer
bugs. The code that appears before is what happened long ago when we
were switched out, but the version that EXECUTED just now is actually
in a different thread. So the assignment to _current before the
switch actually assigned OUR thread (the "new_thread" of the old
context!) to _current.
But obviously the optimizer looks at that code and assumes that the
_current which got assigned to the thread we were switching to long
ago is still correct, and used it when retrieving the swap return
value.
Obviously the real bug here is that the _arch_switch() in question
lacked a memory clobber (and it's getting one).
But we can remove two lines, remove code from inside the interrupt
lock and make the implementation more robust by moving the read to
after the irq_unlock() (which generally also has a memory clobber).
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
In general driver system calls are implemented at a subsystem
layer. However, some drivers may have capabilities specific to
the hardware not covered by the subsystem API. Such drivers may
want to define their own system calls.
This macro makes it simple to validate in the driver-specific
system call handlers that not only does the untrusted device
pointer correspond to the expected subsystem, initialization
state, and caller permissions, but also that the device object
is an instance of a specific driver (and not just any driver in
that subsystem).
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
The function that checks if an object is valid is essentially a boolean
function. Just changing its return type to reflect it.
MISRA-C rule 14.4
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>
If we just had the kernel's implementation, we could
just move this to lib/, but possible arch-specific
implementations dictate that we just make this a
syscall.
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
There was an struct and a variable called _kernel. This is error prone
and a MISRA-C violation. It is changing the struct to have a unique
identifier.
MISRA-C rule 5.8
Signed-off-by: Flavio Ceolin <flavio.ceolin@intel.com>
There is a struct and a macro called _ready_q, this is error
prone. Just removing it.
MISRA-C rule 5.4
Signed-off-by: Flavio Ceolin <flavio.ceolin@intel.com>
In _pend_current_thread the argument key is always a unsigned
interger type and this function forces it to become a signed
interger. This is a dangerous behavior and cant be trusted to
work as expected.
Signed-off-by: Adithya Baglody <adithya.nagaraj.baglody@intel.com>
This API shouldn't take a int type but instead it should take
u32_t. This argument has to be similar to irq_lock() and
irq_unlock().
Signed-off-by: Adithya Baglody <adithya.nagaraj.baglody@intel.com>
I was pretty careful, but these snuck in. Most of them are due to
overbroad string replacements in comments. The pull request is very
large, and I'm too lazy to find exactly where to back-merge all of
these.
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>
_timeout_remaining_get() was a function on a struct _timeout, doing
iteration on the timeout list, but it was defined in timer.c (the
higher level abstraction).
Move it to where it belongs. Also have it return ticks instead of ms
to conform to scheme in the rest of the timeout API. And rename it to
a more standard zephyr name.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
Add the callback parameter to add_timeout(), and remove the thread
argument. Now the "low level" timeout API can be expressed without
reference to threads.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
Now that this is known to be an unused value, remove it from the API.
Note that this caught a few spots where we were passing values (a
non-NULL wait_q with a NULL thread handle) that were always being
ignored before.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
The timeout_q.h scheme, where it declared real functions, but the
stubs for when there was no clock were in wait_q.h was senselessly
weird. Put them in the same file.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
Instead of checking every time we hit the low-level context switch
path to see if the new thread has a "partner" with which it needs to
share time, just run the slice timer always and reset it from the
scheduler at the points where it has already decided a switch needs to
happen. In TICKLESS_KERNEL situations, we pay the cost of extra timer
interrupts at ~10Hz or whatever, which is low (note also that this
kind of regular wakeup architecture is required on SMP anyway so the
scheduler can "notice" threads scheduled by other CPUs). Advantages:
1. Much simpler logic. Significantly smaller code. No variance or
dependence on tickless modes or timer driver (beyond setting a
simple timeout).
2. No arch-specific assembly integration with _Swap() needed
3. Better performance on many workloads, as the accounting now happens
at most once per timer interrupt (~5 Hz) and true rescheduling and
not on every unrelated context switch and interrupt return.
4. It's SMP-safe. The previous scheme kept the slice ticks as a
global variable, which was an unnoticed bug.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
This got broken. Add some #ifery to handle the case. Not clean, will
clean up in a future pass once the API is final.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
There was no good reason to have these rather large functions in a
header. Put them into sys_clock.c for now, pending rework to the
system.
Now the API is clearly visible in a small header.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
The tickless driver had a bunch of "hairy" APIs which forced the timer
drivers to do needless low-level accounting for the benefit of the
kernel, all of which then proceeded to implement them via cut and
paste. Specifically the "program_time" calls forced the driver to
expose to the kernel exactly when the next interrupt was due and how
much time had elapsed, in a parallel API to the existing "what time is
it" and "announce a tick" interrupts that carry the same information.
Remove these from the kernel, replacing them with synthesized logic
written in terms of the simpler APIs.
In some cases there will be a performance impact due to the use of the
64 bit uptime call, but that will go away soon.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
The existing API had two almost identical functions: _set_time() and
_timer_idle_enter(). Both simply instruct the timer driver to set the
next timer interrupt expiration appropriately so that the call to
z_clock_announce() will be made at the requested number of ticks. On
most/all hardware, these should be implementable identically.
Unfortunately because they are specified differently, existing drivers
have implemented them in parallel.
Specify a new, unified, z_clock_set_timeout(). Document it clearly
for implementors. And provide a shim layer for legacy drivers that
will continue to use the old functions.
Note that this patch fixes an existing bug found by inspection: the
old call to _set_time() out of z_clock_announce() failed to test for
the "wait forever" case in the situation where clock_always_on is
true, meaning that a system that reached this point and then never set
another timeout would freeze its uptime clock incorrectly.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
The builtin function __builtin_umul_overflow returns a boolean and
should not checked as an integer.
Signed-off-by: Flavio Ceolin <flavio.ceolin@intel.com>
According C99 the first 31 characters of an identifier must be unique.
Shortening the namespace of the generated objects to achieve it.
C99 - 5.2.4.1
MISRA-C rule 5.1
Signed-off-by: Flavio Ceolin <flavio.ceolin@intel.com>
Change APIs that essentially return a boolean expression - 0 for
false and 1 for true - to return a bool.
MISRA-C rule 14.4
Signed-off-by: Flavio Ceolin <flavio.ceolin@intel.com>
Added k_thread_name_set() and enable thread name setting when declaring
static threads. This is enabled only when THREAD_MONITOR is used. System
threads get a name by default.
Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Add ifdef guard to the z_reset_timeslice() to fix compilation
errors when CONFIG_TIMESLICING is disabled.
Signed-off-by: Daniel Leung <daniel.leung@intel.com>
MISRA C requires that every controlling expression of and if or while
statement have a boolean type.
Signed-off-by: Flavio Ceolin <flavio.ceolin@intel.com>
Rename _DEVICE_STRUCT_SIZE to _DEVICE_STRUCT_SIZEOF. This causes it to
be picked by the script 'gen_offset_header.py' and inserted into the
header file 'include/generated/offsets.h'.
Renaming from x_SIZE to x_SIZEOF will align it's name with the other
symbols that denote a sctruct's size, like K_THREAD_SIZEOF.
Furthermore, it will allow the symbol to be accessed through a header
file define, instead of only as an extern symbol. This is more
flexible, and more aligned with the other symbols in offsets.
Finally, if we are able to move all of offsets.c symbols into the
offsets.h header file we be able to remove offsets.o from the link and
thereby simplify the linking process.
Signed-off-by: Sebastian Bøe <sebastian.boe@nordicsemi.no>
Any word started with underscore followed by and uppercase letter or a
second underscore is a reserved word according with C99.
With have *many* violations on Zephyr's code, this commit is tackling
only the violations caused by headers guards. It also takes the
opportunity to normalize them using the filename in uppercase and
replacing dot with underscore. e.g file.h -> FILE_H
Signed-off-by: Flavio Ceolin <flavio.ceolin@intel.com>
__swap function was returning -EAGAIN in some case, though its return
value was declared as unsigned int.
This commit changes this function to return int since it can return a
negative value and its return was already been propagate as int.
Signed-off-by: Flavio Ceolin <flavio.ceolin@intel.com>
When adding a new runnable thread in tickless mode, we need to detect
whether it will timeslice with the running thread and reset the timer,
otherwise it won't get any CPU time until the next interrupt fires at
some indeterminate time in the future.
This fixes the specific bug discussed in #7193, but the broader
problem of tickless and timeslicing interacting badly remains. The
code as it exists needs some rework to avoid all the #ifdef mess.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
When adding a new runnable thread in tickless mode, we need to detect
whether it will timeslice with the runnable thread and reset the
timer, otherwise it won't get any CPU time until the next interrupt
fires at some indeterminate time in the future.
This fixes the specific bug discussed in #7193, but the broader
problem of tickless and timeslicing interacting badly remains. The
code as it exists needs some rework to avoid all the #ifdef mess.
Note that the patch also moves _ready_thread() from a ksched.h inline
to sched.c.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
This fixes#8669, and is distressingly subtle for a one-line patch:
The list iteration code in _handle_expired_timeouts() would remove the
timeout from our (temporary -- the dlist header is on the stack of our
calling function) list of expired timeouts before invoking the
handler. But sys_dlist_remove() only fixes up the containing list
pointers, leaving garbage in the node. If the action of that handler
is to re-add the timeout (which is very common!) then that will then
try to remove it AGAIN from the same list.
Even then, the common case is that the expired list contains only one
item, so the result is a perfectly valid empty list that affects
nothing. But if you have more than one, you get a corrupt cycle in
the iteration list and things get weird.
As it happens, there's no value in trying to remove this timeout from
the temporary list at all. Just iterate over it naturally.
Really, this design is fragile: we shouldn't be reusing the list nodes
in struct _timeout for this purpose and should figure out some other
mechanism. But this fix should be good for now.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
Move to more generic tracing hooks that can be implemented in different
ways and do not interfere with the kernel.
Signed-off-by: Anas Nashif <anas.nashif@intel.com>
