Move idle task out of run queue #982
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The short of it is that our priority scheduler isn't actually a priority scheduler. I initially made a large PR with a bunch of scheduler changes but realised that because of all the renaming, file history was lost, so I've split the PR into a bunch of smaller PRs. After this is merged I'll create a PR to rename `scheduler_realtime` to `scheduler_priority`. Signed-off-by: Klimenty Tsoutsman <[email protected]>
Depends on theseus-os#980. The realtime scheduler doesn't actually implement rate monotonic scheduling as the period is just used as a glorified inverse priority. Next steps in the scheduler saga: - Allow changing scheduler policy live - Merge `set_priority` and `set_periodicity` (as the periodicity isn't properly used) - Merge scheduler and run queue crates (pending discussion on Discord) - Update docs to better explain crates, particularly `scheduler_epoch` I've implemented some of these and will open a PR once theseus-os#980 and theseus-os#981 are merged. Signed-off-by: Klimenty Tsoutsman <[email protected]>
Signed-off-by: Klimenty Tsoutsman <[email protected]>
tsoutsman
force-pushed
the
remove-idle-task-from-run-queue
branch
from
b680ba1 to
181b884
Compare
This was referenced Jun 19, 2023
Signed-off-by: Klimenty Tsoutsman <[email protected]>
Signed-off-by: Klimenty Tsoutsman <[email protected]>
kevinaboos
approved these changes
Jun 23, 2023
There was a problem hiding this comment.
Looks good!
Regarding the "TODO"s/questions you left, I think it's probably best to continue to return an Option<TaskRef> from select_next_task(), and then have the schedule() function select an idle task independently of the runqueue. This goes hand-in-hand with another change we can make, which is to move the idle task into CPU-local storage, since it "belongs" a CPU instance rather than a runqueue instance. This also makes the runqueue design have fewer concerns and also more flexible, since we may not always want to create exactly one runqueue per CPU.
I'll likely make some of those changes later, or you can do it as part of your series of scheduler PRs; your call.
github-actions bot
pushed a commit
that referenced
this pull request
Jun 23, 2023
* As part of our ongoing scheduler/runqueue redesign, there is no real reason to store each CPU's idle task in a scheduler runqueue. They are only used separately as a "last resort" when no runnable tasks exist, so we can store them alongside the runqueue rather than inside it. * Also, we change the creation of idle tasks to be done as part of initializing the tasking subsystem on each CPU rather than as an explicit step in the captain's init routine. * In the future, each CPU's idle task can be stored in CPU-local storage, since they ideologically "belong" to a CPU instance rather than a runqueue instance. * Unrelated: move `state_transfer` to old crates, as it currently doesn't build any more due to the ongoing scheduler/runqueue changes. Signed-off-by: Klimenty Tsoutsman <[email protected]> 18dc5f5
github-actions bot
pushed a commit
to tsoutsman/Theseus
that referenced
this pull request
Jun 23, 2023
* As part of our ongoing scheduler/runqueue redesign, there is no real reason to store each CPU's idle task in a scheduler runqueue. They are only used separately as a "last resort" when no runnable tasks exist, so we can store them alongside the runqueue rather than inside it. * Also, we change the creation of idle tasks to be done as part of initializing the tasking subsystem on each CPU rather than as an explicit step in the captain's init routine. * In the future, each CPU's idle task can be stored in CPU-local storage, since they ideologically "belong" to a CPU instance rather than a runqueue instance. * Unrelated: move `state_transfer` to old crates, as it currently doesn't build any more due to the ongoing scheduler/runqueue changes. Signed-off-by: Klimenty Tsoutsman <[email protected]> 18dc5f5
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Depends on #980 and #981.
Moves
state_transfertoold_cratesbecause it no longer compiles and would need to be reworked, but it wasn't functional in the first place.