Hotfix: try to handle module remove gracefully

Try to handle module remove more gracefully if in-use when the
display manager is shutting down

CHANGELOG.md

@@ -6,6 +6,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+# [3.1.3] - 2021-03-10
+### Changed
+- Hotfix: gracefully handle removing modules in use caused by display-manager not
+  fully shutdown at the time of trying to remove modules. It will now retry every
+  250ms per module
+
+# [3.1.2] - 2021-03-10
+### Changed
+- Test and create /etc/X11/xorg.conf.d/ if it doesn't exist
+- Hotfix to better report module issues
+
+# [3.1.1] - 2021-03-10
+### Changed
+- Add missing nvidia module nvidia_uvm to gfx ctrl list
+
 # [3.1.0] - 2021-03-09
 ### Added
 - GU502LU led-modes

Cargo.lock

@@ -187,7 +187,7 @@ dependencies = [
 
 [[package]]
 name = "daemon"
-version = "3.1.1"
+version = "3.1.3"
 dependencies = [
  "env_logger",
  "intel-pstate",

daemon/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "daemon"
-version = "3.1.1"
+version = "3.1.3"
 license = "MPL-2.0"
 readme = "README.md"
 authors = ["Luke <luke@ljones.dev>"]
@@ -40,4 +40,4 @@ toml = "^0.5"
 sysfs-class = "^0.1.2" # used for backlight control and baord ID
 rog_fan_curve = { version = "0.1", features = ["serde"] }
 # cpu power management
-intel-pstate = "^0.2"
+intel-pstate = "^0.2"

daemon/src/ctrl_gfx/error.rs

@@ -6,13 +6,7 @@ use crate::error::RogError;
 #[derive(Debug)]
 pub enum GfxError {
     ParseVendor,
-    Path(String, std::io::Error),
-    Read(String, std::io::Error),
-    Write(String, std::io::Error),
-    Module(String, std::io::Error),
     Bus(String, std::io::Error),
-    Command(String, std::io::Error),
-    Modprobe(String),
     DisplayManager(String),
 }
 
@@ -21,13 +15,7 @@ impl fmt::Display for GfxError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
             GfxError::ParseVendor => write!(f, "Could not parse vendor name"),
-            GfxError::Path(path, error) => write!(f, "Path {}: {}", path, error),
-            GfxError::Read(path, error) => write!(f, "Read {}: {}", path, error),
-            GfxError::Write(path, error) => write!(f, "Write {}: {}", path, error),
-            GfxError::Module(func, error) => write!(f, "Module error: {}: {}", func, error),
             GfxError::Bus(func, error) => write!(f, "Bus error: {}: {}", func, error),
-            GfxError::Command(func, error) => write!(f, "Command exec error: {}: {}", func, error),
-            GfxError::Modprobe(detail) => write!(f, "Modprobe error: {}", detail),
             GfxError::DisplayManager(detail) => write!(f, "Display manager: {}", detail),
         }
     }

daemon/src/ctrl_gfx/gfx.rs

@@ -2,10 +2,10 @@ use ctrl_gfx::error::GfxError;
 use ctrl_gfx::*;
 use log::{error, info, warn};
 use rog_types::gfx_vendors::GfxVendors;
-use std::io::Write;
 use std::iter::FromIterator;
 use std::process::Command;
 use std::str::FromStr;
+use std::{io::Write, ops::Add, path::Path};
 use std::{sync::Arc, sync::Mutex};
 use sysfs_class::{PciDevice, SysClass};
 use system::{GraphicsDevice, PciBus};
