From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933518AbcK2UO0 (ORCPT ); Tue, 29 Nov 2016 15:14:26 -0500 Received: from mail.kernel.org ([198.145.29.136]:56450 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933318AbcK2UOL (ORCPT ); Tue, 29 Nov 2016 15:14:11 -0500 Date: Tue, 29 Nov 2016 17:14:04 -0300 From: Arnaldo Carvalho de Melo To: David Ahern Cc: mingo@kernel.org, peterz@infradead.org, namhyung@kernel.org, jolsa@kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] perf sched timehist: Improve error message when analyzing wrong file Message-ID: <20161129201404.GF23423@kernel.org> References: <1480449528-43394-1-git-send-email-dsahern@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1480449528-43394-1-git-send-email-dsahern@gmail.com> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.7.1 (2016-10-04) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Em Tue, Nov 29, 2016 at 12:58:48PM -0700, David Ahern escreveu: > 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. 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 > 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 > > Signed-off-by: David Ahern > --- > 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(-) > > diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c > index 870d94cd20ba..7c3c8176f51f 100644 > --- a/tools/perf/builtin-sched.c > +++ b/tools/perf/builtin-sched.c > @@ -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)) > diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c > index f268201048a0..9bb3e03dcd8e 100644 > --- a/tools/perf/util/session.c > +++ b/tools/perf/util/session.c > @@ -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) > { > diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h > index 4bd758553450..8609eca38c7f 100644 > --- a/tools/perf/util/session.h > +++ b/tools/perf/util/session.h > @@ -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); > -- > 2.7.4 (Apple Git-66)