| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 3 Sep 2015 08:42:13 +0800
From: pi3orama <pi3orama@....com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Wang Nan <wangnan0@...wei.com>,
"acme@...hat.com" <acme@...hat.com>,
"mingo@...nel.org" <mingo@...nel.org>,
"ast@...mgrid.com" <ast@...mgrid.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"lizefan@...wei.com" <lizefan@...wei.com>,
Brendan Gregg <brendan.d.gregg@...il.com>,
Daniel Borkmann <daniel@...earbox.net>,
David Ahern <dsahern@...il.com>, He Kuang <hekuang@...wei.com>,
Jiri Olsa <jolsa@...nel.org>, Kaixu Xia <xiakaixu@...wei.com>,
Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
Peter Zijlstra <a.p.zijlstra@...llo.nl>
Subject: Re: [PATCH 03/31] perf tools: Introduce dummy evsel
发自我的 iPhone
> 在 2015年9月3日，上午8:11，Namhyung Kim <namhyung@...nel.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 <wangnan0@...wei.com>
>> Cc: Arnaldo Carvalho de Melo <acme@...hat.com>
>> Cc: Alexei Starovoitov <ast@...mgrid.com>
>> Cc: Brendan Gregg <brendan.d.gregg@...il.com>
>> Cc: Daniel Borkmann <daniel@...earbox.net>
>> Cc: David Ahern <dsahern@...il.com>
>> Cc: He Kuang <hekuang@...wei.com>
>> Cc: Jiri Olsa <jolsa@...nel.org>
>> Cc: Kaixu Xia <xiakaixu@...wei.com>
>> Cc: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
>> Cc: Namhyung Kim <namhyung@...nel.org>
>> Cc: Peter Zijlstra <a.p.zijlstra@...llo.nl>
>> Cc: Zefan Li <lizefan@...wei.com>
>> Cc: pi3orama@....com
>> Link: http://lkml.kernel.org/r/1440742821-44548-4-git-send-email-wangnan0@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 majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists