Re: [PATCH v6 16/18] perf: enable reading of perf.data files fromdifferent ABI rev

[Stephane Eranian]

On Thu, Feb 9, 2012 at 11:46 PM, David Ahern <dsahern@xxxxxxxxx> wrote:
>
>
> On 02/09/2012 03:41 PM, Stephane Eranian wrote:
>>>> + Â Â /* read perf_file_section, ids are read in caller */
>>>> + Â Â ret = readn(fd, &f_attr->ids, sizeof(f_attr->ids));
>>>
>>> I thought the above 2 lines were to be removed.
>>>
>>
>> No, They are needed to get the offsets of the ids[] table.
>> The field name are confusing. This is not reading the ids[],
>> but just the offset of where the table is. Without that you
>> get bogus offsets. I ran into that bug during testing...
>
> Hmm.... ok, so the perf_file_attr is now read in back to back reads --
> attr then ids -- versus today where it gets both in one readn.
>
>
Yes, you have to do that in case on file sizeof(perf_event_attr) <
tool sizeof(perf_event_attr). perf_event_attr is encapsulated into
perf_file_attr.

>>
>>
>>> David
>>>
>>>
>>>> +
>>>> + Â Â return ret <= 0 ? -1 : 0;
>>>> +}
>>>> +
>>>> Âint perf_session__read_header(struct perf_session *session, int fd)
>>>> Â{
>>>> Â Â Â struct perf_header *header = &session->header;
>>>> @@ -1974,19 +2019,17 @@ int perf_session__read_header(struct perf_session *session, int fd)
>>>> Â Â Â if (session->fd_pipe)
>>>> Â Â Â Â Â Â Â return perf_header__read_pipe(session, fd);
>>>>
>>>> - Â Â if (perf_file_header__read(&f_header, header, fd) < 0) {
>>>> - Â Â Â Â Â Â pr_debug("incompatible file format\n");
>>>> + Â Â if (perf_file_header__read(&f_header, header, fd) < 0)
>>>> Â Â Â Â Â Â Â return -EINVAL;
>>>> - Â Â }
>>>>
>>>> - Â Â nr_attrs = f_header.attrs.size / sizeof(f_attr);
>>>> + Â Â nr_attrs = f_header.attrs.size / f_header.attr_size;
>>>> Â Â Â lseek(fd, f_header.attrs.offset, SEEK_SET);
>>>>
>>>> Â Â Â for (i = 0; i < nr_attrs; i++) {
>>>> Â Â Â Â Â Â Â struct perf_evsel *evsel;
>>>> Â Â Â Â Â Â Â off_t tmp;
>>>>
>>>> - Â Â Â Â Â Â if (readn(fd, &f_attr, sizeof(f_attr)) <= 0)
>>>> + Â Â Â Â Â Â if (read_attr(fd, header, &f_attr) < 0)
>>>> Â Â Â Â Â Â Â Â Â Â Â goto out_errno;
>>>>
>>>> Â Â Â Â Â Â Â if (header->needs_swap)
>>>> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
>>>> index 3f3afed..5476320 100644
>>>> --- a/tools/perf/util/session.c
>>>> +++ b/tools/perf/util/session.c
>>>> @@ -24,7 +24,7 @@ static int perf_session__open(struct perf_session *self, bool force)
>>>> Â Â Â Â Â Â Â self->fd = STDIN_FILENO;
>>>>
>>>> Â Â Â Â Â Â Â if (perf_session__read_header(self, self->fd) < 0)
>>>> - Â Â Â Â Â Â Â Â Â Â pr_err("incompatible file format");
>>>> + Â Â Â Â Â Â Â Â Â Â pr_err("incompatible file format (rerun with -v to learn more)");
>>>>
>>>> Â Â Â Â Â Â Â return 0;
>>>> Â Â Â }
>>>> @@ -56,7 +56,7 @@ static int perf_session__open(struct perf_session *self, bool force)
>>>> Â Â Â }
>>>>
>>>> Â Â Â if (perf_session__read_header(self, self->fd) < 0) {
>>>> - Â Â Â Â Â Â pr_err("incompatible file format");
>>>> + Â Â Â Â Â Â pr_err("incompatible file format (rerun with -v to learn more)");
>>>> Â Â Â Â Â Â Â goto out_close;
>>>> Â Â Â }
>>>>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/