get_cpu_dev_ids is broken on 3900X

[notzed]

I have a 3900X and I noticed that zenmonitor was always reporting the same speed for core 3 and 4, which seemed to be a bug. I traced the code and it turns out it was reading cpu7 for both of them.

Looking closer I noticed that the cpu*/topology/core_id do not cover the range 0-12 as expected by get_cpu_dev_ids().

This is a patch to fix the problem, and it displays the coreid in the list rather than it's index.

I'm sorry it's just inline but for some insane reason github wont accept patches as attachments.

diff --git a/src/include/os.h b/src/include/os.h
index d6af111..9d35850 100644
--- a/src/include/os.h
+++ b/src/include/os.h
@@ -1,4 +1,4 @@
-gboolean os_init();
-void os_update();
-void os_clear_minmax();
-GSList* os_get_sensors();
+gboolean os_init(void);
+void os_update(void);
+void os_clear_minmax(void);
+GSList* os_get_sensors(void);
diff --git a/src/include/sysfs.h b/src/include/sysfs.h
index a079c7d..59ab701 100644
--- a/src/include/sysfs.h
+++ b/src/include/sysfs.h
@@ -1,3 +1,8 @@
 #define SYSFS_DIR_CPUS "/sys/devices/system/cpu"

-gshort* get_cpu_dev_ids();
+struct cpudev {
+   gshort coreid;
+   gshort cpuid;
+};
+
+struct cpudev * get_cpu_dev_ids(void);
diff --git a/src/ss/msr.c b/src/ss/msr.c
index 2ef75ac..c43f087 100644
--- a/src/ss/msr.c
+++ b/src/ss/msr.c
@@ -75,7 +75,7 @@ gulong get_core_energy(gint core) {
 }

 gboolean msr_init() {
-    gshort *cpu_dev_ids = NULL;
+    struct cpudev *cpu_dev_ids = NULL;
     guint i;

     if (!check_zen())
@@ -88,7 +88,7 @@ gboolean msr_init() {
     cpu_dev_ids = get_cpu_dev_ids();
     msr_files = malloc(cores * sizeof (gint));
     for (i = 0; i < cores; i++) {
-        msr_files[i] = open_msr(cpu_dev_ids[i]);
+        msr_files[i] = open_msr(cpu_dev_ids[i].cpuid);
     }
     g_free(cpu_dev_ids);

diff --git a/src/ss/os.c b/src/ss/os.c
index 9b25a04..637c4ec 100644
--- a/src/ss/os.c
+++ b/src/ss/os.c
@@ -9,6 +9,7 @@

 static gchar **frq_files = NULL;
 static guint cores;
+static struct cpudev *cpu_dev_ids;

 gfloat *core_freq;
 gfloat *core_freq_min;
@@ -22,8 +23,7 @@ static gdouble get_frequency(guint coreid) {
     return atoi(data) / 1000000.0;
 }

-gboolean os_init() {
-    gshort *cpu_dev_ids = NULL;
+gboolean os_init(void) {
     guint i;

     if (!check_zen())
@@ -38,9 +38,8 @@ gboolean os_init() {
     for (i = 0; i < cores; i++) {
         frq_files[i] = g_strdup_printf(
                         "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_cur_freq",
-                        cpu_dev_ids[i]);
+                        cpu_dev_ids[i].cpuid);
     }
-    g_free(cpu_dev_ids);

     core_freq = malloc(cores * sizeof (gfloat));
     core_freq_min = malloc(cores * sizeof (gfloat));
@@ -53,7 +52,7 @@ gboolean os_init() {
     return TRUE;
 }

-void os_update() {
+void os_update(void) {
     guint i;

     for (i = 0; i < cores; i++) {
@@ -65,7 +64,7 @@ void os_update() {
     }
 }

-void os_clear_minmax() {
+void os_clear_minmax(void) {
     guint i;

     for (i = 0; i < cores; i++) {
@@ -74,14 +73,14 @@ void os_clear_minmax() {
     }
 }

-GSList* os_get_sensors() {
+GSList* os_get_sensors(void) {
     GSList *list = NULL;
     SensorInit *data;
     guint i;

     for (i = 0; i < cores; i++) {
         data = sensor_init_new();
-        data->label = g_strdup_printf("Core %d Frequency", i);
+        data->label = g_strdup_printf("Core %d Frequency", cpu_dev_ids[i].coreid);
         data->value = &(core_freq[i]);
         data->min = &(core_freq_min[i]);
         data->max = &(core_freq_max[i]);
diff --git a/src/sysfs.c b/src/sysfs.c
index 1536dfb..37f2443 100644
--- a/src/sysfs.c
+++ b/src/sysfs.c
@@ -5,20 +5,44 @@
 #include "sysfs.h"
 #include "zenmonitor.h"

-gshort* get_cpu_dev_ids(){
-    gshort *cpu_dev_ids = NULL;
+#define CORES_MAX 256
+struct bitset {
+    guint bits[CORES_MAX/32];
+};
+
+static int bitset_set(struct bitset *set, int id) {
+    if (id < CORES_MAX) {
+   int v = (set->bits[id/32] >> (id & 31)) & 1;
+
+   set->bits[id/32] |= 1 << (id & 31);
+   return v;
+    }
+    return 1;
+}
+
+static int cmp_cpudev(const void *ap, const void *bp) {
+    return ((struct cpudev *)ap)->coreid - ((struct cpudev *)bp)->coreid;
+}
+
+struct cpudev* get_cpu_dev_ids(void) {
+    struct cpudev *cpu_dev_ids;
     gshort coreid, cpuid;
     GDir *dir;
     const gchar *entry;
     gchar *filename, *buffer;
-    guint cores, i;
+    guint cores;
+    struct bitset seen = { 0 };
+    int i;

     cores = get_core_count();
-    cpu_dev_ids = malloc(cores * sizeof (gshort));
-    memset(cpu_dev_ids, -1, cores * sizeof (gshort));
+    cpu_dev_ids = malloc(cores * sizeof (*cpu_dev_ids));
+    for (i=0;i<cores;i++)
+   cpu_dev_ids[i] = (struct cpudev) { -1, -1 };

     dir = g_dir_open(SYSFS_DIR_CPUS, 0, NULL);
     if (dir) {
+   int i = 0;
+
         while ((entry = g_dir_read_name(dir))) {
             if (sscanf(entry, "cpu%hd", &cpuid) != 1) {
                 continue;
@@ -28,9 +52,9 @@ gshort* get_cpu_dev_ids(){
             if (g_file_get_contents(filename, &buffer, NULL, NULL)) {
                 coreid = (gshort) atoi(buffer);

-                if ((guint) coreid < cores && cpu_dev_ids[coreid] == -1) {
-                    cpu_dev_ids[coreid] = cpuid;
-                }
+       if (i < cores && !bitset_set(&seen, coreid)) {
+           cpu_dev_ids[i++] = (struct cpudev) { coreid, cpuid };
+       }
             }

             g_free(filename);
@@ -38,12 +62,7 @@ gshort* get_cpu_dev_ids(){
         }
     }

-    for (i = 0; i < cores; i++) {
-        if (cpu_dev_ids[i] < 0) {
-            cpu_dev_ids[i] = i;
-        }
-    }
+    qsort(cpu_dev_ids, cores, sizeof(*cpu_dev_ids), cmp_cpudev);

     return cpu_dev_ids;
 }
-

[ocerman]

@notzed Thanks for your report, your patch has been included in 5408b21e7a1a22b79e81a712e2e19d386f3c4d94