| 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
| ||
|
Date: Mon, 8 Oct 2018 14:37:13 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Alexey Budankov <alexey.budankov@...ux.intel.com>
Cc: Jiri Olsa <jolsa@...hat.com>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...nel.org>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Namhyung Kim <namhyung@...nel.org>,
Andi Kleen <ak@...ux.intel.com>,
linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3] perf record: encode -k clockid frequency into Perf
trace
Em Sat, Oct 06, 2018 at 04:14:11PM +0300, Alexey Budankov escreveu:
> On 06.10.2018 14:51, Arnaldo Carvalho de Melo wrote:
> > Em Thu, Oct 04, 2018 at 02:36:57PM +0200, Jiri Olsa escreveu:
> >> On Wed, Oct 03, 2018 at 07:57:12PM +0300, Alexey Budankov wrote:
> >>>
> >>> Store -k clockid frequency into Perf trace to enable timestamps
> >>> derived metrics conversion into wall clock time on reporting stage.
> >>>
> >>> Below is the example of perf report output:
> >>>
> >>> tools/perf/perf record -k raw -- ../../matrix/linux/matrix.gcc
> >>> ...
> >>> [ perf record: Captured and wrote 31.222 MB perf.data (818054 samples) ]
> >>>
> >>> tools/perf/perf report --header
> >>> # ========
> >>> ...
> >>> # event : name = cycles:ppp, , size = 112, { sample_period, sample_freq } = 4000, sample_type = IP|TID|TIME|PERIOD, disabled = 1, inherit = 1, mmap = 1, comm = 1, freq = 1, enable_on_exec = 1, task = 1, precise_ip = 3, sample_id_all = 1, exclude_guest = 1, mmap2 = 1, comm_exec = 1, use_clockid = 1, clockid = 4
> >>> ...
> >>> # clockid frequency: 1000 MHz
> >>> ...
> >>> # ========
> >>>
> >>> Signed-off-by: Alexey Budankov <alexey.budankov@...ux.intel.com>
> >> Reviewed-by: Jiri Olsa <jolsa@...nel.org>
> >
> >
> >
> > CC /tmp/build/perf/builtin-script.o
> > CC /tmp/build/perf/util/header.o
> > util/header.c: In function 'print_clockid':
> > util/header.c:1520:38: error: format '%ld' expects argument of type 'long int', but argument 3 has type 'size_t' {aka 'unsigned int'} [-Werror=format=]
> > fprintf(fp, "# clockid frequency: %ld MHz\n",
> > ~~^
> > %d
> > ff->ph->env.clockid_res_ns * 1000);
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > util/header.c: In function 'process_clockid':
> > util/header.c:2552:22: error: passing argument 2 of 'do_read_u64' from incompatible pointer type [-Werror=incompatible-pointer-types]
> > if (do_read_u64(ff, &clockid_res_ns))
> > ^~~~~~~~~~~~~~~
> > util/header.c:229:49: note: expected 'u64 *' {aka 'long long unsigned int *'} but argument is of type 'size_t *' {aka 'unsigned int *'}
> > static int do_read_u64(struct feat_fd *ff, u64 *addr)
> > ~~~~~^~~~
> > In file included from /usr/mips-linux-gnu/include/string.h:494,
> > from util/string2.h:7,
> > from util/header.c:5:
> > In function 'memcpy',
> > inlined from '__do_read_buf' at util/header.c:202:2,
> > inlined from '__do_read' at util/header.c:213:9,
> > inlined from 'do_read_u64' at util/header.c:233:8,
> > inlined from 'process_clockid' at util/header.c:2552:6:
> > /usr/mips-linux-gnu/include/bits/string_fortified.h:34:10: error: '__builtin___memcpy_chk' forming offset [5, 8] is out of the bounds [0, 4] of object 'clockid_res_ns' with type 'size_t' {aka 'unsigned int'} [-Werror=array-bounds]
> > return __builtin___memcpy_chk (__dest, __src, __len, __bos0 (__dest));
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > util/header.c: In function 'process_clockid':
> > util/header.c:2550:9: note: 'clockid_res_ns' declared here
> > size_t clockid_res_ns;
> > ^~~~~~~~~~~~~~
> > MKDIR /tmp/build/perf/scripts/
> > LD /tmp/build/perf/scripts/libperf-in.o
> > CC /tmp/build/perf/builtin-kmem.o
> > CC /tmp/build/perf/builtin-lock.o
> > CC /tmp/build/perf/ui/browsers/map.o
> > CC /tmp/build/perf/builtin-kvm.o
> > cc1: all warnings being treated as errors
> > mv: cannot stat '/tmp/build/perf/util/.header.o.tmp': No such file or directory
> > make[4]: *** [/git/linux/tools/build/Makefile.build:97: /tmp/build/perf/util/header.o] Error 1
> > make[3]: *** [/git/linux/tools/build/Makefile.build:139: util] Error 2
> > make[3]: *** Waiting for unfinished jobs....
> >
>
> Hope this helps.
>
> ---
> tools/perf/util/header.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 4ce5339158f7..afaebbb53035 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -1517,7 +1517,7 @@ static void print_cpu_topology(struct feat_fd *ff, FILE *fp)
>
> static void print_clockid(struct feat_fd *ff, FILE *fp)
> {
> - fprintf(fp, "# clockid frequency: %ld MHz\n",
> + fprintf(fp, "# clockid frequency: %zd MHz\n",
> ff->ph->env.clockid_res_ns * 1000);
> }
>
> @@ -2549,7 +2549,7 @@ static int process_clockid(struct feat_fd *ff,
> {
> size_t clockid_res_ns;
>
> - if (do_read_u64(ff, &clockid_res_ns))
> + if (__do_read(ff, &clockid_res_ns, sizeof(clockid_res_ns)))
> return -1;
>
> ff->ph->env.clockid_res_ns = clockid_res_ns;
Are you sure about that? Isn't this always encoded as an u64? Also, look
at what else do_read_u64 does besides just reading:
static int do_read_u64(struct feat_fd *ff, u64 *addr)
{
int ret;
ret = __do_read(ff, addr, sizeof(*addr));
if (ret)
return ret;
if (ff->ph->needs_swap)
*addr = bswap_64(*addr);
return 0;
}
/me goes to look at the original patch to see how you encode that
clod_id_res_ns in the perf.data header...
- Arnaldo
Powered by blists - more mailing lists