* [PATCH v3 0/3] perf annotate: Support '--group' option @ 2018-05-18 16:00 Jin Yao 2018-05-18 16:00 ` [PATCH v3 1/3] perf evlist: Create a new function perf_evlist_forced_leader Jin Yao ` (2 more replies) 0 siblings, 3 replies; 6+ messages in thread From: Jin Yao @ 2018-05-18 16:00 UTC (permalink / raw) To: acme, jolsa, peterz, mingo, alexander.shishkin Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao For non-explicit group, perf report has already supported a option '--group' which can enable group output. This patch-set will support perf annotate with the same '--group'. For example, perf record -e cycles,branches ./div perf annotate main --stdio --group : Disassembly of section .text: : : 00000000004004b0 <main>: : main(): : : return i; : } : : int main(void) : { 0.00 0.00 : 4004b0: push %rbx : int i; : int flag; : volatile double x = 1212121212, y = 121212; : : s_randseed = time(0); 0.00 0.00 : 4004b1: xor %edi,%edi : srand(s_randseed); 0.00 0.00 : 4004b3: mov $0x77359400,%ebx : : return i; : } : v3: --------- Move the comment which was on setup_forced_leader() to the new function perf_evlist_forced_leader(). Impact ------ perf evlist: Create a new function perf_evlist_forced_leader perf report: Use perf_evlist_forced_leader to support '--group' v2: --------- Arnaldo points out that it should be done the way it is for perf report --group. v2 refers to this way and the patch is totally rewritten. Init post: ---------- Post the patch 'perf annotate: Support multiple events without group' Jin Yao (3): perf evlist: Create a new function perf_evlist_forced_leader perf report: Use perf_evlist_forced_leader to support '--group' perf annotate: Support '--group' option tools/perf/builtin-annotate.c | 7 +++++++ tools/perf/builtin-report.c | 13 ++----------- tools/perf/util/evlist.c | 15 +++++++++++++++ tools/perf/util/evlist.h | 3 +++ 4 files changed, 27 insertions(+), 11 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 1/3] perf evlist: Create a new function perf_evlist_forced_leader 2018-05-18 16:00 [PATCH v3 0/3] perf annotate: Support '--group' option Jin Yao @ 2018-05-18 16:00 ` Jin Yao 2018-05-18 14:18 ` Arnaldo Carvalho de Melo 2018-05-18 16:00 ` [PATCH v3 2/3] perf report: Use perf_evlist_forced_leader to support '--group' Jin Yao 2018-05-18 16:00 ` [PATCH v3 3/3] perf annotate: Support '--group' option Jin Yao 2 siblings, 1 reply; 6+ messages in thread From: Jin Yao @ 2018-05-18 16:00 UTC (permalink / raw) To: acme, jolsa, peterz, mingo, alexander.shishkin Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao For non-explicit group, perf report supports a option '--group' which can enable group output. We also need to support perf annotate with the same '--group'. Create a new function perf_evlist_forced_leader which contains common code to force setting the group leader. Signed-off-by: Jin Yao <yao.jin@linux.intel.com> --- tools/perf/util/evlist.c | 15 +++++++++++++++ tools/perf/util/evlist.h | 3 +++ 2 files changed, 18 insertions(+) diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index a59281d..6983f6a 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -1795,3 +1795,18 @@ bool perf_evlist__exclude_kernel(struct perf_evlist *evlist) return true; } + +/* + * Events in data file are not collect in groups, but we still want + * the group display. Set the artificial group and set the leader's + * forced_leader flag to notify the display code. + */ +void perf_evlist_forced_leader(struct perf_evlist *evlist) +{ + if (!evlist->nr_groups) { + struct perf_evsel *leader = perf_evlist__first(evlist); + + perf_evlist__set_leader(evlist); + leader->forced_leader = true; + } +} diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h index 6c41b2f..d77d514 100644 --- a/tools/perf/util/evlist.h +++ b/tools/perf/util/evlist.h @@ -309,4 +309,7 @@ struct perf_evsel *perf_evlist__event2evsel(struct perf_evlist *evlist, union perf_event *event); bool perf_evlist__exclude_kernel(struct perf_evlist *evlist); + +void perf_evlist_forced_leader(struct perf_evlist *evlist); + #endif /* __PERF_EVLIST_H */ -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/3] perf evlist: Create a new function perf_evlist_forced_leader 2018-05-18 16:00 ` [PATCH v3 1/3] perf evlist: Create a new function perf_evlist_forced_leader Jin Yao @ 2018-05-18 14:18 ` Arnaldo Carvalho de Melo 2018-05-18 21:30 ` Jin, Yao 0 siblings, 1 reply; 6+ messages in thread From: Arnaldo Carvalho de Melo @ 2018-05-18 14:18 UTC (permalink / raw) To: Jin Yao Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak, kan.liang, yao.jin Em Sat, May 19, 2018 at 12:00:32AM +0800, Jin Yao escreveu: > For non-explicit group, perf report supports a option '--group' > which can enable group output. We also need to support perf annotate > with the same '--group'. > > Create a new function perf_evlist_forced_leader which contains > common code to force setting the group leader. > > Signed-off-by: Jin Yao <yao.jin@linux.intel.com> > --- > tools/perf/util/evlist.c | 15 +++++++++++++++ > tools/perf/util/evlist.h | 3 +++ > 2 files changed, 18 insertions(+) > > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c > index a59281d..6983f6a 100644 > --- a/tools/perf/util/evlist.c > +++ b/tools/perf/util/evlist.c > @@ -1795,3 +1795,18 @@ bool perf_evlist__exclude_kernel(struct perf_evlist *evlist) > > return true; > } > + > +/* > + * Events in data file are not collect in groups, but we still want > + * the group display. Set the artificial group and set the leader's > + * forced_leader flag to notify the display code. > + */ > +void perf_evlist_forced_leader(struct perf_evlist *evlist) This should've been: void perf_evlist__forced_leader(struct perf_evlist *evlist) But then, by the verb conjugation, I thought that this would be a query, i.e. "is this a forced leader?", but then, it returning void didn't match that expectation, which leads me having to actually _read_ that code to understand what is that this function does, which I would, but just to check that it matches what it seems to be, now I'm trying to do it to figure it out what is it that this thing does :-\ I see, its a 'd' to many, this should be: void perf_evlist__force_leader(struct perf_evlist *evlist) Please confirm that this is what you intend by posting a v4, so far, just for this specific patch and any other in the series that uses this function, ok? - Arnaldo > +{ > + if (!evlist->nr_groups) { > + struct perf_evsel *leader = perf_evlist__first(evlist); > + > + perf_evlist__set_leader(evlist); > + leader->forced_leader = true; > + } > +} > diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h > index 6c41b2f..d77d514 100644 > --- a/tools/perf/util/evlist.h > +++ b/tools/perf/util/evlist.h > @@ -309,4 +309,7 @@ struct perf_evsel *perf_evlist__event2evsel(struct perf_evlist *evlist, > union perf_event *event); > > bool perf_evlist__exclude_kernel(struct perf_evlist *evlist); > + > +void perf_evlist_forced_leader(struct perf_evlist *evlist); > + > #endif /* __PERF_EVLIST_H */ > -- > 2.7.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/3] perf evlist: Create a new function perf_evlist_forced_leader 2018-05-18 14:18 ` Arnaldo Carvalho de Melo @ 2018-05-18 21:30 ` Jin, Yao 0 siblings, 0 replies; 6+ messages in thread From: Jin, Yao @ 2018-05-18 21:30 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak, kan.liang, yao.jin On 5/18/2018 10:18 PM, Arnaldo Carvalho de Melo wrote: > Em Sat, May 19, 2018 at 12:00:32AM +0800, Jin Yao escreveu: >> For non-explicit group, perf report supports a option '--group' >> which can enable group output. We also need to support perf annotate >> with the same '--group'. >> >> Create a new function perf_evlist_forced_leader which contains >> common code to force setting the group leader. >> >> Signed-off-by: Jin Yao <yao.jin@linux.intel.com> >> --- >> tools/perf/util/evlist.c | 15 +++++++++++++++ >> tools/perf/util/evlist.h | 3 +++ >> 2 files changed, 18 insertions(+) >> >> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c >> index a59281d..6983f6a 100644 >> --- a/tools/perf/util/evlist.c >> +++ b/tools/perf/util/evlist.c >> @@ -1795,3 +1795,18 @@ bool perf_evlist__exclude_kernel(struct perf_evlist *evlist) >> >> return true; >> } >> + >> +/* >> + * Events in data file are not collect in groups, but we still want >> + * the group display. Set the artificial group and set the leader's >> + * forced_leader flag to notify the display code. >> + */ >> +void perf_evlist_forced_leader(struct perf_evlist *evlist) > > This should've been: > > void perf_evlist__forced_leader(struct perf_evlist *evlist) > > But then, by the verb conjugation, I thought that this would be a query, > i.e. "is this a forced leader?", but then, it returning void didn't > match that expectation, which leads me having to actually _read_ that > code to understand what is that this function does, which I would, but > just to check that it matches what it seems to be, now I'm trying to do > it to figure it out what is it that this thing does :-\ > > > I see, its a 'd' to many, this should be: > > void perf_evlist__force_leader(struct perf_evlist *evlist) > > Please confirm that this is what you intend by posting a v4, so far, > just for this specific patch and any other in the series that uses this > function, ok? > > - Arnaldo > Yes, the function is not a query, it's just to setup the forced_leader. Maybe we can use the name "perf_evlist__setup_forced_leader". But it looks the name is long so the name "perf_evlist__force_leader" is also good. I will use the name "perf_evlist__force_leader" in v4. Thanks Jin Yao >> +{ >> + if (!evlist->nr_groups) { >> + struct perf_evsel *leader = perf_evlist__first(evlist); >> + >> + perf_evlist__set_leader(evlist); >> + leader->forced_leader = true; >> + } >> +} >> diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h >> index 6c41b2f..d77d514 100644 >> --- a/tools/perf/util/evlist.h >> +++ b/tools/perf/util/evlist.h >> @@ -309,4 +309,7 @@ struct perf_evsel *perf_evlist__event2evsel(struct perf_evlist *evlist, >> union perf_event *event); >> >> bool perf_evlist__exclude_kernel(struct perf_evlist *evlist); >> + >> +void perf_evlist_forced_leader(struct perf_evlist *evlist); >> + >> #endif /* __PERF_EVLIST_H */ >> -- >> 2.7.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 2/3] perf report: Use perf_evlist_forced_leader to support '--group' 2018-05-18 16:00 [PATCH v3 0/3] perf annotate: Support '--group' option Jin Yao 2018-05-18 16:00 ` [PATCH v3 1/3] perf evlist: Create a new function perf_evlist_forced_leader Jin Yao @ 2018-05-18 16:00 ` Jin Yao 2018-05-18 16:00 ` [PATCH v3 3/3] perf annotate: Support '--group' option Jin Yao 2 siblings, 0 replies; 6+ messages in thread From: Jin Yao @ 2018-05-18 16:00 UTC (permalink / raw) To: acme, jolsa, peterz, mingo, alexander.shishkin Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao Since we have created a new function perf_evlist_forced_leader, so now remove the old code and use perf_evlist_forced_leader instead. Signed-off-by: Jin Yao <yao.jin@linux.intel.com> --- tools/perf/builtin-report.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c index 4c931af..d41b208 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c @@ -194,20 +194,11 @@ static int hist_iter__branch_callback(struct hist_entry_iter *iter, return err; } -/* - * Events in data file are not collect in groups, but we still want - * the group display. Set the artificial group and set the leader's - * forced_leader flag to notify the display code. - */ static void setup_forced_leader(struct report *report, struct perf_evlist *evlist) { - if (report->group_set && !evlist->nr_groups) { - struct perf_evsel *leader = perf_evlist__first(evlist); - - perf_evlist__set_leader(evlist); - leader->forced_leader = true; - } + if (report->group_set) + perf_evlist_forced_leader(evlist); } static int process_feature_event(struct perf_tool *tool, -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 3/3] perf annotate: Support '--group' option 2018-05-18 16:00 [PATCH v3 0/3] perf annotate: Support '--group' option Jin Yao 2018-05-18 16:00 ` [PATCH v3 1/3] perf evlist: Create a new function perf_evlist_forced_leader Jin Yao 2018-05-18 16:00 ` [PATCH v3 2/3] perf report: Use perf_evlist_forced_leader to support '--group' Jin Yao @ 2018-05-18 16:00 ` Jin Yao 2 siblings, 0 replies; 6+ messages in thread From: Jin Yao @ 2018-05-18 16:00 UTC (permalink / raw) To: acme, jolsa, peterz, mingo, alexander.shishkin Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao With the '--group' option, even for non-explicit group, perf annotate will enable the group output. For example, perf record -e cycles,branches ./div perf annotate main --stdio --group : Disassembly of section .text: : : 00000000004004b0 <main>: : main(): : : return i; : } : : int main(void) : { 0.00 0.00 : 4004b0: push %rbx : int i; : int flag; : volatile double x = 1212121212, y = 121212; : : s_randseed = time(0); 0.00 0.00 : 4004b1: xor %edi,%edi : srand(s_randseed); 0.00 0.00 : 4004b3: mov $0x77359400,%ebx : : return i; : } : But if without --group, there is only one event reported. perf annotate main --stdio : Disassembly of section .text: : : 00000000004004b0 <main>: : main(): : : return i; : } : : int main(void) : { 0.00 : 4004b0: push %rbx : int i; : int flag; : volatile double x = 1212121212, y = 121212; : : s_randseed = time(0); 0.00 : 4004b1: xor %edi,%edi : srand(s_randseed); 0.00 : 4004b3: mov $0x77359400,%ebx : : return i; : } Signed-off-by: Jin Yao <yao.jin@linux.intel.com> --- tools/perf/builtin-annotate.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c index 6e5d9f7..5272d48 100644 --- a/tools/perf/builtin-annotate.c +++ b/tools/perf/builtin-annotate.c @@ -45,6 +45,7 @@ struct perf_annotate { bool print_line; bool skip_missing; bool has_br_stack; + bool group_set; const char *sym_hist_filter; const char *cpu_list; DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS); @@ -508,6 +509,9 @@ int cmd_annotate(int argc, const char **argv) "Don't shorten the displayed pathnames"), OPT_BOOLEAN(0, "skip-missing", &annotate.skip_missing, "Skip symbols that cannot be annotated"), + OPT_BOOLEAN_SET(0, "group", &symbol_conf.event_group, + &annotate.group_set, + "Show event group information together"), OPT_STRING('C', "cpu", &annotate.cpu_list, "cpu", "list of cpus to profile"), OPT_CALLBACK(0, "symfs", NULL, "directory", "Look for files with symbols relative to this directory", @@ -570,6 +574,9 @@ int cmd_annotate(int argc, const char **argv) annotate.has_br_stack = perf_header__has_feat(&annotate.session->header, HEADER_BRANCH_STACK); + if (annotate.group_set) + perf_evlist_forced_leader(annotate.session->evlist); + ret = symbol__annotation_init(); if (ret < 0) goto out_delete; -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-05-18 21:30 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-05-18 16:00 [PATCH v3 0/3] perf annotate: Support '--group' option Jin Yao 2018-05-18 16:00 ` [PATCH v3 1/3] perf evlist: Create a new function perf_evlist_forced_leader Jin Yao 2018-05-18 14:18 ` Arnaldo Carvalho de Melo 2018-05-18 21:30 ` Jin, Yao 2018-05-18 16:00 ` [PATCH v3 2/3] perf report: Use perf_evlist_forced_leader to support '--group' Jin Yao 2018-05-18 16:00 ` [PATCH v3 3/3] perf annotate: Support '--group' option Jin Yao
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).