Skip to content
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.

Sign up for GitHub

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

Jump to bottom

scx_lavd: addressed comments from PR #192 #194

Merged
merged 18 commits into from
Mar 19, 2024
Merged

scx_lavd: addressed comments from PR #192 #194

merged 18 commits into from
Mar 19, 2024

Conversation

multics69
Copy link
Contributor

This PR contains all the fixes for the suggested changes in PR #192. It includes the following changes:

  • scx_lavd: fix formatting issues in main.rs and main.bpf.c
  • scx_lavd: replace num_cpus to scx_utils::Topology
  • scx_lavd: remove unnecessary arg from put_local_rq()
  • scx_lavd: print one scheduling decision by default
  • scs_lavd: improve the description of fairness
  • scx_lavd: remove unnecessary condition check in is_wakeup_wf()
  • scx_lavd: add a utility func, {try_}get_task_ctx()
  • scx_lavd: move scx_bpf_error() calls to get_cpu_ctx{_id}()
  • scx_lavd: improve the clarity of the task state transition
  • scx_lavd: use is_wakeup_ef() in checking wait flag
  • scx_lavd: remove unnecessary condition check at update_stat_for_stop()
  • scx_lavd: remove unnecessary condition check at slice_fully_consumed()
  • scx_lavd: remove redundant latency calculcation at calc_latency_weight()
  • scx_lavd: fix potential CPU stall in lavd_select_cpu()

Changwoo Min added 14 commits March 19, 2024 00:30
scx_lavd: fix formatting issues in main.rs and main.bpf.c
17bce16 
Signed-off-by: Changwoo Min <changwoo@igalia.com>
scx_lavd: replace num_cpus to scx_utils::Topology
ead7d55 
This removes the external carte depenendy and avoides the known bugs
in the num_cpus carte.

Signed-off-by: Changwoo Min <changwoo@igalia.com>
scx_lavd: remove unnecessary arg from put_local_rq()
315e5b3 
cpu_id is unused and not necessary in pu_local_rq(), so it it removed.

Signed-off-by: Changwoo Min <changwoo@igalia.com>
scx_lavd: print one scheduling decision by default
670c1b5 
Signed-off-by: Changwoo Min <changwoo@igalia.com>
scs_lavd: improve the description of fairness
47e7238 
Signed-off-by: Changwoo Min <changwoo@igalia.com>
scx_lavd: remove unnecessary condition check in is_wakeup_wf()
97b4d9c 
We don't need to test SCX_WAKE_SYNC because SCX_WAKE_SYNC should only be
set when SCX_WAKE_TTWU is set.

Signed-off-by: Changwoo Min <changwoo@igalia.com>
scx_lavd: add a utility func, {try_}get_task_ctx()
0fc5591 
get_task_ctx() and try_get_task_ctx() were added for common error
handling for task lookup failure. Since idle "swapper" task is not under
sched_ext, try_get_task_ctx() is added for the case such that idle task
can be searched.

Signed-off-by: Changwoo Min <changwoo@igalia.com>
scx_lavd: move scx_bpf_error() calls to get_cpu_ctx{_id}()
66e1528 
Signed-off-by: Changwoo Min <changwoo@igalia.com>
scx_lavd: improve the clarity of the task state transition
419ccae 
Signed-off-by: Changwoo Min <changwoo@igalia.com>
sdx_lavd: use is_wakeup_ef() in checking wait flag
a27b509 
Signed-off-by: Changwoo Min <changwoo@igalia.com>
scx_lavd: remove unnecessary condition check at update_stat_for_stop()
c2b1a10 
Signed-off-by: Changwoo Min <changwoo@igalia.com>
scx_lavd: remove unnecessary condition check at slice_fully_consumed()
865269f 
Signed-off-by: Changwoo Min <changwoo@igalia.com>
scx_lavd: remove redundant latency calculation at calc_latency_weight()
e41c674 
Signed-off-by: Changwoo Min <changwoo@igalia.com>
scx_lavd: fix potential CPU stall in lavd_select_cpu()
512c4e7 
Returning prev_cpu after picking an idle CPU will cause the idle CPU
stall because the idle core was already punched out from the idle mask
by the scx core so it is no longer idle from the scx core's point of
view.

This fix conducts the idle core selection at the last step so it never
return prev_cpu after picking the idle core.

Signed-off-by: Changwoo Min <changwoo@igalia.com>
htejun
htejun approved these changes Mar 18, 2024
View reviewed changes
Copy link
Contributor

@htejun htejun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. Thanks.

Changwoo Min added 4 commits March 19, 2024 21:19
scx_lavd: fix a typo
24bddad 
Signed-off-by: Changwoo Min <changwoo@igalia.com>
scx_lavd: more explanation about sched_prio_to_latency_weight
fa22823 
Signed-off-by: Changwoo Min <changwoo@igalia.com>
scx_lavd: add a sanity check if runtime is negative
8860f26 
Signed-off-by: Changwoo Min <changwoo@igalia.com>
scx_lavd: entail two invalid task state transitions
b37370b 
Signed-off-by: Changwoo Min <changwoo@igalia.com>
@multics69
Copy link
Contributor Author

@htejun I addressed all your additional comments. Please take a look and if everything is OK, please merge it.
(BTW, I cannot merge by myself since I don't have a write permission).

@htejun htejun merged commit 0e53e7a into sched-ext:main Mar 19, 2024
1 check passed
@htejun
Copy link
Contributor

htejun commented Mar 19, 2024

I merged it but you should have a pending invite for write perm.

@multics69 multics69 deleted the pr192-comments branch March 19, 2024 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@htejun htejun htejun approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@multics69 @htejun