Re: [PATCH] perf: fix perf.data endianness detection

[Stephane Eranian]

On Tue, Sep 20, 2011 at 8:59 PM, David Ahern <dsahern@xxxxxxxxx> wrote:
>
>
> On 09/19/2011 03:56 AM, Stephane Eranian wrote:
>>
>> The current version of perf detects whether or not
>> the perf.data file is written in a different endianness
>> using the attr_size field in the header of the file. This
>> field represents sizeof(struct perf_event) as known to perf
>> record. If the size does not match, then perf tries the
>> byte-swapped version. If they match, then the tool assumes
>> a different endianness.
>>
>> The issue with the approach is that it assumes the size of
>> perf_event_attr always has to match between the file and the
>> tool. However, the kernel API is designed to make it possible
>> to extend perf_event_attr for new features. Consequently, it
>> is not possible to use attr_size to detect endianness.
>>
>> This patch takes another approach by using the magic number
>> written at the beginning of the perf.data file to detect
>> endianness mismatch. The magic number is an eight-byte
>> signature. The patch introduces a new value for this
>> signature. The key difference is that the signature
>> is written differently in the file depending on the
>> endianness. Thus, by comparing the signature from the file
>> with the tool's own signature it is possible to detect if
>> the endianness matches. The new signature is "PERFILE2".
>>
>> Backward compatiblity with existing perf.data file is
>> ensured.
>>
>> Signed-off-by: Stephane Eranian <eranian@xxxxxxxxxx>
>> ---
>>
>> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
>> index b6c1ad1..d824502 100644
>> --- a/tools/perf/util/header.c
>> +++ b/tools/perf/util/header.c
>> @@ -57,9 +57,20 @@ char *perf_header__find_event(u64 id)
>> Â Â Â return NULL;
>> Â}
>>
>> -static const char *__perf_magic = "PERFFILE";
>> -
>> -#define PERF_MAGIC Â (*(u64 *)__perf_magic)
>> +/*
>> + * magic2 = "PERFILE2"
>> + * must be a numerical value to let the endianness
>> + * determine the memory layout. That way we are able
>> + * to detect endianness when reading the perf.data file
>> + * back.
>> + *
>> + * we check for legacy (PERFFILE) format.
>> + */
>> +static const char *__perf_magic1 = "PERFFILE";
>> +static const u64 __perf_magic2 Â Â= 0x32454c4946524550ULL;
>> +static const u64 __perf_magic2_sw = 0x50455246494c4532ULL;
>> +
>> +#define PERF_MAGIC Â (__perf_magic2)
>>
>> Âstruct perf_file_attr {
>> Â Â Â struct perf_event_attr Âattr;
>> @@ -595,25 +606,57 @@ out_free:
>> Â Â Â return err;
>> Â}
>>
>> +static int check_magic_endian(u64 *magic, struct perf_file_header *header,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â struct perf_header *ph)
>> +{
>> + Â Â int ret;
>> +
>> + Â Â /* check for legacy format */
>> + Â Â ret = memcmp(magic, __perf_magic1, sizeof(*magic));
>> + Â Â if (ret == 0) {
>> + Â Â Â Â Â Â pr_debug("legacy perf.data format\n");
>> + Â Â Â Â Â Â if (!header)
>> + Â Â Â Â Â Â Â Â Â Â return -1;
>> +
>> + Â Â Â Â Â Â if (header->attr_size != sizeof(struct perf_file_attr)) {
>> + Â Â Â Â Â Â Â Â Â Â u64 attr_size = bswap_64(header->attr_size);
>> +
>> + Â Â Â Â Â Â Â Â Â Â if (attr_size != sizeof(struct perf_file_attr))
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â return -1;
>> +
>> + Â Â Â Â Â Â Â Â Â Â mem_bswap_64(header, offsetof(struct perf_file_header,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â adds_features));
>> + Â Â Â Â Â Â Â Â Â Â ph->needs_swap = true;
>> + Â Â Â Â Â Â }
>> + Â Â Â Â Â Â return 0;
>> + Â Â }
>> +
>> + Â Â /* check for our magic (same endianness) */
>> + Â Â if (*magic == __perf_magic2)
>> + Â Â Â Â Â Â return 0;
>> +
>> + Â Â /* check for our magic (opposite endianness) */
>> + Â Â if (*magic != __perf_magic2_sw)
>> + Â Â Â Â Â Â return -1;
>> +
>> + Â Â ph->needs_swap = true;
>> +
>> + Â Â return 0;
>> +}
>> +
>> Âint perf_file_header__read(struct perf_file_header *header,
>> Â Â Â Â Â Â Â Â Â Â Â Â Âstruct perf_header *ph, int fd)
>> Â{
>> + Â Â int ret;
>> +
>> Â Â Â lseek(fd, 0, SEEK_SET);
>>
>> - Â Â if (readn(fd, header, sizeof(*header)) <= 0 ||
>> - Â Â Â Â memcmp(&header->magic, __perf_magic, sizeof(header->magic)))
>> + Â Â ret = readn(fd, header, sizeof(*header));
>> + Â Â if (ret <= 0)
>> Â Â Â Â Â Â Â return -1;
>>
>> - Â Â if (header->attr_size != sizeof(struct perf_file_attr)) {
>> - Â Â Â Â Â Â u64 attr_size = bswap_64(header->attr_size);
>> -
>> - Â Â Â Â Â Â if (attr_size != sizeof(struct perf_file_attr))
>> - Â Â Â Â Â Â Â Â Â Â return -1;
>> -
>> - Â Â Â Â Â Â mem_bswap_64(header, offsetof(struct perf_file_header,
>> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â adds_features));
>> - Â Â Â Â Â Â ph->needs_swap = true;
>> - Â Â }
>> + Â Â if (check_magic_endian(&header->magic, header, ph) < 0)
>> + Â Â Â Â Â Â return -1;
>>
>> Â Â Â if (header->size != sizeof(*header)) {
>> Â Â Â Â Â Â Â /* Support the previous format */
>> @@ -824,8 +867,13 @@ static int perf_file_header__read_pipe(struct perf_pipe_file_header *header,
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âstruct perf_header *ph, int fd,
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âbool repipe)
>> Â{
>> - Â Â if (readn(fd, header, sizeof(*header)) <= 0 ||
>> - Â Â Â Â memcmp(&header->magic, __perf_magic, sizeof(header->magic)))
>> + Â Â int ret;
>> +
>> + Â Â ret = readn(fd, header, sizeof(*header));
>> + Â Â if (ret <= 0)
>> + Â Â Â Â Â Â return -1;
>> +
>> + Â Â Âif (check_magic_endian(&header->magic, NULL, ph) < 0)
>> Â Â Â Â Â Â Â return -1;
>>
>> Â Â Â if (repipe && do_write(STDOUT_FILENO, header, sizeof(*header)) < 0)
>
>
> tip + this patch
>
> new PPC data file created and then the data file is analyzed on x86
> $ perf script -i /tmp/perf-new-ppc.data
> Âincompatible file format
>
> on x86:
> $ od -c /tmp/perf-new-ppc.data | head -1
> 0000000 Â 2 Â E Â L Â I Â F Â R Â E Â P Â\0 Â\0 Â\0 Â\0 Â\0 Â\0 Â\0 Â h
>
> So it is using the new magic.
>
> Same file on ppc:
> # od -c /tmp/perf-new-ppc.data | head -1
> 0000000 Â 2 Â E Â L Â I Â F Â R Â E Â P Â\0 Â\0 Â\0 Â\0 Â\0 Â\0 Â\0 Â h
>
Ok, not quite right. The signature in the file needs to be different
between PPC and x86. I think I know where this is coming from.
Let me try again....
Thanks for testing, I'll send an updated patch soon.
--
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/