Skip to content

Fix rounding in _ms_to_ticks() #12509

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

Closed
pizi-nordic opened this issue Jan 16, 2019 · 7 comments · Fixed by #19591
Closed

Fix rounding in _ms_to_ticks() #12509

pizi-nordic opened this issue Jan 16, 2019 · 7 comments · Fixed by #19591
Assignees
Labels
area: Kernel area: Timer Timer bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug
Milestone

Comments

@pizi-nordic
Copy link
Collaborator

At the moment the _ms_to_ticks() rounds down, which results in premature expiration of timeouts scheduled just before edge of tick. To fix that we have to revert #9745 in order to restore rounding up.

@pizi-nordic pizi-nordic added the bug The issue is a bug, or the PR is fixing a bug label Jan 16, 2019
@pizi-nordic pizi-nordic self-assigned this Jan 16, 2019
@pabigot
Copy link
Collaborator

pabigot commented Jan 16, 2019

That's one place where things get rounded down. The same truncation happens in z_clock_elapsed in multiple architectures where fractional ticks are discarded. Overall the need to traffic in durations that are downsampled from the hardware clock resolution in multiple places makes proper management of sleeps and alarms quite difficult.

@nashif nashif added the priority: medium Medium impact/importance bug label Jan 29, 2019
@galak galak added dev-review To be discussed in dev-review meeting and removed dev-review To be discussed in dev-review meeting labels Mar 26, 2019
@galak galak added this to the future milestone Mar 26, 2019
@nashif nashif removed this from the future milestone May 21, 2019
@ioannisg
Copy link
Member

@pizi-nordic @carlescufi is this still applicable, and if so, how can we address it ifor 2.0?

@pizi-nordic
Copy link
Collaborator Author

IMHO it should be fixed. I can prepare PR after I finish userspace logging.

@jenmwms jenmwms added this to the v2.0.0 milestone Aug 23, 2019
@ioannisg
Copy link
Member

IMHO it should be fixed. I can prepare PR after I finish userspace logging.

Could you please address this for 2.0 @pizi-nordic ?

@carlescufi
Copy link
Member

carlescufi commented Aug 27, 2019

IMHO it should be fixed. I can prepare PR after I finish userspace logging.

Could you please address this for 2.0 @pizi-nordic ?

@pizi-nordic
Or actually, will this be fixed by #17155?
And if so, can we just close this and track the issue directly in #9904 ?

@pizi-nordic
Copy link
Collaborator Author

pizi-nordic commented Aug 28, 2019

@carlescufi: Yes, it will be fixed in #17155. However I will prepare a PR with this because #17155 will not go into 2.0 and now the requested change is safe as #16782 is merged.

@pabigot
Copy link
Collaborator

pabigot commented Oct 8, 2019

This is pending in #19591 which uses ceil for ms_to_ticks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Kernel area: Timer Timer bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug
Projects
None yet
7 participants