perf/core improvements and fixes

Hi Ingo,

Please consider pulling,

- Arnaldo

The following changes since commit 0d76ded582c178d3cca55c9112eceb5b0f12f558:

Merge tag 'perf-core-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core (2015-12-14 09:31:39 +0100)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-core-for-mingo

for you to fetch changes up to 7efe0e034c713716060bc7794c7e332589980c70:

perf record: Support custom vmlinux path (2015-12-14 13:04:12 -0300)

----------------------------------------------------------------
perf/core improvements and fixes:

User visible:

- Make command line options always available, even when they depend on some feature being enabled, warning the user about use of such options (Wang Nan)

- Support --vmlinux in perf record, useful, so far, for eBPF, where we will set up events that will be used in the record session (He Kuang)

- Automatically disable collecting branch flags and cycles with --call-graph lbr. This allows avoiding a bunch of extra MSR reads in the PMI on Skylake. (Andi Kleen)

Infrastructure:

- Dump the stack when a 'perf test -v ' entry segfaults, so far we would have to run it under gdb with 'set follow-fork-mode child' set to get a proper backtrace (Arnaldo Carvalho de Melo)

- Initialize the refcnt in 'struct thread' to 1 and fixup its users accordingly, so that we try to have the same refcount model accross the perf codebase (Arnaldo Carvalho de Melo)

- More prep work for moving the subcmd infrastructure out of tools/perf/ and into tools/lib/subcmd/ to be used by other tools/ living utilities (Josh Poimboeuf)

- Fix 'perf test' hist testcases when kptr_restrict is on (Namhyung Kim)

----------------------------------------------------------------
Andi Kleen (1):
perf evsel: Disable branch flags/cycles for --callgraph lbr

Arnaldo Carvalho de Melo (3):
perf tools: Use same signal handling strategy as 'record' perf test: Dump the stack when test segfaults when in verbose mode perf thread: Fix reference count initial state

He Kuang (1):
perf record: Support custom vmlinux path

Josh Poimboeuf (9):
perf build: Remove unnecessary line in Makefile.feature perf test: Add Build file to dependencies for llvm-src-*.c perf test: Remove tarpkg at end of test
perf build: Fix 'make clean'
perf build: Rename LIB_PATH -> API_PATH
perf tools: Create pager.h
perf tools: Remove check for unused PERF_PAGER_IN_USE perf tools: Move help_unknown_cmd() to its own file perf tools: Convert parse-options.c internal functions to static

Namhyung Kim (1):
perf test: Fix hist testcases when kptr_restrict is on

Wang Nan (1):
perf tools: Make options always available, even if required libs not linked

 tools/build/Makefile.feature             |   1 -
 tools/perf/Documentation/perf-record.txt |  10 ++-
 tools/perf/Makefile.perf                 |  15 ++--
 tools/perf/builtin-probe.c               |  15 +++-
 tools/perf/builtin-record.c              |  25 +++++-
 tools/perf/tests/Build                   |   6 +-
 tools/perf/tests/builtin-test.c          |   3 +
 tools/perf/tests/hists_common.c          |   5 +-
 tools/perf/tests/make                    |   3 +-
 tools/perf/util/Build                    |   1 +
 tools/perf/util/cache.h                  |   5 +-
 tools/perf/util/evsel.c                  |  14 +++-
 tools/perf/util/help-unknown-cmd.c       | 103 +++++++++++++++++++++++
 tools/perf/util/help-unknown-cmd.h       |   0
 tools/perf/util/help.c                   | 107 +-----------------------
 tools/perf/util/help.h                   |   3 +
 tools/perf/util/intel-pt.c               |   4 +-
 tools/perf/util/machine.c                |  19 +++--
 tools/perf/util/pager.c                  |   8 +-
 tools/perf/util/pager.h                  |   7 ++
 tools/perf/util/parse-options.c          | 136 +++++++++++++++++++++++++++----
 tools/perf/util/parse-options.h          |  14 ++--
 tools/perf/util/thread.c                 |  10 ++-
 tools/perf/util/util.c                   |   3 +-
 24 files changed, 341 insertions(+), 176 deletions(-)
 create mode 100644 tools/perf/util/help-unknown-cmd.c
 create mode 100644 tools/perf/util/help-unknown-cmd.h
 create mode 100644 tools/perf/util/pager.h

From: Arnaldo Carvalho de Melo <[email protected]>

I.e. don't exit with the signal number, instead set the signal handler to the default one and then raise it again.

Noticed while trying to dump the stack at segfaults in the 'perf test' forked process used to run each test, that inspects signal info at each test.

---
 tools/perf/util/util.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

index 07da970a62a3..aff0cfd83662 100644
@@ -352,7 +352,8 @@ void sighandler_dump_stack(int sig)
 {
 	psignal(sig, "perf");
 	dump_stack();
-	exit(sig);
+	signal(sig, SIG_DFL);
+	raise(sig);
 }
 
 int parse_nsec_time(const char *str, u64 *ptime)

2.1.0

From: Arnaldo Carvalho de Melo <[email protected]>

