lscpu: use CPU types de-duplication

The new CPU type was always allocated when a new relevant value for
the type was found in the cpuinfo file. However, this solution is
fragile because it can result in the parser creating a cputype struct
with incomplete information.

For instance, on ARM systems with multiple CPU types, a new CPU type
would be triggered by a different "CPU part ID". In cases where the
vendor remained the same, a new type would be created later but the
vendor would not be initialized.

The new implementation creates a new CPU type for each CPU (almost)
and then later de-duplicates the array based on vendor, model, etc.

Addresses: https://github.com/util-linux/util-linux/issues/3062
Signed-off-by: Karel Zak <kzak@redhat.com>

diff --git a/include/strutils.h b/include/strutils.h
index b9f3e08ec67279ec930b2ed99f6482c45fb5d5a7..c6172af43c94758598cb6304c92acd597263624c 100644 (file)
--- a/include/strutils.h
+++ b/include/strutils.h
@@ -199,6 +199,36 @@ static inline int strdup_between_offsets(void *stru_dst, void *stru_src, size_t
 #define strdup_between_structs(_dst, _src, _m) \
                strdup_between_offsets((void *)_dst, (void *)_src, offsetof(__typeof__(*(_src)), _m))
 
+static inline int is_nonnull_offset(const void *stru, size_t offset)
+{
+       const char **o;
+
+       if (!stru)
+               return -EINVAL;
+
+       o = (const char **) ((const char *) stru + offset);
+       return *o != NULL;
+}
+
+#define is_nonnull_member(_stru, _m) \
+               is_nonnull_offset((void *) _stru, offsetof(__typeof__(*(_stru)), _m))
+
+static inline int strcmp_offsets(const void *sa, const void *sb, size_t offset)
+{
+       const char **a = (const char **) ((const char *) sa + offset),
+                  **b = (const char **) ((const char *) sb + offset);
+
+       if (!*a && !*b)
+               return 0;
+       if (!*a)
+               return -1;
+       if (!*b)
+               return 1;
+       return strcmp(*a, *b);
+}
+
+#define strcmp_members(_a, _b, _m) \
+               strcmp_offsets((void *) _a, (void *) _b, offsetof(__typeof__(*(_a)), _m))
 
 extern char *xstrmode(mode_t mode, char *str);
 

diff --git a/sys-utils/lscpu-cpu.c b/sys-utils/lscpu-cpu.c
index 9e1e0c4f7ed1460209d01b9fac29ee11abfd7104..0619e7d349cd065b83e3ce62e4c7250f6110f074 100644 (file)
--- a/sys-utils/lscpu-cpu.c
+++ b/sys-utils/lscpu-cpu.c
@@ -76,11 +76,11 @@ int lscpu_cpu_set_type(struct lscpu_cpu *cpu, struct lscpu_cputype *type)
        if (cpu->type == type)
                return 0;
 
+       DBG(CPU, ul_debugobj(cpu, " type %p -> %p", cpu->type, type));
        lscpu_unref_cputype(cpu->type);
        cpu->type = type;
        lscpu_ref_cputype(type);
 
-       DBG(CPU, ul_debugobj(cpu, "cputype set to %s", type ? type->vendor : NULL));
        return 0;
 }
 

diff --git a/sys-utils/lscpu-cputype.c b/sys-utils/lscpu-cputype.c
index bcdf06e8db06dbcb14dfae5ef75d0a5015868fb3..64518fbd8167947236c68062629829b4a7e43120 100644 (file)
--- a/sys-utils/lscpu-cputype.c
+++ b/sys-utils/lscpu-cputype.c
@@ -73,7 +73,7 @@ void lscpu_ref_cputype(struct lscpu_cputype *ct)
 {
        if (ct) {
                ct->refcount++;
-               DBG(TYPE, ul_debugobj(ct, ">>> ref %d", ct->refcount));
+               /*DBG(TYPE, ul_debugobj(ct, ">>> ref %d", ct->refcount));*/
        }
 }
 
