Skip to contentSkip to conversation

perf/core improvements and fixes

GIT PULL New Arnaldo Carvalho de Melo 2 participants 13 messages Jun 19, 2014 – Jun 25, 2014

Subsystems (2)
lore
Share & tools
lore

Arnaldo Carvalho de Melo

author maintainer
#0 lore raw

Hi Ingo,

Please conseider pulling,

- Arnaldo

The following changes since commit a10d60c08cc3bbea9195e2b36440f557373623eb:

sh, perf: Use common PMU interrupt disabled code (2014-06-19 19:37:51 +0200)

are available in the git repository at:

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

for you to fetch changes up to ecdac96899e3db3f428e4d2e978f25e3f8d35a6c:

perf bench sched-messaging: Drop barf() (2014-06-19 16:13:17 -0300)

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

User visible:

. Add --repeat global option to 'perf bench' to be used in benchmarks
such as the existing 'futex' one, that was modified to use it instead of a local option. (Davidlohr Bueso)

. Fix fd -> pathname resolution in 'trace', be it using /proc or
a vfs_getname probe point. (Arnaldo Carvalho de Melo)

. Add suggestion of how to set perf_event_paranoid sysctl, to help
non-root users trying tools like 'trace' to get a working environment. (Arnaldo Carvalho de Melo)

Fixes:

. Fix memory leak in the 'sched-messaging' perf bench test. (Davidlohr Bueso)

. The -o and -n 'perf bench mem' options are mutually exclusive, emit error
when both are specified. (Davidlohr Bueso)

. Fix scrollbar refresh row index in the ui browser, problem exposed now
that headers will be added and will be allowed to be switched on/off. (Jiri Olsa)

Cleanups:

. Remove needless reassignments in 'trace' (Arnaldo Carvalho de Melo)

. Cache the is_exit syscall test in 'trace) (Arnaldo Carvalho de Melo)

. No need to reimplement err() in 'perf bench sched-messaging', drop barf().
(Davidlohr Bueso).

. Remove ev_name argument from perf_evsel__hists_browse, can be obtained
from the other parameters. (Jiri Olsa)

----------------------------------------------------------------
Arnaldo Carvalho de Melo (4):
perf trace: Fix up fd -> pathname resolution
perf evlist: Add suggestion of how to set perf_event_paranoid sysctl perf trace: Remove needless reassignments
perf trace: Cache the is_exit syscall test

Davidlohr Bueso (5):
perf bench sched-messaging: Plug memleak
perf bench: Add --repeat option
perf bench futex: Use global --repeat option
perf bench mem: The -o and -n options are mutually exclusive perf bench sched-messaging: Drop barf()

Jiri Olsa (2):
perf hists browser: Remove ev_name argument from perf_evsel__hists_browse perf ui browser: Fix scrollbar refresh row index

 tools/perf/Documentation/perf-bench.txt |  4 +++
 tools/perf/bench/bench.h                |  1 +
 tools/perf/bench/futex-requeue.c        | 10 +------
 tools/perf/bench/futex-wake.c           | 12 ++-------
 tools/perf/bench/mem-memcpy.c           |  5 ++++
 tools/perf/bench/mem-memset.c           |  5 ++++
 tools/perf/bench/sched-messaging.c      | 47 +++++++++++++++------------------
 tools/perf/builtin-bench.c              |  7 +++++
 tools/perf/builtin-trace.c              | 12 ++++-----
 tools/perf/ui/browser.c                 |  2 +-
 tools/perf/ui/browsers/hists.c          | 25 ++++++++----------
 tools/perf/util/evlist.c                |  5 ++--
 12 files changed, 67 insertions(+), 68 deletions(-)
author maintainer
#1 lore raw

PATCH 1/11 perf trace: Fix up fd -> pathname resolution

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

There was a brown paper bag bug in the patch that introduced a reference implementation on using 'perf probe' made wannabe tracepoints that broke fd -> pathname resolution, fix it:

[root@zoo ~]# perf probe 'vfs_getname=getname_flags:65 pathname=result->name:string' Added new event:

    probe:vfs_getname    (on getname_flags:65 with pathname=result->name:string)

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

perf record -e probe:vfs_getname -aR sleep 1

[root@zoo ~]

Before:

[acme@zoo linux]$ trace touch -e open,fstat /tmp/b

     1.159 ( 0.007 ms): open(filename: 0x7fd73f2fe088, flags: CLOEXEC                         ) = 3
     1.163 ( 0.002 ms): fstat(fd: 3, statbuf: 0x7fff1b25e610                                  ) = 0
     1.192 ( 0.009 ms): open(filename: 0x7fd73f4fedb8, flags: CLOEXEC                         ) = 3
     1.201 ( 0.002 ms): fstat(fd: 3, statbuf: 0x7fff1b25e660                                  ) = 0
     1.501 ( 0.013 ms): open(filename: 0x7fd73f0a1610, flags: CLOEXEC                         ) = 3
     1.505 ( 0.002 ms): fstat(fd: 3, statbuf: 0x7fd73f2ddb60                                  ) = 0
     1.581 ( 0.011 ms): open(filename: 0x7fff1b2603da, flags: CREAT|NOCTTY|NONBLOCK|WRONLY, mode: 438) = 3