E.g.:

  # perf test 26
  26: Test mmap thread lookup                                  : FAILED!
  # perf test -v 26
  26: Test mmap thread lookup                                  :
  --- start ---
  test child forked, pid 9269
  tid = 9269, map = 0x7ff99ff0c000
  tid = 9270, map = 0x7ff99ff0b000
  tid = 9271, map = 0x7ff99ff0a000
  tid = 9272, map = 0x7ff99ff09000
  perf: Segmentation fault
  Obtained 13 stack frames.
  perf(sighandler_dump_stack+0x41) [0x4e3541]
  /lib64/libc.so.6(+0x34960) [0x7ff99d5f6960]
  perf(thread__put+0x5b) [0x4c6f6b]
  perf(machine__process_event+0x14e) [0x4bd37e]
  perf(perf_event__synthesize_threads+0x3aa) [0x48678a]
  perf(test__mmap_thread_lookup+0x20a) [0x474e0a]
  perf() [0x460d56]
  perf(cmd_test+0x589) [0x461319]
  perf() [0x47c641]
  perf(main+0x617) [0x422317]
  /lib64/libc.so.6(__libc_start_main+0xf0) [0x7ff99d5e1fe0]
  perf() [0x422429]
  [(nil)]
  test child interrupted
  ---- end ----
  Test mmap thread lookup: FAILED!
  #

---
 tools/perf/tests/builtin-test.c | 3 +++
 1 file changed, 3 insertions(+)

index 2b1ade1aafc3..fa98406c92e2 100644
@@ -236,6 +236,9 @@ static int run_test(struct test *test, int subtest)
 				dup2(STDOUT_FILENO, STDERR_FILENO);
 				close(nullfd);
 			}
+		} else {
+			signal(SIGSEGV, sighandler_dump_stack);
+			signal(SIGFPE, sighandler_dump_stack);
 		}
 
 		err = test->func(subtest);

2.1.0

From: Arnaldo Carvalho de Melo <[email protected]>

We should always return from thread__new(), the constructor, with the object with a reference count of one, so that:

     struct thread *thread = thread__new();
     thread__put(thread);

Will call thread__delete().

If any reference is made to that 'thread' variable, it better use thread__get(thread) to hold a reference.

We were returning with thread->refcnt set to zero, fix it and some cases where thread__delete() was being called, which were not a problem because just one reference was being used, now that we set it to 1, use thread__put() instead.

---
 tools/perf/util/intel-pt.c |  4 ++--
 tools/perf/util/machine.c  | 19 ++++++++++++-------
 tools/perf/util/thread.c   | 10 ++++++++--
 3 files changed, 22 insertions(+), 11 deletions(-)

index 97f963a3dcb9..81a2eb77ba7f 100644
@@ -1744,7 +1744,7 @@ static void intel_pt_free(struct perf_session *session)
 	auxtrace_heap__free(&pt->heap);
 	intel_pt_free_events(session);
 	session->auxtrace = NULL;
-	thread__delete(pt->unknown_thread);
+	thread__put(pt->unknown_thread);
 	free(pt);
 }
 

@@ -2153,7 +2153,7 @@ int intel_pt_process_auxtrace_info(union perf_event *event,
 	return 0;
 
 err_delete_thread:
-	thread__delete(pt->unknown_thread);
+	thread__zput(pt->unknown_thread);
 err_free_queues:
 	intel_pt_log_disable();
 	auxtrace_queues__free(&pt->queues);

index 1407d5107480..ad79297c76c8 100644
@@ -352,13 +352,18 @@ static void machine__update_thread_pid(struct machine *machine,
 	}
 
 	th->mg = map_groups__get(leader->mg);
-
+out_put:
+	thread__put(leader);
 	return;
-
 out_err:
 	pr_err("Failed to join map groups for %d:%d\n", th->pid_, th->tid);
+	goto out_put;
 }
 
+/*
+ * Caller must eventually drop thread->refcnt returned with a successfull
+ * lookup/new thread inserted.
+ */
 static struct thread *____machine__findnew_thread(struct machine *machine,
 						  pid_t pid, pid_t tid,
 						  bool create)

