From f76214f93783f9f902954cf6e57fd818c4bffe29 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Wed, 19 Dec 2018 10:01:20 -0300 Subject: perf trace: Check if the raw_syscalls:sys_{enter,exit} are setup before setting tp filter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While updating 'perf trace' on an machine with an old precompiled augmented_raw_syscalls.o that didn't setup the syscall map the new 'perf trace' codebase notices the augmented_raw_syscalls.o eBPF event, decides to use it instead of the old raw_syscalls:sys_{enter,exit} method, but then because we don't have the syscall map tries to set the tracepoint filter on the sys_{enter,exit} evsels, that are NULL, segfaulting. Make the code more robust by checking it those tracepoints have their respective evsels in place before trying to set the tp filter. With this we still get everything to work, just not setting up the syscall filters, which is better than a segfault. Now to update the precompiled augmented_raw_syscalls.o and continue development :-) Cc: Adrian Hunter Cc: Jiri Olsa Cc: Luis Cláudio Gonçalves Cc: Namhyung Kim Cc: Wang Nan Link: https://lkml.kernel.org/n/tip-3ft5rjdl05wgz2pwpx2z8btu@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-trace.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c index ebde59e611336..6689c1a114fec 100644 --- a/tools/perf/builtin-trace.c +++ b/tools/perf/builtin-trace.c @@ -2686,7 +2686,9 @@ static int trace__set_ev_qualifier_filter(struct trace *trace) { if (trace->syscalls.map) return trace__set_ev_qualifier_bpf_filter(trace); - return trace__set_ev_qualifier_tp_filter(trace); + if (trace->syscalls.events.sys_enter) + return trace__set_ev_qualifier_tp_filter(trace); + return 0; } static int bpf_map__set_filter_pids(struct bpf_map *map __maybe_unused, -- cgit v1.2.3 From 5ce29d522e9b8cdc158329a8539bcd6206fc5bf4 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Wed, 19 Dec 2018 10:10:51 -0300 Subject: perf beauty mmap: PROT_WRITE should come before PROT_EXEC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To match strace output: # cat mmap.c #include int main(void) { mmap(0, 4096, PROT_READ|PROT_WRITE|PROT_EXEC, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0); return 0; } # strace -e mmap ./mmap |& grep -v ^+++ mmap(NULL, 103484, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7f5bae400000 mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f5bae3fe000 mmap(NULL, 3889792, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7f5bade40000 mmap(0x7f5bae1ec000, 24576, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x1ac000) = 0x7f5bae1ec000 mmap(0x7f5bae1f2000, 14976, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7f5bae1f2000 mmap(NULL, 4096, PROT_READ|PROT_WRITE|PROT_EXEC, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f5bae419000 # trace -e mmap ./mmap |& grep -v ^+++ mmap(NULL, 103484, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7f6646c25000 mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS) = 0x7f6646c23000 mmap(NULL, 3889792, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7f6646665000 mmap(0x7f6646a11000, 24576, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x1ac000) = 0x7f6646a11000 mmap(0x7f6646a17000, 14976, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS) = 0x7f6646a17000 mmap(NULL, 4096, PROT_READ|PROT_WRITE|PROT_EXEC, MAP_PRIVATE|MAP_ANONYMOUS) = 0x7f6646c3e000 # Reported-by: Namhyung Kim Cc: Adrian Hunter Cc: Jiri Olsa Cc: Luis Cláudio Gonçalves Cc: Wang Nan Link: https://lkml.kernel.org/n/tip-nt49d6iqle80cw8f529ovaqi@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/trace/beauty/mmap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/trace/beauty/mmap.c b/tools/perf/trace/beauty/mmap.c index eb31089790e3d..859a8a9db2c62 100644 --- a/tools/perf/trace/beauty/mmap.c +++ b/tools/perf/trace/beauty/mmap.c @@ -18,8 +18,8 @@ static size_t syscall_arg__scnprintf_mmap_prot(char *bf, size_t size, } P_MMAP_PROT(READ); - P_MMAP_PROT(EXEC); P_MMAP_PROT(WRITE); + P_MMAP_PROT(EXEC); P_MMAP_PROT(SEM); P_MMAP_PROT(GROWSDOWN); P_MMAP_PROT(GROWSUP); -- cgit v1.2.3 From 14541b1e7e723859ff2c75c6fc10cdbbec6b8c34 Mon Sep 17 00:00:00 2001 From: Stanislav Fomichev Date: Thu, 15 Nov 2018 16:32:01 -0800 Subject: perf build: Don't unconditionally link the libbfd feature test to -liberty and -lz Current libbfd feature test unconditionally links against -liberty and -lz. While it's required on some systems (e.g. opensuse), it's completely unnecessary on the others, where only -lbdf is sufficient (debian). This patch streamlines (and renames) the following feature checks: feature-libbfd - only link against -lbfd (debian), see commit 2cf9040714f3 ("perf tools: Fix bfd dependency libraries detection") feature-libbfd-liberty - link against -lbfd and -liberty feature-libbfd-liberty-z - link against -lbfd, -liberty and -lz (opensuse), see commit 280e7c48c3b8 ("perf tools: fix BFD detection on opensuse") (feature-liberty{,-z} were renamed to feature-libbfd-liberty{,z} for clarity) The main motivation is to fix this feature test for bpftool which is currently broken on debian (libbfd feature shows OFF, but we still unconditionally link against -lbfd and it works). Tested on debian with only -lbfd installed (without -liberty); I'd appreciate if somebody on the other systems can test this new detection method. Signed-off-by: Stanislav Fomichev Acked-by: Jiri Olsa Cc: Alexander Shishkin Cc: Mathieu Poirier Cc: Namhyung Kim Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/4dfc634cfcfb236883971b5107cf3c28ec8a31be.1542328222.git.sdf@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/build/Makefile.feature | 4 ++-- tools/build/feature/Makefile | 10 +++++----- tools/perf/Makefile.config | 44 +++++++++++++++++++++++--------------------- 3 files changed, 30 insertions(+), 28 deletions(-) diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature index d47b8f73e2e74..5467c6bf9cebe 100644 --- a/tools/build/Makefile.feature +++ b/tools/build/Makefile.feature @@ -82,8 +82,8 @@ FEATURE_TESTS_EXTRA := \ cplus-demangle \ hello \ libbabeltrace \ - liberty \ - liberty-z \ + libbfd-liberty \ + libbfd-liberty-z \ libunwind-debug-frame \ libunwind-debug-frame-arm \ libunwind-debug-frame-aarch64 \ diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile index 2dbcc0d00f52d..7ceb4441b6277 100644 --- a/tools/build/feature/Makefile +++ b/tools/build/feature/Makefile @@ -17,8 +17,8 @@ FILES= \ test-libbfd.bin \ test-disassembler-four-args.bin \ test-reallocarray.bin \ - test-liberty.bin \ - test-liberty-z.bin \ + test-libbfd-liberty.bin \ + test-libbfd-liberty-z.bin \ test-cplus-demangle.bin \ test-libelf.bin \ test-libelf-getphdrnum.bin \ @@ -210,7 +210,7 @@ $(OUTPUT)test-libpython-version.bin: $(BUILD) $(OUTPUT)test-libbfd.bin: - $(BUILD) -DPACKAGE='"perf"' -lbfd -lz -liberty -ldl + $(BUILD) -DPACKAGE='"perf"' -lbfd -ldl $(OUTPUT)test-disassembler-four-args.bin: $(BUILD) -DPACKAGE='"perf"' -lbfd -lopcodes @@ -218,10 +218,10 @@ $(OUTPUT)test-disassembler-four-args.bin: $(OUTPUT)test-reallocarray.bin: $(BUILD) -$(OUTPUT)test-liberty.bin: +$(OUTPUT)test-libbfd-liberty.bin: $(CC) $(CFLAGS) -Wall -Werror -o $@ test-libbfd.c -DPACKAGE='"perf"' $(LDFLAGS) -lbfd -ldl -liberty -$(OUTPUT)test-liberty-z.bin: +$(OUTPUT)test-libbfd-liberty-z.bin: $(CC) $(CFLAGS) -Wall -Werror -o $@ test-libbfd.c -DPACKAGE='"perf"' $(LDFLAGS) -lbfd -ldl -liberty -lz $(OUTPUT)test-cplus-demangle.bin: diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config index 07c1857c3d7a9..b441c88cafa1f 100644 --- a/tools/perf/Makefile.config +++ b/tools/perf/Makefile.config @@ -702,18 +702,20 @@ endif ifeq ($(feature-libbfd), 1) EXTLIBS += -lbfd +else + # we are on a system that requires -liberty and (maybe) -lz + # to link against -lbfd; test each case individually here # call all detections now so we get correct # status in VF output - $(call feature_check,liberty) - $(call feature_check,liberty-z) - $(call feature_check,cplus-demangle) + $(call feature_check,libbfd-liberty) + $(call feature_check,libbfd-liberty-z) - ifeq ($(feature-liberty), 1) - EXTLIBS += -liberty + ifeq ($(feature-libbfd-liberty), 1) + EXTLIBS += -lbfd -liberty else - ifeq ($(feature-liberty-z), 1) - EXTLIBS += -liberty -lz + ifeq ($(feature-libbfd-liberty-z), 1) + EXTLIBS += -lbfd -liberty -lz endif endif endif @@ -723,24 +725,24 @@ ifdef NO_DEMANGLE else ifdef HAVE_CPLUS_DEMANGLE_SUPPORT EXTLIBS += -liberty - CFLAGS += -DHAVE_CPLUS_DEMANGLE_SUPPORT else - ifneq ($(feature-libbfd), 1) - ifneq ($(feature-liberty), 1) - ifneq ($(feature-liberty-z), 1) - # we dont have neither HAVE_CPLUS_DEMANGLE_SUPPORT - # or any of 'bfd iberty z' trinity - ifeq ($(feature-cplus-demangle), 1) - EXTLIBS += -liberty - CFLAGS += -DHAVE_CPLUS_DEMANGLE_SUPPORT - else - msg := $(warning No bfd.h/libbfd found, please install binutils-dev[el]/zlib-static/libiberty-dev to gain symbol demangling) - CFLAGS += -DNO_DEMANGLE - endif - endif + ifeq ($(filter -liberty,$(EXTLIBS)),) + $(call feature_check,cplus-demangle) + + # we dont have neither HAVE_CPLUS_DEMANGLE_SUPPORT + # or any of 'bfd iberty z' trinity + ifeq ($(feature-cplus-demangle), 1) + EXTLIBS += -liberty + else + msg := $(warning No bfd.h/libbfd found, please install binutils-dev[el]/zlib-static/libiberty-dev to gain symbol demangling) + CFLAGS += -DNO_DEMANGLE endif endif endif + + ifneq ($(filter -liberty,$(EXTLIBS)),) + CFLAGS += -DHAVE_CPLUS_DEMANGLE_SUPPORT + endif endif ifneq ($(filter -lbfd,$(EXTLIBS)),) -- cgit v1.2.3 From b9b6a2ea2baf69204a6e5f311e0d24fe3b956f2e Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Wed, 19 Dec 2018 18:54:36 -0300 Subject: perf trace: Do not hardcode the size of the tracepoint common_ fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We shouldn't hardcode the size of the tracepoint common_ fields, use the offset of the 'id'/'__syscallnr' field in the sys_enter event instead. This caused the augmented syscalls code to fail on a particular build of a PREEMPT_RT_FULL kernel where these extra 'common_migrate_disable' and 'common_padding' fields were before the syscall id one: # cat /sys/kernel/debug/tracing/events/raw_syscalls/sys_enter/format name: sys_enter ID: 22 format: field:unsigned short common_type; offset:0; size:2; signed:0; field:unsigned char common_flags; offset:2; size:1; signed:0; field:unsigned char common_preempt_count; offset:3; size:1; signed:0; field:int common_pid; offset:4; size:4; signed:1; field:unsigned short common_migrate_disable; offset:8; size:2; signed:0; field:unsigned short common_padding; offset:10; size:2; signed:0; field:long id; offset:16; size:8; signed:1; field:unsigned long args[6]; offset:24; size:48; signed:0; print fmt: "NR %ld (%lx, %lx, %lx, %lx, %lx, %lx)", REC->id, REC->args[0], REC->args[1], REC->args[2], REC->args[3], REC->args[4], REC->args[5] # All those 'common_' prefixed fields are zeroed when they hit a BPF tracepoint hook, we better just discard those, i.e. somehow pass an offset to the BPF program from the start of the ctx and make adjustments in the 'perf trace' handlers to adjust the offset of the syscall arg offsets obtained from tracefs. Till then, fix it the quick way and add this to the augmented_raw_syscalls.c to bet it to work in such kernels: diff --git a/tools/perf/examples/bpf/augmented_raw_syscalls.c b/tools/perf/examples/bpf/augmented_raw_syscalls.c index 53c233370fae..1f746f931e13 100644 --- a/tools/perf/examples/bpf/augmented_raw_syscalls.c +++ b/tools/perf/examples/bpf/augmented_raw_syscalls.c @@ -38,12 +38,14 @@ struct bpf_map SEC("maps") syscalls = { struct syscall_enter_args { unsigned long long common_tp_fields; + long rt_common_tp_fields; long syscall_nr; unsigned long args[6]; }; struct syscall_exit_args { unsigned long long common_tp_fields; + long rt_common_tp_fields; long syscall_nr; long ret; }; Just to check that this was the case. Fix it properly later, for now remove the hardcoding of the offset in the 'perf trace' side and document the situation with this patch. Cc: Adrian Hunter Cc: Jiri Olsa Cc: Luis Cláudio Gonçalves Cc: Namhyung Kim Cc: Wang Nan Link: https://lkml.kernel.org/n/tip-2pqavrktqkliu5b9nzouio21@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-trace.c | 73 +++++++++++++++++++++++++++++++++------------- 1 file changed, 52 insertions(+), 21 deletions(-) diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c index 6689c1a114fec..1e9e886b28115 100644 --- a/tools/perf/builtin-trace.c +++ b/tools/perf/builtin-trace.c @@ -112,8 +112,9 @@ struct trace { } stats; unsigned int max_stack; unsigned int min_stack; - bool sort_events; + int raw_augmented_syscalls_args_size; bool raw_augmented_syscalls; + bool sort_events; bool not_ev_qualifier; bool live; bool full_time; @@ -283,12 +284,17 @@ out_delete: return -ENOENT; } -static int perf_evsel__init_augmented_syscall_tp(struct perf_evsel *evsel) +static int perf_evsel__init_augmented_syscall_tp(struct perf_evsel *evsel, struct perf_evsel *tp) { struct syscall_tp *sc = evsel->priv = malloc(sizeof(struct syscall_tp)); - if (evsel->priv != NULL) { /* field, sizeof_field, offsetof_field */ - if (__tp_field__init_uint(&sc->id, sizeof(long), sizeof(long long), evsel->needs_swap)) + if (evsel->priv != NULL) { + struct tep_format_field *syscall_id = perf_evsel__field(tp, "id"); + if (syscall_id == NULL) + syscall_id = perf_evsel__field(tp, "__syscall_nr"); + if (syscall_id == NULL) + goto out_delete; + if (__tp_field__init_uint(&sc->id, syscall_id->size, syscall_id->offset, evsel->needs_swap)) goto out_delete; return 0; @@ -1768,16 +1774,16 @@ static int trace__fprintf_sample(struct trace *trace, struct perf_evsel *evsel, return printed; } -static void *syscall__augmented_args(struct syscall *sc, struct perf_sample *sample, int *augmented_args_size, bool raw_augmented) +static void *syscall__augmented_args(struct syscall *sc, struct perf_sample *sample, int *augmented_args_size, int raw_augmented_args_size) { void *augmented_args = NULL; /* * For now with BPF raw_augmented we hook into raw_syscalls:sys_enter - * and there we get all 6 syscall args plus the tracepoint common - * fields (sizeof(long)) and the syscall_nr (another long). So we check - * if that is the case and if so don't look after the sc->args_size, - * but always after the full raw_syscalls:sys_enter payload, which is - * fixed. + * and there we get all 6 syscall args plus the tracepoint common fields + * that gets calculated at the start and the syscall_nr (another long). + * So we check if that is the case and if so don't look after the + * sc->args_size but always after the full raw_syscalls:sys_enter payload, + * which is fixed. * * We'll revisit this later to pass s->args_size to the BPF augmenter * (now tools/perf/examples/bpf/augmented_raw_syscalls.c, so that it @@ -1785,7 +1791,7 @@ static void *syscall__augmented_args(struct syscall *sc, struct perf_sample *sam * use syscalls:sys_enter_NAME, so that we reduce the kernel/userspace * traffic to just what is needed for each syscall. */ - int args_size = raw_augmented ? (8 * (int)sizeof(long)) : sc->args_size; + int args_size = raw_augmented_args_size ?: sc->args_size; *augmented_args_size = sample->raw_size - args_size; if (*augmented_args_size > 0) @@ -1839,7 +1845,7 @@ static int trace__sys_enter(struct trace *trace, struct perf_evsel *evsel, * here and avoid using augmented syscalls when the evsel is the raw_syscalls one. */ if (evsel != trace->syscalls.events.sys_enter) - augmented_args = syscall__augmented_args(sc, sample, &augmented_args_size, trace->raw_augmented_syscalls); + augmented_args = syscall__augmented_args(sc, sample, &augmented_args_size, trace->raw_augmented_syscalls_args_size); ttrace->entry_time = sample->time; msg = ttrace->entry_str; printed += scnprintf(msg + printed, trace__entry_str_size - printed, "%s(", sc->name); @@ -1897,7 +1903,7 @@ static int trace__fprintf_sys_enter(struct trace *trace, struct perf_evsel *evse goto out_put; args = perf_evsel__sc_tp_ptr(evsel, args, sample); - augmented_args = syscall__augmented_args(sc, sample, &augmented_args_size, trace->raw_augmented_syscalls); + augmented_args = syscall__augmented_args(sc, sample, &augmented_args_size, trace->raw_augmented_syscalls_args_size); syscall__scnprintf_args(sc, msg, sizeof(msg), args, augmented_args, augmented_args_size, trace, thread); fprintf(trace->output, "%s", msg); err = 0; @@ -3814,13 +3820,6 @@ int cmd_trace(int argc, const char **argv) * syscall. */ if (trace.syscalls.events.augmented) { - evsel = trace.syscalls.events.augmented; - - if (perf_evsel__init_augmented_syscall_tp(evsel) || - perf_evsel__init_augmented_syscall_tp_args(evsel)) - goto out; - evsel->handler = trace__sys_enter; - evlist__for_each_entry(trace.evlist, evsel) { bool raw_syscalls_sys_exit = strcmp(perf_evsel__name(evsel), "raw_syscalls:sys_exit") == 0; @@ -3829,9 +3828,41 @@ int cmd_trace(int argc, const char **argv) goto init_augmented_syscall_tp; } + if (strcmp(perf_evsel__name(evsel), "raw_syscalls:sys_enter") == 0) { + struct perf_evsel *augmented = trace.syscalls.events.augmented; + if (perf_evsel__init_augmented_syscall_tp(augmented, evsel) || + perf_evsel__init_augmented_syscall_tp_args(augmented)) + goto out; + augmented->handler = trace__sys_enter; + } + if (strstarts(perf_evsel__name(evsel), "syscalls:sys_exit_")) { + struct syscall_tp *sc; init_augmented_syscall_tp: - perf_evsel__init_augmented_syscall_tp(evsel); + if (perf_evsel__init_augmented_syscall_tp(evsel, evsel)) + goto out; + sc = evsel->priv; + /* + * For now with BPF raw_augmented we hook into + * raw_syscalls:sys_enter and there we get all + * 6 syscall args plus the tracepoint common + * fields and the syscall_nr (another long). + * So we check if that is the case and if so + * don't look after the sc->args_size but + * always after the full raw_syscalls:sys_enter + * payload, which is fixed. + * + * We'll revisit this later to pass + * s->args_size to the BPF augmenter (now + * tools/perf/examples/bpf/augmented_raw_syscalls.c, + * so that it copies only what we need for each + * syscall, like what happens when we use + * syscalls:sys_enter_NAME, so that we reduce + * the kernel/userspace traffic to just what is + * needed for each syscall. + */ + if (trace.raw_augmented_syscalls) + trace.raw_augmented_syscalls_args_size = (6 + 1) * sizeof(long) + sc->id.offset; perf_evsel__init_augmented_syscall_tp_ret(evsel); evsel->handler = trace__sys_exit; } -- cgit v1.2.3 From fbe7e42515af6c2ecee04b1c851f78de1d190281 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Fri, 21 Dec 2018 08:48:09 +0000 Subject: perf trace: Use correct SECCOMP prefix spelling, "SECOMP_*" -> "SECCOMP_*" The spelling of the SECCOMP is incorrect, fix these. Signed-off-by: Colin King Cc: Alexander Shishkin Cc: Peter Zijlstra Cc: kernel-janitors@vger.kernel.org Fixes: c65c83ffe904 ("perf trace: Allow asking for not suppressing common string prefixes") Link: http://lkml.kernel.org/r/20181221084809.6108-1-colin.king@canonical.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/trace/beauty/seccomp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/perf/trace/beauty/seccomp.c b/tools/perf/trace/beauty/seccomp.c index 4600c28a3cfe7..637722e2796bc 100644 --- a/tools/perf/trace/beauty/seccomp.c +++ b/tools/perf/trace/beauty/seccomp.c @@ -9,7 +9,7 @@ static size_t syscall_arg__scnprintf_seccomp_op(char *bf, size_t size, struct syscall_arg *arg) { bool show_prefix = arg->show_string_prefix; - const char *prefix = "SECOMP_SET_MODE_"; + const char *prefix = "SECCOMP_SET_MODE_"; int op = arg->val; size_t printed = 0; @@ -34,7 +34,7 @@ static size_t syscall_arg__scnprintf_seccomp_flags(char *bf, size_t size, struct syscall_arg *arg) { bool show_prefix = arg->show_string_prefix; - const char *prefix = "SECOMP_FILTER_FLAG_"; + const char *prefix = "SECCOMP_FILTER_FLAG_"; int printed = 0, flags = arg->val; #define P_FLAG(n) \ -- cgit v1.2.3 From a389aece97938966616ce0336466b98b0351ef10 Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Wed, 26 Dec 2018 12:21:21 +0100 Subject: perf python: Do not force closing original perf descriptor in evlist.get_pollfd() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ondřej reported that when compiled with python3, the python extension regresses in evlist.get_pollfd function behaviour. The evlist.get_pollfd function creates file objects from evlist's fds and returns them in a list. The python3 version also sets them to 'close the original descriptor' when the object dies (is closed), by passing True via the 'closefd' arg in the PyFile_FromFd call. The python's closefd doc says: If closefd is False, the underlying file descriptor will be kept open when the file is closed. That's why the following line in python3 closes all evlist fds: evlist.get_pollfd() the returned list is immediately destroyed and that takes down the original events fds. Passing closefd as False to PyFile_FromFd to fix this. Reported-by: Ondřej Lysoněk Signed-off-by: Jiri Olsa Cc: Alexander Shishkin Cc: Jaroslav Škarvada Cc: Namhyung Kim Cc: Peter Zijlstra Fixes: 66dfdff03d19 ("perf tools: Add Python 3 support") Link: http://lkml.kernel.org/r/20181226112121.5285-1-jolsa@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/python.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c index 47628e85c5ebe..dda0ac978b1eb 100644 --- a/tools/perf/util/python.c +++ b/tools/perf/util/python.c @@ -939,7 +939,8 @@ static PyObject *pyrf_evlist__get_pollfd(struct pyrf_evlist *pevlist, file = PyFile_FromFile(fp, "perf", "r", NULL); #else - file = PyFile_FromFd(evlist->pollfd.entries[i].fd, "perf", "r", -1, NULL, NULL, NULL, 1); + file = PyFile_FromFd(evlist->pollfd.entries[i].fd, "perf", "r", -1, + NULL, NULL, NULL, 0); #endif if (file == NULL) goto free_list; -- cgit v1.2.3 From 61f611593f2c90547cb09c0bf6977414454a27e6 Mon Sep 17 00:00:00 2001 From: Andi Kleen Date: Mon, 19 Nov 2018 21:06:17 -0800 Subject: perf script: Fix LBR skid dump problems in brstackinsn This is a fix for another instance of the skid problem Milian recently found [1] The LBRs don't freeze at the exact same time as the PMI is triggered. The perf script brstackinsn code that dumps LBR assembler assumes that the last branch in the LBR leads to the sample point. But with skid it's possible that the CPU executes one or more branches before the sample, but which do not appear in the LBR. What happens then is either that the sample point is before the last LBR branch. In this case the dumper sees a negative length and ignores it. Or it the sample point is long after the last branch. Then the dumper sees a very long block and dumps it upto its block limit (16k bytes), which is noise in the output. On typical sample session this can happen regularly. This patch tries to detect and handle the situation. On the last block that is dumped by the LBR dumper we always stop on the first branch. If the block length is negative just scan forward to the first branch. Otherwise scan until a branch is found. The PT decoder already has a function that uses the instruction decoder to detect branches, so we can just reuse it here. Then when a terminating branch is found print an indication and stop dumping. This might miss a few instructions, but at least shows no runaway blocks. Signed-off-by: Andi Kleen Acked-by: Adrian Hunter Cc: Jiri Olsa Cc: Milian Wolff Link: http://lkml.kernel.org/r/20181120050617.4119-1-andi@firstfloor.org [ Resolved conflict with dd2e18e9ac20 ("perf tools: Support 'srccode' output") ] Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-script.c | 17 ++++++++++++++++- tools/perf/util/dump-insn.c | 8 ++++++++ tools/perf/util/dump-insn.h | 2 ++ .../perf/util/intel-pt-decoder/intel-pt-insn-decoder.c | 8 ++++++++ 4 files changed, 34 insertions(+), 1 deletion(-) diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c index 3728b50e52e20..88d52ed85ffc1 100644 --- a/tools/perf/builtin-script.c +++ b/tools/perf/builtin-script.c @@ -1073,9 +1073,18 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample, /* * Print final block upto sample + * + * Due to pipeline delays the LBRs might be missing a branch + * or two, which can result in very large or negative blocks + * between final branch and sample. When this happens just + * continue walking after the last TO until we hit a branch. */ start = br->entries[0].to; end = sample->ip; + if (end < start) { + /* Missing jump. Scan 128 bytes for the next branch */ + end = start + 128; + } len = grab_bb(buffer, start, end, machine, thread, &x.is64bit, &x.cpumode, true); printed += ip__fprintf_sym(start, thread, x.cpumode, x.cpu, &lastsym, attr, fp); if (len <= 0) { @@ -1084,7 +1093,6 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample, machine, thread, &x.is64bit, &x.cpumode, false); if (len <= 0) goto out; - printed += fprintf(fp, "\t%016" PRIx64 "\t%s\n", sample->ip, dump_insn(&x, sample->ip, buffer, len, NULL)); if (PRINT_FIELD(SRCCODE)) @@ -1096,6 +1104,13 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample, dump_insn(&x, start + off, buffer + off, len - off, &ilen)); if (ilen == 0) break; + if (arch_is_branch(buffer + off, len - off, x.is64bit) && start + off != sample->ip) { + /* + * Hit a missing branch. Just stop. + */ + printed += fprintf(fp, "\t... not reaching sample ...\n"); + break; + } if (PRINT_FIELD(SRCCODE)) print_srccode(thread, x.cpumode, start + off); } diff --git a/tools/perf/util/dump-insn.c b/tools/perf/util/dump-insn.c index 10988d3de7cec..2bd8585db93c3 100644 --- a/tools/perf/util/dump-insn.c +++ b/tools/perf/util/dump-insn.c @@ -13,3 +13,11 @@ const char *dump_insn(struct perf_insn *x __maybe_unused, *lenp = 0; return "?"; } + +__weak +int arch_is_branch(const unsigned char *buf __maybe_unused, + size_t len __maybe_unused, + int x86_64 __maybe_unused) +{ + return 0; +} diff --git a/tools/perf/util/dump-insn.h b/tools/perf/util/dump-insn.h index 0e06280a88602..6501250615309 100644 --- a/tools/perf/util/dump-insn.h +++ b/tools/perf/util/dump-insn.h @@ -20,4 +20,6 @@ struct perf_insn { const char *dump_insn(struct perf_insn *x, u64 ip, u8 *inbuf, int inlen, int *lenp); +int arch_is_branch(const unsigned char *buf, size_t len, int x86_64); + #endif diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c b/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c index 54818828023bf..1c0e289f01e6f 100644 --- a/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c +++ b/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c @@ -180,6 +180,14 @@ int intel_pt_get_insn(const unsigned char *buf, size_t len, int x86_64, return 0; } +int arch_is_branch(const unsigned char *buf, size_t len, int x86_64) +{ + struct intel_pt_insn in; + if (intel_pt_get_insn(buf, len, x86_64, &in) < 0) + return -1; + return in.branch != INTEL_PT_BR_NO_BRANCH; +} + const char *dump_insn(struct perf_insn *x, uint64_t ip __maybe_unused, u8 *inbuf, int inlen, int *lenp) { -- cgit v1.2.3 From f4a74fcbfd943e128bbd464dc31b4405e1514d63 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Thu, 27 Dec 2018 10:56:12 -0300 Subject: perf trace: Rename thread_thread->paths to thread_trace->files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit So that we can add more per file attributes besides the pathname, such as which ioctl beautifier to use, for cases such as the sound and usbdeffs ioctls, that both use the 'U' command, so we have to differentiate at the major number for the device file. Cc: Adrian Hunter Cc: Jiri Olsa Cc: Luis Cláudio Gonçalves Cc: Namhyung Kim Cc: Wang Nan Link: https://lkml.kernel.org/n/tip-1895cmhrdz2dkl5prf2cj2yj@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-trace.c | 42 +++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c index 1e9e886b28115..d4bca74f282c2 100644 --- a/tools/perf/builtin-trace.c +++ b/tools/perf/builtin-trace.c @@ -957,6 +957,10 @@ static size_t fprintf_duration(unsigned long t, bool calculated, FILE *fp) return printed + fprintf(fp, "): "); } +struct file { + char *pathname; +}; + /** * filename.ptr: The filename char pointer that will be vfs_getname'd * filename.entry_str_pos: Where to insert the string translated from @@ -980,9 +984,9 @@ struct thread_trace { char *name; } filename; struct { - int max; - char **table; - } paths; + int max; + struct file *table; + } files; struct intlist *syscall_stats; }; @@ -992,7 +996,7 @@ static struct thread_trace *thread_trace__new(void) struct thread_trace *ttrace = zalloc(sizeof(struct thread_trace)); if (ttrace) - ttrace->paths.max = -1; + ttrace->files.max = -1; ttrace->syscall_stats = intlist__new(NULL); @@ -1040,26 +1044,26 @@ static int trace__set_fd_pathname(struct thread *thread, int fd, const char *pat { struct thread_trace *ttrace = thread__priv(thread); - if (fd > ttrace->paths.max) { - char **npath = realloc(ttrace->paths.table, (fd + 1) * sizeof(char *)); + if (fd > ttrace->files.max) { + struct file *nfiles = realloc(ttrace->files.table, (fd + 1) * sizeof(struct file)); - if (npath == NULL) + if (nfiles == NULL) return -1; - if (ttrace->paths.max != -1) { - memset(npath + ttrace->paths.max + 1, 0, - (fd - ttrace->paths.max) * sizeof(char *)); + if (ttrace->files.max != -1) { + memset(nfiles + ttrace->files.max + 1, 0, + (fd - ttrace->files.max) * sizeof(struct file)); } else { - memset(npath, 0, (fd + 1) * sizeof(char *)); + memset(nfiles, 0, (fd + 1) * sizeof(struct file)); } - ttrace->paths.table = npath; - ttrace->paths.max = fd; + ttrace->files.table = nfiles; + ttrace->files.max = fd; } - ttrace->paths.table[fd] = strdup(pathname); + ttrace->files.table[fd].pathname = strdup(pathname); - return ttrace->paths.table[fd] != NULL ? 0 : -1; + return ttrace->files.table[fd].pathname != NULL ? 0 : -1; } static int thread__read_fd_path(struct thread *thread, int fd) @@ -1099,7 +1103,7 @@ static const char *thread__fd_path(struct thread *thread, int fd, if (fd < 0) return NULL; - if ((fd > ttrace->paths.max || ttrace->paths.table[fd] == NULL)) { + if ((fd > ttrace->files.max || ttrace->files.table[fd].pathname == NULL)) { if (!trace->live) return NULL; ++trace->stats.proc_getname; @@ -1107,7 +1111,7 @@ static const char *thread__fd_path(struct thread *thread, int fd, return NULL; } - return ttrace->paths.table[fd]; + return ttrace->files.table[fd].pathname; } size_t syscall_arg__scnprintf_fd(char *bf, size_t size, struct syscall_arg *arg) @@ -1146,8 +1150,8 @@ static size_t syscall_arg__scnprintf_close_fd(char *bf, size_t size, size_t printed = syscall_arg__scnprintf_fd(bf, size, arg); struct thread_trace *ttrace = thread__priv(arg->thread); - if (ttrace && fd >= 0 && fd <= ttrace->paths.max) - zfree(&ttrace->paths.table[fd]); + if (ttrace && fd >= 0 && fd <= ttrace->files.max) + zfree(&ttrace->files.table[fd].pathname); return printed; } -- cgit v1.2.3 From d7e134845d6b19288dec5582ce91d6c6052fcdad Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Thu, 27 Dec 2018 11:05:18 -0300 Subject: perf trace: Move the files table resizing to outside set_pathname() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit So that we can have that table expanded when setting other attributes. Cc: Adrian Hunter Cc: Jiri Olsa Cc: Luis Cláudio Gonçalves Cc: Namhyung Kim Cc: Wang Nan Link: https://lkml.kernel.org/n/tip-hzvpe3qwafe6sqcq3bhtbxds@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-trace.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c index d4bca74f282c2..41ab524e128b7 100644 --- a/tools/perf/builtin-trace.c +++ b/tools/perf/builtin-trace.c @@ -1040,15 +1040,13 @@ void syscall_arg__set_ret_scnprintf(struct syscall_arg *arg, static const size_t trace__entry_str_size = 2048; -static int trace__set_fd_pathname(struct thread *thread, int fd, const char *pathname) +static struct file *thread_trace__files_entry(struct thread_trace *ttrace, int fd) { - struct thread_trace *ttrace = thread__priv(thread); - if (fd > ttrace->files.max) { struct file *nfiles = realloc(ttrace->files.table, (fd + 1) * sizeof(struct file)); if (nfiles == NULL) - return -1; + return NULL; if (ttrace->files.max != -1) { memset(nfiles + ttrace->files.max + 1, 0, @@ -1061,9 +1059,21 @@ static int trace__set_fd_pathname(struct thread *thread, int fd, const char *pat ttrace->files.max = fd; } - ttrace->files.table[fd].pathname = strdup(pathname); + return ttrace->files.table + fd; +} + +static int trace__set_fd_pathname(struct thread *thread, int fd, const char *pathname) +{ + struct thread_trace *ttrace = thread__priv(thread); + struct file *file = thread_trace__files_entry(ttrace, fd); + + if (file != NULL) { + file->pathname = strdup(pathname); + if (file->pathname) + return 0; + } - return ttrace->files.table[fd].pathname != NULL ? 0 : -1; + return -1; } static int thread__read_fd_path(struct thread *thread, int fd) -- cgit v1.2.3 From 4bcc4cff6a10e5287d4dd3e73b6b4689f9a0c570 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Thu, 27 Dec 2018 13:32:59 -0300 Subject: perf trace: Store the major number for a file when storing its pathname MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We keep a table for the fds to map them back to pathnames when showing 'fd' based APIs such as write(), store as well the major number for the device the path is in, to use in things like choosing the right ioctl 'cmd' beautifier. Cc: Adrian Hunter Cc: Jiri Olsa Cc: Luis Cláudio Gonçalves Cc: Namhyung Kim Cc: Wang Nan Link: https://lkml.kernel.org/n/tip-qjkds7bnk7v7fk2xhqsb0a4v@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-trace.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c index 41ab524e128b7..4b03f66b17356 100644 --- a/tools/perf/builtin-trace.c +++ b/tools/perf/builtin-trace.c @@ -60,6 +60,7 @@ #include #include #include +#include #include "sane_ctype.h" @@ -959,6 +960,7 @@ static size_t fprintf_duration(unsigned long t, bool calculated, FILE *fp) struct file { char *pathname; + int dev_maj; }; /** @@ -1068,6 +1070,9 @@ static int trace__set_fd_pathname(struct thread *thread, int fd, const char *pat struct file *file = thread_trace__files_entry(ttrace, fd); if (file != NULL) { + struct stat st; + if (stat(pathname, &st) == 0) + file->dev_maj = major(st.st_rdev); file->pathname = strdup(pathname); if (file->pathname) return 0; -- cgit v1.2.3 From 2bd71d11a86e8419a33a131d194fd735a8f74400 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Thu, 27 Dec 2018 14:18:13 -0300 Subject: tools headers uapi: Grab a copy of usbdevice_fs.h MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Will be used to generate the string table for the USBDEVFS_ prefixed ioctl commands. Cc: Adrian Hunter Cc: Jiri Olsa Cc: Luis Cláudio Gonçalves Cc: Namhyung Kim Cc: Wang Nan Link: https://lkml.kernel.org/n/tip-3vrm9b55tdhzn8sw9qazh4z5@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/include/uapi/linux/usbdevice_fs.h | 201 ++++++++++++++++++++++++++++++++ tools/perf/check-headers.sh | 1 + 2 files changed, 202 insertions(+) create mode 100644 tools/include/uapi/linux/usbdevice_fs.h diff --git a/tools/include/uapi/linux/usbdevice_fs.h b/tools/include/uapi/linux/usbdevice_fs.h new file mode 100644 index 0000000000000..964e87217be4e --- /dev/null +++ b/tools/include/uapi/linux/usbdevice_fs.h @@ -0,0 +1,201 @@ +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */ +/*****************************************************************************/ + +/* + * usbdevice_fs.h -- USB device file system. + * + * Copyright (C) 2000 + * Thomas Sailer (sailer@ife.ee.ethz.ch) + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. + * + * History: + * 0.1 04.01.2000 Created + */ + +/*****************************************************************************/ + +#ifndef _UAPI_LINUX_USBDEVICE_FS_H +#define _UAPI_LINUX_USBDEVICE_FS_H + +#include +#include + +/* --------------------------------------------------------------------- */ + +/* usbdevfs ioctl codes */ + +struct usbdevfs_ctrltransfer { + __u8 bRequestType; + __u8 bRequest; + __u16 wValue; + __u16 wIndex; + __u16 wLength; + __u32 timeout; /* in milliseconds */ + void __user *data; +}; + +struct usbdevfs_bulktransfer { + unsigned int ep; + unsigned int len; + unsigned int timeout; /* in milliseconds */ + void __user *data; +}; + +struct usbdevfs_setinterface { + unsigned int interface; + unsigned int altsetting; +}; + +struct usbdevfs_disconnectsignal { + unsigned int signr; + void __user *context; +}; + +#define USBDEVFS_MAXDRIVERNAME 255 + +struct usbdevfs_getdriver { + unsigned int interface; + char driver[USBDEVFS_MAXDRIVERNAME + 1]; +}; + +struct usbdevfs_connectinfo { + unsigned int devnum; + unsigned char slow; +}; + +#define USBDEVFS_URB_SHORT_NOT_OK 0x01 +#define USBDEVFS_URB_ISO_ASAP 0x02 +#define USBDEVFS_URB_BULK_CONTINUATION 0x04 +#define USBDEVFS_URB_NO_FSBR 0x20 /* Not used */ +#define USBDEVFS_URB_ZERO_PACKET 0x40 +#define USBDEVFS_URB_NO_INTERRUPT 0x80 + +#define USBDEVFS_URB_TYPE_ISO 0 +#define USBDEVFS_URB_TYPE_INTERRUPT 1 +#define USBDEVFS_URB_TYPE_CONTROL 2 +#define USBDEVFS_URB_TYPE_BULK 3 + +struct usbdevfs_iso_packet_desc { + unsigned int length; + unsigned int actual_length; + unsigned int status; +}; + +struct usbdevfs_urb { + unsigned char type; + unsigned char endpoint; + int status; + unsigned int flags; + void __user *buffer; + int buffer_length; + int actual_length; + int start_frame; + union { + int number_of_packets; /* Only used for isoc urbs */ + unsigned int stream_id; /* Only used with bulk streams */ + }; + int error_count; + unsigned int signr; /* signal to be sent on completion, + or 0 if none should be sent. */ + void __user *usercontext; + struct usbdevfs_iso_packet_desc iso_frame_desc[0]; +}; + +/* ioctls for talking directly to drivers */ +struct usbdevfs_ioctl { + int ifno; /* interface 0..N ; negative numbers reserved */ + int ioctl_code; /* MUST encode size + direction of data so the + * macros in give correct values */ + void __user *data; /* param buffer (in, or out) */ +}; + +/* You can do most things with hubs just through control messages, + * except find out what device connects to what port. */ +struct usbdevfs_hub_portinfo { + char nports; /* number of downstream ports in this hub */ + char port [127]; /* e.g. port 3 connects to device 27 */ +}; + +/* System and bus capability flags */ +#define USBDEVFS_CAP_ZERO_PACKET 0x01 +#define USBDEVFS_CAP_BULK_CONTINUATION 0x02 +#define USBDEVFS_CAP_NO_PACKET_SIZE_LIM 0x04 +#define USBDEVFS_CAP_BULK_SCATTER_GATHER 0x08 +#define USBDEVFS_CAP_REAP_AFTER_DISCONNECT 0x10 +#define USBDEVFS_CAP_MMAP 0x20 +#define USBDEVFS_CAP_DROP_PRIVILEGES 0x40 + +/* USBDEVFS_DISCONNECT_CLAIM flags & struct */ + +/* disconnect-and-claim if the driver matches the driver field */ +#define USBDEVFS_DISCONNECT_CLAIM_IF_DRIVER 0x01 +/* disconnect-and-claim except when the driver matches the driver field */ +#define USBDEVFS_DISCONNECT_CLAIM_EXCEPT_DRIVER 0x02 + +struct usbdevfs_disconnect_claim { + unsigned int interface; + unsigned int flags; + char driver[USBDEVFS_MAXDRIVERNAME + 1]; +}; + +struct usbdevfs_streams { + unsigned int num_streams; /* Not used by USBDEVFS_FREE_STREAMS */ + unsigned int num_eps; + unsigned char eps[0]; +}; + +/* + * USB_SPEED_* values returned by USBDEVFS_GET_SPEED are defined in + * linux/usb/ch9.h + */ + +#define USBDEVFS_CONTROL _IOWR('U', 0, struct usbdevfs_ctrltransfer) +#define USBDEVFS_CONTROL32 _IOWR('U', 0, struct usbdevfs_ctrltransfer32) +#define USBDEVFS_BULK _IOWR('U', 2, struct usbdevfs_bulktransfer) +#define USBDEVFS_BULK32 _IOWR('U', 2, struct usbdevfs_bulktransfer32) +#define USBDEVFS_RESETEP _IOR('U', 3, unsigned int) +#define USBDEVFS_SETINTERFACE _IOR('U', 4, struct usbdevfs_setinterface) +#define USBDEVFS_SETCONFIGURATION _IOR('U', 5, unsigned int) +#define USBDEVFS_GETDRIVER _IOW('U', 8, struct usbdevfs_getdriver) +#define USBDEVFS_SUBMITURB _IOR('U', 10, struct usbdevfs_urb) +#define USBDEVFS_SUBMITURB32 _IOR('U', 10, struct usbdevfs_urb32) +#define USBDEVFS_DISCARDURB _IO('U', 11) +#define USBDEVFS_REAPURB _IOW('U', 12, void *) +#define USBDEVFS_REAPURB32 _IOW('U', 12, __u32) +#define USBDEVFS_REAPURBNDELAY _IOW('U', 13, void *) +#define USBDEVFS_REAPURBNDELAY32 _IOW('U', 13, __u32) +#define USBDEVFS_DISCSIGNAL _IOR('U', 14, struct usbdevfs_disconnectsignal) +#define USBDEVFS_DISCSIGNAL32 _IOR('U', 14, struct usbdevfs_disconnectsignal32) +#define USBDEVFS_CLAIMINTERFACE _IOR('U', 15, unsigned int) +#define USBDEVFS_RELEASEINTERFACE _IOR('U', 16, unsigned int) +#define USBDEVFS_CONNECTINFO _IOW('U', 17, struct usbdevfs_connectinfo) +#define USBDEVFS_IOCTL _IOWR('U', 18, struct usbdevfs_ioctl) +#define USBDEVFS_IOCTL32 _IOWR('U', 18, struct usbdevfs_ioctl32) +#define USBDEVFS_HUB_PORTINFO _IOR('U', 19, struct usbdevfs_hub_portinfo) +#define USBDEVFS_RESET _IO('U', 20) +#define USBDEVFS_CLEAR_HALT _IOR('U', 21, unsigned int) +#define USBDEVFS_DISCONNECT _IO('U', 22) +#define USBDEVFS_CONNECT _IO('U', 23) +#define USBDEVFS_CLAIM_PORT _IOR('U', 24, unsigned int) +#define USBDEVFS_RELEASE_PORT _IOR('U', 25, unsigned int) +#define USBDEVFS_GET_CAPABILITIES _IOR('U', 26, __u32) +#define USBDEVFS_DISCONNECT_CLAIM _IOR('U', 27, struct usbdevfs_disconnect_claim) +#define USBDEVFS_ALLOC_STREAMS _IOR('U', 28, struct usbdevfs_streams) +#define USBDEVFS_FREE_STREAMS _IOR('U', 29, struct usbdevfs_streams) +#define USBDEVFS_DROP_PRIVILEGES _IOW('U', 30, __u32) +#define USBDEVFS_GET_SPEED _IO('U', 31) + +#endif /* _UAPI_LINUX_USBDEVICE_FS_H */ diff --git a/tools/perf/check-headers.sh b/tools/perf/check-headers.sh index 8e811ea0cf851..6cb98f8570a22 100755 --- a/tools/perf/check-headers.sh +++ b/tools/perf/check-headers.sh @@ -14,6 +14,7 @@ include/uapi/linux/perf_event.h include/uapi/linux/prctl.h include/uapi/linux/sched.h include/uapi/linux/stat.h +include/uapi/linux/usbdevice_fs.h include/uapi/linux/vhost.h include/uapi/sound/asound.h include/linux/bits.h -- cgit v1.2.3 From 870c3f40dc78edec6437a2ced03976b66eeacf49 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Thu, 27 Dec 2018 14:27:35 -0300 Subject: perf beauty ioctl: Add generator for USBDEVFS_ ioctl commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Will be associated with fds with the right device major. $ tools/perf/trace/beauty/usbdevfs_ioctl.sh static const char *usbdevfs_ioctl_cmds[] = { [0] = "CONTROL", [10] = "SUBMITURB", [11] = "DISCARDURB", [12] = "REAPURB", [13] = "REAPURBNDELAY", [14] = "DISCSIGNAL", [15] = "CLAIMINTERFACE", [16] = "RELEASEINTERFACE", [17] = "CONNECTINFO", [18] = "IOCTL", [19] = "HUB_PORTINFO", [20] = "RESET", [21] = "CLEAR_HALT", [22] = "DISCONNECT", [23] = "CONNECT", [24] = "CLAIM_PORT", [25] = "RELEASE_PORT", [26] = "GET_CAPABILITIES", [27] = "DISCONNECT_CLAIM", [28] = "ALLOC_STREAMS", [29] = "FREE_STREAMS", [2] = "BULK", [30] = "DROP_PRIVILEGES", [31] = "GET_SPEED", [3] = "RESETEP", [4] = "SETINTERFACE", [5] = "SETCONFIGURATION", [8] = "GETDRIVER", }; #if 0 static const char *usbdevfs_ioctl_32_cmds[] = { [0] = "CONTROL32", [10] = "SUBMITURB32", [12] = "REAPURB32", [13] = "REAPURBNDELAY32", [14] = "DISCSIGNAL32", [18] = "IOCTL32", [2] = "BULK32", }; #endif $ Leaving the '32' variants commented, later we can try to support those as well, from some other hint (maybe something about the thread issuing the ioctls) and from the _IOC_SIZE(cmd). Cc: Adrian Hunter Cc: Jiri Olsa Cc: Luis Cláudio Gonçalves Cc: Namhyung Kim Cc: Wang Nan Link: https://lkml.kernel.org/n/tip-neq1lrji5k4ku0rktn7ytnri@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/trace/beauty/usbdevfs_ioctl.sh | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100755 tools/perf/trace/beauty/usbdevfs_ioctl.sh diff --git a/tools/perf/trace/beauty/usbdevfs_ioctl.sh b/tools/perf/trace/beauty/usbdevfs_ioctl.sh new file mode 100755 index 0000000000000..930b80f422e83 --- /dev/null +++ b/tools/perf/trace/beauty/usbdevfs_ioctl.sh @@ -0,0 +1,19 @@ +#!/bin/sh +# SPDX-License-Identifier: LGPL-2.1 + +[ $# -eq 1 ] && header_dir=$1 || header_dir=tools/include/uapi/linux/ + +printf "static const char *usbdevfs_ioctl_cmds[] = {\n" +regex="^#[[:space:]]*define[[:space:]]+USBDEVFS_(\w+)[[:space:]]+_IO[WR]{0,2}\([[:space:]]*'U'[[:space:]]*,[[:space:]]*([[:digit:]]+).*" +egrep $regex ${header_dir}/usbdevice_fs.h | egrep -v 'USBDEVFS_\w+32[[:space:]]' | \ + sed -r "s/$regex/\2 \1/g" | \ + sort | xargs printf "\t[%s] = \"%s\",\n" +printf "};\n\n" +printf "#if 0\n" +printf "static const char *usbdevfs_ioctl_32_cmds[] = {\n" +regex="^#[[:space:]]*define[[:space:]]+USBDEVFS_(\w+)[[:space:]]+_IO[WR]{0,2}\([[:space:]]*'U'[[:space:]]*,[[:space:]]*([[:digit:]]+).*" +egrep $regex ${header_dir}/usbdevice_fs.h | egrep 'USBDEVFS_\w+32[[:space:]]' | \ + sed -r "s/$regex/\2 \1/g" | \ + sort | xargs printf "\t[%s] = \"%s\",\n" +printf "};\n" +printf "#endif\n" -- cgit v1.2.3 From 86cf4c659c9ae1609262699caa4b2c3ab7fefa9b Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Thu, 27 Dec 2018 15:10:34 -0300 Subject: perf trace: Wire up ioctl's USBDEBFS_ cmd table generator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit That ends up generating this: [acme@quaco perf]$ cat /tmp/build/perf/trace/beauty/generated/ioctl/usbdevfs_ioctl_array.c static const char *usbdevfs_ioctl_cmds[] = { [0] = "CONTROL", [10] = "SUBMITURB", [11] = "DISCARDURB", [12] = "REAPURB", [13] = "REAPURBNDELAY", [14] = "DISCSIGNAL", [15] = "CLAIMINTERFACE", [16] = "RELEASEINTERFACE", [17] = "CONNECTINFO", [18] = "IOCTL", [19] = "HUB_PORTINFO", [2] = "BULK", [20] = "RESET", [21] = "CLEAR_HALT", [22] = "DISCONNECT", [23] = "CONNECT", [24] = "CLAIM_PORT", [25] = "RELEASE_PORT", [26] = "GET_CAPABILITIES", [27] = "DISCONNECT_CLAIM", [28] = "ALLOC_STREAMS", [29] = "FREE_STREAMS", [3] = "RESETEP", [30] = "DROP_PRIVILEGES", [31] = "GET_SPEED", [4] = "SETINTERFACE", [5] = "SETCONFIGURATION", [8] = "GETDRIVER", }; #if 0 static const char *usbdevfs_ioctl_32_cmds[] = { [0] = "CONTROL32", [10] = "SUBMITURB32", [12] = "REAPURB32", [13] = "REAPURBNDELAY32", [14] = "DISCSIGNAL32", [18] = "IOCTL32", [2] = "BULK32", }; #endif $ Cc: Adrian Hunter Cc: Jiri Olsa Cc: Luis Cláudio Gonçalves Cc: Namhyung Kim Cc: Wang Nan Link: https://lkml.kernel.org/n/tip-hkam6lt1g806l0p4b7buif3n@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/Makefile.perf | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf index bd23e3f308953..ff29c3372ec3b 100644 --- a/tools/perf/Makefile.perf +++ b/tools/perf/Makefile.perf @@ -497,6 +497,12 @@ prctl_option_tbl := $(srctree)/tools/perf/trace/beauty/prctl_option.sh $(prctl_option_array): $(prctl_hdr_dir)/prctl.h $(prctl_option_tbl) $(Q)$(SHELL) '$(prctl_option_tbl)' $(prctl_hdr_dir) > $@ +usbdevfs_ioctl_array := $(beauty_ioctl_outdir)/usbdevfs_ioctl_array.c +usbdevfs_ioctl_tbl := $(srctree)/tools/perf/trace/beauty/usbdevfs_ioctl.sh + +$(usbdevfs_ioctl_array): $(linux_uapi_dir)/usbdevice_fs.h $(usbdevfs_ioctl_tbl) + $(Q)$(SHELL) '$(usbdevfs_ioctl_tbl)' $(linux_uapi_dir) > $@ + x86_arch_prctl_code_array := $(beauty_outdir)/x86_arch_prctl_code_array.c x86_arch_prctl_code_tbl := $(srctree)/tools/perf/trace/beauty/x86_arch_prctl.sh @@ -624,6 +630,7 @@ prepare: $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)common-cmds.h archheaders $(drm_ioc $(mount_flags_array) \ $(perf_ioctl_array) \ $(prctl_option_array) \ + $(usbdevfs_ioctl_array) \ $(x86_arch_prctl_code_array) \ $(rename_flags_array) \ $(arch_errno_name_array) @@ -923,6 +930,7 @@ clean:: $(LIBTRACEEVENT)-clean $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clea $(OUTPUT)$(vhost_virtio_ioctl_array) \ $(OUTPUT)$(perf_ioctl_array) \ $(OUTPUT)$(prctl_option_array) \ + $(OUTPUT)$(usbdevfs_ioctl_array) \ $(OUTPUT)$(x86_arch_prctl_code_array) \ $(OUTPUT)$(rename_flags_array) \ $(OUTPUT)$(arch_errno_name_array) -- cgit v1.2.3 From 2d473389f87ace284bda35a5254bc66e24d0caa9 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Thu, 27 Dec 2018 16:19:33 -0300 Subject: perf trace beauty: Export function to get the files for a thread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit So that beautifiers can access things like dev_maj. Cc: Adrian Hunter Cc: Jiri Olsa Cc: Luis Cláudio Gonçalves Cc: Namhyung Kim Cc: Wang Nan Link: https://lkml.kernel.org/n/tip-wm5o51f206c5pi063dsaeraq@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-trace.c | 10 +++++----- tools/perf/trace/beauty/beauty.h | 7 +++++++ 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c index 4b03f66b17356..adbf28183560a 100644 --- a/tools/perf/builtin-trace.c +++ b/tools/perf/builtin-trace.c @@ -958,11 +958,6 @@ static size_t fprintf_duration(unsigned long t, bool calculated, FILE *fp) return printed + fprintf(fp, "): "); } -struct file { - char *pathname; - int dev_maj; -}; - /** * filename.ptr: The filename char pointer that will be vfs_getname'd * filename.entry_str_pos: Where to insert the string translated from @@ -1064,6 +1059,11 @@ static struct file *thread_trace__files_entry(struct thread_trace *ttrace, int f return ttrace->files.table + fd; } +struct file *thread__files_entry(struct thread *thread, int fd) +{ + return thread_trace__files_entry(thread__priv(thread), fd); +} + static int trace__set_fd_pathname(struct thread *thread, int fd, const char *pathname) { struct thread_trace *ttrace = thread__priv(thread); diff --git a/tools/perf/trace/beauty/beauty.h b/tools/perf/trace/beauty/beauty.h index 83c5b202e00e8..139d485a6f161 100644 --- a/tools/perf/trace/beauty/beauty.h +++ b/tools/perf/trace/beauty/beauty.h @@ -32,6 +32,13 @@ size_t strarray__scnprintf_flags(struct strarray *sa, char *bf, size_t size, boo struct trace; struct thread; +struct file { + char *pathname; + int dev_maj; +}; + +struct file *thread__files_entry(struct thread *thread, int fd); + struct strarrays { int nr_entries; struct strarray **entries; -- cgit v1.2.3 From 38fc9da69f3385667ba8c3d45bd43065491854a6 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Thu, 27 Dec 2018 16:20:21 -0300 Subject: perf trace beauty ioctl: Beautify USBDEVFS_ commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For instance, while debugging the 'galileo' python utility to synchronize fitbit trackers: # perf trace -e ioctl ./run --force ioctl(0, TCSETS, 0x7ffe28666420) = 0 ioctl(0, TCSETS, 0x7ffe28666290) = 0 ioctl(1, TCSETS, 0x7ffe28666290) = 0 ioctl(2, TCSETS, 0x7ffe28666290) = 0 ioctl(3, TCSETS, 0x7ffe286663f0) = -1 ENOTTY (Inappropriate ioctl for device) ioctl(1, TCSETS, 0x7ffe286655a0) = 0 ioctl(1, TCSETS, 0x7ffe28665470) = 0 ioctl(1, TCSETS, 0x7ffe28665470) = 0 ioctl(1, TCSETS, 0x7ffe286654a0) = 0 ioctl(1, TCSETS, 0x7ffe286654a0) = 0 ioctl(1, TCSETS, 0x7ffe28665400) = 0 ioctl(1, TIOCSWINSZ, 0x7ffe286654c0) = 0 ioctl(0, TIOCSWINSZ, 0x7ffe28665560) = 0 ioctl(0, TIOCSWINSZ, 0x7ffe28665560) = 0 ioctl(0, TIOCMGET, 0x7ffe28665560) = 0 ioctl(0, TCSETS, 0x7ffe28665530) = 0 ioctl(10, USBDEVFS_GET_CAPABILITIES, 0x561468dad048) = 0 ioctl(10, USBDEVFS_GETDRIVER, 0x7ffe28665500) = -1 ENODATA (No data available) ioctl(10, USBDEVFS_GETDRIVER, 0x7ffe28665500) = -1 ENODATA (No data available) ioctl(10, USBDEVFS_SETCONFIGURATION, 0x7ffe2866513c) = 0 ioctl(10, USBDEVFS_CLAIMINTERFACE, 0x7ffe286647bc) = 0 ioctl(10, USBDEVFS_SUBMITURB, 0x561468dace40) = 0 ioctl(10, USBDEVFS_REAPURBNDELAY, 0x7ffe28664c10) = 0 ioctl(10, USBDEVFS_REAPURBNDELAY, 0x7ffe28664c10) = -1 EAGAIN (Resource temporarily unavailable) ioctl(10, USBDEVFS_SUBMITURB, 0x561468dace40) = 0 ioctl(10, USBDEVFS_REAPURBNDELAY, 0x7ffe28664dd0) = 0 ioctl(10, USBDEVFS_REAPURBNDELAY, 0x7ffe28664dd0) = -1 EAGAIN (Resource temporarily unavailable) ioctl(10, USBDEVFS_SUBMITURB, 0x561468e72ec0) = 0 ioctl(10, USBDEVFS_REAPURBNDELAY, 0x7ffe28664cc0) = 0 ioctl(10, USBDEVFS_REAPURBNDELAY, 0x7ffe28664cc0) = -1 EAGAIN (Resource temporarily unavailable) ioctl(10, USBDEVFS_RELEASEINTERFACE, 0x7ffe2866463c) = 0 ioctl(10, USBDEVFS_RELEASEINTERFACE, 0x7ffe2866463c) = 0 Tracker: 813F4690C3D1: Synchronisation successful # Cc: Adrian Hunter Cc: Jiri Olsa Cc: Luis Cláudio Gonçalves Cc: Namhyung Kim Cc: Wang Nan Link: https://lkml.kernel.org/n/tip-6x2cawak7jno3gpp5pagzj50@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/trace/beauty/ioctl.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tools/perf/trace/beauty/ioctl.c b/tools/perf/trace/beauty/ioctl.c index 9efeb6a936c24..620350d412096 100644 --- a/tools/perf/trace/beauty/ioctl.c +++ b/tools/perf/trace/beauty/ioctl.c @@ -112,6 +112,17 @@ static size_t ioctl__scnprintf_perf_cmd(int nr, int dir, char *bf, size_t size) return scnprintf(bf, size, "(%#x, %#x, %#x)", 0xAE, nr, dir); } +static size_t ioctl__scnprintf_usbdevfs_cmd(int nr, int dir, char *bf, size_t size) +{ +#include "trace/beauty/generated/ioctl/usbdevfs_ioctl_array.c" + static DEFINE_STRARRAY(usbdevfs_ioctl_cmds, ""); + + if (nr < strarray__usbdevfs_ioctl_cmds.nr_entries && strarray__usbdevfs_ioctl_cmds.entries[nr] != NULL) + return scnprintf(bf, size, "USBDEVFS_%s", strarray__usbdevfs_ioctl_cmds.entries[nr]); + + return scnprintf(bf, size, "(%c, %#x, %#x)", 'U', nr, dir); +} + static size_t ioctl__scnprintf_cmd(unsigned long cmd, char *bf, size_t size, bool show_prefix) { const char *prefix = "_IOC_"; @@ -157,9 +168,20 @@ static size_t ioctl__scnprintf_cmd(unsigned long cmd, char *bf, size_t size, boo return printed + scnprintf(bf + printed, size - printed, ", %#x, %#x, %#x)", type, nr, sz); } +#ifndef USB_DEVICE_MAJOR +#define USB_DEVICE_MAJOR 189 +#endif // USB_DEVICE_MAJOR + size_t syscall_arg__scnprintf_ioctl_cmd(char *bf, size_t size, struct syscall_arg *arg) { unsigned long cmd = arg->val; + unsigned int fd = syscall_arg__val(arg, 0); + struct file *file = thread__files_entry(arg->thread, fd); + + if (file != NULL) { + if (file->dev_maj == USB_DEVICE_MAJOR) + return ioctl__scnprintf_usbdevfs_cmd(_IOC_NR(cmd), _IOC_DIR(cmd), bf, size); + } return ioctl__scnprintf_cmd(cmd, bf, size, arg->show_string_prefix); } -- cgit v1.2.3 From 423701a0c8d754d9a39547f6c48904347e4f1eca Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Fri, 28 Dec 2018 11:18:19 +0100 Subject: perf c2c: Change the default coalesce setup Joe suggested to have the coalesce default set just to 'iaddr', because it's easier to read on the default 'perf c2c report' output. By removing the "pid" field from the default -c/--coalesce option, the 'perf c2c' report will group all the relevant PIDs under the instruction address ('iaddr') bucket. User can always run "-c pid,iaddr" for a more fine grained output on particular PIDs. Suggested-by: Joe Mario Signed-off-by: Jiri Olsa Cc: Alexander Shishkin Cc: Namhyung Kim Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/20181228101820.28010-1-jolsa@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-c2c.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c index f3aa9d02a5ab5..964f3f5ef4208 100644 --- a/tools/perf/builtin-c2c.c +++ b/tools/perf/builtin-c2c.c @@ -68,7 +68,7 @@ struct c2c_hist_entry { struct hist_entry he; }; -static char const *coalesce_default = "pid,iaddr"; +static char const *coalesce_default = "iaddr"; struct perf_c2c { struct perf_tool tool; -- cgit v1.2.3 From c4a75bb9488a9f7b3c7e79017609529d1aa5deab Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Fri, 28 Dec 2018 11:18:20 +0100 Subject: perf c2c: Increase the HITM ratio limit for displayed cachelines The cachelines being reported are the ones with percentages all the way down to 0.05%. That makes for very long output files. Raising that to 0.1%. The user can always specify --show-all if they want all the cachelines with hits. Suggested-by: Joe Mario Signed-off-by: Jiri Olsa Cc: Alexander Shishkin Cc: Namhyung Kim Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/20181228101820.28010-2-jolsa@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-c2c.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c index 964f3f5ef4208..d340d2e427761 100644 --- a/tools/perf/builtin-c2c.c +++ b/tools/perf/builtin-c2c.c @@ -1878,7 +1878,7 @@ static int c2c_hists__reinit(struct c2c_hists *c2c_hists, return hpp_list__parse(&c2c_hists->list, output, sort); } -#define DISPLAY_LINE_LIMIT 0.0005 +#define DISPLAY_LINE_LIMIT 0.001 static bool he__display(struct hist_entry *he, struct c2c_stats *stats) { -- cgit v1.2.3 From f1770e3ca4ec10ce43825de2f855a1831c711195 Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Fri, 12 Oct 2018 07:20:16 -0400 Subject: tools power x86_energy_perf_policy: Override CFLAGS assignments and add LDFLAGS to build command So user could specify outside CFLAGS/LDFLAGS values. Signed-off-by: Jiri Olsa Cc: Herton Krzesinski Cc: Len Brown Link: https://lkml.kernel.org/r/20181212102537.25902-2-jolsa@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/power/x86/x86_energy_perf_policy/Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/power/x86/x86_energy_perf_policy/Makefile b/tools/power/x86/x86_energy_perf_policy/Makefile index f4534fb8b9515..ae7a0e09b7227 100644 --- a/tools/power/x86/x86_energy_perf_policy/Makefile +++ b/tools/power/x86/x86_energy_perf_policy/Makefile @@ -9,12 +9,12 @@ ifeq ("$(origin O)", "command line") endif x86_energy_perf_policy : x86_energy_perf_policy.c -CFLAGS += -Wall -CFLAGS += -DMSRHEADER='"../../../../arch/x86/include/asm/msr-index.h"' +override CFLAGS += -Wall +override CFLAGS += -DMSRHEADER='"../../../../arch/x86/include/asm/msr-index.h"' %: %.c @mkdir -p $(BUILD_OUTPUT) - $(CC) $(CFLAGS) $< -o $(BUILD_OUTPUT)/$@ + $(CC) $(CFLAGS) $< -o $(BUILD_OUTPUT)/$@ $(LDFLAGS) .PHONY : clean clean : -- cgit v1.2.3 From ad6b474f445e587e9b56ec44241b1639bccf6738 Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Wed, 12 Dec 2018 11:25:33 +0100 Subject: tools thermal tmon: Allow overriding CFLAGS assignments So that the user can provide, e.g. distro package alternative values. Signed-off-by: Jiri Olsa Cc: Brian Norris Cc: Herton Krzesinski Cc: Markus Mayer Cc: Zhang Rui Link: http://lkml.kernel.org/r/20181212102537.25902-3-jolsa@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/thermal/tmon/Makefile | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/thermal/tmon/Makefile b/tools/thermal/tmon/Makefile index 735a510230c3f..89a2444c1df26 100644 --- a/tools/thermal/tmon/Makefile +++ b/tools/thermal/tmon/Makefile @@ -6,13 +6,13 @@ VERSION = 1.0 BINDIR=usr/bin WARNFLAGS=-Wall -Wshadow -W -Wformat -Wimplicit-function-declaration -Wimplicit-int -CFLAGS+= -O1 ${WARNFLAGS} +override CFLAGS+= -O1 ${WARNFLAGS} # Add "-fstack-protector" only if toolchain supports it. -CFLAGS+= $(call cc-option,-fstack-protector) +override CFLAGS+= $(call cc-option,-fstack-protector-strong) CC?= $(CROSS_COMPILE)gcc PKG_CONFIG?= pkg-config -CFLAGS+=-D VERSION=\"$(VERSION)\" +override CFLAGS+=-D VERSION=\"$(VERSION)\" LDFLAGS+= TARGET=tmon @@ -29,7 +29,7 @@ TMON_LIBS += $(shell $(PKG_CONFIG) --libs $(STATIC) panelw ncursesw 2> /dev/null $(PKG_CONFIG) --libs $(STATIC) panel ncurses 2> /dev/null || \ echo -lpanel -lncurses) -CFLAGS += $(shell $(PKG_CONFIG) --cflags $(STATIC) panelw ncursesw 2> /dev/null || \ +override CFLAGS += $(shell $(PKG_CONFIG) --cflags $(STATIC) panelw ncursesw 2> /dev/null || \ $(PKG_CONFIG) --cflags $(STATIC) panel ncurses 2> /dev/null) OBJS = tmon.o tui.o sysfs.o pid.o -- cgit v1.2.3 From 6b7f98a37ac63fe94e84a5289962457b4b504a8d Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Wed, 12 Dec 2018 11:25:35 +0100 Subject: tools power turbostat: Override CFLAGS assignments and add LDFLAGS to build command So that the user can specify outside CFLAGS/LDFLAGS values. Signed-off-by: Jiri Olsa Reviewed-by: Andy Shevchenko Cc: Herton Krzesinski Cc: Len Brown Link: http://lkml.kernel.org/r/20181212102537.25902-5-jolsa@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/power/x86/turbostat/Makefile | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/power/x86/turbostat/Makefile b/tools/power/x86/turbostat/Makefile index 2ab25aa382638..1598b4fa0b112 100644 --- a/tools/power/x86/turbostat/Makefile +++ b/tools/power/x86/turbostat/Makefile @@ -9,13 +9,13 @@ ifeq ("$(origin O)", "command line") endif turbostat : turbostat.c -CFLAGS += -Wall -CFLAGS += -DMSRHEADER='"../../../../arch/x86/include/asm/msr-index.h"' -CFLAGS += -DINTEL_FAMILY_HEADER='"../../../../arch/x86/include/asm/intel-family.h"' +override CFLAGS += -Wall +override CFLAGS += -DMSRHEADER='"../../../../arch/x86/include/asm/msr-index.h"' +override CFLAGS += -DINTEL_FAMILY_HEADER='"../../../../arch/x86/include/asm/intel-family.h"' %: %.c @mkdir -p $(BUILD_OUTPUT) - $(CC) $(CFLAGS) $< -o $(BUILD_OUTPUT)/$@ + $(CC) $(CFLAGS) $< -o $(BUILD_OUTPUT)/$@ $(LDFLAGS) .PHONY : clean clean : -- cgit v1.2.3 From 4ccc98a48958da9f89beb71c66a4e05468727951 Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Wed, 12 Dec 2018 11:25:37 +0100 Subject: tools gpio: Allow overriding CFLAGS So that the user can specify outside CFLAGS values. Signed-off-by: Jiri Olsa Reviewed-by: Andy Shevchenko Cc: Hartmut Knaack Cc: Herton Krzesinski Cc: Jonathan Cameron Cc: Lars-Peter Clausen Link: http://lkml.kernel.org/r/20181212102537.25902-7-jolsa@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/gpio/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/gpio/Makefile b/tools/gpio/Makefile index 240eda014b371..6ecdd10678260 100644 --- a/tools/gpio/Makefile +++ b/tools/gpio/Makefile @@ -12,7 +12,7 @@ endif # (this improves performance and avoids hard-to-debug behaviour); MAKEFLAGS += -r -CFLAGS += -O2 -Wall -g -D_GNU_SOURCE -I$(OUTPUT)include +override CFLAGS += -O2 -Wall -g -D_GNU_SOURCE -I$(OUTPUT)include ALL_TARGETS := lsgpio gpio-hammer gpio-event-mon ALL_PROGRAMS := $(patsubst %,$(OUTPUT)%,$(ALL_TARGETS)) -- cgit v1.2.3 From 03b32cb2810814756095dbd91fce0c77617d096b Mon Sep 17 00:00:00 2001 From: Adrian Hunter Date: Fri, 21 Dec 2018 14:06:13 +0200 Subject: perf thread-stack: Simplify some code in thread_stack__process() In preparation for fixing thread stack processing for the idle task, simplify some code in thread_stack__process(). Signed-off-by: Adrian Hunter Acked-by: Jiri Olsa Link: http://lkml.kernel.org/r/20181221120620.9659-2-adrian.hunter@intel.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/thread-stack.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c index 61a4286a74dc9..115dc4b27a1bc 100644 --- a/tools/perf/util/thread-stack.c +++ b/tools/perf/util/thread-stack.c @@ -593,17 +593,13 @@ int thread_stack__process(struct thread *thread, struct comm *comm, struct thread_stack *ts = thread->ts; int err = 0; - if (ts) { - if (!ts->crp) { - /* Supersede thread_stack__event() */ - thread_stack__free(thread); - thread->ts = thread_stack__new(thread, crp); - if (!thread->ts) - return -ENOMEM; - ts = thread->ts; - ts->comm = comm; - } - } else { + if (ts && !ts->crp) { + /* Supersede thread_stack__event() */ + thread_stack__free(thread); + ts = NULL; + } + + if (!ts) { thread->ts = thread_stack__new(thread, crp); if (!thread->ts) return -ENOMEM; -- cgit v1.2.3 From e0b8951190c11797971864c845e0909561525621 Mon Sep 17 00:00:00 2001 From: Adrian Hunter Date: Fri, 21 Dec 2018 14:06:14 +0200 Subject: perf thread-stack: Tidy thread_stack__bottom() usage In preparation for fixing thread stack processing for the idle task, tidy thread_stack__bottom() usage. Specifically, the parameter 'thread' is not needed. Signed-off-by: Adrian Hunter Acked-by: Jiri Olsa Link: http://lkml.kernel.org/r/20181221120620.9659-3-adrian.hunter@intel.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/thread-stack.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c index 115dc4b27a1bc..068c7c8db4be4 100644 --- a/tools/perf/util/thread-stack.c +++ b/tools/perf/util/thread-stack.c @@ -449,7 +449,7 @@ static int thread_stack__pop_cp(struct thread *thread, struct thread_stack *ts, return 1; } -static int thread_stack__bottom(struct thread *thread, struct thread_stack *ts, +static int thread_stack__bottom(struct thread_stack *ts, struct perf_sample *sample, struct addr_location *from_al, struct addr_location *to_al, u64 ref) @@ -474,7 +474,7 @@ static int thread_stack__bottom(struct thread *thread, struct thread_stack *ts, if (!cp) return -ENOMEM; - return thread_stack__push_cp(thread->ts, ip, sample->time, ref, cp, + return thread_stack__push_cp(ts, ip, sample->time, ref, cp, true, false); } @@ -617,8 +617,7 @@ int thread_stack__process(struct thread *thread, struct comm *comm, /* If the stack is empty, put the current symbol on the stack */ if (!ts->cnt) { - err = thread_stack__bottom(thread, ts, sample, from_al, to_al, - ref); + err = thread_stack__bottom(ts, sample, from_al, to_al, ref); if (err) return err; } -- cgit v1.2.3 From bd8e68ace110941f375f5d566b0cd99fe80634b8 Mon Sep 17 00:00:00 2001 From: Adrian Hunter Date: Fri, 21 Dec 2018 14:06:15 +0200 Subject: perf thread-stack: Avoid direct reference to the thread's stack In preparation for fixing thread stack processing for the idle task, avoid direct reference to the thread's stack. The thread stack will change to an array of thread stacks, at which point the meaning of the direct reference will change. Signed-off-by: Adrian Hunter Acked-by: Jiri Olsa Link: http://lkml.kernel.org/r/20181221120620.9659-4-adrian.hunter@intel.com [ Rename thread_stack__ts() to thread__stack() since this operates on a 'thread' struct ] Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/thread-stack.c | 81 +++++++++++++++++++++++++----------------- 1 file changed, 49 insertions(+), 32 deletions(-) diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c index 068c7c8db4be4..d93cd286b0481 100644 --- a/tools/perf/util/thread-stack.c +++ b/tools/perf/util/thread-stack.c @@ -111,9 +111,16 @@ static struct thread_stack *thread_stack__new(struct thread *thread, ts->kernel_start = 1ULL << 63; ts->crp = crp; + thread->ts = ts; + return ts; } +static inline struct thread_stack *thread__stack(struct thread *thread) +{ + return thread ? thread->ts : NULL; +} + static int thread_stack__push(struct thread_stack *ts, u64 ret_addr, bool trace_end) { @@ -226,8 +233,10 @@ static int __thread_stack__flush(struct thread *thread, struct thread_stack *ts) int thread_stack__flush(struct thread *thread) { - if (thread->ts) - return __thread_stack__flush(thread, thread->ts); + struct thread_stack *ts = thread->ts; + + if (ts) + return __thread_stack__flush(thread, ts); return 0; } @@ -235,16 +244,18 @@ int thread_stack__flush(struct thread *thread) int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip, u64 to_ip, u16 insn_len, u64 trace_nr) { + struct thread_stack *ts = thread__stack(thread); + if (!thread) return -EINVAL; - if (!thread->ts) { - thread->ts = thread_stack__new(thread, NULL); - if (!thread->ts) { + if (!ts) { + ts = thread_stack__new(thread, NULL); + if (!ts) { pr_warning("Out of memory: no thread stack\n"); return -ENOMEM; } - thread->ts->trace_nr = trace_nr; + ts->trace_nr = trace_nr; } /* @@ -252,14 +263,14 @@ int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip, * the stack might be completely invalid. Better to report nothing than * to report something misleading, so flush the stack. */ - if (trace_nr != thread->ts->trace_nr) { - if (thread->ts->trace_nr) - __thread_stack__flush(thread, thread->ts); - thread->ts->trace_nr = trace_nr; + if (trace_nr != ts->trace_nr) { + if (ts->trace_nr) + __thread_stack__flush(thread, ts); + ts->trace_nr = trace_nr; } /* Stop here if thread_stack__process() is in use */ - if (thread->ts->crp) + if (ts->crp) return 0; if (flags & PERF_IP_FLAG_CALL) { @@ -270,7 +281,7 @@ int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip, ret_addr = from_ip + insn_len; if (ret_addr == to_ip) return 0; /* Zero-length calls are excluded */ - return thread_stack__push(thread->ts, ret_addr, + return thread_stack__push(ts, ret_addr, flags & PERF_IP_FLAG_TRACE_END); } else if (flags & PERF_IP_FLAG_TRACE_BEGIN) { /* @@ -280,10 +291,10 @@ int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip, * address, so try to pop that. Also, do not expect a call made * when the trace ended, to return, so pop that. */ - thread_stack__pop(thread->ts, to_ip); - thread_stack__pop_trace_end(thread->ts); + thread_stack__pop(ts, to_ip); + thread_stack__pop_trace_end(ts); } else if ((flags & PERF_IP_FLAG_RETURN) && from_ip) { - thread_stack__pop(thread->ts, to_ip); + thread_stack__pop(ts, to_ip); } return 0; @@ -291,21 +302,25 @@ int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip, void thread_stack__set_trace_nr(struct thread *thread, u64 trace_nr) { - if (!thread || !thread->ts) + struct thread_stack *ts = thread__stack(thread); + + if (!ts) return; - if (trace_nr != thread->ts->trace_nr) { - if (thread->ts->trace_nr) - __thread_stack__flush(thread, thread->ts); - thread->ts->trace_nr = trace_nr; + if (trace_nr != ts->trace_nr) { + if (ts->trace_nr) + __thread_stack__flush(thread, ts); + ts->trace_nr = trace_nr; } } void thread_stack__free(struct thread *thread) { - if (thread->ts) { - __thread_stack__flush(thread, thread->ts); - zfree(&thread->ts->stack); + struct thread_stack *ts = thread->ts; + + if (ts) { + __thread_stack__flush(thread, ts); + zfree(&ts->stack); zfree(&thread->ts); } } @@ -318,6 +333,7 @@ static inline u64 callchain_context(u64 ip, u64 kernel_start) void thread_stack__sample(struct thread *thread, struct ip_callchain *chain, size_t sz, u64 ip, u64 kernel_start) { + struct thread_stack *ts = thread__stack(thread); u64 context = callchain_context(ip, kernel_start); u64 last_context; size_t i, j; @@ -330,15 +346,15 @@ void thread_stack__sample(struct thread *thread, struct ip_callchain *chain, chain->ips[0] = context; chain->ips[1] = ip; - if (!thread || !thread->ts) { + if (!ts) { chain->nr = 2; return; } last_context = context; - for (i = 2, j = 1; i < sz && j <= thread->ts->cnt; i++, j++) { - ip = thread->ts->stack[thread->ts->cnt - j].ret_addr; + for (i = 2, j = 1; i < sz && j <= ts->cnt; i++, j++) { + ip = ts->stack[ts->cnt - j].ret_addr; context = callchain_context(ip, kernel_start); if (context != last_context) { if (i >= sz - 1) @@ -590,7 +606,7 @@ int thread_stack__process(struct thread *thread, struct comm *comm, struct addr_location *to_al, u64 ref, struct call_return_processor *crp) { - struct thread_stack *ts = thread->ts; + struct thread_stack *ts = thread__stack(thread); int err = 0; if (ts && !ts->crp) { @@ -600,10 +616,9 @@ int thread_stack__process(struct thread *thread, struct comm *comm, } if (!ts) { - thread->ts = thread_stack__new(thread, crp); - if (!thread->ts) + ts = thread_stack__new(thread, crp); + if (!ts) return -ENOMEM; - ts = thread->ts; ts->comm = comm; } @@ -668,7 +683,9 @@ int thread_stack__process(struct thread *thread, struct comm *comm, size_t thread_stack__depth(struct thread *thread) { - if (!thread->ts) + struct thread_stack *ts = thread__stack(thread); + + if (!ts) return 0; - return thread->ts->cnt; + return ts->cnt; } -- cgit v1.2.3 From f6060ac60190c625101a0b94c2d96e9ca14a7d73 Mon Sep 17 00:00:00 2001 From: Adrian Hunter Date: Fri, 21 Dec 2018 14:06:16 +0200 Subject: perf thread-stack: Allow for a thread stack array In preparation for fixing thread stack processing for the idle task, allow for a thread stack array. Signed-off-by: Adrian Hunter Acked-by: Jiri Olsa Link: http://lkml.kernel.org/r/20181221120620.9659-5-adrian.hunter@intel.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/thread-stack.c | 40 ++++++++++++++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c index d93cd286b0481..a5f7b9d8fc238 100644 --- a/tools/perf/util/thread-stack.c +++ b/tools/perf/util/thread-stack.c @@ -60,6 +60,7 @@ struct thread_stack_entry { * @last_time: last timestamp * @crp: call/return processor * @comm: current comm + * @arr_sz: size of array if this is the first element of an array */ struct thread_stack { struct thread_stack_entry *stack; @@ -71,6 +72,7 @@ struct thread_stack { u64 last_time; struct call_return_processor *crp; struct comm *comm; + unsigned int arr_sz; }; static int thread_stack__grow(struct thread_stack *ts) @@ -100,6 +102,8 @@ static struct thread_stack *thread_stack__new(struct thread *thread, if (!ts) return NULL; + ts->arr_sz = 1; + if (thread_stack__grow(ts)) { free(ts); return NULL; @@ -234,11 +238,19 @@ static int __thread_stack__flush(struct thread *thread, struct thread_stack *ts) int thread_stack__flush(struct thread *thread) { struct thread_stack *ts = thread->ts; + unsigned int pos; + int err = 0; - if (ts) - return __thread_stack__flush(thread, ts); + if (ts) { + for (pos = 0; pos < ts->arr_sz; pos++) { + int ret = __thread_stack__flush(thread, ts + pos); - return 0; + if (ret) + err = ret; + } + } + + return err; } int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip, @@ -314,13 +326,29 @@ void thread_stack__set_trace_nr(struct thread *thread, u64 trace_nr) } } +static void __thread_stack__free(struct thread *thread, struct thread_stack *ts) +{ + __thread_stack__flush(thread, ts); + zfree(&ts->stack); +} + +static void thread_stack__reset(struct thread *thread, struct thread_stack *ts) +{ + unsigned int arr_sz = ts->arr_sz; + + __thread_stack__free(thread, ts); + memset(ts, 0, sizeof(*ts)); + ts->arr_sz = arr_sz; +} + void thread_stack__free(struct thread *thread) { struct thread_stack *ts = thread->ts; + unsigned int pos; if (ts) { - __thread_stack__flush(thread, ts); - zfree(&ts->stack); + for (pos = 0; pos < ts->arr_sz; pos++) + __thread_stack__free(thread, ts + pos); zfree(&thread->ts); } } @@ -611,7 +639,7 @@ int thread_stack__process(struct thread *thread, struct comm *comm, if (ts && !ts->crp) { /* Supersede thread_stack__event() */ - thread_stack__free(thread); + thread_stack__reset(thread, ts); ts = NULL; } -- cgit v1.2.3 From 2e9e8688763ff80f032d9a78c3b4b951fb6dd7a4 Mon Sep 17 00:00:00 2001 From: Adrian Hunter Date: Fri, 21 Dec 2018 14:06:17 +0200 Subject: perf thread-stack: Factor out thread_stack__init() In preparation for fixing thread stack processing for the idle task, factor out thread_stack__init(). Signed-off-by: Adrian Hunter Acked-by: Jiri Olsa Link: http://lkml.kernel.org/r/20181221120620.9659-6-adrian.hunter@intel.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/thread-stack.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c index a5f7b9d8fc238..03770af9e5cda 100644 --- a/tools/perf/util/thread-stack.c +++ b/tools/perf/util/thread-stack.c @@ -93,6 +93,24 @@ static int thread_stack__grow(struct thread_stack *ts) return 0; } +static int thread_stack__init(struct thread_stack *ts, struct thread *thread, + struct call_return_processor *crp) +{ + int err; + + err = thread_stack__grow(ts); + if (err) + return err; + + if (thread->mg && thread->mg->machine) + ts->kernel_start = machine__kernel_start(thread->mg->machine); + else + ts->kernel_start = 1ULL << 63; + ts->crp = crp; + + return 0; +} + static struct thread_stack *thread_stack__new(struct thread *thread, struct call_return_processor *crp) { @@ -104,17 +122,11 @@ static struct thread_stack *thread_stack__new(struct thread *thread, ts->arr_sz = 1; - if (thread_stack__grow(ts)) { + if (thread_stack__init(ts, thread, crp)) { free(ts); return NULL; } - if (thread->mg && thread->mg->machine) - ts->kernel_start = machine__kernel_start(thread->mg->machine); - else - ts->kernel_start = 1ULL << 63; - ts->crp = crp; - thread->ts = ts; return ts; -- cgit v1.2.3 From 139f42f3b3b495e61bb2cfef40e1dd5e845e3052 Mon Sep 17 00:00:00 2001 From: Adrian Hunter Date: Fri, 21 Dec 2018 14:06:18 +0200 Subject: perf thread-stack: Allocate an array of thread stacks In preparation for fixing thread stack processing for the idle task, allocate an array of thread stacks. Signed-off-by: Adrian Hunter Acked-by: Jiri Olsa Link: http://lkml.kernel.org/r/20181221120620.9659-7-adrian.hunter@intel.com [ No need to check for NULL when calling zfree(), noticed by Jiri Olsa ] Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/thread-stack.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c index 03770af9e5cda..248ed3945becc 100644 --- a/tools/perf/util/thread-stack.c +++ b/tools/perf/util/thread-stack.c @@ -114,20 +114,25 @@ static int thread_stack__init(struct thread_stack *ts, struct thread *thread, static struct thread_stack *thread_stack__new(struct thread *thread, struct call_return_processor *crp) { - struct thread_stack *ts; - - ts = zalloc(sizeof(struct thread_stack)); - if (!ts) - return NULL; - - ts->arr_sz = 1; - - if (thread_stack__init(ts, thread, crp)) { - free(ts); - return NULL; + struct thread_stack *ts = thread->ts, *new_ts; + unsigned int old_sz = ts ? ts->arr_sz : 0; + unsigned int new_sz = 1; + + if (!ts || new_sz > old_sz) { + new_ts = calloc(new_sz, sizeof(*ts)); + if (!new_ts) + return NULL; + if (ts) + memcpy(new_ts, ts, old_sz * sizeof(*ts)); + new_ts->arr_sz = new_sz; + zfree(&thread->ts); + thread->ts = new_ts; + ts = new_ts; } - thread->ts = ts; + if (!ts->stack && + thread_stack__init(ts, thread, crp)) + return NULL; return ts; } -- cgit v1.2.3 From 256d92bc93fd40411a02be5cdba74a7bf91e6e09 Mon Sep 17 00:00:00 2001 From: Adrian Hunter Date: Fri, 21 Dec 2018 14:06:19 +0200 Subject: perf thread-stack: Fix thread stack processing for the idle task perf creates a single 'struct thread' to represent the idle task. That is because threads are identified by PID and TID, and the idle task always has PID == TID == 0. However, there are actually separate idle tasks for each CPU. That creates a problem for thread stack processing which assumes that each thread has a single stack, not one stack per CPU. Fix that by passing through the CPU number, and in the case of the idle "thread", pick the thread stack from an array based on the CPU number. Signed-off-by: Adrian Hunter Acked-by: Jiri Olsa Link: http://lkml.kernel.org/r/20181221120620.9659-8-adrian.hunter@intel.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-script.c | 4 +-- tools/perf/util/intel-bts.c | 4 +-- tools/perf/util/intel-pt.c | 6 ++-- tools/perf/util/thread-stack.c | 72 ++++++++++++++++++++++++++++++++++-------- tools/perf/util/thread-stack.h | 8 ++--- 5 files changed, 69 insertions(+), 25 deletions(-) diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c index 88d52ed85ffc1..d079f36d342d4 100644 --- a/tools/perf/builtin-script.c +++ b/tools/perf/builtin-script.c @@ -1182,7 +1182,7 @@ static int perf_sample__fprintf_callindent(struct perf_sample *sample, struct addr_location *al, FILE *fp) { struct perf_event_attr *attr = &evsel->attr; - size_t depth = thread_stack__depth(thread); + size_t depth = thread_stack__depth(thread, sample->cpu); const char *name = NULL; static int spacing; int len = 0; @@ -1716,7 +1716,7 @@ static bool show_event(struct perf_sample *sample, struct thread *thread, struct addr_location *al) { - int depth = thread_stack__depth(thread); + int depth = thread_stack__depth(thread, sample->cpu); if (!symbol_conf.graph_function) return true; diff --git a/tools/perf/util/intel-bts.c b/tools/perf/util/intel-bts.c index 7b27d77306c22..ee6ca65f81f47 100644 --- a/tools/perf/util/intel-bts.c +++ b/tools/perf/util/intel-bts.c @@ -451,7 +451,7 @@ static int intel_bts_process_buffer(struct intel_bts_queue *btsq, continue; intel_bts_get_branch_type(btsq, branch); if (btsq->bts->synth_opts.thread_stack) - thread_stack__event(thread, btsq->sample_flags, + thread_stack__event(thread, btsq->cpu, btsq->sample_flags, le64_to_cpu(branch->from), le64_to_cpu(branch->to), btsq->intel_pt_insn.length, @@ -523,7 +523,7 @@ static int intel_bts_process_queue(struct intel_bts_queue *btsq, u64 *timestamp) !btsq->bts->synth_opts.thread_stack && thread && (!old_buffer || btsq->bts->sampling_mode || (btsq->bts->snapshot_mode && !buffer->consecutive))) - thread_stack__set_trace_nr(thread, buffer->buffer_nr + 1); + thread_stack__set_trace_nr(thread, btsq->cpu, buffer->buffer_nr + 1); err = intel_bts_process_buffer(btsq, buffer, thread); diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c index 149ff361ca789..2e72373ec6df6 100644 --- a/tools/perf/util/intel-pt.c +++ b/tools/perf/util/intel-pt.c @@ -1174,7 +1174,7 @@ static void intel_pt_prep_sample(struct intel_pt *pt, intel_pt_prep_b_sample(pt, ptq, event, sample); if (pt->synth_opts.callchain) { - thread_stack__sample(ptq->thread, ptq->chain, + thread_stack__sample(ptq->thread, ptq->cpu, ptq->chain, pt->synth_opts.callchain_sz + 1, sample->ip, pt->kernel_start); sample->callchain = ptq->chain; @@ -1526,11 +1526,11 @@ static int intel_pt_sample(struct intel_pt_queue *ptq) return 0; if (pt->synth_opts.callchain || pt->synth_opts.thread_stack) - thread_stack__event(ptq->thread, ptq->flags, state->from_ip, + thread_stack__event(ptq->thread, ptq->cpu, ptq->flags, state->from_ip, state->to_ip, ptq->insn_len, state->trace_nr); else - thread_stack__set_trace_nr(ptq->thread, state->trace_nr); + thread_stack__set_trace_nr(ptq->thread, ptq->cpu, state->trace_nr); if (pt->sample_branches) { err = intel_pt_synth_branch_sample(ptq); diff --git a/tools/perf/util/thread-stack.c b/tools/perf/util/thread-stack.c index 248ed3945becc..d52f27f373ce5 100644 --- a/tools/perf/util/thread-stack.c +++ b/tools/perf/util/thread-stack.c @@ -15,6 +15,7 @@ #include #include +#include #include #include "thread.h" #include "event.h" @@ -75,6 +76,16 @@ struct thread_stack { unsigned int arr_sz; }; +/* + * Assume pid == tid == 0 identifies the idle task as defined by + * perf_session__register_idle_thread(). The idle task is really 1 task per cpu, + * and therefore requires a stack for each cpu. + */ +static inline bool thread_stack__per_cpu(struct thread *thread) +{ + return !(thread->tid || thread->pid_); +} + static int thread_stack__grow(struct thread_stack *ts) { struct thread_stack_entry *new_stack; @@ -111,13 +122,16 @@ static int thread_stack__init(struct thread_stack *ts, struct thread *thread, return 0; } -static struct thread_stack *thread_stack__new(struct thread *thread, +static struct thread_stack *thread_stack__new(struct thread *thread, int cpu, struct call_return_processor *crp) { struct thread_stack *ts = thread->ts, *new_ts; unsigned int old_sz = ts ? ts->arr_sz : 0; unsigned int new_sz = 1; + if (thread_stack__per_cpu(thread) && cpu > 0) + new_sz = roundup_pow_of_two(cpu + 1); + if (!ts || new_sz > old_sz) { new_ts = calloc(new_sz, sizeof(*ts)); if (!new_ts) @@ -130,6 +144,10 @@ static struct thread_stack *thread_stack__new(struct thread *thread, ts = new_ts; } + if (thread_stack__per_cpu(thread) && cpu > 0 && + (unsigned int)cpu < ts->arr_sz) + ts += cpu; + if (!ts->stack && thread_stack__init(ts, thread, crp)) return NULL; @@ -137,9 +155,34 @@ static struct thread_stack *thread_stack__new(struct thread *thread, return ts; } -static inline struct thread_stack *thread__stack(struct thread *thread) +static struct thread_stack *thread__cpu_stack(struct thread *thread, int cpu) { - return thread ? thread->ts : NULL; + struct thread_stack *ts = thread->ts; + + if (cpu < 0) + cpu = 0; + + if (!ts || (unsigned int)cpu >= ts->arr_sz) + return NULL; + + ts += cpu; + + if (!ts->stack) + return NULL; + + return ts; +} + +static inline struct thread_stack *thread__stack(struct thread *thread, + int cpu) +{ + if (!thread) + return NULL; + + if (thread_stack__per_cpu(thread)) + return thread__cpu_stack(thread, cpu); + + return thread->ts; } static int thread_stack__push(struct thread_stack *ts, u64 ret_addr, @@ -270,16 +313,16 @@ int thread_stack__flush(struct thread *thread) return err; } -int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip, +int thread_stack__event(struct thread *thread, int cpu, u32 flags, u64 from_ip, u64 to_ip, u16 insn_len, u64 trace_nr) { - struct thread_stack *ts = thread__stack(thread); + struct thread_stack *ts = thread__stack(thread, cpu); if (!thread) return -EINVAL; if (!ts) { - ts = thread_stack__new(thread, NULL); + ts = thread_stack__new(thread, cpu, NULL); if (!ts) { pr_warning("Out of memory: no thread stack\n"); return -ENOMEM; @@ -329,9 +372,9 @@ int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip, return 0; } -void thread_stack__set_trace_nr(struct thread *thread, u64 trace_nr) +void thread_stack__set_trace_nr(struct thread *thread, int cpu, u64 trace_nr) { - struct thread_stack *ts = thread__stack(thread); + struct thread_stack *ts = thread__stack(thread, cpu); if (!ts) return; @@ -375,10 +418,11 @@ static inline u64 callchain_context(u64 ip, u64 kernel_start) return ip < kernel_start ? PERF_CONTEXT_USER : PERF_CONTEXT_KERNEL; } -void thread_stack__sample(struct thread *thread, struct ip_callchain *chain, +void thread_stack__sample(struct thread *thread, int cpu, + struct ip_callchain *chain, size_t sz, u64 ip, u64 kernel_start) { - struct thread_stack *ts = thread__stack(thread); + struct thread_stack *ts = thread__stack(thread, cpu); u64 context = callchain_context(ip, kernel_start); u64 last_context; size_t i, j; @@ -651,7 +695,7 @@ int thread_stack__process(struct thread *thread, struct comm *comm, struct addr_location *to_al, u64 ref, struct call_return_processor *crp) { - struct thread_stack *ts = thread__stack(thread); + struct thread_stack *ts = thread__stack(thread, sample->cpu); int err = 0; if (ts && !ts->crp) { @@ -661,7 +705,7 @@ int thread_stack__process(struct thread *thread, struct comm *comm, } if (!ts) { - ts = thread_stack__new(thread, crp); + ts = thread_stack__new(thread, sample->cpu, crp); if (!ts) return -ENOMEM; ts->comm = comm; @@ -726,9 +770,9 @@ int thread_stack__process(struct thread *thread, struct comm *comm, return err; } -size_t thread_stack__depth(struct thread *thread) +size_t thread_stack__depth(struct thread *thread, int cpu) { - struct thread_stack *ts = thread__stack(thread); + struct thread_stack *ts = thread__stack(thread, cpu); if (!ts) return 0; diff --git a/tools/perf/util/thread-stack.h b/tools/perf/util/thread-stack.h index f97c00a8c2514..1f626f4a1c40f 100644 --- a/tools/perf/util/thread-stack.h +++ b/tools/perf/util/thread-stack.h @@ -80,14 +80,14 @@ struct call_return_processor { void *data; }; -int thread_stack__event(struct thread *thread, u32 flags, u64 from_ip, +int thread_stack__event(struct thread *thread, int cpu, u32 flags, u64 from_ip, u64 to_ip, u16 insn_len, u64 trace_nr); -void thread_stack__set_trace_nr(struct thread *thread, u64 trace_nr); -void thread_stack__sample(struct thread *thread, struct ip_callchain *chain, +void thread_stack__set_trace_nr(struct thread *thread, int cpu, u64 trace_nr); +void thread_stack__sample(struct thread *thread, int cpu, struct ip_callchain *chain, size_t sz, u64 ip, u64 kernel_start); int thread_stack__flush(struct thread *thread); void thread_stack__free(struct thread *thread); -size_t thread_stack__depth(struct thread *thread); +size_t thread_stack__depth(struct thread *thread, int cpu); struct call_return_processor * call_return_processor__new(int (*process)(struct call_return *cr, void *data), -- cgit v1.2.3 From b25756df5b28cd7b6e91200fc5012e7c76e8ec69 Mon Sep 17 00:00:00 2001 From: Adrian Hunter Date: Fri, 21 Dec 2018 14:06:20 +0200 Subject: perf session: Add comment for perf_session__register_idle_thread() Add a comment to perf_session__register_idle_thread() to bring attention to a pitfall with the idle task thread structure. The pitfall is that there should really be a 'struct thread' for the idle task of each cpu, but there is only one that can have pid == tid == 0. Signed-off-by: Adrian Hunter Acked-by: Jiri Olsa Link: http://lkml.kernel.org/r/20181221120620.9659-9-adrian.hunter@intel.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/session.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c index 78a067777144a..5456c84c7dd13 100644 --- a/tools/perf/util/session.c +++ b/tools/perf/util/session.c @@ -1527,6 +1527,13 @@ struct thread *perf_session__findnew(struct perf_session *session, pid_t pid) return machine__findnew_thread(&session->machines.host, -1, pid); } +/* + * Threads are identified by pid and tid, and the idle task has pid == tid == 0. + * So here a single thread is created for that, but actually there is a separate + * idle task per cpu, so there should be one 'struct thread' per cpu, but there + * is only 1. That causes problems for some tools, requiring workarounds. For + * example get_idle_thread() in builtin-sched.c, or thread_stack__per_cpu(). + */ int perf_session__register_idle_thread(struct perf_session *session) { struct thread *thread; -- cgit v1.2.3