Kernel MS Precision


Michael Rosen
 

Zephyr Devs,

 

Weve started moving our project from Zephyr 1.5 to Zephyr 1.7. One big change (aside from the really big ones like unified kernel and all that) is that the APIs for timers and others seems to have changes from taking in values in ticks to taking their arguments in milliseconds. For most applications, this is probably fine but in our case, its really unfortunate. We want to have a tick granularity of 500us (0.5ms) for our system so we can do operations at more precise timings (yes, at the cost of extra timer interrupts), and with the old API, that wasn’t an issue. Youd just change your units to microseconds and do things like:

 

nano_timer_start(&timer, USEC(1500));

 

To get a 1.5ms timer. Now, there seems to be K_MSEC and K_SECONDS to convert from “kernel time measure” as before (replacing just MSEC and SECONDS) but this is just a direct translation and the function itself does the transformation into ticks as the first thing it does. Is there a strong reason why the API was changed to prevent greater than 1ms ticks from being easy to achieve, especially considering users are expected to use K_MSEC and K_SECONDS anyway? I don’t expect any system to have a 1us tick, but just a 0.5ms tick is now impossible (without modifying the kernel code at least).

 

Thanks,

Mike

 

 

 


Benjamin Walsh <benjamin.walsh@...>
 

Hi Mike,

Weve started moving our project from Zephyr 1.5 to Zephyr 1.7. One big
change (aside from the really big ones like unified kernel and all
that) is that the APIs for timers and others seems to have changes
from taking in values in ticks to taking their arguments in
milliseconds. For most applications, this is probably fine but in our
case, its really unfortunate. We want to have a tick granularity of
500us (0.5ms) for our system so we can do operations at more precise
timings (yes, at the cost of extra timer interrupts), and with the old
API, that wasn't an issue. Youd just change your units to microseconds
and do things like:

nano_timer_start(&timer, USEC(1500));

To get a 1.5ms timer. Now, there seems to be K_MSEC and K_SECONDS to
convert from "kernel time measure" as before (replacing just MSEC and
SECONDS) but this is just a direct translation and the function itself
does the transformation into ticks as the first thing it does. Is
there a strong reason why the API was changed to prevent greater than
1ms ticks from being easy to achieve, especially considering users are
expected to use K_MSEC and K_SECONDS anyway? I don't expect any system
to have a 1us tick, but just a 0.5ms tick is now impossible (without
modifying the kernel code at least).
When prototyping the new API, I started with timeouts as an int64_t in
nanoseconds, to allow the same precision as struct timespec, but without
the akwardness of having to create an instance of a struct variable
every time. However, using it when writing test code mostly, I disliked
it for two reasons: 1. often having to pass in large numbers (minor
issue) and 2. having to deal with 64-bit number math. So, I polled
people, on the mailing list IIRC, to find out if having int32_t timeouts
in milliseconds would be reasonable for Zephyr. I received one or two
responses, both positive, so I went with this.

However, having timeouts as int32_t instead of uint32_t, there is
nothing preventing us from using negative values to represent other
units if we want to, in a backwards-compatible way. The only negative
value currently is K_FOREVER (0xffffffff). So, if we wanted to implement
better granularity, we could allow for a higher-rate system clock and
add macros to the API like, e.g.:

#define US_TIMEOUT(us) \
(int32_t)((((uint32_t)(us)) & 0x3fffffff) | 0x80000000)
// ^^^^^^^^^^^^^^^^^^^^^^^^
// keep the two upper bits as control bits just in
// case '10' would mean 'microseconds', '11' could
// mean something else

rc = sem_take(&my_sem, US_TIMEOUT(500));

and have the kernel timeout code decode this as the number of ticks
corresponding to 500us.

This is of course not implemented, but should be somewhat easy to do.

Regards,
Ben

--
Benjamin Walsh, SMTS
WR VxWorks Virtualization Profile
www.windriver.com
Zephyr kernel maintainer
www.zephyrproject.org


Michael Rosen
 

However, having timeouts as int32_t instead of uint32_t, there is nothing
preventing us from using negative values to represent other units if we want
to, in a backwards-compatible way. The only negative value currently is
K_FOREVER (0xffffffff). So, if we wanted to implement better granularity, we
could allow for a higher-rate system clock and add macros to the API like,
e.g.:

