Skip to content

Commit e9068e3

Browse files
iwankgbiwankgb
authored
Merge pull request #3318 from kishen-v/cpupath-fixes
2 parents a85a001 + b7e6727 commit e9068e3

File tree

4 files changed

+53
-40
lines changed

4 files changed

+53
-40
lines changed

‎machine/machine.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ var (
4545
swapCapacityRegexp = regexp.MustCompile(`SwapTotal:\s*([0-9]+) kB`)
4646
vendorIDRegexp = regexp.MustCompile(`vendor_id\s*:\s*(\w+)`)
4747

48-
cpuBusPath = "/sys/bus/cpu/devices/"
48+
cpuAttributesPath = "/sys/devices/system/cpu/"
4949
isMemoryController = regexp.MustCompile("mc[0-9]+")
5050
isDimm = regexp.MustCompile("dimm[0-9]+")
5151
machineArch = getMachineArch()
@@ -76,7 +76,7 @@ func GetPhysicalCores(procInfo []byte) int {
7676
if numCores == 0 {
7777
// read number of cores from /sys/bus/cpu/devices/cpu*/topology/core_id to deal with processors
7878
// for which 'core id' is not available in /proc/cpuinfo
79-
numCores = sysfs.GetUniqueCPUPropertyCount(cpuBusPath, sysfs.CPUCoreID)
79+
numCores = sysfs.GetUniqueCPUPropertyCount(cpuAttributesPath, sysfs.CPUCoreID)
8080
}
8181
if numCores == 0 {
8282
klog.Errorf("Cannot read number of physical cores correctly, number of cores set to %d", numCores)
@@ -90,7 +90,7 @@ func GetSockets(procInfo []byte) int {
9090
if numSocket == 0 {
9191
// read number of sockets from /sys/bus/cpu/devices/cpu*/topology/physical_package_id to deal with processors
9292
// for which 'physical id' is not available in /proc/cpuinfo
93-
numSocket = sysfs.GetUniqueCPUPropertyCount(cpuBusPath, sysfs.CPUPhysicalPackageID)
93+
numSocket = sysfs.GetUniqueCPUPropertyCount(cpuAttributesPath, sysfs.CPUPhysicalPackageID)
9494
}
9595
if numSocket == 0 {
9696
klog.Errorf("Cannot read number of sockets correctly, number of sockets set to %d", numSocket)

‎machine/topology_test.go

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,12 @@ func TestPhysicalCores(t *testing.T) {
4141
}
4242

4343
func TestPhysicalCoresReadingFromCpuBus(t *testing.T) {
44-
origCPUBusPath := cpuBusPath
44+
origCPUAttributesPath := cpuAttributesPath
4545
defer func() {
46-
cpuBusPath = origCPUBusPath
46+
cpuAttributesPath = origCPUAttributesPath
4747
}()
48-
cpuBusPath = "./testdata/sysfs_cpus/" // overwriting package variable to mock sysfs
49-
testfile := "./testdata/cpuinfo_arm" // mock cpuinfo without core id
48+
cpuAttributesPath = "./testdata/sysfs_cpus/" // overwriting package variable to mock sysfs
49+
testfile := "./testdata/cpuinfo_arm" // mock cpuinfo without core id
5050

5151
testcpuinfo, err := os.ReadFile(testfile)
5252
assert.Nil(t, err)
@@ -57,12 +57,12 @@ func TestPhysicalCoresReadingFromCpuBus(t *testing.T) {
5757
}
5858

5959
func TestPhysicalCoresFromWrongSysFs(t *testing.T) {
60-
origCPUBusPath := cpuBusPath
60+
origCPUAttributesPath := cpuAttributesPath
6161
defer func() {
62-
cpuBusPath = origCPUBusPath
62+
cpuAttributesPath = origCPUAttributesPath
6363
}()
64-
cpuBusPath = "./testdata/wrongsysfs" // overwriting package variable to mock sysfs
65-
testfile := "./testdata/cpuinfo_arm" // mock cpuinfo without core id
64+
cpuAttributesPath = "./testdata/wrongsysfs" // overwriting package variable to mock sysfs
65+
testfile := "./testdata/cpuinfo_arm" // mock cpuinfo without core id
6666

6767
testcpuinfo, err := os.ReadFile(testfile)
6868
assert.Nil(t, err)
@@ -84,12 +84,12 @@ func TestSockets(t *testing.T) {
8484
}
8585

8686
func TestSocketsReadingFromCpuBus(t *testing.T) {
87-
origCPUBusPath := cpuBusPath
87+
origCPUAttributesPath := cpuAttributesPath
8888
defer func() {
89-
cpuBusPath = origCPUBusPath
89+
cpuAttributesPath = origCPUAttributesPath
9090
}()
91-
cpuBusPath = "./testdata/wrongsysfs" // overwriting package variable to mock sysfs
92-
testfile := "./testdata/cpuinfo_arm" // mock cpuinfo without physical id
91+
cpuAttributesPath = "./testdata/wrongsysfs" // overwriting package variable to mock sysfs
92+
testfile := "./testdata/cpuinfo_arm" // mock cpuinfo without physical id
9393

9494
testcpuinfo, err := os.ReadFile(testfile)
9595
assert.Nil(t, err)
@@ -103,11 +103,11 @@ func TestSocketsReadingFromWrongSysFs(t *testing.T) {
103103
path, err := filepath.Abs("./testdata/sysfs_cpus/")
104104
assert.NoError(t, err)
105105

106-
origCPUBusPath := cpuBusPath
106+
origCPUAttributesPath := cpuAttributesPath
107107
defer func() {
108-
cpuBusPath = origCPUBusPath
108+
cpuAttributesPath = origCPUAttributesPath
109109
}()
110-
cpuBusPath = path // overwriting package variable to mock sysfs
110+
cpuAttributesPath = path // overwriting package variable to mock sysfs
111111
testfile := "./testdata/cpuinfo_arm" // mock cpuinfo without physical id
112112

113113
testcpuinfo, err := os.ReadFile(testfile)

‎utils/sysfs/sysfs.go

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -123,10 +123,14 @@ type SysFs interface {
123123
IsCPUOnline(dir string) bool
124124
}
125125

126-
type realSysFs struct{}
126+
type realSysFs struct {
127+
cpuPath string
128+
}
127129

128130
func NewRealSysFs() SysFs {
129-
return &realSysFs{}
131+
return &realSysFs{
132+
cpuPath: "/sys/devices/system/cpu",
133+
}
130134
}
131135

132136
func (fs *realSysFs) GetNodesPaths() ([]string, error) {
@@ -402,19 +406,19 @@ func (fs *realSysFs) GetSystemUUID() (string, error) {
402406
}
403407

404408
func (fs *realSysFs) IsCPUOnline(cpuPath string) bool {
405-
onlinePath, err := filepath.Abs(cpuPath + "/../online")
409+
cpuOnlinePath, err := filepath.Abs(fs.cpuPath + "/online")
406410
if err != nil {
407411
klog.V(1).Infof("Unable to get absolute path for %s", cpuPath)
408412
return false
409413
}
410414

411415
// Quick check to determine if file exists: if it does not then kernel CPU hotplug is disabled and all CPUs are online.
412-
_, err = os.Stat(onlinePath)
416+
_, err = os.Stat(cpuOnlinePath)
413417
if err != nil && os.IsNotExist(err) {
414418
return true
415419
}
416420
if err != nil {
417-
klog.V(1).Infof("Unable to stat %s: %s", onlinePath, err)
421+
klog.V(1).Infof("Unable to stat %s: %s", cpuOnlinePath, err)
418422
}
419423

420424
cpuID, err := getCPUID(cpuPath)
@@ -423,7 +427,7 @@ func (fs *realSysFs) IsCPUOnline(cpuPath string) bool {
423427
return false
424428
}
425429

426-
isOnline, err := isCPUOnline(onlinePath, cpuID)
430+
isOnline, err := isCPUOnline(cpuOnlinePath, cpuID)
427431
if err != nil {
428432
klog.V(1).Infof("Unable to get online CPUs list: %s", err)
429433
return false
@@ -475,10 +479,9 @@ func isCPUOnline(path string, cpuID uint16) (bool, error) {
475479
if min > max {
476480
return false, fmt.Errorf("invalid values in %s", path)
477481
}
478-
for i := min; i <= max; i++ {
479-
if uint16(i) == cpuID {
480-
return true, nil
481-
}
482+
// Return true, if the CPU under consideration is in the range of online CPUs.
483+
if cpuID >= uint16(min) && cpuID <= uint16(max) {
484+
return true, nil
482485
}
483486
case 1:
484487
value, err := strconv.ParseUint(s, 10, 16)
@@ -496,21 +499,21 @@ func isCPUOnline(path string, cpuID uint16) (bool, error) {
496499

497500
// Looks for sysfs cpu path containing given CPU property, e.g. core_id or physical_package_id
498501
// and returns number of unique values of given property, exemplary usage: getting number of CPU physical cores
499-
func GetUniqueCPUPropertyCount(cpuBusPath string, propertyName string) int {
500-
absCPUBusPath, err := filepath.Abs(cpuBusPath)
502+
func GetUniqueCPUPropertyCount(cpuAttributesPath string, propertyName string) int {
503+
absCPUAttributesPath, err := filepath.Abs(cpuAttributesPath)
501504
if err != nil {
502-
klog.Errorf("Cannot make %s absolute", cpuBusPath)
505+
klog.Errorf("Cannot make %s absolute", cpuAttributesPath)
503506
return 0
504507
}
505-
pathPattern := absCPUBusPath + "/cpu*[0-9]"
508+
pathPattern := absCPUAttributesPath + "/cpu*[0-9]"
506509
sysCPUPaths, err := filepath.Glob(pathPattern)
507510
if err != nil {
508511
klog.Errorf("Cannot find files matching pattern (pathPattern: %s), number of unique %s set to 0", pathPattern, propertyName)
509512
return 0
510513
}
511-
onlinePath, err := filepath.Abs(cpuBusPath + "/online")
514+
cpuOnlinePath, err := filepath.Abs(cpuAttributesPath + "/online")
512515
if err != nil {
513-
klog.V(1).Infof("Unable to get absolute path for %s", cpuBusPath+"/../online")
516+
klog.V(1).Infof("Unable to get absolute path for %s", cpuAttributesPath+"/../online")
514517
return 0
515518
}
516519

@@ -525,7 +528,7 @@ func GetUniqueCPUPropertyCount(cpuBusPath string, propertyName string) int {
525528
klog.V(1).Infof("Unable to get CPU ID from path %s: %s", sysCPUPath, err)
526529
return 0
527530
}
528-
isOnline, err := isCPUOnline(onlinePath, cpuID)
531+
isOnline, err := isCPUOnline(cpuOnlinePath, cpuID)
529532
if err != nil && !os.IsNotExist(err) {
530533
klog.V(1).Infof("Unable to determine CPU online state: %s", err)
531534
continue

‎utils/sysfs/sysfs_test.go

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,9 @@ func TestGetHugePagesNrWhenFileIsMissing(t *testing.T) {
125125
}
126126

127127
func TestIsCPUOnline(t *testing.T) {
128-
sysFs := NewRealSysFs()
128+
sysFs := &realSysFs{
129+
cpuPath: "./testdata_epyc7402_nohyperthreading",
130+
}
129131
online := sysFs.IsCPUOnline("./testdata_epyc7402_nohyperthreading/cpu14")
130132
assert.True(t, online)
131133

@@ -141,7 +143,9 @@ func TestIsCPUOnlineNoFileAndCPU0MustBeOnline(t *testing.T) {
141143
}()
142144
isX86 = false
143145

144-
sysFs := NewRealSysFs()
146+
sysFs := &realSysFs{
147+
cpuPath: "./testdata/missing_online/node0",
148+
}
145149
online := sysFs.IsCPUOnline("./testdata/missing_online/node0/cpu33")
146150
assert.True(t, online)
147151
}
@@ -167,7 +171,9 @@ func TestCPU0OfflineOnNotx86(t *testing.T) {
167171
}()
168172
isX86 = false
169173

170-
sysFs := NewRealSysFs()
174+
sysFs := &realSysFs{
175+
cpuPath: "./testdata_graviton2",
176+
}
171177
online := sysFs.IsCPUOnline("./testdata_graviton2/cpu0")
172178
assert.False(t, online)
173179
}
@@ -180,7 +186,9 @@ func TestIsCpuOnlineRaspberryPi4(t *testing.T) {
180186
}()
181187
isX86 = false
182188

183-
sysFS := NewRealSysFs()
189+
sysFS := &realSysFs{
190+
cpuPath: "./testdata_rpi4",
191+
}
184192
online := sysFS.IsCPUOnline("./testdata_rpi4/cpu0")
185193
assert.True(t, online)
186194

@@ -202,7 +210,9 @@ func TestIsCpuOnlineGraviton2(t *testing.T) {
202210
}()
203211
isX86 = false
204212

205-
sysFS := NewRealSysFs()
213+
sysFS := &realSysFs{
214+
cpuPath: "./testdata_graviton2",
215+
}
206216
online := sysFS.IsCPUOnline("./testdata_graviton2/cpu0")
207217
assert.False(t, online)
208218

0 commit comments

Comments
 (0)