Timeout API rework #23382
Merged
Timeout API rework #23382
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andyross
requested review from
aescolar,
albertofloyd,
alexanderwachter,
anangl,
andrewboie,
avisconti,
carlescufi,
carlocaione,
daor-oti,
dcpleung,
erwango,
finikorg,
franciscomunoz,
galak,
henrikbrixandersen,
ioannisg,
jakub-uC,
jfischer-no,
kgugala,
loicpoulain,
Mani-Sadhasivam,
MaureenHelm,
nashif,
nategraff-sifive,
nordic-krch,
pabigot,
pfalcon,
pgielda,
sathishkuttan and
scottwcpg
as code owners
|
DNM so we remember to remove the shippable hack. |
nordic-krch
approved these changes
Mar 30, 2020
carlescufi
added a commit
to carlescufi/mcuboot
that referenced
this pull request
Mar 30, 2020
k_sleep() requires explicit conversion of the unit since it takes an opaque type starting with: zephyrproject-rtos/zephyr#23382 Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>
andyross
force-pushed
the
timeout-take-2
branch
from
8916741 to
2883b2b
Compare
|
One more to clean up another crossed issue with the new cavs_timer driver. This also includes a build fix to an upstream bug with cc26x2r1_launchxl that is merely unmasked here. It's not a timer driver thing at all, but it's a simple and obvious patch. Note that we're still seeing a failure in mcuboot though. @nordic-krch , can you take a look at that? |
|
OK, we're good now. Just the one mcuboot failure left. |
carlescufi
added a commit
to mcu-tools/mcuboot
that referenced
this pull request
Mar 30, 2020
k_sleep() requires explicit conversion of the unit since it takes an opaque type starting with: zephyrproject-rtos/zephyr#23382 Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>
carlescufi
added a commit
to zephyrproject-rtos/mcuboot
that referenced
this pull request
Mar 30, 2020
k_sleep() requires explicit conversion of the unit since it takes an opaque type starting with: zephyrproject-rtos/zephyr#23382 Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>
|
Fyi the cc26x2r1_launchxl issue is being addressed in #23929. |
|
@andyross please rebase, the mcuboot build should be fixed now |
|
Remaining issue with mcuboot in PR: mcu-tools/mcuboot#700 |
added 8 commits
March 31, 2020 12:55
Kernel timeouts have always been a 32 bit integer despite the existence of generation macros, and existing code has been inconsistent about using them. Upcoming commits are going to make the timeout arguments opaque, so fix things up to be rigorously correct. Changes include: + Adding a K_TIMEOUT_EQ() macro for code that needs to compare timeout values for equality (e.g. with K_FOREVER or K_NO_WAIT). + Adding a k_msleep() synonym for k_sleep() which can continue to take integral arguments as k_sleep() moves away to timeout arguments. + Pervasively using the K_MSEC(), K_SECONDS(), et. al. macros to generate timeout arguments. + Removing the usage of K_NO_WAIT as the final argument to K_THREAD_DEFINE(). This is just a count of milliseconds and we need to use a zero. This patch include no logic changes and should not affect generated code at all. 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>
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 support for "absolute" timeouts, which are expressed relative to system uptime instead of deltas from current time. These allow for more race-resistant code to be written by allowing application code to do a single timeout computation, once, and then reuse the timeout value even if the thread wakes up and needs to suspend again later. Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
Add timeout generator macros to the public API for micro/nanosecond values and cycles. Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
Add tick-based (i.e. precision resistant) inspection APIs for kernel timeouts visible via k_timer, k_delayed work and thread timeouts (i.e. pended/sleeping threads). These are each available in "remaining" and "expires" variants returning time values relative to current time and system start. All have system calls where applicable (i.e. everywhere but k_delayed_work, which is not a userspace API) The pre-existing millisecond "remaining_get()" predicates for timer and delayed work remain, but are expressed in terms of the newer calls. Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
Add a call to get the system tick count as an official API (and redefine the existing millisecond API in terms of it). Sophisticated applications need to be able to count ticks directly, and the newer timeout API supports that. Uptime should too, for symmetry. Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
Add a doc string for k_timeout_t. Rephrase a few other items for clarity. Fix some syntax goofs. Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
andyross
force-pushed
the
timeout-take-2
branch
from
2883b2b to
c6d5dde
Compare
|
Rebased (actually the only collision was that last patch to cc26x2r1_launchxl , which made mine unnecessary so I just removed it). |
nashif
approved these changes
Mar 31, 2020
This was referenced Dec 18, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Big (but less so than the first version) API rework to make timeouts an opaque type, allowing for their specification in alternate representations like 64 bit types, higher precision units and absolute/uptime values.
See #21305 for the RFC detailing the changes.