[acme@zoo linux]$

After:

[acme@zoo linux]$ trace touch -e open,fstat,dup2,mmap,close /tmp/b

     1.105 ( 0.004 ms): mmap(len: 4096, prot: READ|WRITE, flags: PRIVATE|ANONYMOUS, fd: -1    ) = 0x2fbf000
     1.136 ( 0.008 ms): open(filename: 0x7f8902dbc088, flags: CLOEXEC                         ) = 3
     1.140 ( 0.002 ms): fstat(fd: 3</etc/ld.so.cache>, statbuf: 0x7fff19889ef0                ) = 0
     1.146 ( 0.004 ms): mmap(len: 86079, prot: READ, flags: PRIVATE, fd: 3</etc/ld.so.cache>  ) = 0x2fa9000
     1.149 ( 0.001 ms): close(fd: 3</etc/ld.so.cache>                                         ) = 0
     1.170 ( 0.010 ms): open(filename: 0x7f8902fbcdb8, flags: CLOEXEC                         ) = 3
     1.178 ( 0.002 ms): fstat(fd: 3</lib64/libc.so.6>, statbuf: 0x7fff19889f40                ) = 0
     1.188 ( 0.006 ms): mmap(len: 3924576, prot: EXEC|READ, flags: PRIVATE|DENYWRITE, fd: 3</lib64/libc.so.6>) = 0x29e2000
     1.207 ( 0.007 ms): mmap(addr: 0x7f8902d96000, len: 24576, prot: READ|WRITE, flags: PRIVATE|DENYWRITE|FIXED, fd: 3</lib64/libc.so.6>, off: 1785856) = 0x2d96000
     1.217 ( 0.004 ms): mmap(addr: 0x7f8902d9c000, len: 16992, prot: READ|WRITE, flags: PRIVATE|ANONYMOUS|FIXED, fd: -1) = 0x2d9c000
     1.228 ( 0.002 ms): close(fd: 3</lib64/libc.so.6>                                         ) = 0
     1.243 ( 0.003 ms): mmap(len: 4096, prot: READ|WRITE, flags: PRIVATE|ANONYMOUS, fd: -1    ) = 0x2fa8000
     1.250 ( 0.003 ms): mmap(len: 8192, prot: READ|WRITE, flags: PRIVATE|ANONYMOUS, fd: -1    ) = 0x2fa6000
     1.452 ( 0.010 ms): open(filename: 0x7f8902b5f610, flags: CLOEXEC                         ) = 3
     1.455 ( 0.002 ms): fstat(fd: 3</usr/lib/locale/locale-archive>, statbuf: 0x7f8902d9bb60  ) = 0
     1.461 ( 0.004 ms): mmap(len: 106070960, prot: READ, flags: PRIVATE, fd: 3</usr/lib/locale/locale-archive>) = 0xfc4b9000
     1.469 ( 0.002 ms): close(fd: 3</usr/lib/locale/locale-archive>                           ) = 0
     1.528 ( 0.010 ms): open(filename: 0x7fff1988c3da, flags: CREAT|NOCTTY|NONBLOCK|WRONLY, mode: 438) = 3
     1.532 ( 0.002 ms): dup2(oldfd: 3</tmp/b>                                                 ) = 0
     1.535 ( 0.001 ms): close(fd: 3</tmp/b>                                                   ) = 0
     1.544 ( 0.001 ms): close(                                                                ) = 0
     1.555 ( 0.001 ms): close(fd: 1                                                           ) = 0
     1.558 ( 0.001 ms): close(fd: 2                                                           ) = 0

[acme@zoo linux]$

