Re: [PATCH v10 09/12] perf record: implement -z,--compression_level[=<n>] option
From: Alexey Budankov
Date: Fri May 17 2019 - 12:58:40 EST
On 17.05.2019 18:01, Arnaldo Carvalho de Melo wrote:
> Em Wed, May 15, 2019 at 06:44:29PM +0300, Alexey Budankov escreveu:
>> On 15.05.2019 15:59, Arnaldo Carvalho de Melo wrote:
<SNIP>
>>> 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]$
>>
>> Makes sense. Thanks.
>
> I did it yesterday, all is in my acme/perf/core branch, now testing it
> together with the large pile of patches there accumulated while I was in
> LSF/MM + vacations :-)
>
> All have already passed through most of my test build containers, with
> most of the distros that have libzstd being updated to include it, and
> the make_minimal test build target was updated to build explicitely
> disabling zstd, i.e. with NO_LIBZSTD=1, so that we test with/without it
> in systems where it is installed and also in systems where zstd is not
> even available.
Good news. Thanks!
~Alexey
>
> - Arnaldo
>