Possible bug in parsing core.ID and core.Index

[cyga]

In changes between v0.10.0 to v0.11.0 (more precisely in git diff 59f01e7caa962f36aef7293abaab6a4f770391ba 34f45f4cfdb628931eb1732fa414a7ae97191354) the following changes were introduced:

v0.10.0:
    ID = "core id" from /proc/cpuinfo
    Index = index in /proc/cpuinfo
v0.11.0: 
    ID = not set (0)
    Index = value from /sys/devices/system/cpu/cpuXX/topology/core_id

Is it a bug or a feature?

1) Documentation wasn't changed for it:

ghw.ProcessorCore.ID is the uint32 identifier that the host gave this core. Note that this does not necessarily equate to a zero-based index of the core within a physical package. For example, the core IDs for an Intel Core i7 are 0, 1, 2, 8, 9, and 10
ghw.ProcessorCore.Index is the zero-based index of the core on the physical processor package

2) If this is designed behaviour can you at least keep ID same as Index (as far as I can see, "core id" has the same value on my linuxes as "/sys/devices/system/cpu/cpuXX/topology/core_id") for some period. So, we can adopt this change during transition period?

[jaypipes]

Hi @cyga! Would you mind providing some details about your host system? Specifically what architecture are you using? To answer your question, no, it was not the intention to change these field values, though the implementation of CPU information gathering was indeed refactored in the v0.11.0 timeframe. /cc @rockrush @glimchb

[glimchb]

from a quick look without any testing yet looks like this might be a fix:

