Re: [PATCH 04/18] perf header: Add HEADER_GROUP_DESC feature
On Thu, 29 Nov 2012 15:43:04 -0300, Arnaldo Carvalho de Melo wrote: > Em Thu, Nov 29, 2012 at 03:38:32PM +0900, Namhyung Kim escreveu: >> From: Namhyung Kim >> >> Save group relationship information so that it can be restored when >> perf report is running. >> [snip] >> +static int write_group_desc(int fd, struct perf_header *h __maybe_unused, >> +struct perf_evlist *evlist) >> +{ >> +u32 nr_groups = evlist->nr_groups; >> +struct perf_evsel *evsel; >> + >> +do_write(fd, &nr_groups, sizeof(nr_groups)); >> + >> +list_for_each_entry(evsel, &evlist->entries, node) { >> +if (perf_evsel__is_group_leader(evsel) && >> +evsel->nr_members > 1) { >> +const char *name = evsel->group_name ?: "{anon_group}"; >> +u32 leader_idx = evsel->idx; >> +u32 nr_members = evsel->nr_members; >> + >> +do_write_string(fd, name); >> +do_write(fd, &leader_idx, sizeof(leader_idx)); >> +do_write(fd, &nr_members, sizeof(nr_members)); > > You need to check do_write() return, it can fail. There are some cases > in header.c that don't check it, those should be eventually fixed, most > cases check it tho. Okay. > >> +} >> +} >> +return 0; >> +} [snip] >> +static int process_group_desc(struct perf_file_section *section >> __maybe_unused, >> + struct perf_header *ph, int fd, >> + void *data __maybe_unused) >> +{ >> +size_t ret = -1; >> +u32 i, nr, nr_groups; >> +struct perf_session *session; >> +struct perf_evsel *evsel, *leader = NULL; >> +struct group_desc { >> +char *name; >> +u32 leader_idx; >> +u32 nr_members; >> +} *desc; >> + >> +if (read(fd, &nr_groups, sizeof(nr_groups)) != sizeof(nr_groups)) > > Please use readn() instead in all read() cases in this function. Will do. Thanks, Namhyung -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 04/18] perf header: Add HEADER_GROUP_DESC feature
Em Thu, Nov 29, 2012 at 03:38:32PM +0900, Namhyung Kim escreveu: > From: Namhyung Kim > > Save group relationship information so that it can be restored when > perf report is running. > > Cc: Jiri Olsa > Cc: Stephane Eranian > Acked-by: Jiri Olsa > Signed-off-by: Namhyung Kim > --- > tools/perf/builtin-record.c | 3 + > tools/perf/util/header.c| 153 > > tools/perf/util/header.h| 2 + > 3 files changed, 158 insertions(+) > > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c > index f3151d3c70ce..d50f0dcfe1c1 100644 > --- a/tools/perf/builtin-record.c > +++ b/tools/perf/builtin-record.c > @@ -592,6 +592,9 @@ static int __cmd_record(struct perf_record *rec, int > argc, const char **argv) > goto out_delete_session; > } > > + if (!evsel_list->nr_groups) > + perf_header__clear_feat(&session->header, HEADER_GROUP_DESC); > + > /* >* perf_session__delete(session) will be called at perf_record__exit() >*/ > diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c > index 195a47a8f052..6ae8185842f2 100644 > --- a/tools/perf/util/header.c > +++ b/tools/perf/util/header.c > @@ -1073,6 +1073,41 @@ static int write_pmu_mappings(int fd, struct > perf_header *h __maybe_unused, > } > > /* > + * File format: > + * > + * struct group_descs { > + * u32 nr_groups; > + * struct group_desc { > + * charname[]; > + * u32 leader_idx; > + * u32 nr_members; > + * }[nr_groups]; > + * }; > + */ > +static int write_group_desc(int fd, struct perf_header *h __maybe_unused, > + struct perf_evlist *evlist) > +{ > + u32 nr_groups = evlist->nr_groups; > + struct perf_evsel *evsel; > + > + do_write(fd, &nr_groups, sizeof(nr_groups)); > + > + list_for_each_entry(evsel, &evlist->entries, node) { > + if (perf_evsel__is_group_leader(evsel) && > + evsel->nr_members > 1) { > + const char *name = evsel->group_name ?: "{anon_group}"; > + u32 leader_idx = evsel->idx; > + u32 nr_members = evsel->nr_members; > + > + do_write_string(fd, name); > + do_write(fd, &leader_idx, sizeof(leader_idx)); > + do_write(fd, &nr_members, sizeof(nr_members)); You need to check do_write() return, it can fail. There are some cases in header.c that don't check it, those should be eventually fixed, most cases check it tho. > + } > + } > + return 0; > +} > + > +/* > * default get_cpuid(): nothing gets recorded > * actual implementation must be in arch/$(ARCH)/util/header.c > */ > @@ -1433,6 +1468,31 @@ error: > fprintf(fp, "# pmu mappings: unable to read\n"); > } > > +static void print_group_desc(struct perf_header *ph, int fd __maybe_unused, > + FILE *fp) > +{ > + struct perf_session *session; > + struct perf_evsel *evsel; > + u32 nr = 0; > + > + session = container_of(ph, struct perf_session, header); > + > + list_for_each_entry(evsel, &session->evlist->entries, node) { > + if (perf_evsel__is_group_leader(evsel) && > + evsel->nr_members > 1) { > + fprintf(fp, "# group: %s{%s", evsel->group_name ?: "", > + perf_evsel__name(evsel)); > + > + nr = evsel->nr_members - 1; > + } else if (nr) { > + fprintf(fp, ",%s", perf_evsel__name(evsel)); > + > + if (--nr == 0) > + fprintf(fp, "}\n"); > + } > + } > +} > + > static int __event_process_build_id(struct build_id_event *bev, > char *filename, > struct perf_session *session) > @@ -1947,6 +2007,98 @@ error: > return -1; > } > > +static int process_group_desc(struct perf_file_section *section > __maybe_unused, > + struct perf_header *ph, int fd, > + void *data __maybe_unused) > +{ > + size_t ret = -1; > + u32 i, nr, nr_groups; > + struct perf_session *session; > + struct perf_evsel *evsel, *leader = NULL; > + struct group_desc { > + char *name; > + u32 leader_idx; > + u32 nr_members; > + } *desc; > + > + if (read(fd, &nr_groups, sizeof(nr_groups)) != sizeof(nr_groups)) Please use readn() instead in all read() cases in this function. > + return -1; > + > + if (ph->needs_swap) > + nr_groups = bswap_32(nr_groups); > + > + ph->env.nr_groups = nr_groups; > + if (!nr_groups) { > + pr_debug("group desc not available\n"); > + return 0; > + } > + > + desc = calloc(nr_groups, sizeof(*desc)); > + if (!desc) > +
[PATCH 04/18] perf header: Add HEADER_GROUP_DESC feature
From: Namhyung Kim Save group relationship information so that it can be restored when perf report is running. Cc: Jiri Olsa Cc: Stephane Eranian Acked-by: Jiri Olsa Signed-off-by: Namhyung Kim --- tools/perf/builtin-record.c | 3 + tools/perf/util/header.c| 153 tools/perf/util/header.h| 2 + 3 files changed, 158 insertions(+) diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c index f3151d3c70ce..d50f0dcfe1c1 100644 --- a/tools/perf/builtin-record.c +++ b/tools/perf/builtin-record.c @@ -592,6 +592,9 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv) goto out_delete_session; } + if (!evsel_list->nr_groups) + perf_header__clear_feat(&session->header, HEADER_GROUP_DESC); + /* * perf_session__delete(session) will be called at perf_record__exit() */ diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c index 195a47a8f052..6ae8185842f2 100644 --- a/tools/perf/util/header.c +++ b/tools/perf/util/header.c @@ -1073,6 +1073,41 @@ static int write_pmu_mappings(int fd, struct perf_header *h __maybe_unused, } /* + * File format: + * + * struct group_descs { + * u32 nr_groups; + * struct group_desc { + * charname[]; + * u32 leader_idx; + * u32 nr_members; + * }[nr_groups]; + * }; + */ +static int write_group_desc(int fd, struct perf_header *h __maybe_unused, + struct perf_evlist *evlist) +{ + u32 nr_groups = evlist->nr_groups; + struct perf_evsel *evsel; + + do_write(fd, &nr_groups, sizeof(nr_groups)); + + list_for_each_entry(evsel, &evlist->entries, node) { + if (perf_evsel__is_group_leader(evsel) && + evsel->nr_members > 1) { + const char *name = evsel->group_name ?: "{anon_group}"; + u32 leader_idx = evsel->idx; + u32 nr_members = evsel->nr_members; + + do_write_string(fd, name); + do_write(fd, &leader_idx, sizeof(leader_idx)); + do_write(fd, &nr_members, sizeof(nr_members)); + } + } + return 0; +} + +/* * default get_cpuid(): nothing gets recorded * actual implementation must be in arch/$(ARCH)/util/header.c */ @@ -1433,6 +1468,31 @@ error: fprintf(fp, "# pmu mappings: unable to read\n"); } +static void print_group_desc(struct perf_header *ph, int fd __maybe_unused, +FILE *fp) +{ + struct perf_session *session; + struct perf_evsel *evsel; + u32 nr = 0; + + session = container_of(ph, struct perf_session, header); + + list_for_each_entry(evsel, &session->evlist->entries, node) { + if (perf_evsel__is_group_leader(evsel) && + evsel->nr_members > 1) { + fprintf(fp, "# group: %s{%s", evsel->group_name ?: "", + perf_evsel__name(evsel)); + + nr = evsel->nr_members - 1; + } else if (nr) { + fprintf(fp, ",%s", perf_evsel__name(evsel)); + + if (--nr == 0) + fprintf(fp, "}\n"); + } + } +} + static int __event_process_build_id(struct build_id_event *bev, char *filename, struct perf_session *session) @@ -1947,6 +2007,98 @@ error: return -1; } +static int process_group_desc(struct perf_file_section *section __maybe_unused, + struct perf_header *ph, int fd, + void *data __maybe_unused) +{ + size_t ret = -1; + u32 i, nr, nr_groups; + struct perf_session *session; + struct perf_evsel *evsel, *leader = NULL; + struct group_desc { + char *name; + u32 leader_idx; + u32 nr_members; + } *desc; + + if (read(fd, &nr_groups, sizeof(nr_groups)) != sizeof(nr_groups)) + return -1; + + if (ph->needs_swap) + nr_groups = bswap_32(nr_groups); + + ph->env.nr_groups = nr_groups; + if (!nr_groups) { + pr_debug("group desc not available\n"); + return 0; + } + + desc = calloc(nr_groups, sizeof(*desc)); + if (!desc) + return -1; + + for (i = 0; i < nr_groups; i++) { + desc[i].name = do_read_string(fd, ph); + if (!desc[i].name) + goto out_free; + + if (read(fd, &desc[i].leader_idx, sizeof(u32)) != sizeof(u32)) + goto out_free; + + if (read(fd, &desc[i].nr_members, sizeof(u32)) != sizeof(u32)) + goto out_free; + +