| 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
| ||
|
Message-ID: <CAP-5=fXuR2tK_4FDnRA88a9wQkndbqyeN6G2muppaMhV5mAQ=A@mail.gmail.com>
Date: Mon, 11 Apr 2022 08:29:30 -0700
From: Ian Rogers <irogers@...gle.com>
To: Florian Fischer <florian.fischer@...q.space>
Cc: Xing Zhengjun <zhengjun.xing@...ux.intel.com>,
linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Florian Schmaus <flow@...fau.de>
Subject: Re: [PATCH v3 2/3] perf stat: add rusage utime and stime events
On Sun, Apr 10, 2022 at 9:41 AM Florian Fischer
<florian.fischer@...q.space> wrote:
>
> > > This patch adds two new tool internal events 'rusage_user_time'
> > > and 'rusage_system_time' as well as their aliases 'ru_utime' and
> > > 'ru_stime', similarly to the already present 'duration_time' event.
> > >
> > > Both events use the already collected rusage information obtained by wait4
> > > and tracked in the global ru_stats.
> > >
> > > Examples presenting cache-misses and rusage information in both human and
> > > machine-readable form:
> > >
> > > $ ./perf stat -e duration_time,ru_utime,ru_stime,cache-misses -- grep -q -r duration_time .
> > >
> > > Performance counter stats for 'grep -q -r duration_time .':
> > >
> > > 67,422,542 ns duration_time:u
> > > 50,517,000 ns ru_utime:u
> > > 16,839,000 ns ru_stime:u
> > > 30,937 cache-misses:u
> > >
> > > 0.067422542 seconds time elapsed
> > >
> > > 0.050517000 seconds user
> > > 0.016839000 seconds sys
> > >
> > > $ ./perf stat -x, -e duration_time,ru_utime,ru_stime,cache-misses -- grep -q -r duration_time .
> > > 72134524,ns,duration_time:u,72134524,100.00,,
> > > 65225000,ns,ru_utime:u,65225000,100.00,,
> > > 6865000,ns,ru_stime:u,6865000,100.00,,
> > > 38705,,cache-misses:u,71189328,100.00,,
> >
> > This is really nice. For metric code we currently handle duration_time
> > in a special way, for example:
> > https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/tree/tools/perf/util/metricgroup.c?h=perf/core#n745
> > https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/tree/tools/perf/util/metricgroup.c?h=perf/core#n1131
> > We will need to do something similar with these tool events, but I'm
> > happy that it can be follow-up work.
> >
> > I'm not a huge fan of the names ru_utime and ru_stime, two thoughts
> > here we could do duration_time:u and duration_time:k but I don't think
> > that really makes sense. My preference would be to just call ru_utime
> > user_time and ru_stime system_time.
>
> I considered ru_{u,s}_time only as aliases because those are the field names in
> the rusage struct filled by wait4 and are probably known by perf users.
> The "official" names are currently rusage_{user,system}_time.
> I could change them to only {user,system}_time because those names are more in line
> with the already present duration_time and are independent of the rusage
> implementation detail.
>
> What do you think of?
I like user_time and system_time, short and to the point [1] while
satisfying being intention-revealing. The aliases, ru_utime and
ru_stime, are fine but a bit of an acronym soup. They mean we need to
special case more names in the metric code. I don't know how others
feel but I'd stick to just user_time and system_time.
Thanks,
Ian
[1] https://www.kernel.org/doc/html/v4.10/process/coding-style.html#naming
> ---
> tools/perf/util/parse-events.c | 4 ++--
> tools/perf/util/parse-events.l | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index c232ab79d434..afcba6671748 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -160,11 +160,11 @@ struct event_symbol event_symbols_tool[PERF_TOOL_LAST] = {
> .alias = "",
> },
> [PERF_TOOL_RU_UTIME] = {
> - .symbol = "rusage_user_time",
> + .symbol = "user_time",
> .alias = "ru_utime",
> },
> [PERF_TOOL_RU_STIME] = {
> - .symbol = "rusage_system_time",
> + .symbol = "system_time",
> .alias = "ru_stime",
> },
> };
> diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> index 3c7227b8035c..7ee8613b6011 100644
> --- a/tools/perf/util/parse-events.l
> +++ b/tools/perf/util/parse-events.l
> @@ -353,8 +353,8 @@ alignment-faults { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_AL
> emulation-faults { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_EMULATION_FAULTS); }
> dummy { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_DUMMY); }
> duration_time { return tool(yyscanner, PERF_TOOL_DURATION_TIME); }
> -rusage_user_time|ru_utime { return tool(yyscanner, PERF_TOOL_RU_UTIME); }
> -rusage_system_time|ru_stime { return tool(yyscanner, PERF_TOOL_RU_STIME); }
> +user_time|ru_utime { return tool(yyscanner, PERF_TOOL_RU_UTIME); }
> +system_time|ru_stime { return tool(yyscanner, PERF_TOOL_RU_STIME); }
> bpf-output { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_BPF_OUTPUT); }
> cgroup-switches { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_CGROUP_SWITCHES); }
>
> --
> 2.35.1
>
> Florian Fischer
Powered by blists - more mailing lists