Skip to content

tickless and timeslicing don't play well together #7193

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
andyross opened this issue Apr 25, 2018 · 7 comments
Closed

tickless and timeslicing don't play well together #7193

andyross opened this issue Apr 25, 2018 · 7 comments
Assignees
Labels
area: Kernel bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug
Milestone

Comments

@andyross
Copy link
Collaborator

Specific bug: wake up a thread (or create a new one) at the same priority as the currently running thread, both of which are at the same preemptible priority and in a situation where no other threads exist at that priority. In theory, the CPU timer should be adjusted to reflect the new situation and expire in one slicing tick. In practice, the timer is not adjusted and the new thread won't get any CPU until whenever the next interrupt fires, which could be a long time.

That issue was found via inspection as I stumpled over this code while working on the scheduler. The more general problem is that we have pretty poor test coverage for both timeslicing and tickless behaviors, this situation needs to be tested and handled, along with (probably) others.

@nashif nashif added the bug The issue is a bug, or the PR is fixing a bug label Apr 25, 2018
@jli157
Copy link
Contributor

jli157 commented Apr 26, 2018

Could the issue #7048 be caused by same root cause?

@MaureenHelm MaureenHelm added priority: medium Medium impact/importance bug area: Kernel labels May 1, 2018
@andyross andyross self-assigned this May 24, 2018
@nashif nashif added this to the v1.13.0 milestone Jun 5, 2018
@nashif
Copy link
Member

nashif commented Jul 24, 2018

@andyross will this be addressed for 1.13?

@andyross
Copy link
Collaborator Author

The bug found by inspection will be fixed. Probably need some sort of longer term tracking on enhancing tickless (making it default where possible, expanding tests) and coming up with a better set of tests on timeslicing that look for fairness issues like this.

@pizi-nordic
Copy link
Collaborator

FYI: @pizi-nordic

@andyross
Copy link
Collaborator Author

Fix for the specific bug above is submitted. Probably should reprioritize rather than close this one, as the rework is if anything needed more now than before, given that we've added another #ifdef TICKLESS case.

@nashif nashif modified the milestone: v1.13.0 Aug 26, 2018
andyross pushed a commit to andyross/zephyr that referenced this issue Aug 27, 2018
When adding a new runnable thread in tickless mode, we need to detect
whether it will timeslice with the runnable thread and reset the
timer, otherwise it won't get any CPU time until the next interrupt
fires at some indeterminate time in the future.

This fixes the specific bug discussed in zephyrproject-rtos#7193, but the broader
problem of tickless and timeslicing interacting badly remains.  The
code as it exists needs some rework to avoid all the #ifdef mess.

Note that the patch also moves _ready_thread() from a ksched.h inline
to sched.c.

Signed-off-by: Andy Ross <[email protected]>
@andyross
Copy link
Collaborator Author

andyross commented Aug 27, 2018

Ugh, fixed correctly this time. No idea what happened earlier, presumably I rebased, but then forgot the "-a" on the commit?

(And of course I put that comment in the bug and not the PR. Probably shouldn't be pushing until another coffee or three...)

nashif pushed a commit that referenced this issue Aug 27, 2018
When adding a new runnable thread in tickless mode, we need to detect
whether it will timeslice with the runnable thread and reset the
timer, otherwise it won't get any CPU time until the next interrupt
fires at some indeterminate time in the future.

This fixes the specific bug discussed in #7193, but the broader
problem of tickless and timeslicing interacting badly remains.  The
code as it exists needs some rework to avoid all the #ifdef mess.

Note that the patch also moves _ready_thread() from a ksched.h inline
to sched.c.

Signed-off-by: Andy Ross <[email protected]>
andyross pushed a commit to andyross/zephyr that referenced this issue Aug 28, 2018
When adding a new runnable thread in tickless mode, we need to detect
whether it will timeslice with the running thread and reset the timer,
otherwise it won't get any CPU time until the next interrupt fires at
some indeterminate time in the future.

This fixes the specific bug discussed in zephyrproject-rtos#7193, but the broader
problem of tickless and timeslicing interacting badly remains.  The
code as it exists needs some rework to avoid all the #ifdef mess.

Signed-off-by: Andy Ross <[email protected]>
nashif pushed a commit that referenced this issue Aug 29, 2018
When adding a new runnable thread in tickless mode, we need to detect
whether it will timeslice with the running thread and reset the timer,
otherwise it won't get any CPU time until the next interrupt fires at
some indeterminate time in the future.

This fixes the specific bug discussed in #7193, but the broader
problem of tickless and timeslicing interacting badly remains.  The
code as it exists needs some rework to avoid all the #ifdef mess.

Signed-off-by: Andy Ross <[email protected]>
@nashif nashif modified the milestones: v1.13.0, v1.14.0 Sep 3, 2018
@andyross
Copy link
Collaborator Author

Fixed as of the big timer rework. Tickless is default and all timeslicing tests pass with it enabled. Closing as I'm the one who opened it to begin with.

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

No branches or pull requests

5 participants