Messages in this thread | | | Date | Tue, 23 Nov 2021 20:34:49 -0300 | From | Arnaldo Carvalho de Melo <> | Subject | Re: [PATCH 2/3] perf tools: Fix SMT not detected with large core count |
| |
Em Tue, Nov 23, 2021 at 02:48:20PM -0800, Ian Rogers escreveu: > 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.
Also can't we just do as ./tools/perf/util/cputopo.c and use instead core_cpus_list?
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
> 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
| |