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

[Arnaldo Carvalho de Melo <acme@kernel.org>]

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