On Tue, Nov 23, 2021 at 3:34 PM Arnaldo Carvalho de Melo
[off-list ref] wrote:

Em Tue, Nov 23, 2021 at 02:48:20PM -0800, Ian Rogers escreveu:

quoted

sysfs__read_int returns 0 on success, and so the fast read path was
always failing.

Please split this into two patches, the above part should be in one, and
the strtoull in another.

Will do.

Also can't we just do as ./tools/perf/util/cputopo.c and use instead
core_cpus_list?

It is more complex for the list case as the list may have a range. It
shouldn't really matter with the active fix in any case. I think
ideally we'd have an abstraction like cpuset in util-linux:
https://github.com/util-linux/util-linux/blob/master/lib/cpuset.c
This is a bit beyond what I'm trying to fix here.

Thanks,
Ian

On a 5950x:

⬢[acme@toolbox perf]$ cat /sys/devices/system/cpu/cpu31/topology/core_cpus
80008000
⬢[acme@toolbox perf]$ cat /sys/devices/system/cpu/cpu31/topology/core_cpus_list
15,31
⬢[acme@toolbox perf]$ cat /sys/devices/system/cpu/cpu0/topology/core_cpus
00010001
⬢[acme@toolbox perf]$ cat /sys/devices/system/cpu/cpu0/topology/core_cpus_list
0,16
⬢[acme@toolbox perf]$

- Arnaldo

quoted

strtoull can only read a 64-bit bitmap. On an AMD EPYC core_cpus may look
like:
00000000,00000000,00000000,00000001,00000000,00000000,00000000,00000001
and so the sibling wasn't spotted. Fix by writing a simple hweight string
parser.

Fixes: bb629484d924 (perf tools: Simplify checking if SMT is active.)
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/smt.c | 68 ++++++++++++++++++++++++++++++++++++-------
 1 file changed, 58 insertions(+), 10 deletions(-)

diff --git a/tools/perf/util/smt.c b/tools/perf/util/smt.c
index 20bacd5972ad..2636be65305a 100644
--- a/tools/perf/util/smt.c
+++ b/tools/perf/util/smt.c

@@ -5,6 +5,56 @@
 #include "api/fs/fs.h"
 #include "smt.h"

+/**
+ * hweight_str - Returns the number of bits set in str. Stops at first non-hex
+ *          or ',' character.
+ */
+static int hweight_str(char *str)
+{
+     int result = 0;
+
+     while (*str) {
+             switch (*str++) {
+             case '0':
+             case ',':
+                     break;
+             case '1':
+             case '2':
+             case '4':
+             case '8':
+                     result++;
+                     break;
+             case '3':
+             case '5':
+             case '6':
+             case '9':
+             case 'a':
+             case 'A':
+             case 'c':
+             case 'C':
+                     result += 2;
+                     break;
+             case '7':
+             case 'b':
+             case 'B':
+             case 'd':
+             case 'D':
+             case 'e':
+             case 'E':
+                     result += 3;
+                     break;
+             case 'f':
+             case 'F':
+                     result += 4;
+                     break;
+             default:
+                     goto done;
+             }
+     }
+done:
+     return result;
+}
+
 int smt_on(void)
 {
      static bool cached;

@@ -15,9 +65,12 @@ int smt_on(void)
      if (cached)
              return cached_result;

-     if (sysfs__read_int("devices/system/cpu/smt/active", &cached_result) > 0)
-             goto done;
+     if (sysfs__read_int("devices/system/cpu/smt/active", &cached_result) >= 0) {
+             cached = true;
+             return cached_result;
+     }

+     cached_result = 0;
      ncpu = sysconf(_SC_NPROCESSORS_CONF);
      for (cpu = 0; cpu < ncpu; cpu++) {
              unsigned long long siblings;

@@ -35,18 +88,13 @@ int smt_on(void)
                              continue;
              }
              /* Entry is hex, but does not have 0x, so need custom parser */
-             siblings = strtoull(str, NULL, 16);
+             siblings = hweight_str(str);
              free(str);
-             if (hweight64(siblings) > 1) {
+             if (siblings > 1) {
                      cached_result = 1;
-                     cached = true;
                      break;
              }
      }
-     if (!cached) {
-             cached_result = 0;
-done:
-             cached = true;
-     }
+     cached = true;
      return cached_result;
 }
--

2.34.0.rc2.393.gf8c9666880-goog

--

- Arnaldo