linux-kernel - Re: [PATCH 02/25] perf stat record: Add record command

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20151106133303.GB13236@kernel.org>
Date:	Fri, 6 Nov 2015 10:33:03 -0300
From:	Arnaldo Carvalho de Melo <acme@...nel.org>
To:	Jiri Olsa <jolsa@...hat.com>
Cc:	Jiri Olsa <jolsa@...nel.org>, lkml <linux-kernel@...r.kernel.org>,
	David Ahern <dsahern@...il.com>,
	Ingo Molnar <mingo@...nel.org>,
	Namhyung Kim <namhyung@...nel.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Kan Liang <kan.liang@...el.com>
Subject: Re: [PATCH 02/25] perf stat record: Add record command

Em Fri, Nov 06, 2015 at 09:24:00AM +0100, Jiri Olsa escreveu:
> On Thu, Nov 05, 2015 at 05:51:01PM -0300, Arnaldo Carvalho de Melo wrote:
> 
> SNIP
> 
> > In the second case it almost works, modulo that warning.
> > 
> > I think that what we need to achieve is for older tools to be able to, with a
> > file produced by 'perf stat record', to show this:
> > 
> > [root@zoo ~]# perf report --no-header --stdio
> > Error:
> > The perf.data file has no samples!
> > # To display the perf.data header info, please use --header/--header-only options.
> > #
> > [root@zoo ~]# 
> > 
> > 
> > I.e. the file should look like one that is produced by this command, purposely
> > to not create any sample:
> > 
> > # perf record -e syscalls:sys_enter_accept usleep 1
> > [ perf record: Woken up 1 times to write data ]
> > [ perf record: Captured and wrote 0.018 MB perf.data ]
> > 
> > 
> > I applied the first patch and added it to that perf/stat branch.
> 
> well.. it's either simple patches and step by step
> functionality or one big with everything..

Humm, no, there are several things we should strive for, and
bisectability is one of the first, it requires smaller, self contained
patches, sure, but it also requires that at after applying each patch we
have sane output from the tools.

So, after applying the patch above we get a message that says the file
is corrupted, and more than that, it even forgets to put a newline,
further breaking the output.
 
> [PATCH 02/25] perf stat record: Add record command
>    - adds record command that creates empty perf.data
> 
> [PATCH 03/25] perf stat record: Initialize record features
>    - adds FEATURES initialization for stat data
> 
> [PATCH 04/25] perf stat record: Synthesize stat record data
>    - adds meta data
> 
> [PATCH 05/25] perf stat record: Store events IDs in perf data file
>    - adds event IDs
> ...
> 
> 
> you get proper warning right after patch 3/25, where
> we store STAT feature bit and properly check it when
> opening perf.data

But that will be will _new_ tools, right? I'm talking about getting sane
output from _older_, unmodified, tools, like I demonstrated.

Anyway, I'll take the time to fix the broken missing newline and will
check those first few patches to see if I have a suggestion for you on
how to group them.

- Arnaldo

> I can merge patch 2 and 3 to get the proper warning
> from begining.. but that'd be bigger patch ;-)
> 
> thanks,
> jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/