On 19/07/22 20:09, Ian Rogers wrote:
> On Mon, Jul 11, 2022 at 2:32 AM Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
>>
>> Factor out evsel__id_hdr_size() so it can be reused.
>>
>> This is needed by perf inject. When injecting events from a guest perf.data
>> file, there is a possibility that the sample ID numbers conflict. To
>> re-write an ID sample, the old one needs to be removed first, which means
>> determining how big it is with evsel__id_hdr_size() and then subtracting
>> that from the event size.
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
>> ---
>> tools/perf/util/evlist.c | 28 +---------------------------
>> tools/perf/util/evsel.c | 26 ++++++++++++++++++++++++++
>> tools/perf/util/evsel.h | 2 ++
>> 3 files changed, 29 insertions(+), 27 deletions(-)
>>
>> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
>> index 48af7d379d82..03fbe151b0c4 100644
>> --- a/tools/perf/util/evlist.c
>> +++ b/tools/perf/util/evlist.c
>> @@ -1244,34 +1244,8 @@ bool evlist__valid_read_format(struct evlist *evlist)
>> u16 evlist__id_hdr_size(struct evlist *evlist)
>> {
>> struct evsel *first = evlist__first(evlist);
>> - struct perf_sample *data;
>> - u64 sample_type;
>> - u16 size = 0;
>>
>> - if (!first->core.attr.sample_id_all)
>> - goto out;
>> -
>> - sample_type = first->core.attr.sample_type;
>> -
>> - if (sample_type & PERF_SAMPLE_TID)
>> - size += sizeof(data->tid) * 2;
>> -
>> - if (sample_type & PERF_SAMPLE_TIME)
>> - size += sizeof(data->time);
>> -
>> - if (sample_type & PERF_SAMPLE_ID)
>> - size += sizeof(data->id);
>> -
>> - if (sample_type & PERF_SAMPLE_STREAM_ID)
>> - size += sizeof(data->stream_id);
>> -
>> - if (sample_type & PERF_SAMPLE_CPU)
>> - size += sizeof(data->cpu) * 2;
>> -
>> - if (sample_type & PERF_SAMPLE_IDENTIFIER)
>> - size += sizeof(data->id);
>> -out:
>> - return size;
>> + return first->core.attr.sample_id_all ? evsel__id_hdr_size(first) : 0;
>> }
>>
>> bool evlist__valid_sample_id_all(struct evlist *evlist)
>> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
>> index a67cc3f2fa74..9a30ccb7b104 100644
>> --- a/tools/perf/util/evsel.c
>> +++ b/tools/perf/util/evsel.c
>> @@ -2724,6 +2724,32 @@ int evsel__parse_sample_timestamp(struct evsel *evsel, union perf_event *event,
>> return 0;
>> }
>>
>> +u16 evsel__id_hdr_size(struct evsel *evsel)
>> +{
>> + u64 sample_type = evsel->core.attr.sample_type;
>
> As this just uses core, would it be more appropriate to put it in libperf?
AFAIK we move to libperf only as needed.
>
>> + u16 size = 0;
>
> Perhaps size_t or int? u16 seems odd.
Event header size member is 16-bit
>
>> +
>> + if (sample_type & PERF_SAMPLE_TID)
>> + size += sizeof(u64);
>> +
>> + if (sample_type & PERF_SAMPLE_TIME)
>> + size += sizeof(u64);
>> +
>> + if (sample_type & PERF_SAMPLE_ID)
>> + size += sizeof(u64);
>> +
>> + if (sample_type & PERF_SAMPLE_STREAM_ID)
>> + size += sizeof(u64);
>> +
>> + if (sample_type & PERF_SAMPLE_CPU)
>> + size += sizeof(u64);
>> +
>> + if (sample_type & PERF_SAMPLE_IDENTIFIER)
>> + size += sizeof(u64);
>> +
>> + return size;
>> +}
>> +
>> struct tep_format_field *evsel__field(struct evsel *evsel, const char *name)
>> {
>> return tep_find_field(evsel->tp_format, name);
>> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
>> index 92bed8e2f7d8..699448f2bc2b 100644
>> --- a/tools/perf/util/evsel.h
>> +++ b/tools/perf/util/evsel.h
>> @@ -381,6 +381,8 @@ int evsel__parse_sample(struct evsel *evsel, union perf_event *event,
>> int evsel__parse_sample_timestamp(struct evsel *evsel, union perf_event *event,
>> u64 *timestamp);
>>
>> +u16 evsel__id_hdr_size(struct evsel *evsel);
>> +
>
> A comment would be nice, I know this is just moving code about but
> this is a new function.
>
> Thanks,
> Ian
>
>> static inline struct evsel *evsel__next(struct evsel *evsel)
>> {
>> return list_entry(evsel->core.node.next, struct evsel, core.node);
>> --
>> 2.25.1
>>
