From ccbad0c0bc339e3c2391371ac55e817bf3e7f076 Mon Sep 17 00:00:00 2001 From: Frame Date: Tue, 26 Apr 2022 22:02:30 +0800 Subject: [PATCH] fix cpuinfo panic on arm64 (#97) Signed-off-by: saintube --- pkg/util/cpuinfo.go | 16 ++++--- pkg/util/cpuinfo_test.go | 33 ++++++++++++++ pkg/util/system/lscpu.go | 51 +++++++++++++++++++++ pkg/util/system/lscpu_arm64.go | 57 +++++++++++++++++++++++ pkg/util/system/lscpu_arm64_test.go | 71 +++++++++++++++++++++++++++++ 5 files changed, 221 insertions(+), 7 deletions(-) create mode 100644 pkg/util/system/lscpu.go create mode 100644 pkg/util/system/lscpu_arm64.go create mode 100644 pkg/util/system/lscpu_arm64_test.go diff --git a/pkg/util/cpuinfo.go b/pkg/util/cpuinfo.go index 9a8e976f3..b91f62c36 100644 --- a/pkg/util/cpuinfo.go +++ b/pkg/util/cpuinfo.go @@ -27,6 +27,8 @@ import ( "time" "k8s.io/klog/v2" + + "github.com/koordinator-sh/koordinator/pkg/util/system" ) const cpuCmdTimeout = 3 * time.Second @@ -135,7 +137,7 @@ func getProcessorInfos(lsCPUStr string) ([]ProcessorInfo, error) { var processorInfos []ProcessorInfo for _, line := range strings.Split(lsCPUStr, "\n") { items := strings.Fields(line) - if len(items) < 4 { + if len(items) < 6 { continue } cpu, err := strconv.Atoi(items[0]) @@ -151,25 +153,25 @@ func getProcessorInfos(lsCPUStr string) ([]ProcessorInfo, error) { if err != nil { continue } - cacheInfos := strings.TrimSpace(items[4]) - online := strings.TrimSpace(items[5]) - cacheinfo := strings.Split(strings.TrimSpace(cacheInfos), ":") - l1l2 := cacheinfo[0] - l3, err := strconv.Atoi(cacheinfo[3]) + l1l2, l3, err := system.GetCacheInfo(items[4]) if err != nil { continue } + online := strings.TrimSpace(items[5]) info := ProcessorInfo{ CPUID: int32(cpu), CoreID: int32(core), SocketID: int32(socket), NodeID: int32(node), L1dl1il2: l1l2, - L3: int32(l3), + L3: l3, Online: online, } processorInfos = append(processorInfos, info) } + if len(processorInfos) <= 0 { + return nil, fmt.Errorf("no valid processor info") + } // sorted by cpu topology // NOTE: in some cases, max(cpuId[...]) can be not equal to len(processors) diff --git a/pkg/util/cpuinfo_test.go b/pkg/util/cpuinfo_test.go index f4f96cf91..73dde009e 100644 --- a/pkg/util/cpuinfo_test.go +++ b/pkg/util/cpuinfo_test.go @@ -38,6 +38,39 @@ func Test_getProcessorInfos(t *testing.T) { want: nil, wantErr: true, }, + { + name: "do not panic for invalid lsCPUStr", + args: args{ + lsCPUStr: ` +CPU NODE SOCKET CORE L1d:L1i:L2:L3 ONLINE +0 0 0 - - yes +1 0 0 - - yes +`}, + want: nil, + wantErr: true, + }, + { + name: "do not panic for invalid cache info", + args: args{ + lsCPUStr: ` +CPU NODE SOCKET CORE L1d:L1i:L2:L3 ONLINE +0 0 0 0 - yes +1 0 0 0 no cacheinfo yes +`}, + want: nil, + wantErr: true, + }, + { + name: "do not panic for invalid cache info 1", + args: args{ + lsCPUStr: ` +CPU NODE SOCKET CORE L1d:L1i:L2:L3 ONLINE +0 0 0 0 3:3:3:- yes +1 0 0 1 3:3:3:- yes +`}, + want: nil, + wantErr: true, + }, { name: "read partial lsCPUStr", args: args{ diff --git a/pkg/util/system/lscpu.go b/pkg/util/system/lscpu.go new file mode 100644 index 000000000..95f03ccc6 --- /dev/null +++ b/pkg/util/system/lscpu.go @@ -0,0 +1,51 @@ +//go:build !arm64 +// +build !arm64 + +/* + Copyright 2022 The Koordinator Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package system + +import ( + "fmt" + "strconv" + "strings" +) + +// GetCacheInfo parses the output of `lscpu -e=CACHE` into l1l2 and l3 infos +// e.g. +// - input: "1:1:1:0" +// - output: "1", 0, nil +func GetCacheInfo(str string) (string, int32, error) { + // e.g. + // $ `lscpu -e=CPU,NODE,SOCKET,CORE,CACHE,ONLINE` + // CPU NODE SOCKET CORE L1d:L1i:L2:L3 ONLINE + // 0 0 0 0 0:0:0:0 yes + // 1 0 0 0 0:0:0:0 yes + // 2 0 0 1 1:1:1:0 yes + // 3 0 0 1 1:1:1:0 yes + infos := strings.Split(strings.TrimSpace(str), ":") + // assert l1, l2 are private cache, so they has the same id with the core + if len(infos) != 4 { + return "", 0, fmt.Errorf("invalid cache info %s", str) + } + l1l2 := infos[0] + l3, err := strconv.Atoi(infos[3]) + if err != nil { + return "", 0, err + } + return l1l2, int32(l3), nil +} diff --git a/pkg/util/system/lscpu_arm64.go b/pkg/util/system/lscpu_arm64.go new file mode 100644 index 000000000..a103861d8 --- /dev/null +++ b/pkg/util/system/lscpu_arm64.go @@ -0,0 +1,57 @@ +//go:build arm64 +// +build arm64 + +/* + Copyright 2022 The Koordinator Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package system + +import ( + "fmt" + "strconv" + "strings" +) + +// GetCacheInfo parses the output of `lscpu -e=CACHE` into l1l2 and l3 infos +// e.g. +// - input: "-" +// - output: "0", 0, nil +func GetCacheInfo(str string) (string, int32, error) { + // NOTE: `lscpu` can return empty cache info on arm64 platforms. We return a mocked info in this case. + // e.g. + // $ `lscpu -e=CPU,NODE,SOCKET,CORE,CACHE,ONLINE` + // CPU NODE SOCKET CORE CACHE ONLINE + // 0 0 0 0 - yes + // 1 0 0 1 - yes + // 2 0 0 2 - yes + // 3 0 0 3 - yes + s := strings.TrimSpace(str) + if s == "-" { + return "0", 0, nil + } + // otherwise we suppose the input is valid cache info + // assert l1, l2 are private cache, so they has the same id with the core + infos := strings.Split(s, ":") + if len(infos) != 4 { + return "", 0, fmt.Errorf("invalid format for cache info") + } + l1l2 := infos[0] + l3, err := strconv.Atoi(infos[3]) + if err != nil { + return "", 0, err + } + return l1l2, int32(l3), nil +} diff --git a/pkg/util/system/lscpu_arm64_test.go b/pkg/util/system/lscpu_arm64_test.go new file mode 100644 index 000000000..c398b64ff --- /dev/null +++ b/pkg/util/system/lscpu_arm64_test.go @@ -0,0 +1,71 @@ +//go:build arm64 +// +build arm64 + +/* + Copyright 2022 The Koordinator Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package system + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestGetCacheInfo(t *testing.T) { + tests := []struct { + name string + arg string + want string + want1 int32 + wantErr bool + }{ + { + name: "empty cache info", + arg: "- ", + want: "0", + want1: 0, + wantErr: false, + }, + { + name: "empty cache info 1", + arg: "-", + want: "0", + want1: 0, + wantErr: false, + }, + { + name: "valid cache info", + arg: "1:1:1:0", + want: "1", + want1: 0, + wantErr: false, + }, + { + name: "invalid cache info", + arg: "1:1:1:xxxx", + want: "", + want1: 0, + wantErr: true, + }, + } + for _, tt := range tests { + got, got1, gotErr := GetCacheInfo(tt.arg) + assert.Equal(t, tt.want, got) + assert.Equal(t, tt.want1, got1) + assert.Equal(t, tt.wantErr, gotErr != nil) + } +}