Skip to content

Commit da26e37

Browse files
Paweł Szulikiwankgb
authored andcommitted
Fix incorrect CPU topology on single NUMA and multi socket platform.
Signed-off-by: Paweł Szulik <[email protected]>
1 parent 37c56aa commit da26e37

File tree

2 files changed

+193
-84
lines changed

2 files changed

+193
-84
lines changed

utils/sysinfo/sysinfo.go

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,7 @@ func getCoresInfo(sysFs sysfs.SysFs, cpuDirs []string) ([]info.Core, error) {
383383
for _, cpuDir := range cpuDirs {
384384
cpuID, err := getMatchedInt(cpuDirRegExp, cpuDir)
385385
if err != nil {
386-
return nil, fmt.Errorf("Unexpected format of CPU directory, cpuDirRegExp %s, cpuDir: %s", cpuDirRegExp, cpuDir)
386+
return nil, fmt.Errorf("unexpected format of CPU directory, cpuDirRegExp %s, cpuDir: %s", cpuDirRegExp, cpuDir)
387387
}
388388
if !sysFs.IsCPUOnline(cpuDir) {
389389
continue
@@ -401,9 +401,22 @@ func getCoresInfo(sysFs sysfs.SysFs, cpuDirs []string) ([]info.Core, error) {
401401
return nil, err
402402
}
403403

404+
rawPhysicalPackageID, err := sysFs.GetCPUPhysicalPackageID(cpuDir)
405+
if os.IsNotExist(err) {
406+
klog.Warningf("Cannot read physical package id for %s, physical_package_id file does not exist, err: %s", cpuDir, err)
407+
continue
408+
} else if err != nil {
409+
return nil, err
410+
}
411+
412+
physicalPackageID, err := strconv.Atoi(rawPhysicalPackageID)
413+
if err != nil {
414+
return nil, err
415+
}
416+
404417
coreIDx := -1
405418
for id, core := range cores {
406-
if core.Id == physicalID {
419+
if core.Id == physicalID && core.SocketID == physicalPackageID {
407420
coreIDx = id
408421
}
409422
}
@@ -414,25 +427,14 @@ func getCoresInfo(sysFs sysfs.SysFs, cpuDirs []string) ([]info.Core, error) {
414427
desiredCore := &cores[coreIDx]
415428

416429
desiredCore.Id = physicalID
430+
desiredCore.SocketID = physicalPackageID
431+
417432
if len(desiredCore.Threads) == 0 {
418433
desiredCore.Threads = []int{cpuID}
419434
} else {
420435
desiredCore.Threads = append(desiredCore.Threads, cpuID)
421436
}
422437

423-
rawPhysicalPackageID, err := sysFs.GetCPUPhysicalPackageID(cpuDir)
424-
if os.IsNotExist(err) {
425-
klog.Warningf("Cannot read physical package id for %s, physical_package_id file does not exist, err: %s", cpuDir, err)
426-
continue
427-
} else if err != nil {
428-
return nil, err
429-
}
430-
431-
physicalPackageID, err := strconv.Atoi(rawPhysicalPackageID)
432-
if err != nil {
433-
return nil, err
434-
}
435-
desiredCore.SocketID = physicalPackageID
436438
}
437439
return cores, nil
438440
}

utils/sysinfo/sysinfo_test.go

Lines changed: 176 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -104,71 +104,62 @@ func TestGetHugePagesInfoWithWrongNrHugePageValue(t *testing.T) {
104104
}
105105

106106
func TestGetNodesInfo(t *testing.T) {
107-
fakeSys := &fakesysfs.FakeSysFs{}
108-
c := sysfs.CacheInfo{
109-
Size: 32 * 1024,
110-
Type: "unified",
111-
Level: 3,
112-
Cpus: 2,
113-
}
114-
fakeSys.SetCacheInfo(c)
115-
116-
nodesPaths := []string{
117-
"/fakeSysfs/devices/system/node/node0",
118-
"/fakeSysfs/devices/system/node/node1",
119-
}
120-
fakeSys.SetNodesPaths(nodesPaths, nil)
121-
122-
cpusPaths := map[string][]string{
123-
"/fakeSysfs/devices/system/node/node0": {
124-
"/fakeSysfs/devices/system/node/node0/cpu0",
125-
"/fakeSysfs/devices/system/node/node0/cpu1",
126-
},
127-
"/fakeSysfs/devices/system/node/node1": {
128-
"/fakeSysfs/devices/system/node/node0/cpu2",
129-
"/fakeSysfs/devices/system/node/node0/cpu3",
130-
},
131-
}
132-
fakeSys.SetCPUsPaths(cpusPaths, nil)
133-
134-
coreThread := map[string]string{
135-
"/fakeSysfs/devices/system/node/node0/cpu0": "0",
136-
"/fakeSysfs/devices/system/node/node0/cpu1": "0",
137-
"/fakeSysfs/devices/system/node/node0/cpu2": "1",
138-
"/fakeSysfs/devices/system/node/node0/cpu3": "1",
139-
}
140-
fakeSys.SetCoreThreads(coreThread, nil)
141-
142-
memTotal := "MemTotal: 32817192 kB"
143-
fakeSys.SetMemory(memTotal, nil)
144-
145-
hugePages := []os.FileInfo{
146-
&fakesysfs.FileInfo{EntryName: "hugepages-2048kB"},
147-
}
148-
fakeSys.SetHugePages(hugePages, nil)
149-
150-
hugePageNr := map[string]string{
151-
"/fakeSysfs/devices/system/node/node0/hugepages/hugepages-2048kB/nr_hugepages": "1",
152-
"/fakeSysfs/devices/system/node/node1/hugepages/hugepages-2048kB/nr_hugepages": "1",
153-
}
154-
fakeSys.SetHugePagesNr(hugePageNr, nil)
155-
156-
physicalPackageIDs := map[string]string{
157-
"/fakeSysfs/devices/system/node/node0/cpu0": "0",
158-
"/fakeSysfs/devices/system/node/node0/cpu1": "0",
159-
"/fakeSysfs/devices/system/node/node0/cpu2": "1",
160-
"/fakeSysfs/devices/system/node/node0/cpu3": "1",
161-
}
162-
fakeSys.SetPhysicalPackageIDs(physicalPackageIDs, nil)
163-
164-
nodes, cores, err := GetNodesInfo(fakeSys)
165-
assert.Nil(t, err)
166-
assert.Equal(t, 2, len(nodes))
167-
assert.Equal(t, 4, cores)
168-
169-
nodesJSON, err := json.Marshal(nodes)
170-
assert.Nil(t, err)
171-
expectedNodes := `
107+
testCases := []struct {
108+
cache sysfs.CacheInfo
109+
nodesPaths []string
110+
cpusPaths map[string][]string
111+
coresThreads map[string]string
112+
memTotal string
113+
hugePages []os.FileInfo
114+
hugePageNr map[string]string
115+
physicalPackageIDs map[string]string
116+
nodes int
117+
cores int
118+
expectedNodes string
119+
}{
120+
{
121+
sysfs.CacheInfo{
122+
Size: 32 * 1024,
123+
Type: "unified",
124+
Level: 3,
125+
Cpus: 2,
126+
},
127+
[]string{
128+
"/fakeSysfs/devices/system/node/node0",
129+
"/fakeSysfs/devices/system/node/node1"},
130+
map[string][]string{
131+
"/fakeSysfs/devices/system/node/node0": {
132+
"/fakeSysfs/devices/system/node/node0/cpu0",
133+
"/fakeSysfs/devices/system/node/node0/cpu1",
134+
},
135+
"/fakeSysfs/devices/system/node/node1": {
136+
"/fakeSysfs/devices/system/node/node0/cpu2",
137+
"/fakeSysfs/devices/system/node/node0/cpu3",
138+
},
139+
},
140+
map[string]string{
141+
"/fakeSysfs/devices/system/node/node0/cpu0": "0",
142+
"/fakeSysfs/devices/system/node/node0/cpu1": "0",
143+
"/fakeSysfs/devices/system/node/node0/cpu2": "1",
144+
"/fakeSysfs/devices/system/node/node0/cpu3": "1",
145+
},
146+
"MemTotal: 32817192 kB",
147+
[]os.FileInfo{
148+
&fakesysfs.FileInfo{EntryName: "hugepages-2048kB"},
149+
},
150+
map[string]string{
151+
"/fakeSysfs/devices/system/node/node0/hugepages/hugepages-2048kB/nr_hugepages": "1",
152+
"/fakeSysfs/devices/system/node/node1/hugepages/hugepages-2048kB/nr_hugepages": "1",
153+
},
154+
map[string]string{
155+
"/fakeSysfs/devices/system/node/node0/cpu0": "0",
156+
"/fakeSysfs/devices/system/node/node0/cpu1": "0",
157+
"/fakeSysfs/devices/system/node/node0/cpu2": "1",
158+
"/fakeSysfs/devices/system/node/node0/cpu3": "1",
159+
},
160+
2,
161+
4,
162+
`
172163
[
173164
{
174165
"node_id": 0,
@@ -227,8 +218,124 @@ func TestGetNodesInfo(t *testing.T) {
227218
]
228219
}
229220
]
230-
`
231-
assert.JSONEq(t, expectedNodes, string(nodesJSON))
221+
`,
222+
},
223+
{
224+
sysfs.CacheInfo{
225+
Size: 32 * 1024,
226+
Type: "unified",
227+
Level: 3,
228+
Cpus: 6,
229+
},
230+
[]string{
231+
"/fakeSysfs/devices/system/node/node0"},
232+
map[string][]string{
233+
"/fakeSysfs/devices/system/node/node0": {
234+
"/fakeSysfs/devices/system/node/node0/cpu0",
235+
"/fakeSysfs/devices/system/node/node0/cpu1",
236+
"/fakeSysfs/devices/system/node/node0/cpu2",
237+
"/fakeSysfs/devices/system/node/node0/cpu3",
238+
"/fakeSysfs/devices/system/node/node0/cpu4",
239+
"/fakeSysfs/devices/system/node/node0/cpu5",
240+
},
241+
},
242+
map[string]string{
243+
"/fakeSysfs/devices/system/node/node0/cpu0": "0",
244+
"/fakeSysfs/devices/system/node/node0/cpu1": "0",
245+
"/fakeSysfs/devices/system/node/node0/cpu2": "1",
246+
"/fakeSysfs/devices/system/node/node0/cpu3": "1",
247+
"/fakeSysfs/devices/system/node/node0/cpu4": "2",
248+
"/fakeSysfs/devices/system/node/node0/cpu5": "2",
249+
},
250+
"MemTotal: 32817192 kB",
251+
[]os.FileInfo{
252+
&fakesysfs.FileInfo{EntryName: "hugepages-2048kB"},
253+
},
254+
map[string]string{
255+
"/fakeSysfs/devices/system/node/node0/hugepages/hugepages-2048kB/nr_hugepages": "1",
256+
},
257+
map[string]string{
258+
"/fakeSysfs/devices/system/node/node0/cpu0": "0",
259+
"/fakeSysfs/devices/system/node/node0/cpu1": "0",
260+
"/fakeSysfs/devices/system/node/node0/cpu2": "1",
261+
"/fakeSysfs/devices/system/node/node0/cpu3": "1",
262+
"/fakeSysfs/devices/system/node/node0/cpu4": "2",
263+
"/fakeSysfs/devices/system/node/node0/cpu5": "2",
264+
},
265+
1,
266+
6,
267+
`
268+
[
269+
{
270+
"node_id": 0,
271+
"memory": 33604804608,
272+
"hugepages": [
273+
{
274+
"page_size": 2048,
275+
"num_pages": 1
276+
}
277+
],
278+
"cores": [
279+
{
280+
"core_id": 0,
281+
"thread_ids": [
282+
0,
283+
1
284+
],
285+
"caches": null,
286+
"socket_id": 0
287+
},
288+
{
289+
"core_id": 1,
290+
"thread_ids": [
291+
2,
292+
3
293+
],
294+
"caches": null,
295+
"socket_id": 1
296+
},
297+
{
298+
"core_id": 2,
299+
"thread_ids": [
300+
4,
301+
5
302+
],
303+
"caches": null,
304+
"socket_id": 2
305+
}
306+
],
307+
"caches": [
308+
{
309+
"size": 32768,
310+
"type": "unified",
311+
"level": 3
312+
}
313+
]
314+
}
315+
]
316+
`,
317+
},
318+
}
319+
320+
for _, test := range testCases {
321+
fakeSys := &fakesysfs.FakeSysFs{}
322+
fakeSys.SetCacheInfo(test.cache)
323+
fakeSys.SetNodesPaths(test.nodesPaths, nil)
324+
fakeSys.SetCPUsPaths(test.cpusPaths, nil)
325+
fakeSys.SetCoreThreads(test.coresThreads, nil)
326+
fakeSys.SetMemory(test.memTotal, nil)
327+
fakeSys.SetHugePages(test.hugePages, nil)
328+
fakeSys.SetHugePagesNr(test.hugePageNr, nil)
329+
fakeSys.SetPhysicalPackageIDs(test.physicalPackageIDs, nil)
330+
nodes, cores, err := GetNodesInfo(fakeSys)
331+
assert.Nil(t, err)
332+
assert.Equal(t, test.nodes, len(nodes))
333+
assert.Equal(t, test.cores, cores)
334+
335+
nodesJSON, err := json.Marshal(nodes)
336+
assert.Nil(t, err)
337+
assert.JSONEq(t, test.expectedNodes, string(nodesJSON))
338+
}
232339
}
233340

234341
func TestGetNodesInfoWithOfflineCPUs(t *testing.T) {
@@ -996,7 +1103,7 @@ func TestGetNodesWhenPhysicalPackageIDMissingForOneCPU(t *testing.T) {
9961103
assert.Nil(t, err)
9971104

9981105
assert.Equal(t, 1, len(nodes))
999-
assert.Equal(t, 2, cores)
1106+
assert.Equal(t, 1, cores)
10001107

10011108
sort.Slice(nodes, func(i, j int) bool {
10021109
return nodes[i].Id < nodes[j].Id
@@ -1019,7 +1126,7 @@ func TestGetNodesWhenPhysicalPackageIDMissingForOneCPU(t *testing.T) {
10191126
{
10201127
"core_id":0,
10211128
"thread_ids":[
1022-
0, 1
1129+
0
10231130
],
10241131
"caches":null,
10251132
"socket_id":0

0 commit comments

Comments
 (0)