[PATCH 1/3] perf: Make -f the default for perf record

[PATCH 1/3] perf: Make -f the default for perf record

[Frederic Weisbecker]

Force the overwriting mode by default if append mode is not explicit.
Adding -f every time one uses perf on a daily basis quickly becomes a
burden.

Keep the -f among the options though to avoid breaking some random
users scripts.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 tools/perf/Documentation/perf-record.txt |    2 +-
 tools/perf/builtin-record.c              |   40 ++++++++++++++++++-----------
 2 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index fc46c0b..b29bd2d 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -58,7 +58,7 @@ OPTIONS
 
 -f::
 --force::
-	Overwrite existing data file.
+	Overwrite existing data file. (deprecated)
 
 -c::
 --count=::
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 9a95136..dcda899 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -26,6 +26,11 @@
 #include <unistd.h>
 #include <sched.h>
 
+enum write_mode_t {
+	WRITE_FORCE,
+	WRITE_APPEND
+};
+
 static int			*fd[MAX_NR_CPUS][MAX_COUNTERS];
 
 static long			default_interval		=      0;
@@ -47,8 +52,7 @@ static pid_t			*all_tids			=      NULL;
 static int			thread_num			=      0;
 static pid_t			child_pid			=     -1;
 static bool			inherit				=   true;
-static bool			force				=  false;
-static bool			append_file			=  false;
+static enum write_mode_t	write_mode			= WRITE_FORCE;
 static bool			call_graph			=  false;
 static bool			inherit_stat			=  false;
 static bool			no_samples			=  false;
@@ -450,26 +454,19 @@ static int __cmd_record(int argc, const char **argv)
 	}
 
 	if (!stat(output_name, &st) && st.st_size) {
-		if (!force) {
-			if (!append_file) {
-				pr_err("Error, output file %s exists, use -A "
-				       "to append or -f to overwrite.\n",
-				       output_name);
-				exit(-1);
-			}
-		} else {
+		if (write_mode == WRITE_FORCE) {
 			char oldname[PATH_MAX];
 			snprintf(oldname, sizeof(oldname), "%s.old",
 				 output_name);
 			unlink(oldname);
 			rename(output_name, oldname);
 		}
-	} else {
-		append_file = false;
+	} else if (write_mode == WRITE_APPEND) {
+		write_mode = WRITE_FORCE;
 	}
 
 	flags = O_CREAT|O_RDWR;
-	if (append_file)
+	if (write_mode == WRITE_APPEND)
 		file_new = 0;
 	else
 		flags |= O_TRUNC;
@@ -480,7 +477,8 @@ static int __cmd_record(int argc, const char **argv)
 		exit(-1);
 	}
 
