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.

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

scx_rustland: lowlatency improvements #59

Merged
merged 4 commits into from
Dec 31, 2023
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions scheds/rust/scx_rustland/src/bpf/main.bpf.c
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ s32 BPF_STRUCT_OPS(rustland_select_cpu, struct task_struct *p, s32 prev_cpu,
* .select_cpu()), since this function may be called on a different CPU (so we
* cannot check the current CPU directly).
*/
static bool is_task_cpu_available(struct task_struct *p)
static bool is_task_cpu_available(struct task_struct *p, u64 enq_flags)
{
struct task_ctx *tctx;

Expand All @@ -322,6 +322,12 @@ static bool is_task_cpu_available(struct task_struct *p)
if (is_kthread(p) && p->nr_cpus_allowed == 1)
return true;

/*
* No scheduling required if it's the last task running.
*/
if (enq_flags & SCX_ENQ_LAST)
return true;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't quite understand what difference this commit makes given that the default behavior w/o SCX_ENQ_LAST is auto-enq on local dsq. Does this make any behavior difference?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't quite understand what difference this commit makes given that the default behavior w/o SCX_ENQ_LAST is auto-enq on local dsq. Does this make any behavior difference?

For some reasons with this in place the CPU utilization of scx_rustland drops significantly when the system is idle (like 0.3-0.5%), without this change when the system is idle I can see some cpu usage spikes of up to 5-10%,but I'm not sure why it's happening... My theory was that with SCX_ENQ_LAST in place we could save some unnecessary usersched invocations.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's surprising given that the two essentially are doing the same thing. Maybe there's something timing dependent going on or I'm just confused.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's surprising given that the two essentially are doing the same thing. Maybe there's something timing dependent going on or I'm just confused.

I'll run some tests with and without that and collect more info, now I want to understand what's going on exactly :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@htejun it's SCX_OPS_ENQ_LAST set in sched_ext_ops.flags that seems to make a difference, the check in is_task_cpu_available() does't make any difference and can be dropped apparently.


/*
* For regular tasks always rely on force_local to determine if we can
* bypass the scheduler.
Expand Down Expand Up @@ -365,7 +371,7 @@ void BPF_STRUCT_OPS(rustland_enqueue, struct task_struct *p, u64 enq_flags)
* Dispatch the task on the local FIFO directly if the selected task's
* CPU is available (no scheduling decision required).
*/
if (is_task_cpu_available(p)) {
if (is_task_cpu_available(p, enq_flags)) {
dispatch_local(p, enq_flags);
__sync_fetch_and_add(&nr_kernel_dispatches, 1);
return;
Expand Down Expand Up @@ -626,6 +632,7 @@ struct sched_ext_ops rustland = {
.prep_enable = (void *)rustland_prep_enable,
.init = (void *)rustland_init,
.exit = (void *)rustland_exit,
.flags = SCX_OPS_ENQ_LAST,
.timeout_ms = 5000,
.name = "rustland",
};