balloons: set frequency scaling governor only when requested

Avoid enforcing the frequency scaling governor if the user hasn't explicitly requested it. Previously, we attempted to set it regardless, leading to unnecessary error logs. Furthermore, fix formatting issue when logging error case. Signed-off-by: Feruzjon Muyassarov <[email protected]>
containers · Oct 14, 2024 · dc2c310 · dc2c310
1 parent 09d705a
commit dc2c310
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 26 deletions.
diff --git a/pkg/resmgr/control/cpu/api.go b/pkg/resmgr/control/cpu/api.go
@@ -61,9 +61,6 @@ func Assign(c cache.Cache, class string, cpus ...int) error {
 		if err := ctl.enforceCpufreq(class, cpus...); err != nil {
 			log.Error("cpufreq enforcement failed: %v", err)
 		}
-		if err := ctl.enforceCpufreqGovernor(class, cpus...); err != nil {
-			log.Error("cpufreq governor enforcement failed: %v", err)
-		}
 		if err := ctl.enforceUncore(assignments, cpus...); err != nil {
 			log.Error("uncore frequency enforcement failed: %v", err)
 		}

diff --git a/pkg/resmgr/control/cpu/cpu.go b/pkg/resmgr/control/cpu/cpu.go
@@ -16,7 +16,6 @@ package cpu
 
 import (
 	"fmt"
-	"strconv"
 
 	"github.com/containers/nri-plugins/pkg/utils/cpuset"
 
@@ -123,30 +122,15 @@ func (ctl *cpuctl) PostStopHook(c cache.Container) error {
 	return nil
 }
 
-// enforceCpufreqGovernor enforces a class-specific cpufreq governor to a cpuset.
-func (ctl *cpuctl) enforceCpufreqGovernor(class string, cpusIds ...int) error {
-	if _, ok := ctl.classes[class]; !ok {
-		return fmt.Errorf("non-existent cpu class %q", class)
-	}
-	governor := ctl.classes[class].FreqGovernor
-	for cpu := range cpusIds {
-		log.Info(strconv.Itoa(cpu), governor)
-	}
-	log.Debug("enforcing cpu frequency governor %q on %v", governor, cpusIds)
-	if err := utils.SetScalingGovernorForCPUs(cpusIds, governor); err != nil {
-		return fmt.Errorf("Cannot set cpufreq governor %d: %w", governor, err)
-	}
-	return nil
-}
-
 // enforceCpufreq enforces a class-specific cpufreq configuration to a cpuset
 func (ctl *cpuctl) enforceCpufreq(class string, cpus ...int) error {
-	if _, ok := ctl.classes[class]; !ok {
+	c, ok := ctl.classes[class]
+	if !ok {
 		return fmt.Errorf("non-existent cpu class %q", class)
 	}
 
-	min := int(ctl.classes[class].MinFreq)
-	max := int(ctl.classes[class].MaxFreq)
+	min := int(c.MinFreq)
+	max := int(c.MaxFreq)
 	log.Debug("enforcing cpu frequency limits {%d, %d} from class %q on %v", min, max, class, cpus)
 
 	if err := utils.SetCPUsScalingMinFreq(cpus, min); err != nil {
@@ -157,6 +141,14 @@ func (ctl *cpuctl) enforceCpufreq(class string, cpus ...int) error {
 		return fmt.Errorf("Cannot set max freq %d: %w", max, err)
 	}
 
+	if governor := c.FreqGovernor; governor != "" {
+		log.Debug("enforcing cpu frequency governor %q from class %q on %v", governor, class, cpus)
+
+		if err := utils.SetScalingGovernorForCPUs(cpus, governor); err != nil {
+			return fmt.Errorf("cannot set cpufreq governor %q: %w", governor, err)
+		}
+	}
+
 	return nil
 }
 
@@ -276,9 +268,6 @@ func (ctl *cpuctl) configure(cfg *cfgapi.Config) error {
 			if err := ctl.enforceCpufreq(class, cpus.SortedMembers()...); err != nil {
 				log.Error("cpufreq enforcement on re-configure failed: %v", err)
 			}
-			if err := ctl.enforceCpufreqGovernor(class, cpus.SortedMembers()...); err != nil {
-				log.Error("cpufreq enforcement on re-configure failed: %v", err)
-			}
 		} else {
 			// TODO: what should we really do with classes that do not exist in
 			// the configuration anymore? Now we remember the CPUs assigned to