Re: [PATCH 1/2] perf arm-spe: Add arm_spe_record to synthesized

[Ali Saidi]

On 26/01/2022 19:07, German Gomez wrote:
[...]
>>> Have you tried this with perf-inject? I think it would need the PERF_SAMPLE_RAW bit in the sample_type,
>> Yes I've tried the following and it worked as expected with the original
>> perf.data or the perf.data.jitted after perf-inject. 
>>
>> perf record -e arm_spe_0/jitter=1/ -k 1 java ...
>> perf  inject -f --jit -i perf.data -o perf.data.jitted
>
>This is not injecting the synthesized samples. I think it is still    
>processing from the aux trace. Try adding "--itrace=i1i --strip" to the
>inject command to remove the AUXTRACE events. Judging by the raw
>samples, the data is missing:
>
> [...]

Yep, you're correct here. If I use the command above the raw samples are lost.

>>> Although I quickly looked over the perf inject code and it looks like it's expecting some type of padding:
>>>
>>> [...]
>>>
>>> I'm seeing some comments in utils/event.h related to this on the intel events.
>> Yes i noticed this too,but looking at how the raw data is added to the same
>> other places like intel-pt.c:1703 the perf_synth__raw*() functions are used to
>> strip away the 4 bytes bytes before the data is added to the sample. The other
>> places i can find the padding used is in builtin-script.c but given we have the
>> --dump-raw-trace option it's not clear to me that it's needed to wrap the
>> arm_spe_event in another struct with padding like perf_synth_intel_ptwrite?
>
>I think the intel use case makes sense because the layout of the data
>is fixed and documented. If we modify the struct arm_spe_record later it
>may not be obvious how to match it to the raw data of an older perf.data
>file. And we're generating bigger files with redundant information.

Not injecting the samples into the perf trace, but having a way to support
custom scripts parsing the data would be really useful and much faster than
trying to parse back the --dump-raw-trace output into something useful. The
other way to go would be to put a header that describes the version of the spe
struct at the head of it to address any future changes, but I'm not familiar
with a workflow that would benefit from the added complexity. 

Thanks,
Ali