scx_lavd: Clean up task state transition tracking #199
Conversation
This file contains 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
Since https://doc.rust-lang.org/edition-guide/rust-2018/path-changes.html, extern crate declarations aren't necessary. Let's drop them.
scx_lavd tracks task state transitions and updates statistics on each valid transition. However, there's an asymmetry between the runnable/running and stopping/quiescent transitions. In the former, the runnable and running transitions are accounted separately in update_stat_for_enq() and update_stat_for_run(), respectively. However, in the latter, the two transitions are combined together in update_stat_for_stop(). This asymmetry leads to incorrect accounting. For example, a task's load should be added to the cpu's load sum when the task gets enqueued and subtracted when the task is no longer runnable (quiescent). The former is accounted correctly from update_stat_for_enq() but the latter is done whenever the task stops. A task can transit between running and stopping multiple times before becoming quiescent, so the asymmetry can end up subtracting the load of a task which is still running from the cpu's load sum. This patch: - introduces LAVD_TASK_STAT_QUIESCENT and updates transit_task_stat() so that it can handle all valid state transitions including the multiple back and forth transitions between two pairs - QUIESCENT <-> ENQ and RUNNING <-> STOPPING. - restores the symmetry by moving load adjustments part from update_stat_for_stop() to new update_stat_for_quiescent(). This removes a good chunk of ignored transitions. The next patch will take care of the rest.
LAVD_TASK_STAT_ENQ is tracking a subset of runnable task state transitions - the ones which end up calling ops.enqueue(). However, what it is trying to track is a task becoming runnable so that its load can be added to the cpu's load sum. Move the LAVD_TASK_STAT_ENQ state transition and update_stat_for_enq() invocation to ops.runnable() which is called for all runnable transitions. Note that when all the methods are invoked, the invocation order would be ops.select_cpu(), runnable() and then enqueue(). So, this change moves update_stat_for_enq() invocation before calc_when_to_run() for put_global_rq(). update_stat_for_enq() updates taskc->load_actual which is consumed by calc_greedy_ratio() and thus affects calc_when_to_run(). Before this patch, calc_greedy_ratio() would use load_actual which doesn't reflect the last running period. After this patch, the latest running period will be reflected when the task gets queued to the global queue. The difference is unlikely to matter but it'd probably make sense to make it more consistent (e.g. do it at the end of quiescent transition). After this change, transit_task_stat() doesn't detect any invalid transitions.
transit_task_stat() is now tracking the same runnable, running, stopping, quiescent transitions that sched_ext core already tracks and always returns %true. Let's remove it.
multics69
reviewed
Mar 28, 2024
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.
scx_lavd was implementing custom task state transition tracking to avoid updating statistics on invalid transitions. However, sched_ext core already implements clearly defined task state transitions. This PR updates scx_lavd so that it uses sched_ext's task state transitions. Note that the commits are structured so that
transit_task_stat()is verified to always concur with sched_ext state transitions before the custom mechanism is removed.While at it, this PR also removes the unnecessary
extern cratedeclarations.