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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 23 Dec 2020 12:33:35 +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 On 12/23/2020 8:56 AM, Jin, Yao wrote: > 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. > For '-', another difficulty is it may conflict with the hardware cache event. Say we remove the "- { return '-'; }" from parse-events.l, such as: diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l index 9db5097317f4..145653d1ce16 100644 --- a/tools/perf/util/parse-events.l +++ b/tools/perf/util/parse-events.l @@ -387,7 +387,6 @@ r{num_raw_hex} { return raw(yyscanner); } {name} { return pmu_str_check(yyscanner, _parse_state); } {name_tag} { return str(yyscanner, PE_NAME); } "/" { BEGIN(config); return '/'; } -- { return '-'; } , { BEGIN(event); return ','; } : { return ':'; } "{" { BEGIN(event); return '{'; } The syntax of '-<event>' is supported. root@...-ppc:~# ./perf stat -e -cycles -a -- sleep 1 Performance counter stats for 'system wide': 14,008,859 -cycles 1.001471494 seconds time elapsed But the parsing of hardware cache event would be failed. :( root@...-ppc:~# ./perf stat -e LLC-stores -a -- sleep 1 event syntax error: 'LLC-stores' \___ parser error That complicates things. :( Thanks Jin Yao > 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