Cc Adrian Hunter <[email protected]>
Cc David Ahern <[email protected]>
Cc Don Zickus <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Mike Galbraith <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/builtin-trace.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
tools/perf/builtin-trace.c +2 -2
index f954c26de231..5ab2f674fed2 100644
@@ -1276,11 +1276,11 @@ 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->paths.max || ttrace->paths.table[fd] == NULL)) {
 		if (!trace->live)
 			return NULL;
 		++trace->stats.proc_getname;
-		if (thread__read_fd_path(thread, fd)) {
+		if (thread__read_fd_path(thread, fd))
 			return NULL;
 	}
 
Signature · 1 line
1.9.3
author maintainer
#2 lore raw

PATCH 2/11 perf evlist: Add suggestion of how to set perf_event_paranoid sysctl

In reply to Arnaldo Carvalho de Melo

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

Minor hint to speed up problem resolution and get 'trace' working for non root users.

Cc Adrian Hunter <[email protected]>
Cc David Ahern <[email protected]>
Cc Don Zickus <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Mike Galbraith <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/util/evlist.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
tools/perf/util/evlist.c +3 -2
index 59ef2802fcf6..c51223ac25f4 100644
@@ -1214,10 +1214,11 @@ int perf_evlist__strerror_open(struct perf_evlist *evlist __maybe_unused,
 					     "For your workloads it needs to be <= 1\nHint:\t");
 		}
 		printed += scnprintf(buf + printed, size - printed,
-				     "For system wide tracing it needs to be set to -1");
+				     "For system wide tracing it needs to be set to -1.\n");
 
 		printed += scnprintf(buf + printed, size - printed,
-				    ".\nHint:\tThe current value is %d.", value);
+				    "Hint:\tTry: 'sudo sh -c \"echo -1 > /proc/sys/kernel/perf_event_paranoid\"'\n"
+				    "Hint:\tThe current value is %d.", value);
 		break;
 	default:
 		scnprintf(buf, size, "%s", emsg);
Signature · 1 line
1.9.3
author maintainer
#3 lore raw

PATCH 3/11 perf trace: Remove needless reassignments

In reply to Arnaldo Carvalho de Melo

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

The thread->priv value is already obtained a few lines earlier from the thread__trace() call. Leftovers from before thread__trace().

Cc Adrian Hunter <[email protected]>
Cc David Ahern <[email protected]>
Cc Don Zickus <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Mike Galbraith <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/builtin-trace.c | 3 ---
 1 file changed, 3 deletions(-)
