From: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
To: Namhyung Kim <namhyung.kim@lge.com>
Cc: Namhyung Kim <namhyung@gmail.com>,
Peter Zijlstra <a.p.zijlstra@chello.nl>,
Paul Mackerras <paulus@samba.org>, Ingo Molnar <mingo@elte.hu>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 08/11] perf tools: Consolidate target task/cpu checking
Date: Mon, 13 Feb 2012 16:39:38 -0200 [thread overview]
Message-ID: <20120213183937.GH15955@infradead.org> (raw)
In-Reply-To: <1329118064-9412-9-git-send-email-namhyung.kim@lge.com>
Em Mon, Feb 13, 2012 at 04:27:40PM +0900, Namhyung Kim escreveu:
> There are places that check whether target task/cpu is given or not
> and some of them didn't check newly introduced uid or cpu list. Add
> and use three of helper functions to treat them properly.
Following the changes I suggested these:
bool no_target_task(struct perf_maps_opts *maps);
bool no_target_cpu(struct perf_maps_opts *maps);
bool no_target_maps(struct perf_maps_opts *maps);
become
bool perf_target__no_task(struct perf_target *target);
bool perf_target__no_cpu(struct perf_target *target);
bool perf_target__empty(struct perf_target *target);
- Arnaldo
> Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
> ---
> tools/perf/builtin-record.c | 5 +----
> tools/perf/builtin-stat.c | 11 +++++------
> tools/perf/util/evlist.c | 7 +++----
> tools/perf/util/evsel.c | 3 +--
> tools/perf/util/usage.c | 16 ++++++++++++++++
> tools/perf/util/util.h | 3 +++
> 6 files changed, 29 insertions(+), 16 deletions(-)
>
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index a8abee03a62d..fbf71edd31ba 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -740,10 +740,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
>
> argc = parse_options(argc, argv, record_options, record_usage,
> PARSE_OPT_STOP_AT_NON_OPTION);
> - if (!argc && rec->opts.maps.target_pid == -1 &&
> - rec->opts.maps.target_tid == -1 &&
> - !rec->opts.maps.system_wide && !rec->opts.maps.cpu_list &&
> - !rec->opts.maps.uid_str)
> + if (!argc && no_target_maps(&rec->opts.maps))
> usage_with_options(record_usage, record_options);
>
> if (rec->force && rec->append_file) {
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index f1f4e8f60a3e..d080254a2d0c 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -296,7 +296,7 @@ static int create_perf_stat_counter(struct perf_evsel *evsel,
> if (maps.system_wide)
> return perf_evsel__open_per_cpu(evsel, evsel_list->cpus,
> group, group_fd);
> - if (maps.target_pid == -1 && maps.target_tid == -1) {
> + if (no_target_task(&maps)) {
> attr->disabled = 1;
> attr->enable_on_exec = 1;
> }
> @@ -446,8 +446,7 @@ static int run_perf_stat(int argc __used, const char **argv)
> exit(-1);
> }
>
> - if (maps.target_tid == -1 && maps.target_pid == -1 &&
> - !maps.system_wide)
> + if (no_target_maps(&maps))
> evsel_list->threads->map[0] = child_pid;
>
> /*
> @@ -969,7 +968,7 @@ static void print_stat(int argc, const char **argv)
> if (!csv_output) {
> fprintf(output, "\n");
> fprintf(output, " Performance counter stats for ");
> - if(maps.target_pid == -1 && maps.target_tid == -1) {
> + if (no_target_task(&maps)) {
> fprintf(output, "\'%s", argv[0]);
> for (i = 1; i < argc; i++)
> fprintf(output, " %s", argv[i]);
> @@ -1191,13 +1190,13 @@ int cmd_stat(int argc, const char **argv, const char *prefix __used)
> } else if (big_num_opt == 0) /* User passed --no-big-num */
> big_num = false;
>
> - if (!argc && maps.target_pid == -1 && maps.target_tid == -1)
> + if (!argc && no_target_task(&maps))
> usage_with_options(stat_usage, options);
> if (run_count <= 0)
> usage_with_options(stat_usage, options);
>
> /* no_aggr, cgroup are for system-wide only */
> - if ((no_aggr || nr_cgroups) && !maps.system_wide) {
> + if ((no_aggr || nr_cgroups) && !no_target_cpu(&maps)) {
> fprintf(stderr, "both cgroup and no-aggregation "
> "modes only available in system-wide mode\n");
>
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index b444f311897c..aa0418034d82 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -602,9 +602,9 @@ int perf_evlist__create_maps(struct perf_evlist *evlist,
> if (evlist->threads == NULL)
> return -1;
>
> - if (maps->uid != UINT_MAX || maps->target_tid != -1)
> + if (!no_target_task(maps))
> evlist->cpus = cpu_map__dummy_new();
> - else if (!maps->system_wide && maps->cpu_list == NULL)
> + else if (no_target_cpu(maps))
> evlist->cpus = cpu_map__dummy_new();
> else
> evlist->cpus = cpu_map__new(maps->cpu_list);
> @@ -823,8 +823,7 @@ int perf_evlist__prepare_workload(struct perf_evlist *evlist,
> exit(-1);
> }
>
> - if (!opts->maps.system_wide && opts->maps.target_tid == -1 &&
> - opts->maps.target_pid == -1)
> + if (no_target_maps(&opts->maps))
> evlist->threads->map[0] = evlist->workload.pid;
>
> close(child_ready_pipe[1]);
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 6a4e97d5f6d5..e7ddf27f240b 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -130,8 +130,7 @@ void perf_evsel__config(struct perf_evsel *evsel, struct perf_record_opts *opts)
> attr->mmap = track;
> attr->comm = track;
>
> - if (opts->maps.target_pid == -1 && opts->maps.target_tid == -1 &&
> - !opts->maps.system_wide) {
> + if (no_target_maps(&opts->maps)) {
> attr->disabled = 1;
> attr->enable_on_exec = 1;
> }
> diff --git a/tools/perf/util/usage.c b/tools/perf/util/usage.c
> index 08423c037a7e..d7e77fde966d 100644
> --- a/tools/perf/util/usage.c
> +++ b/tools/perf/util/usage.c
> @@ -147,3 +147,19 @@ void check_target_maps(struct perf_maps_opts *maps)
> maps->system_wide = false;
> }
> }
> +
> +bool no_target_task(struct perf_maps_opts *maps)
> +{
> + return maps->target_pid == -1 && maps->target_tid == -1 &&
> + maps->uid_str == NULL;
> +}
> +
> +bool no_target_cpu(struct perf_maps_opts *maps)
> +{
> + return !maps->system_wide && maps->cpu_list == NULL;
> +}
> +
> +bool no_target_maps(struct perf_maps_opts *maps)
> +{
> + return no_target_task(maps) && no_target_cpu(maps);
> +}
> diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
> index 2573985f8408..e1343eea14eb 100644
> --- a/tools/perf/util/util.h
> +++ b/tools/perf/util/util.h
> @@ -248,6 +248,9 @@ void event_attr_init(struct perf_event_attr *attr);
>
> uid_t parse_target_uid(const char *str);
> void check_target_maps(struct perf_maps_opts *maps);
> +bool no_target_task(struct perf_maps_opts *maps);
> +bool no_target_cpu(struct perf_maps_opts *maps);
> +bool no_target_maps(struct perf_maps_opts *maps);
>
> #define _STR(x) #x
> #define STR(x) _STR(x)
> --
> 1.7.9
next prev parent reply other threads:[~2012-02-13 18:39 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <1329118064-9412-1-git-send-email-namhyung.kim@lge.com>
2012-02-13 7:27 ` [PATCH 01/11] perf tools: Introduce struct perf_maps_opts Namhyung Kim
2012-02-13 7:44 ` [RFC PATCHSET] perf: Fix cpu/thread map and group event handling Namhyung Kim
2012-02-13 18:32 ` [PATCH 01/11] perf tools: Introduce struct perf_maps_opts Arnaldo Carvalho de Melo
2012-02-13 18:50 ` David Ahern
2012-02-13 19:05 ` Arnaldo Carvalho de Melo
2012-02-13 19:19 ` David Ahern
2012-02-13 20:12 ` Arnaldo Carvalho de Melo
2012-02-13 7:27 ` [PATCH 02/11] perf stat: Convert to perf_maps_opts Namhyung Kim
2012-02-13 18:33 ` Arnaldo Carvalho de Melo
2012-02-13 7:27 ` [PATCH 03/11] perf top: " Namhyung Kim
2012-02-13 7:27 ` [PATCH 04/11] perf tools: Introduce check_target_maps() helper Namhyung Kim
2012-02-13 18:36 ` Arnaldo Carvalho de Melo
2012-02-13 7:27 ` [PATCH 05/11] perf tools: Make perf_evlist__create_maps() take struct perf_maps_opts Namhyung Kim
2012-02-13 18:36 ` Arnaldo Carvalho de Melo
2012-02-13 7:27 ` [PATCH 06/11] perf tools: Check more combinations of PID/TID, UID and CPU switches Namhyung Kim
2012-02-13 7:27 ` [PATCH 07/11] perf tools: Fix creation of cpu map Namhyung Kim
2012-02-13 7:27 ` [PATCH 08/11] perf tools: Consolidate target task/cpu checking Namhyung Kim
2012-02-13 18:39 ` Arnaldo Carvalho de Melo [this message]
2012-02-13 7:27 ` [PATCH 09/11] perf stat: Use perf_evlist__create_maps Namhyung Kim
2012-02-13 18:40 ` Arnaldo Carvalho de Melo
2012-02-13 7:27 ` [PATCH 10/11] perf stat: Fix event grouping on forked task Namhyung Kim
2012-02-13 18:41 ` Arnaldo Carvalho de Melo
2012-02-14 1:20 ` [PATCH] " Namhyung Kim
2012-02-13 7:27 ` [PATCH 11/11] perf record: " Namhyung Kim
2012-02-13 18:42 ` Arnaldo Carvalho de Melo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120213183937.GH15955@infradead.org \
--to=acme@ghostprotocols.net \
--cc=a.p.zijlstra@chello.nl \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=namhyung.kim@lge.com \
--cc=namhyung@gmail.com \
--cc=paulus@samba.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).