Messages in this thread | | | From | Arnaldo Carvalho de Melo <> | Date | Thu, 6 Jun 2019 16:12:10 -0300 | Subject | Re: [PATCH V3 2/5] perf header: Add die information in CPU topology |
| |
Em Tue, Jun 04, 2019 at 03:50:41PM -0700, kan.liang@linux.intel.com escreveu: > From: Kan Liang <kan.liang@linux.intel.com> > > With the new CPUID.1F, a new level type of CPU topology, 'die', is > introduced. The 'die' information in CPU topology should be added in > perf header. > > To be compatible with old perf.data, the patch checks the section size > before reading the die information. The new info is added at the end of > the cpu_topology section, the old perf tool ignores the extra data. > It never reads data crossing the section boundary. > > The new perf tool with the patch can be used on legacy kernel. Add a > new function has_die_topology() to check if die topology information is > supported by kernel. The function only check X86 and CPU 0. Assuming > other CPUs have same topology.
You're changing the header, how would a new tool handle an old perf.data where this 'die_id' is not present? What about an old tool dealing with a perf.data with this die_id?
I couldn't see any provision for that, am I missing something?
/me goes to read tools/perf/util/cputopo.c ...
Yeah, its just the description on the perf.data doc file that confused me, I'll clarify that after finishing reviewing/applying this patchkit.
- Arnaldo
> Use similar method for core and socket to support die id and sibling > dies string. > > Signed-off-by: Kan Liang <kan.liang@linux.intel.com> > --- > > No changes since V2. > > tools/perf/Documentation/perf.data-file-format.txt | 9 ++- > tools/perf/util/cputopo.c | 76 +++++++++++++++-- > tools/perf/util/cputopo.h | 2 + > tools/perf/util/env.c | 1 + > tools/perf/util/env.h | 3 + > tools/perf/util/header.c | 94 ++++++++++++++++++++-- > 6 files changed, 172 insertions(+), 13 deletions(-) > > diff --git a/tools/perf/Documentation/perf.data-file-format.txt b/tools/perf/Documentation/perf.data-file-format.txt > index 6375e6f..0165e92 100644 > --- a/tools/perf/Documentation/perf.data-file-format.txt > +++ b/tools/perf/Documentation/perf.data-file-format.txt > @@ -153,7 +153,7 @@ struct { > > String lists defining the core and CPU threads topology. > The string lists are followed by a variable length array > -which contains core_id and socket_id of each cpu. > +which contains core_id, die_id (for x86) and socket_id of each cpu. > The number of entries can be determined by the size of the > section minus the sizes of both string lists. > > @@ -162,14 +162,19 @@ struct { > struct perf_header_string_list threads; /* Variable length */ > struct { > uint32_t core_id; > + uint32_t die_id; > uint32_t socket_id; > } cpus[nr]; /* Variable length records */ > };
- Arnaldo
> Example: > - sibling cores : 0-3 > + sibling cores : 0-8 > + sibling dies : 0-3 > + sibling dies : 4-7 > sibling threads : 0-1 > sibling threads : 2-3 > + sibling threads : 4-5 > + sibling threads : 6-7 > > HEADER_NUMA_TOPOLOGY = 14, > > diff --git a/tools/perf/util/cputopo.c b/tools/perf/util/cputopo.c > index ece0710..85fa87f 100644 > --- a/tools/perf/util/cputopo.c > +++ b/tools/perf/util/cputopo.c > @@ -1,5 +1,6 @@ > // SPDX-License-Identifier: GPL-2.0 > #include <sys/param.h> > +#include <sys/utsname.h> > #include <inttypes.h> > #include <api/fs/fs.h> > > @@ -8,9 +9,10 @@ > #include "util.h" > #include "env.h" > > - > #define CORE_SIB_FMT \ > "%s/devices/system/cpu/cpu%d/topology/core_siblings_list" > +#define DIE_SIB_FMT \ > + "%s/devices/system/cpu/cpu%d/topology/die_cpus_list" > #define THRD_SIB_FMT \ > "%s/devices/system/cpu/cpu%d/topology/thread_siblings_list" > #define NODE_ONLINE_FMT \ > @@ -34,12 +36,12 @@ static int build_cpu_topology(struct cpu_topology *tp, int cpu) > sysfs__mountpoint(), cpu); > fp = fopen(filename, "r"); > if (!fp) > - goto try_threads; > + goto try_dies; > > sret = getline(&buf, &len, fp); > fclose(fp); > if (sret <= 0) > - goto try_threads; > + goto try_dies; > > p = strchr(buf, '\n'); > if (p) > @@ -57,6 +59,37 @@ static int build_cpu_topology(struct cpu_topology *tp, int cpu) > } > ret = 0; > > +try_dies: > + if (!tp->die_siblings) > + goto try_threads; > + > + scnprintf(filename, MAXPATHLEN, DIE_SIB_FMT, > + sysfs__mountpoint(), cpu); > + fp = fopen(filename, "r"); > + if (!fp) > + goto try_threads; > + > + sret = getline(&buf, &len, fp); > + fclose(fp); > + if (sret <= 0) > + goto try_threads; > + > + p = strchr(buf, '\n'); > + if (p) > + *p = '\0'; > + > + for (i = 0; i < tp->die_sib; i++) { > + if (!strcmp(buf, tp->die_siblings[i])) > + break; > + } > + if (i == tp->die_sib) { > + tp->die_siblings[i] = buf; > + tp->die_sib++; > + buf = NULL; > + len = 0; > + } > + ret = 0; > + > try_threads: > scnprintf(filename, MAXPATHLEN, THRD_SIB_FMT, > sysfs__mountpoint(), cpu); > @@ -98,21 +131,46 @@ void cpu_topology__delete(struct cpu_topology *tp) > for (i = 0 ; i < tp->core_sib; i++) > zfree(&tp->core_siblings[i]); > > + if (tp->die_sib) { > + for (i = 0 ; i < tp->die_sib; i++) > + zfree(&tp->die_siblings[i]); > + } > + > for (i = 0 ; i < tp->thread_sib; i++) > zfree(&tp->thread_siblings[i]); > > free(tp); > } > > +static bool has_die_topology(void) > +{ > + char filename[MAXPATHLEN]; > + struct utsname uts; > + > + if (uname(&uts) < 0) > + return false; > + > + if (strncmp(uts.machine, "x86_64", 6)) > + return false; > + > + scnprintf(filename, MAXPATHLEN, DIE_SIB_FMT, > + sysfs__mountpoint(), 0); > + if (access(filename, F_OK) == -1) > + return false; > + > + return true; > +} > + > struct cpu_topology *cpu_topology__new(void) > { > struct cpu_topology *tp = NULL; > void *addr; > - u32 nr, i; > + u32 nr, i, nr_addr; > size_t sz; > long ncpus; > int ret = -1; > struct cpu_map *map; > + bool has_die = has_die_topology(); > > ncpus = cpu__max_present_cpu(); > > @@ -126,7 +184,11 @@ struct cpu_topology *cpu_topology__new(void) > nr = (u32)(ncpus & UINT_MAX); > > sz = nr * sizeof(char *); > - addr = calloc(1, sizeof(*tp) + 2 * sz); > + if (has_die) > + nr_addr = 3; > + else > + nr_addr = 2; > + addr = calloc(1, sizeof(*tp) + nr_addr * sz); > if (!addr) > goto out_free; > > @@ -134,6 +196,10 @@ struct cpu_topology *cpu_topology__new(void) > addr += sizeof(*tp); > tp->core_siblings = addr; > addr += sz; > + if (has_die) { > + tp->die_siblings = addr; > + addr += sz; > + } > tp->thread_siblings = addr; > > for (i = 0; i < nr; i++) { > diff --git a/tools/perf/util/cputopo.h b/tools/perf/util/cputopo.h > index 47a97e7..bae2f1d 100644 > --- a/tools/perf/util/cputopo.h > +++ b/tools/perf/util/cputopo.h > @@ -7,8 +7,10 @@ > > struct cpu_topology { > u32 core_sib; > + u32 die_sib; > u32 thread_sib; > char **core_siblings; > + char **die_siblings; > char **thread_siblings; > }; > > diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c > index 6a3eaf7..1cc7a18 100644 > --- a/tools/perf/util/env.c > +++ b/tools/perf/util/env.c > @@ -246,6 +246,7 @@ int perf_env__read_cpu_topology_map(struct perf_env *env) > for (cpu = 0; cpu < nr_cpus; ++cpu) { > env->cpu[cpu].core_id = cpu_map__get_core_id(cpu); > env->cpu[cpu].socket_id = cpu_map__get_socket_id(cpu); > + env->cpu[cpu].die_id = cpu_map__get_die_id(cpu); > } > > env->nr_cpus_avail = nr_cpus; > diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h > index 271a90b..d5d9865 100644 > --- a/tools/perf/util/env.h > +++ b/tools/perf/util/env.h > @@ -9,6 +9,7 @@ > > struct cpu_topology_map { > int socket_id; > + int die_id; > int core_id; > }; > > @@ -49,6 +50,7 @@ struct perf_env { > > int nr_cmdline; > int nr_sibling_cores; > + int nr_sibling_dies; > int nr_sibling_threads; > int nr_numa_nodes; > int nr_memory_nodes; > @@ -57,6 +59,7 @@ struct perf_env { > char *cmdline; > const char **cmdline_argv; > char *sibling_cores; > + char *sibling_dies; > char *sibling_threads; > char *pmu_mappings; > struct cpu_topology_map *cpu; > diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c > index 847ae51..6497625 100644 > --- a/tools/perf/util/header.c > +++ b/tools/perf/util/header.c > @@ -599,6 +599,27 @@ static int write_cpu_topology(struct feat_fd *ff, > if (ret < 0) > return ret; > } > + > + if (!tp->die_sib) > + goto done; > + > + ret = do_write(ff, &tp->die_sib, sizeof(tp->die_sib)); > + if (ret < 0) > + goto done; > + > + for (i = 0; i < tp->die_sib; i++) { > + ret = do_write_string(ff, tp->die_siblings[i]); > + if (ret < 0) > + goto done; > + } > + > + for (j = 0; j < perf_env.nr_cpus_avail; j++) { > + ret = do_write(ff, &perf_env.cpu[j].die_id, > + sizeof(perf_env.cpu[j].die_id)); > + if (ret < 0) > + return ret; > + } > + > done: > cpu_topology__delete(tp); > return ret; > @@ -1443,6 +1464,16 @@ static void print_cpu_topology(struct feat_fd *ff, FILE *fp) > str += strlen(str) + 1; > } > > + if (ph->env.nr_sibling_dies) { > + nr = ph->env.nr_sibling_dies; > + str = ph->env.sibling_dies; > + > + for (i = 0; i < nr; i++) { > + fprintf(fp, "# sibling dies : %s\n", str); > + str += strlen(str) + 1; > + } > + } > + > nr = ph->env.nr_sibling_threads; > str = ph->env.sibling_threads; > > @@ -1451,12 +1482,28 @@ static void print_cpu_topology(struct feat_fd *ff, FILE *fp) > str += strlen(str) + 1; > } > > - if (ph->env.cpu != NULL) { > - for (i = 0; i < cpu_nr; i++) > - fprintf(fp, "# CPU %d: Core ID %d, Socket ID %d\n", i, > - ph->env.cpu[i].core_id, ph->env.cpu[i].socket_id); > - } else > - fprintf(fp, "# Core ID and Socket ID information is not available\n"); > + if (ph->env.nr_sibling_dies) { > + if (ph->env.cpu != NULL) { > + for (i = 0; i < cpu_nr; i++) > + fprintf(fp, "# CPU %d: Core ID %d, " > + "Die ID %d, Socket ID %d\n", > + i, ph->env.cpu[i].core_id, > + ph->env.cpu[i].die_id, > + ph->env.cpu[i].socket_id); > + } else > + fprintf(fp, "# Core ID, Die ID and Socket ID " > + "information is not available\n"); > + } else { > + if (ph->env.cpu != NULL) { > + for (i = 0; i < cpu_nr; i++) > + fprintf(fp, "# CPU %d: Core ID %d, " > + "Socket ID %d\n", > + i, ph->env.cpu[i].core_id, > + ph->env.cpu[i].socket_id); > + } else > + fprintf(fp, "# Core ID and Socket ID " > + "information is not available\n"); > + } > } > > static void print_clockid(struct feat_fd *ff, FILE *fp) > @@ -2214,6 +2261,7 @@ static int process_cpu_topology(struct feat_fd *ff, void *data __maybe_unused) > goto free_cpu; > > ph->env.cpu[i].core_id = nr; > + size += sizeof(u32); > > if (do_read_u32(ff, &nr)) > goto free_cpu; > @@ -2225,6 +2273,40 @@ static int process_cpu_topology(struct feat_fd *ff, void *data __maybe_unused) > } > > ph->env.cpu[i].socket_id = nr; > + size += sizeof(u32); > + } > + > + /* > + * The header may be from old perf, > + * which doesn't include die information. > + */ > + if (ff->size <= size) > + return 0; > + > + if (do_read_u32(ff, &nr)) > + return -1; > + > + ph->env.nr_sibling_dies = nr; > + size += sizeof(u32); > + > + for (i = 0; i < nr; i++) { > + str = do_read_string(ff); > + if (!str) > + goto error; > + > + /* include a NULL character at the end */ > + if (strbuf_add(&sb, str, strlen(str) + 1) < 0) > + goto error; > + size += string_size(str); > + free(str); > + } > + ph->env.sibling_dies = strbuf_detach(&sb, NULL); > + > + for (i = 0; i < (u32)cpu_nr; i++) { > + if (do_read_u32(ff, &nr)) > + goto free_cpu; > + > + ph->env.cpu[i].die_id = nr; > } > > return 0; > -- > 2.7.4
--
- Arnaldo
| |