tools/perf/builtin-trace.c +0 -3
index 5ab2f674fed2..28c86e21fad9 100644
@@ -1629,7 +1629,6 @@ static int trace__sys_enter(struct trace *trace, struct perf_evsel *evsel,
 		return -1;
 
 	args = perf_evsel__sc_tp_ptr(evsel, args, sample);
-	ttrace = thread->priv;
 
 	if (ttrace->entry_str == NULL) {
 		ttrace->entry_str = malloc(1024);
@@ -1687,8 +1686,6 @@ static int trace__sys_exit(struct trace *trace, struct perf_evsel *evsel,
 		++trace->stats.vfs_getname;
 	}
 
-	ttrace = thread->priv;
-
 	ttrace->exit_time = sample->time;
 
 	if (ttrace->entry_time) {
Signature · 1 line
1.9.3
author maintainer
#4 lore raw

PATCH 4/11 perf trace: Cache the is_exit syscall test

In reply to Arnaldo Carvalho de Melo

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

No need to use two strcmp calls per syscall entry, do it just once, when reading the per syscall info.

Cc Adrian Hunter <[email protected]>
Cc David Ahern <[email protected]>
Cc Don Zickus <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Mike Galbraith <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/builtin-trace.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
tools/perf/builtin-trace.c +4 -1
index 28c86e21fad9..5549cee61680 100644
@@ -1108,6 +1108,7 @@ struct syscall {
 	struct event_format *tp_format;
 	const char	    *name;
 	bool		    filtered;
+	bool		    is_exit;
 	struct syscall_fmt  *fmt;
 	size_t		    (**arg_scnprintf)(char *bf, size_t size, struct syscall_arg *arg);
 	void		    **arg_parm;
@@ -1473,6 +1474,8 @@ static int trace__read_syscall_info(struct trace *trace, int id)
 	if (sc->tp_format == NULL)
 		return -1;
 
+	sc->is_exit = !strcmp(name, "exit_group") || !strcmp(name, "exit");
+
 	return syscall__set_arg_fmts(sc);
 }
 
@@ -1643,7 +1646,7 @@ static int trace__sys_enter(struct trace *trace, struct perf_evsel *evsel,
 	printed += syscall__scnprintf_args(sc, msg + printed, 1024 - printed,
 					   args, trace, thread);
 
-	if (!strcmp(sc->name, "exit_group") || !strcmp(sc->name, "exit")) {
+	if (sc->is_exit) {
 		if (!trace->duration_filter && !trace->summary_only) {
 			trace__fprintf_entry_head(trace, thread, 1, sample->time, trace->output);
 			fprintf(trace->output, "%-70s\n", ttrace->entry_str);
Signature · 1 line
1.9.3
author maintainer
#5 lore raw

PATCH 5/11 perf hists browser: Remove ev_name argument from perf_evsel__hists_browse

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

Removing ev_name argument from perf_evsel__hists_browse function, because it's not needed. We can get the name out of the 'struct perf_evsel' which is passed as argument as well.

Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/ui/browsers/hists.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)
tools/perf/ui/browsers/hists.c +11 -14
index 52c03fbbba17..1bd35e8ed9f1 100644
@@ -32,8 +32,7 @@ struct hist_browser {
 
 extern void hist_browser__init_hpp(void);
 
-static int hists__browser_title(struct hists *hists, char *bf, size_t size,
-				const char *ev_name);
+static int hists__browser_title(struct hists *hists, char *bf, size_t size);
 static void hist_browser__update_nr_entries(struct hist_browser *hb);
 
 static struct rb_node *hists__filter_entries(struct rb_node *nd,
@@ -345,7 +344,7 @@ static void ui_browser__warn_lost_events(struct ui_browser *browser)
 		"Or reduce the sampling frequency.");
 }
 
-static int hist_browser__run(struct hist_browser *browser, const char *ev_name,
+static int hist_browser__run(struct hist_browser *browser,
 			     struct hist_browser_timer *hbt)
 {
 	int key;
@@ -356,7 +355,7 @@ static int hist_browser__run(struct hist_browser *browser, const char *ev_name,
 	browser->b.nr_entries = hist_browser__nr_entries(browser);
 
 	hist_browser__refresh_dimensions(browser);
-	hists__browser_title(browser->hists, title, sizeof(title), ev_name);
+	hists__browser_title(browser->hists, title, sizeof(title));
 
 	if (ui_browser__show(&browser->b, title,
 			     "Press '?' for help on key bindings") < 0)
@@ -383,7 +382,7 @@ static int hist_browser__run(struct hist_browser *browser, const char *ev_name,
 				ui_browser__warn_lost_events(&browser->b);
 			}
 
-			hists__browser_title(browser->hists, title, sizeof(title), ev_name);
+			hists__browser_title(browser->hists, title, sizeof(title));
 			ui_browser__show_title(&browser->b, title);
 			continue;
 		}
@@ -1212,8 +1211,7 @@ static struct thread *hist_browser__selected_thread(struct hist_browser *browser
 	return browser->he_selection->thread;
 }
 
-static int hists__browser_title(struct hists *hists, char *bf, size_t size,
-				const char *ev_name)
+static int hists__browser_title(struct hists *hists, char *bf, size_t size)
 {
 	char unit;
 	int printed;
@@ -1222,6 +1220,7 @@ static int hists__browser_title(struct hists *hists, char *bf, size_t size,
 	unsigned long nr_samples = hists->stats.nr_events[PERF_RECORD_SAMPLE];
 	u64 nr_events = hists->stats.total_period;
 	struct perf_evsel *evsel = hists_to_evsel(hists);
+	const char *ev_name = perf_evsel__name(evsel);
 	char buf[512];
 	size_t buflen = sizeof(buf);
 
@@ -1389,7 +1388,7 @@ static void hist_browser__update_nr_entries(struct hist_browser *hb)
 }
 
 static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
-				    const char *helpline, const char *ev_name,
+				    const char *helpline,
 				    bool left_exits,
 				    struct hist_browser_timer *hbt,
 				    float min_pcnt,
@@ -1464,7 +1463,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 
 		nr_options = 0;
 
-		key = hist_browser__run(browser, ev_name, hbt);
+		key = hist_browser__run(browser, hbt);
 
 		if (browser->he_selection != NULL) {
 			thread = hist_browser__selected_thread(browser);
@@ -1832,7 +1831,7 @@ static int perf_evsel_menu__run(struct perf_evsel_menu *menu,
 {
 	struct perf_evlist *evlist = menu->b.priv;
 	struct perf_evsel *pos;
-	const char *ev_name, *title = "Available samples";
+	const char *title = "Available samples";
 	int delay_secs = hbt ? hbt->refresh : 0;
 	int key;
 
@@ -1865,9 +1864,8 @@ browse_hists:
 			 */
 			if (hbt)
 				hbt->timer(hbt->arg);
-			ev_name = perf_evsel__name(pos);
 			key = perf_evsel__hists_browse(pos, nr_events, help,
-						       ev_name, true, hbt,
+						       true, hbt,
 						       menu->min_pcnt,
 						       menu->env);
 			ui_browser__show_title(&menu->b, title);
@@ -1971,10 +1969,9 @@ int perf_evlist__tui_browse_hists(struct perf_evlist *evlist, const char *help,
 single_entry:
 	if (nr_entries == 1) {
 		struct perf_evsel *first = perf_evlist__first(evlist);
-		const char *ev_name = perf_evsel__name(first);
 
 		return perf_evsel__hists_browse(first, nr_entries, help,
-						ev_name, false, hbt, min_pcnt,
+						false, hbt, min_pcnt,
 						env);
 	}
 
Signature · 1 line
1.9.3
author maintainer
#6 lore raw

PATCH 6/11 perf ui browser: Fix scrollbar refresh row index

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

The ui_browser__gotorc function needs offset from 'y' member, so the row index has to begin with 0, which happens by accident in current code, because we display only one header line.

The bug shows when we want to display more than 1 header lines like columns headers in following patches.

Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/ui/browser.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/ui/browser.c +1 -1
index 3ccf6e14f89b..9d2294efc00c 100644
@@ -279,7 +279,7 @@ static void ui_browser__scrollbar_set(struct ui_browser *browser)
 {
 	int height = browser->height, h = 0, pct = 0,
 	    col = browser->width,
-	    row = browser->y - 1;
+	    row = 0;
 
 	if (browser->nr_entries > 1) {
 		pct = ((browser->index * (browser->height - 1)) /
Signature · 1 line
1.9.3
author maintainer
#7 lore raw

PATCH 7/11 perf bench sched-messaging: Plug memleak

In reply to Arnaldo Carvalho de Melo

From: Davidlohr Bueso <[email protected]>

Explicitly free the thread array ('pth_tab').

---
 tools/perf/bench/sched-messaging.c | 2 ++
 1 file changed, 2 insertions(+)
tools/perf/bench/sched-messaging.c +2 -0
index cc1190a0849b..fc4fe91ee098 100644
@@ -332,5 +332,7 @@ int bench_sched_messaging(int argc, const char **argv,
 		break;
 	}
 
+	free(pth_tab);
+
 	return 0;
 }
Signature · 1 line
1.9.3
author maintainer
#8 lore raw

PATCH 8/11 perf bench: Add --repeat option

In reply to Arnaldo Carvalho de Melo

From: Davidlohr Bueso <[email protected]>

There are a number of benchmarks that do single runs and as a result does not really help users gain a general idea of how the workload performs. So the user must either manually do multiple runs or just use single bogus results.

This option will enable users to specify the amount of runs (arbitrarily defaulted to 10, to use the existing benchmarks default) through the '--repeat' option. Add it to perf-bench instead of implementing it always in each specific benchmark.

[ Kept the existing default of 10, changing it to something else should
be done on separate patch ]

---
 tools/perf/Documentation/perf-bench.txt | 4 ++++
 tools/perf/bench/bench.h                | 1 +
 tools/perf/builtin-bench.c              | 7 +++++++
 3 files changed, 12 insertions(+)
tools/perf/Documentation/perf-bench.txt +4 -0
index 4464ad770d51..f6480cbf309b 100644
@@ -16,6 +16,10 @@ This 'perf bench' command is a general framework for benchmark suites.
 
 COMMON OPTIONS
 --------------
+-r::
+--repeat=::
+Specify amount of times to repeat the run (default 10).
+
 -f::
 --format=::
 Specify format style.
tools/perf/bench/bench.h +1 -0
index eba46709b279..3c4dd44d45cb 100644
@@ -43,5 +43,6 @@ extern int bench_futex_requeue(int argc, const char **argv, const char *prefix);
 #define BENCH_FORMAT_UNKNOWN		-1
 
 extern int bench_format;
+extern unsigned int bench_repeat;
 
 #endif
tools/perf/builtin-bench.c +7 -0
index 1e6e77710545..b9a56fa83330 100644
@@ -104,9 +104,11 @@ static const char *bench_format_str;
 
 /* Output/formatting style, exported to benchmark modules: */
 int bench_format = BENCH_FORMAT_DEFAULT;
+unsigned int bench_repeat = 10; /* default number of times to repeat the run */
 
 static const struct option bench_options[] = {
 	OPT_STRING('f', "format", &bench_format_str, "default", "Specify format style"),
+	OPT_UINTEGER('r', "repeat",  &bench_repeat,   "Specify amount of times to repeat the run"),
 	OPT_END()
 };
 
@@ -226,6 +228,11 @@ int cmd_bench(int argc, const char **argv, const char *prefix __maybe_unused)
 		goto end;
 	}
 
+	if (bench_repeat == 0) {
+		printf("Invalid repeat option: Must specify a positive value\n");
+		goto end;
+	}
+
 	if (argc < 1) {
 		print_usage();
 		goto end;
Signature · 1 line
1.9.3
author maintainer
#9 lore raw

PATCH 9/11 perf bench futex: Use global --repeat option

In reply to Arnaldo Carvalho de Melo

From: Davidlohr Bueso <[email protected]>

This option is available through perf-bench, use it instead and free the local option.

---
 tools/perf/bench/futex-requeue.c | 10 +---------
 tools/perf/bench/futex-wake.c    | 12 ++----------
 2 files changed, 3 insertions(+), 19 deletions(-)
tools/perf/bench/futex-requeue.c +1 -9
index a16255876f1d..732403bfd31a 100644
@@ -29,13 +29,6 @@ static u_int32_t futex1 = 0, futex2 = 0;
  */
 static unsigned int nrequeue = 1;
 
-/*
- * There can be significant variance from run to run,
- * the more repeats, the more exact the overall avg and
- * the better idea of the futex latency.
- */
-static unsigned int repeat = 10;
-
 static pthread_t *worker;
 static bool done = 0, silent = 0;
 static pthread_mutex_t thread_lock;
@@ -46,7 +39,6 @@ static unsigned int ncpus, threads_starting, nthreads = 0;
 static const struct option options[] = {
 	OPT_UINTEGER('t', "threads",  &nthreads, "Specify amount of threads"),
 	OPT_UINTEGER('q', "nrequeue", &nrequeue, "Specify amount of threads to requeue at once"),
-	OPT_UINTEGER('r', "repeat",   &repeat,   "Specify amount of times to repeat the run"),
 	OPT_BOOLEAN( 's', "silent",   &silent,   "Silent mode: do not display data/details"),
 	OPT_END()
 };
@@ -146,7 +138,7 @@ int bench_futex_requeue(int argc, const char **argv,
 	pthread_cond_init(&thread_parent, NULL);
 	pthread_cond_init(&thread_worker, NULL);
 
-	for (j = 0; j < repeat && !done; j++) {
+	for (j = 0; j < bench_repeat && !done; j++) {
 		unsigned int nrequeued = 0;
 		struct timeval start, end, runtime;
 
tools/perf/bench/futex-wake.c +2 -10
index d096169b161e..50022cbce87e 100644
@@ -30,15 +30,8 @@ static u_int32_t futex1 = 0;
  */
 static unsigned int nwakes = 1;
 
-/*
- * There can be significant variance from run to run,
- * the more repeats, the more exact the overall avg and
- * the better idea of the futex latency.
- */
-static unsigned int repeat = 10;
-
 pthread_t *worker;
-static bool done = 0, silent = 0;
+static bool done = false, silent = false;
 static pthread_mutex_t thread_lock;
 static pthread_cond_t thread_parent, thread_worker;
 static struct stats waketime_stats, wakeup_stats;
@@ -47,7 +40,6 @@ static unsigned int ncpus, threads_starting, nthreads = 0;
 static const struct option options[] = {
 	OPT_UINTEGER('t', "threads", &nthreads, "Specify amount of threads"),
 	OPT_UINTEGER('w', "nwakes",  &nwakes,   "Specify amount of threads to wake at once"),
-	OPT_UINTEGER('r', "repeat",  &repeat,   "Specify amount of times to repeat the run"),
 	OPT_BOOLEAN( 's', "silent",  &silent,   "Silent mode: do not display data/details"),
 	OPT_END()
 };
@@ -149,7 +141,7 @@ int bench_futex_wake(int argc, const char **argv,
 	pthread_cond_init(&thread_parent, NULL);
 	pthread_cond_init(&thread_worker, NULL);
 
-	for (j = 0; j < repeat && !done; j++) {
+	for (j = 0; j < bench_repeat && !done; j++) {
 		unsigned int nwoken = 0;
 		struct timeval start, end, runtime;
 
Signature · 1 line
1.9.3
author maintainer
#10 lore raw

PATCH 10/11 perf bench mem: The -o and -n options are mutually exclusive

In reply to Arnaldo Carvalho de Melo

From: Davidlohr Bueso <[email protected]>

-o, --only-prefault Show only the result with page faults before mem*
-n, --no-prefault Show only the result without page faults before mem*

Makes no sense to call together. Applies to both memset and memcpy.

---
 tools/perf/bench/mem-memcpy.c | 5 +++++
 tools/perf/bench/mem-memset.c | 5 +++++
 2 files changed, 10 insertions(+)
tools/perf/bench/mem-memcpy.c +5 -0
index 5ce71d3b72cf..e622c3e96efc 100644
@@ -189,6 +189,11 @@ int bench_mem_memcpy(int argc, const char **argv,
 	argc = parse_options(argc, argv, options,
 			     bench_mem_memcpy_usage, 0);
 
+	if (no_prefault && only_prefault) {
+		fprintf(stderr, "Invalid options: -o and -n are mutually exclusive\n");
+		return 1;
+	}
+
 	if (use_cycle)
 		init_cycle();
 
tools/perf/bench/mem-memset.c +5 -0
index 9af79d2b18e5..2a65468619f0 100644
@@ -181,6 +181,11 @@ int bench_mem_memset(int argc, const char **argv,
 	argc = parse_options(argc, argv, options,
 			     bench_mem_memset_usage, 0);
 
+	if (no_prefault && only_prefault) {
+		fprintf(stderr, "Invalid options: -o and -n are mutually exclusive\n");
+		return 1;
+	}
+
 	if (use_cycle)
 		init_cycle();
 
Signature · 1 line
1.9.3
author maintainer
#11 lore raw

PATCH 11/11 perf bench sched-messaging: Drop barf()

In reply to Arnaldo Carvalho de Melo

From: Davidlohr Bueso <[email protected]>

Instead of reinventing the wheel, we can use err(2) when dealing with fatal errors. Exit code is now always EXIT_FAILURE (1).

---
 tools/perf/bench/sched-messaging.c | 45 ++++++++++++++++----------------------
 1 file changed, 19 insertions(+), 26 deletions(-)
tools/perf/bench/sched-messaging.c +19 -26
index fc4fe91ee098..52a56599a543 100644
@@ -28,6 +28,7 @@
 #include <sys/time.h>
 #include <sys/poll.h>
 #include <limits.h>
+#include <err.h>
 
 #define DATASIZE 100
 
@@ -50,12 +51,6 @@ struct receiver_context {
 	int wakefd;
 };
 
-static void barf(const char *msg)
-{
-	fprintf(stderr, "%s (error: %s)\n", msg, strerror(errno));
-	exit(1);
-}
-
 static void fdpair(int fds[2])
 {
 	if (use_pipes) {
@@ -66,7 +61,7 @@ static void fdpair(int fds[2])
 			return;
 	}
 
-	barf(use_pipes ? "pipe()" : "socketpair()");
+	err(EXIT_FAILURE, use_pipes ? "pipe()" : "socketpair()");
 }
 
 /* Block until we're ready to go */
@@ -77,11 +72,11 @@ static void ready(int ready_out, int wakefd)
 
 	/* Tell them we're ready. */
 	if (write(ready_out, &dummy, 1) != 1)
-		barf("CLIENT: ready write");
+		err(EXIT_FAILURE, "CLIENT: ready write");
 
 	/* Wait for "GO" signal */
 	if (poll(&pollfd, 1, -1) != 1)
-		barf("poll");
+		err(EXIT_FAILURE, "poll");
 }
 
 /* Sender sprays loops messages down each file descriptor */
@@ -101,7 +96,7 @@ again:
 			ret = write(ctx->out_fds[j], data + done,
 				    sizeof(data)-done);
 			if (ret < 0)
-				barf("SENDER: write");
+				err(EXIT_FAILURE, "SENDER: write");
 			done += ret;
 			if (done < DATASIZE)
 				goto again;
@@ -131,7 +126,7 @@ static void *receiver(struct receiver_context* ctx)
 again:
 		ret = read(ctx->in_fds[0], data + done, DATASIZE - done);
 		if (ret < 0)
-			barf("SERVER: read");
+			err(EXIT_FAILURE, "SERVER: read");
 		done += ret;
 		if (done < DATASIZE)
 			goto again;
@@ -144,14 +139,14 @@ static pthread_t create_worker(void *ctx, void *(*func)(void *))
 {
 	pthread_attr_t attr;
 	pthread_t childid;
-	int err;
+	int ret;
 
 	if (!thread_mode) {
 		/* process mode */
 		/* Fork the receiver. */
 		switch (fork()) {
 		case -1:
-			barf("fork()");
+			err(EXIT_FAILURE, "fork()");
 			break;
 		case 0:
 			(*func) (ctx);
@@ -165,19 +160,17 @@ static pthread_t create_worker(void *ctx, void *(*func)(void *))
 	}
 
 	if (pthread_attr_init(&attr) != 0)
-		barf("pthread_attr_init:");
+		err(EXIT_FAILURE, "pthread_attr_init:");
 
 #ifndef __ia64__
 	if (pthread_attr_setstacksize(&attr, PTHREAD_STACK_MIN) != 0)
-		barf("pthread_attr_setstacksize");
+		err(EXIT_FAILURE, "pthread_attr_setstacksize");
 #endif
 
-	err = pthread_create(&childid, &attr, func, ctx);
-	if (err != 0) {
-		fprintf(stderr, "pthread_create failed: %s (%d)\n",
-			strerror(err), err);
-		exit(-1);
-	}
+	ret = pthread_create(&childid, &attr, func, ctx);
+	if (ret != 0)
+		err(EXIT_FAILURE, "pthread_create failed");
+
 	return childid;
 }
 
@@ -207,14 +200,14 @@ static unsigned int group(pthread_t *pth,
 			+ num_fds * sizeof(int));
 
 	if (!snd_ctx)
-		barf("malloc()");
+		err(EXIT_FAILURE, "malloc()");
 
 	for (i = 0; i < num_fds; i++) {
 		int fds[2];
 		struct receiver_context *ctx = malloc(sizeof(*ctx));
 
 		if (!ctx)
-			barf("malloc()");
+			err(EXIT_FAILURE, "malloc()");
 
 
 		/* Create the pipe between client and server */
@@ -281,7 +274,7 @@ int bench_sched_messaging(int argc, const char **argv,
 
 	pth_tab = malloc(num_fds * 2 * num_groups * sizeof(pthread_t));
 	if (!pth_tab)
-		barf("main:malloc()");
+		err(EXIT_FAILURE, "main:malloc()");
 
 	fdpair(readyfds);
 	fdpair(wakefds);
@@ -294,13 +287,13 @@ int bench_sched_messaging(int argc, const char **argv,
 	/* Wait for everyone to be ready */
 	for (i = 0; i < total_children; i++)
 		if (read(readyfds[0], &dummy, 1) != 1)
-			barf("Reading for readyfds");
+			err(EXIT_FAILURE, "Reading for readyfds");
 
 	gettimeofday(&start, NULL);
 
 	/* Kick them off */
 	if (write(wakefds[1], &dummy, 1) != 1)
-		barf("Writing to start them");
+		err(EXIT_FAILURE, "Writing to start them");
 
 	/* Reap them all */
 	for (i = 0; i < total_children; i++)
Signature · 1 line
1.9.3
#12 lore raw
In reply to Arnaldo Carvalho de Melo
Quoted 77 lines · Arnaldo Carvalho de Melo

Hi Ingo,

Please conseider pulling,

- Arnaldo

The following changes since commit a10d60c08cc3bbea9195e2b36440f557373623eb:

sh, perf: Use common PMU interrupt disabled code (2014-06-19 19:37:51 +0200)

are available in the git repository at:

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

for you to fetch changes up to ecdac96899e3db3f428e4d2e978f25e3f8d35a6c:

perf bench sched-messaging: Drop barf() (2014-06-19 16:13:17 -0300)

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

User visible:

. Add --repeat global option to 'perf bench' to be used in benchmarks
such as the existing 'futex' one, that was modified to use it instead of a local option. (Davidlohr Bueso)

. Fix fd -> pathname resolution in 'trace', be it using /proc or
a vfs_getname probe point. (Arnaldo Carvalho de Melo)

. Add suggestion of how to set perf_event_paranoid sysctl, to help
non-root users trying tools like 'trace' to get a working environment. (Arnaldo Carvalho de Melo)

Fixes:

. Fix memory leak in the 'sched-messaging' perf bench test. (Davidlohr Bueso)

. The -o and -n 'perf bench mem' options are mutually exclusive, emit error
when both are specified. (Davidlohr Bueso)

. Fix scrollbar refresh row index in the ui browser, problem exposed now
that headers will be added and will be allowed to be switched on/off. (Jiri Olsa)

Cleanups:

. Remove needless reassignments in 'trace' (Arnaldo Carvalho de Melo)

. Cache the is_exit syscall test in 'trace) (Arnaldo Carvalho de Melo)

. No need to reimplement err() in 'perf bench sched-messaging', drop barf().
(Davidlohr Bueso).

. Remove ev_name argument from perf_evsel__hists_browse, can be obtained
from the other parameters. (Jiri Olsa)

----------------------------------------------------------------
Arnaldo Carvalho de Melo (4):
perf trace: Fix up fd -> pathname resolution
perf evlist: Add suggestion of how to set perf_event_paranoid sysctl perf trace: Remove needless reassignments
perf trace: Cache the is_exit syscall test

Davidlohr Bueso (5):
perf bench sched-messaging: Plug memleak
perf bench: Add --repeat option
perf bench futex: Use global --repeat option
perf bench mem: The -o and -n options are mutually exclusive perf bench sched-messaging: Drop barf()

Jiri Olsa (2):
perf hists browser: Remove ev_name argument from perf_evsel__hists_browse perf ui browser: Fix scrollbar refresh row index

Quoted 13 lines
 tools/perf/Documentation/perf-bench.txt |  4 +++
 tools/perf/bench/bench.h                |  1 +
 tools/perf/bench/futex-requeue.c        | 10 +------
 tools/perf/bench/futex-wake.c           | 12 ++-------
 tools/perf/bench/mem-memcpy.c           |  5 ++++
 tools/perf/bench/mem-memset.c           |  5 ++++
 tools/perf/bench/sched-messaging.c      | 47 +++++++++++++++------------------
 tools/perf/builtin-bench.c              |  7 +++++
 tools/perf/builtin-trace.c              | 12 ++++-----
 tools/perf/ui/browser.c                 |  2 +-
 tools/perf/ui/browsers/hists.c          | 25 ++++++++----------
 tools/perf/util/evlist.c                |  5 ++--
 12 files changed, 67 insertions(+), 68 deletions(-)

Pulled, thanks a lot Arnaldo!

Ingo