Re: [PATCH 03/31] perf tools: Introduce dummy evsel

[pi3orama]

发自我的 iPhone

> 在 2015年9月3日，上午8:11，Namhyung Kim  写道：
> 
> Hi,
> 
>> On Sat, Aug 29, 2015 at 04:21:37AM +, 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 
>> Cc: Arnaldo Carvalho de Melo 
>> Cc: Alexei Starovoitov 
>> Cc: Brendan Gregg 
>> Cc: Daniel Borkmann 
>> Cc: David Ahern 
>> Cc: He Kuang 
>> Cc: Jiri Olsa 
>> Cc: Kaixu Xia 
>> Cc: Masami Hiramatsu 
>> Cc: Namhyung Kim 
>> Cc: Peter Zijlstra 
>> Cc: Zefan Li 
>> 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(>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, );
>> 
>>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
>> 

Re: [PATCH 03/31] perf tools: Introduce dummy evsel

[Namhyung Kim]

Hi,

On Sat, Aug 29, 2015 at 04:21:37AM +, 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.

> 
> 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 
> Cc: Arnaldo Carvalho de Melo 
> Cc: Alexei Starovoitov 
> Cc: Brendan Gregg 
> Cc: Daniel Borkmann 
> Cc: David Ahern 
> Cc: He Kuang 
> Cc: Jiri Olsa 
> Cc: Kaixu Xia 
> Cc: Masami Hiramatsu 
> Cc: Namhyung Kim 
> Cc: Peter Zijlstra 
> Cc: Zefan Li 
> 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(>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, );
>  
>   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;
>  }
> +
> 

Re: [PATCH 03/31] perf tools: Introduce dummy evsel

[pi3orama]

发自我的 iPhone

> 在 2015年9月3日，上午8:11，Namhyung Kim  写道：
> 
> Hi,
> 
>> On Sat, Aug 29, 2015 at 04:21:37AM +, 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 
>> Cc: Arnaldo Carvalho de Melo 
>> Cc: Alexei Starovoitov 
>> Cc: Brendan Gregg 
>> Cc: Daniel Borkmann 
>> Cc: David Ahern 
>> Cc: He Kuang 
>> Cc: Jiri Olsa 
>> Cc: Kaixu Xia 
>> Cc: Masami Hiramatsu 
>> Cc: Namhyung Kim 
>> Cc: Peter Zijlstra 
>> Cc: Zefan Li 
>> 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(>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, );
>> 
>>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, 

Re: [PATCH 03/31] perf tools: Introduce dummy evsel

[Namhyung Kim]

Hi,

On Sat, Aug 29, 2015 at 04:21:37AM +, 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.

> 
> 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 
> Cc: Arnaldo Carvalho de Melo 
> Cc: Alexei Starovoitov 
> Cc: Brendan Gregg 
> Cc: Daniel Borkmann 
> Cc: David Ahern 
> Cc: He Kuang 
> Cc: Jiri Olsa 
> Cc: Kaixu Xia 
> Cc: Masami Hiramatsu 
> Cc: Namhyung Kim 
> Cc: Peter Zijlstra 
> Cc: Zefan Li 
> 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(>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, );
>  
>   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 

Re: [PATCH 03/31] perf tools: Introduce dummy evsel

[Arnaldo Carvalho de Melo]

Em Sat, Aug 29, 2015 at 04:21:37AM +, Wang Nan escreveu:
> 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'.

Summary: this patch ended up adding too many subtle clever tricks to
achieve what it needs to accomplish, please try to clearly describe the
problem, then describe how you implemented it.
 
> Doesn't link other event selectors, if passing a BPF object file to
> '--event', nothing is linked onto evlist.

-ENOPARSE, can you rewrite the above sentence?

You mean you will segregate the events that related to eBPF to process
them in a separate step? Like, for instance, putting them in a separate
evlist or perhaps flipping a bit like: evsel->process_me_later = true
and then avoid those and process them at some later stage?

> 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.

Looks like a roundabout way of applying the --filter to the eBPF, but I
really need to read the patch then... See more below.
 
> 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.
 
> Signed-off-by: Wang Nan 
> Cc: Arnaldo Carvalho de Melo 
> Cc: Alexei Starovoitov 
> Cc: Brendan Gregg 
> Cc: Daniel Borkmann 
> Cc: David Ahern 
> Cc: He Kuang 
> Cc: Jiri Olsa 
> Cc: Kaixu Xia 
> Cc: Masami Hiramatsu 
> Cc: Namhyung Kim 
> Cc: Peter Zijlstra 
> Cc: Zefan Li 
> 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(>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, );
>  
>   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
> +++ 

Re: [PATCH 03/31] perf tools: Introduce dummy evsel

[Arnaldo Carvalho de Melo]

Em Sat, Aug 29, 2015 at 04:21:37AM +, Wang Nan escreveu:
> 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'.

Summary: this patch ended up adding too many subtle clever tricks to
achieve what it needs to accomplish, please try to clearly describe the
problem, then describe how you implemented it.
 
> Doesn't link other event selectors, if passing a BPF object file to
> '--event', nothing is linked onto evlist.

-ENOPARSE, can you rewrite the above sentence?

You mean you will segregate the events that related to eBPF to process
them in a separate step? Like, for instance, putting them in a separate
evlist or perhaps flipping a bit like: evsel->process_me_later = true
and then avoid those and process them at some later stage?

> 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.

Looks like a roundabout way of applying the --filter to the eBPF, but I
really need to read the patch then... See more below.
 
> 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.
 
> Signed-off-by: Wang Nan 
> Cc: Arnaldo Carvalho de Melo 
> Cc: Alexei Starovoitov 
> Cc: Brendan Gregg 
> Cc: Daniel Borkmann 
> Cc: David Ahern 
> Cc: He Kuang 
> Cc: Jiri Olsa 
> Cc: Kaixu Xia 
> Cc: Masami Hiramatsu 
> Cc: Namhyung Kim 
> Cc: Peter Zijlstra 
> Cc: Zefan Li 
> 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(>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, );
>  
>   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);
> + 

[PATCH 03/31] perf tools: Introduce dummy evsel

[Wang Nan]

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.

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.

Signed-off-by: Wang Nan 
Cc: Arnaldo Carvalho de Melo 
Cc: Alexei Starovoitov 
Cc: Brendan Gregg 
Cc: Daniel Borkmann 
Cc: David Ahern 
Cc: He Kuang 
Cc: Jiri Olsa 
Cc: Kaixu Xia 
Cc: Masami Hiramatsu 
Cc: Namhyung Kim 
Cc: Peter Zijlstra 
Cc: Zefan Li 
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(>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, );
 
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(>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 

[PATCH 03/31] perf tools: Introduce dummy evsel

[Wang Nan]

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.

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.

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)) {
+