From c4724c22e5a3edf97e5adb3176888c7fda447a8f Mon Sep 17 00:00:00 2001 From: alxndrv <> Date: Sun, 16 Feb 2025 18:52:42 +0200 Subject: [PATCH 01/11] `chcpu`: Create subcommand crate --- Cargo.lock | 9 +++++++++ Cargo.toml | 2 ++ src/uu/chcpu/Cargo.toml | 15 +++++++++++++++ src/uu/chcpu/src/chcpu.rs | 19 +++++++++++++++++++ src/uu/chcpu/src/main.rs | 1 + 5 files changed, 46 insertions(+) create mode 100644 src/uu/chcpu/Cargo.toml create mode 100644 src/uu/chcpu/src/chcpu.rs create mode 100644 src/uu/chcpu/src/main.rs diff --git a/Cargo.lock b/Cargo.lock index d7d32f5d..6a623c40 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -969,6 +969,7 @@ dependencies = [ "tempfile", "textwrap", "uu_blockdev", + "uu_chcpu", "uu_ctrlaltdel", "uu_dmesg", "uu_fsfreeze", @@ -995,6 +996,14 @@ dependencies = [ "uucore", ] +[[package]] +name = "uu_chcpu" +version = "0.0.1" +dependencies = [ + "clap", + "uucore", +] + [[package]] name = "uu_ctrlaltdel" version = "0.0.1" diff --git a/Cargo.toml b/Cargo.toml index e4242c5f..a08d3e00 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,6 +27,7 @@ uudoc = [] feat_common_core = [ "blockdev", + "chcpu", "ctrlaltdel", "dmesg", "fsfreeze", @@ -73,6 +74,7 @@ uucore = { workspace = true } # blockdev = { optional = true, version = "0.0.1", package = "uu_blockdev", path = "src/uu/blockdev" } +chcpu = { optional = true, version = "0.0.1", package = "uu_chcpu", path = "src/uu/chcpu" } ctrlaltdel = { optional = true, version = "0.0.1", package = "uu_ctrlaltdel", path = "src/uu/ctrlaltdel" } dmesg = { optional = true, version = "0.0.1", package = "uu_dmesg", path = "src/uu/dmesg" } fsfreeze = { optional = true, version = "0.0.1", package = "uu_fsfreeze", path = "src/uu/fsfreeze" } diff --git a/src/uu/chcpu/Cargo.toml b/src/uu/chcpu/Cargo.toml new file mode 100644 index 00000000..f62c1843 --- /dev/null +++ b/src/uu/chcpu/Cargo.toml @@ -0,0 +1,15 @@ +[package] +name = "uu_chcpu" +version = "0.0.1" +edition = "2021" + +[lib] +path = "src/chcpu.rs" + +[[bin]] +name = "chcpu" +path = "src/main.rs" + +[dependencies] +uucore = { workspace = true } +clap = { workspace = true } diff --git a/src/uu/chcpu/src/chcpu.rs b/src/uu/chcpu/src/chcpu.rs new file mode 100644 index 00000000..968f8548 --- /dev/null +++ b/src/uu/chcpu/src/chcpu.rs @@ -0,0 +1,19 @@ +// This file is part of the uutils util-linux package. +// +// For the full copyright and license information, please view the LICENSE +// file that was distributed with this source code. + +use clap::{crate_version, Command}; +use uucore::error::UResult; + +#[uucore::main] +pub fn uumain(_args: impl uucore::Args) -> UResult<()> { + println!("chcpu: Hello world"); + Ok(()) +} + +pub fn uu_app() -> Command { + Command::new(uucore::util_name()) + .version(crate_version!()) + .infer_long_args(true) +} diff --git a/src/uu/chcpu/src/main.rs b/src/uu/chcpu/src/main.rs new file mode 100644 index 00000000..fe72ddc7 --- /dev/null +++ b/src/uu/chcpu/src/main.rs @@ -0,0 +1 @@ +uucore::bin!(uu_chcpu); From e685d8f49bca2759eccd4911766d22f07059d614 Mon Sep 17 00:00:00 2001 From: alxndrv <> Date: Sun, 16 Feb 2025 19:18:38 +0200 Subject: [PATCH 02/11] `chcpu`: Set up core CLI options --- src/uu/chcpu/chcpu.md | 7 +++ src/uu/chcpu/src/chcpu.rs | 109 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 112 insertions(+), 4 deletions(-) create mode 100644 src/uu/chcpu/chcpu.md diff --git a/src/uu/chcpu/chcpu.md b/src/uu/chcpu/chcpu.md new file mode 100644 index 00000000..6ed48e84 --- /dev/null +++ b/src/uu/chcpu/chcpu.md @@ -0,0 +1,7 @@ +# chcpu + +``` +chcpu [OPTION]... +``` + +Configure CPUs in a multi-processor system. diff --git a/src/uu/chcpu/src/chcpu.rs b/src/uu/chcpu/src/chcpu.rs index 968f8548..b9ff5722 100644 --- a/src/uu/chcpu/src/chcpu.rs +++ b/src/uu/chcpu/src/chcpu.rs @@ -3,17 +3,118 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -use clap::{crate_version, Command}; -use uucore::error::UResult; +use clap::{crate_version, Arg, ArgAction, ArgGroup, Command}; +use uucore::{error::UResult, format_usage, help_about, help_usage}; + +fn enable_cpus(cpu_list: &str) { + println!("Enabling CPUs: {}", cpu_list) +} + +fn disable_cpus(cpu_list: &str) { + println!("Disabling CPUs: {}", cpu_list) +} + +fn configure_cpus(cpu_list: &str) { + println!("Configuring CPUs: {}", cpu_list) +} + +fn deconfigure_cpus(cpu_list: &str) { + println!("Deconfiguring CPUs: {}", cpu_list) +} + +fn set_dispatch_mode(mode: &str) { + println!("Setting dispatch mode to: {}", mode) +} #[uucore::main] -pub fn uumain(_args: impl uucore::Args) -> UResult<()> { - println!("chcpu: Hello world"); +pub fn uumain(args: impl uucore::Args) -> UResult<()> { + let matches = uu_app().try_get_matches_from(args)?; + + if let Some(cpu_list) = matches.get_one::(options::ENABLE) { + enable_cpus(cpu_list); + } + + if let Some(cpu_list) = matches.get_one::(options::DISABLE) { + disable_cpus(cpu_list); + } + + if let Some(cpu_list) = matches.get_one::(options::CONFIGURE) { + configure_cpus(cpu_list); + } + + if let Some(cpu_list) = matches.get_one::(options::DECONFIGURE) { + deconfigure_cpus(cpu_list); + } + + if let Some(mode) = matches.get_one::(options::DISPATCH) { + set_dispatch_mode(mode); + } + Ok(()) } +mod options { + pub const ENABLE: &str = "enable"; + pub const DISABLE: &str = "disable"; + pub const CONFIGURE: &str = "configure"; + pub const DECONFIGURE: &str = "deconfigure"; + pub const DISPATCH: &str = "dispatch"; +} + +const ABOUT: &str = help_about!("chcpu.md"); +const USAGE: &str = help_usage!("chcpu.md"); + pub fn uu_app() -> Command { Command::new(uucore::util_name()) .version(crate_version!()) + .about(ABOUT) + .override_usage(format_usage(USAGE)) .infer_long_args(true) + .arg( + Arg::new(options::ENABLE) + .short('e') + .long("enable") + .value_name("cpu-list") + .action(ArgAction::Set), + ) + .arg( + Arg::new(options::DISABLE) + .short('d') + .long("disable") + .value_name("cpu-list") + .action(ArgAction::Set), + ) + .arg( + Arg::new(options::CONFIGURE) + .short('c') + .long("configure") + .value_name("cpu-list") + .action(ArgAction::Set), + ) + .arg( + Arg::new(options::DECONFIGURE) + .short('g') + .long("deconfigure") + .value_name("cpu-list") + .action(ArgAction::Set), + ) + .arg( + Arg::new(options::DISPATCH) + .short('p') + .long("dispatch") + .value_name("mode") + .action(ArgAction::Set), + ) + .group( + ArgGroup::new("action") + .args(vec![ + options::ENABLE, + options::DISABLE, + options::CONFIGURE, + options::DECONFIGURE, + options::DISPATCH, + ]) + .multiple(false) // These 5 are mutually exclusive + .required(true), + ) } From 5b6e53df9160ee47e2278285605c632e7d414fd9 Mon Sep 17 00:00:00 2001 From: alxndrv <> Date: Mon, 17 Feb 2025 17:26:03 +0200 Subject: [PATCH 03/11] `chcpu`: Implement disabling and enabling of CPUs --- src/uu/chcpu/src/chcpu.rs | 127 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 122 insertions(+), 5 deletions(-) diff --git a/src/uu/chcpu/src/chcpu.rs b/src/uu/chcpu/src/chcpu.rs index b9ff5722..174288d8 100644 --- a/src/uu/chcpu/src/chcpu.rs +++ b/src/uu/chcpu/src/chcpu.rs @@ -3,27 +3,144 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. +use std::{ + fs::{self, File}, + io::Write, + path::PathBuf, +}; + use clap::{crate_version, Arg, ArgAction, ArgGroup, Command}; use uucore::{error::UResult, format_usage, help_about, help_usage}; +// Takes in a human-readable list of CPUs, and returns a list of indices parsed from that list +// These can come in the form of a plain range like `X-Y`, or a comma-separated ranges and indices ie. `1,3-4,7-8,10` +// Kernel docs with examples: https://www.kernel.org/doc/html/latest/admin-guide/cputopology.html +fn parse_cpu_list(list: &str) -> Vec { + let mut out: Vec = vec![]; + + if list.is_empty() { + return out; + } + + for part in list.trim().split(",") { + if part.contains("-") { + let bounds: Vec<_> = part.split("-").flat_map(|x| x.parse::()).collect(); + assert_eq!(bounds.len(), 2); + for idx in bounds[0]..bounds[1] + 1 { + out.push(idx) + } + } else { + let idx = part.parse::().expect("Invalid CPU index value"); + out.push(idx); + } + } + out +} + +#[derive(Debug)] +struct Cpu(usize); + +impl Cpu { + fn get_path(&self) -> PathBuf { + PathBuf::from(format!("/sys/devices/system/cpu/cpu{}", self.0)) + } + + // CPUs which are not hot-pluggable will not have the `/online` file in their directory + fn is_hotpluggable(&self) -> bool { + let path = self.get_path().join("online"); + path.exists() + } + + fn is_online(&self) -> bool { + if let Ok(state) = fs::read_to_string(self.get_path().join("online")) { + match state.trim() { + "0" => return false, + "1" => return true, + other => panic!("Unrecognized CPU online state: {}", other), + } + }; + + // Just in case the caller forgot to check `is_hotpluggable` first, + // instead of panicing that the file doesn't exist, return true + // This is because a non-hotpluggable CPU is assumed to be always online + true + } + + fn enable(&self) { + if !self.is_hotpluggable() { + println!("CPU {} is not hot-pluggable", self.0); + return; + } + + if self.is_online() { + println!("CPU {} is already enabled", self.0); + return; + } + + let result = + File::create(self.get_path().join("online")).and_then(|mut f| f.write_all(b"1")); + match result { + Ok(_) => println!("CPU {} enabled", self.0), + Err(e) => println!("CPU {} enable failed: {:#}", self.0, e.kind()), + } + } + + fn disable(&self) { + if !self.is_hotpluggable() { + println!("CPU {} is not hot-pluggable", self.0); + return; + } + + if !self.is_online() { + println!("CPU {} is already disabled", self.0); + return; + } + + if get_online_cpus().len() == 1 { + println!("CPU {} disable failed (last enabled CPU)", self.0); + return; + } + + let result = + File::create(self.get_path().join("online")).and_then(|mut f| f.write_all(b"0")); + match result { + Ok(_) => println!("CPU {} disabled", self.0), + Err(e) => println!("CPU {} disable failed: {:#}", self.0, e.kind()), + } + } +} + +fn get_online_cpus() -> Vec { + let cpu_list = fs::read_to_string("/sys/devices/system/cpu/online").unwrap(); + parse_cpu_list(&cpu_list).iter().map(|n| Cpu(*n)).collect() +} + fn enable_cpus(cpu_list: &str) { - println!("Enabling CPUs: {}", cpu_list) + let to_enable = parse_cpu_list(cpu_list).into_iter().map(Cpu); + + for cpu in to_enable { + cpu.enable(); + } } fn disable_cpus(cpu_list: &str) { - println!("Disabling CPUs: {}", cpu_list) + let to_disable = parse_cpu_list(cpu_list).into_iter().map(Cpu); + + for cpu in to_disable { + cpu.disable(); + } } fn configure_cpus(cpu_list: &str) { - println!("Configuring CPUs: {}", cpu_list) + todo!("Configuring CPUs: {}", cpu_list); } fn deconfigure_cpus(cpu_list: &str) { - println!("Deconfiguring CPUs: {}", cpu_list) + todo!("Deconfiguring CPUs: {}", cpu_list); } fn set_dispatch_mode(mode: &str) { - println!("Setting dispatch mode to: {}", mode) + todo!("Setting dispatch mode to: {}", mode); } #[uucore::main] From 2c7f1d30b27ac9343daec15778480baed1e53031 Mon Sep 17 00:00:00 2001 From: alxndrv <> Date: Mon, 17 Feb 2025 17:49:03 +0200 Subject: [PATCH 04/11] `chcpu`: Check that CPU exists before touching it --- src/uu/chcpu/src/chcpu.rs | 34 ++++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/src/uu/chcpu/src/chcpu.rs b/src/uu/chcpu/src/chcpu.rs index 174288d8..2724373e 100644 --- a/src/uu/chcpu/src/chcpu.rs +++ b/src/uu/chcpu/src/chcpu.rs @@ -45,6 +45,14 @@ impl Cpu { PathBuf::from(format!("/sys/devices/system/cpu/cpu{}", self.0)) } + fn ensure_exists(&self) -> bool { + if !self.get_path().exists() { + println!("CPU {} does not exist", self.0); + return false; + }; + true + } + // CPUs which are not hot-pluggable will not have the `/online` file in their directory fn is_hotpluggable(&self) -> bool { let path = self.get_path().join("online"); @@ -108,6 +116,10 @@ impl Cpu { Err(e) => println!("CPU {} disable failed: {:#}", self.0, e.kind()), } } + + fn configure(&self) {} + + fn deconfigure(&self) {} } fn get_online_cpus() -> Vec { @@ -119,7 +131,9 @@ fn enable_cpus(cpu_list: &str) { let to_enable = parse_cpu_list(cpu_list).into_iter().map(Cpu); for cpu in to_enable { - cpu.enable(); + if cpu.ensure_exists() { + cpu.enable(); + }; } } @@ -127,16 +141,28 @@ fn disable_cpus(cpu_list: &str) { let to_disable = parse_cpu_list(cpu_list).into_iter().map(Cpu); for cpu in to_disable { - cpu.disable(); + if cpu.ensure_exists() { + cpu.disable(); + } } } fn configure_cpus(cpu_list: &str) { - todo!("Configuring CPUs: {}", cpu_list); + let to_configure = parse_cpu_list(cpu_list).into_iter().map(Cpu); + for cpu in to_configure { + if cpu.ensure_exists() { + cpu.configure(); + } + } } fn deconfigure_cpus(cpu_list: &str) { - todo!("Deconfiguring CPUs: {}", cpu_list); + let to_deconfigure = parse_cpu_list(cpu_list).into_iter().map(Cpu); + for cpu in to_deconfigure { + if cpu.ensure_exists() { + cpu.deconfigure(); + } + } } fn set_dispatch_mode(mode: &str) { From 37eaf9ad830546c35300a276131578f0f8618c42 Mon Sep 17 00:00:00 2001 From: alxndrv <> Date: Mon, 17 Feb 2025 18:03:40 +0200 Subject: [PATCH 05/11] `chcpu`: Implement configure/deconfigure options --- src/uu/chcpu/src/chcpu.rs | 58 +++++++++++++++++++++++++++++++++++---- 1 file changed, 52 insertions(+), 6 deletions(-) diff --git a/src/uu/chcpu/src/chcpu.rs b/src/uu/chcpu/src/chcpu.rs index 2724373e..19383d3b 100644 --- a/src/uu/chcpu/src/chcpu.rs +++ b/src/uu/chcpu/src/chcpu.rs @@ -55,8 +55,7 @@ impl Cpu { // CPUs which are not hot-pluggable will not have the `/online` file in their directory fn is_hotpluggable(&self) -> bool { - let path = self.get_path().join("online"); - path.exists() + self.get_path().join("online").exists() } fn is_online(&self) -> bool { @@ -89,7 +88,7 @@ impl Cpu { File::create(self.get_path().join("online")).and_then(|mut f| f.write_all(b"1")); match result { Ok(_) => println!("CPU {} enabled", self.0), - Err(e) => println!("CPU {} enable failed: {:#}", self.0, e.kind()), + Err(e) => println!("CPU {} enable failed: {}", self.0, e.kind()), } } @@ -113,13 +112,60 @@ impl Cpu { File::create(self.get_path().join("online")).and_then(|mut f| f.write_all(b"0")); match result { Ok(_) => println!("CPU {} disabled", self.0), - Err(e) => println!("CPU {} disable failed: {:#}", self.0, e.kind()), + Err(e) => println!("CPU {} disable failed: {}", self.0, e.kind()), } } - fn configure(&self) {} + fn is_configurable(&self) -> bool { + self.get_path().join("configure").exists() + } + + fn configure(&self) { + if !self.is_configurable() { + println!("CPU {} is not configurable", self.0); + return; + } + + let cfg_path = self.get_path().join("configure"); + + let configured = fs::read_to_string(&cfg_path).unwrap(); + if configured.trim() == "1" { + println!("CPU {} is already configured", self.0); + return; + }; + + let result = File::create(cfg_path).and_then(|mut f| f.write_all(b"1")); + match result { + Ok(_) => println!("CPU {} configured", self.0), + Err(e) => println!("CPU {} configure failed: {}", self.0, e.kind()), + }; + } + + fn deconfigure(&self) { + if !self.is_configurable() { + println!("CPU {} is not configurable", self.0); + return; + } - fn deconfigure(&self) {} + let cfg_path = self.get_path().join("configure"); + + let configured = fs::read_to_string(&cfg_path).unwrap(); + if configured.trim() == "0" { + println!("CPU {} is already deconfigured", self.0); + return; + }; + + if self.is_online() { + println!("CPU {} deconfigure failed (CPU is enabled)", self.0); + return; + } + + let result = File::create(cfg_path).and_then(|mut f| f.write_all(b"0")); + match result { + Ok(_) => println!("CPU {} deconfigured", self.0), + Err(e) => println!("CPU {} deconfigure failed: {}", self.0, e.kind()), + }; + } } fn get_online_cpus() -> Vec { From bdc30a2e6894ab8290ebf497d8e7835cd51ed6c6 Mon Sep 17 00:00:00 2001 From: alxndrv <> Date: Mon, 17 Feb 2025 18:21:37 +0200 Subject: [PATCH 06/11] `chcpu`: Add support for triggering CPU rescan --- src/uu/chcpu/src/chcpu.rs | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/src/uu/chcpu/src/chcpu.rs b/src/uu/chcpu/src/chcpu.rs index 19383d3b..e8b818a8 100644 --- a/src/uu/chcpu/src/chcpu.rs +++ b/src/uu/chcpu/src/chcpu.rs @@ -173,6 +173,24 @@ fn get_online_cpus() -> Vec { parse_cpu_list(&cpu_list).iter().map(|n| Cpu(*n)).collect() } +fn trigger_rescan() { + let path = PathBuf::from("/sys/devices/system/cpu/rescan"); + + if !path.exists() { + // TODO: This should exit gracefully with a ExitCode::FAILURE instead of quietly returning + println!("This system does not support rescanning of CPUs"); + return; + } + + let result = File::create(path).and_then(|mut f| f.write_all(b"1")); + match result { + Ok(_) => println!("Triggered rescan of CPUs"), + + // TODO: This needs to exit with ExitCode::FAILURE + Err(e) => println!("Failed to trigger rescan of CPUs: {}", e.kind()), + }; +} + fn enable_cpus(cpu_list: &str) { let to_enable = parse_cpu_list(cpu_list).into_iter().map(Cpu); @@ -219,6 +237,10 @@ fn set_dispatch_mode(mode: &str) { pub fn uumain(args: impl uucore::Args) -> UResult<()> { let matches = uu_app().try_get_matches_from(args)?; + if matches.get_flag(options::RESCAN) { + trigger_rescan(); + } + if let Some(cpu_list) = matches.get_one::(options::ENABLE) { enable_cpus(cpu_list); } @@ -248,6 +270,7 @@ mod options { pub const CONFIGURE: &str = "configure"; pub const DECONFIGURE: &str = "deconfigure"; pub const DISPATCH: &str = "dispatch"; + pub const RESCAN: &str = "rescan"; } const ABOUT: &str = help_about!("chcpu.md"); @@ -304,6 +327,11 @@ pub fn uu_app() -> Command { options::DISPATCH, ]) .multiple(false) // These 5 are mutually exclusive - .required(true), + ) + .arg( + Arg::new(options::RESCAN) + .short('r') + .long("rescan") + .action(ArgAction::SetTrue), ) } From 39443cb291d7e77fc1a0c515a5390056f35208d3 Mon Sep 17 00:00:00 2001 From: alxndrv <> Date: Mon, 17 Feb 2025 18:34:30 +0200 Subject: [PATCH 07/11] `chcpu`: Implement --dispatch option --- src/uu/chcpu/src/chcpu.rs | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/src/uu/chcpu/src/chcpu.rs b/src/uu/chcpu/src/chcpu.rs index e8b818a8..64f05ed8 100644 --- a/src/uu/chcpu/src/chcpu.rs +++ b/src/uu/chcpu/src/chcpu.rs @@ -230,7 +230,34 @@ fn deconfigure_cpus(cpu_list: &str) { } fn set_dispatch_mode(mode: &str) { - todo!("Setting dispatch mode to: {}", mode); + let mode_num: u8 = match mode { + "horizontal" => 0, + "vertical" => 1, + _ => { + // TODO: Maybe `clap` could validate this argument automatically for us? + println!( + "Unsupported dispatching mode: '{}'. Must be either 'horizontal' or 'vertical'", + mode + ); + return; + } + }; + + let path = PathBuf::from("/sys/devices/system/cpu/dispatching"); + + if !path.exists() { + // TODO: This should exit gracefully with a ExitCode::FAILURE instead of quietly returning + println!("This system does not support setting the dispatching mode of CPUs"); + return; + } + + let result = File::create(path).and_then(|mut f| f.write_all(&[mode_num])); + match result { + Ok(_) => println!("Successfully set {} dispatching mode", mode), + + // TODO: This needs to exit with ExitCode::FAILURE + Err(e) => println!("Failed to set {} dispatching mode: {}", mode, e.kind()), + }; } #[uucore::main] @@ -326,7 +353,7 @@ pub fn uu_app() -> Command { options::DECONFIGURE, options::DISPATCH, ]) - .multiple(false) // These 5 are mutually exclusive + .multiple(false), // These 5 are mutually exclusive ) .arg( Arg::new(options::RESCAN) From acb44fba1ecd0ab1a40b55e72e9153f6c00dbfb2 Mon Sep 17 00:00:00 2001 From: alxndrv <> Date: Mon, 17 Feb 2025 18:36:53 +0200 Subject: [PATCH 08/11] `chcpu`: Add test for parse_cpu_list --- src/uu/chcpu/src/chcpu.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/uu/chcpu/src/chcpu.rs b/src/uu/chcpu/src/chcpu.rs index 64f05ed8..b036abcb 100644 --- a/src/uu/chcpu/src/chcpu.rs +++ b/src/uu/chcpu/src/chcpu.rs @@ -37,6 +37,21 @@ fn parse_cpu_list(list: &str) -> Vec { out } +#[test] +fn test_parse_cpu_list() { + assert_eq!(parse_cpu_list(""), Vec::::new()); + assert_eq!(parse_cpu_list("1-3"), Vec::::from([1, 2, 3])); + assert_eq!(parse_cpu_list("1,2,3"), Vec::::from([1, 2, 3])); + assert_eq!( + parse_cpu_list("1,3-6,8"), + Vec::::from([1, 3, 4, 5, 6, 8]) + ); + assert_eq!( + parse_cpu_list("1-2,3-5,7"), + Vec::::from([1, 2, 3, 4, 5, 7]) + ); +} + #[derive(Debug)] struct Cpu(usize); From a8f931d8da50466ffa767cba33f87067c1adf515 Mon Sep 17 00:00:00 2001 From: alxndrv <> Date: Tue, 18 Feb 2025 00:20:22 +0200 Subject: [PATCH 09/11] `chcpu`: Clean up code as per review suggestions --- src/uu/chcpu/src/chcpu.rs | 118 ++++++++++++++++---------------------- 1 file changed, 50 insertions(+), 68 deletions(-) diff --git a/src/uu/chcpu/src/chcpu.rs b/src/uu/chcpu/src/chcpu.rs index b036abcb..89588446 100644 --- a/src/uu/chcpu/src/chcpu.rs +++ b/src/uu/chcpu/src/chcpu.rs @@ -37,21 +37,6 @@ fn parse_cpu_list(list: &str) -> Vec { out } -#[test] -fn test_parse_cpu_list() { - assert_eq!(parse_cpu_list(""), Vec::::new()); - assert_eq!(parse_cpu_list("1-3"), Vec::::from([1, 2, 3])); - assert_eq!(parse_cpu_list("1,2,3"), Vec::::from([1, 2, 3])); - assert_eq!( - parse_cpu_list("1,3-6,8"), - Vec::::from([1, 3, 4, 5, 6, 8]) - ); - assert_eq!( - parse_cpu_list("1-2,3-5,7"), - Vec::::from([1, 2, 3, 4, 5, 7]) - ); -} - #[derive(Debug)] struct Cpu(usize); @@ -60,6 +45,14 @@ impl Cpu { PathBuf::from(format!("/sys/devices/system/cpu/cpu{}", self.0)) } + fn write_to_cpu_file(&self, file: &str, value: &[u8]) -> std::io::Result<()> { + File::create(self.get_path().join(file)).and_then(|mut f| f.write_all(value)) + } + + fn read_cpu_file(&self, file: &str) -> String { + fs::read_to_string(self.get_path().join(file)).unwrap() + } + fn ensure_exists(&self) -> bool { if !self.get_path().exists() { println!("CPU {} does not exist", self.0); @@ -74,18 +67,16 @@ impl Cpu { } fn is_online(&self) -> bool { - if let Ok(state) = fs::read_to_string(self.get_path().join("online")) { - match state.trim() { - "0" => return false, - "1" => return true, + fs::read_to_string(self.get_path().join("online")) + .map(|content| match content.trim() { + "0" => false, + "1" => true, other => panic!("Unrecognized CPU online state: {}", other), - } - }; - - // Just in case the caller forgot to check `is_hotpluggable` first, - // instead of panicing that the file doesn't exist, return true - // This is because a non-hotpluggable CPU is assumed to be always online - true + }) + // Just in case the caller forgot to check `is_hotpluggable` first, + // instead of panicing that the file doesn't exist, return true + // This is because a non-hotpluggable CPU is assumed to be always online + .unwrap_or(true) } fn enable(&self) { @@ -99,9 +90,7 @@ impl Cpu { return; } - let result = - File::create(self.get_path().join("online")).and_then(|mut f| f.write_all(b"1")); - match result { + match self.write_to_cpu_file("online", b"1") { Ok(_) => println!("CPU {} enabled", self.0), Err(e) => println!("CPU {} enable failed: {}", self.0, e.kind()), } @@ -123,9 +112,7 @@ impl Cpu { return; } - let result = - File::create(self.get_path().join("online")).and_then(|mut f| f.write_all(b"0")); - match result { + match self.write_to_cpu_file("online", b"0") { Ok(_) => println!("CPU {} disabled", self.0), Err(e) => println!("CPU {} disable failed: {}", self.0, e.kind()), } @@ -141,16 +128,13 @@ impl Cpu { return; } - let cfg_path = self.get_path().join("configure"); - - let configured = fs::read_to_string(&cfg_path).unwrap(); + let configured = self.read_cpu_file("configure"); if configured.trim() == "1" { println!("CPU {} is already configured", self.0); return; }; - let result = File::create(cfg_path).and_then(|mut f| f.write_all(b"1")); - match result { + match self.write_to_cpu_file("configure", b"1") { Ok(_) => println!("CPU {} configured", self.0), Err(e) => println!("CPU {} configure failed: {}", self.0, e.kind()), }; @@ -162,9 +146,7 @@ impl Cpu { return; } - let cfg_path = self.get_path().join("configure"); - - let configured = fs::read_to_string(&cfg_path).unwrap(); + let configured = self.read_cpu_file("configure"); if configured.trim() == "0" { println!("CPU {} is already deconfigured", self.0); return; @@ -175,8 +157,7 @@ impl Cpu { return; } - let result = File::create(cfg_path).and_then(|mut f| f.write_all(b"0")); - match result { + match self.write_to_cpu_file("configure", b"0") { Ok(_) => println!("CPU {} deconfigured", self.0), Err(e) => println!("CPU {} deconfigure failed: {}", self.0, e.kind()), }; @@ -206,42 +187,28 @@ fn trigger_rescan() { }; } -fn enable_cpus(cpu_list: &str) { - let to_enable = parse_cpu_list(cpu_list).into_iter().map(Cpu); +fn process_cpus(cpu_list: &str, action: impl Fn(&Cpu)) { + parse_cpu_list(cpu_list) + .into_iter() + .map(Cpu) + .filter(Cpu::ensure_exists) + .for_each(|cpu| action(&cpu)); +} - for cpu in to_enable { - if cpu.ensure_exists() { - cpu.enable(); - }; - } +fn enable_cpus(cpu_list: &str) { + process_cpus(cpu_list, Cpu::enable); } fn disable_cpus(cpu_list: &str) { - let to_disable = parse_cpu_list(cpu_list).into_iter().map(Cpu); - - for cpu in to_disable { - if cpu.ensure_exists() { - cpu.disable(); - } - } + process_cpus(cpu_list, Cpu::disable); } fn configure_cpus(cpu_list: &str) { - let to_configure = parse_cpu_list(cpu_list).into_iter().map(Cpu); - for cpu in to_configure { - if cpu.ensure_exists() { - cpu.configure(); - } - } + process_cpus(cpu_list, Cpu::configure); } fn deconfigure_cpus(cpu_list: &str) { - let to_deconfigure = parse_cpu_list(cpu_list).into_iter().map(Cpu); - for cpu in to_deconfigure { - if cpu.ensure_exists() { - cpu.deconfigure(); - } - } + process_cpus(cpu_list, Cpu::deconfigure); } fn set_dispatch_mode(mode: &str) { @@ -377,3 +344,18 @@ pub fn uu_app() -> Command { .action(ArgAction::SetTrue), ) } + +#[test] +fn test_parse_cpu_list() { + assert_eq!(parse_cpu_list(""), Vec::::new()); + assert_eq!(parse_cpu_list("1-3"), Vec::::from([1, 2, 3])); + assert_eq!(parse_cpu_list("1,2,3"), Vec::::from([1, 2, 3])); + assert_eq!( + parse_cpu_list("1,3-6,8"), + Vec::::from([1, 3, 4, 5, 6, 8]) + ); + assert_eq!( + parse_cpu_list("1-2,3-5,7"), + Vec::::from([1, 2, 3, 4, 5, 7]) + ); +} From 62ef9770023516b398d805ff0b56d1c9c04ac43d Mon Sep 17 00:00:00 2001 From: alxndrv <> Date: Sun, 9 Mar 2025 18:29:04 +0200 Subject: [PATCH 10/11] `chcpu`: Add basic error handling --- src/uu/chcpu/src/chcpu.rs | 237 +++++++++++++++++++++++++------------- 1 file changed, 156 insertions(+), 81 deletions(-) diff --git a/src/uu/chcpu/src/chcpu.rs b/src/uu/chcpu/src/chcpu.rs index 89588446..3636763c 100644 --- a/src/uu/chcpu/src/chcpu.rs +++ b/src/uu/chcpu/src/chcpu.rs @@ -10,22 +10,30 @@ use std::{ }; use clap::{crate_version, Arg, ArgAction, ArgGroup, Command}; -use uucore::{error::UResult, format_usage, help_about, help_usage}; +use uucore::{ + error::{UResult, USimpleError}, + format_usage, help_about, help_usage, +}; // Takes in a human-readable list of CPUs, and returns a list of indices parsed from that list // These can come in the form of a plain range like `X-Y`, or a comma-separated ranges and indices ie. `1,3-4,7-8,10` // Kernel docs with examples: https://www.kernel.org/doc/html/latest/admin-guide/cputopology.html -fn parse_cpu_list(list: &str) -> Vec { +// TODO: Move function to uucore +fn parse_cpu_list(list: &str) -> UResult> { let mut out: Vec = vec![]; if list.is_empty() { - return out; + return Ok(out); } for part in list.trim().split(",") { if part.contains("-") { let bounds: Vec<_> = part.split("-").flat_map(|x| x.parse::()).collect(); - assert_eq!(bounds.len(), 2); + + if bounds.len() != 2 { + return Err(USimpleError::new(1, format!("Invalid CPU range: {}", part))); + } + for idx in bounds[0]..bounds[1] + 1 { out.push(idx) } @@ -34,7 +42,7 @@ fn parse_cpu_list(list: &str) -> Vec { out.push(idx); } } - out + Ok(out) } #[derive(Debug)] @@ -79,167 +87,228 @@ impl Cpu { .unwrap_or(true) } - fn enable(&self) { + fn enable(&self) -> UResult<()> { if !self.is_hotpluggable() { - println!("CPU {} is not hot-pluggable", self.0); - return; + return Err(USimpleError::new( + 1, + format!("CPU {} is not hot-pluggable", self.0), + )); } if self.is_online() { - println!("CPU {} is already enabled", self.0); - return; + return Err(USimpleError::new( + 1, + format!("CPU {} is already enabled", self.0), + )); } match self.write_to_cpu_file("online", b"1") { Ok(_) => println!("CPU {} enabled", self.0), - Err(e) => println!("CPU {} enable failed: {}", self.0, e.kind()), - } + Err(e) => { + return Err(USimpleError::new( + 1, + format!("CPU {} enable failed: {}", self.0, e.kind()), + )) + } + }; + Ok(()) } - fn disable(&self) { + fn disable(&self) -> UResult<()> { if !self.is_hotpluggable() { - println!("CPU {} is not hot-pluggable", self.0); - return; + return Err(USimpleError::new( + 1, + format!("CPU {} is not hot-pluggable", self.0), + )); } if !self.is_online() { - println!("CPU {} is already disabled", self.0); - return; + return Err(USimpleError::new( + 1, + format!("CPU {} is already disabled", self.0), + )); } - if get_online_cpus().len() == 1 { - println!("CPU {} disable failed (last enabled CPU)", self.0); - return; + if get_online_cpus()?.len() == 1 { + return Err(USimpleError::new( + 1, + format!("CPU {} disable failed (last enabled CPU)", self.0), + )); } match self.write_to_cpu_file("online", b"0") { Ok(_) => println!("CPU {} disabled", self.0), - Err(e) => println!("CPU {} disable failed: {}", self.0, e.kind()), - } + Err(e) => { + return Err(USimpleError::new( + 1, + format!("CPU {} disable failed: {}", self.0, e.kind()), + )) + } + }; + + Ok(()) } fn is_configurable(&self) -> bool { self.get_path().join("configure").exists() } - fn configure(&self) { + fn configure(&self) -> UResult<()> { if !self.is_configurable() { - println!("CPU {} is not configurable", self.0); - return; + return Err(USimpleError::new( + 1, + format!("CPU {} is not configurable", self.0), + )); } let configured = self.read_cpu_file("configure"); if configured.trim() == "1" { - println!("CPU {} is already configured", self.0); - return; + return Err(USimpleError::new( + 1, + format!("CPU {} is already configured", self.0), + )); }; match self.write_to_cpu_file("configure", b"1") { Ok(_) => println!("CPU {} configured", self.0), - Err(e) => println!("CPU {} configure failed: {}", self.0, e.kind()), + Err(e) => { + return Err(USimpleError::new( + 1, + format!("CPU {} configure failed: {}", self.0, e.kind()), + )) + } }; + Ok(()) } - fn deconfigure(&self) { + fn deconfigure(&self) -> UResult<()> { if !self.is_configurable() { - println!("CPU {} is not configurable", self.0); - return; + return Err(USimpleError::new( + 1, + format!("CPU {} is not configurable", self.0), + )); } let configured = self.read_cpu_file("configure"); if configured.trim() == "0" { - println!("CPU {} is already deconfigured", self.0); - return; + return Err(USimpleError::new( + 1, + format!("CPU {} is already deconfigured", self.0), + )); }; if self.is_online() { - println!("CPU {} deconfigure failed (CPU is enabled)", self.0); - return; + return Err(USimpleError::new( + 1, + format!("CPU {} deconfigure failed (CPU is enabled)", self.0), + )); } match self.write_to_cpu_file("configure", b"0") { Ok(_) => println!("CPU {} deconfigured", self.0), - Err(e) => println!("CPU {} deconfigure failed: {}", self.0, e.kind()), + Err(e) => { + return Err(USimpleError::new( + 1, + format!("CPU {} deconfigure failed: {}", self.0, e.kind()), + )) + } }; + + Ok(()) } } -fn get_online_cpus() -> Vec { - let cpu_list = fs::read_to_string("/sys/devices/system/cpu/online").unwrap(); - parse_cpu_list(&cpu_list).iter().map(|n| Cpu(*n)).collect() +fn get_online_cpus() -> UResult> { + let cpu_list = fs::read_to_string("/sys/devices/system/cpu/online")?; + let cpus = parse_cpu_list(&cpu_list)?.iter().map(|n| Cpu(*n)).collect(); + Ok(cpus) } -fn trigger_rescan() { +fn trigger_rescan() -> UResult<()> { let path = PathBuf::from("/sys/devices/system/cpu/rescan"); if !path.exists() { - // TODO: This should exit gracefully with a ExitCode::FAILURE instead of quietly returning - println!("This system does not support rescanning of CPUs"); - return; + return Err(USimpleError::new( + 1, + "This system does not support rescanning of CPUs", + )); } let result = File::create(path).and_then(|mut f| f.write_all(b"1")); match result { Ok(_) => println!("Triggered rescan of CPUs"), - - // TODO: This needs to exit with ExitCode::FAILURE - Err(e) => println!("Failed to trigger rescan of CPUs: {}", e.kind()), + Err(e) => { + return Err(USimpleError::new( + 1, + format!("Failed to trigger rescan of CPUs: {}", e.kind()), + )) + } }; + + Ok(()) } -fn process_cpus(cpu_list: &str, action: impl Fn(&Cpu)) { - parse_cpu_list(cpu_list) +fn process_cpus(cpu_list: &str, action: impl Fn(&Cpu) -> UResult<()>) -> UResult<()> { + parse_cpu_list(cpu_list)? .into_iter() .map(Cpu) .filter(Cpu::ensure_exists) - .for_each(|cpu| action(&cpu)); + .try_for_each(|cpu| action(&cpu))?; + Ok(()) } -fn enable_cpus(cpu_list: &str) { - process_cpus(cpu_list, Cpu::enable); +fn enable_cpus(cpu_list: &str) -> UResult<()> { + process_cpus(cpu_list, Cpu::enable) } -fn disable_cpus(cpu_list: &str) { - process_cpus(cpu_list, Cpu::disable); +fn disable_cpus(cpu_list: &str) -> UResult<()> { + process_cpus(cpu_list, Cpu::disable) } -fn configure_cpus(cpu_list: &str) { - process_cpus(cpu_list, Cpu::configure); +fn configure_cpus(cpu_list: &str) -> UResult<()> { + process_cpus(cpu_list, Cpu::configure) } -fn deconfigure_cpus(cpu_list: &str) { - process_cpus(cpu_list, Cpu::deconfigure); +fn deconfigure_cpus(cpu_list: &str) -> UResult<()> { + process_cpus(cpu_list, Cpu::deconfigure) } -fn set_dispatch_mode(mode: &str) { +fn set_dispatch_mode(mode: &str) -> UResult<()> { let mode_num: u8 = match mode { "horizontal" => 0, "vertical" => 1, _ => { - // TODO: Maybe `clap` could validate this argument automatically for us? - println!( - "Unsupported dispatching mode: '{}'. Must be either 'horizontal' or 'vertical'", - mode - ); - return; + return Err(USimpleError::new( + 1, + format!( + "Unsupported dispatching mode: '{}'. Must be either 'horizontal' or 'vertical'", + mode + ), + )) } }; let path = PathBuf::from("/sys/devices/system/cpu/dispatching"); if !path.exists() { - // TODO: This should exit gracefully with a ExitCode::FAILURE instead of quietly returning - println!("This system does not support setting the dispatching mode of CPUs"); - return; + return Err(USimpleError::new( + 1, + "This system does not support setting the dispatching mode of CPUs", + )); } let result = File::create(path).and_then(|mut f| f.write_all(&[mode_num])); match result { Ok(_) => println!("Successfully set {} dispatching mode", mode), - - // TODO: This needs to exit with ExitCode::FAILURE - Err(e) => println!("Failed to set {} dispatching mode: {}", mode, e.kind()), + Err(e) => { + return Err(USimpleError::new( + 1, + format!("Failed to set {} dispatching mode: {}", mode, e.kind()), + )) + } }; + + Ok(()) } #[uucore::main] @@ -247,27 +316,27 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let matches = uu_app().try_get_matches_from(args)?; if matches.get_flag(options::RESCAN) { - trigger_rescan(); + trigger_rescan()?; } if let Some(cpu_list) = matches.get_one::(options::ENABLE) { - enable_cpus(cpu_list); + enable_cpus(cpu_list)?; } if let Some(cpu_list) = matches.get_one::(options::DISABLE) { - disable_cpus(cpu_list); + disable_cpus(cpu_list)?; } if let Some(cpu_list) = matches.get_one::(options::CONFIGURE) { - configure_cpus(cpu_list); + configure_cpus(cpu_list)?; } if let Some(cpu_list) = matches.get_one::(options::DECONFIGURE) { - deconfigure_cpus(cpu_list); + deconfigure_cpus(cpu_list)?; } if let Some(mode) = matches.get_one::(options::DISPATCH) { - set_dispatch_mode(mode); + set_dispatch_mode(mode)?; } Ok(()) @@ -347,15 +416,21 @@ pub fn uu_app() -> Command { #[test] fn test_parse_cpu_list() { - assert_eq!(parse_cpu_list(""), Vec::::new()); - assert_eq!(parse_cpu_list("1-3"), Vec::::from([1, 2, 3])); - assert_eq!(parse_cpu_list("1,2,3"), Vec::::from([1, 2, 3])); + assert_eq!(parse_cpu_list("").unwrap(), Vec::::new()); + assert_eq!( + parse_cpu_list("1-3").unwrap(), + Vec::::from([1, 2, 3]) + ); + assert_eq!( + parse_cpu_list("1,2,3").unwrap(), + Vec::::from([1, 2, 3]) + ); assert_eq!( - parse_cpu_list("1,3-6,8"), + parse_cpu_list("1,3-6,8").unwrap(), Vec::::from([1, 3, 4, 5, 6, 8]) ); assert_eq!( - parse_cpu_list("1-2,3-5,7"), + parse_cpu_list("1-2,3-5,7").unwrap(), Vec::::from([1, 2, 3, 4, 5, 7]) ); } From 4266929d9cff68c92002c96b916d10eeaf65b8c7 Mon Sep 17 00:00:00 2001 From: alxndrv <> Date: Mon, 10 Mar 2025 19:33:33 +0200 Subject: [PATCH 11/11] `chcpu`: Add testing skeleton for new utility --- src/uu/chcpu/src/chcpu.rs | 13 ++++++------ tests/by-util/test_chcpu.rs | 42 +++++++++++++++++++++++++++++++++++++ tests/tests.rs | 4 ++++ 3 files changed, 53 insertions(+), 6 deletions(-) create mode 100644 tests/by-util/test_chcpu.rs diff --git a/src/uu/chcpu/src/chcpu.rs b/src/uu/chcpu/src/chcpu.rs index 3636763c..8b65df8b 100644 --- a/src/uu/chcpu/src/chcpu.rs +++ b/src/uu/chcpu/src/chcpu.rs @@ -61,12 +61,14 @@ impl Cpu { fs::read_to_string(self.get_path().join(file)).unwrap() } - fn ensure_exists(&self) -> bool { + fn ensure_exists(&self) -> UResult { if !self.get_path().exists() { - println!("CPU {} does not exist", self.0); - return false; + return Err(USimpleError::new( + 1, + format!("CPU {} does not exist", self.0), + )); }; - true + Ok(true) } // CPUs which are not hot-pluggable will not have the `/online` file in their directory @@ -252,8 +254,7 @@ fn process_cpus(cpu_list: &str, action: impl Fn(&Cpu) -> UResult<()>) -> UResult parse_cpu_list(cpu_list)? .into_iter() .map(Cpu) - .filter(Cpu::ensure_exists) - .try_for_each(|cpu| action(&cpu))?; + .try_for_each(|cpu| cpu.ensure_exists().and_then(|_| action(&cpu)))?; Ok(()) } diff --git a/tests/by-util/test_chcpu.rs b/tests/by-util/test_chcpu.rs new file mode 100644 index 00000000..603728dc --- /dev/null +++ b/tests/by-util/test_chcpu.rs @@ -0,0 +1,42 @@ +// This file is part of the uutils util-linux package. +// +// For the full copyright and license information, please view the LICENSE +// file that was distributed with this source code. + +use crate::common::util::TestScenario; + +#[test] +fn test_invalid_arg() { + new_ucmd!().arg("--definitely-invalid").fails().code_is(1); +} + +#[test] +fn test_invalid_cpu_range() { + new_ucmd!() + .arg("-e") + .arg("non-numeric-range") + .fails() + .code_is(1); +} + +#[test] +fn test_invalid_cpu_index() { + new_ucmd!() + .arg("-e") + .arg("10000") // Assuming no test environment will ever have 10000 CPUs + .fails() + .code_is(1) + .stderr_contains("CPU 10000 does not exist"); +} + +#[test] +fn test_invalid_dispatch_mode() { + new_ucmd!() + .arg("-p") + .arg("not-horizontal-or-vertical") + .fails() + .code_is(1) + .stderr_contains("Unsupported dispatching mode"); +} + +// TODO: Find a way to implement "happy-case" tests that doesn't rely on the host `/sys/` filesystem diff --git a/tests/tests.rs b/tests/tests.rs index 8d72ee63..de7f13df 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -52,3 +52,7 @@ mod test_dmesg; #[cfg(feature = "fsfreeze")] #[path = "by-util/test_fsfreeze.rs"] mod test_fsfreeze; + +#[cfg(feature = "chcpu")] +#[path = "by-util/test_chcpu.rs"] +mod test_chcpu;