From 58ba603afcbf9fd88ceba4b917c52006ae3a6e34 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Wed, 14 May 2025 18:58:06 +0300 Subject: [PATCH] cpu: handle sysfs errors more gracefully --- src/cpu.rs | 44 +++++++++++++++++ src/engine.rs | 118 ++++++++++++++++++++++++++++++++++++++++++---- src/monitor.rs | 43 +++++++++++------ src/util/error.rs | 12 +++++ 4 files changed, 193 insertions(+), 24 deletions(-) diff --git a/src/cpu.rs b/src/cpu.rs index ac8092c..e8d4bb6 100644 --- a/src/cpu.rs +++ b/src/cpu.rs @@ -79,6 +79,17 @@ where } pub fn set_governor(governor: &str, core_id: Option) -> Result<()> { + // First, check if the requested governor is available on the system + let available_governors = get_available_governors()?; + + if !available_governors.contains(&governor.to_string()) { + return Err(ControlError::InvalidGovernor(format!( + "Governor '{}' is not available. Available governors: {}", + governor, + available_governors.join(", ") + ))); + } + let action = |id: u32| { let path = format!("/sys/devices/system/cpu/cpu{id}/cpufreq/scaling_governor"); if Path::new(&path).exists() { @@ -93,6 +104,39 @@ pub fn set_governor(governor: &str, core_id: Option) -> Result<()> { core_id.map_or_else(|| for_each_cpu_core(action), action) } +/// Retrieves the list of available CPU governors on the system +pub fn get_available_governors() -> Result> { + // Check cpu0 for available governors + let path = "/sys/devices/system/cpu/cpu0/cpufreq/scaling_available_governors"; + if !Path::new(path).exists() { + return Err(ControlError::NotSupported( + "Could not determine available governors".to_string(), + )); + } + + let content = fs::read_to_string(path).map_err(|e| { + if e.kind() == io::ErrorKind::PermissionDenied { + ControlError::PermissionDenied(format!("Permission denied reading from {path}")) + } else { + ControlError::ReadError(format!("Failed to read from {path}: {e}")) + } + })?; + + // Parse the space-separated list of governors + let governors = content + .split_whitespace() + .map(ToString::to_string) + .collect::>(); + + if governors.is_empty() { + return Err(ControlError::ParseError( + "No available governors found".to_string(), + )); + } + + Ok(governors) +} + pub fn set_turbo(setting: TurboSetting) -> Result<()> { let value_pstate = match setting { TurboSetting::Always => "0", // no_turbo = 0 means turbo is enabled diff --git a/src/engine.rs b/src/engine.rs index c83d2f2..4f94ff6 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1,8 +1,8 @@ use crate::config::{AppConfig, ProfileConfig}; use crate::core::{OperationalMode, SystemReport, TurboSetting}; use crate::cpu::{self}; -use crate::util::error::EngineError; -use log::{debug, info}; +use crate::util::error::{ControlError, EngineError}; +use log::{debug, info, warn}; /// Determines the appropriate CPU profile based on power status or forced mode, /// and applies the settings using functions from the `cpu` module. @@ -17,6 +17,18 @@ pub fn determine_and_apply_settings( "Governor override is active: '{}'. Setting governor.", override_governor.trim() ); + + // Check if the governor is available before applying + let available_governors = report.cpu_global.available_governors.clone(); + if !available_governors.contains(&override_governor.trim().to_string()) { + return Err(EngineError::ConfigurationError(format!( + "Governor '{}' from override file is not available on this system. Available governors: {}", + override_governor.trim(), + available_governors.join(", ") + ))); + } + + // Apply the override governor setting cpu::set_governor(override_governor.trim(), None)?; } @@ -52,8 +64,16 @@ pub fn determine_and_apply_settings( // Apply settings from selected_profile_config if let Some(governor) = &selected_profile_config.governor { - info!("Setting governor to '{governor}'"); - cpu::set_governor(governor, None)?; + // Check if the governor is available before trying to set it + if report.cpu_global.available_governors.contains(governor) { + info!("Setting governor to '{governor}'"); + cpu::set_governor(governor, None)?; + } else { + let available = report.cpu_global.available_governors.join(", "); + warn!( + "Configured governor '{governor}' is not available on this system. Available governors: {available}. Skipping." + ); + } } if let Some(turbo_setting) = selected_profile_config.turbo { @@ -63,33 +83,111 @@ pub fn determine_and_apply_settings( debug!("Managing turbo in auto mode based on system conditions"); manage_auto_turbo(report, selected_profile_config)?; } - _ => cpu::set_turbo(turbo_setting)?, + _ => { + // Try to set turbo, but handle the error gracefully + if let Err(e) = cpu::set_turbo(turbo_setting) { + // If the feature is not supported, just log a warning instead of failing + if matches!(e, ControlError::NotSupported(_)) { + warn!( + "Turbo boost control is not supported on this system. Skipping turbo setting." + ); + } else { + // For other errors, propagate them + return Err(EngineError::ControlError(e)); + } + } + } } } if let Some(epp) = &selected_profile_config.epp { info!("Setting EPP to '{epp}'"); - cpu::set_epp(epp, None)?; + // Try to set EPP, but handle the error gracefully + if let Err(e) = cpu::set_epp(epp, None) { + // If the feature is not supported, just log a warning instead of failing + if matches!(e, ControlError::NotSupported(_)) + || e.to_string().contains("No such file or directory") + { + warn!("EPP setting is not supported on this system. Skipping EPP configuration."); + } else { + return Err(EngineError::ControlError(e)); + } + } } if let Some(epb) = &selected_profile_config.epb { info!("Setting EPB to '{epb}'"); - cpu::set_epb(epb, None)?; + // Try to set EPB, but handle the error gracefully + if let Err(e) = cpu::set_epb(epb, None) { + // If the feature is not supported, just log a warning instead of failing + if matches!(e, ControlError::NotSupported(_)) + || e.to_string().contains("No such file or directory") + { + warn!("EPB setting is not supported on this system. Skipping EPB configuration."); + } else { + return Err(EngineError::ControlError(e)); + } + } } if let Some(min_freq) = selected_profile_config.min_freq_mhz { info!("Setting min frequency to '{min_freq} MHz'"); - cpu::set_min_frequency(min_freq, None)?; + if let Err(e) = cpu::set_min_frequency(min_freq, None) { + // If the feature is not supported, just log a warning instead of failing + if matches!(e, ControlError::NotSupported(_)) + || e.to_string().contains("No such file or directory") + { + warn!( + "CPU frequency control is not supported on this system. Skipping min frequency setting." + ); + } else { + return Err(EngineError::ControlError(e)); + } + } } if let Some(max_freq) = selected_profile_config.max_freq_mhz { info!("Setting max frequency to '{max_freq} MHz'"); - cpu::set_max_frequency(max_freq, None)?; + if let Err(e) = cpu::set_max_frequency(max_freq, None) { + // If the feature is not supported, just log a warning instead of failing + if matches!(e, ControlError::NotSupported(_)) + || e.to_string().contains("No such file or directory") + { + warn!( + "CPU frequency control is not supported on this system. Skipping max frequency setting." + ); + } else { + return Err(EngineError::ControlError(e)); + } + } } if let Some(profile) = &selected_profile_config.platform_profile { info!("Setting platform profile to '{profile}'"); - cpu::set_platform_profile(profile)?; + // Try to get available platform profiles first to validate + match cpu::get_platform_profiles() { + Ok(available_profiles) => { + if available_profiles.contains(profile) { + cpu::set_platform_profile(profile)?; + } else { + warn!( + "Platform profile '{}' is not available. Available profiles: {}. Skipping.", + profile, + available_profiles.join(", ") + ); + } + } + Err(e) => { + // If platform profiles are not supported, log a warning and continue + if matches!(e, ControlError::NotSupported(_)) { + warn!( + "Platform profile control is not supported on this system. Skipping profile setting." + ); + } else { + return Err(EngineError::ControlError(e)); + } + } + } } debug!("Profile settings applied successfully."); diff --git a/src/monitor.rs b/src/monitor.rs index 259e872..91997a1 100644 --- a/src/monitor.rs +++ b/src/monitor.rs @@ -403,25 +403,40 @@ pub fn get_all_cpu_core_info() -> Result> { } pub fn get_cpu_global_info(cpu_cores: &[CpuCoreInfo]) -> CpuGlobalInfo { - // FIXME: Assume global settings can be read from cpu0 or are consistent. - // This might not work properly for heterogeneous systems (e.g. big.LITTLE) - let cpufreq_base_path = Path::new("/sys/devices/system/cpu/cpu0/cpufreq/"); + // Find a valid CPU to read global settings from + // Try cpu0 first, then fall back to any available CPU with cpufreq + let mut cpufreq_base_path_buf = PathBuf::from("/sys/devices/system/cpu/cpu0/cpufreq/"); + + if !cpufreq_base_path_buf.exists() { + // Fallback: find first available CPU with cpufreq + for i in 1..=get_logical_core_count().unwrap_or(1) { + let test_path = format!("/sys/devices/system/cpu/cpu{}/cpufreq/", i - 1); + let test_path_buf = PathBuf::from(&test_path); + if test_path_buf.exists() { + cpufreq_base_path_buf = test_path_buf; + break; + } + } + } + let turbo_status_path = Path::new("/sys/devices/system/cpu/intel_pstate/no_turbo"); let boost_path = Path::new("/sys/devices/system/cpu/cpufreq/boost"); - let current_governor = if cpufreq_base_path.join("scaling_governor").exists() { - read_sysfs_file_trimmed(cpufreq_base_path.join("scaling_governor")).ok() + + let current_governor = if cpufreq_base_path_buf.join("scaling_governor").exists() { + read_sysfs_file_trimmed(cpufreq_base_path_buf.join("scaling_governor")).ok() } else { None }; - let available_governors = if cpufreq_base_path + let available_governors = if cpufreq_base_path_buf .join("scaling_available_governors") .exists() { - read_sysfs_file_trimmed(cpufreq_base_path.join("scaling_available_governors")).map_or_else( - |_| vec![], - |s| s.split_whitespace().map(String::from).collect(), - ) + read_sysfs_file_trimmed(cpufreq_base_path_buf.join("scaling_available_governors")) + .map_or_else( + |_| vec![], + |s| s.split_whitespace().map(String::from).collect(), + ) } else { vec![] }; @@ -437,17 +452,16 @@ pub fn get_cpu_global_info(cpu_cores: &[CpuCoreInfo]) -> CpuGlobalInfo { } else { None }; + // EPP (Energy Performance Preference) let energy_perf_pref = - read_sysfs_file_trimmed(cpufreq_base_path.join("energy_performance_preference")).ok(); + read_sysfs_file_trimmed(cpufreq_base_path_buf.join("energy_performance_preference")).ok(); // EPB (Energy Performance Bias) let energy_perf_bias = - read_sysfs_file_trimmed(cpufreq_base_path.join("energy_performance_bias")).ok(); + read_sysfs_file_trimmed(cpufreq_base_path_buf.join("energy_performance_bias")).ok(); let platform_profile = read_sysfs_file_trimmed("/sys/firmware/acpi/platform_profile").ok(); - let _platform_profile_choices = - read_sysfs_file_trimmed("/sys/firmware/acpi/platform_profile_choices").ok(); // Calculate average CPU temperature from the core temperatures let average_temperature_celsius = if cpu_cores.is_empty() { @@ -471,6 +485,7 @@ pub fn get_cpu_global_info(cpu_cores: &[CpuCoreInfo]) -> CpuGlobalInfo { } }; + // Return the constructed CpuGlobalInfo CpuGlobalInfo { current_governor, available_governors, diff --git a/src/util/error.rs b/src/util/error.rs index 3ee0a86..7b92b8f 100644 --- a/src/util/error.rs +++ b/src/util/error.rs @@ -8,6 +8,9 @@ pub enum ControlError { NotSupported(String), PermissionDenied(String), InvalidProfile(String), + InvalidGovernor(String), + ParseError(String), + ReadError(String), } impl From for ControlError { @@ -35,6 +38,15 @@ impl std::fmt::Display for ControlError { "Invalid platform control profile {s} supplied, please provide a valid one." ) } + Self::InvalidGovernor(s) => { + write!(f, "Invalid governor: {s}") + } + Self::ParseError(s) => { + write!(f, "Failed to parse value: {s}") + } + Self::ReadError(s) => { + write!(f, "Failed to read sysfs path: {s}") + } } } }