发自我的 iPhone
> 在 2015年9月3日,上午8:11,Namhyung Kim <namhy...@kernel.org> 写道: > > Hi, > >> On Sat, Aug 29, 2015 at 04:21:37AM +0000, Wang Nan wrote: >> This patch allows linking dummy evsel onto evlist as a placeholder. It >> is for following patch which allows passing BPF object using '--event >> object.o'. >> >> Doesn't link other event selectors, if passing a BPF object file to >> '--event', nothing is linked onto evlist. Instead, events described in >> BPF object file are probed and linked in a delayed manner because we >> want do all probing work together. Therefore, evsel for events in BPF >> object would be linked at the end of evlist. Which causes a small >> problem that, if passing '--filter' setting after object file, the >> filter option won't be correctly applied to those events. >> >> This patch links dummy onto evlist, so following --filter can be >> collected by the dummy evsel. For this reason dummy evsels are set to >> PERF_TYPE_TRACEPOINT. > > I understand the need of the dummy event. But we already have dummy > event so it's confusing to have similar event IMHO. So what about > using existing dummy event instead? You can save a link to a bpf > object in the dummy evsel (to check it later) and change to allow > setting filter on dummy events IMHO. > Yes, in my working-in-progress implement I use existing dummy event. Connect it to the object by setting its name to the name of object. Thank you. >> >> Due to the possibility of existance of dummy evsel, >> perf_evlist__purge_dummy() must be called right after parse_options(). >> This patch adds it to record, top, trace and stat builtin commands. >> Further patch moves it down after real BPF events are processed with. > > IMHO it'd be better to do this kind of job in a single place - > e.g. perf_evlist__config() ? - so that other commands get benefits > from it easily. > > Thanks, > Namhyung > > >> >> Signed-off-by: Wang Nan <wangn...@huawei.com> >> Cc: Arnaldo Carvalho de Melo <a...@redhat.com> >> Cc: Alexei Starovoitov <a...@plumgrid.com> >> Cc: Brendan Gregg <brendan.d.gr...@gmail.com> >> Cc: Daniel Borkmann <dan...@iogearbox.net> >> Cc: David Ahern <dsah...@gmail.com> >> Cc: He Kuang <heku...@huawei.com> >> Cc: Jiri Olsa <jo...@kernel.org> >> Cc: Kaixu Xia <xiaka...@huawei.com> >> Cc: Masami Hiramatsu <masami.hiramatsu...@hitachi.com> >> Cc: Namhyung Kim <namhy...@kernel.org> >> Cc: Peter Zijlstra <a.p.zijls...@chello.nl> >> Cc: Zefan Li <lize...@huawei.com> >> Cc: pi3or...@163.com >> Link: >> http://lkml.kernel.org/r/1440742821-44548-4-git-send-email-wangn...@huawei.com >> --- >> tools/perf/builtin-record.c | 2 ++ >> tools/perf/builtin-stat.c | 1 + >> tools/perf/builtin-top.c | 1 + >> tools/perf/builtin-trace.c | 1 + >> tools/perf/util/evlist.c | 19 +++++++++++++++++++ >> tools/perf/util/evlist.h | 1 + >> tools/perf/util/evsel.c | 32 ++++++++++++++++++++++++++++++++ >> tools/perf/util/evsel.h | 6 ++++++ >> tools/perf/util/parse-events.c | 25 +++++++++++++++++++++---- >> 9 files changed, 84 insertions(+), 4 deletions(-) >> >> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c >> index a660022..81829de 100644 >> --- a/tools/perf/builtin-record.c >> +++ b/tools/perf/builtin-record.c >> @@ -1112,6 +1112,8 @@ int cmd_record(int argc, const char **argv, const char >> *prefix __maybe_unused) >> >> argc = parse_options(argc, argv, record_options, record_usage, >> PARSE_OPT_STOP_AT_NON_OPTION); >> + perf_evlist__purge_dummy(rec->evlist); >> + >> if (!argc && target__none(&rec->opts.target)) >> usage_with_options(record_usage, record_options); >> >> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c >> index 7aa039b..99b62f1 100644 >> --- a/tools/perf/builtin-stat.c >> +++ b/tools/perf/builtin-stat.c >> @@ -1208,6 +1208,7 @@ int cmd_stat(int argc, const char **argv, const char >> *prefix __maybe_unused) >> >> argc = parse_options(argc, argv, options, stat_usage, >> PARSE_OPT_STOP_AT_NON_OPTION); >> + perf_evlist__purge_dummy(evsel_list); >> >> interval = stat_config.interval; >> >> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c >> index 8c465c8..246203b 100644 >> --- a/tools/perf/builtin-top.c >> +++ b/tools/perf/builtin-top.c >> @@ -1198,6 +1198,7 @@ int cmd_top(int argc, const char **argv, const char >> *prefix __maybe_unused) >> perf_config(perf_top_config, &top); >> >> argc = parse_options(argc, argv, options, top_usage, 0); >> + perf_evlist__purge_dummy(top.evlist); >> if (argc) >> usage_with_options(top_usage, options); >> >> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c >> index 4e3abba..57712b9 100644 >> --- a/tools/perf/builtin-trace.c >> +++ b/tools/perf/builtin-trace.c >> @@ -3099,6 +3099,7 @@ int cmd_trace(int argc, const char **argv, const char >> *prefix __maybe_unused) >> >> argc = parse_options_subcommand(argc, argv, trace_options, >> trace_subcommands, >> trace_usage, PARSE_OPT_STOP_AT_NON_OPTION); >> + perf_evlist__purge_dummy(trace.evlist); >> >> if (trace.trace_pgfaults) { >> trace.opts.sample_address = true; >> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c >> index 8d00039..8a4e64d 100644 >> --- a/tools/perf/util/evlist.c >> +++ b/tools/perf/util/evlist.c >> @@ -1696,3 +1696,22 @@ void perf_evlist__set_tracking_event(struct >> perf_evlist *evlist, >> >> tracking_evsel->tracking = true; >> } >> + >> +void perf_evlist__purge_dummy(struct perf_evlist *evlist) >> +{ >> + struct perf_evsel *pos, *n; >> + >> + /* >> + * Remove all dummy events. >> + * During linking, we don't touch anything except link >> + * it into evlist. As a result, we don't >> + * need to adjust evlist->nr_entries during removal. >> + */ >> + >> + evlist__for_each_safe(evlist, n, pos) { >> + if (perf_evsel__is_dummy(pos)) { >> + list_del_init(&pos->node); >> + perf_evsel__delete(pos); >> + } >> + } >> +} >> diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h >> index b39a619..7f15727 100644 >> --- a/tools/perf/util/evlist.h >> +++ b/tools/perf/util/evlist.h >> @@ -181,6 +181,7 @@ bool perf_evlist__valid_read_format(struct perf_evlist >> *evlist); >> void perf_evlist__splice_list_tail(struct perf_evlist *evlist, >> struct list_head *list, >> int nr_entries); >> +void perf_evlist__purge_dummy(struct perf_evlist *evlist); >> >> static inline struct perf_evsel *perf_evlist__first(struct perf_evlist >> *evlist) >> { >> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c >> index bac25f4..01267f4 100644 >> --- a/tools/perf/util/evsel.c >> +++ b/tools/perf/util/evsel.c >> @@ -213,6 +213,7 @@ void perf_evsel__init(struct perf_evsel *evsel, >> evsel->sample_size = __perf_evsel__sample_size(attr->sample_type); >> perf_evsel__calc_id_pos(evsel); >> evsel->cmdline_group_boundary = false; >> + evsel->is_dummy = false; >> } >> >> struct perf_evsel *perf_evsel__new_idx(struct perf_event_attr *attr, int idx) >> @@ -225,6 +226,37 @@ struct perf_evsel *perf_evsel__new_idx(struct >> perf_event_attr *attr, int idx) >> return evsel; >> } >> >> +struct perf_evsel *perf_evsel__new_dummy(const char *name) >> +{ >> + struct perf_evsel *evsel = zalloc(perf_evsel__object.size); >> + >> + if (!evsel) >> + return NULL; >> + >> + /* >> + * Don't need call perf_evsel__init() for dummy evsel. >> + * Keep it simple. >> + */ >> + evsel->name = strdup(name); >> + if (!evsel->name) >> + goto out_free; >> + >> + INIT_LIST_HEAD(&evsel->node); >> + INIT_LIST_HEAD(&evsel->config_terms); >> + >> + evsel->cmdline_group_boundary = false; >> + /* >> + * Set dummy evsel as TRACEPOINT event so it can collect filter >> + * options. >> + */ >> + evsel->attr.type = PERF_TYPE_TRACEPOINT; >> + evsel->is_dummy = true; >> + return evsel; >> +out_free: >> + free(evsel); >> + return NULL; >> +} >> + >> struct perf_evsel *perf_evsel__newtp_idx(const char *sys, const char *name, >> int idx) >> { >> struct perf_evsel *evsel = zalloc(perf_evsel__object.size); >> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h >> index 298e6bb..0b8e47d 100644 >> --- a/tools/perf/util/evsel.h >> +++ b/tools/perf/util/evsel.h >> @@ -118,6 +118,7 @@ struct perf_evsel { >> struct perf_evsel *leader; >> char *group_name; >> bool cmdline_group_boundary; >> + bool is_dummy; >> struct list_head config_terms; >> }; >> >> @@ -153,6 +154,11 @@ int perf_evsel__object_config(size_t object_size, >> void (*fini)(struct perf_evsel *evsel)); >> >> struct perf_evsel *perf_evsel__new_idx(struct perf_event_attr *attr, int >> idx); >> +struct perf_evsel *perf_evsel__new_dummy(const char *name); >> +static inline bool perf_evsel__is_dummy(struct perf_evsel *evsel) >> +{ >> + return evsel->is_dummy; >> +} >> >> static inline struct perf_evsel *perf_evsel__new(struct perf_event_attr >> *attr) >> { >> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c >> index 14cd7e3..71d91fb 100644 >> --- a/tools/perf/util/parse-events.c >> +++ b/tools/perf/util/parse-events.c >> @@ -1141,7 +1141,7 @@ int parse_events(struct perf_evlist *evlist, const >> char *str, >> perf_pmu__parse_cleanup(); >> if (!ret) { >> int entries = data.idx - evlist->nr_entries; >> - struct perf_evsel *last; >> + struct perf_evsel *last = NULL; >> >> if (!list_empty(&data.list)) { >> last = list_entry(data.list.prev, >> @@ -1149,8 +1149,25 @@ int parse_events(struct perf_evlist *evlist, const >> char *str, >> last->cmdline_group_boundary = true; >> } >> >> - perf_evlist__splice_list_tail(evlist, &data.list, entries); >> - evlist->nr_groups += data.nr_groups; >> + if (last && perf_evsel__is_dummy(last)) { >> + if (!list_is_singular(&data.list)) { >> + parse_events_evlist_error(&data, 0, >> + "Dummy evsel error: not on a singular list"); >> + return -1; >> + } >> + /* >> + * We are introducing a dummy event. Don't touch >> + * anything, just link it. >> + * >> + * Don't use perf_evlist__splice_list_tail() since >> + * it alerts evlist->nr_entries, which affect header >> + * of resulting perf.data. >> + */ >> + list_splice_tail(&data.list, &evlist->entries); >> + } else { >> + perf_evlist__splice_list_tail(evlist, &data.list, entries); >> + evlist->nr_groups += data.nr_groups; >> + } >> >> return 0; >> } >> @@ -1256,7 +1273,7 @@ foreach_evsel_in_last_glob(struct perf_evlist *evlist, >> struct perf_evsel *last = NULL; >> int err; >> >> - if (evlist->nr_entries > 0) >> + if (!list_empty(&evlist->entries)) >> last = perf_evlist__last(evlist); >> >> do { >> -- >> 2.1.0 >> -- 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/