From 43e0f83940d2f4ec2a001988702db26e1b6192a5 Mon Sep 17 00:00:00 2001 From: Markus Lehtonen Date: Thu, 12 Aug 2021 18:28:41 +0300 Subject: [PATCH] source/cpu: better error reporting Drop confusing errors in the log when intel pstate or cstate driver is not enabled in the system. However, we still log an error if sysfs is not available at all, in which case we're not able to detect these correctly. --- source/cpu/cpu.go | 6 +++--- source/cpu/cstate.go | 36 ++++++++++++++++++++++++++---------- source/cpu/pstate.go | 26 ++++++++++++++++++++------ 3 files changed, 49 insertions(+), 19 deletions(-) diff --git a/source/cpu/cpu.go b/source/cpu/cpu.go index 1544cd18d..6bc5878bb 100644 --- a/source/cpu/cpu.go +++ b/source/cpu/cpu.go @@ -147,10 +147,10 @@ func (s *Source) Discover() (source.Features, error) { } // Detect cstate configuration - cstate, err := detectCstate() + cstate, ok, err := detectCstate() if err != nil { - klog.Errorf("failed to detect cstate: %v", err) - } else { + klog.Error(err) + } else if ok { features["cstate.enabled"] = cstate } diff --git a/source/cpu/cstate.go b/source/cpu/cstate.go index a446925f5..10e475a0f 100644 --- a/source/cpu/cstate.go +++ b/source/cpu/cstate.go @@ -19,33 +19,49 @@ package cpu import ( "fmt" "io/ioutil" + "os" + "path/filepath" "strconv" "strings" + "k8s.io/klog/v2" + "sigs.k8s.io/node-feature-discovery/source" ) // Discover if c-states are enabled -func detectCstate() (bool, error) { - // When the intel_idle driver is in use (default), check setting of max_cstates - driver, err := ioutil.ReadFile(source.SysfsDir.Path("devices/system/cpu/cpuidle/current_driver")) - if err != nil { - return false, fmt.Errorf("cannot get driver for cpuidle: %s", err.Error()) +func detectCstate() (bool, bool, error) { + // Check that sysfs is available + sysfsBase := source.SysfsDir.Path("devices/system/cpu") + if _, err := os.Stat(sysfsBase); err != nil { + return false, false, fmt.Errorf("unable to detect cstate status: %w", err) + } + cpuidleDir := filepath.Join(sysfsBase, "cpuidle") + if _, err := os.Stat(cpuidleDir); os.IsNotExist(err) { + klog.V(1).Info("cpuidle disabled in the kernel") + return false, false, nil } - if strings.TrimSpace(string(driver)) != "intel_idle" { + // When the intel_idle driver is in use (default), check setting of max_cstates + driver, err := ioutil.ReadFile(filepath.Join(cpuidleDir, "current_driver")) + if err != nil { + return false, false, fmt.Errorf("cannot get driver for cpuidle: %w", err) + } + + if d := strings.TrimSpace(string(driver)); d != "intel_idle" { // Currently only checking intel_idle driver for cstates - return false, fmt.Errorf("intel_idle driver is not in use: %s", string(driver)) + klog.V(1).Infof("intel_idle driver is not in use (%s is active)", d) + return false, false, nil } data, err := ioutil.ReadFile(source.SysfsDir.Path("module/intel_idle/parameters/max_cstate")) if err != nil { - return false, fmt.Errorf("cannot determine cstate from max_cstates: %s", err.Error()) + return false, false, fmt.Errorf("cannot determine cstate from max_cstates: %w", err) } cstates, err := strconv.Atoi(strings.TrimSpace(string(data))) if err != nil { - return false, fmt.Errorf("non-integer value of cstates: %s", err.Error()) + return false, false, fmt.Errorf("non-integer value of cstates: %w", err) } - return cstates > 0, nil + return cstates > 0, true, nil } diff --git a/source/cpu/pstate.go b/source/cpu/pstate.go index c040df564..f100f7bd1 100644 --- a/source/cpu/pstate.go +++ b/source/cpu/pstate.go @@ -19,6 +19,8 @@ package cpu import ( "fmt" "io/ioutil" + "os" + "path/filepath" "runtime" "strings" @@ -35,10 +37,21 @@ func detectPstate() (map[string]string, error) { return nil, nil } + // Check that sysfs is available + sysfsBase := source.SysfsDir.Path("devices/system/cpu") + if _, err := os.Stat(sysfsBase); err != nil { + return nil, fmt.Errorf("unable to detect pstate status: %w", err) + } + pstateDir := filepath.Join(sysfsBase, "intel_pstate") + if _, err := os.Stat(pstateDir); os.IsNotExist(err) { + klog.V(1).Info("intel pstate driver not enabled") + return nil, nil + } + // Get global pstate status - data, err := ioutil.ReadFile(source.SysfsDir.Path("devices/system/cpu/intel_pstate/status")) + data, err := ioutil.ReadFile(filepath.Join(pstateDir, "status")) if err != nil { - return nil, fmt.Errorf("could not read pstate status: %s", err.Error()) + return nil, fmt.Errorf("could not read pstate status: %w", err) } status := strings.TrimSpace(string(data)) if status == "off" { @@ -49,7 +62,7 @@ func detectPstate() (map[string]string, error) { features := map[string]string{"status": status} // Check turbo boost - bytes, err := ioutil.ReadFile(source.SysfsDir.Path("devices/system/cpu/intel_pstate/no_turbo")) + bytes, err := ioutil.ReadFile(filepath.Join(pstateDir, "no_turbo")) if err != nil { klog.Errorf("can't detect whether turbo boost is enabled: %s", err.Error()) } else { @@ -65,7 +78,8 @@ func detectPstate() (map[string]string, error) { } // Determine scaling governor that is being used - policies, err := ioutil.ReadDir(source.SysfsDir.Path("devices/system/cpu/cpufreq")) + cpufreqDir := filepath.Join(sysfsBase, "cpufreq") + policies, err := ioutil.ReadDir(cpufreqDir) if err != nil { klog.Errorf("failed to read cpufreq directory: %s", err.Error()) return features, nil @@ -74,7 +88,7 @@ func detectPstate() (map[string]string, error) { scaling := "" for _, policy := range policies { // Ensure at least one cpu is using this policy - cpus, err := ioutil.ReadFile(source.SysfsDir.Path("devices/system/cpu/cpufreq", policy.Name(), "affected_cpus")) + cpus, err := ioutil.ReadFile(filepath.Join(cpufreqDir, policy.Name(), "affected_cpus")) if err != nil { klog.Errorf("could not read cpufreq policy %s affected_cpus", policy.Name()) continue @@ -84,7 +98,7 @@ func detectPstate() (map[string]string, error) { continue } - data, err := ioutil.ReadFile(source.SysfsDir.Path("devices/system/cpu/cpufreq", policy.Name(), "scaling_governor")) + data, err := ioutil.ReadFile(filepath.Join(cpufreqDir, policy.Name(), "scaling_governor")) if err != nil { klog.Errorf("could not read cpufreq policy %s scaling_governor", policy.Name()) continue