@@ -133,15 +133,27 @@ static void fprintf_cputypes(FILE *f, struct lscpu_cxt *cxt)
        for (i = 0; i < cxt->ncputypes; i++) {
                struct lscpu_cputype *ct = cxt->cputypes[i];
 
-               fprintf(f, "\n vendor: %s\n", ct->vendor);
-               fprintf(f, " machinetype: %s\n", ct->machinetype);
-               fprintf(f, " family: %s\n", ct->family);
-               fprintf(f, " model: %s\n", ct->model);
-               fprintf(f, " modelname: %s\n", ct->modelname);
-               fprintf(f, " revision: %s\n", ct->revision);
-               fprintf(f, " stepping: %s\n", ct->stepping);
-               fprintf(f, " mtid: %s\n", ct->mtid);
-               fprintf(f, " addrsz: %s\n", ct->addrsz);
+               fprintf(f, "\ncpu type: %p\n", ct);
+               if (ct->vendor)
+                       fprintf(f, " vendor: %s\n", ct->vendor);
+               if (ct->machinetype)
+                       fprintf(f, " machinetype: %s\n", ct->machinetype);
+               if (ct->family)
+                       fprintf(f, " family: %s\n", ct->family);
+               if (ct->model)
+                       fprintf(f, " model: %s\n", ct->model);
+               if (ct->modelname)
+                       fprintf(f, " modelname: %s\n", ct->modelname);
+               if (ct->revision)
+                       fprintf(f, " revision: %s\n", ct->revision);
+               if (ct->stepping)
+                       fprintf(f, " stepping: %s\n", ct->stepping);
+               if (ct->mtid)
+                       fprintf(f, " mtid: %s\n", ct->mtid);
+               if (ct->addrsz)
+                       fprintf(f, " addrsz: %s\n", ct->addrsz);
+               if (ct->bogomips)
+                       fprintf(f, " bogomips: %s\n", ct->bogomips);
        }
 }
 
@@ -292,21 +304,112 @@ struct cpuinfo_parser {
        unsigned int            curr_type_added : 1;
 };
 
-static int is_different_cputype(struct lscpu_cputype *ct, size_t offset, const char *value)
+/* Be careful when defining which fields should be used to differentiate
+ * between CPU types. It is possible that CPUs differentiate in flags or
+ * BogoMIPS values, but it seems better to ignore it.
+ */
+static int cmp_cputype(const void *x, const void *y)
 {
-       switch (offset) {
-       case offsetof(struct lscpu_cputype, vendor):
-               return ct->vendor && value && strcmp(ct->vendor, value) != 0;
-       case offsetof(struct lscpu_cputype, model):
-               return ct->model && value && strcmp(ct->model, value) != 0;
-       case offsetof(struct lscpu_cputype, modelname):
-               return ct->modelname && value && strcmp(ct->modelname, value) != 0;
-       case offsetof(struct lscpu_cputype, stepping):
-               return ct->stepping && value && strcmp(ct->stepping, value) != 0;
-       }
+       const struct lscpu_cputype *a = *((const struct lscpu_cputype **) x),
+                                  *b = *((const struct lscpu_cputype **) y);
+       int rc;
+
+       if ((rc = strcmp_members(a, b, vendor)))
+               return rc;
+       if ((rc = strcmp_members(a, b, model)))
+               return rc;
+       if ((rc = strcmp_members(a, b, modelname)))
+               return rc;
+       if ((rc = strcmp_members(a, b, stepping)))
+               return rc;
+
        return 0;
 }
 