@@ -376,7 +381,7 @@ static struct thread *____machine__findnew_thread(struct machine *machine,
 	if (th != NULL) {
 		if (th->tid == tid) {
 			machine__update_thread_pid(machine, th, pid);
-			return th;
+			return thread__get(th);
 		}
 
 		machine->last_match = NULL;

@@ -389,7 +394,7 @@ static struct thread *____machine__findnew_thread(struct machine *machine,
 		if (th->tid == tid) {
 			machine->last_match = th;
 			machine__update_thread_pid(machine, th, pid);
-			return th;
+			return thread__get(th);
 		}
 
 		if (tid < th->tid)

@@ -417,7 +422,7 @@ static struct thread *____machine__findnew_thread(struct machine *machine,
 		if (thread__init_map_groups(th, machine)) {
 			rb_erase_init(&th->rb_node, &machine->threads);
 			RB_CLEAR_NODE(&th->rb_node);
-			thread__delete(th);
+			thread__put(th);
 			return NULL;
 		}
 		/*

@@ -441,7 +446,7 @@ struct thread *machine__findnew_thread(struct machine *machine, pid_t pid,
 	struct thread *th;
 
 	pthread_rwlock_wrlock(&machine->threads_lock);
-	th = thread__get(__machine__findnew_thread(machine, pid, tid));
+	th = __machine__findnew_thread(machine, pid, tid);
 	pthread_rwlock_unlock(&machine->threads_lock);
 	return th;
 }

@@ -451,7 +456,7 @@ struct thread *machine__find_thread(struct machine *machine, pid_t pid,
 {
 	struct thread *th;
 	pthread_rwlock_rdlock(&machine->threads_lock);
-	th =  thread__get(____machine__findnew_thread(machine, pid, tid, false));
+	th =  ____machine__findnew_thread(machine, pid, tid, false);
 	pthread_rwlock_unlock(&machine->threads_lock);
 	return th;
 }

index 0a9ae8014729..dfd00c6dad6e 100644
@@ -19,8 +19,10 @@ int thread__init_map_groups(struct thread *thread, struct machine *machine)
 		thread->mg = map_groups__new(machine);
 	} else {
 		leader = __machine__findnew_thread(machine, pid, pid);
-		if (leader)
+		if (leader) {
 			thread->mg = map_groups__get(leader->mg);
+			thread__put(leader);
+		}
 	}
 
 	return thread->mg ? 0 : -1;

@@ -53,7 +55,7 @@ struct thread *thread__new(pid_t pid, pid_t tid)
 			goto err_thread;
 
 		list_add(&comm->list, &thread->comm_list);
-		atomic_set(&thread->refcnt, 0);
+		atomic_set(&thread->refcnt, 1);
 		RB_CLEAR_NODE(&thread->rb_node);
 	}
 

@@ -95,6 +97,10 @@ struct thread *thread__get(struct thread *thread)
 void thread__put(struct thread *thread)
 {
 	if (thread && atomic_dec_and_test(&thread->refcnt)) {
+		/*
+		 * Remove it from the dead_threads list, as last reference
+		 * is gone.
+		 */
 		list_del_init(&thread->node);
 		thread__delete(thread);
 	}

2.1.0

From: Andi Kleen <[email protected]>

[The kernel patch needed for this is in tip now (b16a5b52eb9 perf/x86:
Add option to disable ...) So this user tools patch to make use of it should be merged now]

Automatically disable collecting branch flags and cycles with --call-graph lbr. This allows avoiding a bunch of extra MSR reads in the PMI on Skylake.

When the kernel doesn't support the new flags they are automatically cleared in the fallback code.

v2: Switch to use branch_sample_type instead of sample_type. Adjust description.
Fix the fallback logic.

---
 tools/perf/util/evsel.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

index 47f033089349..544e4400de13 100644
@@ -36,6 +36,7 @@ static struct {
 	bool cloexec;
 	bool clockid;
 	bool clockid_wrong;
+	bool lbr_flags;
 } perf_missing_features;
 
 static clockid_t clockid;

@@ -574,7 +575,9 @@ perf_evsel__config_callgraph(struct perf_evsel *evsel,
 			} else {
 				perf_evsel__set_sample_bit(evsel, BRANCH_STACK);
 				attr->branch_sample_type = PERF_SAMPLE_BRANCH_USER |
-							PERF_SAMPLE_BRANCH_CALL_STACK;
+							PERF_SAMPLE_BRANCH_CALL_STACK |
+							PERF_SAMPLE_BRANCH_NO_CYCLES |
+							PERF_SAMPLE_BRANCH_NO_FLAGS;
 			}
 		} else
 			 pr_warning("Cannot use LBR callstack with branch stack. "

@@ -1337,6 +1340,9 @@ fallback_missing_features:
 		evsel->attr.mmap2 = 0;
 	if (perf_missing_features.exclude_guest)
 		evsel->attr.exclude_guest = evsel->attr.exclude_host = 0;
+	if (perf_missing_features.lbr_flags)
+		evsel->attr.branch_sample_type &= ~(PERF_SAMPLE_BRANCH_NO_FLAGS |
+				     PERF_SAMPLE_BRANCH_NO_CYCLES);
 retry_sample_id:
 	if (perf_missing_features.sample_id_all)
 		evsel->attr.sample_id_all = 0;

@@ -1455,6 +1461,12 @@ try_fallback:
 	} else if (!perf_missing_features.sample_id_all) {
 		perf_missing_features.sample_id_all = true;
 		goto retry_sample_id;
+	} else if (!perf_missing_features.lbr_flags &&
+			(evsel->attr.branch_sample_type &
+			 (PERF_SAMPLE_BRANCH_NO_CYCLES |
+			  PERF_SAMPLE_BRANCH_NO_FLAGS))) {
+		perf_missing_features.lbr_flags = true;
+		goto fallback_missing_features;
 	}
 
 out_close:

2.1.0

From: Namhyung Kim <[email protected]>

Currently if kptr_restrict is enabled, all hist tests failed with segfaults. This is because machine__create_kernel_maps() in setup_fake_machine() failed in that situation, and it called machine__delete() on the error path. But outer callers again called machines__exit() causing double free for the host machine.

---
 tools/perf/tests/hists_common.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

index 46f453b1de60..bcfd081ee1d2 100644
@@ -88,8 +88,8 @@ struct machine *setup_fake_machine(struct machines *machines)
 	}
 
 	if (machine__create_kernel_maps(machine)) {
-		pr_debug("Not enough memory for machine setup\n");
-		goto out;
+		pr_debug("Cannot create kernel maps\n");
+		return NULL;
 	}
 
 	for (i = 0; i < ARRAY_SIZE(fake_threads); i++) {

@@ -155,7 +155,6 @@ struct machine *setup_fake_machine(struct machines *machines)
 out:
 	pr_debug("Not enough memory for machine setup\n");
 	machine__delete_threads(machine);
-	machine__delete(machine);
 	return NULL;
 }
 

2.1.0

Hi,

I have a similar patch on this problem at [1][2].

This patch applied solves only half of the problem I seen. Although double free is gone, in case when kptr_restrict is enabled a normal user still gets incorrect result. For example:

 $ ./perf test -v 'filtering hist entries'
 25: Test filtering hist entries                              :
 --- start ---
 test child forked, pid 4186
 Cannot create kernel maps
 test child finished with 0
 ---- end ----
 Test filtering hist entries: Ok

The test body doesn't get called at all. I think in this case we'd better return 'Skip'.

I'd like to send another for it.

[1] http://lkml.kernel.org/g/[email protected]
[2] http://lkml.kernel.org/g/[email protected]

From: Josh Poimboeuf <[email protected]>

This line always silently fails because it doesn't add the 'test-' prefix to the .bin file.

And it seems to be unnecessary anyway: the line immediately after it does all the individual feature checks.

---
 tools/build/Makefile.feature | 1 -
 1 file changed, 1 deletion(-)

index 37ff4c9f92f1..b8c31ece2d96 100644
@@ -101,7 +101,6 @@ ifeq ($(feature-all), 1)
   #
   $(foreach feat,$(FEATURE_TESTS),$(call feature_set,$(feat)))
 else
-  $(shell $(MAKE) OUTPUT=$(OUTPUT_FEATURES) CFLAGS="$(EXTRA_CFLAGS)" LDFLAGS=$(LDFLAGS) -i -j -C $(feature_dir) $(addsuffix .bin,$(FEATURE_TESTS)) >/dev/null 2>&1)
   $(foreach feat,$(FEATURE_TESTS),$(call feature_check,$(feat)))
 endif
 

2.1.0

From: Josh Poimboeuf <[email protected]>

Because the Build file writes source code to the generated llvm-src-*.c files, it should be listed as one of the dependencies, so that any future changes to the code being echoed won't require a 'make clean'.

---
 tools/perf/tests/Build | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

index 0ff8a973b81c..f23fb7ed4400 100644
@@ -35,21 +35,21 @@ perf-y += llvm.o llvm-src-base.o llvm-src-kbuild.o llvm-src-prologue.o
 perf-y += bpf.o
 perf-y += topology.o
 
-$(OUTPUT)tests/llvm-src-base.c: tests/bpf-script-example.c
+$(OUTPUT)tests/llvm-src-base.c: tests/bpf-script-example.c tests/Build
 	$(call rule_mkdir)
 	$(Q)echo '#include <tests/llvm.h>' > $@
 	$(Q)echo 'const char test_llvm__bpf_base_prog[] =' >> $@
 	$(Q)sed -e 's/"/\\"/g' -e 's/\(.*\)/"\1\\n"/g' $< >> $@
 	$(Q)echo ';' >> $@
 
-$(OUTPUT)tests/llvm-src-kbuild.c: tests/bpf-script-test-kbuild.c
+$(OUTPUT)tests/llvm-src-kbuild.c: tests/bpf-script-test-kbuild.c tests/Build
 	$(call rule_mkdir)
 	$(Q)echo '#include <tests/llvm.h>' > $@
 	$(Q)echo 'const char test_llvm__bpf_test_kbuild_prog[] =' >> $@
 	$(Q)sed -e 's/"/\\"/g' -e 's/\(.*\)/"\1\\n"/g' $< >> $@
 	$(Q)echo ';' >> $@
 
-$(OUTPUT)tests/llvm-src-prologue.c: tests/bpf-script-test-prologue.c
+$(OUTPUT)tests/llvm-src-prologue.c: tests/bpf-script-test-prologue.c tests/Build
 	$(call rule_mkdir)
 	$(Q)echo '#include <tests/llvm.h>' > $@
 	$(Q)echo 'const char test_llvm__bpf_test_prologue_prog[] =' >> $@

2.1.0

From: Josh Poimboeuf <[email protected]>

---
 tools/perf/tests/make | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

index 8ea3dffc5065..c1fbb8e884c0 100644
@@ -259,7 +259,8 @@ $(run_O):
 tarpkg:
 	@cmd="$(PERF)/tests/perf-targz-src-pkg $(PERF)"; \
 	echo "- $@: $$cmd" && echo $$cmd > $@ && \
-	( eval $$cmd ) >> $@ 2>&1
+	( eval $$cmd ) >> $@ 2>&1 && \
+	rm -f $@
 
 make_kernelsrc:
 	@echo "- make -C <kernelsrc> tools/perf"

2.1.0

From: Josh Poimboeuf <[email protected]>

Add some missing files to the 'make clean' target.

---
 tools/perf/Makefile.perf | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

index 929a32ba15f5..906c72364d3a 100644
@@ -582,15 +582,16 @@ $(INSTALL_DOC_TARGETS):
 #
 config-clean:
 	$(call QUIET_CLEAN, config)
-	$(Q)$(MAKE) -C $(srctree)/tools/build/feature/ clean >/dev/null
+	$(Q)$(MAKE) -C $(srctree)/tools/build/feature/ $(if $(OUTPUT),OUTPUT=$(OUTPUT)feature/,) clean >/dev/null
 
 clean: $(LIBTRACEEVENT)-clean $(LIBAPI)-clean $(LIBBPF)-clean config-clean
 	$(call QUIET_CLEAN, core-objs)  $(RM) $(LIB_FILE) $(OUTPUT)perf-archive $(OUTPUT)perf-with-kcore $(LANG_BINDINGS)
-	$(Q)find . -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete
+	$(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
 	$(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)util/intel-pt-decoder/inat-tables.c $(OUTPUT)fixdep \
+		$(OUTPUT)tests/llvm-src-{base,kbuild,prologue}.c
 	$(QUIET_SUBDIR0)Documentation $(QUIET_SUBDIR1) clean
 	$(python-clean)
 

2.1.0

From: Josh Poimboeuf <[email protected]>

'LIB_PATH' is a misnomer because there are multiple library paths.

---
 tools/perf/Makefile.perf | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

index 906c72364d3a..388ec64fa39b 100644
@@ -185,13 +185,13 @@ ifneq ($(OUTPUT),)
   TE_PATH=$(OUTPUT)
   BPF_PATH=$(OUTPUT)
 ifneq ($(subdir),)
-  LIB_PATH=$(OUTPUT)/../lib/api/
+  API_PATH=$(OUTPUT)/../lib/api/
 else
-  LIB_PATH=$(OUTPUT)
+  API_PATH=$(OUTPUT)
 endif
 else
   TE_PATH=$(TRACE_EVENT_DIR)
-  LIB_PATH=$(LIB_DIR)
+  API_PATH=$(LIB_DIR)
   BPF_PATH=$(BPF_DIR)
 endif
 

@@ -201,7 +201,7 @@ export LIBTRACEEVENT
 LIBTRACEEVENT_DYNAMIC_LIST = $(TE_PATH)libtraceevent-dynamic-list
 LIBTRACEEVENT_DYNAMIC_LIST_LDFLAGS = -Xlinker --dynamic-list=$(LIBTRACEEVENT_DYNAMIC_LIST)
 
-LIBAPI = $(LIB_PATH)libapi.a
+LIBAPI = $(API_PATH)libapi.a
 export LIBAPI
 
 LIBBPF = $(BPF_PATH)libbpf.a

2.1.0

From: Josh Poimboeuf <[email protected]>

Move the 'pager' function prototypes into a new pager.h so that the pager code can be moved out to a library.

---
 tools/perf/util/cache.h | 5 +----
 tools/perf/util/pager.h | 7 +++++++
 2 files changed, 8 insertions(+), 4 deletions(-)
 create mode 100644 tools/perf/util/pager.h

index 4c2b76499dd5..9ca4a58f160d 100644
@@ -4,6 +4,7 @@
 #include <stdbool.h>
 #include "util.h"
 #include "strbuf.h"
+#include "pager.h"
 #include "../perf.h"
 #include "../ui/ui.h"
 

@@ -28,10 +29,6 @@ extern int perf_config_bool(const char *, const char *);
 extern int config_error_nonbool(const char *);
 extern const char *perf_config_dirname(const char *, const char *);
 
-/* pager.c */
-extern void setup_pager(void);
-extern int pager_in_use(void);
-
 char *alias_lookup(const char *alias);
 int split_cmdline(char *cmdline, const char ***argv);
 

new file mode 100644
index 000000000000..2794a83b7e85
@@ -0,0 +1,7 @@
+#ifndef __PERF_PAGER_H
+#define __PERF_PAGER_H
+
+extern void setup_pager(void);
+extern int pager_in_use(void);
+
+#endif /* __PERF_PAGER_H */

2.1.0

From: Josh Poimboeuf <[email protected]>

PERF_PAGER_IN_USE doesn't seem to be used anywhere, so let's remove it.

This will also make it easier to move pager.c into a separate library.

---
 tools/perf/util/pager.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

index 53ef006a951c..7dcbef64b609 100644
@@ -85,11 +85,5 @@ void setup_pager(void)
 
 int pager_in_use(void)
 {
-	const char *env;
-
-	if (spawned_pager)
-		return 1;
-
-	env = getenv("PERF_PAGER_IN_USE");
-	return env ? perf_config_bool("PERF_PAGER_IN_USE", env) : 0;
+	return spawned_pager;
 }

2.1.0

From: Josh Poimboeuf <[email protected]>

help_unknown_cmd() is quite perf-specific because it relies on some perf_config*() functions. Move it and its supporting functions out into a separate file so that help.c can be moved to a library.

---
 tools/perf/util/Build              |   1 +
 tools/perf/util/help-unknown-cmd.c | 103 +++++++++++++++++++++++++++++++++++
 tools/perf/util/help-unknown-cmd.h |   0
 tools/perf/util/help.c             | 107 ++-----------------------------------
 tools/perf/util/help.h             |   3 ++
 5 files changed, 110 insertions(+), 104 deletions(-)
 create mode 100644 tools/perf/util/help-unknown-cmd.c
 create mode 100644 tools/perf/util/help-unknown-cmd.h

index 65fef5951c7d..99b3dae57806 100644
@@ -87,6 +87,7 @@ libperf-$(CONFIG_AUXTRACE) += intel-bts.o
 libperf-y += parse-branch-options.o
 libperf-y += parse-regs-options.o
 libperf-y += term.o
+libperf-y += help-unknown-cmd.o
 
 libperf-$(CONFIG_LIBBPF) += bpf-loader.o
 libperf-$(CONFIG_BPF_PROLOGUE) += bpf-prologue.o

new file mode 100644
index 000000000000..e69de29bb2d1

index 7f5c6dedd714..14851b0e44f5 100644
@@ -20,6 +20,9 @@ void load_command_list(const char *prefix,
 		struct cmdnames *main_cmds,
 		struct cmdnames *other_cmds);
 void add_cmdname(struct cmdnames *cmds, const char *name, size_t len);
+void clean_cmdnames(struct cmdnames *cmds);
+int cmdname_compare(const void *a, const void *b);
+void uniq(struct cmdnames *cmds);
 /* Here we require that excludes is a sorted list. */
 void exclude_cmds(struct cmdnames *cmds, struct cmdnames *excludes);
 int is_in_cmdlist(struct cmdnames *c, const char *s);

2.1.0

From: Josh Poimboeuf <[email protected]>

---
 tools/perf/util/parse-options.c | 18 +++++++++---------
 tools/perf/util/parse-options.h |  9 ---------
 2 files changed, 9 insertions(+), 18 deletions(-)

index de3290b47db1..14b2bee81e75 100644
@@ -360,8 +360,8 @@ static void check_typos(const char *arg, const struct option *options)
 	}
 }
 
