From 1afb7d58356932f93ae3c4f18258428e0cb67bf6 Mon Sep 17 00:00:00 2001 From: Daniel Hodges Date: Fri, 15 Nov 2024 08:53:30 -0800 Subject: [PATCH 1/2] scx_layered: Fix formatting Signed-off-by: Daniel Hodges --- scheds/rust/scx_layered/src/bpf/main.bpf.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scheds/rust/scx_layered/src/bpf/main.bpf.c b/scheds/rust/scx_layered/src/bpf/main.bpf.c index 4c89c6b..320f9d0 100644 --- a/scheds/rust/scx_layered/src/bpf/main.bpf.c +++ b/scheds/rust/scx_layered/src/bpf/main.bpf.c @@ -2631,7 +2631,8 @@ s32 BPF_STRUCT_OPS_SLEEPABLE(layered_init) bpf_for(i, 0, nr_possible_cpus) { const volatile u8 *u8_ptr; - init_antistall_dsq = bpf_map_lookup_percpu_elem(&antistall_cpu_dsq, &zero, i); + init_antistall_dsq = bpf_map_lookup_percpu_elem(&antistall_cpu_dsq, + &zero, i); if (init_antistall_dsq) { *init_antistall_dsq = SCX_DSQ_INVALID; } From d35d5271f5eabf10447143fff2227509202e189c Mon Sep 17 00:00:00 2001 From: Jake Hillion Date: Fri, 15 Nov 2024 21:56:53 +0000 Subject: [PATCH 2/2] layered: split out common parts of LayerKind MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We duplicate the definition of most fields in every layer kind. This makes reading the config harder than it needs to be, and turns every simple read of a common field into a `match` statement that is largely redundant. Utilise `#[serde(flatten)]` to embed a common struct into each of the LayerKind variants. Rather than matching on the type this can be directly accessed with `.kind.common()` and `.kind.common_mut()`. Alternatively, you can extend existing matches to match out the common parts as demonstrated in this diff where necessary. There is some further code cleanup that can be done in the changed read sites, but I wanted to make it clear that this change doesn't change behaviour, so tried to make these changes in the least obtrusive way. Drive-by: fix the formatting of the lazy_static section in main.rs by using `lazy_static::lazy_static`. Test plan: ``` # main $ cargo build --release && target/release/scx_layered --example /tmp/test_old.json # this change $ cargo build --release && target/release/scx_layered --example /tmp/test_new.json $ diff /tmp/test_{old,new}.json # no diff ``` --- scheds/rust/scx_layered/src/config.rs | 148 ++++++------ scheds/rust/scx_layered/src/lib.rs | 1 + scheds/rust/scx_layered/src/main.rs | 313 ++++++++++++-------------- scheds/rust/scx_layered/src/stats.rs | 9 +- 4 files changed, 206 insertions(+), 265 deletions(-) diff --git a/scheds/rust/scx_layered/src/config.rs b/scheds/rust/scx_layered/src/config.rs index 61cbbcb..583e202 100644 --- a/scheds/rust/scx_layered/src/config.rs +++ b/scheds/rust/scx_layered/src/config.rs @@ -41,20 +41,20 @@ impl LayerSpec { Ok(config.specs) } - pub fn nodes(&self) -> Vec { - match &self.kind { - LayerKind::Confined { nodes, .. } - | LayerKind::Open { nodes, .. } - | LayerKind::Grouped { nodes, .. } => nodes.clone(), - } + pub fn nodes(&self) -> &Vec { + &self.kind.common().nodes } - pub fn llcs(&self) -> Vec { - match &self.kind { - LayerKind::Confined { llcs, .. } - | LayerKind::Open { llcs, .. } - | LayerKind::Grouped { llcs, .. } => llcs.clone(), - } + pub fn llcs(&self) -> &Vec { + &self.kind.common().llcs + } + + pub fn nodes_mut(&mut self) -> &mut Vec { + &mut self.kind.common_mut().nodes + } + + pub fn llcs_mut(&mut self) -> &mut Vec { + &mut self.kind.common_mut().llcs } } @@ -73,91 +73,55 @@ pub enum LayerMatch { TGIDEquals(u32), } +#[derive(Clone, Debug, Serialize, Deserialize)] +pub struct LayerCommon { + #[serde(default)] + pub min_exec_us: u64, + #[serde(default)] + pub yield_ignore: f64, + #[serde(default)] + pub slice_us: u64, + #[serde(default)] + pub preempt: bool, + #[serde(default)] + pub preempt_first: bool, + #[serde(default)] + pub exclusive: bool, + #[serde(default)] + pub weight: u32, + #[serde(default)] + pub idle_smt: bool, + #[serde(default)] + pub growth_algo: LayerGrowthAlgo, + #[serde(default)] + pub perf: u64, + #[serde(default)] + pub nodes: Vec, + #[serde(default)] + pub llcs: Vec, +} + #[derive(Clone, Debug, Serialize, Deserialize)] pub enum LayerKind { Confined { util_range: (f64, f64), #[serde(default)] cpus_range: Option<(usize, usize)>, - #[serde(default)] - min_exec_us: u64, - #[serde(default)] - yield_ignore: f64, - #[serde(default)] - slice_us: u64, - #[serde(default)] - preempt: bool, - #[serde(default)] - preempt_first: bool, - #[serde(default)] - exclusive: bool, - #[serde(default)] - weight: u32, - #[serde(default)] - idle_smt: bool, - #[serde(default)] - growth_algo: LayerGrowthAlgo, - #[serde(default)] - perf: u64, - #[serde(default)] - nodes: Vec, - #[serde(default)] - llcs: Vec, + + #[serde(flatten)] + common: LayerCommon, }, Grouped { util_range: (f64, f64), #[serde(default)] cpus_range: Option<(usize, usize)>, - #[serde(default)] - min_exec_us: u64, - #[serde(default)] - yield_ignore: f64, - #[serde(default)] - slice_us: u64, - #[serde(default)] - preempt: bool, - #[serde(default)] - preempt_first: bool, - #[serde(default)] - exclusive: bool, - #[serde(default)] - weight: u32, - #[serde(default)] - idle_smt: bool, - #[serde(default)] - growth_algo: LayerGrowthAlgo, - #[serde(default)] - perf: u64, - #[serde(default)] - nodes: Vec, - #[serde(default)] - llcs: Vec, + + #[serde(flatten)] + common: LayerCommon, }, Open { - #[serde(default)] - min_exec_us: u64, - #[serde(default)] - yield_ignore: f64, - #[serde(default)] - slice_us: u64, - #[serde(default)] - preempt: bool, - #[serde(default)] - preempt_first: bool, - #[serde(default)] - exclusive: bool, - #[serde(default)] - weight: u32, - #[serde(default)] - idle_smt: bool, - #[serde(default)] - growth_algo: LayerGrowthAlgo, - #[serde(default)] - perf: u64, - #[serde(default)] - nodes: Vec, - #[serde(default)] - llcs: Vec, + #[serde(flatten)] + common: LayerCommon, }, } @@ -169,4 +133,20 @@ impl LayerKind { LayerKind::Open { .. } => bpf_intf::layer_kind_LAYER_KIND_OPEN as i32, } } + + pub fn common(&self) -> &LayerCommon { + match self { + LayerKind::Confined { common, .. } + | LayerKind::Grouped { common, .. } + | LayerKind::Open { common, .. } => common, + } + } + + pub fn common_mut(&mut self) -> &mut LayerCommon { + match self { + LayerKind::Confined { common, .. } + | LayerKind::Grouped { common, .. } + | LayerKind::Open { common, .. } => common, + } + } } diff --git a/scheds/rust/scx_layered/src/lib.rs b/scheds/rust/scx_layered/src/lib.rs index b040cc4..eb6a8fe 100644 --- a/scheds/rust/scx_layered/src/lib.rs +++ b/scheds/rust/scx_layered/src/lib.rs @@ -12,6 +12,7 @@ use std::collections::BTreeMap; use anyhow::bail; use anyhow::Result; use bitvec::prelude::*; +pub use config::LayerCommon; pub use config::LayerConfig; pub use config::LayerKind; pub use config::LayerMatch; diff --git a/scheds/rust/scx_layered/src/main.rs b/scheds/rust/scx_layered/src/main.rs index 43255e8..99a3143 100644 --- a/scheds/rust/scx_layered/src/main.rs +++ b/scheds/rust/scx_layered/src/main.rs @@ -28,6 +28,7 @@ use bitvec::prelude::*; pub use bpf_skel::*; use clap::Parser; use crossbeam::channel::RecvTimeoutError; +use lazy_static::lazy_static; use libbpf_rs::skel::OpenSkel; use libbpf_rs::skel::Skel; use libbpf_rs::skel::SkelBuilder; @@ -74,108 +75,112 @@ const NR_LSTATS: usize = bpf_intf::layer_stat_idx_NR_LSTATS as usize; const NR_LAYER_MATCH_KINDS: usize = bpf_intf::layer_match_kind_NR_LAYER_MATCH_KINDS as usize; const MAX_LAYER_NAME: usize = bpf_intf::consts_MAX_LAYER_NAME as usize; -#[rustfmt::skip] -lazy_static::lazy_static! { +lazy_static! { static ref NR_POSSIBLE_CPUS: usize = libbpf_rs::num_possible_cpus().unwrap(); static ref USAGE_DECAY: f64 = 0.5f64.powf(1.0 / USAGE_HALF_LIFE_F64); - static ref EXAMPLE_CONFIG: LayerConfig = - LayerConfig { - specs: vec![ - LayerSpec { - name: "batch".into(), - comment: Some("tasks under system.slice or tasks with nice value > 0".into()), - matches: vec![ - vec![LayerMatch::CgroupPrefix("system.slice/".into())], - vec![LayerMatch::NiceAbove(0)], - ], - kind: LayerKind::Confined { - cpus_range: Some((0, 16)), - util_range: (0.8, 0.9), - min_exec_us: 1000, - yield_ignore: 0.0, - preempt: false, - preempt_first: false, - exclusive: false, - idle_smt: false, + static ref EXAMPLE_CONFIG: LayerConfig = LayerConfig { + specs: vec![ + LayerSpec { + name: "batch".into(), + comment: Some("tasks under system.slice or tasks with nice value > 0".into()), + matches: vec![ + vec![LayerMatch::CgroupPrefix("system.slice/".into())], + vec![LayerMatch::NiceAbove(0)], + ], + kind: LayerKind::Confined { + util_range: (0.8, 0.9), + cpus_range: Some((0, 16)), + common: LayerCommon { + min_exec_us: 1000, + yield_ignore: 0.0, + preempt: false, + preempt_first: false, + exclusive: false, + idle_smt: false, slice_us: 20000, weight: DEFAULT_LAYER_WEIGHT, growth_algo: LayerGrowthAlgo::Sticky, - perf: 1024, - nodes: vec![], - llcs: vec![], + perf: 1024, + nodes: vec![], + llcs: vec![], }, - }, - LayerSpec { - name: "immediate".into(), - comment: Some("tasks under workload.slice with nice value < 0".into()), - matches: vec![vec![ - LayerMatch::CgroupPrefix("workload.slice/".into()), - LayerMatch::NiceBelow(0), - ]], - kind: LayerKind::Open { - min_exec_us: 100, - yield_ignore: 0.25, - preempt: true, - preempt_first: false, - exclusive: true, - idle_smt: false, + }, + }, + LayerSpec { + name: "immediate".into(), + comment: Some("tasks under workload.slice with nice value < 0".into()), + matches: vec![vec![ + LayerMatch::CgroupPrefix("workload.slice/".into()), + LayerMatch::NiceBelow(0), + ]], + kind: LayerKind::Open { + common: LayerCommon { + min_exec_us: 100, + yield_ignore: 0.25, + preempt: true, + preempt_first: false, + exclusive: true, + idle_smt: false, slice_us: 20000, weight: DEFAULT_LAYER_WEIGHT, growth_algo: LayerGrowthAlgo::Sticky, - perf: 1024, - nodes: vec![], - llcs: vec![], + perf: 1024, + nodes: vec![], + llcs: vec![], }, - }, - LayerSpec { - name: "stress-ng".into(), - comment: Some("stress-ng test layer".into()), - matches: vec![vec![ - LayerMatch::CommPrefix("stress-ng".into()), - ], - vec![ - LayerMatch::PcommPrefix("stress-ng".into()), - ]], - kind: LayerKind::Confined { - cpus_range: None, - min_exec_us: 800, - yield_ignore: 0.0, - util_range: (0.2, 0.8), - preempt: true, - preempt_first: false, - exclusive: false, - idle_smt: false, + }, + }, + LayerSpec { + name: "stress-ng".into(), + comment: Some("stress-ng test layer".into()), + matches: vec![ + vec![LayerMatch::CommPrefix("stress-ng".into()),], + vec![LayerMatch::PcommPrefix("stress-ng".into()),] + ], + kind: LayerKind::Confined { + cpus_range: None, + util_range: (0.2, 0.8), + common: LayerCommon { + min_exec_us: 800, + yield_ignore: 0.0, + preempt: true, + preempt_first: false, + exclusive: false, + idle_smt: false, slice_us: 800, weight: DEFAULT_LAYER_WEIGHT, growth_algo: LayerGrowthAlgo::Topo, - perf: 1024, - nodes: vec![], - llcs: vec![], + perf: 1024, + nodes: vec![], + llcs: vec![], }, - }, - LayerSpec { - name: "normal".into(), - comment: Some("the rest".into()), - matches: vec![vec![]], - kind: LayerKind::Grouped { - cpus_range: None, - util_range: (0.5, 0.6), - min_exec_us: 200, - yield_ignore: 0.0, - preempt: false, - preempt_first: false, - exclusive: false, - idle_smt: false, + }, + }, + LayerSpec { + name: "normal".into(), + comment: Some("the rest".into()), + matches: vec![vec![]], + kind: LayerKind::Grouped { + cpus_range: None, + util_range: (0.5, 0.6), + common: LayerCommon { + min_exec_us: 200, + yield_ignore: 0.0, + preempt: false, + preempt_first: false, + exclusive: false, + idle_smt: false, slice_us: 20000, weight: DEFAULT_LAYER_WEIGHT, growth_algo: LayerGrowthAlgo::Linear, - perf: 1024, - nodes: vec![], - llcs: vec![], + perf: 1024, + nodes: vec![], + llcs: vec![], }, - }, - ], - }; + }, + }, + ], + }; } /// scx_layered: A highly configurable multi-layer sched_ext scheduler @@ -923,8 +928,7 @@ impl Layer { LayerKind::Confined { cpus_range, util_range, - nodes, - llcs, + common: LayerCommon { nodes, llcs, .. }, .. } => { let cpus_range = cpus_range.unwrap_or((0, std::usize::MAX)); @@ -962,7 +966,14 @@ impl Layer { bail!("invalid util_range {:?}", util_range); } } - LayerKind::Grouped { nodes, llcs, .. } | LayerKind::Open { nodes, llcs, .. } => { + LayerKind::Grouped { + common: LayerCommon { nodes, llcs, .. }, + .. + } + | LayerKind::Open { + common: LayerCommon { nodes, llcs, .. }, + .. + } => { if nodes.len() == 0 && llcs.len() == 0 { allowed_cpus.fill(true); } else { @@ -987,23 +998,13 @@ impl Layer { } } - let layer_growth_algo = match &kind { - LayerKind::Confined { growth_algo, .. } - | LayerKind::Grouped { growth_algo, .. } - | LayerKind::Open { growth_algo, .. } => growth_algo.clone(), - }; - let preempt = match &kind { - LayerKind::Confined { preempt, .. } - | LayerKind::Grouped { preempt, .. } - | LayerKind::Open { preempt, .. } => preempt.clone(), - }; + let layer_growth_algo = kind.common().growth_algo.clone(); + let preempt = kind.common().preempt; let core_order = layer_growth_algo.layer_core_order(cpu_pool, spec, idx, topo); debug!( "layer: {} algo: {:?} core order: {:?}", - name, - layer_growth_algo.clone(), - core_order + name, &layer_growth_algo, core_order ); Ok(Self { @@ -1289,8 +1290,8 @@ impl<'a> Scheduler<'a> { layer.nr_match_ors = spec.matches.len() as u32; layer.kind = spec.kind.as_bpf_enum(); - match &spec.kind { - LayerKind::Confined { + { + let LayerCommon { min_exec_us, yield_ignore, perf, @@ -1303,70 +1304,42 @@ impl<'a> Scheduler<'a> { slice_us, weight, .. - } - | LayerKind::Grouped { - min_exec_us, - yield_ignore, - perf, - preempt, - preempt_first, - exclusive, - idle_smt, - growth_algo, - nodes, - slice_us, - weight, - .. - } - | LayerKind::Open { - min_exec_us, - yield_ignore, - perf, - preempt, - preempt_first, - exclusive, - idle_smt, - growth_algo, - nodes, - slice_us, - weight, - .. - } => { - layer.slice_ns = if *slice_us > 0 { - *slice_us * 1000 - } else { - opts.slice_us * 1000 - }; - layer.min_exec_ns = min_exec_us * 1000; - layer.yield_step_ns = if *yield_ignore > 0.999 { - 0 - } else if *yield_ignore < 0.001 { - layer.slice_ns - } else { - (layer.slice_ns as f64 * (1.0 - *yield_ignore)) as u64 - }; - let mut layer_name: String = spec.name.clone(); - layer_name.truncate(MAX_LAYER_NAME); - copy_into_cstr(&mut layer.name, layer_name.as_str()); - layer.preempt.write(*preempt); - layer.preempt_first.write(*preempt_first); - layer.exclusive.write(*exclusive); - layer.idle_smt.write(*idle_smt); - layer.growth_algo = growth_algo.as_bpf_enum(); - layer.weight = if *weight <= MAX_LAYER_WEIGHT && *weight >= MIN_LAYER_WEIGHT { - *weight - } else { - DEFAULT_LAYER_WEIGHT - }; - layer_weights.push(layer.weight.try_into().unwrap()); - layer.perf = u32::try_from(*perf)?; - layer.node_mask = nodemask_from_nodes(nodes) as u64; - for topo_node in topo.nodes() { - if !nodes.contains(&topo_node.id()) { - continue; - } - layer.cache_mask |= cachemask_from_llcs(&topo_node.llcs()) as u64; + } = spec.kind.common(); + + layer.slice_ns = if *slice_us > 0 { + *slice_us * 1000 + } else { + opts.slice_us * 1000 + }; + layer.min_exec_ns = min_exec_us * 1000; + layer.yield_step_ns = if *yield_ignore > 0.999 { + 0 + } else if *yield_ignore < 0.001 { + layer.slice_ns + } else { + (layer.slice_ns as f64 * (1.0 - *yield_ignore)) as u64 + }; + let mut layer_name: String = spec.name.clone(); + layer_name.truncate(MAX_LAYER_NAME); + copy_into_cstr(&mut layer.name, layer_name.as_str()); + layer.preempt.write(*preempt); + layer.preempt_first.write(*preempt_first); + layer.exclusive.write(*exclusive); + layer.idle_smt.write(*idle_smt); + layer.growth_algo = growth_algo.as_bpf_enum(); + layer.weight = if *weight <= MAX_LAYER_WEIGHT && *weight >= MIN_LAYER_WEIGHT { + *weight + } else { + DEFAULT_LAYER_WEIGHT + }; + layer_weights.push(layer.weight.try_into().unwrap()); + layer.perf = u32::try_from(*perf)?; + layer.node_mask = nodemask_from_nodes(nodes) as u64; + for topo_node in topo.nodes() { + if !nodes.contains(&topo_node.id()) { + continue; } + layer.cache_mask |= cachemask_from_llcs(&topo_node.llcs()) as u64; } } @@ -1449,14 +1422,8 @@ impl<'a> Scheduler<'a> { .into_iter() .cloned() .map(|mut s| { - match &mut s.kind { - LayerKind::Confined { nodes, llcs, .. } - | LayerKind::Open { nodes, llcs, .. } - | LayerKind::Grouped { nodes, llcs, .. } => { - nodes.truncate(0); - llcs.truncate(0); - } - }; + s.kind.common_mut().nodes.clear(); + s.kind.common_mut().llcs.clear(); s }) .collect() diff --git a/scheds/rust/scx_layered/src/stats.rs b/scheds/rust/scx_layered/src/stats.rs index 9eaf4c4..3388bde 100644 --- a/scheds/rust/scx_layered/src/stats.rs +++ b/scheds/rust/scx_layered/src/stats.rs @@ -25,7 +25,6 @@ use serde::Serialize; use crate::bpf_intf; use crate::BpfStats; use crate::Layer; -use crate::LayerKind; use crate::Stats; fn fmt_pct(v: f64) -> String { @@ -174,12 +173,6 @@ impl LayerStats { if b != 0.0 { a / b * 100.0 } else { 0.0 } }; - let is_excl = match &layer.kind { - LayerKind::Confined { exclusive, .. } - | LayerKind::Grouped { exclusive, .. } - | LayerKind::Open { exclusive, .. } => *exclusive, - } as u32; - Self { util: stats.layer_utils[lidx] * 100.0, util_frac: calc_frac(stats.layer_utils[lidx], stats.total_util), @@ -206,7 +199,7 @@ impl LayerStats { keep: lstat_pct(bpf_intf::layer_stat_idx_LSTAT_KEEP), keep_fail_max_exec: lstat_pct(bpf_intf::layer_stat_idx_LSTAT_KEEP_FAIL_MAX_EXEC), keep_fail_busy: lstat_pct(bpf_intf::layer_stat_idx_LSTAT_KEEP_FAIL_BUSY), - is_excl, + is_excl: layer.kind.common().exclusive as u32, excl_collision: lstat_pct(bpf_intf::layer_stat_idx_LSTAT_EXCL_COLLISION), excl_preempt: lstat_pct(bpf_intf::layer_stat_idx_LSTAT_EXCL_PREEMPT), kick: lstat_pct(bpf_intf::layer_stat_idx_LSTAT_KICK),