+static void replace_cpu_type(struct lscpu_cxt *cxt,
+                       struct lscpu_cputype *old, struct lscpu_cputype *new)
+{
+       size_t i;
+
+       for (i = 0; i < cxt->npossibles; i++) {
+               struct lscpu_cpu *cpu = cxt->cpus[i];
+
+               if (cpu && cpu->type == old)
+                       lscpu_cpu_set_type(cpu, new);
+       }
+}
+
+static void deduplicate_cputypes(struct lscpu_cxt *cxt)
+{
+       size_t i, u;
+
+       if (!cxt->ncputypes)
+               return;
+
+       /* sort */
+       DBG(GATHER, ul_debug("de-duplicate %zu CPU types", cxt->ncputypes));
+       qsort(cxt->cputypes, cxt->ncputypes,
+                       sizeof(struct lscpu_cputype *), cmp_cputype);
+
+       /* remove the same entries */
+       for (u = 0, i = 1; i < cxt->ncputypes; i++) {
+               struct lscpu_cputype *ct = cxt->cputypes[i];
+
+               DBG(TYPE, ul_debugobj(ct, "compare with %p", cxt->cputypes[u]));
+
+               if (cmp_cputype(&cxt->cputypes[u], &ct) == 0) {
+                       DBG(TYPE, ul_debugobj(ct, " duplicated"));
+                       replace_cpu_type(cxt, ct, cxt->cputypes[u]);
+                       lscpu_unref_cputype(ct);
+               } else {
+                       DBG(TYPE, ul_debugobj(ct, " uniq"));
+                       u++;
+                       if (u != i)
+                               cxt->cputypes[u] = ct;
+               }
+       }
+
+       cxt->ncputypes = u + 1;
+
+       /* In certain cases (e.g. ppc), the cpuinfo file may contain additional
+        * information at the end. The parser reads this information up to the
+        * last cputype, with previous cputypes being a subset. In such cases, the
+        * last cputype should be used.
+        *
+        * Let's ignore cputypes[0] if it only contains data that is already covered
+        * by cputypes[1].
+        */
+       if (cxt->ncputypes == 2) {
+               static const size_t items[] = {
+                       offsetof(struct lscpu_cputype, model),
+                       offsetof(struct lscpu_cputype, modelname),
+                       offsetof(struct lscpu_cputype, vendor),
+                       offsetof(struct lscpu_cputype, bogomips),
+                       offsetof(struct lscpu_cputype, revision),
+               };
+               struct lscpu_cputype *ct = cxt->cputypes[0],    /* subset ? */
+                                    *mt = cxt->cputypes[1];    /* master ? */
+
+               DBG(TYPE, ul_debugobj(ct, "checking items in %p", mt));
+               for (i = 0; i < ARRAY_SIZE(items); i++) {
+                       if (!is_nonnull_offset(ct, items[i]))
+                               continue;
+                       if (strcmp_offsets(mt, ct, items[i]) != 0)
+                               break;
+               }
+
+               if (i == ARRAY_SIZE(items)) {
+                       replace_cpu_type(cxt, ct, mt);
+                       lscpu_unref_cputype(ct);
+                       cxt->cputypes[0] = cxt->cputypes[1];
+                       cxt->ncputypes = 1;
+               }
+       }
+
+       DBG(GATHER, ul_debug(" %zu uniq CPU types", cxt->ncputypes));
+}
+
+
 /* canonicalize @str -- remove number at the end return the
  * number by @keynum. This is usable for example for "processor 5" or "cache1"
  * cpuinfo lines */
@@ -562,7 +665,8 @@ int lscpu_read_cpuinfo(struct lscpu_cxt *cxt)
                        }
                        break;
                case CPUINFO_LINE_CPUTYPE:
-                       if (pr->curr_type && is_different_cputype(pr->curr_type, pattern->offset, value)) {
+                       if (pr->curr_type && is_nonnull_offset(pr->curr_type, pattern->offset)) {
+                               /* Don't overwrite the current type, create a new */
                                lscpu_unref_cputype(pr->curr_type);
                                pr->curr_type = NULL;
                        }
@@ -581,11 +685,13 @@ int lscpu_read_cpuinfo(struct lscpu_cxt *cxt)
                }
        } while (1);
 
-       DBG(GATHER, fprintf_cputypes(stderr, cxt));
-
        if (pr->curr_cpu && !pr->curr_cpu->type)
                lscpu_cpu_set_type(pr->curr_cpu, pr->curr_type);
 
+       deduplicate_cputypes(cxt);
+
+       DBG(GATHER, fprintf_cputypes(stderr, cxt));
+
        lscpu_unref_cputype(pr->curr_type);
        lscpu_unref_cpu(pr->curr_cpu);
 

diff --git a/sys-utils/lscpu.1.adoc b/sys-utils/lscpu.1.adoc
index 4b55bcfe751b3029a02c77c2deceec4d9912b2dd..26c078bcdb1c343ba70399e5258bf99e2d540d09 100644 (file)
--- a/sys-utils/lscpu.1.adoc
+++ b/sys-utils/lscpu.1.adoc
@@ -94,7 +94,9 @@ Output all available columns. This option must be combined with either *--extend
 
 == BUGS
 
-The basic overview of CPU family, model, etc. is always based on the first CPU only.
+The basic overview of CPU models is based on heuristics, taking into account differences such as
+CPU model names and implementer IDs. In some (unusual) cases, CPUs may differentiate in flags or BogoMIPS,
+but these differences are ignored in the lscpu overview.
 
 Sometimes in Xen Dom0 the kernel reports wrong data.