* [PATCH 1/5] perf build: Add special fixdep cleaning rule
2017-02-17 14:00 [PATCH 0/5] perf tools: Few fixes Jiri Olsa
@ 2017-02-17 14:00 ` Jiri Olsa
2017-02-21 8:13 ` [tip:perf/urgent] " tip-bot for Jiri Olsa
2017-02-17 14:00 ` [PATCH 2/5] perf tools: Move new_term arguments into struct parse_events_term template Jiri Olsa
` (3 subsequent siblings)
4 siblings, 1 reply; 30+ messages in thread
From: Jiri Olsa @ 2017-02-17 14:00 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: David Ahern, Namhyung Kim, Peter Zijlstra, lkml, Ingo Molnar
From: Jiri Olsa <jolsa@redhat.com>
Ingo reported following build failure:
On Sat, Feb 11, 2017 at 12:12:34PM +0100, Ingo Molnar wrote:
>
> So I had this oldish 32-bit 15.10 Ubuntu installation around (fully updated), and
> trying to build perf gave me:
>
> deimos:~/tip/tools/perf> make
> BUILD: Doing 'make -j4' parallel build
> make[3]: *** No rule to make target '/usr/include/x86_64-linux-gnu/sys/types.h', needed by 'fixdep.o'. Stop.
> Makefile:42: recipe for target 'fixdep-in.o' failed
> make[2]: *** [fixdep-in.o] Error 2
> /home/mingo/tip/tools/build/Makefile.include:4: recipe for target 'fixdep' failed
> make[1]: *** [fixdep] Error 2
> Makefile:68: recipe for target 'all' failed
> make: *** [all] Error 2
>
> Now this got a bit better after I did a 'make mrproper' in the kernel tree:
>
> deimos:~/tip/tools/perf> make
> BUILD: Doing 'make -j4' parallel build
> HOSTCC fixdep.o
> /home/mingo/tip/tools/build/fixdep: 1: /home/mingo/tip/tools/build/fixdep: Syntax error: "(" unexpected
> /home/mingo/tip/tools/build/Makefile.build:101: recipe for target 'fixdep.o' failed
> make[3]: *** [fixdep.o] Error 2
> Makefile:42: recipe for target 'fixdep-in.o' failed
> make[2]: *** [fixdep-in.o] Error 2
> /home/mingo/tip/tools/build/Makefile.include:4: recipe for target 'fixdep' failed
> make[1]: *** [fixdep] Error 2
> Makefile:68: recipe for target 'all' failed
> make: *** [all] Error 2
>
> After some digging it turns out that my 'fixdep' binary was 64-bit:
>
> deimos:~/tip/tools/perf> file /home/mingo/tip/tools/build/fixdep
> /home/mingo/tip/tools/build/fixdep: ELF 64-bit LSB executable, x86-64, version 1
> (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, for GNU/Linux
> 2.6.32, BuildID[sha1]=d527f736b57b5ba47210fbcb562a3b52867d21c1, not stripped
>
> But it did not get cleaned out by 'make clean'.
>
> Only after I did a 'make clean' in tools/ itself, did it get built properly.
It shows we don't clean up properly the fixdep objects,
so adding special rule for that.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/n/tip-0u41aa6z3t2vtodya7h3nom2@git.kernel.org
---
tools/build/Makefile | 4 ++--
tools/build/Makefile.include | 3 +++
tools/perf/Makefile.perf | 4 ++--
3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/tools/build/Makefile b/tools/build/Makefile
index aaf7ed329a45..477f00eda591 100644
--- a/tools/build/Makefile
+++ b/tools/build/Makefile
@@ -35,8 +35,8 @@ all: $(OUTPUT)fixdep
clean:
$(call QUIET_CLEAN, fixdep)
- $(Q)find . -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete
- $(Q)rm -f fixdep
+ $(Q)find $(if $(OUTPUT),$(OUTPUT),.) -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete
+ $(Q)rm -f $(OUTPUT)fixdep
$(OUTPUT)fixdep-in.o: FORCE
$(Q)$(MAKE) $(build)=fixdep
diff --git a/tools/build/Makefile.include b/tools/build/Makefile.include
index ad22e4e7bc59..d360f39a445b 100644
--- a/tools/build/Makefile.include
+++ b/tools/build/Makefile.include
@@ -3,4 +3,7 @@ build := -f $(srctree)/tools/build/Makefile.build dir=. obj
fixdep:
$(Q)$(MAKE) -C $(srctree)/tools/build CFLAGS= LDFLAGS= $(OUTPUT)fixdep
+fixdep-clean:
+ $(Q)$(MAKE) -C $(srctree)/tools/build clean
+
.PHONY: fixdep
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 4da19b6ba94a..79fe31f20a17 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -726,13 +726,13 @@ config-clean:
$(call QUIET_CLEAN, config)
$(Q)$(MAKE) -C $(srctree)/tools/build/feature/ $(if $(OUTPUT),OUTPUT=$(OUTPUT)feature/,) clean >/dev/null
-clean:: $(LIBTRACEEVENT)-clean $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean config-clean
+clean:: $(LIBTRACEEVENT)-clean $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean config-clean fixdep-clean
$(call QUIET_CLEAN, core-objs) $(RM) $(LIB_FILE) $(OUTPUT)perf-archive $(OUTPUT)perf-with-kcore $(LANG_BINDINGS)
$(Q)find $(if $(OUTPUT),$(OUTPUT),.) -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete
$(Q)$(RM) $(OUTPUT).config-detected
$(call QUIET_CLEAN, core-progs) $(RM) $(ALL_PROGRAMS) perf perf-read-vdso32 perf-read-vdsox32 $(OUTPUT)pmu-events/jevents $(OUTPUT)$(LIBJVMTI).so
$(call QUIET_CLEAN, core-gen) $(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo $(OUTPUT)common-cmds.h TAGS tags cscope* $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)FEATURE-DUMP $(OUTPUT)util/*-bison* $(OUTPUT)util/*-flex* \
- $(OUTPUT)util/intel-pt-decoder/inat-tables.c $(OUTPUT)fixdep \
+ $(OUTPUT)util/intel-pt-decoder/inat-tables.c \
$(OUTPUT)tests/llvm-src-{base,kbuild,prologue,relocation}.c \
$(OUTPUT)pmu-events/pmu-events.c
$(QUIET_SUBDIR0)Documentation $(QUIET_SUBDIR1) clean
--
2.7.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* [tip:perf/urgent] perf build: Add special fixdep cleaning rule
2017-02-17 14:00 ` [PATCH 1/5] perf build: Add special fixdep cleaning rule Jiri Olsa
@ 2017-02-21 8:13 ` tip-bot for Jiri Olsa
0 siblings, 0 replies; 30+ messages in thread
From: tip-bot for Jiri Olsa @ 2017-02-21 8:13 UTC (permalink / raw)
To: linux-tip-commits
Cc: tglx, acme, dsahern, linux-kernel, hpa, jolsa, mingo, namhyung,
a.p.zijlstra, jolsa
Commit-ID: 85e0d509654c2e2e58b29e50a883acd4c4e8807d
Gitweb: http://git.kernel.org/tip/85e0d509654c2e2e58b29e50a883acd4c4e8807d
Author: Jiri Olsa <jolsa@redhat.com>
AuthorDate: Fri, 17 Feb 2017 15:00:54 +0100
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 17 Feb 2017 16:04:38 -0300
perf build: Add special fixdep cleaning rule
Ingo reported following build failure:
On Sat, Feb 11, 2017 at 12:12:34PM +0100, Ingo Molnar wrote:
>
> So I had this oldish 32-bit 15.10 Ubuntu installation around (fully updated), and
> trying to build perf gave me:
>
> deimos:~/tip/tools/perf> make
> BUILD: Doing 'make -j4' parallel build
> make[3]: *** No rule to make target '/usr/include/x86_64-linux-gnu/sys/types.h', needed by 'fixdep.o'. Stop.
> Makefile:42: recipe for target 'fixdep-in.o' failed
> make[2]: *** [fixdep-in.o] Error 2
> /home/mingo/tip/tools/build/Makefile.include:4: recipe for target 'fixdep' failed
> make[1]: *** [fixdep] Error 2
> Makefile:68: recipe for target 'all' failed
> make: *** [all] Error 2
>
> Now this got a bit better after I did a 'make mrproper' in the kernel tree:
>
> deimos:~/tip/tools/perf> make
> BUILD: Doing 'make -j4' parallel build
> HOSTCC fixdep.o
> /home/mingo/tip/tools/build/fixdep: 1: /home/mingo/tip/tools/build/fixdep: Syntax error: "(" unexpected
> /home/mingo/tip/tools/build/Makefile.build:101: recipe for target 'fixdep.o' failed
> make[3]: *** [fixdep.o] Error 2
> Makefile:42: recipe for target 'fixdep-in.o' failed
> make[2]: *** [fixdep-in.o] Error 2
> /home/mingo/tip/tools/build/Makefile.include:4: recipe for target 'fixdep' failed
> make[1]: *** [fixdep] Error 2
> Makefile:68: recipe for target 'all' failed
> make: *** [all] Error 2
>
> After some digging it turns out that my 'fixdep' binary was 64-bit:
>
> deimos:~/tip/tools/perf> file /home/mingo/tip/tools/build/fixdep
> /home/mingo/tip/tools/build/fixdep: ELF 64-bit LSB executable, x86-64, version 1
> (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, for GNU/Linux
> 2.6.32, BuildID[sha1]=d527f736b57b5ba47210fbcb562a3b52867d21c1, not stripped
>
> But it did not get cleaned out by 'make clean'.
>
> Only after I did a 'make clean' in tools/ itself, did it get built properly.
It shows we don't clean up properly the fixdep objects, so adding
special rule for that.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Reported-by: Ingo Molnar <mingo@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1487340058-10496-2-git-send-email-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/build/Makefile | 4 ++--
tools/build/Makefile.include | 3 +++
tools/perf/Makefile.perf | 4 ++--
3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/tools/build/Makefile b/tools/build/Makefile
index aaf7ed3..477f00e 100644
--- a/tools/build/Makefile
+++ b/tools/build/Makefile
@@ -35,8 +35,8 @@ all: $(OUTPUT)fixdep
clean:
$(call QUIET_CLEAN, fixdep)
- $(Q)find . -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete
- $(Q)rm -f fixdep
+ $(Q)find $(if $(OUTPUT),$(OUTPUT),.) -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete
+ $(Q)rm -f $(OUTPUT)fixdep
$(OUTPUT)fixdep-in.o: FORCE
$(Q)$(MAKE) $(build)=fixdep
diff --git a/tools/build/Makefile.include b/tools/build/Makefile.include
index ad22e4e..d360f39a4 100644
--- a/tools/build/Makefile.include
+++ b/tools/build/Makefile.include
@@ -3,4 +3,7 @@ build := -f $(srctree)/tools/build/Makefile.build dir=. obj
fixdep:
$(Q)$(MAKE) -C $(srctree)/tools/build CFLAGS= LDFLAGS= $(OUTPUT)fixdep
+fixdep-clean:
+ $(Q)$(MAKE) -C $(srctree)/tools/build clean
+
.PHONY: fixdep
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 4da19b6..79fe31f 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -726,13 +726,13 @@ config-clean:
$(call QUIET_CLEAN, config)
$(Q)$(MAKE) -C $(srctree)/tools/build/feature/ $(if $(OUTPUT),OUTPUT=$(OUTPUT)feature/,) clean >/dev/null
-clean:: $(LIBTRACEEVENT)-clean $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean config-clean
+clean:: $(LIBTRACEEVENT)-clean $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean config-clean fixdep-clean
$(call QUIET_CLEAN, core-objs) $(RM) $(LIB_FILE) $(OUTPUT)perf-archive $(OUTPUT)perf-with-kcore $(LANG_BINDINGS)
$(Q)find $(if $(OUTPUT),$(OUTPUT),.) -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete
$(Q)$(RM) $(OUTPUT).config-detected
$(call QUIET_CLEAN, core-progs) $(RM) $(ALL_PROGRAMS) perf perf-read-vdso32 perf-read-vdsox32 $(OUTPUT)pmu-events/jevents $(OUTPUT)$(LIBJVMTI).so
$(call QUIET_CLEAN, core-gen) $(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo $(OUTPUT)common-cmds.h TAGS tags cscope* $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)FEATURE-DUMP $(OUTPUT)util/*-bison* $(OUTPUT)util/*-flex* \
- $(OUTPUT)util/intel-pt-decoder/inat-tables.c $(OUTPUT)fixdep \
+ $(OUTPUT)util/intel-pt-decoder/inat-tables.c \
$(OUTPUT)tests/llvm-src-{base,kbuild,prologue,relocation}.c \
$(OUTPUT)pmu-events/pmu-events.c
$(QUIET_SUBDIR0)Documentation $(QUIET_SUBDIR1) clean
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 2/5] perf tools: Move new_term arguments into struct parse_events_term template
2017-02-17 14:00 [PATCH 0/5] perf tools: Few fixes Jiri Olsa
2017-02-17 14:00 ` [PATCH 1/5] perf build: Add special fixdep cleaning rule Jiri Olsa
@ 2017-02-17 14:00 ` Jiri Olsa
2017-02-21 8:13 ` [tip:perf/urgent] " tip-bot for Jiri Olsa
2017-02-17 14:00 ` [PATCH 3/5] perf tools: Fail on using multiple bits long terms without value Jiri Olsa
` (2 subsequent siblings)
4 siblings, 1 reply; 30+ messages in thread
From: Jiri Olsa @ 2017-02-17 14:00 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: David Ahern, Namhyung Kim, Peter Zijlstra, lkml, Ingo Molnar
We need to add yet another parameter to new_term
function in following patch, so it's better to
move first all the current params into template
struct parse_events_term and use it as a single
argument.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/n/tip-5vdq8gfbsva97vwrfmjl5p6f@git.kernel.org
---
tools/perf/util/parse-events.c | 69 ++++++++++++++++++++++++------------------
1 file changed, 39 insertions(+), 30 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 281e44af31e2..984d99a8fdc5 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -2318,24 +2318,20 @@ int parse_events__is_hardcoded_term(struct parse_events_term *term)
return term->type_term != PARSE_EVENTS__TERM_TYPE_USER;
}
-static int new_term(struct parse_events_term **_term, int type_val,
- int type_term, char *config,
- char *str, u64 num, int err_term, int err_val)
+static int new_term(struct parse_events_term **_term,
+ struct parse_events_term *temp,
+ char *str, u64 num)
{
struct parse_events_term *term;
- term = zalloc(sizeof(*term));
+ term = malloc(sizeof(*term));
if (!term)
return -ENOMEM;
+ *term = *temp;
INIT_LIST_HEAD(&term->list);
- term->type_val = type_val;
- term->type_term = type_term;
- term->config = config;
- term->err_term = err_term;
- term->err_val = err_val;
- switch (type_val) {
+ switch (term->type_val) {
case PARSE_EVENTS__TERM_TYPE_NUM:
term->val.num = num;
break;
@@ -2358,10 +2354,15 @@ int parse_events_term__num(struct parse_events_term **term,
YYLTYPE *loc_term = loc_term_;
YYLTYPE *loc_val = loc_val_;
- return new_term(term, PARSE_EVENTS__TERM_TYPE_NUM, type_term,
- config, NULL, num,
- loc_term ? loc_term->first_column : 0,
- loc_val ? loc_val->first_column : 0);
+ struct parse_events_term temp = {
+ .type_val = PARSE_EVENTS__TERM_TYPE_NUM,
+ .type_term = type_term,
+ .config = config,
+ .err_term = loc_term ? loc_term->first_column : 0,
+ .err_val = loc_val ? loc_val->first_column : 0,
+ };
+
+ return new_term(term, &temp, NULL, num);
}
int parse_events_term__str(struct parse_events_term **term,
@@ -2371,37 +2372,45 @@ int parse_events_term__str(struct parse_events_term **term,
YYLTYPE *loc_term = loc_term_;
YYLTYPE *loc_val = loc_val_;
- return new_term(term, PARSE_EVENTS__TERM_TYPE_STR, type_term,
- config, str, 0,
- loc_term ? loc_term->first_column : 0,
- loc_val ? loc_val->first_column : 0);
+ struct parse_events_term temp = {
+ .type_val = PARSE_EVENTS__TERM_TYPE_STR,
+ .type_term = type_term,
+ .config = config,
+ .err_term = loc_term ? loc_term->first_column : 0,
+ .err_val = loc_val ? loc_val->first_column : 0,
+ };
+
+ return new_term(term, &temp, str, 0);
}
int parse_events_term__sym_hw(struct parse_events_term **term,
char *config, unsigned idx)
{
struct event_symbol *sym;
+ struct parse_events_term temp = {
+ .type_val = PARSE_EVENTS__TERM_TYPE_STR,
+ .type_term = PARSE_EVENTS__TERM_TYPE_USER,
+ .config = config ?: (char *) "event",
+ };
BUG_ON(idx >= PERF_COUNT_HW_MAX);
sym = &event_symbols_hw[idx];
- if (config)
- return new_term(term, PARSE_EVENTS__TERM_TYPE_STR,
- PARSE_EVENTS__TERM_TYPE_USER, config,
- (char *) sym->symbol, 0, 0, 0);
- else
- return new_term(term, PARSE_EVENTS__TERM_TYPE_STR,
- PARSE_EVENTS__TERM_TYPE_USER,
- (char *) "event", (char *) sym->symbol,
- 0, 0, 0);
+ return new_term(term, &temp, (char *) sym->symbol, 0);
}
int parse_events_term__clone(struct parse_events_term **new,
struct parse_events_term *term)
{
- return new_term(new, term->type_val, term->type_term, term->config,
- term->val.str, term->val.num,
- term->err_term, term->err_val);
+ struct parse_events_term temp = {
+ .type_val = term->type_val,
+ .type_term = term->type_term,
+ .config = term->config,
+ .err_term = term->err_term,
+ .err_val = term->err_val,
+ };
+
+ return new_term(new, &temp, term->val.str, term->val.num);
}
void parse_events_terms__purge(struct list_head *terms)
--
2.7.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* [tip:perf/urgent] perf tools: Move new_term arguments into struct parse_events_term template
2017-02-17 14:00 ` [PATCH 2/5] perf tools: Move new_term arguments into struct parse_events_term template Jiri Olsa
@ 2017-02-21 8:13 ` tip-bot for Jiri Olsa
0 siblings, 0 replies; 30+ messages in thread
From: tip-bot for Jiri Olsa @ 2017-02-21 8:13 UTC (permalink / raw)
To: linux-tip-commits
Cc: tglx, namhyung, linux-kernel, jolsa, acme, dsahern, mingo,
a.p.zijlstra, hpa
Commit-ID: 67b49b38f7bd6f34319b540ce824d5697241b3a8
Gitweb: http://git.kernel.org/tip/67b49b38f7bd6f34319b540ce824d5697241b3a8
Author: Jiri Olsa <jolsa@kernel.org>
AuthorDate: Fri, 17 Feb 2017 15:00:55 +0100
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 17 Feb 2017 17:27:54 -0300
perf tools: Move new_term arguments into struct parse_events_term template
We need to add yet another parameter to new_term function in following
patch, so it's better to move first all the current params into template
struct parse_events_term and use it as a single argument.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1487340058-10496-3-git-send-email-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/parse-events.c | 69 ++++++++++++++++++++++++------------------
1 file changed, 39 insertions(+), 30 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 281e44a..984d99a 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -2318,24 +2318,20 @@ int parse_events__is_hardcoded_term(struct parse_events_term *term)
return term->type_term != PARSE_EVENTS__TERM_TYPE_USER;
}
-static int new_term(struct parse_events_term **_term, int type_val,
- int type_term, char *config,
- char *str, u64 num, int err_term, int err_val)
+static int new_term(struct parse_events_term **_term,
+ struct parse_events_term *temp,
+ char *str, u64 num)
{
struct parse_events_term *term;
- term = zalloc(sizeof(*term));
+ term = malloc(sizeof(*term));
if (!term)
return -ENOMEM;
+ *term = *temp;
INIT_LIST_HEAD(&term->list);
- term->type_val = type_val;
- term->type_term = type_term;
- term->config = config;
- term->err_term = err_term;
- term->err_val = err_val;
- switch (type_val) {
+ switch (term->type_val) {
case PARSE_EVENTS__TERM_TYPE_NUM:
term->val.num = num;
break;
@@ -2358,10 +2354,15 @@ int parse_events_term__num(struct parse_events_term **term,
YYLTYPE *loc_term = loc_term_;
YYLTYPE *loc_val = loc_val_;
- return new_term(term, PARSE_EVENTS__TERM_TYPE_NUM, type_term,
- config, NULL, num,
- loc_term ? loc_term->first_column : 0,
- loc_val ? loc_val->first_column : 0);
+ struct parse_events_term temp = {
+ .type_val = PARSE_EVENTS__TERM_TYPE_NUM,
+ .type_term = type_term,
+ .config = config,
+ .err_term = loc_term ? loc_term->first_column : 0,
+ .err_val = loc_val ? loc_val->first_column : 0,
+ };
+
+ return new_term(term, &temp, NULL, num);
}
int parse_events_term__str(struct parse_events_term **term,
@@ -2371,37 +2372,45 @@ int parse_events_term__str(struct parse_events_term **term,
YYLTYPE *loc_term = loc_term_;
YYLTYPE *loc_val = loc_val_;
- return new_term(term, PARSE_EVENTS__TERM_TYPE_STR, type_term,
- config, str, 0,
- loc_term ? loc_term->first_column : 0,
- loc_val ? loc_val->first_column : 0);
+ struct parse_events_term temp = {
+ .type_val = PARSE_EVENTS__TERM_TYPE_STR,
+ .type_term = type_term,
+ .config = config,
+ .err_term = loc_term ? loc_term->first_column : 0,
+ .err_val = loc_val ? loc_val->first_column : 0,
+ };
+
+ return new_term(term, &temp, str, 0);
}
int parse_events_term__sym_hw(struct parse_events_term **term,
char *config, unsigned idx)
{
struct event_symbol *sym;
+ struct parse_events_term temp = {
+ .type_val = PARSE_EVENTS__TERM_TYPE_STR,
+ .type_term = PARSE_EVENTS__TERM_TYPE_USER,
+ .config = config ?: (char *) "event",
+ };
BUG_ON(idx >= PERF_COUNT_HW_MAX);
sym = &event_symbols_hw[idx];
- if (config)
- return new_term(term, PARSE_EVENTS__TERM_TYPE_STR,
- PARSE_EVENTS__TERM_TYPE_USER, config,
- (char *) sym->symbol, 0, 0, 0);
- else
- return new_term(term, PARSE_EVENTS__TERM_TYPE_STR,
- PARSE_EVENTS__TERM_TYPE_USER,
- (char *) "event", (char *) sym->symbol,
- 0, 0, 0);
+ return new_term(term, &temp, (char *) sym->symbol, 0);
}
int parse_events_term__clone(struct parse_events_term **new,
struct parse_events_term *term)
{
- return new_term(new, term->type_val, term->type_term, term->config,
- term->val.str, term->val.num,
- term->err_term, term->err_val);
+ struct parse_events_term temp = {
+ .type_val = term->type_val,
+ .type_term = term->type_term,
+ .config = term->config,
+ .err_term = term->err_term,
+ .err_val = term->err_val,
+ };
+
+ return new_term(new, &temp, term->val.str, term->val.num);
}
void parse_events_terms__purge(struct list_head *terms)
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 3/5] perf tools: Fail on using multiple bits long terms without value
2017-02-17 14:00 [PATCH 0/5] perf tools: Few fixes Jiri Olsa
2017-02-17 14:00 ` [PATCH 1/5] perf build: Add special fixdep cleaning rule Jiri Olsa
2017-02-17 14:00 ` [PATCH 2/5] perf tools: Move new_term arguments into struct parse_events_term template Jiri Olsa
@ 2017-02-17 14:00 ` Jiri Olsa
2017-02-21 8:14 ` [tip:perf/urgent] " tip-bot for Jiri Olsa
2017-02-17 14:00 ` [PATCH 4/5] perf stat: Add -a as a default target Jiri Olsa
2017-02-17 14:00 ` [PATCH 5/5] perf record: Add -a as a " Jiri Olsa
4 siblings, 1 reply; 30+ messages in thread
From: Jiri Olsa @ 2017-02-17 14:00 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: David Ahern, Namhyung Kim, Peter Zijlstra, lkml, Ingo Molnar
Currently we allow not to specify value for numeric terms
and we set them to value 1. This was originaly meant just
for single bit terms to allow user to type:
$ perf record -e 'cpu/cpu-cycles,any'
instead of:
$ perf record -e 'cpu/cpu-cycles,any=1'
However it works also for multi bits terms like:
$ perf record -e 'cpu/event/' ls
...
$ perf evlist -v
..., config: 0x1, ...
After discussion with Peter making such term usage
to fail, like:
$ perf record -e 'cpu/event/' ls
event syntax error: 'cpu/event/'
\___ no value assigned for term
...
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/n/tip-5vdq8gfbsva97vwrfmjl5p6f@git.kernel.org
---
tools/perf/util/parse-events.c | 2 ++
tools/perf/util/parse-events.h | 2 ++
tools/perf/util/parse-events.y | 14 +++++++-------
tools/perf/util/pmu.c | 13 +++++++++++--
4 files changed, 22 insertions(+), 9 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 984d99a8fdc5..67a8aebc67ab 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -2349,6 +2349,7 @@ static int new_term(struct parse_events_term **_term,
int parse_events_term__num(struct parse_events_term **term,
int type_term, char *config, u64 num,
+ bool no_value,
void *loc_term_, void *loc_val_)
{
YYLTYPE *loc_term = loc_term_;
@@ -2358,6 +2359,7 @@ int parse_events_term__num(struct parse_events_term **term,
.type_val = PARSE_EVENTS__TERM_TYPE_NUM,
.type_term = type_term,
.config = config,
+ .no_value = no_value,
.err_term = loc_term ? loc_term->first_column : 0,
.err_val = loc_val ? loc_val->first_column : 0,
};
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index da246a3ddb69..1af6a267c21b 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -94,6 +94,7 @@ struct parse_events_term {
int type_term;
struct list_head list;
bool used;
+ bool no_value;
/* error string indexes for within parsed string */
int err_term;
@@ -122,6 +123,7 @@ void parse_events__shrink_config_terms(void);
int parse_events__is_hardcoded_term(struct parse_events_term *term);
int parse_events_term__num(struct parse_events_term **term,
int type_term, char *config, u64 num,
+ bool novalue,
void *loc_term, void *loc_val);
int parse_events_term__str(struct parse_events_term **term,
int type_term, char *config, char *str,
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index a14b47ab3879..30f018ea1370 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -252,7 +252,7 @@ PE_KERNEL_PMU_EVENT sep_dc
if (!strcasecmp(alias->name, $1)) {
ALLOC_LIST(head);
ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
- $1, 1, &@1, NULL));
+ $1, 1, false, &@1, NULL));
list_add_tail(&term->list, head);
if (!parse_events_add_pmu(data, list,
@@ -282,7 +282,7 @@ PE_PMU_EVENT_PRE '-' PE_PMU_EVENT_SUF sep_dc
ALLOC_LIST(head);
ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
- &pmu_name, 1, &@1, NULL));
+ &pmu_name, 1, false, &@1, NULL));
list_add_tail(&term->list, head);
ALLOC_LIST(list);
@@ -548,7 +548,7 @@ PE_NAME '=' PE_VALUE
struct parse_events_term *term;
ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
- $1, $3, &@1, &@3));
+ $1, $3, false, &@1, &@3));
$$ = term;
}
|
@@ -566,7 +566,7 @@ PE_NAME
struct parse_events_term *term;
ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
- $1, 1, &@1, NULL));
+ $1, 1, true, &@1, NULL));
$$ = term;
}
|
@@ -591,7 +591,7 @@ PE_TERM '=' PE_VALUE
{
struct parse_events_term *term;
- ABORT_ON(parse_events_term__num(&term, (int)$1, NULL, $3, &@1, &@3));
+ ABORT_ON(parse_events_term__num(&term, (int)$1, NULL, $3, false, &@1, &@3));
$$ = term;
}
|
@@ -599,7 +599,7 @@ PE_TERM
{
struct parse_events_term *term;
- ABORT_ON(parse_events_term__num(&term, (int)$1, NULL, 1, &@1, NULL));
+ ABORT_ON(parse_events_term__num(&term, (int)$1, NULL, 1, true, &@1, NULL));
$$ = term;
}
|
@@ -620,7 +620,7 @@ PE_NAME array '=' PE_VALUE
struct parse_events_term *term;
ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
- $1, $4, &@1, &@4));
+ $1, $4, false, &@1, &@4));
term->array = $2;
$$ = term;
}
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 49bfee0e3d9e..63cb46cb9b0f 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -834,9 +834,18 @@ static int pmu_config_term(struct list_head *formats,
* Either directly use a numeric term, or try to translate string terms
* using event parameters.
*/
- if (term->type_val == PARSE_EVENTS__TERM_TYPE_NUM)
+ if (term->type_val == PARSE_EVENTS__TERM_TYPE_NUM) {
+ if (term->no_value &&
+ bitmap_weight(format->bits, PERF_PMU_FORMAT_BITS) > 1) {
+ if (err) {
+ err->idx = term->err_val;
+ err->str = strdup("no value assigned for term");
+ }
+ return -EINVAL;
+ }
+
val = term->val.num;
- else if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR) {
+ } else if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR) {
if (strcmp(term->val.str, "?")) {
if (verbose) {
pr_info("Invalid sysfs entry %s=%s\n",
--
2.7.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* [tip:perf/urgent] perf tools: Fail on using multiple bits long terms without value
2017-02-17 14:00 ` [PATCH 3/5] perf tools: Fail on using multiple bits long terms without value Jiri Olsa
@ 2017-02-21 8:14 ` tip-bot for Jiri Olsa
0 siblings, 0 replies; 30+ messages in thread
From: tip-bot for Jiri Olsa @ 2017-02-21 8:14 UTC (permalink / raw)
To: linux-tip-commits
Cc: dsahern, acme, mingo, linux-kernel, jolsa, hpa, a.p.zijlstra,
namhyung, tglx
Commit-ID: 99e7138eb7897aa0ccc6661173ae2d7e79721e05
Gitweb: http://git.kernel.org/tip/99e7138eb7897aa0ccc6661173ae2d7e79721e05
Author: Jiri Olsa <jolsa@kernel.org>
AuthorDate: Fri, 17 Feb 2017 15:00:56 +0100
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 17 Feb 2017 17:28:22 -0300
perf tools: Fail on using multiple bits long terms without value
Currently we allow not to specify value for numeric terms and we set
them to value 1. This was originaly meant just for single bit terms to
allow user to type:
$ perf record -e 'cpu/cpu-cycles,any'
instead of:
$ perf record -e 'cpu/cpu-cycles,any=1'
However it works also for multi bits terms like:
$ perf record -e 'cpu/event/' ls
...
$ perf evlist -v
..., config: 0x1, ...
After discussion with Peter we decided making such term usage to fail,
like:
$ perf record -e 'cpu/event/' ls
event syntax error: 'cpu/event/'
\___ no value assigned for term
...
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1487340058-10496-4-git-send-email-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/parse-events.c | 2 ++
tools/perf/util/parse-events.h | 2 ++
tools/perf/util/parse-events.y | 14 +++++++-------
tools/perf/util/pmu.c | 13 +++++++++++--
4 files changed, 22 insertions(+), 9 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 984d99a..67a8aeb 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -2349,6 +2349,7 @@ static int new_term(struct parse_events_term **_term,
int parse_events_term__num(struct parse_events_term **term,
int type_term, char *config, u64 num,
+ bool no_value,
void *loc_term_, void *loc_val_)
{
YYLTYPE *loc_term = loc_term_;
@@ -2358,6 +2359,7 @@ int parse_events_term__num(struct parse_events_term **term,
.type_val = PARSE_EVENTS__TERM_TYPE_NUM,
.type_term = type_term,
.config = config,
+ .no_value = no_value,
.err_term = loc_term ? loc_term->first_column : 0,
.err_val = loc_val ? loc_val->first_column : 0,
};
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index da246a3..1af6a26 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -94,6 +94,7 @@ struct parse_events_term {
int type_term;
struct list_head list;
bool used;
+ bool no_value;
/* error string indexes for within parsed string */
int err_term;
@@ -122,6 +123,7 @@ void parse_events__shrink_config_terms(void);
int parse_events__is_hardcoded_term(struct parse_events_term *term);
int parse_events_term__num(struct parse_events_term **term,
int type_term, char *config, u64 num,
+ bool novalue,
void *loc_term, void *loc_val);
int parse_events_term__str(struct parse_events_term **term,
int type_term, char *config, char *str,
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index a14b47a..30f018e 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -252,7 +252,7 @@ PE_KERNEL_PMU_EVENT sep_dc
if (!strcasecmp(alias->name, $1)) {
ALLOC_LIST(head);
ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
- $1, 1, &@1, NULL));
+ $1, 1, false, &@1, NULL));
list_add_tail(&term->list, head);
if (!parse_events_add_pmu(data, list,
@@ -282,7 +282,7 @@ PE_PMU_EVENT_PRE '-' PE_PMU_EVENT_SUF sep_dc
ALLOC_LIST(head);
ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
- &pmu_name, 1, &@1, NULL));
+ &pmu_name, 1, false, &@1, NULL));
list_add_tail(&term->list, head);
ALLOC_LIST(list);
@@ -548,7 +548,7 @@ PE_NAME '=' PE_VALUE
struct parse_events_term *term;
ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
- $1, $3, &@1, &@3));
+ $1, $3, false, &@1, &@3));
$$ = term;
}
|
@@ -566,7 +566,7 @@ PE_NAME
struct parse_events_term *term;
ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
- $1, 1, &@1, NULL));
+ $1, 1, true, &@1, NULL));
$$ = term;
}
|
@@ -591,7 +591,7 @@ PE_TERM '=' PE_VALUE
{
struct parse_events_term *term;
- ABORT_ON(parse_events_term__num(&term, (int)$1, NULL, $3, &@1, &@3));
+ ABORT_ON(parse_events_term__num(&term, (int)$1, NULL, $3, false, &@1, &@3));
$$ = term;
}
|
@@ -599,7 +599,7 @@ PE_TERM
{
struct parse_events_term *term;
- ABORT_ON(parse_events_term__num(&term, (int)$1, NULL, 1, &@1, NULL));
+ ABORT_ON(parse_events_term__num(&term, (int)$1, NULL, 1, true, &@1, NULL));
$$ = term;
}
|
@@ -620,7 +620,7 @@ PE_NAME array '=' PE_VALUE
struct parse_events_term *term;
ABORT_ON(parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
- $1, $4, &@1, &@4));
+ $1, $4, false, &@1, &@4));
term->array = $2;
$$ = term;
}
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 49bfee0..63cb46c 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -834,9 +834,18 @@ static int pmu_config_term(struct list_head *formats,
* Either directly use a numeric term, or try to translate string terms
* using event parameters.
*/
- if (term->type_val == PARSE_EVENTS__TERM_TYPE_NUM)
+ if (term->type_val == PARSE_EVENTS__TERM_TYPE_NUM) {
+ if (term->no_value &&
+ bitmap_weight(format->bits, PERF_PMU_FORMAT_BITS) > 1) {
+ if (err) {
+ err->idx = term->err_val;
+ err->str = strdup("no value assigned for term");
+ }
+ return -EINVAL;
+ }
+
val = term->val.num;
- else if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR) {
+ } else if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR) {
if (strcmp(term->val.str, "?")) {
if (verbose) {
pr_info("Invalid sysfs entry %s=%s\n",
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 4/5] perf stat: Add -a as a default target
2017-02-17 14:00 [PATCH 0/5] perf tools: Few fixes Jiri Olsa
` (2 preceding siblings ...)
2017-02-17 14:00 ` [PATCH 3/5] perf tools: Fail on using multiple bits long terms without value Jiri Olsa
@ 2017-02-17 14:00 ` Jiri Olsa
2017-02-17 14:27 ` Arnaldo Carvalho de Melo
2017-02-17 14:00 ` [PATCH 5/5] perf record: Add -a as a " Jiri Olsa
4 siblings, 1 reply; 30+ messages in thread
From: Jiri Olsa @ 2017-02-17 14:00 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: David Ahern, Namhyung Kim, Peter Zijlstra, Borislav Petkov, lkml,
Ingo Molnar
Boris asked for default -a option in case we monitor
only uncore events. While implementing that I thought
it might be actually useful to make it overall default.
# perf stat
Warning: No target specified, setting system-wide collection (-a).
...
Requested-by: Borislav Petkov <bp@alien8.de>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Borislav Petkov <bp@alien8.de>
Link: http://lkml.kernel.org/n/tip-tq11of2qlz8kxpxzva05d54l@git.kernel.org
---
tools/perf/Documentation/perf-stat.txt | 2 +-
tools/perf/builtin-stat.c | 6 ++++--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index d96ccd4844df..aecf2a87e7d6 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -63,7 +63,7 @@ report::
-a::
--all-cpus::
- system-wide collection from all CPUs
+ system-wide collection from all CPUs (default if no target is specified)
-c::
--scale::
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index f28719178b51..0d09ec7029d3 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -2445,8 +2445,10 @@ int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
} else if (big_num_opt == 0) /* User passed --no-big-num */
big_num = false;
- if (!argc && target__none(&target))
- usage_with_options(stat_usage, stat_options);
+ if (!argc && target__none(&target)) {
+ pr_warning("Warning: No target specified, setting system-wide collection (-a).\n");
+ target.system_wide = true;
+ }
if (run_count < 0) {
pr_err("Run count must be a positive number\n");
--
2.7.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH 4/5] perf stat: Add -a as a default target
2017-02-17 14:00 ` [PATCH 4/5] perf stat: Add -a as a default target Jiri Olsa
@ 2017-02-17 14:27 ` Arnaldo Carvalho de Melo
2017-02-17 14:33 ` Jiri Olsa
0 siblings, 1 reply; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-02-17 14:27 UTC (permalink / raw)
To: Jiri Olsa
Cc: David Ahern, Namhyung Kim, Peter Zijlstra, Borislav Petkov, lkml,
Ingo Molnar
Em Fri, Feb 17, 2017 at 03:00:57PM +0100, Jiri Olsa escreveu:
> Boris asked for default -a option in case we monitor
> only uncore events. While implementing that I thought
> it might be actually useful to make it overall default.
>
> # perf stat
> Warning: No target specified, setting system-wide collection (-a).
Humm, would be interesting to disable this after a few warnings? Just
one?
BTW, this is how 'perf trace' works since day one, i.e. no target means
system wide syscall tracing.
- Arnaldo
> ...
>
> Requested-by: Borislav Petkov <bp@alien8.de>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Borislav Petkov <bp@alien8.de>
> Link: http://lkml.kernel.org/n/tip-tq11of2qlz8kxpxzva05d54l@git.kernel.org
> ---
> tools/perf/Documentation/perf-stat.txt | 2 +-
> tools/perf/builtin-stat.c | 6 ++++--
> 2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
> index d96ccd4844df..aecf2a87e7d6 100644
> --- a/tools/perf/Documentation/perf-stat.txt
> +++ b/tools/perf/Documentation/perf-stat.txt
> @@ -63,7 +63,7 @@ report::
>
> -a::
> --all-cpus::
> - system-wide collection from all CPUs
> + system-wide collection from all CPUs (default if no target is specified)
>
> -c::
> --scale::
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index f28719178b51..0d09ec7029d3 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -2445,8 +2445,10 @@ int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
> } else if (big_num_opt == 0) /* User passed --no-big-num */
> big_num = false;
>
> - if (!argc && target__none(&target))
> - usage_with_options(stat_usage, stat_options);
> + if (!argc && target__none(&target)) {
> + pr_warning("Warning: No target specified, setting system-wide collection (-a).\n");
> + target.system_wide = true;
> + }
>
> if (run_count < 0) {
> pr_err("Run count must be a positive number\n");
> --
> 2.7.4
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 4/5] perf stat: Add -a as a default target
2017-02-17 14:27 ` Arnaldo Carvalho de Melo
@ 2017-02-17 14:33 ` Jiri Olsa
2017-02-17 14:41 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 30+ messages in thread
From: Jiri Olsa @ 2017-02-17 14:33 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, David Ahern, Namhyung Kim, Peter Zijlstra,
Borislav Petkov, lkml, Ingo Molnar
On Fri, Feb 17, 2017 at 11:27:47AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Feb 17, 2017 at 03:00:57PM +0100, Jiri Olsa escreveu:
> > Boris asked for default -a option in case we monitor
> > only uncore events. While implementing that I thought
> > it might be actually useful to make it overall default.
> >
> > # perf stat
> > Warning: No target specified, setting system-wide collection (-a).
>
> Humm, would be interesting to disable this after a few warnings? Just
> one?
not sure it's good idea to keep the count of that somewhere..
how about i make the warning smaller ;-)
# perf stat
Forced system wide target.
...
>
> BTW, this is how 'perf trace' works since day one, i.e. no target means
> system wide syscall tracing.
or we could omit the warning completely as probably perf trace does
thanks,
jirka
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 4/5] perf stat: Add -a as a default target
2017-02-17 14:33 ` Jiri Olsa
@ 2017-02-17 14:41 ` Arnaldo Carvalho de Melo
2017-02-17 14:43 ` Jiri Olsa
2017-02-17 17:00 ` [PATCHv2 " Jiri Olsa
0 siblings, 2 replies; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-02-17 14:41 UTC (permalink / raw)
To: Jiri Olsa
Cc: Jiri Olsa, David Ahern, Namhyung Kim, Peter Zijlstra,
Borislav Petkov, lkml, Ingo Molnar
Em Fri, Feb 17, 2017 at 03:33:27PM +0100, Jiri Olsa escreveu:
> On Fri, Feb 17, 2017 at 11:27:47AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Fri, Feb 17, 2017 at 03:00:57PM +0100, Jiri Olsa escreveu:
> > > Boris asked for default -a option in case we monitor
> > > only uncore events. While implementing that I thought
> > > it might be actually useful to make it overall default.
> > > # perf stat
> > > Warning: No target specified, setting system-wide collection (-a).
> > Humm, would be interesting to disable this after a few warnings? Just
> > one?
> not sure it's good idea to keep the count of that somewhere..
> how about i make the warning smaller ;-)
> # perf stat
> Forced system wide target.
> ...
> > BTW, this is how 'perf trace' works since day one, i.e. no target means
> > system wide syscall tracing.
> or we could omit the warning completely as probably perf trace does
I think that we should have some note on the Documentation (have you
added it?) and be done with it.
Another thing possiblity my mind, print that at the end? like:
perf record
^C[ perf record: Woken up 1 times to write data - system wide samples ]
[ perf record: Captured and wrote 1.738 MB perf.data (7565 samples) ]
----------
Then people will thing, hey, so now it does systemwide samples when I
pass no target, I don't have anymore to type _three_ keys! cool! :-)
- Arnaldo
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 4/5] perf stat: Add -a as a default target
2017-02-17 14:41 ` Arnaldo Carvalho de Melo
@ 2017-02-17 14:43 ` Jiri Olsa
2017-02-17 17:00 ` [PATCHv2 " Jiri Olsa
1 sibling, 0 replies; 30+ messages in thread
From: Jiri Olsa @ 2017-02-17 14:43 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, David Ahern, Namhyung Kim, Peter Zijlstra,
Borislav Petkov, lkml, Ingo Molnar
On Fri, Feb 17, 2017 at 11:41:28AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Feb 17, 2017 at 03:33:27PM +0100, Jiri Olsa escreveu:
> > On Fri, Feb 17, 2017 at 11:27:47AM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Fri, Feb 17, 2017 at 03:00:57PM +0100, Jiri Olsa escreveu:
> > > > Boris asked for default -a option in case we monitor
> > > > only uncore events. While implementing that I thought
> > > > it might be actually useful to make it overall default.
>
> > > > # perf stat
> > > > Warning: No target specified, setting system-wide collection (-a).
>
> > > Humm, would be interesting to disable this after a few warnings? Just
> > > one?
>
> > not sure it's good idea to keep the count of that somewhere..
> > how about i make the warning smaller ;-)
>
> > # perf stat
> > Forced system wide target.
> > ...
>
> > > BTW, this is how 'perf trace' works since day one, i.e. no target means
> > > system wide syscall tracing.
>
> > or we could omit the warning completely as probably perf trace does
>
> I think that we should have some note on the Documentation (have you
> added it?) and be done with it.
>
> Another thing possiblity my mind, print that at the end? like:
>
> perf record
> ^C[ perf record: Woken up 1 times to write data - system wide samples ]
> [ perf record: Captured and wrote 1.738 MB perf.data (7565 samples) ]
>
> ----------
>
> Then people will thing, hey, so now it does systemwide samples when I
> pass no target, I don't have anymore to type _three_ keys! cool! :-)
ok, I'll check on that and send new version for the last 2 patches
thanks,
jirka
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCHv2 4/5] perf stat: Add -a as a default target
2017-02-17 14:41 ` Arnaldo Carvalho de Melo
2017-02-17 14:43 ` Jiri Olsa
@ 2017-02-17 17:00 ` Jiri Olsa
2017-02-17 17:48 ` Boris Petkov
2017-02-21 8:14 ` [tip:perf/urgent] perf stat: Add -a as " tip-bot for Jiri Olsa
1 sibling, 2 replies; 30+ messages in thread
From: Jiri Olsa @ 2017-02-17 17:00 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, David Ahern, Namhyung Kim, Peter Zijlstra,
Borislav Petkov, lkml, Ingo Molnar
On Fri, Feb 17, 2017 at 11:41:28AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Feb 17, 2017 at 03:33:27PM +0100, Jiri Olsa escreveu:
> > On Fri, Feb 17, 2017 at 11:27:47AM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Fri, Feb 17, 2017 at 03:00:57PM +0100, Jiri Olsa escreveu:
> > > > Boris asked for default -a option in case we monitor
> > > > only uncore events. While implementing that I thought
> > > > it might be actually useful to make it overall default.
>
> > > > # perf stat
> > > > Warning: No target specified, setting system-wide collection (-a).
>
> > > Humm, would be interesting to disable this after a few warnings? Just
> > > one?
>
> > not sure it's good idea to keep the count of that somewhere..
> > how about i make the warning smaller ;-)
>
> > # perf stat
> > Forced system wide target.
> > ...
>
> > > BTW, this is how 'perf trace' works since day one, i.e. no target means
> > > system wide syscall tracing.
>
> > or we could omit the warning completely as probably perf trace does
>
> I think that we should have some note on the Documentation (have you
> added it?) and be done with it.
>
> Another thing possiblity my mind, print that at the end? like:
>
> perf record
> ^C[ perf record: Woken up 1 times to write data - system wide samples ]
> [ perf record: Captured and wrote 1.738 MB perf.data (7565 samples) ]
>
> ----------
>
> Then people will thing, hey, so now it does systemwide samples when I
> pass no target, I don't have anymore to type _three_ keys! cool! :-)
ok, posting the change with no warning
jirka
---
Boris asked for default -a option in case we monitor
only uncore events. While implementing that I thought
it might be actually useful to make it overall default.
Running 'perf stat' will now collect system wide data.
Requested-by: Borislav Petkov <bp@alien8.de>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/n/tip-tq11of2qlz8kxpxzva05d54l@git.kernel.org
---
tools/perf/Documentation/perf-stat.txt | 2 +-
tools/perf/builtin-stat.c | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index d96ccd4844df..aecf2a87e7d6 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -63,7 +63,7 @@ report::
-a::
--all-cpus::
- system-wide collection from all CPUs
+ system-wide collection from all CPUs (default if no target is specified)
-c::
--scale::
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index f28719178b51..70ed4f9d014e 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -2445,8 +2445,9 @@ int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
} else if (big_num_opt == 0) /* User passed --no-big-num */
big_num = false;
+ /* Make system wide (-a) the default target. */
if (!argc && target__none(&target))
- usage_with_options(stat_usage, stat_options);
+ target.system_wide = true;
if (run_count < 0) {
pr_err("Run count must be a positive number\n");
--
2.7.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCHv2 4/5] perf stat: Add -a as a default target
2017-02-17 17:00 ` [PATCHv2 " Jiri Olsa
@ 2017-02-17 17:48 ` Boris Petkov
2017-02-18 17:52 ` Borislav Petkov
2017-02-21 8:14 ` [tip:perf/urgent] perf stat: Add -a as " tip-bot for Jiri Olsa
1 sibling, 1 reply; 30+ messages in thread
From: Boris Petkov @ 2017-02-17 17:48 UTC (permalink / raw)
To: Jiri Olsa, Arnaldo Carvalho de Melo
Cc: Jiri Olsa, David Ahern, Namhyung Kim, Peter Zijlstra, lkml, Ingo Molnar
On February 17, 2017 6:00:34 PM GMT+01:00, Jiri Olsa <jolsa@redhat.com> wrote:
>---
>Boris asked for default -a option in case we monitor
>only uncore events. While implementing that I thought
>it might be actually useful to make it overall default.
>
>Running 'perf stat' will now collect system wide data.
>
>Requested-by: Borislav Petkov <bp@alien8.de>
>Signed-off-by: Jiri Olsa <jolsa@kernel.org>
>Cc: David Ahern <dsahern@gmail.com>
>Cc: Namhyung Kim <namhyung@kernel.org>
>Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
>Link:
>http://lkml.kernel.org/n/tip-tq11of2qlz8kxpxzva05d54l@git.kernel.org
LGTM.
Acked-by: me
Thanks.
--
Sent from a small device: formatting sux and brevity is inevitable.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCHv2 4/5] perf stat: Add -a as a default target
2017-02-17 17:48 ` Boris Petkov
@ 2017-02-18 17:52 ` Borislav Petkov
2017-02-20 7:13 ` Jiri Olsa
0 siblings, 1 reply; 30+ messages in thread
From: Borislav Petkov @ 2017-02-18 17:52 UTC (permalink / raw)
To: Jiri Olsa, Arnaldo Carvalho de Melo
Cc: Jiri Olsa, David Ahern, Namhyung Kim, Peter Zijlstra, lkml, Ingo Molnar
On Fri, Feb 17, 2017 at 06:48:13PM +0100, Boris Petkov wrote:
> LGTM.
>
> Acked-by: me
Well, it looks good but actually trying it is a different story. For
example:
$ ./perf stat -e amd_nb/event=0xe0,umask=0x1f/ sleep 1
still says <not supported> because argc is not 0.
So how about the below diff instead?
$ ./perf stat -e amd_nb/event=0xe0,umask=0x1f/
without args dumps the usage message and
$ ./perf stat -e amd_nb/event=0xe0,umask=0x1f/ sleep 1
actually does the system-wide thing:
Performance counter stats for 'system wide':
196,469 amd_nb/event=0xe0,umask=0x1f/
1.001815180 seconds time elapsed
Hmmm?
tools/perf/Documentation/perf-stat.txt | 2 +-
tools/perf/builtin-stat.c | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
Index: linux/tools/perf/Documentation/perf-stat.txt
===================================================================
--- linux.orig/tools/perf/Documentation/perf-stat.txt 2017-02-18 18:38:23.751960730 +0100
+++ linux/tools/perf/Documentation/perf-stat.txt 2017-02-18 18:38:23.743960730 +0100
@@ -63,7 +63,7 @@ report::
-a::
--all-cpus::
- system-wide collection from all CPUs
+ system-wide collection from all CPUs (default if no target is specified)
-c::
--scale::
Index: linux/tools/perf/builtin-stat.c
===================================================================
--- linux.orig/tools/perf/builtin-stat.c 2017-02-18 18:38:23.751960730 +0100
+++ linux/tools/perf/builtin-stat.c 2017-02-18 18:48:33.531959828 +0100
@@ -2445,8 +2445,12 @@ int cmd_stat(int argc, const char **argv
} else if (big_num_opt == 0) /* User passed --no-big-num */
big_num = false;
- if (!argc && target__none(&target))
- usage_with_options(stat_usage, stat_options);
+ /* Make system wide (-a) the default target. */
+ if (target__none(&target)) {
+ if (!argc)
+ usage_with_options(stat_usage, stat_options);
+ target.system_wide = true;
+ }
if (run_count < 0) {
pr_err("Run count must be a positive number\n");
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCHv2 4/5] perf stat: Add -a as a default target
2017-02-18 17:52 ` Borislav Petkov
@ 2017-02-20 7:13 ` Jiri Olsa
[not found] ` <20170220134433.GI4109@kernel.org>
0 siblings, 1 reply; 30+ messages in thread
From: Jiri Olsa @ 2017-02-20 7:13 UTC (permalink / raw)
To: Borislav Petkov
Cc: Arnaldo Carvalho de Melo, Jiri Olsa, David Ahern, Namhyung Kim,
Peter Zijlstra, lkml, Ingo Molnar
On Sat, Feb 18, 2017 at 06:52:25PM +0100, Borislav Petkov wrote:
> On Fri, Feb 17, 2017 at 06:48:13PM +0100, Boris Petkov wrote:
> > LGTM.
> >
> > Acked-by: me
>
> Well, it looks good but actually trying it is a different story. For
> example:
>
> $ ./perf stat -e amd_nb/event=0xe0,umask=0x1f/ sleep 1
>
> still says <not supported> because argc is not 0.
>
> So how about the below diff instead?
>
> $ ./perf stat -e amd_nb/event=0xe0,umask=0x1f/
>
> without args dumps the usage message and
>
> $ ./perf stat -e amd_nb/event=0xe0,umask=0x1f/ sleep 1
>
> actually does the system-wide thing:
>
> Performance counter stats for 'system wide':
>
> 196,469 amd_nb/event=0xe0,umask=0x1f/
>
> 1.001815180 seconds time elapsed
>
> Hmmm?
ugh, I thought it was too easy ;-)
it looks good to me
thanks,
jirka
^ permalink raw reply [flat|nested] 30+ messages in thread
* [tip:perf/urgent] perf stat: Add -a as default target
2017-02-17 17:00 ` [PATCHv2 " Jiri Olsa
2017-02-17 17:48 ` Boris Petkov
@ 2017-02-21 8:14 ` tip-bot for Jiri Olsa
1 sibling, 0 replies; 30+ messages in thread
From: tip-bot for Jiri Olsa @ 2017-02-21 8:14 UTC (permalink / raw)
To: linux-tip-commits
Cc: tglx, a.p.zijlstra, dsahern, namhyung, acme, jolsa, linux-kernel,
bp, jolsa, mingo, hpa
Commit-ID: 0d79f8b93187c771b6971acfaba67f4e2f1e0710
Gitweb: http://git.kernel.org/tip/0d79f8b93187c771b6971acfaba67f4e2f1e0710
Author: Jiri Olsa <jolsa@redhat.com>
AuthorDate: Fri, 17 Feb 2017 18:00:34 +0100
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 17 Feb 2017 17:31:10 -0300
perf stat: Add -a as default target
Boris asked for default -a option in case we monitor only uncore events.
While implementing that I thought it might be actually useful to make it
overall default.
Running 'perf stat' will now collect system wide data.
Committer note:
Testing it:
# perf stat
^C
Performance counter stats for 'system wide':
3571.559178 cpu-clock (msec) # 4.000 CPUs utilized
3,346 context-switches # 0.937 K/sec
277 cpu-migrations # 0.078 K/sec
57,271 page-faults # 0.016 M/sec
4,535,633,835 cycles # 1.270 GHz
6,389,736,516 instructions # 1.41 insn per cycle
1,541,293,875 branches # 431.547 M/sec
14,526,396 branch-misses # 0.94% of all branches
0.892950118 seconds time elapsed
#
Requested-and-Acked-by: Borislav Petkov <bp@alien8.de>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20170217170034.GB15389@krava
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/Documentation/perf-stat.txt | 2 +-
tools/perf/builtin-stat.c | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index d96ccd4..aecf2a8 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -63,7 +63,7 @@ report::
-a::
--all-cpus::
- system-wide collection from all CPUs
+ system-wide collection from all CPUs (default if no target is specified)
-c::
--scale::
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index ca27a8a..9989b03 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -2445,8 +2445,9 @@ int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
} else if (big_num_opt == 0) /* User passed --no-big-num */
big_num = false;
+ /* Make system wide (-a) the default target. */
if (!argc && target__none(&target))
- usage_with_options(stat_usage, stat_options);
+ target.system_wide = true;
if (run_count < 0) {
pr_err("Run count must be a positive number\n");
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 5/5] perf record: Add -a as a default target
2017-02-17 14:00 [PATCH 0/5] perf tools: Few fixes Jiri Olsa
` (3 preceding siblings ...)
2017-02-17 14:00 ` [PATCH 4/5] perf stat: Add -a as a default target Jiri Olsa
@ 2017-02-17 14:00 ` Jiri Olsa
2017-02-17 14:28 ` Arnaldo Carvalho de Melo
4 siblings, 1 reply; 30+ messages in thread
From: Jiri Olsa @ 2017-02-17 14:00 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: David Ahern, Namhyung Kim, Peter Zijlstra, lkml, Ingo Molnar
Adding system wide (-a) option as a default target
if non is specified.
# perf record
Warning: No target specified, setting system-wide collection (-a).
...
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/n/tip-tq11of2qlz8kxpxzva05d54l@git.kernel.org
---
tools/perf/Documentation/perf-record.txt | 2 +-
tools/perf/builtin-record.c | 6 ++++--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 27256bc68eda..b16003ec14a7 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -157,7 +157,7 @@ OPTIONS
-a::
--all-cpus::
- System-wide collection from all CPUs.
+ System-wide collection from all CPUs (default if no target is specified).
-p::
--pid=::
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 6cd6776052e7..dab23f6b10f3 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1677,8 +1677,10 @@ 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);
- if (!argc && target__none(&rec->opts.target))
- usage_with_options(record_usage, record_options);
+ if (!argc && target__none(&rec->opts.target)) {
+ pr_warning("Warning: No target specified, setting system-wide collection (-a).\n");
+ rec->opts.target.system_wide = true;
+ }
if (nr_cgroups && !rec->opts.target.system_wide) {
usage_with_options_msg(record_usage, record_options,
--
2.7.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH 5/5] perf record: Add -a as a default target
2017-02-17 14:00 ` [PATCH 5/5] perf record: Add -a as a " Jiri Olsa
@ 2017-02-17 14:28 ` Arnaldo Carvalho de Melo
2017-02-17 17:00 ` Jiri Olsa
0 siblings, 1 reply; 30+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-02-17 14:28 UTC (permalink / raw)
To: Jiri Olsa; +Cc: David Ahern, Namhyung Kim, Peter Zijlstra, lkml, Ingo Molnar
Em Fri, Feb 17, 2017 at 03:00:58PM +0100, Jiri Olsa escreveu:
> Adding system wide (-a) option as a default target
> if non is specified.
>
> # perf record
> Warning: No target specified, setting system-wide collection (-a).
Ditto, this warning will get annoying after a while...
> ...
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Link: http://lkml.kernel.org/n/tip-tq11of2qlz8kxpxzva05d54l@git.kernel.org
> ---
> tools/perf/Documentation/perf-record.txt | 2 +-
> tools/perf/builtin-record.c | 6 ++++--
> 2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
> index 27256bc68eda..b16003ec14a7 100644
> --- a/tools/perf/Documentation/perf-record.txt
> +++ b/tools/perf/Documentation/perf-record.txt
> @@ -157,7 +157,7 @@ OPTIONS
>
> -a::
> --all-cpus::
> - System-wide collection from all CPUs.
> + System-wide collection from all CPUs (default if no target is specified).
>
> -p::
> --pid=::
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 6cd6776052e7..dab23f6b10f3 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -1677,8 +1677,10 @@ 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);
> - if (!argc && target__none(&rec->opts.target))
> - usage_with_options(record_usage, record_options);
> + if (!argc && target__none(&rec->opts.target)) {
> + pr_warning("Warning: No target specified, setting system-wide collection (-a).\n");
> + rec->opts.target.system_wide = true;
> + }
>
> if (nr_cgroups && !rec->opts.target.system_wide) {
> usage_with_options_msg(record_usage, record_options,
> --
> 2.7.4
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 5/5] perf record: Add -a as a default target
2017-02-17 14:28 ` Arnaldo Carvalho de Melo
@ 2017-02-17 17:00 ` Jiri Olsa
2017-02-21 8:15 ` [tip:perf/urgent] perf record: Add -a as " tip-bot for Jiri Olsa
0 siblings, 1 reply; 30+ messages in thread
From: Jiri Olsa @ 2017-02-17 17:00 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, David Ahern, Namhyung Kim, Peter Zijlstra, lkml, Ingo Molnar
On Fri, Feb 17, 2017 at 11:28:14AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Feb 17, 2017 at 03:00:58PM +0100, Jiri Olsa escreveu:
> > Adding system wide (-a) option as a default target
> > if non is specified.
> >
> > # perf record
> > Warning: No target specified, setting system-wide collection (-a).
>
> Ditto, this warning will get annoying after a while...
ok, posting the change with no warning
jirka
---
Adding system wide (-a) option as a default target
if non is specified.
Running 'perf record' will now collect system wide data.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/n/tip-tq11of2qlz8kxpxzva05d54l@git.kernel.org
---
tools/perf/Documentation/perf-record.txt | 2 +-
tools/perf/builtin-record.c | 4 +++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 27256bc68eda..b16003ec14a7 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -157,7 +157,7 @@ OPTIONS
-a::
--all-cpus::
- System-wide collection from all CPUs.
+ System-wide collection from all CPUs (default if no target is specified).
-p::
--pid=::
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 6cd6776052e7..b87bbef73394 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1677,8 +1677,10 @@ 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);
+
+ /* Make system wide (-a) the default target. */
if (!argc && target__none(&rec->opts.target))
- usage_with_options(record_usage, record_options);
+ rec->opts.target.system_wide = true;
if (nr_cgroups && !rec->opts.target.system_wide) {
usage_with_options_msg(record_usage, record_options,
--
2.7.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* [tip:perf/urgent] perf record: Add -a as default target
2017-02-17 17:00 ` Jiri Olsa
@ 2017-02-21 8:15 ` tip-bot for Jiri Olsa
0 siblings, 0 replies; 30+ messages in thread
From: tip-bot for Jiri Olsa @ 2017-02-21 8:15 UTC (permalink / raw)
To: linux-tip-commits
Cc: jolsa, bp, namhyung, jolsa, dsahern, acme, linux-kernel, hpa,
mingo, tglx, a.p.zijlstra
Commit-ID: 483635a9d0802d5ffbe402ceac5b93ddb2acb138
Gitweb: http://git.kernel.org/tip/483635a9d0802d5ffbe402ceac5b93ddb2acb138
Author: Jiri Olsa <jolsa@redhat.com>
AuthorDate: Fri, 17 Feb 2017 18:00:18 +0100
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 17 Feb 2017 17:32:38 -0300
perf record: Add -a as default target
Running 'perf record' with no target (-a, -p, -t, etc) will now collect
system wide data.
Commiter notes:
Testing it:
[root@jouet ~]# perf record
^C[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 1.351 MB perf.data (366 samples) ]
#
is equivalent to:
# perf record -a
^C[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 1.411 MB perf.data (978 samples) ]
#
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20170217170018.GA15389@krava
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/Documentation/perf-record.txt | 2 +-
tools/perf/builtin-record.c | 4 +++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 27256bc..b16003e 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -157,7 +157,7 @@ OPTIONS
-a::
--all-cpus::
- System-wide collection from all CPUs.
+ System-wide collection from all CPUs (default if no target is specified).
-p::
--pid=::
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 6cd6776..b87bbef 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1677,8 +1677,10 @@ 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);
+
+ /* Make system wide (-a) the default target. */
if (!argc && target__none(&rec->opts.target))
- usage_with_options(record_usage, record_options);
+ rec->opts.target.system_wide = true;
if (nr_cgroups && !rec->opts.target.system_wide) {
usage_with_options_msg(record_usage, record_options,
^ permalink raw reply related [flat|nested] 30+ messages in thread