2012-05-20 02:17:22

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [GIT PULL 0/6] perf/core fixes

Hi Ingo,

This also includes a merge with tip/perf/urgent, please consider
pulling,

- Arnaldo

The following changes since commit 978da300c7a65494692b329a6a4cbf364afc37c5:

perf/x86/ibs: Fix undefined reference to `get_ibs_caps' (2012-05-14 14:31:35 +0200)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux perf/core

for you to fetch changes up to 5e1c81d98a5621007824b49dde556fead5ff9c6c:

perf evsel: Create events initially disabled -- again (2012-05-18 16:02:42 -0300)

----------------------------------------------------------------
Fixes for perf/core:

. Rename some perf_target methods to avoid double negation, from Namhyung Kim.

. Revert change to use per task events with inheritance, from Namhyung Kim.

. Events should start disabled till children starts running, from David Ahern.

Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>

----------------------------------------------------------------
Arnaldo Carvalho de Melo (1):
Merge remote-tracking branch 'tip/perf/urgent' into perf/core

David Ahern (1):
perf evsel: Create events initially disabled -- again

Jiri Olsa (2):
perf hists: Fix callchain ip printf format
perf tools: Split term type into value type and term type

Namhyung Kim (3):
perf target: Rename functions to avoid double negation
Revert 'perf evlist: Fix creation of cpu map'
perf target: Add uses_mmap field


2012-05-20 02:16:49

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 6/6] perf evsel: Create events initially disabled -- again

From: David Ahern <[email protected]>

764e16a changed perf-record to create events disabled by default and
enable them once perf initializations are done. This setting was dropped
by 0f82ebc. Now perf events are once again generated during perf's
initialization phase (e.g., generating maps).

As an example, perf opens a lot of files at startup. Unpatched:

perf record -e syscalls:sys_enter_open -ga -fo /tmp/perf.data -- sleep 1
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.087 MB /tmp/perf.data (~3798 samples) ]

Using perf-script to look at the samples shows the perf command generating
563 of the 566 total events.

Patched:

perf record -e syscalls:sys_enter_open -ga -fo /tmp/perf.data -- sleep 1
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.028 MB /tmp/perf.data (~1206 samples) ]

Using perf-script to look at the samples does not show perf command.

Signed-off-by: David Ahern <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/evsel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index d7a2b4b..f4f427c 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -70,6 +70,7 @@ void perf_evsel__config(struct perf_evsel *evsel, struct perf_record_opts *opts,
struct perf_event_attr *attr = &evsel->attr;
int track = !evsel->idx; /* only the first counter needs these */

+ attr->disabled = 1;
attr->sample_id_all = opts->sample_id_all_missing ? 0 : 1;
attr->inherit = !opts->no_inherit;
attr->read_format = PERF_FORMAT_TOTAL_TIME_ENABLED |
@@ -138,7 +139,6 @@ void perf_evsel__config(struct perf_evsel *evsel, struct perf_record_opts *opts,

if (perf_target__none(&opts->target) &&
(!opts->group || evsel == first)) {
- attr->disabled = 1;
attr->enable_on_exec = 1;
}
}
--
1.7.9.2.358.g22243

2012-05-20 02:17:00

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 4/6] perf hists: Fix callchain ip printf format

From: Jiri Olsa <[email protected]>

The callchain address is stored as u64. Current code uses following
format string to display callchain address:

"%p\n", (void *)(long)chain->ip

This way we lose upper 32 bits if we report 64 bit addresses in 32 bit
environment. Fixing this to always display whole 64 bits.

Note, running following to test perf endianity handling:
test 1)
- origin system:
# perf record -a -- sleep 10 (any perf record will do)
# perf report > report.origin
# perf archive perf.data

- copy the perf.data, report.origin and perf.data.tar.bz2
to a target system and run:
# tar xjvf perf.data.tar.bz2 -C ~/.debug
# perf report > report.target
# diff -u report.origin report.target

- the diff should produce no output
(besides some white space stuff and possibly different
date/TZ output)

test 2)
- origin system:
# perf record -ag -fo /tmp/perf.data -- sleep 1
- mount origin system root to the target system on /mnt/origin
- target system:
# perf script --symfs /mnt/origin -I -i /mnt/origin/tmp/perf.data \
--kallsyms /mnt/origin/proc/kallsyms
- complete perf.data header is displayed

Signed-off-by: Jiri Olsa <[email protected]>
Cc: Corey Ashford <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/hist.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 9f6d630..1293b5e 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -599,7 +599,7 @@ static size_t ipchain__fprintf_graph(FILE *fp, struct callchain_list *chain,
if (chain->ms.sym)
ret += fprintf(fp, "%s\n", chain->ms.sym->name);
else
- ret += fprintf(fp, "%p\n", (void *)(long)chain->ip);
+ ret += fprintf(fp, "0x%0" PRIx64 "\n", chain->ip);

return ret;
}
--
1.7.9.2.358.g22243

2012-05-20 02:16:56

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 2/6] Revert 'perf evlist: Fix creation of cpu map'

From: Namhyung Kim <[email protected]>

The commit 55261f46702c ("perf evlist: Fix creation of cpu map") changed
to create a per-task event when no cpu target is specified. However it
caused a problem since perf-task do not allow event inheritance due to
scalability issues so that the result will contain samples only from
parent, not from its children.

So we should use perf-task-per-cpu events anyway to get the right
result. Revert it.

Reported-by: Linus Torvalds <[email protected]>
Analysed-by: Ingo Molnar <[email protected]>
Acked-and-tested-by: Peter Zijlstra <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/evlist.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 80bef28..87889f3 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -609,10 +609,10 @@ int perf_evlist__create_maps(struct perf_evlist *evlist,
if (evlist->threads == NULL)
return -1;

- if (perf_target__has_cpu(target))
- evlist->cpus = cpu_map__new(target->cpu_list);
- else
+ if (perf_target__has_task(target))
evlist->cpus = cpu_map__dummy_new();
+ else
+ evlist->cpus = cpu_map__new(target->cpu_list);

if (evlist->cpus == NULL)
goto out_delete_threads;
--
1.7.9.2.358.g22243

2012-05-20 02:16:59

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 5/6] perf tools: Split term type into value type and term type

From: Jiri Olsa <[email protected]>

Introducing type_val and type_term for term instead of a single type
value. Currently the term type marked out the value type as well.

With this change we can have future string term values being specified
by user and translated into proper number along the processing.

Signed-off-by: Jiri Olsa <[email protected]>
Cc: Corey Ashford <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Robert Richter <[email protected]>
Cc: Stephane Eranian <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/parse-events.c | 45 +++++++++++++++++++-------
tools/perf/util/parse-events.h | 21 ++++++------
tools/perf/util/parse-events.y | 16 ++++-----
tools/perf/util/pmu.c | 70 ++++++++++++++++++++++++----------------
4 files changed, 96 insertions(+), 56 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 5b3a0ef..c7fc18a 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -593,17 +593,27 @@ int parse_events_add_breakpoint(struct list_head *list, int *idx,
static int config_term(struct perf_event_attr *attr,
struct parse_events__term *term)
{
- switch (term->type) {
+#define CHECK_TYPE_VAL(type) \
+do { \
+ if (PARSE_EVENTS__TERM_TYPE_ ## type != term->type_val) \
+ return -EINVAL; \
+} while (0)
+
+ switch (term->type_term) {
case PARSE_EVENTS__TERM_TYPE_CONFIG:
+ CHECK_TYPE_VAL(NUM);
attr->config = term->val.num;
break;
case PARSE_EVENTS__TERM_TYPE_CONFIG1:
+ CHECK_TYPE_VAL(NUM);
attr->config1 = term->val.num;
break;
case PARSE_EVENTS__TERM_TYPE_CONFIG2:
+ CHECK_TYPE_VAL(NUM);
attr->config2 = term->val.num;
break;
case PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD:
+ CHECK_TYPE_VAL(NUM);
attr->sample_period = term->val.num;
break;
case PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE:
@@ -615,7 +625,9 @@ static int config_term(struct perf_event_attr *attr,
default:
return -EINVAL;
}
+
return 0;
+#undef CHECK_TYPE_VAL
}

static int config_attr(struct perf_event_attr *attr,
@@ -1015,11 +1027,12 @@ void print_events(const char *event_glob)

int parse_events__is_hardcoded_term(struct parse_events__term *term)
{
- return term->type <= PARSE_EVENTS__TERM_TYPE_HARDCODED_MAX;
+ return term->type_term != PARSE_EVENTS__TERM_TYPE_USER;
}

-int parse_events__new_term(struct parse_events__term **_term, int type,
- char *config, char *str, long num)
+static int new_term(struct parse_events__term **_term, int type_val,
+ int type_term, char *config,
+ char *str, long num)
{
struct parse_events__term *term;

@@ -1028,15 +1041,11 @@ int parse_events__new_term(struct parse_events__term **_term, int type,
return -ENOMEM;

INIT_LIST_HEAD(&term->list);
- term->type = type;
+ term->type_val = type_val;
+ term->type_term = type_term;
term->config = config;

- switch (type) {
- case PARSE_EVENTS__TERM_TYPE_CONFIG:
- case PARSE_EVENTS__TERM_TYPE_CONFIG1:
- case PARSE_EVENTS__TERM_TYPE_CONFIG2:
- case PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD:
- case PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE:
+ switch (type_val) {
case PARSE_EVENTS__TERM_TYPE_NUM:
term->val.num = num;
break;
@@ -1051,6 +1060,20 @@ int parse_events__new_term(struct parse_events__term **_term, int type,
return 0;
}

+int parse_events__term_num(struct parse_events__term **term,
+ int type_term, char *config, long num)
+{
+ return new_term(term, PARSE_EVENTS__TERM_TYPE_NUM, type_term,
+ config, NULL, num);
+}
+
+int parse_events__term_str(struct parse_events__term **term,
+ int type_term, char *config, char *str)
+{
+ return new_term(term, PARSE_EVENTS__TERM_TYPE_STR, type_term,
+ config, str, 0);
+}
+
void parse_events__free_terms(struct list_head *terms)
{
struct parse_events__term *term, *h;
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index 5cb0028..3fddd61 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -36,16 +36,17 @@ extern int parse_filter(const struct option *opt, const char *str, int unset);
#define EVENTS_HELP_MAX (128*1024)

enum {
+ PARSE_EVENTS__TERM_TYPE_NUM,
+ PARSE_EVENTS__TERM_TYPE_STR,
+};
+
+enum {
+ PARSE_EVENTS__TERM_TYPE_USER,
PARSE_EVENTS__TERM_TYPE_CONFIG,
PARSE_EVENTS__TERM_TYPE_CONFIG1,
PARSE_EVENTS__TERM_TYPE_CONFIG2,
PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD,
PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE,
- PARSE_EVENTS__TERM_TYPE_NUM,
- PARSE_EVENTS__TERM_TYPE_STR,
-
- PARSE_EVENTS__TERM_TYPE_HARDCODED_MAX =
- PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE,
};

struct parse_events__term {
@@ -54,14 +55,16 @@ struct parse_events__term {
char *str;
long num;
} val;
- int type;
-
+ int type_val;
+ int type_term;
struct list_head list;
};

int parse_events__is_hardcoded_term(struct parse_events__term *term);
-int parse_events__new_term(struct parse_events__term **term, int type,
- char *config, char *str, long num);
+int parse_events__term_num(struct parse_events__term **_term,
+ int type_term, char *config, long num);
+int parse_events__term_str(struct parse_events__term **_term,
+ int type_term, char *config, char *str);
void parse_events__free_terms(struct list_head *terms);
int parse_events_modifier(struct list_head *list __used, char *str __used);
int parse_events_add_tracepoint(struct list_head *list, int *idx,
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index d9637da..936913e 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -176,8 +176,8 @@ PE_NAME '=' PE_NAME
{
struct parse_events__term *term;

- ABORT_ON(parse_events__new_term(&term, PARSE_EVENTS__TERM_TYPE_STR,
- $1, $3, 0));
+ ABORT_ON(parse_events__term_str(&term, PARSE_EVENTS__TERM_TYPE_USER,
+ $1, $3));
$$ = term;
}
|
@@ -185,8 +185,8 @@ PE_NAME '=' PE_VALUE
{
struct parse_events__term *term;

- ABORT_ON(parse_events__new_term(&term, PARSE_EVENTS__TERM_TYPE_NUM,
- $1, NULL, $3));
+ ABORT_ON(parse_events__term_num(&term, PARSE_EVENTS__TERM_TYPE_USER,
+ $1, $3));
$$ = term;
}
|
@@ -194,8 +194,8 @@ PE_NAME
{
struct parse_events__term *term;

- ABORT_ON(parse_events__new_term(&term, PARSE_EVENTS__TERM_TYPE_NUM,
- $1, NULL, 1));
+ ABORT_ON(parse_events__term_num(&term, PARSE_EVENTS__TERM_TYPE_USER,
+ $1, 1));
$$ = term;
}
|
@@ -203,7 +203,7 @@ PE_TERM '=' PE_VALUE
{
struct parse_events__term *term;

- ABORT_ON(parse_events__new_term(&term, $1, NULL, NULL, $3));
+ ABORT_ON(parse_events__term_num(&term, $1, NULL, $3));
$$ = term;
}
|
@@ -211,7 +211,7 @@ PE_TERM
{
struct parse_events__term *term;

- ABORT_ON(parse_events__new_term(&term, $1, NULL, NULL, 1));
+ ABORT_ON(parse_events__term_num(&term, $1, NULL, 1));
$$ = term;
}

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index cb08a11..8ee219b 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -225,7 +225,7 @@ static int pmu_config_term(struct list_head *formats,
if (parse_events__is_hardcoded_term(term))
return 0;

- if (term->type != PARSE_EVENTS__TERM_TYPE_NUM)
+ if (term->type_val != PARSE_EVENTS__TERM_TYPE_NUM)
return -EINVAL;

format = pmu_find_format(formats, term->config);
@@ -246,6 +246,11 @@ static int pmu_config_term(struct list_head *formats,
return -EINVAL;
}

+ /*
+ * XXX If we ever decide to go with string values for
+ * non-hardcoded terms, here's the place to translate
+ * them into value.
+ */
*vp |= pmu_format_value(format->bits, term->val.num);
return 0;
}
@@ -324,49 +329,58 @@ static struct test_format {
/* Simulated users input. */
static struct parse_events__term test_terms[] = {
{
- .config = (char *) "krava01",
- .val.num = 15,
- .type = PARSE_EVENTS__TERM_TYPE_NUM,
+ .config = (char *) "krava01",
+ .val.num = 15,
+ .type_val = PARSE_EVENTS__TERM_TYPE_NUM,
+ .type_term = PARSE_EVENTS__TERM_TYPE_USER,
},
{
- .config = (char *) "krava02",
- .val.num = 170,
- .type = PARSE_EVENTS__TERM_TYPE_NUM,
+ .config = (char *) "krava02",
+ .val.num = 170,
+ .type_val = PARSE_EVENTS__TERM_TYPE_NUM,
+ .type_term = PARSE_EVENTS__TERM_TYPE_USER,
},
{
- .config = (char *) "krava03",
- .val.num = 1,
- .type = PARSE_EVENTS__TERM_TYPE_NUM,
+ .config = (char *) "krava03",
+ .val.num = 1,
+ .type_val = PARSE_EVENTS__TERM_TYPE_NUM,
+ .type_term = PARSE_EVENTS__TERM_TYPE_USER,
},
{
- .config = (char *) "krava11",
- .val.num = 27,
- .type = PARSE_EVENTS__TERM_TYPE_NUM,
+ .config = (char *) "krava11",
+ .val.num = 27,
+ .type_val = PARSE_EVENTS__TERM_TYPE_NUM,
+ .type_term = PARSE_EVENTS__TERM_TYPE_USER,
},
{
- .config = (char *) "krava12",
- .val.num = 1,
- .type = PARSE_EVENTS__TERM_TYPE_NUM,
+ .config = (char *) "krava12",
+ .val.num = 1,
+ .type_val = PARSE_EVENTS__TERM_TYPE_NUM,
+ .type_term = PARSE_EVENTS__TERM_TYPE_USER,
},
{
- .config = (char *) "krava13",
- .val.num = 2,
- .type = PARSE_EVENTS__TERM_TYPE_NUM,
+ .config = (char *) "krava13",
+ .val.num = 2,
+ .type_val = PARSE_EVENTS__TERM_TYPE_NUM,
+ .type_term = PARSE_EVENTS__TERM_TYPE_USER,
},
{
- .config = (char *) "krava21",
- .val.num = 119,
- .type = PARSE_EVENTS__TERM_TYPE_NUM,
+ .config = (char *) "krava21",
+ .val.num = 119,
+ .type_val = PARSE_EVENTS__TERM_TYPE_NUM,
+ .type_term = PARSE_EVENTS__TERM_TYPE_USER,
},
{
- .config = (char *) "krava22",
- .val.num = 11,
- .type = PARSE_EVENTS__TERM_TYPE_NUM,
+ .config = (char *) "krava22",
+ .val.num = 11,
+ .type_val = PARSE_EVENTS__TERM_TYPE_NUM,
+ .type_term = PARSE_EVENTS__TERM_TYPE_USER,
},
{
- .config = (char *) "krava23",
- .val.num = 2,
- .type = PARSE_EVENTS__TERM_TYPE_NUM,
+ .config = (char *) "krava23",
+ .val.num = 2,
+ .type_val = PARSE_EVENTS__TERM_TYPE_NUM,
+ .type_term = PARSE_EVENTS__TERM_TYPE_USER,
},
};
#define TERMS_CNT (sizeof(test_terms) / sizeof(struct parse_events__term))
--
1.7.9.2.358.g22243

2012-05-20 02:17:56

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 3/6] perf target: Add uses_mmap field

From: Namhyung Kim <[email protected]>

If perf doesn't mmap on event (like perf stat), it should not create
per-task-per-cpu events. So just use a dummy cpu map to create a
per-task event for this case.

Signed-off-by: Namhyung Kim <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
[ committer note: renamed .need_mmap to .uses_mmap ]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-record.c | 3 +++
tools/perf/builtin-test.c | 1 +
tools/perf/builtin-top.c | 3 +++
tools/perf/util/evlist.c | 2 ++
tools/perf/util/target.h | 1 +
5 files changed, 10 insertions(+)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index d19058a..8a3dfac 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -754,6 +754,9 @@ static struct perf_record record = {
.user_freq = UINT_MAX,
.user_interval = ULLONG_MAX,
.freq = 1000,
+ .target = {
+ .uses_mmap = true,
+ },
},
.write_mode = WRITE_FORCE,
.file_new = true,
diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c
index 9d9abbb..4eaa665 100644
--- a/tools/perf/builtin-test.c
+++ b/tools/perf/builtin-test.c
@@ -1167,6 +1167,7 @@ static int test__PERF_RECORD(void)
struct perf_record_opts opts = {
.target = {
.uid = UINT_MAX,
+ .uses_mmap = true,
},
.no_delay = true,
.freq = 10,
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 553560a..3e981a7 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1162,6 +1162,9 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
.freq = 1000, /* 1 KHz */
.mmap_pages = 128,
.sym_pcnt_filter = 5,
+ .target = {
+ .uses_mmap = true,
+ },
};
char callchain_default_opt[] = "fractal,0.5,callee";
const struct option options[] = {
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 87889f3..4ac5f5a 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -611,6 +611,8 @@ int perf_evlist__create_maps(struct perf_evlist *evlist,

if (perf_target__has_task(target))
evlist->cpus = cpu_map__dummy_new();
+ else if (!perf_target__has_cpu(target) && !target->uses_mmap)
+ evlist->cpus = cpu_map__dummy_new();
else
evlist->cpus = cpu_map__new(target->cpu_list);

diff --git a/tools/perf/util/target.h b/tools/perf/util/target.h
index c43f632..a4be857 100644
--- a/tools/perf/util/target.h
+++ b/tools/perf/util/target.h
@@ -11,6 +11,7 @@ struct perf_target {
const char *uid_str;
uid_t uid;
bool system_wide;
+ bool uses_mmap;
};

enum perf_target_errno {
--
1.7.9.2.358.g22243

2012-05-20 02:17:55

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 1/6] perf target: Rename functions to avoid double negation

From: Namhyung Kim <[email protected]>

Rename perf_target__no_{cpu,task} to perf_target__has_{cpu,task} because
it's more intuitive and easy to parse (for human beings) when used with
negation.

The names are came out from David Ahern. It is intended to be a
mechanical substitution without any functional change.

The perf_target__none remains unchanged since I couldn't find a right
name and it is hardly used with negation.

Signed-off-by: Namhyung Kim <[email protected]>
Suggested-by: David Ahern <[email protected]>
Suggested-by: Ingo Molnar <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-stat.c | 14 +++++++-------
tools/perf/builtin-top.c | 2 +-
tools/perf/util/evlist.c | 2 +-
tools/perf/util/evsel.c | 2 +-
tools/perf/util/target.h | 10 +++++-----
5 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index d060568..0f4b51a 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -292,10 +292,10 @@ static int create_perf_stat_counter(struct perf_evsel *evsel,

attr->inherit = !no_inherit;

- if (!perf_target__no_cpu(&target))
+ if (perf_target__has_cpu(&target))
return perf_evsel__open_per_cpu(evsel, evsel_list->cpus,
group, group_fd);
- if (perf_target__no_task(&target) && (!group || evsel == first)) {
+ if (!perf_target__has_task(&target) && (!group || evsel == first)) {
attr->disabled = 1;
attr->enable_on_exec = 1;
}
@@ -972,7 +972,7 @@ static void print_stat(int argc, const char **argv)
if (!csv_output) {
fprintf(output, "\n");
fprintf(output, " Performance counter stats for ");
- if (perf_target__no_task(&target)) {
+ if (!perf_target__has_task(&target)) {
fprintf(output, "\'%s", argv[0]);
for (i = 1; i < argc; i++)
fprintf(output, " %s", argv[i]);
@@ -1194,13 +1194,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 && perf_target__no_task(&target))
+ if (!argc && !perf_target__has_task(&target))
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) && perf_target__no_cpu(&target)) {
+ if ((no_aggr || nr_cgroups) && !perf_target__has_cpu(&target)) {
fprintf(stderr, "both cgroup and no-aggregation "
"modes only available in system-wide mode\n");

@@ -1213,9 +1213,9 @@ int cmd_stat(int argc, const char **argv, const char *prefix __used)
perf_target__validate(&target);

if (perf_evlist__create_maps(evsel_list, &target) < 0) {
- if (!perf_target__no_task(&target))
+ if (perf_target__has_task(&target))
pr_err("Problems finding threads of monitor\n");
- if (!perf_target__no_cpu(&target))
+ if (perf_target__has_cpu(&target))
perror("failed to parse CPUs map");

usage_with_options(stat_usage, options);
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 4eb6171..553560a 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1020,7 +1020,7 @@ static int __cmd_top(struct perf_top *top)
if (ret)
goto out_delete;

- if (!perf_target__no_task(&top->target))
+ if (perf_target__has_task(&top->target))
perf_event__synthesize_thread_map(&top->tool, top->evlist->threads,
perf_event__process,
&top->session->host_machine);
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 1201daf..80bef28 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -609,7 +609,7 @@ int perf_evlist__create_maps(struct perf_evlist *evlist,
if (evlist->threads == NULL)
return -1;

- if (!perf_target__no_cpu(target))
+ if (perf_target__has_cpu(target))
evlist->cpus = cpu_map__new(target->cpu_list);
else
evlist->cpus = cpu_map__dummy_new();
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 21eaab2..d7a2b4b 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -115,7 +115,7 @@ void perf_evsel__config(struct perf_evsel *evsel, struct perf_record_opts *opts,

if (!opts->sample_id_all_missing &&
(opts->sample_time || !opts->no_inherit ||
- !perf_target__no_cpu(&opts->target)))
+ perf_target__has_cpu(&opts->target)))
attr->sample_type |= PERF_SAMPLE_TIME;

if (opts->raw_samples) {
diff --git a/tools/perf/util/target.h b/tools/perf/util/target.h
index 127cff3..c43f632 100644
--- a/tools/perf/util/target.h
+++ b/tools/perf/util/target.h
@@ -46,19 +46,19 @@ enum perf_target_errno perf_target__parse_uid(struct perf_target *target);
int perf_target__strerror(struct perf_target *target, int errnum, char *buf,
size_t buflen);

-static inline bool perf_target__no_task(struct perf_target *target)
+static inline bool perf_target__has_task(struct perf_target *target)
{
- return !target->pid && !target->tid && !target->uid_str;
+ return target->tid || target->pid || target->uid_str;
}

-static inline bool perf_target__no_cpu(struct perf_target *target)
+static inline bool perf_target__has_cpu(struct perf_target *target)
{
- return !target->system_wide && !target->cpu_list;
+ return target->system_wide || target->cpu_list;
}

static inline bool perf_target__none(struct perf_target *target)
{
- return perf_target__no_task(target) && perf_target__no_cpu(target);
+ return !perf_target__has_task(target) && !perf_target__has_cpu(target);
}

#endif /* _PERF_TARGET_H */
--
1.7.9.2.358.g22243

2012-05-20 02:55:17

by David Ahern

[permalink] [raw]
Subject: Re: [GIT PULL 0/6] perf/core fixes

On 5/19/12 8:16 PM, Arnaldo Carvalho de Melo wrote:

> Arnaldo Carvalho de Melo (1):
> Merge remote-tracking branch 'tip/perf/urgent' into perf/core
>
> David Ahern (1):
> perf evsel: Create events initially disabled -- again
>
> Jiri Olsa (2):
> perf hists: Fix callchain ip printf format
> perf tools: Split term type into value type and term type
>
> Namhyung Kim (3):
> perf target: Rename functions to avoid double negation
> Revert 'perf evlist: Fix creation of cpu map'
> perf target: Add uses_mmap field

What about Stephane's pipe mode patch set? The first 3 and the last one
of the 5 are good fixes to pick up.

David

2012-05-21 07:19:29

by Ingo Molnar

[permalink] [raw]
Subject: Re: [GIT PULL 0/6] perf/core fixes


* Arnaldo Carvalho de Melo <[email protected]> wrote:

> Hi Ingo,
>
> This also includes a merge with tip/perf/urgent, please consider
> pulling,
>
> - Arnaldo
>
> The following changes since commit 978da300c7a65494692b329a6a4cbf364afc37c5:
>
> perf/x86/ibs: Fix undefined reference to `get_ibs_caps' (2012-05-14 14:31:35 +0200)
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux perf/core
>
> for you to fetch changes up to 5e1c81d98a5621007824b49dde556fead5ff9c6c:
>
> perf evsel: Create events initially disabled -- again (2012-05-18 16:02:42 -0300)
>
> ----------------------------------------------------------------
> Fixes for perf/core:
>
> . Rename some perf_target methods to avoid double negation, from Namhyung Kim.
>
> . Revert change to use per task events with inheritance, from Namhyung Kim.
>
> . Events should start disabled till children starts running, from David Ahern.
>
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
>
> ----------------------------------------------------------------
> Arnaldo Carvalho de Melo (1):
> Merge remote-tracking branch 'tip/perf/urgent' into perf/core
>
> David Ahern (1):
> perf evsel: Create events initially disabled -- again
>
> Jiri Olsa (2):
> perf hists: Fix callchain ip printf format
> perf tools: Split term type into value type and term type
>
> Namhyung Kim (3):
> perf target: Rename functions to avoid double negation
> Revert 'perf evlist: Fix creation of cpu map'
> perf target: Add uses_mmap field

Pulled, thanks a lot Arnaldo!

This should also fix the regression Linus reported, AFAICS.

Thanks,

Ingo