Change the format of name() API and remove remaining device_index

furiosa-ai · Apr 16, 2024 · e3aa449 · e3aa449
1 parent e171f6b
commit e3aa449
Show file tree

Hide file tree

Showing 12 changed files with 61 additions and 52 deletions.
diff --git a/bindings/python/furiosa_device.pyi b/bindings/python/furiosa_device.pyi
@@ -60,7 +60,7 @@ class DeviceConfig:
 class DeviceFile:
     def path(self) -> str: ...
     def filename(self) -> str: ...
-    def device_index(self) -> int: ...
+    def devfile_index(self) -> int: ...
     def core_range(self) -> CoreRange: ...
     def mode(self) -> DeviceMode: ...
     def __repr(self) -> str: ...
@@ -87,7 +87,7 @@ class PerformanceCounter:
 
 class Device:
     def name(self) -> str: ...
-    def device_index(self) -> int: ...
+    def devfile_index(self) -> int: ...
     def arch(self) -> Arch: ...
     def alive(self) -> bool: ...
     def atr_error(self) -> List[Dict[str, int]]: ...

diff --git a/bindings/python/src/device.rs b/bindings/python/src/device.rs
@@ -132,8 +132,8 @@ impl DevicePy {
         self.inner.name()
     }
 
-    /// Returns the device index (e.g., 0 for npu0).
-    fn device_index(&self) -> u8 {
+    /// Returns the device file index (e.g., 0 for npu0).
+    fn devfile_index(&self) -> u8 {
         self.inner.devfile_index()
     }
 
@@ -360,7 +360,7 @@ impl DeviceFilePy {
     }
 
     /// Returns the device index (e.g., 1 for npu1pe0).
-    fn device_index(&self) -> u8 {
+    fn devfile_index(&self) -> u8 {
         self.inner.devfile_index()
     }
 

diff --git a/device-api/bin/list_clock_frequency.rs b/device-api/bin/list_clock_frequency.rs
@@ -3,7 +3,7 @@ use furiosa_device::{list_devices, DeviceError};
 #[tokio::main]
 async fn main() -> Result<(), DeviceError> {
     for device in list_devices().await? {
-        println!("-- npu{} --", device.devfile_index());
+        println!("-- {} --", device);
         for frequency in device.clock_frequency()? {
             println!(
                 "{:15}: {} {}",

diff --git a/device-api/bin/list_hwmon.rs b/device-api/bin/list_hwmon.rs
@@ -5,7 +5,7 @@ async fn main() -> Result<(), DeviceError> {
     for device in list_devices().await? {
         let fetcher = device.get_hwmon_fetcher();
 
-        println!("-- npu{} --", device.devfile_index());
+        println!("-- {} --", device);
         println!("Current");
         for sensor_value in fetcher.read_currents().await? {
             println!(

diff --git a/device-api/src/arch/renegade.rs b/device-api/src/arch/renegade.rs
@@ -1,4 +1,5 @@
 use std::collections::HashMap;
+use std::fmt::Debug;
 use std::path::PathBuf;
 
 use strum::IntoEnumIterator;
@@ -11,7 +12,7 @@ use crate::Arch;
 use crate::ClockFrequency;
 use crate::DeviceError;
 
-#[derive(Clone)]
+#[derive(Clone, Debug)]
 pub struct RenegadeInner {
     arch: Arch,
     devfile_index: u8,
@@ -21,23 +22,23 @@ pub struct RenegadeInner {
 }
 
 impl RenegadeInner {
-    pub fn new(arch: Arch, device_index: u8, sysfs: PathBuf) -> DeviceResult<Self> {
+    pub fn new(arch: Arch, devfile_index: u8, sysfs: PathBuf) -> DeviceResult<Self> {
         let mgmt_root = sysfs.join(format!(
-            "class/renegade_mgmt/renegade!npu{device_index}mgmt"
+            "class/renegade_mgmt/renegade!npu{devfile_index}mgmt"
         ));
         let mgmt_cache = MgmtCache::init(&mgmt_root, StaticMgmtFile::iter())?;
 
         Ok(RenegadeInner {
             arch,
-            devfile_index: device_index,
+            devfile_index,
             sysfs,
             mgmt_root,
             mgmt_cache,
         })
     }
 }
 
-#[derive(Clone, PartialEq, Eq, Hash, EnumIter)]
+#[derive(Clone, Debug, PartialEq, Eq, Hash, EnumIter)]
 enum StaticMgmtFile {
     BusName,
     Dev,
@@ -82,6 +83,10 @@ impl DeviceMgmt for RenegadeInner {
         self.arch
     }
 
+    fn name(&self) -> String {
+        format!("/dev/renegade/npu{}", self.devfile_index())
+    }
+
     fn alive(&self) -> DeviceResult<bool> {
         self.read_mgmt_to_string(npu_mgmt::file::DEVICE_STATE)
             .map(|v| v == "good")

diff --git a/device-api/src/arch/warboy.rs b/device-api/src/arch/warboy.rs
@@ -1,4 +1,5 @@
 use std::collections::HashMap;
+use std::fmt::Debug;
 use std::path::PathBuf;
 
 use strum::IntoEnumIterator;
@@ -10,7 +11,7 @@ use crate::perf_regs::PerformanceCounter;
 use crate::sysfs::npu_mgmt::{self, MgmtCache, MgmtFile, MgmtFileIO};
 use crate::{Arch, ClockFrequency, DeviceError, DeviceFile};
 
-#[derive(Clone)]
+#[derive(Clone, Debug)]
 pub struct WarboyInner {
     arch: Arch,
     devfile_index: u8,
@@ -20,21 +21,21 @@ pub struct WarboyInner {
 }
 
 impl WarboyInner {
-    pub fn new(arch: Arch, device_index: u8, sysfs: PathBuf) -> DeviceResult<Self> {
-        let mgmt_root = sysfs.join(format!("class/npu_mgmt/npu{device_index}_mgmt"));
+    pub fn new(arch: Arch, devfile_index: u8, sysfs: PathBuf) -> DeviceResult<Self> {
+        let mgmt_root = sysfs.join(format!("class/npu_mgmt/npu{devfile_index}_mgmt"));
         let mgmt_cache = MgmtCache::init(&mgmt_root, StaticMgmtFile::iter())?;
 
         Ok(WarboyInner {
             arch,
-            devfile_index: device_index,
+            devfile_index,
             sysfs,
             mgmt_root,
             mgmt_cache,
         })
     }
 }
 
-#[derive(Clone, PartialEq, Eq, Hash, EnumIter)]
+#[derive(Clone, Debug, PartialEq, Eq, Hash, EnumIter)]
 enum StaticMgmtFile {
     BusName,
     Dev,
@@ -78,6 +79,10 @@ impl DeviceMgmt for WarboyInner {
         self.arch
     }
 
+    fn name(&self) -> String {
+        format!("/dev/npu{}", self.devfile_index())
+    }
+
     fn alive(&self) -> DeviceResult<bool> {
         self.read_mgmt_to_string(npu_mgmt::file::ALIVE)
             .and_then(|v| {

diff --git a/device-api/src/blocking.rs b/device-api/src/blocking.rs
@@ -100,11 +100,11 @@ pub(crate) fn get_device_inner(
     if is_furiosa_device(arch, idx, sysfs) {
         let inner = arch.create_inner(idx, devfs, sysfs)?;
         let busname = inner.busname();
-        let hwmon_fetcher = hwmon_fetcher_new(sysfs, idx, &busname)?;
+        let hwmon_fetcher = hwmon_fetcher_new(sysfs, &inner.name(), &busname)?;
         let device = collect_devices(inner, hwmon_fetcher, paths)?;
         Ok(device)
     } else {
-        Err(DeviceError::device_not_found(format!("npu{idx}")))
+        Err(DeviceError::device_not_found(format!("{arch}/{idx}")))
     }
 }
 
@@ -180,14 +180,14 @@ pub fn get_status_all(device: &Device) -> DeviceResult<HashMap<CoreIdx, CoreStat
 
 fn hwmon_fetcher_new(
     base_dir: &str,
-    device_index: u8,
+    device_name: &str,
     busname: &str,
 ) -> DeviceResult<hwmon::Fetcher> {
     Ok(hwmon::Fetcher {
-        device_index,
+        device_name: device_name.to_string(),
         sensor_container: hwmon::SensorContainer::new_blocking(base_dir, busname).map_err(
             |cause| DeviceError::HwmonError {
-                device_index,
+                device_name: device_name.to_string(),
                 cause,
             },
         )?,

diff --git a/device-api/src/device.rs b/device-api/src/device.rs
@@ -46,22 +46,17 @@ pub struct Device {
 }
 
 pub(crate) trait DeviceInner:
-    DeviceMgmt + DeviceCtrl + DevicePerf + DynClone + Send + Sync
+    DeviceMgmt + DeviceCtrl + DevicePerf + DynClone + Send + Sync + Debug
 {
 }
 
 dyn_clone::clone_trait_object!(DeviceInner);
 
-impl core::fmt::Debug for dyn DeviceInner {
-    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
-        write!(f, "{}/{}", self.arch(), self.devfile_index())
-    }
-}
-
 pub(crate) trait DeviceMgmt {
     fn sysfs(&self) -> &PathBuf;
-    fn devfile_index(&self) -> u8;
     fn arch(&self) -> Arch;
+    fn devfile_index(&self) -> u8;
+    fn name(&self) -> String;
     fn busname(&self) -> String;
     fn pci_dev(&self) -> String;
     fn device_sn(&self) -> String;
@@ -100,12 +95,12 @@ impl Device {
         }
     }
 
-    /// Returns the name of the device (e.g., npu0).
+    /// Returns the name of the device, represented by the common prefix of the character device file (e.g., /dev/npu0).
     pub fn name(&self) -> String {
-        format!("npu{}", self.devfile_index())
+        self.inner.name()
     }
 
-    /// Returns the device index (e.g., 0 for npu0).
+    /// Returns the device file index (e.g., 0 for /dev/npu0).
     pub fn devfile_index(&self) -> u8 {
         self.inner.devfile_index()
     }
@@ -289,7 +284,7 @@ impl Device {
 
 impl Display for Device {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
-        write!(f, "npu{}", self.devfile_index())
+        write!(f, "{}", self.name())
     }
 }
 

diff --git a/device-api/src/error.rs b/device-api/src/error.rs
@@ -26,8 +26,11 @@ pub enum DeviceError {
     UnknownArch { arch: String, rev: String },
     #[error("Incompatible device driver: {cause}")]
     IncompatibleDriver { cause: String },
-    #[error("HwmonError: [npu{device_index}] {cause}")]
-    HwmonError { device_index: u8, cause: HwmonError },
+    #[error("HwmonError: [{device_name}] {cause}")]
+    HwmonError {
+        device_name: String,
+        cause: HwmonError,
+    },
     #[error("PerformanceCounterError: {cause}")]
     PerformanceCounterError { cause: PerformanceCounterError },
     #[error("Unexpected value: {message}")]
@@ -61,11 +64,8 @@ impl DeviceError {
         }
     }
 
-    pub(crate) fn hwmon_error(device_index: u8, cause: HwmonError) -> DeviceError {
-        DeviceError::HwmonError {
-            device_index,
-            cause,
-        }
+    pub(crate) fn hwmon_error(device_name: String, cause: HwmonError) -> DeviceError {
+        DeviceError::HwmonError { device_name, cause }
     }
 
     pub(crate) fn performance_counter_error(cause: PerformanceCounterError) -> DeviceError {

diff --git a/device-api/src/hwmon.rs b/device-api/src/hwmon.rs
@@ -373,18 +373,22 @@ pub struct SensorValue {
 
 #[derive(Debug, PartialEq, Eq, Clone)]
 pub struct Fetcher {
-    pub(crate) device_index: u8,
+    pub(crate) device_name: String,
     pub(crate) sensor_container: SensorContainer,
 }
 
 impl Fetcher {
-    pub(crate) async fn new(base_dir: &str, device_index: u8, busname: &str) -> DeviceResult<Self> {
+    pub(crate) async fn new(
+        base_dir: &str,
+        device_name: &str,
+        busname: &str,
+    ) -> DeviceResult<Self> {
         let sensor_container = SensorContainer::new(base_dir, busname)
             .await
-            .map_err(|e| DeviceError::hwmon_error(device_index, e))?;
+            .map_err(|e| DeviceError::hwmon_error(device_name.to_string(), e))?;
 
         Ok(Self {
-            device_index,
+            device_name: device_name.to_string(),
             sensor_container,
         })
     }
@@ -428,11 +432,11 @@ impl Fetcher {
             for sensor in sensors {
                 let (label, value) = sensor
                     .read_blocking(name)
-                    .map_err(|e| DeviceError::hwmon_error(self.device_index, e))?;
+                    .map_err(|e| DeviceError::hwmon_error(self.device_name.clone(), e))?;
 
                 let value: i32 = value.parse().map_err(|_| {
                     DeviceError::hwmon_error(
-                        self.device_index,
+                        self.device_name.clone(),
                         error::HwmonError::UnexpectedValueFormat {
                             sensor_name: label.clone(),
                             value,
@@ -455,11 +459,11 @@ impl Fetcher {
                 let (label, value) = sensor
                     .read_item(name)
                     .await
-                    .map_err(|e| DeviceError::hwmon_error(self.device_index, e))?;
+                    .map_err(|e| DeviceError::hwmon_error(self.device_name.clone(), e))?;
 
                 let value: i32 = value.parse().map_err(|_| {
                     DeviceError::hwmon_error(
-                        self.device_index,
+                        self.device_name.clone(),
                         error::HwmonError::UnexpectedValueFormat {
                             sensor_name: label.clone(),
                             value,
@@ -620,7 +624,7 @@ mod tests {
 
     #[tokio::test]
     async fn fetcher_read_test() -> DeviceResult<()> {
-        let fetcher = Fetcher::new("../test_data/test-0/sys", 0, "0000:6d:00.0").await?;
+        let fetcher = Fetcher::new("../test_data/test-0/sys", "/dev/npu0", "0000:6d:00.0").await?;
 
         let currents = fetcher.read_currents().await?;
         assert_eq!(currents.len(), 2);

diff --git a/device-api/src/list.rs b/device-api/src/list.rs
@@ -55,7 +55,7 @@ pub(crate) async fn get_device_inner(
     if is_furiosa_device(arch, idx, sysfs).await {
         let inner = arch.create_inner(idx, devfs, sysfs)?;
         let busname = inner.busname();
-        let hwmon_fetcher = crate::hwmon::Fetcher::new(sysfs, idx, &busname).await?;
+        let hwmon_fetcher = crate::hwmon::Fetcher::new(sysfs, &inner.name(), &busname).await?;
 
         let device = collect_devices(inner, hwmon_fetcher, paths)?;
         Ok(device)

diff --git a/device-api/src/sysfs.rs b/device-api/src/sysfs.rs
@@ -138,7 +138,7 @@ pub mod npu_mgmt {
         fn filename(&self) -> &'static str;
     }
 
-    #[derive(Clone)]
+    #[derive(Clone, Debug)]
     pub(crate) struct MgmtCache<K: Eq + Hash + MgmtFile> {
         cache: HashMap<K, String>,
     }