linux-kernel - Re: [PATCH] perf sched timehist: Improve error message when analyzing wrong file

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20161129201404.GF23423@kernel.org>
Date:   Tue, 29 Nov 2016 17:14:04 -0300
From:   Arnaldo Carvalho de Melo <acme@...nel.org>
To:     David Ahern <dsahern@...il.com>
Cc:     mingo@...nel.org, peterz@...radead.org, namhyung@...nel.org,
        jolsa@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] perf sched timehist: Improve error message when
 analyzing wrong file

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@...et ~]# perf sched timehist
>     No trace sample to read. Did you call 'perf record -R'?
> 
>     [root@...et ~]# 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@...et ~]# 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@...et ~]# perf kmem stat --page
No page allocation events found.  Have you run 'perf kmem record --page'?
[root@...et ~]# perf kmem stat --alloc
No slab allocation events found.  Have you run 'perf kmem record --slab'?
[root@...et ~]#

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@...-vbox:/tmp$ perf sched timehist
>     sched_switch event does not exist in file; nothing to analyze
> 
> Signed-off-by: David Ahern <dsahern@...il.com>
> ---
>  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)