Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug Fix: SystemCPUfreq fails when any core is offline #497

Merged
merged 1 commit into from
Mar 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
74 changes: 68 additions & 6 deletions sysfs/system_cpu.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,49 @@ func parseCPUThermalThrottle(cpuPath string) (*CPUThermalThrottle, error) {
return &t, nil
}

func binSearch(elem uint16, elemSlice *[]uint16) bool {
if len(*elemSlice) == 0 {
return false
}

if len(*elemSlice) == 1 {
return elem == (*elemSlice)[0]
}

start := 0
end := len(*elemSlice) - 1

var mid int

for start <= end {
mid = (start + end) / 2
if (*elemSlice)[mid] == elem {
return true
} else if (*elemSlice)[mid] > elem {
end = mid - 1
} else if (*elemSlice)[mid] < elem {
start = mid + 1
}
}

return false
}

func filterOfflineCPUs(offlineCpus *[]uint16, cpus *[]string) error {
for i, cpu := range *cpus {
cpuName := strings.TrimPrefix(filepath.Base(cpu), "cpu")
cpuNameUint16, err := strconv.Atoi(cpuName)
if err != nil {
return err
}
if binSearch(uint16(cpuNameUint16), offlineCpus) {
*cpus = append((*cpus)[:i], (*cpus)[i+1:]...)
}
}

return nil
}

// SystemCpufreq returns CPU frequency metrics for all CPUs.
func (fs FS) SystemCpufreq() ([]SystemCPUCpufreqStats, error) {
var g errgroup.Group
Expand All @@ -151,6 +194,25 @@ func (fs FS) SystemCpufreq() ([]SystemCPUCpufreqStats, error) {
return nil, err
}

line, err := util.ReadFileNoStat(fs.sys.Path("devices/system/cpu/offline"))
if err != nil {
return nil, err
}

if strings.TrimSpace(string(line)) != "" {
offlineCPUs, err := parseCPURange(line)
if err != nil {
return nil, err
}

if len(offlineCPUs) > 0 {
err = filterOfflineCPUs(&offlineCPUs, &cpus)
if err != nil {
return nil, err
}
}
}

systemCpufreq := make([]SystemCPUCpufreqStats, len(cpus))
for i, cpu := range cpus {
cpuName := strings.TrimPrefix(filepath.Base(cpu), "cpu")
Expand Down Expand Up @@ -251,12 +313,12 @@ func (fs FS) IsolatedCPUs() ([]uint16, error) {
return nil, err
}

return parseIsolatedCPUs(isolcpus)
return parseCPURange(isolcpus)
}

func parseIsolatedCPUs(data []byte) ([]uint16, error) {
func parseCPURange(data []byte) ([]uint16, error) {

var isolcpusInt = []uint16{}
var cpusInt = []uint16{}

for _, cpu := range strings.Split(strings.TrimRight(string(data), "\n"), ",") {
if cpu == "" {
Expand All @@ -277,7 +339,7 @@ func parseIsolatedCPUs(data []byte) ([]uint16, error) {
}

for i := startRange; i <= endRange; i++ {
isolcpusInt = append(isolcpusInt, uint16(i))
cpusInt = append(cpusInt, uint16(i))
}
continue
}
Expand All @@ -286,7 +348,7 @@ func parseIsolatedCPUs(data []byte) ([]uint16, error) {
if err != nil {
return nil, err
}
isolcpusInt = append(isolcpusInt, uint16(cpuN))
cpusInt = append(cpusInt, uint16(cpuN))
}
return isolcpusInt, nil
return cpusInt, nil
}
23 changes: 22 additions & 1 deletion sysfs/system_cpu_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ func TestIsolatedParsingCPU(t *testing.T) {
}
for _, params := range testParams {
t.Run("blabla", func(t *testing.T) {
res, err := parseIsolatedCPUs(params.in)
res, err := parseCPURange(params.in)
if !reflect.DeepEqual(res, params.res) {
t.Fatalf("should have %v result: got %v", params.res, res)
}
Expand Down Expand Up @@ -193,3 +193,24 @@ func TestIsolatedCPUs(t *testing.T) {
t.Errorf("Error not correct: want %v, have %v", nil, err)
}
}

func TestBinSearch(t *testing.T) {
var testParams = []struct {
elem uint16
elems []uint16
res bool
}{
{3, []uint16{1, 3, 5, 7, 9}, true},
{4, []uint16{1, 3, 5, 7, 9}, false},
{2, []uint16{}, false},
}

for _, param := range testParams {
res := binSearch(param.elem, &param.elems)

if res != param.res {
t.Fatalf("Result not correct: want %v, have %v", param.res, res)
}

}
}
5 changes: 5 additions & 0 deletions testdata/fixtures.ttar
Original file line number Diff line number Diff line change
Expand Up @@ -12721,6 +12721,11 @@ Lines: 1
1,2-7,9
Mode: 664
# ttar - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Path: fixtures/sys/devices/system/cpu/offline
Lines: 1
2,12-15
Mode: 664
# ttar - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Directory: fixtures/sys/devices/system/node
Mode: 775
# ttar - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Expand Down