Skip to content

kernel: Improve precision of ticks and ms conversions #9745

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

Merged
merged 2 commits into from
Aug 31, 2018

Conversation

carlescufi
Copy link
Member

The following 2 improvements are contained in this patch:

  • When converting from ms to ticks, instead of using hardware cycles
    per tick, use hardware cycles per second. This ensures that the
    multiplication is done before the division, increasing precision.
  • When converting from ticks to ms, instead of using cycles per tick
    and cycles per sec, use ticks per sec. This too increases the
    precision.

The concept is to make the dividend as large as possible compared to the
divisor in order to lose as little precision as possible.

Fixes #8898
Fixes #9459
Fixes #9466
Fixes #9468

Signed-off-by: Vinayak Kariappa Chettimada [email protected]

include/kernel.h Outdated
@@ -1363,7 +1363,8 @@ static ALWAYS_INLINE s32_t _ms_to_ticks(s32_t ms)
/* use 64-bit math to keep precision */
return (s32_t)ceiling_fraction(
(s64_t)ms * sys_clock_hw_cycles_per_sec,
(s64_t)MSEC_PER_SEC * sys_clock_hw_cycles_per_tick);
((s64_t)MSEC_PER_SEC * sys_clock_hw_cycles_per_sec) /
sys_clock_ticks_per_sec);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I had an extra space prefixed here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you did, fixed now

@carlescufi carlescufi force-pushed the vinayak-timer-fix branch 3 times, most recently from e72ddf8 to ea64ab2 Compare August 31, 2018 10:43
@codecov-io
Copy link

codecov-io commented Aug 31, 2018

Codecov Report

Merging #9745 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #9745   +/-   ##
=======================================
  Coverage   52.24%   52.24%           
=======================================
  Files         212      212           
  Lines       25950    25950           
  Branches     5593     5593           
=======================================
  Hits        13557    13557           
  Misses      10159    10159           
  Partials     2234     2234
Impacted Files Coverage Δ
include/kernel.h 98.52% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 19b7eaa...bc087b3. Read the comment docs.

@carlescufi
Copy link
Member Author

@nashif not sure about the failures, seem related to stack overflow

The following 2 improvements are contained in this patch:

- When converting from ms to ticks, instead of using hardware cycles
  per tick, use hardware cycles per second. This ensures that the
  multiplication is done before the division, increasing precision.
- When converting from ticks to ms, instead of using cycles per tick
  and cycles per sec, use ticks per sec. This too increases the
  precision.

The concept is to make the dividend as large as possible compared to the
divisor in order to lose as little precision as possible.

Fixes zephyrproject-rtos#8898
Fixes zephyrproject-rtos#9459
Fixes zephyrproject-rtos#9466
Fixes zephyrproject-rtos#9468

Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
Signed-off-by: Carles Cufi <[email protected]>
Failed with:

Running test suite suite_preempt
===================================================================
starting test - test_preempt
***** Stack Check Fail! *****
Current thread ID = 0x0040019c
eax: 0x00400254, ebx: 0x00400254, ecx: 0x004002b0, edx: 0x00000001
esi: 0x004001f8, edi: 0x00401080, ebp: 0x00408024, esp: 0x00408000
eflags: 0x00000046 cs: 0x0008
call trace:
eip: 0x00002115
     0x000021b8 (0x40019c)
     0x00002685 (0x4001f8)
     0x00004f65 (0x401120)
     0x00005187 (0x401120)
     0x000051c2 (0x40019c)
     0x000052be (0xffffffff)
     0x00005bab (0x246)
     0x000019c4 (0x400078)
Fatal fault in thread 0x0040019c! Aborting.

Signed-off-by: Anas Nashif <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants