[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <02c473b8-ea6c-1a19-6dd1-0041d8944303@linux.intel.com>
Date: Wed, 23 Dec 2020 08:56:15 +0800
From: "Jin, Yao" <yao.jin@...ux.intel.com>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: jolsa@...nel.org, peterz@...radead.org, mingo@...hat.com,
alexander.shishkin@...ux.intel.com, Linux-kernel@...r.kernel.org,
ak@...ux.intel.com, kan.liang@...el.com, yao.jin@...el.com
Subject: Re: [PATCH] perf stat: Create '--add-default' option to append
default list
Hi Arnaldo,
On 12/23/2020 12:15 AM, Arnaldo Carvalho de Melo wrote:
> Em Tue, Dec 22, 2020 at 09:11:31AM +0800, Jin Yao escreveu:
>> The event default list includes the most common events which are widely
>> used by users. But with -e option, the current perf only counts the events
>> assigned by -e option. Users may want to collect some extra events with
>> the default list. For this case, users have to manually add all the events
>> from the default list. It's inconvenient. Also, users may don't know how to
>> get the default list.
>>
>> It's better to add a new option to append default list to the -e events.
>> The new option is '--add-default'.
>>
>> Before:
>>
>> root@...-ppc:~# ./perf stat -e power/energy-pkg/ -a -- sleep 1
>>
>> Performance counter stats for 'system wide':
>>
>> 2.05 Joules power/energy-pkg/
>>
>> 1.000857974 seconds time elapsed
>>
>> After:
>>
>> root@...-ppc:~# ./perf stat -e power/energy-pkg/ -a --add-default -- sleep 1
>
> I thought about:
>
> perf stat -e +power/energy-pkg/ -a -- sleep 1
>
I was surprised to see that '+<event>' syntax had been supported.
root@...-ppc:~# ./perf stat -e +power/energy-pkg/ -a -- sleep 1
Performance counter stats for 'system wide':
1.99 Joules +power/energy-pkg/
1.000877852 seconds time elapsed
root@...-ppc:~# ./perf stat -e +power/energy-pkg/,+cycles -a -- sleep 1
Performance counter stats for 'system wide':
2.00 Joules +power/energy-pkg/
13,780,620 +cycles
1.001639147 seconds time elapsed
Are there any scripts or usages need the prefix '+' before event? I don't know. But if we append the
'+<event>' to the default event list, will break something potentially?
> Which would have its counterpart:
>
> perf stat -e -cycles -0a --sleep 1
>
> To remove an event from the defaults, perhaps to deal with some specific
> hardware where the default or what is in -d, -dd, -ddd, etc can't all be
> counted. I.e. - and + would remove or add from whaver list was there at
> that point.
>
> - Arnaldo
>
Yes, + and - are more flexible solution. Just for above question, will '+<event>' break existing
usage? And for '-', I don't know if user can remember clearly for what the events are in default list.
Thanks
Jin Yao
>> Performance counter stats for 'system wide':
>>
>> 2.10 Joules power/energy-pkg/ # 0.000 K/sec
>> 8,009.89 msec cpu-clock # 7.995 CPUs utilized
>> 140 context-switches # 0.017 K/sec
>> 9 cpu-migrations # 0.001 K/sec
>> 66 page-faults # 0.008 K/sec
>> 10,671,929 cycles # 0.001 GHz
>> 4,736,880 instructions # 0.44 insn per cycle
>> 942,951 branches # 0.118 M/sec
>> 76,096 branch-misses # 8.07% of all branches
>>
>> 1.001809960 seconds time elapsed
>>
>> Signed-off-by: Jin Yao <yao.jin@...ux.intel.com>
>> ---
>> tools/perf/Documentation/perf-stat.txt | 5 +++++
>> tools/perf/builtin-stat.c | 4 +++-
>> tools/perf/util/stat.h | 1 +
>> 3 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
>> index 5d4a673d7621..75a83c2e4dc5 100644
>> --- a/tools/perf/Documentation/perf-stat.txt
>> +++ b/tools/perf/Documentation/perf-stat.txt
>> @@ -438,6 +438,11 @@ convenient for post processing.
>> --summary::
>> Print summary for interval mode (-I).
>>
>> +--add-default::
>> +The default event list includes the most common events which are widely
>> +used by users. But with -e option, the perf only counts the events assigned
>> +by -e option. This options appends the default event list to the -e events.
>> +
>> EXAMPLES
>> --------
>>
>> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
>> index 89c32692f40c..6ac7b946f9a7 100644
>> --- a/tools/perf/builtin-stat.c
>> +++ b/tools/perf/builtin-stat.c
>> @@ -1173,6 +1173,8 @@ static struct option stat_options[] = {
>> "print summary for interval mode"),
>> OPT_BOOLEAN(0, "quiet", &stat_config.quiet,
>> "don't print output (useful with record)"),
>> + OPT_BOOLEAN(0, "add-default", &stat_config.add_default,
>> + "add default events"),
>> #ifdef HAVE_LIBPFM
>> OPT_CALLBACK(0, "pfm-events", &evsel_list, "event",
>> "libpfm4 event selector. use 'perf list' to list available events",
>> @@ -1755,7 +1757,7 @@ static int add_default_attributes(void)
>> free(str);
>> }
>>
>> - if (!evsel_list->core.nr_entries) {
>> + if (!evsel_list->core.nr_entries || stat_config.add_default) {
>> if (target__has_cpu(&target))
>> default_attrs0[0].config = PERF_COUNT_SW_CPU_CLOCK;
>>
>> diff --git a/tools/perf/util/stat.h b/tools/perf/util/stat.h
>> index 9979b4b100f2..6ccc6936348c 100644
>> --- a/tools/perf/util/stat.h
>> +++ b/tools/perf/util/stat.h
>> @@ -123,6 +123,7 @@ struct perf_stat_config {
>> bool metric_no_merge;
>> bool stop_read_counter;
>> bool quiet;
>> + bool add_default;
>> FILE *output;
>> unsigned int interval;
>> unsigned int timeout;
>> --
>> 2.17.1
>>
>
Powered by blists - more mailing lists