tosc-rs / mnemos

An Operating System for Building Small Computers
https://mnemos.dev
Apache License 2.0
253 stars 18 forks source link

Investigate main event loop logic #257

Open spookyvision opened 1 year ago

spookyvision commented 1 year ago

this part of a canonical kernel event loop:

if next_turn.expired == 0 || !tick.has_remaining {

seems to always evaluate to true - at least in the pomelo simulator.

hawkw commented 1 year ago

This is working as intended --- it's a result of there not being very many tasks running on the kernel.

tick.has_remaining() will only ever be true if more than 256 tasks were woken since the last tick: https://github.com/hawkw/mycelium/blob/a06e352494ca83cddf541c61a06a898f4c4e356c/maitake/src/scheduler.rs#L837

next_turn.expired would be greater than 0 if the timer wheel turn woke any tasks. but if we are (almost) always woken by a timer interrupt, and we advance the timer immediately after the timer interrupt fires, those tasks have already been woken. we might see the second turn of the timer wheel (after ticking the scheduler) also wake tasks if the scheduler tick took longer than it does currently (e.g. there are more tasks running, or some tasks took a very long time to execute). but, our tick durations are currently very short, so it's extremely unlikely that any new timeouts would have completed since the last turn of the timer wheel.

This if condition is intended so that the kernel only goes to sleep and waits for an interrupt if there is no ready work remaining after the scheduler tick. Since the kernel isn't running very many tasks and we can poll all woken tasks in a single scheduler tick, we pretty much never need to continue ticking and can go ahead and wait for an interrupt, which is the correct behavior here. This might not be in the case in the future, if significantly more tasks are running on the scheduler. In that case, we might occasionally need to keep ticking until all the ready tasks have been polled, before going to sleep.

hawkw commented 1 year ago

Maybe we should add a longer comment in the source explaining this...