Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
Date:   Wed, 15 May 2019 09:59:51 -0300
To:     Alexey Budankov <alexey.budankov@linux.intel.com>
Cc:     Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>,
        Jiri Olsa <jolsa@redhat.com>,
        Namhyung Kim <namhyung@kernel.org>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Andi Kleen <ak@linux.intel.com>,
        linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v10 09/12] perf record: implement
 -z,--compression_level[=<n>] option
Message-ID: <20190515123802.GA23162@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.11.3 (2019-02-01)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Em Wed, May 15, 2019 at 11:43:30AM +0300, Alexey Budankov escreveu:
> On 15.05.2019 0:46, Arnaldo Carvalho de Melo wrote:
> > Em Tue, May 14, 2019 at 05:20:41PM -0300, Arnaldo Carvalho de Melo escreveu:
> >> Em Mon, Mar 18, 2019 at 08:44:42PM +0300, Alexey Budankov escreveu:

> >>> Implemented -z,--compression_level[=<n>] option that enables compression
> >>> of mmaped kernel data buffers content in runtime during perf record
> >>> mode collection. Default option value is 1 (fastest compression).

> > <SNIP>

> >> [root@quaco ~]# perf record -z2
> >> ^C[ perf record: Woken up 1 times to write data ]
> >> 0x1746e0 [0x76]: failed to process type: 81 [Invalid argument]
> >> [ perf record: Captured and wrote 1.568 MB perf.data, compressed (original 0.452 MB, ratio is 3.995) ]

> >> [root@quaco ~]#

> > So, its the buildid processing at the end, so we can't do build-id
> > processing when using PERF_RECORD_COMPRESSED, otherwise we'd have to
> > uncompress at the end to find the PERF_RECORD_FORK/PERF_RECORD_MMAP,
> > etc.

> > [root@quaco ~]# perf record -z2  --no-buildid sleep 1
> > [ perf record: Woken up 1 times to write data ]
> > [ perf record: Captured and wrote 0.020 MB perf.data, compressed (original 0.001 MB, ratio is 2.153) ]
> > [root@quaco ~]# perf report -D | grep PERF_RECORD_COMP
> > 0x4f40 [0x195]: failed to process type: 81 [Invalid argument]
> > Error:
> > failed to process sample
> > 0 0x4f40 [0x195]: PERF_RECORD_COMPRESSED
> > [root@quaco ~]#

> > I'll play with it tomorrow.

> Applied the whole patch set on top of the current perf/core 
> and the whole thing functions as expected.

It doesn't, see the reported error above, these three lines, that
shouldn't be there:

0x4f40 [0x195]: failed to process type: 81 [Invalid argument]
Error:
failed to process sample

That is because at this point in the patch series a record was
introduced that is not being handled by the build id processing done, by
default, at the end of the 'perf record' session, and, as explained
above, needs fixing so that when we do 'git bisect' looking for a non
expected "failed to process type: 81" kind of error, this doesn't
appear.

I added the changes below to this cset and will continue from there:

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index d84a4885e341..f8d21991f94c 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -2284,6 +2284,12 @@ int cmd_record(int argc, const char **argv)
 			"cgroup monitoring only available in system-wide mode");
 
 	}
+
+	if (rec->opts.comp_level != 0) {
+		pr_debug("Compression enabled, disabling build id collection at the end of the session\n");
+		rec->no_buildid = true;
+	}
+
 	if (rec->opts.record_switch_events &&
 	    !perf_can_record_switch_events()) {
 		ui__error("kernel does not support recording context switch events\n");

---------------------------------------------------------------------------

[acme@quaco perf]$ perf record -z2 sleep 1
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.001 MB perf.data, compressed (original 0.001 MB, ratio is 2.292) ]
[acme@quaco perf]$ perf record -v -z2 sleep 1
Compression enabled, disabling build id collection at the end of the session
Using CPUID GenuineIntel-6-8E-A
nr_cblocks: 0
affinity: SYS
mmap flush: 1
comp level: 2
mmap size 528384B
Couldn't start the BPF side band thread:
BPF programs starting from now on won't be annotatable
perf_event__synthesize_bpf_events: can't get next program: Operation not permitted
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.001 MB perf.data, compressed (original 0.001 MB, ratio is 2.305) ]
[acme@quaco perf]$

Will check if its possible to get rid of the following in this patch, to
keep bisection working for this case as well:

[acme@quaco perf]$ perf report -D | grep COMPRESS
0x1b8 [0x169]: failed to process type: 81 [Invalid argument]
Error:
failed to process sample
0 0x1b8 [0x169]: PERF_RECORD_COMPRESSED
[acme@quaco perf]$

- Arnaldo