Skip to contentSkip to conversation

perf sched timehist: Improve error message when analyzing wrong file

PATCH Superseded David Ahern 2 participants 3 messages Nov 29, 2016 perf events

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 perf sched timehist: Improve error message when analyzing wrong file

Arnaldo reported an unhelpful error message when running perf sched timehist on a file that did not contain sched tracepoints:

[root@jouet ~]# perf sched timehist
No trace sample to read. Did you call 'perf record -R'?

[root@jouet ~]# perf evlist -v
cycles:ppp: size: 112, { sample_period, sample_freq }: 4000, sample_type: IP|TID|TIME|CALLCHAIN|CPU|PERIOD, disabled: 1, inherit: 1, mmap: 1, comm: 1, freq: 1, task: 1, precise_ip: 3, sample_id_all: 1, exclude_guest: 1, mmap2: 1, comm_exec: 1

Add a helper to determine if a session has a specific event name, and then use that helper for timehist. For perf sched timehist the analysis requires at least sched_switch event, so check for it.

Now when analyzing a file without sched tracepoints you get:

    root@f21-vbox:/tmp$ perf sched timehist
    sched_switch event does not exist in file; nothing to analyze
---
 tools/perf/builtin-sched.c |  5 ++++-
 tools/perf/util/session.c  | 12 ++++++++++++
 tools/perf/util/session.h  |  2 ++
 3 files changed, 18 insertions(+), 1 deletion(-)
tools/perf/builtin-sched.c +4 -1
index 870d94cd20ba..7c3c8176f51f 100644
@@ -2629,8 +2629,11 @@ static int perf_sched__timehist(struct perf_sched *sched)
 	if (perf_session__set_tracepoints_handlers(session, handlers))
 		goto out;
 
-	if (!perf_session__has_traces(session, "record -R"))
+	/* sched_switch event at a minimum needs to exist */
+	if (!perf_session__has_event(session, "sched:sched_switch")) {
+		pr_err("sched_switch event does not exist in file; nothing to analyze\n");
 		goto out;
+	}
 
 	if (sched->show_migrations &&
 	    perf_session__set_tracepoints_handlers(session, migrate_handlers))
tools/perf/util/session.c +12 -0
index f268201048a0..9bb3e03dcd8e 100644
@@ -1897,6 +1897,18 @@ bool perf_session__has_traces(struct perf_session *session, const char *msg)
 	return false;
 }
 
