From f45c0c73a693140888050b73b51b1922bac7e307 Mon Sep 17 00:00:00 2001 From: Vimal Kumar Date: Sun, 14 Apr 2024 23:37:09 +0530 Subject: [PATCH] fix(acpi): Fixed how kepler searches for ACPI in hwmon ACPI power meter in hwmon need not always be present in "/sys/class/hwmon/hwmon2" This commit fixes this problem by - looking inside each folder in "/sys/class/hwmon" - improving the method to detect presece of acpi power meter. (this method is inspired from lm-sensors project) Signed-off-by: Vimal Kumar --- pkg/sensors/platform/power.go | 4 +- pkg/sensors/platform/source/acpi.go | 87 ++++++++++++++--------------- pkg/utils/utils.go | 15 +++++ 3 files changed, 58 insertions(+), 48 deletions(-) diff --git a/pkg/sensors/platform/power.go b/pkg/sensors/platform/power.go index 5f517d9856..bcfac1bc78 100644 --- a/pkg/sensors/platform/power.go +++ b/pkg/sensors/platform/power.go @@ -65,11 +65,11 @@ func InitPowerImpl() { powerImpl = &source.PowerHMC{} } else if redfish := source.NewRedfishClient(); redfish != nil && redfish.IsSystemCollectionSupported() { powerImpl = redfish - } else if acpi := source.NewACPIPowerMeter(); acpi != nil && acpi.CollectEnergy { + } else if acpi := source.NewACPIPowerMeter(); acpi != nil && acpi.IsInitialized { powerImpl = acpi } - klog.V(1).Infof("using %s to obtain power", powerImpl.GetName()) + klog.V(1).Infof("using %s to obtain platform power", powerImpl.GetName()) } func GetSourceName() string { diff --git a/pkg/sensors/platform/source/acpi.go b/pkg/sensors/platform/source/acpi.go index a9b4b31bf0..4c54f0643a 100644 --- a/pkg/sensors/platform/source/acpi.go +++ b/pkg/sensors/platform/source/acpi.go @@ -18,7 +18,6 @@ package source import ( "fmt" - "io/fs" "os" "path/filepath" "runtime" @@ -27,14 +26,14 @@ import ( "time" "github.com/sustainable-computing-io/kepler/pkg/config" + "github.com/sustainable-computing-io/kepler/pkg/utils" "k8s.io/klog/v2" ) const ( freqPathDir = "/sys/devices/system/cpu/cpufreq/" freqPath = "/sys/devices/system/cpu/cpufreq/policy%d/scaling_cur_freq" - hwmonPowerPath = "/sys/class/hwmon/hwmon2/device/" - acpiPowerPath = "/sys/devices/LNXSYSTM:00" + hwmonRoot = "/sys/class/hwmon" acpiPowerFilePrefix = "power" acpiPowerFileSuffix = "_average" poolingInterval = 3000 * time.Millisecond // in seconds @@ -48,53 +47,55 @@ var ( // Advanced Configuration and Power Interface (APCI) makes the system hardware sensor status // information available to the operating system via hwmon in sysfs. type ACPI struct { - CollectEnergy bool + IsInitialized bool powerPath string } func NewACPIPowerMeter() *ACPI { - acpi := &ACPI{powerPath: hwmonPowerPath} - if acpi.IsHWMONCollectionSupported() { - acpi.CollectEnergy = true - klog.V(5).Infof("Using the HWMON power meter path: %s\n", acpi.powerPath) - } else { - // if the acpi power_average file is not in the hwmon path, try to find the acpi path - acpi.powerPath = findACPIPowerPath() - if acpi.powerPath != "" { - acpi.CollectEnergy = true - klog.V(5).Infof("Using the ACPI power meter path: %s\n", acpi.powerPath) - } else { - klog.Infoln("Could not find any ACPI power meter path. Is it a VM?") - } + + path, err := detecthwmonACPIPath() + if err != nil { + klog.V(0).ErrorS(err, "initialization of ACPI power meter failed.") + return nil } + klog.V(0).Infof("acpi power source initialized with path: %q", path) - return acpi + return &ACPI{powerPath: path, IsInitialized: true} } -func findACPIPowerPath() string { - var powerPath string - err := filepath.WalkDir(acpiPowerPath, func(path string, info fs.DirEntry, err error) error { +/* +detecthwmonACPIPath looks for an entry in /sys/class/hwmon which has an attribute +"name" set to "power_meter" and a subsystem named "acpi" +*/ +func detecthwmonACPIPath() (string, error) { + d, err := os.ReadDir(hwmonRoot) + if err != nil { + return "", fmt.Errorf("could not read %s", hwmonRoot) + } + for _, ent := range d { + var name []byte + devicePath, err := utils.Realpath(filepath.Join(hwmonRoot, ent.Name(), "device")) if err != nil { - return err + return "", fmt.Errorf("error occurred in reading hwmon device %w", err) } - if info.IsDir() && (info.Name() == "power" || - strings.Contains(info.Name(), "INTL") || - strings.Contains(info.Name(), "PNP") || - strings.Contains(info.Name(), "input") || - strings.Contains(info.Name(), "device:") || - strings.Contains(info.Name(), "wakeup")) { - return filepath.SkipDir + name, err = os.ReadFile(filepath.Join(hwmonRoot, ent.Name(), "name")) + if err != nil { + name, err = os.ReadFile(filepath.Join(devicePath, "name")) + if err != nil { + return "", fmt.Errorf("error occurred in reading file %w", err) + } } - if !info.IsDir() && strings.Contains(info.Name(), "_average") { - powerPath = path[:(len(path) - len(info.Name()))] + strname := strings.Trim(string(name), "\n ") + ssname, err := utils.Realpath(filepath.Join(devicePath, "subsystem")) + if err != nil { + return "", fmt.Errorf("error occurred in reading hwmon device %w", err) + } + ssname = filepath.Base(ssname) + if strname == "power_meter" && ssname == "acpi" { + return devicePath, nil } - return nil - }) - if err != nil { - klog.V(3).Infof("Could not find any ACPI power meter path: %v\n", err) - return "" } - return powerPath + return "", fmt.Errorf("could not find acpi power meter in hwmon") } func (ACPI) GetName() string { @@ -143,22 +144,16 @@ func (a *ACPI) GetCPUCoreFrequency() map[int32]uint64 { } func (a *ACPI) IsSystemCollectionSupported() bool { - return a.CollectEnergy -} - -func (a *ACPI) IsHWMONCollectionSupported() bool { - // we do not use fmt.Sprintf because it is expensive in the performance standpoint - file := a.powerPath + acpiPowerFilePrefix + "1" + acpiPowerFileSuffix - _, err := os.ReadFile(file) - return err == nil + return a.IsInitialized } // GetEnergyFromHost returns the accumulated energy consumption func (a *ACPI) GetAbsEnergyFromPlatform() (map[string]float64, error) { power := map[string]float64{} + // TODO: the files in acpi power meter device does not depend on number of CPUs. The below loop will run only once for i := int32(1); i <= numCPUS; i++ { - path := a.powerPath + acpiPowerFilePrefix + strconv.Itoa(int(i)) + acpiPowerFileSuffix + path := a.powerPath + "/" + acpiPowerFilePrefix + strconv.Itoa(int(i)) + acpiPowerFileSuffix data, err := os.ReadFile(path) if err != nil { break diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 0166131d3c..63fc0df044 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -5,6 +5,7 @@ import ( "encoding/binary" "fmt" "os" + "path/filepath" "strings" "unsafe" ) @@ -69,3 +70,17 @@ func GetPathFromPID(searchPath string, pid uint64) (string, error) { } return "", fmt.Errorf("could not find cgroup description entry for pid %d", pid) } + +func Realpath(path string) (string, error) { + absPath, err := filepath.Abs(path) + if err != nil { + return "", err + } + + resolvedPath, err := filepath.EvalSymlinks(absPath) + if err != nil { + return "", err + } + + return resolvedPath, nil +}