-	session = perf_session__new(output_name, O_WRONLY, force);
+	session = perf_session__new(output_name, O_WRONLY,
+				    write_mode == WRITE_FORCE);
 	if (session == NULL) {
 		pr_err("Not enough memory for reading perf file header\n");
 		return -1;
@@ -667,6 +665,8 @@ static const char * const record_usage[] = {
 	NULL
 };
 
+static bool force, append_file;
+
 static const struct option options[] = {
 	OPT_CALLBACK('e', "event", NULL, "event",
 		     "event selector. use 'perf list' to list available events",
@@ -688,7 +688,7 @@ static const struct option options[] = {
 	OPT_INTEGER('C', "profile_cpu", &profile_cpu,
 			    "CPU to profile on"),
 	OPT_BOOLEAN('f', "force", &force,
-			"overwrite existing data file"),
+			"overwrite existing data file (deprecated)"),
 	OPT_LONG('c', "count", &default_interval,
 		    "event period to sample"),
 	OPT_STRING('o', "output", &output_name, "file",
@@ -725,6 +725,16 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 		!system_wide && profile_cpu == -1)
 		usage_with_options(record_usage, options);
 
+	if (force && append_file) {
+		fprintf(stderr, "Can't overwrite and append at the same time."
+				" You need to choose between -f and -A");
+		usage_with_options(record_usage, options);
+	} else if (append_file) {
+		write_mode = WRITE_APPEND;
+	} else {
+		write_mode = WRITE_FORCE;
+	}
+
 	symbol__init();
 
 	if (!nr_counters) {
-- 
1.6.2.3

[PATCH 2/3] perf: Always record tracepoints raw samples from perf record

[Frederic Weisbecker]

Trace events are mostly used for tracing rather than simple
counting. Don't bother anymore with adding -R when using them,
just record raw samples of trace events every time.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 tools/perf/Documentation/perf-record.txt |    2 +-
 tools/perf/util/parse-events.c           |   14 ++++----------
 2 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index b29bd2d..020d871 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -101,7 +101,7 @@ OPTIONS
 
 -R::
 --raw-samples::
-Collect raw sample records from all opened counters (typically for tracepoint counters).
+Collect raw sample records from all opened counters (default for tracepoint counters).
 
 SEE ALSO
 --------
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 435781e..880070c 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -410,7 +410,6 @@ static enum event_result
 parse_single_tracepoint_event(char *sys_name,
 			      const char *evt_name,
 			      unsigned int evt_length,
-			      char *flags,
 			      struct perf_event_attr *attr,
 			      const char **strp)
 {
@@ -419,13 +418,9 @@ parse_single_tracepoint_event(char *sys_name,
 	u64 id;
 	int fd;
 
-	if (flags) {
-		if (!strncmp(flags, "record", strlen(flags))) {
-			attr->sample_type |= PERF_SAMPLE_RAW;
-			attr->sample_type |= PERF_SAMPLE_TIME;
-			attr->sample_type |= PERF_SAMPLE_CPU;
-		}
-	}
+	attr->sample_type |= PERF_SAMPLE_RAW;
+	attr->sample_type |= PERF_SAMPLE_TIME;
+	attr->sample_type |= PERF_SAMPLE_CPU;
 
 	snprintf(evt_path, MAXPATHLEN, "%s/%s/%s/id", debugfs_path,
 		 sys_name, evt_name);
@@ -533,8 +528,7 @@ static enum event_result parse_tracepoint_event(const char **strp,
 						       flags);
 	} else
 		return parse_single_tracepoint_event(sys_name, evt_name,
-						     evt_length, flags,
-						     attr, strp);
+						     evt_length, attr, strp);
 }
 
 static enum event_result
-- 
1.6.2.3

[PATCH 3/3] perf: Make the trace events sample period default to 1

[Frederic Weisbecker]

Trace events are mostly used for tracing and then require not to
be lost when possible. As opposite to hardware events that really
require to trigger after a given sample period, trace events mostly
need to trigger everytime.

It is a frustrating experience to trace with perf and realize we
lost a lot of events because we forgot the "-c 1" option.

Then default sample_period to 1 for trace events but let the user
override it.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 tools/perf/builtin-record.c    |   36 ++++++++++++++++++++++--------------
 tools/perf/util/parse-events.c |    2 ++
 2 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index dcda899..ca2affc 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -33,11 +33,13 @@ enum write_mode_t {
 
 static int			*fd[MAX_NR_CPUS][MAX_COUNTERS];
 
+static unsigned int		user_interval 			= UINT_MAX;
 static long			default_interval		=      0;
 
 static int			nr_cpus				=      0;
 static unsigned int		page_size;
 static unsigned int		mmap_pages			=    128;
+static unsigned int		user_freq 			= UINT_MAX;
 static int			freq				=   1000;
 static int			output;
 static const char		*output_name			= "perf.data";
@@ -255,10 +257,19 @@ static void create_counter(int counter, int cpu)
 	if (nr_counters > 1)
 		attr->sample_type |= PERF_SAMPLE_ID;
 
-	if (freq) {
-		attr->sample_type	|= PERF_SAMPLE_PERIOD;
-		attr->freq		= 1;
-		attr->sample_freq	= freq;
+	/*
+	 * We default some events to a 1 default interval. But keep
+	 * it a weak assumption overridable by the user.
+	 */
+	if (!attr->sample_period || (user_freq != UINT_MAX &&
+				     user_interval != UINT_MAX)) {
+		if (freq) {
+			attr->sample_type	|= PERF_SAMPLE_PERIOD;
+			attr->freq		= 1;
+			attr->sample_freq	= freq;
+		} else {
+			attr->sample_period = default_interval;
+		}
 	}
 
 	if (no_samples)
@@ -689,13 +700,13 @@ static const struct option options[] = {
 			    "CPU to profile on"),
 	OPT_BOOLEAN('f', "force", &force,
 			"overwrite existing data file (deprecated)"),
-	OPT_LONG('c', "count", &default_interval,
+	OPT_LONG('c', "count", &user_interval,
 		    "event period to sample"),
 	OPT_STRING('o', "output", &output_name, "file",
 		    "output file name"),
 	OPT_BOOLEAN('i', "inherit", &inherit,
 		    "child tasks inherit counters"),
-	OPT_INTEGER('F', "freq", &freq,
+	OPT_INTEGER('F', "freq", &user_freq,
 		    "profile at this frequency"),
 	OPT_INTEGER('m', "mmap-pages", &mmap_pages,
 		    "number of mmap data pages"),
@@ -716,7 +727,6 @@ static const struct option options[] = {
 
 int cmd_record(int argc, const char **argv, const char *prefix __used)
 {
-	int counter;
 	int i,j;
 
 	argc = parse_options(argc, argv, options, record_usage,
@@ -774,6 +784,11 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 	if (!event_array)
 		return -ENOMEM;
 
+	if (user_interval != UINT_MAX)
+		default_interval = user_interval;
+	if (user_freq != UINT_MAX)
+		freq = user_freq;
+
 	/*
 	 * User specified count overrides default frequency.
 	 */
@@ -786,12 +801,5 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 		exit(EXIT_FAILURE);
 	}
 
-	for (counter = 0; counter < nr_counters; counter++) {
-		if (attrs[counter].sample_period)
-			continue;
-
-		attrs[counter].sample_period = default_interval;
-	}
-
 	return __cmd_record(argc, argv);
 }
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 880070c..3b4ec67 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -422,6 +422,8 @@ parse_single_tracepoint_event(char *sys_name,
 	attr->sample_type |= PERF_SAMPLE_TIME;
 	attr->sample_type |= PERF_SAMPLE_CPU;
 
+	attr->sample_period = 1;
+
 	snprintf(evt_path, MAXPATHLEN, "%s/%s/%s/id", debugfs_path,
 		 sys_name, evt_name);
 
-- 
1.6.2.3