-
Notifications
You must be signed in to change notification settings - Fork 288
scx_lavd: restrict direct dispatch to usable, uncongested CPUs #3735
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -411,6 +411,34 @@ bool queued_on_cpu(struct cpu_ctx *cpuc) | |
| return false; | ||
| } | ||
|
|
||
| __hidden | ||
| bool is_cpu_congested(struct cpu_ctx *cpuc) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was gazing at the function for a while and comparing it with the queued_on_cpu. When LAVD_CPU_CONGESTED_THRES == 1, it appears to me that the logic is almost the same except for the order of the if condition. I think the function is paving the road for future enhancements for possible longer congestions with different scenarios. There could be a case where a higher-priority task (or any kind of urgency) is chiming in and jumping the queue based on the congestion length. |
||
| { | ||
| int nr; | ||
|
|
||
| nr = scx_bpf_dsq_nr_queued(SCX_DSQ_LOCAL_ON | cpuc->cpu_id); | ||
| if (nr >= LAVD_CPU_CONGESTED_THRES) | ||
| return true; | ||
|
|
||
| if (use_cpdom_dsq()) { | ||
| nr += scx_bpf_dsq_nr_queued(cpdom_to_dsq(cpuc->cpdom_id)); | ||
| if (nr >= LAVD_CPU_CONGESTED_THRES) | ||
| return true; | ||
|
|
||
| nr += scx_bpf_dsq_nr_queued(cpdom_to_turb_dsq(cpuc->cpdom_id)); | ||
| if (nr >= LAVD_CPU_CONGESTED_THRES) | ||
| return true; | ||
| } | ||
|
|
||
| if (use_per_cpu_dsq()) { | ||
| nr += scx_bpf_dsq_nr_queued(cpu_to_dsq(cpuc->cpu_id)); | ||
| if (nr >= LAVD_CPU_CONGESTED_THRES) | ||
| return true; | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| __hidden | ||
| u64 peek_dsq_vtime(u64 dsq_id) | ||
| { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking another case could be worth a revisit—the lock holder condition. It seems that we currently don't have any restriction on the lock holder, which cannot be preempted. If the lock holder runs for a long time, it could impact the tasks waiting in the DSQs. And if it's a real bug (meaning that the lock holder runs indefinitely), it could trigger a soft lockup or other errors. I think I was overthinking the problem. Overall, it looks good to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think avoiding a CPU that holds a lock for direct dispatch makes sense. I will experiment it.