From: Stephane Eranian <eranian@google.com>
To: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: linux-kernel@vger.kernel.org, peterz@infradead.org,
mingo@elte.hu, robert.richter@amd.com, ming.m.lin@intel.com,
andi@firstfloor.org, asharma@fb.com, ravitillo@lbl.gov,
vweaver1@eecs.utk.edu, khandual@linux.vnet.ibm.com,
dsahern@gmail.com
Subject: Re: [PATCH v5 14/18] perf: fix endianness detection in perf.data
Date: Mon, 6 Feb 2012 19:18:29 +0100 [thread overview]
Message-ID: <CABPqkBRPJhiufo46GxRgudEAV=Ed+dTL62k+q28BagwDAxOBtw@mail.gmail.com> (raw)
In-Reply-To: <20120206181704.GF6367@infradead.org>
On Mon, Feb 6, 2012 at 7:17 PM, Arnaldo Carvalho de Melo
<acme@redhat.com> wrote:
> Em Thu, Feb 02, 2012 at 01:54:44PM +0100, Stephane Eranian escreveu:
>> 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_attr) as known
>> to perf record. If the sizes do 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 perf record and
>> perf report. However, the kernel perf_event ABI is extensible.
>> New fields can be added to struct perf_event_attr. 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. The magic number is an eight-byte signature.
>> It's primary purpose is to identify (signature) a perf.data
>> file. But it could also be used to encode the endianness.
>>
>> 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 endianness. The new signature is "PERFILE2".
>>
>> Backward compatiblity with existing perf.data file is
>> ensured.
>
> Looks ok, but IIRC David Ahern interacted with you on this specific
> patch in the past, having his Acked-by and/or Tested-by would be great,
> David?
>
I agree, I am still waiting for the results of his test on big-endian systems.
I don't have any unfortunately.
> - Arnaldo
>
>> Signed-off-by: Stephane Eranian <eranian@google.com>
>> ---
>> tools/perf/util/header.c | 77 ++++++++++++++++++++++++++++++++++++++--------
>> 1 files changed, 64 insertions(+), 13 deletions(-)
>>
>> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
>> index ecd7f4d..6f4187d 100644
>> --- a/tools/perf/util/header.c
>> +++ b/tools/perf/util/header.c
>> @@ -63,9 +63,20 @@ char *perf_header__find_event(u64 id)
>> return NULL;
>> }
>>
>> -static const char *__perf_magic = "PERFFILE";
>> +/*
>> + * 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 (*(u64 *)__perf_magic)
>> +#define PERF_MAGIC __perf_magic2
>>
>> struct perf_file_attr {
>> struct perf_event_attr attr;
>> @@ -1620,24 +1631,59 @@ int perf_header__process_sections(struct perf_header *header, int fd,
>> 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;
>> +
>> + ph->needs_swap = true;
>> + }
>> + return 0;
>> + }
>> +
>> + /* check magic number with same endianness */
>> + if (*magic == __perf_magic2)
>> + return 0;
>> +
>> + /* check magic number but 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;
>> + if (check_magic_endian(&header->magic, header, ph) < 0)
>> + return -1;
>>
>> + if (ph->needs_swap) {
>> mem_bswap_64(header, offsetof(struct perf_file_header,
>> - adds_features));
>> - ph->needs_swap = true;
>> + adds_features));
>> }
>>
>> if (header->size != sizeof(*header)) {
>> @@ -1873,8 +1919,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)
>> --
>> 1.7.4.1
next prev parent reply other threads:[~2012-02-06 18:18 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-02-02 12:54 [PATCH v5 00/18] perf: add support for sampling taken branches Stephane Eranian
2012-02-02 12:54 ` [PATCH v5 01/18] perf: add generic taken branch sampling support Stephane Eranian
2012-02-02 12:54 ` [PATCH v5 02/18] perf: add Intel LBR MSR definitions Stephane Eranian
2012-02-02 12:54 ` [PATCH v5 03/18] perf: add Intel X86 LBR sharing logic Stephane Eranian
2012-02-02 12:54 ` [PATCH v5 04/18] perf: sync branch stack sampling with X86 precise_sampling Stephane Eranian
2012-02-02 12:54 ` [PATCH v5 05/18] perf: add Intel X86 LBR mappings for PERF_SAMPLE_BRANCH filters Stephane Eranian
2012-02-02 12:54 ` [PATCH v5 06/18] perf: disable LBR support for older Intel Atom processors Stephane Eranian
2012-02-02 12:54 ` [PATCH v5 07/18] perf: implement PERF_SAMPLE_BRANCH for Intel X86 Stephane Eranian
2012-02-02 12:54 ` [PATCH v5 08/18] perf: add LBR software filter support " Stephane Eranian
2012-02-02 12:54 ` [PATCH v5 09/18] perf: disable PERF_SAMPLE_BRANCH_* when not supported Stephane Eranian
2012-02-06 19:23 ` Peter Zijlstra
2012-02-06 19:59 ` Stephane Eranian
2012-02-02 12:54 ` [PATCH v5 10/18] perf: add hook to flush branch_stack on context switch Stephane Eranian
2012-02-02 12:54 ` [PATCH v5 11/18] perf: add code to support PERF_SAMPLE_BRANCH_STACK Stephane Eranian
2012-02-06 18:06 ` Arnaldo Carvalho de Melo
2012-02-07 14:11 ` Stephane Eranian
2012-02-07 15:21 ` Arnaldo Carvalho de Melo
2012-02-02 12:54 ` [PATCH v5 12/18] perf: add support for sampling taken branch to perf record Stephane Eranian
2012-02-06 18:08 ` Arnaldo Carvalho de Melo
2012-02-02 12:54 ` [PATCH v5 13/18] perf: add support for taken branch sampling to perf report Stephane Eranian
2012-02-06 18:14 ` Arnaldo Carvalho de Melo
2012-02-02 12:54 ` [PATCH v5 14/18] perf: fix endianness detection in perf.data Stephane Eranian
2012-02-06 18:17 ` Arnaldo Carvalho de Melo
2012-02-06 18:18 ` Stephane Eranian [this message]
2012-02-06 21:47 ` David Ahern
2012-02-06 22:06 ` Arnaldo Carvalho de Melo
2012-02-06 22:29 ` David Ahern
2012-02-07 14:13 ` Stephane Eranian
2012-02-07 14:38 ` Arnaldo Carvalho de Melo
2012-02-17 9:42 ` [tip:perf/core] perf tools: " tip-bot for Stephane Eranian
2012-02-02 12:54 ` [PATCH v5 15/18] perf: add ABI reference sizes Stephane Eranian
2012-02-02 12:54 ` [PATCH v5 16/18] perf: enable reading of perf.data files from different ABI rev Stephane Eranian
2012-02-06 18:19 ` Arnaldo Carvalho de Melo
2012-02-06 18:22 ` Arnaldo Carvalho de Melo
2012-02-07 7:03 ` Anshuman Khandual
2012-02-07 14:52 ` Arnaldo Carvalho de Melo
2012-02-06 22:19 ` David Ahern
2012-02-07 15:50 ` Stephane Eranian
2012-02-07 16:41 ` David Ahern
2012-02-07 17:42 ` Stephane Eranian
2012-02-07 17:57 ` David Ahern
2012-02-02 12:54 ` [PATCH v5 17/18] perf: fix bug print_event_desc() Stephane Eranian
2012-02-02 12:54 ` [PATCH v5 18/18] perf: make perf able to read file from older ABIs Stephane Eranian
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CABPqkBRPJhiufo46GxRgudEAV=Ed+dTL62k+q28BagwDAxOBtw@mail.gmail.com' \
--to=eranian@google.com \
--cc=acme@redhat.com \
--cc=andi@firstfloor.org \
--cc=asharma@fb.com \
--cc=dsahern@gmail.com \
--cc=khandual@linux.vnet.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=ming.m.lin@intel.com \
--cc=mingo@elte.hu \
--cc=peterz@infradead.org \
--cc=ravitillo@lbl.gov \
--cc=robert.richter@amd.com \
--cc=vweaver1@eecs.utk.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).