From e90375828db4c36e3fce634a01c8582d16474031 Mon Sep 17 00:00:00 2001 From: "Luke D. Jones" Date: Sun, 17 Dec 2023 21:23:50 +1300 Subject: [PATCH] Fix: nuke some async deadlocks in fan-curves --- CHANGELOG.md | 2 + Cargo.lock | 26 ++++++------ Cargo.toml | 4 +- asusd/src/ctrl_fancurves.rs | 82 ++++++++++++++++++++++--------------- asusd/src/ctrl_platform.rs | 38 +++++++++-------- 5 files changed, 89 insertions(+), 63 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ffadf763..3e7e4c6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [v5.0.2] ### Changed +- Fan-curves: nuke a few async deadlocks - Anime: force power/wakeup disabled to prevent idiotic random wakes ## [v5.0.1] diff --git a/Cargo.lock b/Cargo.lock index b56d8224..7629e405 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -199,7 +199,7 @@ checksum = "96d30a06541fbafbc7f82ed10c06164cfbd2c401138f6addd8404629c4b16711" [[package]] name = "asusctl" -version = "5.0.1" +version = "5.0.2" dependencies = [ "asusd", "cargo-husky", @@ -218,7 +218,7 @@ dependencies = [ [[package]] name = "asusd" -version = "5.0.1" +version = "5.0.2" dependencies = [ "async-trait", "cargo-husky", @@ -243,7 +243,7 @@ dependencies = [ [[package]] name = "asusd-user" -version = "5.0.1" +version = "5.0.2" dependencies = [ "cargo-husky", "config-traits", @@ -846,7 +846,7 @@ dependencies = [ [[package]] name = "config-traits" -version = "5.0.1" +version = "5.0.2" dependencies = [ "cargo-husky", "log", @@ -899,7 +899,7 @@ dependencies = [ [[package]] name = "cpuctl" -version = "5.0.1" +version = "5.0.2" [[package]] name = "cpufeatures" @@ -1026,7 +1026,7 @@ dependencies = [ [[package]] name = "dmi_id" -version = "5.0.1" +version = "5.0.2" dependencies = [ "log", "udev", @@ -2846,7 +2846,7 @@ checksum = "c08c74e62047bb2de4ff487b251e4a92e24f48745648451635cec7d591162d9f" [[package]] name = "rog-control-center" -version = "5.0.1" +version = "5.0.2" dependencies = [ "asusd", "cargo-husky", @@ -2879,7 +2879,7 @@ dependencies = [ [[package]] name = "rog_anime" -version = "5.0.1" +version = "5.0.2" dependencies = [ "cargo-husky", "dmi_id", @@ -2896,7 +2896,7 @@ dependencies = [ [[package]] name = "rog_aura" -version = "5.0.1" +version = "5.0.2" dependencies = [ "cargo-husky", "dmi_id", @@ -2910,7 +2910,7 @@ dependencies = [ [[package]] name = "rog_dbus" -version = "5.0.1" +version = "5.0.2" dependencies = [ "asusd", "cargo-husky", @@ -2923,7 +2923,7 @@ dependencies = [ [[package]] name = "rog_platform" -version = "5.0.1" +version = "5.0.2" dependencies = [ "cargo-husky", "concat-idents", @@ -2940,7 +2940,7 @@ dependencies = [ [[package]] name = "rog_profiles" -version = "5.0.1" +version = "5.0.2" dependencies = [ "cargo-husky", "log", @@ -2954,7 +2954,7 @@ dependencies = [ [[package]] name = "rog_simulators" -version = "5.0.1" +version = "5.0.2" dependencies = [ "glam", "log", diff --git a/Cargo.toml b/Cargo.toml index b7945cdc..7d4d01c1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,11 +4,11 @@ default-members = ["asusctl", "asusd", "asusd-user", "cpuctl", "rog-control-cent resolver = "2" [workspace.package] -version = "5.0.1" +version = "5.0.2" [workspace.dependencies] async-trait = "^0.1" -tokio = { version = "^1.23.0", features = ["macros", "sync", "rt-multi-thread"]} +tokio = { version = "^1.23.0", default-features = false, features = ["macros", "sync"]} concat-idents = "^1.1" dirs = "^4.0" smol = "^1.3" diff --git a/asusd/src/ctrl_fancurves.rs b/asusd/src/ctrl_fancurves.rs index ed8b1116..4fa44211 100644 --- a/asusd/src/ctrl_fancurves.rs +++ b/asusd/src/ctrl_fancurves.rs @@ -109,19 +109,18 @@ impl CtrlFanCurveZbus { } pub async fn update_profiles_from_config(&self) { - let mut fan_curves = self.fan_curves.lock().await; - let config = self.config.lock().await; - fan_curves.balanced = config.balanced.clone(); - fan_curves.performance = config.performance.clone(); - fan_curves.quiet = config.quiet.clone(); + self.fan_curves.lock().await.balanced = self.config.lock().await.balanced.clone(); + self.fan_curves.lock().await.performance = self.config.lock().await.performance.clone(); + self.fan_curves.lock().await.quiet = self.config.lock().await.quiet.clone(); } + /// Because this locks both config and fan_curves, it means nothing else can + /// hold a lock across this function call. Stupid choice to do this and + /// needs to be fixed. pub async fn update_config_from_profiles(&self) { - let fan_curves = self.fan_curves.lock().await; - let mut config = self.config.lock().await; - config.balanced = fan_curves.balanced.clone(); - config.performance = fan_curves.performance.clone(); - config.quiet = fan_curves.quiet.clone(); + self.config.lock().await.balanced = self.fan_curves.lock().await.balanced.clone(); + self.config.lock().await.performance = self.fan_curves.lock().await.performance.clone(); + self.config.lock().await.quiet = self.fan_curves.lock().await.quiet.clone(); } } @@ -134,10 +133,14 @@ impl CtrlFanCurveZbus { profile: PlatformPolicy, enabled: bool, ) -> zbus::fdo::Result<()> { - let mut fan_curves = self.fan_curves.lock().await; - fan_curves.set_profile_curves_enabled(profile, enabled); - fan_curves.write_profile_curve_to_platform(profile, &mut find_fan_curve_node()?)?; - + self.fan_curves + .lock() + .await + .set_profile_curves_enabled(profile, enabled); + self.fan_curves + .lock() + .await + .write_profile_curve_to_platform(profile, &mut find_fan_curve_node()?)?; self.update_config_from_profiles().await; self.config.lock().await.write(); Ok(()) @@ -151,10 +154,14 @@ impl CtrlFanCurveZbus { fan: FanCurvePU, enabled: bool, ) -> zbus::fdo::Result<()> { - let mut fan_curves = self.fan_curves.lock().await; - fan_curves.set_profile_fan_curve_enabled(profile, fan, enabled); - fan_curves.write_profile_curve_to_platform(profile, &mut find_fan_curve_node()?)?; - + self.fan_curves + .lock() + .await + .set_profile_fan_curve_enabled(profile, fan, enabled); + self.fan_curves + .lock() + .await + .write_profile_curve_to_platform(profile, &mut find_fan_curve_node()?)?; self.update_config_from_profiles().await; self.config.lock().await.write(); Ok(()) @@ -165,9 +172,13 @@ impl CtrlFanCurveZbus { &mut self, profile: PlatformPolicy, ) -> zbus::fdo::Result> { - let fan_curves = self.fan_curves.lock().await; - let curve = fan_curves.get_fan_curves_for(profile); - Ok(curve.to_vec()) + let curve = self + .fan_curves + .lock() + .await + .get_fan_curves_for(profile) + .to_vec(); + Ok(curve) } /// Set the fan curve for the specified profile. @@ -177,13 +188,16 @@ impl CtrlFanCurveZbus { profile: PlatformPolicy, curve: CurveData, ) -> zbus::fdo::Result<()> { - let mut fan_curves = self.fan_curves.lock().await; - fan_curves.save_fan_curve(curve, profile)?; - fan_curves.write_profile_curve_to_platform(profile, &mut find_fan_curve_node()?)?; - + self.fan_curves + .lock() + .await + .save_fan_curve(curve, profile)?; + self.fan_curves + .lock() + .await + .write_profile_curve_to_platform(profile, &mut find_fan_curve_node()?)?; self.update_config_from_profiles().await; self.config.lock().await.write(); - Ok(()) } @@ -193,10 +207,11 @@ impl CtrlFanCurveZbus { /// Each platform_profile has a different default and the defualt can be /// read only for the currently active profile. async fn set_active_curve_to_defaults(&mut self) -> zbus::fdo::Result<()> { - let mut fan_curves = self.fan_curves.lock().await; let active = self.platform.get_throttle_thermal_policy()?; - fan_curves.set_active_curve_to_defaults(active.into(), &mut find_fan_curve_node()?)?; - + self.fan_curves + .lock() + .await + .set_active_curve_to_defaults(active.into(), &mut find_fan_curve_node()?)?; self.update_config_from_profiles().await; self.config.lock().await.write(); Ok(()) @@ -208,15 +223,18 @@ impl CtrlFanCurveZbus { /// Each platform_profile has a different default and the defualt can be /// read only for the currently active profile. async fn reset_profile_curves(&self, profile: PlatformPolicy) -> zbus::fdo::Result<()> { - let mut fan_curves = self.fan_curves.lock().await; - let active = self .platform .get_throttle_thermal_policy() .unwrap_or(PlatformPolicy::Balanced.into()); self.platform.set_throttle_thermal_policy(profile.into())?; - fan_curves.set_active_curve_to_defaults(active.into(), &mut find_fan_curve_node()?)?; + { + self.fan_curves + .lock() + .await + .set_active_curve_to_defaults(active.into(), &mut find_fan_curve_node()?)?; + } self.platform.set_throttle_thermal_policy(active)?; self.update_config_from_profiles().await; diff --git a/asusd/src/ctrl_platform.rs b/asusd/src/ctrl_platform.rs index 35316051..24550c4d 100644 --- a/asusd/src/ctrl_platform.rs +++ b/asusd/src/ctrl_platform.rs @@ -4,7 +4,7 @@ use std::sync::Arc; use async_trait::async_trait; use config_traits::StdConfig; use log::{debug, error, info, warn}; -use rog_platform::cpu::CPUControl; +use rog_platform::cpu::{CPUControl, CPUGovernor}; use rog_platform::platform::{GpuMode, PlatformPolicy, Properties, RogPlatform}; use rog_platform::power::AsusPower; use zbus::export::futures_util::lock::Mutex; @@ -180,6 +180,23 @@ impl CtrlPlatform { } } + fn check_and_set_epp(&self, profile: PlatformPolicy) { + info!("PlatformPolicy setting EPP"); + if let Some(cpu) = self.cpu_control.as_ref() { + if let Ok(epp) = cpu.get_available_epp() { + debug!("Available EPP: {epp:?}"); + if epp.contains(&profile.into()) { + debug!("Setting {profile:?}"); + cpu.set_epp(profile.into()).ok(); + } else if let Ok(gov) = cpu.get_governor() { + if gov != CPUGovernor::Powersave { + warn!("powersave governor is not is use, you should use it."); + } + } + } + } + } + async fn update_policy_ac_or_bat(&self, power_plugged: bool) { let profile = if power_plugged { self.config.lock().await.platform_policy_on_ac @@ -189,9 +206,7 @@ impl CtrlPlatform { self.platform .set_throttle_thermal_policy(profile.into()) .ok(); - if let Some(cpu) = self.cpu_control.as_ref() { - cpu.set_epp(profile.into()).ok(); - } + self.check_and_set_epp(profile); } } @@ -325,10 +340,7 @@ impl CtrlPlatform { let policy = PlatformPolicy::next(&policy); if self.platform.has_throttle_thermal_policy() { - if let Some(cpu) = self.cpu_control.as_ref() { - info!("PlatformPolicy setting EPP"); - cpu.set_epp(policy.into())? - } + self.check_and_set_epp(policy); self.platform .set_throttle_thermal_policy(policy.into()) .map_err(|err| { @@ -354,10 +366,7 @@ impl CtrlPlatform { async fn set_throttle_thermal_policy(&mut self, policy: PlatformPolicy) -> Result<(), FdoErr> { // TODO: watch for external changes if self.platform.has_throttle_thermal_policy() { - if let Some(cpu) = self.cpu_control.as_ref() { - info!("PlatformPolicy setting EPP"); - cpu.set_epp(policy.into())? - } + self.check_and_set_epp(policy); self.config.lock().await.platform_policy_to_restore = policy; self.platform .set_throttle_thermal_policy(policy.into()) @@ -688,10 +697,7 @@ impl CtrlTask for CtrlPlatform { error!("Platform: get_throttle_thermal_policy error: {e}"); }) { - if let Some(cpu) = ctrl.cpu_control.as_ref() { - info!("PlatformPolicy setting EPP"); - cpu.set_epp(profile.into()).ok(); - } + ctrl.check_and_set_epp(profile); ctrl.config.lock().await.platform_policy_to_restore = profile; } }