Milestones toward generalized representation of timeouts

[pabigot]

PR #17155 proposed a proof-of-concept for an opaque timeout representation that would allow the flexibility required by #19282. Over time the changes grew to include:

• A complete family of functions to convert between time-based, tick-based, and cycle-based durations including control of rounding (now separately submitted in New, orthogonal and complete time conversion API #19591);
• A representation of timeout values as an opaque struct that encoded timeout features;
• New API like K_TIMEOUT_MS(x) to replace K_MSEC(x) by storing the count in system ticks rather than milliseconds.
• Conversion of existing code to the new API.

Although the initial steps were promising the PR grew to include a conversion of the entire Zephyr tree to support the new API, and making the new API the default. This exposed the following problems:

• The PR became unreviewable as it modified almost 600 files.
• It revealed many cases where kernel (threads), driver, and subsystem code stored delay values in other structures, used them as values in expressions, and passed them through internal functions.
• Global changes to the API were made without giving codeowners an opportunity to consider how new capabilities should be used (i.e. where the transition between integral durations and timeouts should be done)
• The new representation of timeouts as a count of ticks required duration calculations that had been done at compile-time to be performed at runtime as 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() can accept a runtime-specified delay.

This issue is intended to address the request for more details on what would make the changes in PR #17155 acceptable. It does that by breaking the task down into discrete, focused stages (i.e. Pull Requests) that can be reviewed and merged without building up massive changes (with one exception that's unavoidable):

• Unify the existing kernel APIs
• Replace the integer timeout representation with an opaque structure
• Transition to representing relative timeouts as unsigned values
• Support using 64-bit timeouts
• Support for high resolution timeouts

It does not extend to cover all features of #17155, which may be revisited after the initial stages of supporting a flexible representation have been completed.

Unify the existing kernel APIs

Ensure that all places where literal integers are passed to kernel API that will in the future take a k_timeout_t are converted to use the timeout macros K_NO_WAIT, K_FOREVER, and K_MSEC(x).

This is currently being done using Coccinelle, with #19650 being the most recent step.

Replace the integer timeout representation with an opaque structure

We need a one-time global change for public API involving timeouts to convert from s32_t to the opaque k_timeout_t type. This change will likely affect hundreds of files. To keep it reviewable and trusted it must not include any behavioral changes beyond the structural type change.

To accomplish this the PR should change all kernel API where a timeout is represented as an s32_t to instead be represented by:

typedef struct k_timeout_t {
    s32_t ms;
} k_timeout_t;

It should change the generating macros something like this:

#define K_NO_WAIT_VALUE 0
#define K_FOREVER_VALUE -1
#define K_NO_WAIT ((k_timeout_t){.ms = K_NO_WAIT_VALUE})
#define K_FOREVER ((k_timeout_t){.ms = K_FOREVER_VALUE})
#define K_MSEC(x) ((k_timeout_t){.ms = x})

It should add functionality something like this:

#define K_TIMEOUT_GET_VALUE(t) (t).ms
#define K_TIMEOUT_IS_NO_WAIT(t) (K_TIMEOUT_GET_VALUE(t) == K_NO_WAIT_VALUE)
#define K_TIMEOUT_IS_FOREVER(t) (K_TIMEOUT_GET_VALUE(t) == K_FOREVER_VALUE)

Any use of timeout values that requires knowing the duration value or comparing the duration to the marked K_NO_WAIT or K_FOREVER should convert to the new helper macros.

The purpose of this is to identify, by the resulting compilation errors, all places where code converts between an integer representation and a k_timeout_t representation, and determine which representation is appropriate for each case. There are likely to be many of them.

It would be possible to use Coccinelle to convert existing uses, but ideally this step should involve the maintainers of code that must be changed, to determine at what point the integral duration should be converted to a timeout.

Transition to representing timeouts as unsigned values

Zephyr does not currently support a relative timeout that is in the past: all such timeouts are treated as equivalent to K_NO_WAIT. The use of signed values for delays cuts the maximum representable delay in half. It also impacts expression evaluation as any overflow will invoke undefined behavior.

Replace the k_timeout_t structure with:

#define K_FOREVER_VALUE (u32_t)-1
typedef struct k_timeout_t {
  u32_t ms;
} k_timeout_t;

Identify places where this causes a behavioral change and rework the logic to be correct.

Support using 64-bit timeouts

Add a Kconfig option such as CONFIG_SYS_TIMEOUT_64BIT that would change k_timeout_t to:

#ifdef CONFIG_SYS_TIMEOUT_64BIT
typedef u64_t k_timeout_value_t;
#else
typedef u32_t k_timeout_value_t;
#endif
#define K_FOREVER_VALUE (k_timeout_value_t)-1
typedef struct k_timeout_t {
  k_timeout_value_t value;
} k_timeout_t;

#define K_TIMEOUT_GET_VALUE(t) (t).value

Support for high resolution timeouts

At this point, with 64-bit timeout support, we can move to using higher resolution (sub-millisecond) timeout values as proposed in #19656.

Support for additional enhancements

Other desires described in #19282 can be designed and implemented after we've reached the stages described above.

[pabigot]

@andyross please review. I'd like to work with you towards our goal.

[carlescufi]

@nashif @andyross I think this is a great description of a suggested sequence of discrete steps to achieve the goal of moving to the opaque structures and 64-bit timeout support.

[andyross]

cleaning out old cruft, the relevant treaty was signed long ago and peace has reigned since