-void parse_options_start(struct parse_opt_ctx_t *ctx,
-			 int argc, const char **argv, int flags)
+static void parse_options_start(struct parse_opt_ctx_t *ctx,
+				int argc, const char **argv, int flags)
 {
 	memset(ctx, 0, sizeof(*ctx));
 	ctx->argc = argc - 1;

@@ -378,9 +378,9 @@ static int usage_with_options_internal(const char * const *,
 				       const struct option *, int,
 				       struct parse_opt_ctx_t *);
 
-int parse_options_step(struct parse_opt_ctx_t *ctx,
-		       const struct option *options,
-		       const char * const usagestr[])
+static int parse_options_step(struct parse_opt_ctx_t *ctx,
+			      const struct option *options,
+			      const char * const usagestr[])
 {
 	int internal_help = !(ctx->flags & PARSE_OPT_NO_INTERNAL_HELP);
 	int excl_short_opt = 1;

@@ -489,7 +489,7 @@ exclusive:
 	return PARSE_OPT_HELP;
 }
 
-int parse_options_end(struct parse_opt_ctx_t *ctx)
+static int parse_options_end(struct parse_opt_ctx_t *ctx)
 {
 	memmove(ctx->out + ctx->cpidx, ctx->argv, ctx->argc * sizeof(*ctx->out));
 	ctx->out[ctx->cpidx + ctx->argc] = NULL;

@@ -717,9 +717,9 @@ static bool option__in_argv(const struct option *opt, const struct parse_opt_ctx
 	return false;
 }
 
-int usage_with_options_internal(const char * const *usagestr,
-				const struct option *opts, int full,
-				struct parse_opt_ctx_t *ctx)
+static int usage_with_options_internal(const char * const *usagestr,
+				       const struct option *opts, int full,
+				       struct parse_opt_ctx_t *ctx)
 {
 	struct option *ordered;
 

index a8e407bc251e..dd1236d49903 100644
@@ -195,15 +195,6 @@ extern int parse_options_usage(const char * const *usagestr,
 			       const char *optstr,
 			       bool short_opt);
 
-extern void parse_options_start(struct parse_opt_ctx_t *ctx,
-				int argc, const char **argv, int flags);
-
-extern int parse_options_step(struct parse_opt_ctx_t *ctx,
-			      const struct option *options,
-			      const char * const usagestr[]);
-
-extern int parse_options_end(struct parse_opt_ctx_t *ctx);
-
 
 /*----- some often used options -----*/
 extern int parse_opt_abbrev_cb(const struct option *, const char *, int);

2.1.0

From: Wang Nan <[email protected]>

This patch keeps options of perf builtins same in all conditions. If one option is disabled because of compiling options, users should be notified.

Masami suggested another implementation in [1] that, by adding a OPTION_NEXT_DEPENDS option before those options in the 'struct option' array, options parser knows an option is disabled. However, in some cases this array is reordered (options__order()). In addition, in parse-option.c that array is const, so we can't simply merge information in decorator option into the affacted option.

This patch chooses a simpler implementation that, introducing a set_option_nobuild() function and two option parsing flags. Builtins with such options should call set_option_nobuild() before option parsing. The complexity of this patch is because we want some of options can be skipped safely. In this case their arguments should also be consumed.

Options in 'perf record' and 'perf probe' are fixed in this patch.

[1] http://lkml.kernel.org/g/50399556C9727B4D88A595C8584AAB3752627CD4@GSjpTKYDCembx32.service.hitachi.net

Test result:

Normal case:

  # ./perf probe --vmlinux /tmp/vmlinux sys_write
  Added new event:
    probe:sys_write      (on sys_write)

You can now use it in all perf tools, such as:

perf record -e probe:sys_write -aR sleep 1

Build with NO_DWARF=1:

  # ./perf probe -L sys_write
    Error: switch `L' is not available because NO_DWARF=1

Usage: perf probe [<options>] 'PROBEDEF' ['PROBEDEF' ...]

      or: perf probe [<options>] --add 'PROBEDEF' [--add 'PROBEDEF' ...]
      or: perf probe [<options>] --del '[GROUP:]EVENT' ...
      or: perf probe --list [GROUP:]EVENT ...
      or: perf probe [<options>] --funcs

    -L, --line <FUNC[:RLN[+NUM|-RLN2]]|SRC:ALN[+NUM|-ALN2]>
                          Show source code lines.
                          (not built-in because NO_DWARF=1)

  # ./perf probe -k /tmp/vmlinux sys_write
    Warning: switch `k' is being ignored because NO_DWARF=1
  Added new event:
    probe:sys_write      (on sys_write)

You can now use it in all perf tools, such as:

perf record -e probe:sys_write -aR sleep 1

  # ./perf probe --vmlinux /tmp/vmlinux sys_write
    Warning: option `vmlinux' is being ignored because NO_DWARF=1
  Added new event:
  [SNIP]

  # ./perf probe -l
   Usage: perf probe [<options>] 'PROBEDEF' ['PROBEDEF' ...]
      or: perf probe [<options>] --add 'PROBEDEF' [--add 'PROBEDEF' ...]
...
    -k, --vmlinux <file>  vmlinux pathname
                          (not built-in because NO_DWARF=1)
    -L, --line <FUNC[:RLN[+NUM|-RLN2]]|SRC:ALN[+NUM|-ALN2]>
                          Show source code lines.
                          (not built-in because NO_DWARF=1)
...
    -V, --vars <FUNC[@SRC][+OFF|%return|:RL|;PT]|SRC:AL|SRC;PT>
                          Show accessible variables on PROBEDEF
                          (not built-in because NO_DWARF=1)
        --externs         Show external variables too (with --vars only)
                          (not built-in because NO_DWARF=1)
        --no-inlines      Don't search inlined functions
                          (not built-in because NO_DWARF=1)
        --range           Show variables location range in scope (with --vars only)
                          (not built-in because NO_DWARF=1)

---
 tools/perf/builtin-probe.c      |  15 ++++-
 tools/perf/builtin-record.c     |   9 ++-
 tools/perf/util/parse-options.c | 118 +++++++++++++++++++++++++++++++++++++---
 tools/perf/util/parse-options.h |   5 ++
 4 files changed, 134 insertions(+), 13 deletions(-)

index 132afc97676c..dbe2ea5a2932 100644
@@ -249,6 +249,9 @@ static int opt_show_vars(const struct option *opt,
 
 	return ret;
 }
+#else
+# define opt_show_lines NULL
+# define opt_show_vars NULL
 #endif
 static int opt_add_probe_event(const struct option *opt,
 			      const char *str, int unset __maybe_unused)

@@ -473,7 +476,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 		opt_add_probe_event),
 	OPT_BOOLEAN('f', "force", &probe_conf.force_add, "forcibly add events"
 		    " with existing name"),
-#ifdef HAVE_DWARF_SUPPORT
 	OPT_CALLBACK('L', "line", NULL,
 		     "FUNC[:RLN[+NUM|-RLN2]]|SRC:ALN[+NUM|-ALN2]",
 		     "Show source code lines.", opt_show_lines),

@@ -490,7 +492,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 		   "directory", "path to kernel source"),
 	OPT_BOOLEAN('\0', "no-inlines", &probe_conf.no_inlines,
 		"Don't search inlined functions"),
-#endif
 	OPT__DRY_RUN(&probe_event_dry_run),
 	OPT_INTEGER('\0', "max-probes", &probe_conf.max_probes,
 		 "Set how many probe points can be found for a probe."),

@@ -521,6 +522,16 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 #ifdef HAVE_DWARF_SUPPORT
 	set_option_flag(options, 'L', "line", PARSE_OPT_EXCLUSIVE);
 	set_option_flag(options, 'V', "vars", PARSE_OPT_EXCLUSIVE);
+#else
+# define set_nobuild(s, l, c) set_option_nobuild(options, s, l, "NO_DWARF=1", c)
+	set_nobuild('L', "line", false);
+	set_nobuild('V', "vars", false);
+	set_nobuild('\0', "externs", false);
+	set_nobuild('\0', "range", false);
+	set_nobuild('k', "vmlinux", true);
+	set_nobuild('s', "source", true);
+	set_nobuild('\0', "no-inlines", true);
+# undef set_nobuild
 #endif
 	set_option_flag(options, 'F', "funcs", PARSE_OPT_EXCLUSIVE);
 

index 199fc31e3919..c2ba377e8d3a 100644
@@ -1113,12 +1113,10 @@ struct option __record_options[] = {
 			"per thread proc mmap processing timeout in ms"),
 	OPT_BOOLEAN(0, "switch-events", &record.opts.record_switch_events,
 		    "Record context switch events"),
-#ifdef HAVE_LIBBPF_SUPPORT
 	OPT_STRING(0, "clang-path", &llvm_param.clang_path, "clang path",
 		   "clang binary to use for compiling BPF scriptlets"),
 	OPT_STRING(0, "clang-opt", &llvm_param.clang_opt, "clang options",
 		   "options passed to clang when compiling BPF scriptlets"),
-#endif
 	OPT_END()
 };
 

@@ -1130,6 +1128,13 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
 	struct record *rec = &record;
 	char errbuf[BUFSIZ];
 
+#ifndef HAVE_LIBBPF_SUPPORT
+# define set_nobuild(s, l, c) set_option_nobuild(record_options, s, l, "NO_LIBBPF=1", c)
+	set_nobuild('\0', "clang-path", true);
+	set_nobuild('\0', "clang-opt", true);
+# undef set_nobuild
+#endif
+
 	rec->evlist = perf_evlist__new();
 	if (rec->evlist == NULL)
 		return -ENOMEM;

index dd1236d49903..1231960d6148 100644
@@ -41,6 +41,8 @@ enum parse_opt_option_flags {
 	PARSE_OPT_DISABLED = 32,
 	PARSE_OPT_EXCLUSIVE = 64,
 	PARSE_OPT_NOEMPTY  = 128,
+	PARSE_OPT_NOBUILD  = 256,
+	PARSE_OPT_CANSKIP  = 512,
 };
 
 struct option;

@@ -96,6 +98,7 @@ struct option {
 	void *value;
 	const char *argh;
 	const char *help;
+	const char *build_opt;
 
 	int flags;
 	parse_opt_cb *callback;

@@ -217,4 +220,6 @@ extern int parse_opt_verbosity_cb(const struct option *, const char *, int);
 extern const char *parse_options_fix_filename(const char *prefix, const char *file);
 
 void set_option_flag(struct option *opts, int sopt, const char *lopt, int flag);
+void set_option_nobuild(struct option *opts, int shortopt, const char *longopt,
+			const char *build_opt, bool can_skip);
 #endif /* __PERF_PARSE_OPTIONS_H */

2.1.0

From: He Kuang <[email protected]>

Make perf-record command support --vmlinux option if BPF_PROLOGUE is on.

'perf record' needs vmlinux as the source of DWARF info to generate prologue for BPF programs, so path of vmlinux should be specified.

Short name 'k' has been taken by 'clockid'. This patch skips the short option name and uses '--vmlinux' for vmlinux path.

Documentation is also updated.

Test result:

In a production (or broken) environment:
(by:

  # rm -rf ~/.debug/
  # mv /lib/modules/`uname -r`/build/vmlinux /tmp/
 )

 # ./perf record -e ./test_bpf_base.c ls
 Failed to find the path for kernel: No such file or directory
 event syntax error: './test_bpf_base.c'
                      \___ You need to check probing points in BPF file
 ...

 # ./perf record --vmlinux /tmp/vmlinux -e ./test_bpf_base.c ls
 ...
 [ perf record: Woken up 1 times to write data ]
 [ perf record: Captured and wrote 0.011 MB perf.data ]

Help messages when build with NO_LIBBPF:

 # ./perf record -h
        --transaction     sample transaction flags (special events only)
        --vmlinux <file>  vmlinux pathname
                          (not built-in because NO_LIBBPF=1)
 # ./perf record --vmlinux /tmp/vmlinux ls /
  Warning: option `vmlinux' is being ignored because NO_LIBBPF=1
 ...
 [ perf record: Woken up 1 times to write data ]
 [ perf record: Captured and wrote 0.011 MB perf.data (11 samples) ]

Help messages when build with NO_DWARF:

 # ./perf record -h
        --transaction     sample transaction flags (special events only)
        --vmlinux <file>  vmlinux pathname
                          (not built-in because NO_DWARF=1)

---
 tools/perf/Documentation/perf-record.txt | 10 ++++++++--
 tools/perf/builtin-record.c              | 16 ++++++++++++++++
 2 files changed, 24 insertions(+), 2 deletions(-)

index e630a7d2c348..8d032f4e50bf 100644
@@ -314,11 +314,17 @@ This option sets the time out limit. The default value is 500 ms.
 Record context switch events i.e. events of type PERF_RECORD_SWITCH or
 PERF_RECORD_SWITCH_CPU_WIDE.
 
---clang-path::
+--clang-path=PATH::
 Path to clang binary to use for compiling BPF scriptlets.
+(enabled when BPF support is on)
 
---clang-opt::
+--clang-opt=OPTIONS::
 Options passed to clang when compiling BPF scriptlets.
+(enabled when BPF support is on)
+
+--vmlinux=PATH::
+Specify vmlinux path which has debuginfo.
+(enabled when BPF prologue is on)
 
 SEE ALSO
 --------

index c2ba377e8d3a..3ef3c79e7534 100644
@@ -1117,6 +1117,8 @@ struct option __record_options[] = {
 		   "clang binary to use for compiling BPF scriptlets"),
 	OPT_STRING(0, "clang-opt", &llvm_param.clang_opt, "clang options",
 		   "options passed to clang when compiling BPF scriptlets"),
+	OPT_STRING(0, "vmlinux", &symbol_conf.vmlinux_name,
+		   "file", "vmlinux pathname"),
 	OPT_END()
 };
 

@@ -1135,6 +1137,20 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
 # undef set_nobuild
 #endif
 
+#ifndef HAVE_BPF_PROLOGUE
+# if !defined (HAVE_DWARF_SUPPORT)
+#  define REASON  "NO_DWARF=1"
+# elif !defined (HAVE_LIBBPF_SUPPORT)
+#  define REASON  "NO_LIBBPF=1"
+# else
+#  define REASON  "this architecture doesn't support BPF prologue"
+# endif
+# define set_nobuild(s, l, c) set_option_nobuild(record_options, s, l, REASON, c)
+	set_nobuild('\0', "vmlinux", true);
+# undef set_nobuild
+# undef REASON
+#endif
+
 	rec->evlist = perf_evlist__new();
 	if (rec->evlist == NULL)
 		return -ENOMEM;

2.1.0