On Mon, Sep 07, 2015 at 12:51:55PM +0000, Wang Nan wrote:
Commit e1e499aba570a2ea84d29822b7ea637ac41d9a51 (perf tools: Addperf_event__preprocess_sample initializes al.socket from current system
processor socket info to hist_entry and addr_location) reads env->cpu
array for each sample using index al.cpu. However, al.cpu can be -1 if
sample doesn't select PERF_SAMPLE_CPU. Also, env->cpu can be invalid if
feature CPU_TOPOLOGY not selected. We should validate env->cpu and al.cpu
before setting al.socket.
Signed-off-by: Wang Nan <wangnan0@xxxxxxxxxx>
Cc: Kan Liang <kan.liang@xxxxxxxxx>
Cc: Adrian Hunter <adrian.hunter@xxxxxxxxx>
Cc: Andi Kleen <ak@xxxxxxxxxxxxxxx>
Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
Cc: Namhyung Kim <namhyung@xxxxxxxxxx>
Cc: Stephane Eranian <eranian@xxxxxxxxxx>
Cc: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
---
Although theoretically CPU_TOPOLOGY feature should always be selected by
'perf record', I did generate a perf.data without that feature. It has
header like this:
# perf report -i ./bad.perf.data --header-only
# ========
# captured on: Thu Jan 8 09:30:15 2009
# hostname : localhost
# os release : 3.10.49-gd672fc4
# perf version : 4.2.gc9df
# arch : aarch64
# nrcpus online : 8
# nrcpus avail : 8
# total memory : 1850768 kB
# cmdline : /system/bin/perf record -e sync:sync_timeline -e kgsl:kgsl_register_event -g -a sleep 5
# event : name = sync:sync_timeline, , id = { 1107, 1108, 1109, 1110, 1111, 1112 }, type = 2, size = 112, config = 0x3e7, { sample_period, sample_freq } = 1, sample_type = IP|TID|TIME|CALLCHAIN|ID|CPU|PERIOD|RAW, read_format = ID, disabled = 1, inherit = 1, mmap = 1, comm = 1, task = 1, sample_id_all = 1, exclude_guest = 1, mmap2 = 1, comm_exec = 1
# event : name = kgsl:kgsl_register_event, , id = { 1113, 1114, 1115, 1116, 1117, 1118 }, type = 2, size = 112, config = 0x350, { sample_period, sample_freq } = 1, sample_type = IP|TID|TIME|CALLCHAIN|ID|CPU|PERIOD|RAW, read_format = ID, disabled = 1, inherit = 1, sample_id_all = 1, exclude_guest = 1
# pmu mappings: cpu = 4, software = 1, tracepoint = 2
# ========
#
It should be:
# ========
# captured on: Thu Jan 8 11:26:41 2009
...
# HEADER_CPU_TOPOLOGY info available, use -I to display
# pmu mappings: cpu = 4, software = 1, tracepoint = 2
# ========
However, bad perf.data appears randomly. I can't stably reproduce it, so I
guess there might have another invalid memory accessing.
---
tools/perf/builtin-report.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 4b43245..16d097d 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -158,8 +158,16 @@ static int process_sample_event(struct perf_tool *tool,
return -1;
}
- /* read socket id from perf.data for perf report */
- al.socket = env->cpu[al.cpu].socket_id;
+ /*
+ * read socket id from perf.data for perf report
+ * al.cpu is invalid if PERF_SAMPLE_CPU is not selected by this
+ * sample.
+ * env->cpu is invalid if CPU_TOPOLOGY feature is not set in
+ * header.
+ */
+ al.socket = -1;
+ if (env->cpu && al.cpu >= 0)
+ al.socket = env->cpu[al.cpu].socket_id;
do we want to move this over there?
also this change is just report specific, and we could need
this in at least perf top
jirka