From d7c72606d97e6f462a99b79e55b39808147d4c8b Mon Sep 17 00:00:00 2001 From: Milos Vyletel Date: Mon, 8 Jun 2015 16:50:16 +0200 Subject: [PATCH 1/7] perf tools: Avoid possible race condition in copyfile() Use unique temporary files when copying to buildid dir to prevent races in case multiple instances are trying to copy same file. This is done by - creating template in form /..XXXXXX where the suffix is used by mkstemp() to create unique file - change file mode - copy content - if successful link temp file to target file - unlink temp file At this point the only file left at target path should be the desired one either created by us or other instance if we raced. This should also prevent not yet fully copied files to be visible to to other perf instances that could try to parse them. On top of that slow_copyfile no longer needs to deal with file mode when creating file since temporary file is already created and mode is set. Succesfully tested by myself by running perf record, archive and reading the data on other system and by running perf buildid-cache on perf binary itself. I also did revert fix from 0635b0f that to exposes previously fixed race with EEXIST and recreator test passed sucessfully. Signed-off-by: Milos Vyletel Acked-by: Ingo Molnar Cc: Andy Shevchenko Cc: Don Zickus Cc: Jiri Olsa Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Steven Rostedt Link: http://lkml.kernel.org/r/1433775018-19868-1-git-send-email-milos@redhat.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/util.c | 46 ++++++++++++++++++++++++++++-------------- 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c index 0c264bc685ac..edc2d633b332 100644 --- a/tools/perf/util/util.c +++ b/tools/perf/util/util.c @@ -115,20 +115,17 @@ int rm_rf(char *path) return rmdir(path); } -static int slow_copyfile(const char *from, const char *to, mode_t mode) +static int slow_copyfile(const char *from, const char *to) { int err = -1; char *line = NULL; size_t n; FILE *from_fp = fopen(from, "r"), *to_fp; - mode_t old_umask; if (from_fp == NULL) goto out; - old_umask = umask(mode ^ 0777); to_fp = fopen(to, "w"); - umask(old_umask); if (to_fp == NULL) goto out_fclose_from; @@ -178,29 +175,48 @@ int copyfile_mode(const char *from, const char *to, mode_t mode) int fromfd, tofd; struct stat st; int err = -1; + char *tmp = NULL, *ptr = NULL; if (stat(from, &st)) goto out; - if (st.st_size == 0) /* /proc? do it slowly... */ - return slow_copyfile(from, to, mode); + /* extra 'x' at the end is to reserve space for '.' */ + if (asprintf(&tmp, "%s.XXXXXXx", to) < 0) { + tmp = NULL; + goto out; + } + ptr = strrchr(tmp, '/'); + if (!ptr) + goto out; + ptr = memmove(ptr + 1, ptr, strlen(ptr) - 1); + *ptr = '.'; + + tofd = mkstemp(tmp); + if (tofd < 0) + goto out; + + if (fchmod(tofd, mode)) + goto out_close_to; + + if (st.st_size == 0) { /* /proc? do it slowly... */ + err = slow_copyfile(from, tmp); + goto out_close_to; + } fromfd = open(from, O_RDONLY); if (fromfd < 0) - goto out; - - tofd = creat(to, mode); - if (tofd < 0) - goto out_close_from; + goto out_close_to; err = copyfile_offset(fromfd, 0, tofd, 0, st.st_size); - close(tofd); - if (err) - unlink(to); -out_close_from: close(fromfd); +out_close_to: + close(tofd); + if (!err) + err = link(tmp, to); + unlink(tmp); out: + free(tmp); return err; } From 5610032135c05e7bf9cba231826577a01719d010 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Wed, 10 Jun 2015 16:48:50 +0200 Subject: [PATCH 2/7] perf record: Amend option summaries Because there's too many options and I cannot read, I frequently get confused between -c and -P, and try to do things like: perf record -P 50000 -- foo Which does not work; try and make the option description slightly longer and hopefully less confusing. Signed-off-by: Peter Zijlstra (Intel) Link: http://lkml.kernel.org/r/20150610144850.GP19282@twins.programming.kicks-ass.net [ Do those changes on the man page as well ] Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/Documentation/perf-record.txt | 10 +++++++--- tools/perf/builtin-record.c | 7 +++---- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt index 280533ebf9df..6fdf78625c51 100644 --- a/tools/perf/Documentation/perf-record.txt +++ b/tools/perf/Documentation/perf-record.txt @@ -152,12 +152,16 @@ OPTIONS -d:: --data:: - Sample addresses. + Record the sample addresses. -T:: --timestamp:: - Sample timestamps. Use it with 'perf report -D' to see the timestamps, - for instance. + Record the sample timestamps. Use it with 'perf report -D' to see the + timestamps, for instance. + +-P:: +--period:: + Record the sample period. -n:: --no-samples:: diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c index d3731cce7c1c..4d6cdeb94fe1 100644 --- a/tools/perf/builtin-record.c +++ b/tools/perf/builtin-record.c @@ -1027,10 +1027,9 @@ struct option __record_options[] = { OPT_BOOLEAN('q', "quiet", &quiet, "don't print any message"), OPT_BOOLEAN('s', "stat", &record.opts.inherit_stat, "per thread counts"), - OPT_BOOLEAN('d', "data", &record.opts.sample_address, - "Sample addresses"), - OPT_BOOLEAN('T', "timestamp", &record.opts.sample_time, "Sample timestamps"), - OPT_BOOLEAN('P', "period", &record.opts.period, "Sample period"), + OPT_BOOLEAN('d', "data", &record.opts.sample_address, "Record the sample addresses"), + OPT_BOOLEAN('T', "timestamp", &record.opts.sample_time, "Record the sample timestamps"), + OPT_BOOLEAN('P', "period", &record.opts.period, "Record the sample period"), OPT_BOOLEAN('n', "no-samples", &record.opts.no_samples, "don't sample"), OPT_BOOLEAN('N', "no-buildid-cache", &record.no_buildid_cache, From 7310aed77ef2928bcfb8ee5ad71e2b091166b85e Mon Sep 17 00:00:00 2001 From: Adrian Hunter Date: Thu, 11 Jun 2015 15:51:04 +0300 Subject: [PATCH 3/7] perf evsel: Display 0x for hex values when printing the attribute Need to display '0x' prefix for hex values otherwise it is not obvious they are hex. Signed-off-by: Adrian Hunter Cc: Jiri Olsa Link: http://lkml.kernel.org/r/1434027064-7554-1-git-send-email-adrian.hunter@intel.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/evsel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c index a3e36fc634dc..d4f9994ae47f 100644 --- a/tools/perf/util/evsel.c +++ b/tools/perf/util/evsel.c @@ -1058,7 +1058,7 @@ static void __p_read_format(char *buf, size_t size, u64 value) #define BUF_SIZE 1024 -#define p_hex(val) snprintf(buf, BUF_SIZE, "%"PRIx64, (uint64_t)(val)) +#define p_hex(val) snprintf(buf, BUF_SIZE, "%#"PRIx64, (uint64_t)(val)) #define p_unsigned(val) snprintf(buf, BUF_SIZE, "%"PRIu64, (uint64_t)(val)) #define p_signed(val) snprintf(buf, BUF_SIZE, "%"PRId64, (int64_t)(val)) #define p_sample_type(val) __p_sample_type(buf, BUF_SIZE, val) From cb5ef60067c11cc8887122f6f168c21941c5d624 Mon Sep 17 00:00:00 2001 From: Kan Liang Date: Thu, 11 Jun 2015 02:32:40 -0400 Subject: [PATCH 4/7] perf stat: Error out unsupported group leader immediately perf stat ignores the unsupported event and continue to count supported event. But if the unsupported event is group leader, perf tool will crash. After applying this patch, the unsupported group leader will error out immediately. Without this patch: $ perf stat -x, -e '{node-prefetch-refs,cycles}' -- sleep 1 perf: util/evsel.c:1009: get_group_fd: Assertion `!(fd == -1)' failed. Aborted (core dumped) With this patch: $ perf stat -x, -e '{node-prefetch-refs,cycles}' -- sleep 1 Error: The node-prefetch-refs event is not supported. Commiter note: Here I got a different output, but no core dump: [acme@zoo linux]$ perf stat -x, -e '{node-prefetch-refs,cycles}' -- sleep 1 Error: The sys_perf_event_open() syscall returned with 22 (Invalid argument) for event (node-prefetch-refs). /bin/dmesg may provide additional information. No CONFIG_PERF_EVENTS=y kernel support configured? Signed-off-by: Kan Liang Tested-by: Arnaldo Carvalho de Melo Cc: Andi Kleen Link: http://lkml.kernel.org/r/1434004360-8570-1-git-send-email-kan.liang@intel.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-stat.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c index ff3d25803400..b24ecee95fec 100644 --- a/tools/perf/builtin-stat.c +++ b/tools/perf/builtin-stat.c @@ -549,7 +549,10 @@ static int __run_perf_stat(int argc, const char **argv) ui__warning("%s event is not supported by the kernel.\n", perf_evsel__name(counter)); counter->supported = false; - continue; + + if ((counter->leader != counter) || + !(counter->leader->nr_members > 1)) + continue; } perf_evsel__open_strerror(counter, &target, From 6ba29c2fa5adcc33b201faec99057b6a72bd5029 Mon Sep 17 00:00:00 2001 From: He Kuang Date: Thu, 11 Jun 2015 12:44:24 +0000 Subject: [PATCH 5/7] perf tools: Fix build failure on 32-bit arch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Failed in 32bit arch build like this: CC /opt/h00206996/output/perf/arm32/builtin-record.o util/session.c: In function ‘perf_session__warn_about_errors’: util/session.c:1304:9: error: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 2 has type ‘long long unsigned int’ [-Werror=format=] builtin-report.c: In function ‘perf_evlist__tty_browse_hists’: builtin-report.c:323:2: error: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 3 has type ‘u64’ [-Werror=format=] Replace %lu format strings in warning message with PRIu64 for u64 'total_lost_samples' to fix this problem. Signed-off-by: He Kuang Cc: Jiri Olsa Cc: Kan Liang Cc: Peter Zijlstra Cc: Wang Nan Link: http://lkml.kernel.org/r/1434026664-71642-1-git-send-email-hekuang@huawei.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-report.c | 2 +- tools/perf/util/session.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c index 628090b478ab..32626ea3e227 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c @@ -320,7 +320,7 @@ static int perf_evlist__tty_browse_hists(struct perf_evlist *evlist, { struct perf_evsel *pos; - fprintf(stdout, "#\n# Total Lost Samples: %lu\n#\n", evlist->stats.total_lost_samples); + fprintf(stdout, "#\n# Total Lost Samples: %" PRIu64 "\n#\n", evlist->stats.total_lost_samples); evlist__for_each(evlist, pos) { struct hists *hists = evsel__hists(pos); const char *evname = perf_evsel__name(pos); diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c index 88d87bf3049f..f31e024ddf7d 100644 --- a/tools/perf/util/session.c +++ b/tools/perf/util/session.c @@ -1299,7 +1299,7 @@ static void perf_session__warn_about_errors(const struct perf_session *session) drop_rate = (double)stats->total_lost_samples / (double) (stats->nr_events[PERF_RECORD_SAMPLE] + stats->total_lost_samples); if (drop_rate > 0.05) { - ui__warning("Processed %lu samples and lost %3.2f%% samples!\n\n", + ui__warning("Processed %" PRIu64 " samples and lost %3.2f%% samples!\n\n", stats->nr_events[PERF_RECORD_SAMPLE] + stats->total_lost_samples, drop_rate * 100.0); } From a1c2552dba788c2c5e71c5bf5bcf3971caf3dfa1 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Thu, 11 Jun 2015 22:47:54 -0300 Subject: [PATCH 6/7] trace: Beautify perf_event_open syscall Syswide tracing and then running 'stat' and 'trace': $ perf trace -e perf_event_open 1034.649 (0.019 ms): perf/6133 perf_event_open(attr_uptr: 0x36f0360, pid: 16134, cpu: -1, group_fd: -1, flags: FD_CLOEXEC) = -1 EINVAL Invalid argument 1034.670 (0.008 ms): perf/6133 perf_event_open(attr_uptr: 0x36f0360, pid: 16134, cpu: -1, group_fd: -1) = -1 EINVAL Invalid argument 1034.681 (0.007 ms): perf/6133 perf_event_open(attr_uptr: 0x36f0360, pid: 16134, cpu: -1, group_fd: -1) = -1 EINVAL Invalid argument 1034.692 (0.007 ms): perf/6133 perf_event_open(attr_uptr: 0x36f0360, pid: 16134, cpu: -1, group_fd: -1) = -1 EINVAL Invalid argument 9986.983 (0.014 ms): trace/6139 perf_event_open(attr_uptr: 0x7ffd9c629320, pid: -1, group_fd: -1, flags: FD_CLOEXEC) = 3 9987.026 (0.016 ms): trace/6139 perf_event_open(attr_uptr: 0x37c7e70, pid: -1, group_fd: -1, flags: FD_CLOEXEC) = 3 9987.041 (0.008 ms): trace/6139 perf_event_open(attr_uptr: 0x37c7e70, pid: -1, group_fd: -1, flags: FD_CLOEXEC) = 3 9987.489 (0.092 ms): trace/6139 perf_event_open(attr_uptr: 0x3795ee0, pid: 16140, group_fd: -1, flags: FD_CLOEXEC) = 3 9987.536 (0.044 ms): trace/6139 perf_event_open(attr_uptr: 0x3795ee0, pid: 16140, cpu: 1, group_fd: -1, flags: FD_CLOEXEC) = 4 9987.580 (0.041 ms): trace/6139 perf_event_open(attr_uptr: 0x3795ee0, pid: 16140, cpu: 2, group_fd: -1, flags: FD_CLOEXEC) = 5 9987.620 (0.037 ms): trace/6139 perf_event_open(attr_uptr: 0x3795ee0, pid: 16140, cpu: 3, group_fd: -1, flags: FD_CLOEXEC) = 7 9987.659 (0.035 ms): trace/6139 perf_event_open(attr_uptr: 0x37975d0, pid: 16140, group_fd: -1, flags: FD_CLOEXEC) = 8 9987.692 (0.031 ms): trace/6139 perf_event_open(attr_uptr: 0x37975d0, pid: 16140, cpu: 1, group_fd: -1, flags: FD_CLOEXEC) = 9 9987.727 (0.032 ms): trace/6139 perf_event_open(attr_uptr: 0x37975d0, pid: 16140, cpu: 2, group_fd: -1, flags: FD_CLOEXEC) = 10 9987.761 (0.031 ms): trace/6139 perf_event_open(attr_uptr: 0x37975d0, pid: 16140, cpu: 3, group_fd: -1, flags: FD_CLOEXEC) = 11 Need to intercept perf_copy_attr() with a kprobe or with eBPF... Cc: Adrian Hunter Cc: Borislav Petkov Cc: David Ahern Cc: Frederic Weisbecker Cc: Jiri Olsa Cc: Namhyung Kim Link: http://lkml.kernel.org/n/tip-njb105hab2i3t5dexym9lskl@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-trace.c | 58 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c index a05490d06374..4bf805b2fbf6 100644 --- a/tools/perf/builtin-trace.c +++ b/tools/perf/builtin-trace.c @@ -68,6 +68,23 @@ # define MSG_CMSG_CLOEXEC 0x40000000 #endif +#ifndef PERF_FLAG_FD_NO_GROUP +# define PERF_FLAG_FD_NO_GROUP (1UL << 0) +#endif + +#ifndef PERF_FLAG_FD_OUTPUT +# define PERF_FLAG_FD_OUTPUT (1UL << 1) +#endif + +#ifndef PERF_FLAG_PID_CGROUP +# define PERF_FLAG_PID_CGROUP (1UL << 2) /* pid=cgroup id, per-cpu mode only */ +#endif + +#ifndef PERF_FLAG_FD_CLOEXEC +# define PERF_FLAG_FD_CLOEXEC (1UL << 3) /* O_CLOEXEC */ +#endif + + struct tp_field { int offset; union { @@ -358,6 +375,14 @@ static size_t syscall_arg__scnprintf_hex(char *bf, size_t size, #define SCA_HEX syscall_arg__scnprintf_hex +static size_t syscall_arg__scnprintf_int(char *bf, size_t size, + struct syscall_arg *arg) +{ + return scnprintf(bf, size, "%d", arg->val); +} + +#define SCA_INT syscall_arg__scnprintf_int + static size_t syscall_arg__scnprintf_mmap_prot(char *bf, size_t size, struct syscall_arg *arg) { @@ -810,6 +835,34 @@ static size_t syscall_arg__scnprintf_open_flags(char *bf, size_t size, #define SCA_OPEN_FLAGS syscall_arg__scnprintf_open_flags +static size_t syscall_arg__scnprintf_perf_flags(char *bf, size_t size, + struct syscall_arg *arg) +{ + int printed = 0, flags = arg->val; + + if (flags == 0) + return 0; + +#define P_FLAG(n) \ + if (flags & PERF_FLAG_##n) { \ + printed += scnprintf(bf + printed, size - printed, "%s%s", printed ? "|" : "", #n); \ + flags &= ~PERF_FLAG_##n; \ + } + + P_FLAG(FD_NO_GROUP); + P_FLAG(FD_OUTPUT); + P_FLAG(PID_CGROUP); + P_FLAG(FD_CLOEXEC); +#undef P_FLAG + + if (flags) + printed += scnprintf(bf + printed, size - printed, "%s%#x", printed ? "|" : "", flags); + + return printed; +} + +#define SCA_PERF_FLAGS syscall_arg__scnprintf_perf_flags + static size_t syscall_arg__scnprintf_eventfd_flags(char *bf, size_t size, struct syscall_arg *arg) { @@ -1077,6 +1130,11 @@ static struct syscall_fmt { { .name = "openat", .errmsg = true, .arg_scnprintf = { [0] = SCA_FDAT, /* dfd */ [2] = SCA_OPEN_FLAGS, /* flags */ }, }, + { .name = "perf_event_open", .errmsg = true, + .arg_scnprintf = { [1] = SCA_INT, /* pid */ + [2] = SCA_INT, /* cpu */ + [3] = SCA_FD, /* group_fd */ + [4] = SCA_PERF_FLAGS, /* flags */ }, }, { .name = "pipe2", .errmsg = true, .arg_scnprintf = { [1] = SCA_PIPE_FLAGS, /* flags */ }, }, { .name = "poll", .errmsg = true, .timeout = true, }, From c8ad7063626406181a7ebab10cb31b4f741b13d4 Mon Sep 17 00:00:00 2001 From: David Ahern Date: Fri, 5 Jun 2015 13:42:53 -0400 Subject: [PATCH 7/7] perf tools: Update MANIFEST per files removed from kernel Building perf out of kernel tree is currently broken because the MANIFEST file refers to kernel files that have been removed. With this patch make perf-targz-src-pkg succeeds as does building perf using the generated tarfile. Signed-off-by: David Ahern Link: http://lkml.kernel.org/r/1433526173-172332-1-git-send-email-david.ahern@oracle.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/MANIFEST | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tools/perf/MANIFEST b/tools/perf/MANIFEST index a0bdd6124583..fe50a1b34aa0 100644 --- a/tools/perf/MANIFEST +++ b/tools/perf/MANIFEST @@ -50,24 +50,20 @@ include/asm-generic/bitops/const_hweight.h include/asm-generic/bitops/fls64.h include/asm-generic/bitops/__fls.h include/asm-generic/bitops/fls.h -include/linux/const.h include/linux/perf_event.h include/linux/rbtree.h include/linux/list.h include/linux/hash.h include/linux/stringify.h -lib/find_next_bit.c lib/hweight.c lib/rbtree.c include/linux/swab.h arch/*/include/asm/unistd*.h -arch/*/include/asm/perf_regs.h arch/*/include/uapi/asm/unistd*.h arch/*/include/uapi/asm/perf_regs.h arch/*/lib/memcpy*.S arch/*/lib/memset*.S include/linux/poison.h -include/linux/magic.h include/linux/hw_breakpoint.h include/linux/rbtree_augmented.h include/uapi/linux/perf_event.h