Messages in this thread | | | Date | Tue, 6 Dec 2011 11:36:30 -0200 | From | Arnaldo Carvalho de Melo <> | Subject | Re: [PATCH 08/10] perf tool: Unify handling of features when writing feature section |
| |
Em Tue, Dec 06, 2011 at 11:32:38AM +0100, Robert Richter escreveu: > The features HEADER_TRACE_INFO and HEADER_BUILD_ID are handled > different when writing the feature section. All other features are > simply disabled on failure and writing the section goes on without > returning an error. There is no reason for these special cases. This > patch unifies handling of the features. > > This should be ok since all features can be parsed independently. > Offset and size of a feature's block is stored in struct perf_file_ > section right after the data block of perf.data (see perf_session__ > write_header()). Thus, if a feature does not exist then other features > can be processed anyway. > > Also moving special code for HEADER_BUILD_ID out to write_build_id().
For this one I just would add a big fat warning that if there are build-ids on the system but the table can't be written, then extreme care has to be taken when doing a perf report.
I.e. one has to be completely sure that the binaries hasn't changed if not validating the build-ids.
If you fix that please add this warning as well when no build-ids are found, which hopefully is the odd case these days as all distros I'm aware of have build-ids in all DSOs.
- Arnaldo
> Cc: Frederic Weisbecker <fweisbec@gmail.com> > Cc: Stephane Eranian <eranian@google.com> > Signed-off-by: Robert Richter <robert.richter@amd.com> > --- > tools/perf/util/header.c | 14 +++++--------- > 1 files changed, 5 insertions(+), 9 deletions(-) > > diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c > index dda3c5f..5cfe06a 100644 > --- a/tools/perf/util/header.c > +++ b/tools/perf/util/header.c > @@ -445,6 +445,9 @@ static int write_build_id(int fd, struct perf_header *h, > > session = container_of(h, struct perf_session, header); > > + if (!perf_session__read_build_ids(session, true)) > + return -1; > + > err = dsos__write_buildid_table(h, fd); > if (err < 0) { > pr_debug("failed to write buildid table\n"); > @@ -1413,10 +1416,6 @@ static int perf_header__adds_write(struct perf_header *header, > > session = container_of(header, struct perf_session, header); > > - if (perf_header__has_feat(header, HEADER_BUILD_ID && > - !perf_session__read_build_ids(session, true))) > - perf_header__clear_feat(header, HEADER_BUILD_ID); > - > nr_sections = bitmap_weight(header->adds_features, HEADER_FEAT_BITS); > if (!nr_sections) > return 0; > @@ -1432,13 +1431,11 @@ static int perf_header__adds_write(struct perf_header *header, > > err = do_write_feat(fd, header, HEADER_TRACE_INFO, &p, evlist); > if (err) > - goto out_free; > + perf_header__clear_feat(header, HEADER_TRACE_INFO); > > err = do_write_feat(fd, header, HEADER_BUILD_ID, &p, evlist); > - if (err) { > + if (err) > perf_header__clear_feat(header, HEADER_BUILD_ID); > - goto out_free; > - } > > err = do_write_feat(fd, header, HEADER_HOSTNAME, &p, evlist); > if (err) > @@ -1496,7 +1493,6 @@ static int perf_header__adds_write(struct perf_header *header, > err = do_write(fd, feat_sec, sec_size); > if (err < 0) > pr_debug("failed to write feature section\n"); > -out_free: > free(feat_sec); > return err; > } > -- > 1.7.7 >
| |