Re: [PATCH 3/4] perf tools: Add perf_evsel__read_counter function

From: Arnaldo Carvalho de Melo
Date: Tue Jul 25 2017 - 21:41:58 EST

Em Fri, Jul 21, 2017 at 02:12:11PM +0200, Jiri Olsa escreveu:
> Adding perf_evsel__read_counter function to read single or
> group counter. After calling this function the counter's
> evsel::counts struct is filled with values for the counter
> and member of its group if there are any.
>
> Link: http://lkml.kernel.org/n/tip-itsuxdyt7rp4mvij1t6k7bcl@xxxxxxxxxxxxxx
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
> tools/perf/util/evsel.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++++
> tools/perf/util/evsel.h | 2 +
> tools/perf/util/stat.c | 3 ++
> tools/perf/util/stat.h | 5 ++-
> 4 files changed, 108 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 4dd0fcc06db9..89aecf3a35c7 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -1302,6 +1302,106 @@ int perf_evsel__read(struct perf_evsel *evsel, int cpu, int thread,
> return 0;
> }
>
> +static int
> +perf_evsel__read_one(struct perf_evsel *evsel, int cpu, int thread)
> +{
> + struct perf_counts_values *count = perf_counts(evsel->counts, cpu, thread);
> +
> + return perf_evsel__read(evsel, cpu, thread, count);
> +}
> +
> +static void
> +perf_evsel__set_count(struct perf_evsel *counter, int cpu, int thread,
> + u64 val, u64 ena, u64 run)
> +{
> + struct perf_counts_values *count;
> +
> + count = perf_counts(counter->counts, cpu, thread);
> +
> + count->val = val;
> + count->ena = ena;
> + count->run = run;
> +}
> +
> +static int
> +perf_evsel__process_group_data(struct perf_evsel *leader,
> + int cpu, int thread, u64 *data)
> +{
> + u64 read_format = leader->attr.read_format;
> + struct sample_read_value *v;
> + u64 nr, ena = 0, run = 0, i;
> +
> + nr = *data++;
> +
> + if (nr != (u64) leader->nr_members)
> + return -EINVAL;
> +
> + if (read_format & PERF_FORMAT_TOTAL_TIME_ENABLED)
> + ena = *data++;
> +
> + if (read_format & PERF_FORMAT_TOTAL_TIME_RUNNING)
> + run = *data++;
> +
> + v = (struct sample_read_value *) data;
> +
> + perf_evsel__set_count(leader, cpu, thread,
> + v[0].value, ena, run);
> +
> + for (i = 1; i < nr; i++) {
> + struct perf_evsel *counter;
> +
> + counter = perf_evlist__id2evsel(leader->evlist, v[i].id);
> + if (!counter)
> + return -EINVAL;
> +
> + perf_evsel__set_count(counter, cpu, thread,
> + v[i].value, ena, run);
> + }
> +
> + return 0;
> +}
> +
> +static int
> +perf_evsel__read_group(struct perf_evsel *leader, int cpu, int thread)
> +{
> + struct perf_stat_evsel *ps = leader->priv;
> + u64 read_format = leader->attr.read_format;
> + int size = perf_evsel__read_size(leader);
> + u64 *data = ps->group_data;
> +
> + if (!(read_format & PERF_FORMAT_ID))
> + return -EINVAL;
> +
> + if (!perf_evsel__is_group_leader(leader))
> + return -EINVAL;
> +
> + if (!data) {
> + data = zalloc(size);
> + if (!data)
> + return -ENOMEM;
> +
> + ps->group_data = data;
> + }
> +
> + if (FD(leader, cpu, thread) < 0)
> + return -EINVAL;
> +
> + if (readn(FD(leader, cpu, thread), data, size) <= 0)
> + return -errno;
> +
> + return perf_evsel__process_group_data(leader, cpu, thread, data);
> +}
> +
> +int perf_evsel__read_counter(struct perf_evsel *evsel, int cpu, int thread)
> +{
> + u64 read_format = evsel->attr.read_format;
> +
> + if (read_format & PERF_FORMAT_GROUP)
> + return perf_evsel__read_group(evsel, cpu, thread);
> + else
> + return perf_evsel__read_one(evsel, cpu, thread);
> +}
> +
> int __perf_evsel__read_on_cpu(struct perf_evsel *evsel,
> int cpu, int thread, bool scale)
> {
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index fb40ca3c6519..de03c18daaf0 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -299,6 +299,8 @@ static inline bool perf_evsel__match2(struct perf_evsel *e1,
> int perf_evsel__read(struct perf_evsel *evsel, int cpu, int thread,
> struct perf_counts_values *count);
>
> +int perf_evsel__read_counter(struct perf_evsel *evsel, int cpu, int thread);
> +
> int __perf_evsel__read_on_cpu(struct perf_evsel *evsel,
> int cpu, int thread, bool scale);
>
> diff --git a/tools/perf/util/stat.c b/tools/perf/util/stat.c
> index 53b9a994a3dc..2c258554f94d 100644
> --- a/tools/perf/util/stat.c
> +++ b/tools/perf/util/stat.c
> @@ -128,6 +128,9 @@ static int perf_evsel__alloc_stat_priv(struct perf_evsel *evsel)
>
> static void perf_evsel__free_stat_priv(struct perf_evsel *evsel)
> {
> + struct perf_stat_evsel *ps = evsel->priv;
> +
> + free(ps->group_data);

Humm, are you sure you can always do that, i.e. that evsel->priv is not
NULL?

Program received signal SIGSEGV, Segmentation fault.
0x000000000054ca04 in perf_evsel__free_stat_priv (evsel=0x2454be0) at util/stat.c:133
133 free(ps->group_data);
Missing separate debuginfos, use: dnf debuginfo-install audit-libs-2.7.7-1.fc25.x86_64 elfutils-libelf-0.169-1.fc25.x86_64 elfutils-libs-0.169-1.fc25.x86_64 libunwind-1.2-1.fc25.x86_64 perl-libs-5.24.1-386.fc25.x86_64 python-libs-2.7.13-2.fc25.x86_64 slang-2.3.0-7.fc25.x86_64
(gdb) bt
#0 0x000000000054ca04 in perf_evsel__free_stat_priv (evsel=0x2454be0) at util/stat.c:133
#1 0x000000000054cc93 in perf_evlist__free_stats (evlist=0x24541a0) at util/stat.c:189
#2 0x0000000000460311 in cmd_script (argc=0, argv=0x7fffffffe170) at builtin-script.c:3075
#3 0x00000000004be56c in run_builtin (p=0xa359f8 <commands+408>, argc=1, argv=0x7fffffffe170) at perf.c:296
#4 0x00000000004be7d9 in handle_internal_command (argc=1, argv=0x7fffffffe170) at perf.c:348
#5 0x00000000004be92b in run_argv (argcp=0x7fffffffdfcc, argv=0x7fffffffdfc0) at perf.c:392
#6 0x00000000004bed05 in main (argc=1, argv=0x7fffffffe170) at perf.c:530
(gdb)

I'm dropping the series, please retest with 'perf script'.

- Arnaldo

> zfree(&evsel->priv);
> }
>
> diff --git a/tools/perf/util/stat.h b/tools/perf/util/stat.h
> index 7522bf10b03e..eacaf958e19d 100644
> --- a/tools/perf/util/stat.h
> +++ b/tools/perf/util/stat.h
> @@ -28,8 +28,9 @@ enum perf_stat_evsel_id {
> };
>
> struct perf_stat_evsel {
> - struct stats res_stats[3];
> - enum perf_stat_evsel_id id;
> + struct stats res_stats[3];
> + enum perf_stat_evsel_id id;
> + u64 *group_data;
> };
>
> enum aggr_mode {
> --
> 2.9.4