From: Jiri Olsa <jolsa@redhat.com>
To: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>, Ian Rogers <irogers@google.com>,
lkml <linux-kernel@vger.kernel.org>,
Peter Zijlstra <a.p.zijlstra@chello.nl>,
Ingo Molnar <mingo@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Namhyung Kim <namhyung@kernel.org>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Michael Petlan <mpetlan@redhat.com>,
Stephane Eranian <eranian@google.com>
Subject: Re: [PATCH 7/9] perf tools: Add size to struct perf_record_header_build_id
Date: Wed, 14 Oct 2020 15:21:46 +0200 [thread overview]
Message-ID: <20201014132146.GB1382146@krava> (raw)
In-Reply-To: <20201014115908.GE3100363@kernel.org>
On Wed, Oct 14, 2020 at 08:59:08AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Oct 13, 2020 at 09:24:39PM +0200, Jiri Olsa escreveu:
> > We do not store size with build ids in perf data,
> > but there's enough space to do it. Adding misc bit
> > PERF_RECORD_MISC_BUILD_ID_SIZE to mark build id event
> > with size.
> >
> > With this fix the dso with md5 build id will have correct
> > build id data and will be usable for debuginfod processing
> > if needed (coming in following patches).
> >
> > Acked-by: Ian Rogers <irogers@google.com>
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> > tools/lib/perf/include/perf/event.h | 12 +++++++++++-
> > tools/perf/util/build-id.c | 8 +++++---
> > tools/perf/util/header.c | 10 +++++++---
> > 3 files changed, 23 insertions(+), 7 deletions(-)
> >
> > diff --git a/tools/lib/perf/include/perf/event.h b/tools/lib/perf/include/perf/event.h
> > index a6dbba6b9073..988c539bedb6 100644
> > --- a/tools/lib/perf/include/perf/event.h
> > +++ b/tools/lib/perf/include/perf/event.h
> > @@ -201,10 +201,20 @@ struct perf_record_header_tracing_data {
> > __u32 size;
> > };
> >
> > +#define PERF_RECORD_MISC_BUILD_ID_SIZE (1 << 15)
> > +
> > struct perf_record_header_build_id {
> > struct perf_event_header header;
> > pid_t pid;
> > - __u8 build_id[24];
> > + union {
> > + __u8 build_id[24];
> > + struct {
> > + __u8 data[20];
> > + __u8 size;
> > + __u8 reserved1__;
> > + __u16 reserved2__;
> > + };
> > + };
> > char filename[];
> > };
>
> Hey, shouldn't we just append the extra info at the end, i.e. keep it
> like:
>
> struct perf_record_header_build_id {
> struct perf_event_header header;
> pid_t pid;
> __u8 build_id[24];
> char filename[];
> __u8 size;
> };
>
>
> No need for PERF_RECORD_MISC_BUILD_ID_SIZE, older tools will continue
> working with new perf data files.
hum, then how would we tell if the last byte (size) is present or not?
>
> OTOH BUILD_ID_SIZE is 20 and the space on this header is 24, so the last
> 4 bytes were not being used, so older tools don't look into it, they
> should continue working, have you tested this case? I.e. getting the
> perf binary in, say, fedora and check that it works with this new
> perf_record_header_build_id layout?
yes, that still works (tested), because we copied only 20 bytes
of the build_id[24] and did not care about the rest
jirka
next prev parent reply other threads:[~2020-10-14 13:21 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-13 19:24 [PATCHv2 0/9] perf tools: Add support for build id with different sizes Jiri Olsa
2020-10-13 19:24 ` [PATCH 1/9] perf tools: Use build_id object in dso Jiri Olsa
2020-10-13 19:24 ` [PATCH 2/9] perf tools: Pass build_id object to filename__read_build_id Jiri Olsa
2020-10-13 19:24 ` [PATCH 3/9] perf tools: Pass build id object to sysfs__read_build_id Jiri Olsa
2020-10-13 19:24 ` [PATCH 4/9] perf tools: Pass build_id object to build_id__sprintf Jiri Olsa
2020-10-13 19:24 ` [PATCH 5/9] perf tools: Pass build_id object to dso__set_build_id Jiri Olsa
2020-10-14 11:51 ` Arnaldo Carvalho de Melo
2020-10-14 11:59 ` Jiri Olsa
2020-10-13 19:24 ` [PATCH 6/9] perf tools: Pass build_id object to dso__build_id_equal Jiri Olsa
2020-10-13 19:24 ` [PATCH 7/9] perf tools: Add size to struct perf_record_header_build_id Jiri Olsa
2020-10-14 11:59 ` Arnaldo Carvalho de Melo
2020-10-14 13:21 ` Jiri Olsa [this message]
2020-10-14 15:32 ` Arnaldo Carvalho de Melo
2020-10-14 14:24 ` Arnaldo Carvalho de Melo
2020-10-13 19:24 ` [PATCH 8/9] perf tools: Align buildid list output for short build ids Jiri Olsa
2020-10-13 19:24 ` [PATCH 9/9] perf tools: Add build id shell test Jiri Olsa
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=20201014132146.GB1382146@krava \
--to=jolsa@redhat.com \
--cc=a.p.zijlstra@chello.nl \
--cc=acme@kernel.org \
--cc=alexander.shishkin@linux.intel.com \
--cc=eranian@google.com \
--cc=irogers@google.com \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@kernel.org \
--cc=mpetlan@redhat.com \
--cc=namhyung@kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.