#define US_TIMEOUT(us) \
(int32_t)((((uint32_t)(us)) & 0x3fffffff) | 0x80000000)
// ^^^^^^^^^^^^^^^^^^^^^^^^
// keep the two upper bits as control bits just in
// case '10' would mean 'microseconds', '11' could
// mean something else

rc = sem_take(&my_sem, US_TIMEOUT(500));

and have the kernel timeout code decode this as the number of ticks
corresponding to 500us.

This is of course not implemented, but should be somewhat easy to do.
While Im not sure Im 100% convinced, having negative numbers just be the number in ticks as it was in the old API should be fine. Im curious though why you went for ns when most system wouldn't be able to handle sub microsecond ticks; is there actually support for sub-tick timeouts/timers?

Though at the same time, if users are expected to use the macros, why change it away from ticks at all? And if they aren't, why have the K_MSEC macro at all? If ticks are the most precise unit you can use for timers/timeouts and ticks are user configurable, why hold the API rigid to ms? Sorry I didn't jump on this topic when you were doing the redesign or I would have contributed this perspective then :(

Thanks,
Mike


Daniel Thompson <daniel.thompson@...>
 

On 23/03/17 14:49, Benjamin Walsh wrote:
Hi Mike,

Weve started moving our project from Zephyr 1.5 to Zephyr 1.7. One big
change (aside from the really big ones like unified kernel and all
that) is that the APIs for timers and others seems to have changes
from taking in values in ticks to taking their arguments in
milliseconds. For most applications, this is probably fine but in our
case, its really unfortunate. We want to have a tick granularity of
500us (0.5ms) for our system so we can do operations at more precise
timings (yes, at the cost of extra timer interrupts), and with the old
API, that wasn't an issue. Youd just change your units to microseconds
and do things like:

nano_timer_start(&timer, USEC(1500));

To get a 1.5ms timer. Now, there seems to be K_MSEC and K_SECONDS to
convert from "kernel time measure" as before (replacing just MSEC and
SECONDS) but this is just a direct translation and the function itself
does the transformation into ticks as the first thing it does. Is
there a strong reason why the API was changed to prevent greater than
1ms ticks from being easy to achieve, especially considering users are
expected to use K_MSEC and K_SECONDS anyway? I don't expect any system
to have a 1us tick, but just a 0.5ms tick is now impossible (without
modifying the kernel code at least).
When prototyping the new API, I started with timeouts as an int64_t in
nanoseconds, to allow the same precision as struct timespec, but without
the akwardness of having to create an instance of a struct variable
every time. However, using it when writing test code mostly, I disliked
it for two reasons: 1. often having to pass in large numbers (minor
issue) and 2. having to deal with 64-bit number math. So, I polled
people, on the mailing list IIRC, to find out if having int32_t timeouts
in milliseconds would be reasonable for Zephyr. I received one or two
responses, both positive, so I went with this.

However, having timeouts as int32_t instead of uint32_t, there is
nothing preventing us from using negative values to represent other
units if we want to, in a backwards-compatible way. The only negative
value currently is K_FOREVER (0xffffffff). So, if we wanted to implement
better granularity, we could allow for a higher-rate system clock and
add macros to the API like, e.g.:

#define US_TIMEOUT(us) \
(int32_t)((((uint32_t)(us)) & 0x3fffffff) | 0x80000000)
// ^^^^^^^^^^^^^^^^^^^^^^^^
// keep the two upper bits as control bits just in
// case '10' would mean 'microseconds', '11' could
// mean something else
Regarding reserving upper bits, perhaps think about this the other way around. What is the largest sane microsecond sleep? We need only to reserve enough bits to accommodate that...

For example once you are sleeping >100ms you should, perhaps, start questioning what the microsecond precision is needed for.


Daniel.


rc = sem_take(&my_sem, US_TIMEOUT(500));

and have the kernel timeout code decode this as the number of ticks
corresponding to 500us.

This is of course not implemented, but should be somewhat easy to do.

Regards,
Ben


Benjamin Walsh <benjamin.walsh@...>
 

On Thu, Mar 23, 2017 at 03:40:42PM +0000, Rosen, Michael R wrote:
However, having timeouts as int32_t instead of uint32_t, there is nothing
preventing us from using negative values to represent other units if we want
to, in a backwards-compatible way. The only negative value currently is
K_FOREVER (0xffffffff). So, if we wanted to implement better granularity, we
could allow for a higher-rate system clock and add macros to the API like,
e.g.:

#define US_TIMEOUT(us) \
(int32_t)((((uint32_t)(us)) & 0x3fffffff) | 0x80000000)
// ^^^^^^^^^^^^^^^^^^^^^^^^
// keep the two upper bits as control bits just in
// case '10' would mean 'microseconds', '11' could
// mean something else

rc = sem_take(&my_sem, US_TIMEOUT(500));

and have the kernel timeout code decode this as the number of ticks
corresponding to 500us.

This is of course not implemented, but should be somewhat easy to do.
While Im not sure Im 100% convinced, having negative numbers just be
the number in ticks as it was in the old API should be fine. Im
curious though why you went for ns when most system wouldn't be able
to handle sub microsecond ticks; is there actually support for
sub-tick timeouts/timers?
ns or us, you still needed 64-bit timeouts for either, which ended up
being the major concern. 64-bit math can be costly: we've seen stack
usage increase of 80 bytes on some architectures. Also, I don't think
2000s is enough for a maximum timeout, with us resolution and signed
32-bit.

Though at the same time, if users are expected to use the macros, why
change it away from ticks at all? And if they aren't, why have the
K_MSEC macro at all? If ticks are the most precise unit you can use
I think ms is the unit most users will use, and that one does not always
need a macro.

I don't remember why we added K_MSEC(), maybe to round up the API
with K_SECONDS, etc.

The users can decide to use the macros or not.

Do you prefer

k_sleep(3600000);

or

k_sleep(K_HOURS(1));

?

Which one is less confusing or error-prone ?

However

k_sleep(100);

is perfectly readable.

for timers/timeouts and ticks are user configurable, why hold the API
rigid to ms? Sorry I didn't jump on this topic when you were doing the
redesign or I would have contributed this perspective then :(
Because we wanted to go to a tickless kernel, so we wanted to have the
API use common time units instead of ticks.

This was covered in the unified kernel RFC. You can search the mailing
list of still find an early version here:

https://gerrit.zephyrproject.org/r/#/c/2255/2/incoming/unified_kernel.rst

It's still pretty accurate, but it's missing the ms timeouts discussion,
since we decided not to use gerrit to handle RFCs after this was
published, and thus never updated.

My take on this is that there is a way forward that is
backwards-compatible with the current API. It should probably be
configurable, since the timeout code will take hit to handle the two
time units.

That work can be turned into a Jira and prioritized if deemed important
enough, and it seems it is, for you at least, which means that probably
other people will hit the same issue.

Regards,
Ben


Andreas Lenz
 

Hi Ben,

#define US_TIMEOUT(us) \
(int32_t)((((uint32_t)(us)) & 0x3fffffff) | 0x80000000)
// ^^^^^^^^^^^^^^^^^^^^^^^^
// keep the two upper bits as control bits just in
// case '10' would mean 'microseconds', '11' could
// mean something else
You could also use the full bits and add one additional byte to specify the unit of the number.
Timers store their unit together with duration and period. For example
k_timer_start(timer, 100, 0, K_MSECONDS)
k_timer_start(timer, 100, 0, K_USECONDS)

For the "mean something else", I have a use case for low-priority, or lazy timers.
They don't prevent the kernel to go into idle and expire later when the system wakes up again.
What I have in mind is battery monitoring where checks should be done about once every hour, but only when the system is active.
However, K_FOREVER might be problematic as the time can wrap.

Best regards,
Andreas


Benjamin Walsh <benjamin.walsh@...>
 

On Fri, Mar 24, 2017 at 08:24:17AM +0000, Andreas Lenz wrote:
Hi Ben,

#define US_TIMEOUT(us) \
(int32_t)((((uint32_t)(us)) & 0x3fffffff) | 0x80000000)
// ^^^^^^^^^^^^^^^^^^^^^^^^
// keep the two upper bits as control bits just in
// case '10' would mean 'microseconds', '11' could
// mean something else
You could also use the full bits and add one additional byte to
specify the unit of the number.

Timers store their unit together with duration and period. For example
k_timer_start(timer, 100, 0, K_MSECONDS)
k_timer_start(timer, 100, 0, K_USECONDS)
Yeah, but that is not backwards-compatible with the API. And that only
works for timers, not the other APIs that take timeouts. Although, that
might be irrelevant.

For the "mean something else", I have a use case for low-priority, or
lazy timers.

They don't prevent the kernel to go into idle and expire later when
the system wakes up again.
Interesting idea. That could be a new API for timers though, it doesn't
have to modify an already existing one.

k_timer_start_lazy(timer, <timeout>);

Actually, it would probably have to be handled differently as well,
since the current implementation of timeouts does not handle having more
expired ticks than the next timer to expire, and this condition would
happen with this new feature when the kernel is in tickless idle.

What I have in mind is battery monitoring where checks should be done
about once every hour, but only when the system is active.

However, K_FOREVER might be problematic as the time can wrap.

Best regards,
Andreas


Marti Bolivar <marti.bolivar@...>
 

On 24 March 2017 at 10:05, Benjamin Walsh <benjamin.walsh@windriver.com> wrote:
On Fri, Mar 24, 2017 at 08:24:17AM +0000, Andreas Lenz wrote:
Hi Ben,

#define US_TIMEOUT(us) \
(int32_t)((((uint32_t)(us)) & 0x3fffffff) | 0x80000000)
// ^^^^^^^^^^^^^^^^^^^^^^^^
// keep the two upper bits as control bits just in
// case '10' would mean 'microseconds', '11' could
// mean something else
You could also use the full bits and add one additional byte to
specify the unit of the number.

Timers store their unit together with duration and period. For example
k_timer_start(timer, 100, 0, K_MSECONDS)
k_timer_start(timer, 100, 0, K_USECONDS)
Yeah, but that is not backwards-compatible with the API. And that only
works for timers, not the other APIs that take timeouts. Although, that
might be irrelevant.

For the "mean something else", I have a use case for low-priority, or
lazy timers.

They don't prevent the kernel to go into idle and expire later when
the system wakes up again.
Interesting idea. That could be a new API for timers though, it doesn't
have to modify an already existing one.
In a similar vein, could you add a new timer API that takes units, and
(conditioned on a config option to avoid 64 bit math on targets that
want to avoid it) implement the existing timer API on top of it for
compatibility?

void k_timer_start_prec(timer, duration, period, units);
int64_t k_timer_remaining_get(timer, units);

Marti


Marti Bolivar <marti.bolivar@...>
 

On 24 March 2017 at 10:16, Marti Bolivar <marti.bolivar@linaro.org> wrote:
On 24 March 2017 at 10:05, Benjamin Walsh <benjamin.walsh@windriver.com> wrote:
On Fri, Mar 24, 2017 at 08:24:17AM +0000, Andreas Lenz wrote:
Hi Ben,

#define US_TIMEOUT(us) \
(int32_t)((((uint32_t)(us)) & 0x3fffffff) | 0x80000000)
// ^^^^^^^^^^^^^^^^^^^^^^^^
// keep the two upper bits as control bits just in
// case '10' would mean 'microseconds', '11' could
// mean something else
You could also use the full bits and add one additional byte to
specify the unit of the number.

Timers store their unit together with duration and period. For example
k_timer_start(timer, 100, 0, K_MSECONDS)
k_timer_start(timer, 100, 0, K_USECONDS)
Yeah, but that is not backwards-compatible with the API. And that only
works for timers, not the other APIs that take timeouts. Although, that
might be irrelevant.

For the "mean something else", I have a use case for low-priority, or
lazy timers.

They don't prevent the kernel to go into idle and expire later when
the system wakes up again.
Interesting idea. That could be a new API for timers though, it doesn't
have to modify an already existing one.
In a similar vein, could you add a new timer API that takes units, and
(conditioned on a config option to avoid 64 bit math on targets that
want to avoid it) implement the existing timer API on top of it for
compatibility?

void k_timer_start_prec(timer, duration, period, units);
int64_t k_timer_remaining_get(timer, units);
Meant k_timer_remaining_get_prec above. Sorry.


Marti