+bool perf_session__has_event(struct perf_session *session, const char *name)
+{
+	struct perf_evsel *evsel;
+
+	evlist__for_each_entry(session->evlist, evsel) {
+		if (!strcmp(perf_evsel__name(evsel), name))
+			return true;
+	}
+
+	return false;
+}
+
 int maps__set_kallsyms_ref_reloc_sym(struct map **maps,
 				     const char *symbol_name, u64 addr)
 {
tools/perf/util/session.h +2 -0
index 4bd758553450..8609eca38c7f 100644
@@ -64,6 +64,8 @@ int perf_session__resolve_callchain(struct perf_session *session,
 
 bool perf_session__has_traces(struct perf_session *session, const char *msg);
 
+bool perf_session__has_event(struct perf_session *session, const char *name);
+
 void perf_event__attr_swap(struct perf_event_attr *attr);
 
 int perf_session__create_kernel_maps(struct perf_session *session);
Signature · 1 line
2.7.4 (Apple Git-66)
#1 lore raw

Em Tue, Nov 29, 2016 at 12:58:48PM -0700, David Ahern escreveu:

Quoted 6 lines

Arnaldo reported an unhelpful error message when running perf sched timehist on a file that did not contain sched tracepoints:

[root@jouet ~]# perf sched timehist
No trace sample to read. Did you call 'perf record -R'?

Quoted 6 lines

[root@jouet ~]# perf evlist -v
cycles:ppp: size: 112, { sample_period, sample_freq }: 4000, sample_type: IP|TID|TIME|CALLCHAIN|CPU|PERIOD, disabled: 1, inherit: 1, mmap: 1, comm: 1, freq: 1, task: 1, precise_ip: 3, sample_id_all: 1, exclude_guest: 1, mmap2: 1, comm_exec: 1

Add a helper to determine if a session has a specific event name, and then use that helper for timehist. For perf sched timehist the analysis requires at least sched_switch event, so check for it.

That is a much better error message, thanks! I went on to take a look if we had some pre-existing facility for that and found it, i.e. 'perf kmem' already checks this and we get:

[root@jouet ~]# perf record usleep 1
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.020 MB perf.data (7 samples) ] [root@jouet ~]# perf kmem stat --page
No page allocation events found. Have you run 'perf kmem record --page'? [root@jouet ~]# perf kmem stat --alloc
No slab allocation events found. Have you run 'perf kmem record --slab'? [root@jouet ~]#

It uses things like:

        if (kmem_page) {
                struct perf_evsel *evsel;

                evsel = perf_evlist__find_tracepoint_by_name(session->evlist,
                                                             "kmem:mm_page_alloc");
                if (evsel == NULL) {
                        pr_err(errmsg, "page", "page");
                        goto out_delete;
                }

- Arnaldo

Quoted 66 lines

Now when analyzing a file without sched tracepoints you get:

    root@f21-vbox:/tmp$ perf sched timehist
    sched_switch event does not exist in file; nothing to analyze
---
 tools/perf/builtin-sched.c |  5 ++++-
 tools/perf/util/session.c  | 12 ++++++++++++
 tools/perf/util/session.h  |  2 ++
 3 files changed, 18 insertions(+), 1 deletion(-)
tools/perf/builtin-sched.c +4 -1
index 870d94cd20ba..7c3c8176f51f 100644
@@ -2629,8 +2629,11 @@ static int perf_sched__timehist(struct perf_sched *sched)
 	if (perf_session__set_tracepoints_handlers(session, handlers))
 		goto out;
 
-	if (!perf_session__has_traces(session, "record -R"))
+	/* sched_switch event at a minimum needs to exist */
+	if (!perf_session__has_event(session, "sched:sched_switch")) {
+		pr_err("sched_switch event does not exist in file; nothing to analyze\n");
 		goto out;
+	}
 
 	if (sched->show_migrations &&
 	    perf_session__set_tracepoints_handlers(session, migrate_handlers))
tools/perf/util/session.c +12 -0
index f268201048a0..9bb3e03dcd8e 100644
@@ -1897,6 +1897,18 @@ bool perf_session__has_traces(struct perf_session *session, const char *msg)
 	return false;
 }
 
+bool perf_session__has_event(struct perf_session *session, const char *name)
+{
+	struct perf_evsel *evsel;
+
+	evlist__for_each_entry(session->evlist, evsel) {
+		if (!strcmp(perf_evsel__name(evsel), name))
+			return true;
+	}
+
+	return false;
+}
+
 int maps__set_kallsyms_ref_reloc_sym(struct map **maps,
 				     const char *symbol_name, u64 addr)
 {
tools/perf/util/session.h +2 -0
index 4bd758553450..8609eca38c7f 100644
@@ -64,6 +64,8 @@ int perf_session__resolve_callchain(struct perf_session *session,
 
 bool perf_session__has_traces(struct perf_session *session, const char *msg);
 
+bool perf_session__has_event(struct perf_session *session, const char *name);
+
 void perf_event__attr_swap(struct perf_event_attr *attr);
 
 int perf_session__create_kernel_maps(struct perf_session *session);
Signature · 1 line
2.7.4 (Apple Git-66)
#2 lore raw
Quoted 12 lines · Arnaldo Carvalho de Melo

It uses things like:

        if (kmem_page) {
                struct perf_evsel *evsel;

                evsel = perf_evlist__find_tracepoint_by_name(session->evlist,
                                                             "kmem:mm_page_alloc");
                if (evsel == NULL) {
                        pr_err(errmsg, "page", "page");
                        goto out_delete;
                }

I thought something like existed but did not find it looking at kvm and sched commands. Will re-spin to be consistent with kmem.