diff --git a/pkg/cpu/cpu_linux.go b/pkg/cpu/cpu_linux.go
index d4b048d..274ca05 100644
--- a/pkg/cpu/cpu_linux.go
+++ b/pkg/cpu/cpu_linux.go
@@ -138,7 +138,7 @@ func processorsGet(ctx *context.Context) []*Processor {
                coreID := util.SafeIntFromFile(ctx, coreIdPath)
                core := CoreByID(proc.Cores, coreID)
                if core == nil {
-                       core = &ProcessorCore{Index: coreID, NumThreads: 1}
+                       core = &ProcessorCore{ID: coreID, Index: len(proc.Cores), NumThreads: 1}
                        proc.Cores = append(proc.Cores, core)
                        proc.NumCores += 1
                } else {
(END)

[cyga]

Hi @cyga! Would you mind providing some details about your host system? Specifically what architecture are you using? To answer your question, no, it was not the intention to change these field values, though the implementation of CPU information gathering was indeed refactored in the v0.11.0 timeframe. /cc @rockrush @glimchb

$ uname -m
x86_64

CPUs layout using snapshot made previously and using v0.10.0:

$ cat main.go
package main

import (
        "fmt"

        "github.com/jaypipes/ghw"
)

func main() {
        info, err := ghw.CPU(ghw.WithSnapshot(ghw.SnapshotOptions{Path: "testdata/linux-amd64-7407565ac48041e48099d838f0818125.tar.gz"}))
        if err != nil {
                panic(err)
        }
        for _, processor := range info.Processors {
                fmt.Printf("processor ID=%d\n", processor.ID)
                for _, core := range processor.Cores {
                        fmt.Printf("  core id=%d, index=%d\n", core.ID, core.Index)
                        for _, logCore := range core.LogicalProcessors {
                                fmt.Printf("    log core ID=%d\n", logCore)
                        }
                }
        }
}

$ go run main.go
processor ID=0
  core id=0, index=0
    log core ID=0
    log core ID=16
  core id=1, index=1
    log core ID=2
    log core ID=18
  core id=2, index=2
    log core ID=4
    log core ID=20
  core id=3, index=3
    log core ID=6
    log core ID=22
  core id=4, index=4
    log core ID=8
    log core ID=24
  core id=5, index=5
    log core ID=10
    log core ID=26
  core id=6, index=6
    log core ID=12
    log core ID=28
  core id=7, index=7
    log core ID=14
    log core ID=30
processor ID=1
  core id=0, index=0
    log core ID=1
    log core ID=17
  core id=1, index=1
    log core ID=3
    log core ID=19
  core id=2, index=2
    log core ID=5
    log core ID=21
  core id=3, index=3
    log core ID=7
    log core ID=23
  core id=4, index=4
    log core ID=9
    log core ID=25
  core id=5, index=5
    log core ID=11
    log core ID=27
  core id=6, index=6
    log core ID=13
    log core ID=29
  core id=7, index=7
    log core ID=15
    log core ID=31

[cyga]

from a quick look without any testing yet looks like this might be a fix:

diff --git a/pkg/cpu/cpu_linux.go b/pkg/cpu/cpu_linux.go
index d4b048d..274ca05 100644
--- a/pkg/cpu/cpu_linux.go
+++ b/pkg/cpu/cpu_linux.go
@@ -138,7 +138,7 @@ func processorsGet(ctx *context.Context) []*Processor {
                coreID := util.SafeIntFromFile(ctx, coreIdPath)
                core := CoreByID(proc.Cores, coreID)
                if core == nil {
-                       core = &ProcessorCore{Index: coreID, NumThreads: 1}
+                       core = &ProcessorCore{ID: coreID, Index: len(proc.Cores), NumThreads: 1}
                        proc.Cores = append(proc.Cores, core)
                        proc.NumCores += 1
                } else {
(END)

I also thought of smth similar, but it looks like cores are groupped differently in that case. For example, the following is the output for the program I posted above with the patch you showed:

processor ID=0
  core id=0, index=0
    log core ID=0
    log core ID=16
  core id=5, index=1
    log core ID=10
    log core ID=18
    log core ID=2
  core id=6, index=2
    log core ID=12
    log core ID=20
    log core ID=4
  core id=7, index=3
    log core ID=14
    log core ID=22
    log core ID=6
  core id=4, index=4
    log core ID=24
    log core ID=8
  core id=5, index=5
    log core ID=26
  core id=6, index=6
    log core ID=28
  core id=7, index=7
    log core ID=30
processor ID=1
  core id=0, index=0
    log core ID=1
    log core ID=17
  core id=5, index=1
    log core ID=11
    log core ID=19
    log core ID=3
  core id=6, index=2
    log core ID=13
    log core ID=21
    log core ID=5
  core id=7, index=3
    log core ID=15
    log core ID=23
    log core ID=7
  core id=4, index=4
    log core ID=25
    log core ID=9
  core id=5, index=5
    log core ID=27
  core id=6, index=6
    log core ID=29
  core id=7, index=7
    log core ID=31

[cyga]

from a quick look without any testing yet looks like this might be a fix:

diff --git a/pkg/cpu/cpu_linux.go b/pkg/cpu/cpu_linux.go
index d4b048d..274ca05 100644
--- a/pkg/cpu/cpu_linux.go
+++ b/pkg/cpu/cpu_linux.go
@@ -138,7 +138,7 @@ func processorsGet(ctx *context.Context) []*Processor {
                coreID := util.SafeIntFromFile(ctx, coreIdPath)
                core := CoreByID(proc.Cores, coreID)
                if core == nil {
-                       core = &ProcessorCore{Index: coreID, NumThreads: 1}
+                       core = &ProcessorCore{ID: coreID, Index: len(proc.Cores), NumThreads: 1}
                        proc.Cores = append(proc.Cores, core)
                        proc.NumCores += 1
                } else {
(END)

It's more like:

diff --git a/pkg/cpu/cpu_linux.go b/pkg/cpu/cpu_linux.go
index d4b048d..3cec259 100644
--- a/pkg/cpu/cpu_linux.go
+++ b/pkg/cpu/cpu_linux.go
@@ -48,7 +48,7 @@ func ProcByID(procs []*Processor, id int) *Processor {

 func CoreByID(cores []*ProcessorCore, id int) *ProcessorCore {
        for cid := range cores {
-               if cores[cid].Index == id {
+               if cores[cid].ID == id {
                        return cores[cid]
                }
        }
@@ -138,7 +138,11 @@ func processorsGet(ctx *context.Context) []*Processor {
                coreID := util.SafeIntFromFile(ctx, coreIdPath)
                core := CoreByID(proc.Cores, coreID)
                if core == nil {
-                       core = &ProcessorCore{Index: coreID, NumThreads: 1}
+                       core = &ProcessorCore{
+                               ID: coreID,
+                               Index: len(proc.Cores),
+                               NumThreads: 1,
+                       }
                        proc.Cores = append(proc.Cores, core)
                        proc.NumCores += 1
                } else {

but the output is not exactly the same I need to better check it later.

[jaypipes]

I can indeed confirm that the indexing of logical processors changed from v0.10.0 to the latest commit on main (36ff37eb3bdfcb8de352217a6406f12b31746e56):

jaypipes@thelio:~/src/github.com/jaypipes/ghw$ go run cmd/ghwc/main.go cpu
cpu (1 physical package, 8 cores, 16 hardware threads)
 physical package #0 (8 cores, 16 hardware threads)
  processor core #0 (2 threads), logical processors [0 8]
  processor core #1 (2 threads), logical processors [1 9]
  processor core #2 (2 threads), logical processors [10 2]
  processor core #3 (2 threads), logical processors [11 3]
  processor core #4 (2 threads), logical processors [12 4]
  processor core #5 (2 threads), logical processors [13 5]
  processor core #6 (2 threads), logical processors [14 6]
  processor core #7 (2 threads), logical processors [15 7]
  capabilities: [msr pae mce cx8 apic sep
                 mtrr pge mca cmov pat pse36
                 clflush mmx fxsr sse sse2 ht
                 syscall nx mmxext fxsr_opt pdpe1gb rdtscp
                 lm constant_tsc rep_good nopl nonstop_tsc cpuid
                 extd_apicid aperfmperf rapl pni pclmulqdq monitor
                 ssse3 fma cx16 sse4_1 sse4_2 movbe
                 popcnt aes xsave avx f16c rdrand
                 lahf_lm cmp_legacy svm extapic cr8_legacy abm
                 sse4a misalignsse 3dnowprefetch osvw skinit wdt
                 tce topoext perfctr_core perfctr_nb bpext perfctr_llc
                 mwaitx cpb hw_pstate ssbd ibpb vmmcall
                 fsgsbase bmi1 avx2 smep bmi2 rdseed
                 adx smap clflushopt sha_ni xsaveopt xsavec
                 xgetbv1 xsaves clzero irperf xsaveerptr arat
                 npt lbrv svm_lock nrip_save tsc_scale vmcb_clean
                 flushbyasid decodeassists pausefilter pfthreshold avic v_vmsave_vmload
                 vgif overflow_recov succor smca sev sev_es]
jaypipes@thelio:~/src/github.com/jaypipes/ghw$ git checkout v0.10.0
Note: switching to 'v0.10.0'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at ae21148 Merge pull request #330 from jaypipes/issue-267
jaypipes@thelio:~/src/github.com/jaypipes/ghw$ go run cmd/ghwc/main.go cpu
cpu (1 physical package, 8 cores, 16 hardware threads)
 physical package #0 (8 cores, 16 hardware threads)
  processor core #0 (2 threads), logical processors [0 8]
  processor core #1 (2 threads), logical processors [1 9]
  processor core #2 (2 threads), logical processors [2 10]
  processor core #3 (2 threads), logical processors [3 11]
  processor core #4 (2 threads), logical processors [4 12]
  processor core #5 (2 threads), logical processors [5 13]
  processor core #6 (2 threads), logical processors [6 14]
  processor core #7 (2 threads), logical processors [7 15]
  capabilities: [msr pae mce cx8 apic sep
                 mtrr pge mca cmov pat pse36
                 clflush mmx fxsr sse sse2 ht
                 syscall nx mmxext fxsr_opt pdpe1gb rdtscp
                 lm constant_tsc rep_good nopl nonstop_tsc cpuid
                 extd_apicid aperfmperf rapl pni pclmulqdq monitor
                 ssse3 fma cx16 sse4_1 sse4_2 movbe
                 popcnt aes xsave avx f16c rdrand
                 lahf_lm cmp_legacy svm extapic cr8_legacy abm
                 sse4a misalignsse 3dnowprefetch osvw skinit wdt
                 tce topoext perfctr_core perfctr_nb bpext perfctr_llc
                 mwaitx cpb hw_pstate ssbd ibpb vmmcall
                 fsgsbase bmi1 avx2 smep bmi2 rdseed
                 adx smap clflushopt sha_ni xsaveopt xsavec
                 xgetbv1 xsaves clzero irperf xsaveerptr arat
                 npt lbrv svm_lock nrip_save tsc_scale vmcb_clean
                 flushbyasid decodeassists pausefilter pfthreshold avic v_vmsave_vmload
                 vgif overflow_recov succor smca sev sev_es]

Working on a fix now...

[jaypipes]

Never mind... the logical processors are the same. It's just that the slice of logical processor IDs is in a different order...

[jaypipes]

@cyga @glimchb if you wouldn't mind giving the latest ghw release (v0.12.0) a try, that would be great. It includes a fix for this bug.

[glimchb]

on it @jaypipes renovate already picked it up https://github.com/opiproject/opi-smbios-bridge/pull/131

[cyga]

@cyga @glimchb if you wouldn't mind giving the latest ghw release (v0.12.0) a try, that would be great. It includes a fix for this bug.

Yes, it works the same way on my examples as v0.10.0 did. Thanks.