Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generalizing duration representation in clock-agnostic way #19656

Open
pabigot opened this issue Oct 7, 2019 · 1 comment
Open

Generalizing duration representation in clock-agnostic way #19656

pabigot opened this issue Oct 7, 2019 · 1 comment
Assignees
Labels
area: Kernel RFC Request For Comments: want input from the community

Comments

@pabigot
Copy link
Collaborator

pabigot commented Oct 7, 2019

Zephyr kernel APIs specify a timeout in milliseconds, encoded as a signed 32-bit integer. This API must switch to a more flexible encoding to support elements of #19282 in which timeouts would support higher resolutions (perhaps down to nanoseconds), absolute as well as relative deadlines, and a clock that would be used as the basis for the timeout.

This RFC proposes a path to preserving timeout encodings with the existing helper macros (viz. K_MSEC(n), K_SECONDS(n)) that reduces the impact on existing code and abstracts from platform-specific characteristics like clock frequency while improving resolution and providing forwards compatibility.

This change is proposed to be executed in non-initial step of #19655.

Problem description

PR #17155 proposed a proof-of-concept for an opaque timeout representation that would allow the flexibility required by #19282. Part of the design and implementation added new API like K_TIMEOUT_MS(x) to replace K_MSEC(x) by storing the count in system ticks rather than milliseconds.

The representation of timeouts as a count of ticks rather than milliseconds requires duration calculations that had been done at compile-time to be performed at runtime, since the time-to-ticks conversion depends on sys_clock_hw_cycles_per_sec() which may not be a compile-time constant. This impacts use of K_THREAD_DEFINE() which requires a compile-time delay while k_thread_create() which can accept a runtime-specified delay.

A desire to avoid that conversion is the subject of this RFC. Implementation of this should follow the change of timeout representation to k_timeout_t and of the duration count to an unsigned 64-bit type.

Background

Current timeouts are represented in milliseconds, and literal values are constructed in one of these ways:

  • K_NO_WAIT is a constant representing a zero duration. This generally means there will be no sleep/block/context switch as a result of selecting this timeout. In the current implementation this evaluates to 0.
  • K_FOREVER is a constant representing an unbounded timeout. In the current implementation this evalutes to -1.
  • K_MSEC(x) provides the value x unchanged. Where x is less than -1 the implied behavior is generally equivalent to having passed zero (i.e. equivalent to K_NO_WAIT.

Additional macros K_SECONDS(), K_MINUTES(), K_HOURS() construct equivalent durations in milliseconds.

Feedback on #17155 in a discussion of whether k_sleep() should also be converted to take a timeout show strong support for retaining macros like these that clearly express the intended duration.

Proposed change

#19030 is a step within #19282 that proposes Zephyr support multiple clocks that can be used to control timeouts, including as candidates the system clock (with tick resolution), a hardware clock (with cycle resolution), and an optional realtime clock (associated with an RTC peripheral or conditioned system clock). Note that in this model the marked values K_NO_WAIT and K_FOREVER are independent of the clock.

I propose another (virtual) clock K_CLOCK_DIVIDED_SECONDS that is compatible with the current represention, which represents durations as fractional seconds. The resolution of the representation is controlled by a new Kconfig parameter CONFIG_SYS_TIMEOUT_DIVIDED_SECONDS_HZ which would default to 1000.

Then define K_MSEC(x) to produce a k_timeout_t value that expresses the desired duration in ticks of this clock:

#define K_USEC(x) (((x) * (u32_t)CONFIG_SYS_TIMEOUT_DIVIDED_SECONDS_HZ + 999999U) / 1000000U)
#define K_MSEC(x) (((x) * (u32_t)CONFIG_SYS_TIMEOUT_DIVIDED_SECONDS_HZ + 999U) / 1000U)
#define K_SECONDS(x) ((x) * (u32_t)CONFIG_SYS_TIMEOUT_DIVIDED_SECONDS_HZ)
#define K_MINUTES(x) ((x) * K_SECONDS(60U))
#define K_HOURS(x) ((x) * K_MINUTES(60U))

The benefits of this are:

  • There is no need to change any code that constructs timeouts using the existing macros. Only the resulting value will change. This will eliminate the need to change most existing code.
  • The macros continue to be resolvable at compile-time, eliminating any dependency on runtime-determined clock speed.

Note that this feature may be added without generalizing the timeout representation to include a specific clock, as in this case K_CLOCK_DIVIDED_SECONDS would be the default clock (contrary to the original proposal in #19030). The timeout would then be converted from divided clock ticks to system ticks at the point where it's introduced into the existing struct _timeout infrastructure, exactly as is done today. k_divsec_to_ticks() can be added as an extension of #19591.

@zephyrbot
Copy link
Collaborator

Hi @peter-mitsis, @andyross,

This issue, marked as an RFC, was opened a while ago and did not get any traction. It was just assigned to you based on the labels. If you don't consider yourself the right person to address this issue, please re-assing it to the right person.

Please take a moment to review if the issue is still relevant to the project. If it is, please provide feedback and direction on how to move forward. If it is not, has already been addressed, is a duplicate, or is no longer relevant, please close it with a short comment explaining the reason.

@pabigot you are also encouraged to help moving this issue forward by providing additional information and confirming this request/issue is still relevant to you.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Kernel RFC Request For Comments: want input from the community
Projects
Status: No status
Development

No branches or pull requests

4 participants