Skip to content

scx_p2dq: Prefer dispatching into local CPU DSQs #2307

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+29 −2
Open

scx_p2dq: Prefer dispatching into local CPU DSQs #2307

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions scheds/rust/scx_p2dq/src/bpf/intf.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ enum stat_idx {
P2DQ_STAT_LLC_MIGRATION,
P2DQ_STAT_NODE_MIGRATION,
P2DQ_STAT_KEEP,
P2DQ_STAT_ENQ_CPU,
P2DQ_STAT_ENQ_LLC,
P2DQ_STAT_ENQ_INTR,
P2DQ_STAT_ENQ_MIG,
Expand Down
19 changes: 18 additions & 1 deletion scheds/rust/scx_p2dq/src/bpf/main.bpf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -870,7 +870,6 @@ static __always_inline void async_p2dq_enqueue(struct enqueue_promise *ret,
ret->kind = P2DQ_ENQUEUE_PROMISE_COMPLETE;
return;
}

if (cpuc->nice_task)
enq_flags |= SCX_ENQ_PREEMPT;

Expand All @@ -890,6 +889,21 @@ static __always_inline void async_p2dq_enqueue(struct enqueue_promise *ret,
return;
}

// If the last CPU is running an interactive task then reenque to the
// CPUs local DSQ. This should reduce the number of migrations.
if (taskc->last_cpu_dsq_id != SCX_DSQ_INVALID &&
taskc->interactive &&
scx_bpf_dsq_nr_queued(taskc->last_cpu_dsq_id) == 0 &&
scx_bpf_dsq_nr_queued(SCX_DSQ_LOCAL_ON|taskc->last_cpu) == 0) {
ret->kind = P2DQ_ENQUEUE_PROMISE_VTIME;
ret->vtime.dsq_id = taskc->last_cpu_dsq_id;
ret->vtime.enq_flags = enq_flags;
ret->vtime.slice_ns = taskc->slice_ns;
ret->vtime.vtime = p->scx.dsq_vtime;
stat_inc(P2DQ_STAT_ENQ_CPU);
return;
}

bool migrate = can_migrate(taskc, llcx);
if (interactive_dsq && taskc->interactive && !migrate) {
taskc->dsq_id = llcx->intr_dsq;
Expand Down Expand Up @@ -967,6 +981,8 @@ static __always_inline int p2dq_running_impl(struct task_struct *p)
taskc->llc_id = llcx->id;
taskc->node_id = llcx->node_id;
taskc->was_nice = p->scx.weight < 100;
taskc->last_cpu = task_cpu;
taskc->last_cpu_dsq_id = cpuc->affn_dsq;
cpuc->interactive = taskc->interactive;
cpuc->dsq_index = taskc->dsq_index;
cpuc->nice_task = p->scx.weight < 100;
Expand Down Expand Up @@ -1335,6 +1351,7 @@ static __always_inline s32 p2dq_init_task_impl(struct task_struct *p,
} else if (p->scx.weight > 100) {
taskc->dsq_index = nr_dsqs_per_llc - 1;
}
taskc->last_cpu_dsq_id = SCX_DSQ_INVALID;
taskc->last_dsq_index = taskc->dsq_index;
taskc->slice_ns = slice_ns;
taskc->all_cpus = p->cpus_ptr == &p->cpus_mask &&
Expand Down
3 changes: 3 additions & 0 deletions scheds/rust/scx_p2dq/src/bpf/types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ struct p2dq_timer {
struct cpu_ctx {
int id;
u32 llc_id;
u64 dsq_id;
u64 affn_dsq;
u32 dsq_index;
u64 slice_ns;
Expand Down Expand Up @@ -61,10 +62,12 @@ struct task_p2dq {
u32 node_id;
u64 used;
u64 last_dsq_id;
u64 last_cpu_dsq_id;
u64 last_run_started;
u64 last_run_at;
u64 llc_runs; /* how many runs on the current LLC */
int last_dsq_index;
s32 last_cpu;
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you saving last_cpu in the task context for efficiency reasons? Why not using scx_bpf_task_cpu(p) directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was roughly my thinking: If a task finds a different CPU in select that isn't idle it won't get direct dispatched and then hit the enqueue path. However, if the tasks old CPU is idle then it's probably the best CPU to use. Does scx_bpf_task_cpu(p) return the previous CPU in enqueue if a CPU has been selected in select?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nope, it returns the CPU selected in ops.select_cpu(). But probably in your case it doesn't make any difference, because if the task is not directly dispatched you return prev_cpu from ops.select_cpu(), which is the previous running CPU.

However, if ops.select_cpu() can return a different CPU without doing direct dispatch you're going to ignore this hint and use the prev running CPU anyway.

bool interactive;
bool was_nice;

Expand Down
2 changes: 2 additions & 0 deletions scheds/rust/scx_p2dq/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ use bpf_intf::stat_idx_P2DQ_STAT_DIRECT;
use bpf_intf::stat_idx_P2DQ_STAT_DISPATCH_PICK2;
use bpf_intf::stat_idx_P2DQ_STAT_DSQ_CHANGE;
use bpf_intf::stat_idx_P2DQ_STAT_DSQ_SAME;
use bpf_intf::stat_idx_P2DQ_STAT_ENQ_CPU;
use bpf_intf::stat_idx_P2DQ_STAT_ENQ_INTR;
use bpf_intf::stat_idx_P2DQ_STAT_ENQ_LLC;
use bpf_intf::stat_idx_P2DQ_STAT_ENQ_MIG;
Expand Down Expand Up @@ -152,6 +153,7 @@ impl<'a> Scheduler<'a> {
dsq_change: stats[stat_idx_P2DQ_STAT_DSQ_CHANGE as usize],
same_dsq: stats[stat_idx_P2DQ_STAT_DSQ_SAME as usize],
keep: stats[stat_idx_P2DQ_STAT_KEEP as usize],
enq_cpu: stats[stat_idx_P2DQ_STAT_ENQ_CPU as usize],
enq_llc: stats[stat_idx_P2DQ_STAT_ENQ_LLC as usize],
enq_intr: stats[stat_idx_P2DQ_STAT_ENQ_INTR as usize],
enq_mig: stats[stat_idx_P2DQ_STAT_ENQ_MIG as usize],
Expand Down
6 changes: 5 additions & 1 deletion scheds/rust/scx_p2dq/src/stats.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ pub struct Metrics {
pub same_dsq: u64,
#[stat(desc = "Number of times a task kept running")]
pub keep: u64,
#[stat(desc = "Number of times a task was enqueued to CPU DSQ")]
pub enq_cpu: u64,
#[stat(desc = "Number of times a task was enqueued to LLC DSQ")]
pub enq_llc: u64,
#[stat(desc = "Number of times a task was enqueued to interactive DSQ")]
Expand Down Expand Up @@ -53,12 +55,13 @@ impl Metrics {
fn format<W: Write>(&self, w: &mut W) -> Result<()> {
writeln!(
w,
"direct/idle/keep {}/{}/{}\n\tdsq same/migrate {}/{}\n\tenq llc/intr/mig {}/{}/{}",
"direct/idle/keep {}/{}/{}\n\tdsq same/migrate {}/{}\n\tenq cpu/llc/intr/mig {}/{}/{}/{}",
self.direct,
self.idle,
self.keep,
self.same_dsq,
self.dsq_change,
self.enq_cpu,
self.enq_llc,
self.enq_intr,
self.enq_mig,
Expand All @@ -84,6 +87,7 @@ impl Metrics {
dsq_change: self.dsq_change - rhs.dsq_change,
same_dsq: self.same_dsq - rhs.same_dsq,
keep: self.keep - rhs.keep,
enq_cpu: self.enq_cpu - rhs.enq_cpu,
enq_llc: self.enq_llc - rhs.enq_llc,
enq_intr: self.enq_intr - rhs.enq_intr,
enq_mig: self.enq_mig - rhs.enq_mig,
Expand Down