@@ -47,8 +47,9 @@ impl Dbus for CtrlGraphics {
 
     fn set_vendor(&mut self, vendor: String) {
         if let Ok(tmp) = GfxVendors::from_str(&vendor) {
+            info!("Switching gfx mode to {}", vendor);
             let msg = self.set_gfx_config(tmp).unwrap_or_else(|err| {
-                warn!("{}", err);
+                error!("{}", err);
                 format!("Failed: {}", err.to_string())
             });
             self.notify_gfx(&vendor)
@@ -173,7 +174,7 @@ impl CtrlGraphics {
 
     fn get_runtime_status() -> Result<String, RogError> {
         const PATH: &str = "/sys/bus/pci/devices/0000:01:00.0/power/runtime_status";
-        let buf = std::fs::read_to_string(PATH).map_err(|err| GfxError::Read(PATH.into(), err))?;
+        let buf = std::fs::read_to_string(PATH).map_err(|err| RogError::Read(PATH.into(), err))?;
         Ok(buf)
     }
 
@@ -190,7 +191,7 @@ impl CtrlGraphics {
             .arg(action)
             .arg("nvidia-fallback.service")
             .status()
-            .map_err(|err| GfxError::Command("systemctl".into(), err))?;
+            .map_err(|err| RogError::Command("systemctl".into(), err))?;
 
         if !status.success() {
             // Error is ignored in case this service is removed
@@ -210,17 +211,22 @@ impl CtrlGraphics {
             [PRIMARY_GPU_BEGIN, PRIMARY_GPU_END].concat()
         };
 
-        info!("Writing {}", PRIMARY_GPU_XORG_PATH);
+        if !Path::new(XORG_PATH).exists() {
+            std::fs::create_dir(XORG_PATH).map_err(|err| RogError::Write(XORG_PATH.into(), err))?;
+        }
+
+        let file = XORG_PATH.to_string().add(XORG_FILE);
+        info!("Writing {}", file);
         let mut file = std::fs::OpenOptions::new()
             .create(true)
             .truncate(true)
             .write(true)
-            .open(PRIMARY_GPU_XORG_PATH)
-            .map_err(|err| GfxError::Write(PRIMARY_GPU_XORG_PATH.into(), err))?;
+            .open(&file)
+            .map_err(|err| RogError::Write(file, err))?;
 
         file.write_all(&text)
             .and_then(|_| file.sync_all())
-            .map_err(|err| GfxError::Write(MODPROBE_PATH.into(), err))?;
+            .map_err(|err| RogError::Write(MODPROBE_PATH.into(), err))?;
         Ok(())
     }
 
@@ -232,11 +238,11 @@ impl CtrlGraphics {
             .truncate(true)
             .write(true)
             .open(MODPROBE_PATH)
-            .map_err(|err| GfxError::Path(MODPROBE_PATH.into(), err))?;
+            .map_err(|err| RogError::Path(MODPROBE_PATH.into(), err))?;
 
         file.write_all(MODPROBE_BASE)
             .and_then(|_| file.sync_all())
-            .map_err(|err| GfxError::Write(MODPROBE_PATH.into(), err))?;
+            .map_err(|err| RogError::Write(MODPROBE_PATH.into(), err))?;
 
         Ok(())
     }
@@ -249,55 +255,84 @@ impl CtrlGraphics {
         let removes = self.nvidia.iter().map(|dev| dev.remove());
 
         Result::from_iter(unbinds.chain(removes))
-            .map_err(|err| GfxError::Command("device unbind error".into(), err))?;
+            .map_err(|err| RogError::Command("device unbind error".into(), err))?;
 
         Ok(())
     }
 
     fn do_driver_action(driver: &str, action: &str) -> Result<(), RogError> {
-        let mut cmd = Command::new(action);
+        let mut cmd;
+        if Self::kmod_exists() {
+            info!("using kmod");
+            cmd = Command::new("kmod");
+            cmd.arg(action);
+        } else {
+            cmd = Command::new(action);
+        }
         cmd.arg(driver);
 
-        let status = cmd
-            .status()
-            .map_err(|err| GfxError::Command(format!("{:?}", cmd), err))?;
-        if !status.success() {
-            let msg = format!("{} {} failed: {:?}", action, driver, status);
-            error!("{}", msg);
-            return Err(GfxError::Modprobe(msg).into());
+        let mut count = 0;
+        const MAX_TRIES: i32 = 6;
+        loop {
+            if count > MAX_TRIES {
+                let msg = format!("{} {} failed for unknown reason", action, driver);
+                error!("{}", msg);
+                return Ok(()) //Err(RogError::Modprobe(msg));
+            }
+
+            let output = cmd
+                .output()
+                .map_err(|err| RogError::Command(format!("{:?}", cmd), err))?;
+            if !output.status.success() {
+                if output.stderr.ends_with("is not currently loaded\n".as_bytes()) {
+                    return Ok(())
+                }
+                if output.stderr.ends_with("Permission denied\n".as_bytes()) {
+                    let msg = format!("{} {} failed: {:?}", action, driver, String::from_utf8_lossy(&output.stderr));
+                    warn!("{}", msg);
+                    warn!("It may be safe to ignore the above error, run `lsmod |grep nvidia` to confirm modules loaded");
+                    return Ok(())
+                }
+                if count == MAX_TRIES {
+                    let msg = format!("{} {} failed: {:?}", action, driver, String::from_utf8_lossy(&output.stderr));
+                    return Err(RogError::Modprobe(msg));
+                }
+            }
+
+            count += 1;
+            std::thread::sleep(std::time::Duration::from_millis(250));
         }
-        Ok(())
     }
 
     fn do_display_manager_action(action: &str) -> Result<(), RogError> {
-        let service = "display-manager.service";
         let mut cmd = Command::new("systemctl");
         cmd.arg(action);
-        cmd.arg(service);
+        cmd.arg(DISPLAY_MANAGER);
 
         let status = cmd
             .status()
-            .map_err(|err| GfxError::Command(format!("{:?}", cmd), err))?;
+            .map_err(|err| RogError::Command(format!("{:?}", cmd), err))?;
         if !status.success() {
-            let msg = format!("systemctl {} {} failed: {:?}", action, service, status);
-            error!("{}", msg);
+            let msg = format!(
+                "systemctl {} {} failed: {:?}",
+                action, DISPLAY_MANAGER, status
+            );
             return Err(GfxError::DisplayManager(msg).into());
         }
         Ok(())
     }
 
     fn wait_display_manager_inactive() -> Result<(), RogError> {
-        let service = "display-manager.service";
         let mut cmd = Command::new("systemctl");
         cmd.arg("is-active");
-        cmd.arg(service);
+        cmd.arg(DISPLAY_MANAGER);
 
         let mut count = 0;
 
         while count <= 4 {
             let output = cmd
                 .output()
-                .map_err(|err| GfxError::Command(format!("{:?}", cmd), err))?;
+                .map_err(|err| RogError::Command(format!("{:?}", cmd), err))?;
             if output.stdout.starts_with("inactive".as_bytes()) {
                 return Ok(());
             }
@@ -309,6 +344,17 @@ impl CtrlGraphics {
         );
     }
 
+    fn kmod_exists() -> bool {
+        let mut cmd = Command::new("which");
+        cmd.arg("kmod");
+        if let Ok(output) = cmd
+        .output() {
+            return output.status.success() && output.stdout.ends_with("kmod".as_bytes())
+        }
+        //Path::new("/usr/bin/kmod").exists()
+        false
+    }
+
     pub fn do_vendor_tasks(&mut self, vendor: GfxVendors) -> Result<(), RogError> {
         Self::write_xorg_conf(vendor)?;
         Self::write_modprobe_conf()?; // TODO: Not required here, should put in startup?
@@ -318,18 +364,16 @@ impl CtrlGraphics {
             .rescan()
             .map_err(|err| GfxError::Bus("bus rescan error".into(), err))?;
 
-        let drivers = vec!["nvidia_drm", "nvidia_uvm", "nvidia_modeset", "nvidia"]; // i2c_nvidia_gpu?
-
         match vendor {
             GfxVendors::Nvidia | GfxVendors::Hybrid | GfxVendors::Compute => {
-                for driver in drivers {
+                for driver in NVIDIA_DRIVERS.iter() {
                     Self::do_driver_action(driver, "modprobe")?;
                 }
             }
             // TODO: compute mode, needs different setup
             // GfxVendors::Compute => {}
             GfxVendors::Integrated => {
-                for driver in drivers {
+                for driver in NVIDIA_DRIVERS.iter() {
                     Self::do_driver_action(driver, "rmmod")?;
                 }
                 self.unbind_remove_nvidia()?;

daemon/src/ctrl_gfx/mod.rs

@@ -4,6 +4,10 @@ pub mod gfx;
 
 pub mod system;
 
+const NVIDIA_DRIVERS: [&str; 4] = ["nvidia_drm", "nvidia_modeset", "nvidia_uvm", "nvidia"];
+
+const DISPLAY_MANAGER: &str = "display-manager.service";
+
 const MODPROBE_PATH: &str = "/etc/modprobe.d/asusd.conf";
 
 static MODPROBE_BASE: &[u8] = br#"# Automatically generated by asusd
@@ -15,7 +19,8 @@ options nvidia NVreg_DynamicPowerManagement=0x02
 options nvidia-drm modeset=1
 "#;
 
-const PRIMARY_GPU_XORG_PATH: &str = "/etc/X11/xorg.conf.d/90-nvidia-primary.conf";
+const XORG_FILE: &str = "90-nvidia-primary.conf";
+const XORG_PATH: &str = "/etc/X11/xorg.conf.d/";
 
 static PRIMARY_GPU_BEGIN: &[u8] = br#"# Automatically generated by asusd
 Section "OutputClass"

daemon/src/daemon.rs

@@ -31,9 +31,9 @@ pub fn main() -> Result<(), Box<dyn std::error::Error>> {
         .filter(None, LevelFilter::Info)
         .init();
 
-    info!("daemon version {}", daemon::VERSION);
-    info!(" rog-dbus version {}", rog_dbus::VERSION);
-    info!("rog-types version {}", rog_types::VERSION);
+    info!("   daemon v{}", daemon::VERSION);
+    info!(" rog-dbus v{}", rog_dbus::VERSION);
+    info!("rog-types v{}", rog_types::VERSION);
 
     start_daemon()?;
     Ok(())
@@ -121,7 +121,7 @@ fn start_daemon() -> Result<(), Box<dyn Error>> {
                     }
                 }
                 ctrl.reload()
-                    .unwrap_or_else(|err| warn!("Gfx controller: {}", err));
+                    .unwrap_or_else(|err| error!("Gfx controller: {}", err));
                 ctrl.add_to_server(&mut object_server);
             }
             Err(err) => {

daemon/src/error.rs

@@ -26,6 +26,8 @@ pub enum RogError {
     ReloadFail(String),
     GfxSwitching(GfxError),
     Initramfs(String),
+    Modprobe(String),
+    Command(String, std::io::Error),
 }
 
 impl fmt::Display for RogError {
@@ -50,6 +52,8 @@ impl fmt::Display for RogError {
             RogError::ReloadFail(deets) => write!(f, "Task error: {}", deets),
             RogError::GfxSwitching(deets) => write!(f, "Graphics switching error: {}", deets),
             RogError::Initramfs(detail) => write!(f, "Initiramfs error: {}", detail),
+            RogError::Modprobe(detail) => write!(f, "Modprobe error: {}", detail),
+            RogError::Command(func, error) => write!(f, "Command exec error: {}: {}", func, error),
         }
     }
 }

data/asusd.service

@@ -2,6 +2,7 @@
 Description=ASUS Notebook Control
 StartLimitInterval=200
 StartLimitBurst=2
+Before=display-manager.service
 
 [Service]
 ExecStart=/usr/bin/asusd
@@ -9,5 +10,4 @@ Restart=on-failure
 Restart=always
 RestartSec=1
 Type=dbus
-BusName=org.asuslinux.Daemon
-Before=display-manager.service
+BusName=org.asuslinux.Daemon