Re: [PATCH v5 08/10] perf report: implement record trace decompression

[Alexey Budankov <alexey.budankov@linux.intel.com>]

On 05.03.2019 15:25, Jiri Olsa wrote:
> On Fri, Mar 01, 2019 at 07:06:23PM +0300, Alexey Budankov wrote:
> 
> SNIP
> 
>> +static int __perf_session__process_decomp_events(struct perf_session *session)
>> +{
>> +	s64 skip;
>> +	u64 size, file_pos = 0;
>> +	union perf_event *event;
>> +	struct decomp *decomp = session->decomp_last;
>> +
>> +	if (!decomp)
>> +		return 0;
>> +
>> +	while (decomp->head < decomp->size && !session_done()) {
>> +		event = fetch_mmaped_event(session, decomp->head, decomp->size, decomp->data);
>> +		if (!event)
>> +			break;
>> +
>> +		size = event->header.size;
>> +		if (size < sizeof(struct perf_event_header) ||
>> +		    (skip = perf_session__process_event(session, event, file_pos)) < 0) {
>> +			pr_err("%#" PRIx64 " [%#x]: failed to process type: %d\n",
>> +				decomp->file_pos + decomp->head, event->header.size, event->header.type);
>> +			return -EINVAL;
>> +		}
>> +
>> +		if (skip)
>> +			size += skip;
>> +
>> +		decomp->head += size;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>  /*
>>   * On 64bit we can mmap the data file in one go. No need for tiny mmap
>>   * slices. On 32bit we use 32MB.
>> @@ -1933,6 +2051,10 @@ reader__process_events(struct reader *rd, struct perf_session *session,
>>  	head += size;
>>  	file_pos += size;
>>  
>> +	err = __perf_session__process_decomp_events(session);
>> +	if (err)
>> +		goto out;
> 
> why don't we process decompressed events directly from the 
> perf_session__process_compressed_event callback?
> 
> there would be no need for 'struct decomp' list logic

It would, because events need to stay in memory after decompression.
It looks reasonable to keep new processing code at the same place 
where the processing is implemented currently.

~Alexey

> 
> jirka
>