Re: [PATCH V3 2/5] perf header: Add die information in CPU topology
From: Liang, Kan
Date: Thu Jun 06 2019 - 17:00:44 EST
On 6/6/2019 4:08 PM, Arnaldo Carvalho de Melo wrote:
Em Thu, Jun 06, 2019 at 04:12:10PM -0300, Arnaldo Carvalho de Melo escreveu:
Em Tue, Jun 04, 2019 at 03:50:41PM -0700, kan.liang@xxxxxxxxxxxxxxx escreveu:
From: Kan Liang <kan.liang@xxxxxxxxxxxxxxx>
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.
So I have this on top, please check.
It looks good to me.
Thanks,
Kan
- Arnaldo
commit a9396a70fc7101c108e1c91fa1771557bbbb57a1
Author: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
Date: Thu Jun 6 17:03:18 2019 -0300
perf data: Fix perf.data documentation for HEADER_CPU_TOPOLOGY
The 'die' info isn't in the same array as core and socket ids, and we
missed the 'dies' string list, that comes right after the 'core' +
'socket' id variable length array, followed by the VLA for the dies.
Cc: Adrian Hunter <adrian.hunter@xxxxxxxxx>
Cc: Andi Kleen <ak@xxxxxxxxxxxxxxx>
Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
Cc: Kan Liang <kan.liang@xxxxxxxxxxxxxxx>
Cc: Namhyung Kim <namhyung@xxxxxxxxxx>
Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
Fixes: c9cb12c5ba08 ("perf header: Add die information in CPU topology")
Link: https://lkml.kernel.org/n/tip-nubi6mxp2n8ofvlx7ph6k3h6@xxxxxxxxxxxxxx
Signed-off-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
diff --git a/tools/perf/Documentation/perf.data-file-format.txt b/tools/perf/Documentation/perf.data-file-format.txt
index de78183f6881..5f54feb19977 100644
--- a/tools/perf/Documentation/perf.data-file-format.txt
+++ b/tools/perf/Documentation/perf.data-file-format.txt
@@ -151,20 +151,35 @@ struct {
HEADER_CPU_TOPOLOGY = 13,
-String lists defining the core and CPU threads topology.
-The string lists are followed by a variable length array
-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.
-
struct {
+ /*
+ * First revision of HEADER_CPU_TOPOLOGY
+ *
+ * See 'struct perf_header_string_list' definition earlier
+ * in this file.
+ */
+
struct perf_header_string_list cores; /* Variable length */
struct perf_header_string_list threads; /* Variable length */
+
+ /*
+ * Second revision of HEADER_CPU_TOPOLOGY, older tools
+ * will not consider what comes next
+ */
+
struct {
uint32_t core_id;
- uint32_t die_id;
uint32_t socket_id;
} cpus[nr]; /* Variable length records */
+ /* 'nr' comes from previously processed HEADER_NRCPUS's nr_cpu_avail */
+
+ /*
+ * Third revision of HEADER_CPU_TOPOLOGY, older tools
+ * will not consider what comes next
+ */
+
+ struct perf_header_string_list dies; /* Variable length */
+ uint32_t die_id[nr_cpus_avail]; /* from previously processed HEADER_NR_CPUS, VLA */
};
Example: