linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] perf: cleanups related to die/exit and error handling
@ 2012-08-26 18:24 David Ahern
  2012-08-26 18:24 ` [PATCH 1/7] perf tool: flush_sample_queue needs to handle errors from handlers David Ahern
                   ` (6 more replies)
  0 siblings, 7 replies; 15+ messages in thread
From: David Ahern @ 2012-08-26 18:24 UTC (permalink / raw)
  To: acme, linux-kernel; +Cc: David Ahern

Round 1 removing the use of die() and exit(). It's rather slow and tedious
to check all the error paths and make sure error propogation is properly
done and handled, so submitting patches in phases as I have time to work
on it.

David Ahern (7):
  perf tool: flush_sample_queue needs to handle errors from handlers
  perf tool: handle errors in synthesized event functions
  perf lock: remove use of die and handle errors
  perf stat: remove use of die/exit and handle errors
  perf help: remove remove use of die and handle errors
  perf script: remove use of die/exit
  perf record: remove use of die/exit

 tools/perf/builtin-help.c   |   48 ++++++++----
 tools/perf/builtin-lock.c   |  181 +++++++++++++++++++++++++++++--------------
 tools/perf/builtin-record.c |  158 +++++++++++++++++++++++++------------
 tools/perf/builtin-script.c |   60 +++++++++-----
 tools/perf/builtin-stat.c   |    7 +-
 tools/perf/util/event.c     |   35 ++++++---
 tools/perf/util/session.c   |   24 +++---
 7 files changed, 353 insertions(+), 160 deletions(-)

-- 
1.7.10.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/7] perf tool: flush_sample_queue needs to handle errors from handlers
  2012-08-26 18:24 [PATCH 0/7] perf: cleanups related to die/exit and error handling David Ahern
@ 2012-08-26 18:24 ` David Ahern
  2012-09-07  5:56   ` [tip:perf/core] perf session: " tip-bot for David Ahern
  2012-08-26 18:24 ` [PATCH 2/7] perf tool: handle errors in synthesized event functions David Ahern
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: David Ahern @ 2012-08-26 18:24 UTC (permalink / raw)
  To: acme, linux-kernel
  Cc: David Ahern, Ingo Molnar, Frederic Weisbecker, Peter Zijlstra

Allows errors to propogate through event processing code and back
to commands.

Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 tools/perf/util/session.c |   24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index f7bb7ae..9453758 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -692,7 +692,7 @@ static int perf_session_deliver_event(struct perf_session *session,
 				      struct perf_tool *tool,
 				      u64 file_offset);
 
-static void flush_sample_queue(struct perf_session *s,
+static int flush_sample_queue(struct perf_session *s,
 			       struct perf_tool *tool)
 {
 	struct ordered_samples *os = &s->ordered_samples;
@@ -705,7 +705,7 @@ static void flush_sample_queue(struct perf_session *s,
 	int ret;
 
 	if (!tool->ordered_samples || !limit)
-		return;
+		return 0;
 
 	list_for_each_entry_safe(iter, tmp, head, list) {
 		if (iter->timestamp > limit)
@@ -715,9 +715,12 @@ static void flush_sample_queue(struct perf_session *s,
 						s->header.needs_swap);
 		if (ret)
 			pr_err("Can't parse sample, err = %d\n", ret);
-		else
-			perf_session_deliver_event(s, iter->event, &sample, tool,
-						   iter->file_offset);
+		else {
+			ret = perf_session_deliver_event(s, iter->event, &sample, tool,
+							 iter->file_offset);
+			if (ret)
+				return ret;
+		}
 
 		os->last_flush = iter->timestamp;
 		list_del(&iter->list);
@@ -737,6 +740,8 @@ static void flush_sample_queue(struct perf_session *s,
 	}
 
 	os->nr_samples = 0;
+
+	return 0;
 }
 
 /*
@@ -782,10 +787,11 @@ static int process_finished_round(struct perf_tool *tool,
 				  union perf_event *event __used,
 				  struct perf_session *session)
 {
-	flush_sample_queue(session, tool);
-	session->ordered_samples.next_flush = session->ordered_samples.max_timestamp;
+	int ret = flush_sample_queue(session, tool);
+	if (!ret)
+		session->ordered_samples.next_flush = session->ordered_samples.max_timestamp;
 
-	return 0;
+	return ret;
 }
 
 /* The queue is ordered by time */
@@ -1443,7 +1449,7 @@ more:
 	err = 0;
 	/* do the final flush for ordered samples */
 	session->ordered_samples.next_flush = ULLONG_MAX;
-	flush_sample_queue(session, tool);
+	err = flush_sample_queue(session, tool);
 out_err:
 	perf_session__warn_about_errors(session, tool);
 	perf_session_free_sample_buffers(session);
-- 
1.7.10.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/7] perf tool: handle errors in synthesized event functions
  2012-08-26 18:24 [PATCH 0/7] perf: cleanups related to die/exit and error handling David Ahern
  2012-08-26 18:24 ` [PATCH 1/7] perf tool: flush_sample_queue needs to handle errors from handlers David Ahern
@ 2012-08-26 18:24 ` David Ahern
  2012-09-07  5:57   ` [tip:perf/core] " tip-bot for David Ahern
  2012-08-26 18:24 ` [PATCH 3/7] perf lock: remove use of die and handle errors David Ahern
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: David Ahern @ 2012-08-26 18:24 UTC (permalink / raw)
  To: acme, linux-kernel
  Cc: David Ahern, Ingo Molnar, Frederic Weisbecker, Peter Zijlstra

Handle error from process callback and propogate back to caller.

Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 tools/perf/util/event.c |   35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 3a0f1a5..84ff6f16 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -120,7 +120,9 @@ static pid_t perf_event__synthesize_comm(struct perf_tool *tool,
 	if (!full) {
 		event->comm.tid = pid;
 
-		process(tool, event, &synth_sample, machine);
+		if (process(tool, event, &synth_sample, machine) != 0)
+			return -1;
+
 		goto out;
 	}
 
@@ -151,7 +153,10 @@ static pid_t perf_event__synthesize_comm(struct perf_tool *tool,
 
 		event->comm.tid = pid;
 
-		process(tool, event, &synth_sample, machine);
+		if (process(tool, event, &synth_sample, machine) != 0) {
+			tgid = -1;
+			break;
+		}
 	}
 
 	closedir(tasks);
@@ -167,6 +172,7 @@ static int perf_event__synthesize_mmap_events(struct perf_tool *tool,
 {
 	char filename[PATH_MAX];
 	FILE *fp;
+	int rc = 0;
 
 	snprintf(filename, sizeof(filename), "/proc/%d/maps", pid);
 
@@ -231,18 +237,22 @@ static int perf_event__synthesize_mmap_events(struct perf_tool *tool,
 			event->mmap.pid = tgid;
 			event->mmap.tid = pid;
 
-			process(tool, event, &synth_sample, machine);
+			if (process(tool, event, &synth_sample, machine) != 0) {
+				rc = -1;
+				break;
+			}
 		}
 	}
 
 	fclose(fp);
-	return 0;
+	return rc;
 }
 
 int perf_event__synthesize_modules(struct perf_tool *tool,
 				   perf_event__handler_t process,
 				   struct machine *machine)
 {
+	int rc = 0;
 	struct rb_node *nd;
 	struct map_groups *kmaps = &machine->kmaps;
 	union perf_event *event = zalloc((sizeof(event->mmap) +
@@ -284,11 +294,14 @@ int perf_event__synthesize_modules(struct perf_tool *tool,
 
 		memcpy(event->mmap.filename, pos->dso->long_name,
 		       pos->dso->long_name_len + 1);
-		process(tool, event, &synth_sample, machine);
+		if (process(tool, event, &synth_sample, machine) != 0) {
+			rc = -1;
+			break;
+		}
 	}
 
 	free(event);
-	return 0;
+	return rc;
 }
 
 static int __event__synthesize_thread(union perf_event *comm_event,
@@ -392,12 +405,16 @@ int perf_event__synthesize_threads(struct perf_tool *tool,
 		if (*end) /* only interested in proper numerical dirents */
 			continue;
 
-		__event__synthesize_thread(comm_event, mmap_event, pid, 1,
-					   process, tool, machine);
+		if (__event__synthesize_thread(comm_event, mmap_event, pid, 1,
+					   process, tool, machine) != 0) {
+			err = -1;
+			goto out_closedir;
+		}
 	}
 
-	closedir(proc);
 	err = 0;
+out_closedir:
+	closedir(proc);
 out_free_mmap:
 	free(mmap_event);
 out_free_comm:
-- 
1.7.10.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/7] perf lock: remove use of die and handle errors
  2012-08-26 18:24 [PATCH 0/7] perf: cleanups related to die/exit and error handling David Ahern
  2012-08-26 18:24 ` [PATCH 1/7] perf tool: flush_sample_queue needs to handle errors from handlers David Ahern
  2012-08-26 18:24 ` [PATCH 2/7] perf tool: handle errors in synthesized event functions David Ahern
@ 2012-08-26 18:24 ` David Ahern
  2012-09-07  5:58   ` [tip:perf/core] perf lock: Remove " tip-bot for David Ahern
  2012-08-26 18:24 ` [PATCH 4/7] perf stat: remove use of die/exit " David Ahern
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: David Ahern @ 2012-08-26 18:24 UTC (permalink / raw)
  To: acme, linux-kernel
  Cc: David Ahern, Ingo Molnar, Frederic Weisbecker, Peter Zijlstra

Allows perf to clean up properly on exit.

Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 tools/perf/builtin-lock.c |  181 +++++++++++++++++++++++++++++++--------------
 1 file changed, 124 insertions(+), 57 deletions(-)

diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index 585aae2..75153c8 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -161,8 +161,10 @@ static struct thread_stat *thread_stat_findnew_after_first(u32 tid)
 		return st;
 
 	st = zalloc(sizeof(struct thread_stat));
-	if (!st)
-		die("memory allocation failed\n");
+	if (!st) {
+		pr_err("memory allocation failed\n");
+		return NULL;
+	}
 
 	st->tid = tid;
 	INIT_LIST_HEAD(&st->seq_list);
@@ -181,8 +183,10 @@ static struct thread_stat *thread_stat_findnew_first(u32 tid)
 	struct thread_stat *st;
 
 	st = zalloc(sizeof(struct thread_stat));
-	if (!st)
-		die("memory allocation failed\n");
+	if (!st) {
+		pr_err("memory allocation failed\n");
+		return NULL;
+	}
 	st->tid = tid;
 	INIT_LIST_HEAD(&st->seq_list);
 
@@ -248,18 +252,20 @@ struct lock_key keys[] = {
 	{ NULL, NULL }
 };
 
-static void select_key(void)
+static int select_key(void)
 {
 	int i;
 
 	for (i = 0; keys[i].name; i++) {
 		if (!strcmp(keys[i].name, sort_key)) {
 			compare = keys[i].key;
-			return;
+			return 0;
 		}
 	}
 
-	die("Unknown compare key:%s\n", sort_key);
+	pr_err("Unknown compare key: %s\n", sort_key);
+
+	return -1;
 }
 
 static void insert_to_result(struct lock_stat *st,
@@ -324,7 +330,8 @@ static struct lock_stat *lock_stat_findnew(void *addr, const char *name)
 	return new;
 
 alloc_failed:
-	die("memory allocation failed\n");
+	pr_err("memory allocation failed\n");
+	return NULL;
 }
 
 static const char *input_name;
@@ -356,16 +363,16 @@ struct trace_release_event {
 };
 
 struct trace_lock_handler {
-	void (*acquire_event)(struct trace_acquire_event *,
+	int (*acquire_event)(struct trace_acquire_event *,
 			      const struct perf_sample *sample);
 
-	void (*acquired_event)(struct trace_acquired_event *,
+	int (*acquired_event)(struct trace_acquired_event *,
 			       const struct perf_sample *sample);
 
-	void (*contended_event)(struct trace_contended_event *,
+	int (*contended_event)(struct trace_contended_event *,
 				const struct perf_sample *sample);
 
-	void (*release_event)(struct trace_release_event *,
+	int (*release_event)(struct trace_release_event *,
 			      const struct perf_sample *sample);
 };
 
@@ -379,8 +386,10 @@ static struct lock_seq_stat *get_seq(struct thread_stat *ts, void *addr)
 	}
 
 	seq = zalloc(sizeof(struct lock_seq_stat));
-	if (!seq)
-		die("Not enough memory\n");
+	if (!seq) {
+		pr_err("memory allocation failed\n");
+		return NULL;
+	}
 	seq->state = SEQ_STATE_UNINITIALIZED;
 	seq->addr = addr;
 
@@ -403,7 +412,7 @@ enum acquire_flags {
 	READ_LOCK = 2,
 };
 
-static void
+static int
 report_lock_acquire_event(struct trace_acquire_event *acquire_event,
 			  const struct perf_sample *sample)
 {
@@ -412,11 +421,18 @@ report_lock_acquire_event(struct trace_acquire_event *acquire_event,
 	struct lock_seq_stat *seq;
 
 	ls = lock_stat_findnew(acquire_event->addr, acquire_event->name);
+	if (!ls)
+		return -1;
 	if (ls->discard)
-		return;
+		return 0;
 
 	ts = thread_stat_findnew(sample->tid);
+	if (!ts)
+		return -1;
+
 	seq = get_seq(ts, acquire_event->addr);
+	if (!seq)
+		return -1;
 
 	switch (seq->state) {
 	case SEQ_STATE_UNINITIALIZED:
@@ -461,10 +477,10 @@ broken:
 	ls->nr_acquire++;
 	seq->prev_event_time = sample->time;
 end:
-	return;
+	return 0;
 }
 
-static void
+static int
 report_lock_acquired_event(struct trace_acquired_event *acquired_event,
 			   const struct perf_sample *sample)
 {
@@ -475,16 +491,23 @@ report_lock_acquired_event(struct trace_acquired_event *acquired_event,
 	u64 contended_term;
 
 	ls = lock_stat_findnew(acquired_event->addr, acquired_event->name);
+	if (!ls)
+		return -1;
 	if (ls->discard)
-		return;
+		return 0;
 
 	ts = thread_stat_findnew(sample->tid);
+	if (!ts)
+		return -1;
+
 	seq = get_seq(ts, acquired_event->addr);
+	if (!seq)
+		return -1;
 
 	switch (seq->state) {
 	case SEQ_STATE_UNINITIALIZED:
 		/* orphan event, do nothing */
-		return;
+		return 0;
 	case SEQ_STATE_ACQUIRING:
 		break;
 	case SEQ_STATE_CONTENDED:
@@ -515,10 +538,10 @@ report_lock_acquired_event(struct trace_acquired_event *acquired_event,
 	ls->nr_acquired++;
 	seq->prev_event_time = timestamp;
 end:
-	return;
+	return 0;
 }
 
-static void
+static int
 report_lock_contended_event(struct trace_contended_event *contended_event,
 			    const struct perf_sample *sample)
 {
@@ -527,16 +550,23 @@ report_lock_contended_event(struct trace_contended_event *contended_event,
 	struct lock_seq_stat *seq;
 
 	ls = lock_stat_findnew(contended_event->addr, contended_event->name);
+	if (!ls)
+		return -1;
 	if (ls->discard)
-		return;
+		return 0;
 
 	ts = thread_stat_findnew(sample->tid);
+	if (!ts)
+		return -1;
+
 	seq = get_seq(ts, contended_event->addr);
+	if (!seq)
+		return -1;
 
 	switch (seq->state) {
 	case SEQ_STATE_UNINITIALIZED:
 		/* orphan event, do nothing */
-		return;
+		return 0;
 	case SEQ_STATE_ACQUIRING:
 		break;
 	case SEQ_STATE_RELEASED:
@@ -559,10 +589,10 @@ report_lock_contended_event(struct trace_contended_event *contended_event,
 	ls->nr_contended++;
 	seq->prev_event_time = sample->time;
 end:
-	return;
+	return 0;
 }
 
-static void
+static int
 report_lock_release_event(struct trace_release_event *release_event,
 			  const struct perf_sample *sample)
 {
@@ -571,11 +601,18 @@ report_lock_release_event(struct trace_release_event *release_event,
 	struct lock_seq_stat *seq;
 
 	ls = lock_stat_findnew(release_event->addr, release_event->name);
+	if (!ls)
+		return -1;
 	if (ls->discard)
-		return;
+		return 0;
 
 	ts = thread_stat_findnew(sample->tid);
+	if (!ts)
+		return -1;
+
 	seq = get_seq(ts, release_event->addr);
+	if (!seq)
+		return -1;
 
 	switch (seq->state) {
 	case SEQ_STATE_UNINITIALIZED:
@@ -609,7 +646,7 @@ free_seq:
 	list_del(&seq->list);
 	free(seq);
 end:
-	return;
+	return 0;
 }
 
 /* lock oriented handlers */
@@ -623,13 +660,14 @@ static struct trace_lock_handler report_lock_ops  = {
 
 static struct trace_lock_handler *trace_handler;
 
-static void perf_evsel__process_lock_acquire(struct perf_evsel *evsel,
+static int perf_evsel__process_lock_acquire(struct perf_evsel *evsel,
 					     struct perf_sample *sample)
 {
 	struct trace_acquire_event acquire_event;
 	struct event_format *event = evsel->tp_format;
 	void *data = sample->raw_data;
 	u64 tmp;		/* this is required for casting... */
+	int rc = 0;
 
 	tmp = raw_field_value(event, "lockdep_addr", data);
 	memcpy(&acquire_event.addr, &tmp, sizeof(void *));
@@ -637,70 +675,84 @@ static void perf_evsel__process_lock_acquire(struct perf_evsel *evsel,
 	acquire_event.flag = (int)raw_field_value(event, "flag", data);
 
 	if (trace_handler->acquire_event)
-		trace_handler->acquire_event(&acquire_event, sample);
+		rc = trace_handler->acquire_event(&acquire_event, sample);
+
+	return rc;
 }
 
-static void perf_evsel__process_lock_acquired(struct perf_evsel *evsel,
+static int perf_evsel__process_lock_acquired(struct perf_evsel *evsel,
 					      struct perf_sample *sample)
 {
 	struct trace_acquired_event acquired_event;
 	struct event_format *event = evsel->tp_format;
 	void *data = sample->raw_data;
 	u64 tmp;		/* this is required for casting... */
+	int rc = 0;
 
 	tmp = raw_field_value(event, "lockdep_addr", data);
 	memcpy(&acquired_event.addr, &tmp, sizeof(void *));
 	acquired_event.name = (char *)raw_field_ptr(event, "name", data);
 
-	if (trace_handler->acquire_event)
-		trace_handler->acquired_event(&acquired_event, sample);
+	if (trace_handler->acquired_event)
+		rc = trace_handler->acquired_event(&acquired_event, sample);
+
+	return rc;
 }
 
-static void perf_evsel__process_lock_contended(struct perf_evsel *evsel,
+static int perf_evsel__process_lock_contended(struct perf_evsel *evsel,
 					       struct perf_sample *sample)
 {
 	struct trace_contended_event contended_event;
 	struct event_format *event = evsel->tp_format;
 	void *data = sample->raw_data;
 	u64 tmp;		/* this is required for casting... */
+	int rc = 0;
 
 	tmp = raw_field_value(event, "lockdep_addr", data);
 	memcpy(&contended_event.addr, &tmp, sizeof(void *));
 	contended_event.name = (char *)raw_field_ptr(event, "name", data);
 
-	if (trace_handler->acquire_event)
-		trace_handler->contended_event(&contended_event, sample);
+	if (trace_handler->contended_event)
+		rc = trace_handler->contended_event(&contended_event, sample);
+
+	return rc;
 }
 
-static void perf_evsel__process_lock_release(struct perf_evsel *evsel,
+static int perf_evsel__process_lock_release(struct perf_evsel *evsel,
 					     struct perf_sample *sample)
 {
 	struct trace_release_event release_event;
 	struct event_format *event = evsel->tp_format;
 	void *data = sample->raw_data;
 	u64 tmp;		/* this is required for casting... */
+	int rc = 0;
 
 	tmp = raw_field_value(event, "lockdep_addr", data);
 	memcpy(&release_event.addr, &tmp, sizeof(void *));
 	release_event.name = (char *)raw_field_ptr(event, "name", data);
 
-	if (trace_handler->acquire_event)
-		trace_handler->release_event(&release_event, sample);
+	if (trace_handler->release_event)
+		rc = trace_handler->release_event(&release_event, sample);
+
+	return rc;
 }
 
-static void perf_evsel__process_lock_event(struct perf_evsel *evsel,
+static int perf_evsel__process_lock_event(struct perf_evsel *evsel,
 					   struct perf_sample *sample)
 {
 	struct event_format *event = evsel->tp_format;
+	int rc = 0;
 
 	if (!strcmp(event->name, "lock_acquire"))
-		perf_evsel__process_lock_acquire(evsel, sample);
+		rc = perf_evsel__process_lock_acquire(evsel, sample);
 	if (!strcmp(event->name, "lock_acquired"))
-		perf_evsel__process_lock_acquired(evsel, sample);
+		rc = perf_evsel__process_lock_acquired(evsel, sample);
 	if (!strcmp(event->name, "lock_contended"))
-		perf_evsel__process_lock_contended(evsel, sample);
+		rc = perf_evsel__process_lock_contended(evsel, sample);
 	if (!strcmp(event->name, "lock_release"))
-		perf_evsel__process_lock_release(evsel, sample);
+		rc = perf_evsel__process_lock_release(evsel, sample);
+
+	return rc;
 }
 
 static void print_bad_events(int bad, int total)
@@ -802,14 +854,20 @@ static void dump_map(void)
 	}
 }
 
-static void dump_info(void)
+static int dump_info(void)
 {
+	int rc = 0;
+
 	if (info_threads)
 		dump_threads();
 	else if (info_map)
 		dump_map();
-	else
-		die("Unknown type of information\n");
+	else {
+		rc = -1;
+		pr_err("Unknown type of information\n");
+	}
+
+	return rc;
 }
 
 static int process_sample_event(struct perf_tool *tool __used,
@@ -826,8 +884,7 @@ static int process_sample_event(struct perf_tool *tool __used,
 		return -1;
 	}
 
-	perf_evsel__process_lock_event(evsel, sample);
-	return 0;
+	return perf_evsel__process_lock_event(evsel, sample);
 }
 
 static struct perf_tool eops = {
@@ -839,8 +896,10 @@ static struct perf_tool eops = {
 static int read_events(void)
 {
 	session = perf_session__new(input_name, O_RDONLY, 0, false, &eops);
-	if (!session)
-		die("Initializing perf session failed\n");
+	if (!session) {
+		pr_err("Initializing perf session failed\n");
+		return -1;
+	}
 
 	return perf_session__process_events(session, &eops);
 }
@@ -857,13 +916,18 @@ static void sort_result(void)
 	}
 }
 
-static void __cmd_report(void)
+static int __cmd_report(void)
 {
 	setup_pager();
-	select_key();
-	read_events();
+
+	if ((select_key() != 0) ||
+	    (read_events() != 0))
+		return -1;
+
 	sort_result();
 	print_result();
+
+	return 0;
 }
 
 static const char * const report_usage[] = {
@@ -959,6 +1023,7 @@ static int __cmd_record(int argc, const char **argv)
 int cmd_lock(int argc, const char **argv, const char *prefix __used)
 {
 	unsigned int i;
+	int rc = 0;
 
 	symbol__init();
 	for (i = 0; i < LOCKHASH_SIZE; i++)
@@ -993,11 +1058,13 @@ int cmd_lock(int argc, const char **argv, const char *prefix __used)
 		/* recycling report_lock_ops */
 		trace_handler = &report_lock_ops;
 		setup_pager();
-		read_events();
-		dump_info();
+		if (read_events() != 0)
+			rc = -1;
+		else
+			rc = dump_info();
 	} else {
 		usage_with_options(lock_usage, lock_options);
 	}
 
-	return 0;
+	return rc;
 }
-- 
1.7.10.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 4/7] perf stat: remove use of die/exit and handle errors
  2012-08-26 18:24 [PATCH 0/7] perf: cleanups related to die/exit and error handling David Ahern
                   ` (2 preceding siblings ...)
  2012-08-26 18:24 ` [PATCH 3/7] perf lock: remove use of die and handle errors David Ahern
@ 2012-08-26 18:24 ` David Ahern
  2012-09-07  5:58   ` [tip:perf/core] perf stat: Remove use of die/ exit " tip-bot for David Ahern
  2012-08-26 18:24 ` [PATCH 5/7] perf help: remove remove use of die " David Ahern
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: David Ahern @ 2012-08-26 18:24 UTC (permalink / raw)
  To: acme, linux-kernel
  Cc: David Ahern, Ingo Molnar, Frederic Weisbecker, Peter Zijlstra

Allows perf to clean up properly on program termination.

Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 tools/perf/builtin-stat.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index d53d8ab..02f49eb 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -428,7 +428,7 @@ static int run_perf_stat(int argc __used, const char **argv)
 
 	if (forks && (pipe(child_ready_pipe) < 0 || pipe(go_pipe) < 0)) {
 		perror("failed to create pipes");
-		exit(1);
+		return -1;
 	}
 
 	if (forks) {
@@ -510,7 +510,8 @@ static int run_perf_stat(int argc __used, const char **argv)
 			}
 			if (child_pid != -1)
 				kill(child_pid, SIGTERM);
-			die("Not all events could be opened.\n");
+
+			pr_err("Not all events could be opened.\n");
 			return -1;
 		}
 		counter->supported = true;
@@ -1189,7 +1190,7 @@ int cmd_stat(int argc, const char **argv, const char *prefix __used)
 		output = fopen(output_name, mode);
 		if (!output) {
 			perror("failed to create output file");
-			exit(-1);
+			return -1;
 		}
 		clock_gettime(CLOCK_REALTIME, &tm);
 		fprintf(output, "# started on %s\n", ctime(&tm.tv_sec));
-- 
1.7.10.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 5/7] perf help: remove remove use of die and handle errors
  2012-08-26 18:24 [PATCH 0/7] perf: cleanups related to die/exit and error handling David Ahern
                   ` (3 preceding siblings ...)
  2012-08-26 18:24 ` [PATCH 4/7] perf stat: remove use of die/exit " David Ahern
@ 2012-08-26 18:24 ` David Ahern
  2012-09-07  5:59   ` [tip:perf/core] perf help: Remove " tip-bot for David Ahern
  2012-08-26 18:24 ` [PATCH 6/7] perf script: remove use of die/exit David Ahern
  2012-08-26 18:24 ` [PATCH 7/7] perf record: remove " David Ahern
  6 siblings, 1 reply; 15+ messages in thread
From: David Ahern @ 2012-08-26 18:24 UTC (permalink / raw)
  To: acme, linux-kernel
  Cc: David Ahern, Ingo Molnar, Frederic Weisbecker, Peter Zijlstra

Allows perf to clean up properly on exit.

Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 tools/perf/builtin-help.c |   48 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 34 insertions(+), 14 deletions(-)

diff --git a/tools/perf/builtin-help.c b/tools/perf/builtin-help.c
index 6d5a8a7..f9daae5 100644
--- a/tools/perf/builtin-help.c
+++ b/tools/perf/builtin-help.c
@@ -24,13 +24,14 @@ static struct man_viewer_info_list {
 } *man_viewer_info_list;
 
 enum help_format {
+	HELP_FORMAT_NONE,
 	HELP_FORMAT_MAN,
 	HELP_FORMAT_INFO,
 	HELP_FORMAT_WEB,
 };
 
 static bool show_all = false;
-static enum help_format help_format = HELP_FORMAT_MAN;
+static enum help_format help_format = HELP_FORMAT_NONE;
 static struct option builtin_help_options[] = {
 	OPT_BOOLEAN('a', "all", &show_all, "print all available commands"),
 	OPT_SET_UINT('m', "man", &help_format, "show man page", HELP_FORMAT_MAN),
@@ -54,7 +55,9 @@ static enum help_format parse_help_format(const char *format)
 		return HELP_FORMAT_INFO;
 	if (!strcmp(format, "web") || !strcmp(format, "html"))
 		return HELP_FORMAT_WEB;
-	die("unrecognized help format '%s'", format);
+
+	pr_err("unrecognized help format '%s'", format);
+	return HELP_FORMAT_NONE;
 }
 
 static const char *get_man_viewer_info(const char *name)
@@ -259,6 +262,8 @@ static int perf_help_config(const char *var, const char *value, void *cb)
 		if (!value)
 			return config_error_nonbool(var);
 		help_format = parse_help_format(value);
+		if (help_format == HELP_FORMAT_NONE)
+			return -1;
 		return 0;
 	}
 	if (!strcmp(var, "man.viewer")) {
@@ -352,7 +357,7 @@ static void exec_viewer(const char *name, const char *page)
 		warning("'%s': unknown man viewer.", name);
 }
 
-static void show_man_page(const char *perf_cmd)
+static int show_man_page(const char *perf_cmd)
 {
 	struct man_viewer_list *viewer;
 	const char *page = cmd_to_page(perf_cmd);
@@ -365,28 +370,35 @@ static void show_man_page(const char *perf_cmd)
 	if (fallback)
 		exec_viewer(fallback, page);
 	exec_viewer("man", page);
-	die("no man viewer handled the request");
+
+	pr_err("no man viewer handled the request");
+	return -1;
 }
 
-static void show_info_page(const char *perf_cmd)
+static int show_info_page(const char *perf_cmd)
 {
 	const char *page = cmd_to_page(perf_cmd);
 	setenv("INFOPATH", system_path(PERF_INFO_PATH), 1);
 	execlp("info", "info", "perfman", page, NULL);
+	return -1;
 }
 
-static void get_html_page_path(struct strbuf *page_path, const char *page)
+static int get_html_page_path(struct strbuf *page_path, const char *page)
 {
 	struct stat st;
 	const char *html_path = system_path(PERF_HTML_PATH);
 
 	/* Check that we have a perf documentation directory. */
 	if (stat(mkpath("%s/perf.html", html_path), &st)
-	    || !S_ISREG(st.st_mode))
-		die("'%s': not a documentation directory.", html_path);
+	    || !S_ISREG(st.st_mode)) {
+		pr_err("'%s': not a documentation directory.", html_path);
+		return -1;
+	}
 
 	strbuf_init(page_path, 0);
 	strbuf_addf(page_path, "%s/%s.html", html_path, page);
+
+	return 0;
 }
 
 /*
@@ -401,19 +413,23 @@ static void open_html(const char *path)
 }
 #endif
 
-static void show_html_page(const char *perf_cmd)
+static int show_html_page(const char *perf_cmd)
 {
 	const char *page = cmd_to_page(perf_cmd);
 	struct strbuf page_path; /* it leaks but we exec bellow */
 
-	get_html_page_path(&page_path, page);
+	if (get_html_page_path(&page_path, page) != 0)
+		return -1;
 
 	open_html(page_path.buf);
+
+	return 0;
 }
 
 int cmd_help(int argc, const char **argv, const char *prefix __used)
 {
 	const char *alias;
+	int rc = 0;
 
 	load_command_list("perf-", &main_cmds, &other_cmds);
 
@@ -444,16 +460,20 @@ int cmd_help(int argc, const char **argv, const char *prefix __used)
 
 	switch (help_format) {
 	case HELP_FORMAT_MAN:
-		show_man_page(argv[0]);
+		rc = show_man_page(argv[0]);
 		break;
 	case HELP_FORMAT_INFO:
-		show_info_page(argv[0]);
+		rc = show_info_page(argv[0]);
 		break;
 	case HELP_FORMAT_WEB:
-		show_html_page(argv[0]);
+		rc = show_html_page(argv[0]);
+		break;
+	case HELP_FORMAT_NONE:
+		/* fall-through */
 	default:
+		rc = -1;
 		break;
 	}
 
-	return 0;
+	return rc;
 }
-- 
1.7.10.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 6/7] perf script: remove use of die/exit
  2012-08-26 18:24 [PATCH 0/7] perf: cleanups related to die/exit and error handling David Ahern
                   ` (4 preceding siblings ...)
  2012-08-26 18:24 ` [PATCH 5/7] perf help: remove remove use of die " David Ahern
@ 2012-08-26 18:24 ` David Ahern
  2012-09-07  6:00   ` [tip:perf/core] perf script: Remove " tip-bot for David Ahern
  2012-08-26 18:24 ` [PATCH 7/7] perf record: remove " David Ahern
  6 siblings, 1 reply; 15+ messages in thread
From: David Ahern @ 2012-08-26 18:24 UTC (permalink / raw)
  To: acme, linux-kernel
  Cc: David Ahern, Ingo Molnar, Frederic Weisbecker, Peter Zijlstra

Allows perf to clean up properly on exit. Only exits left are exec
failures which are appropriate and usage callbacks that list available
options.

Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 tools/perf/builtin-script.c |   60 +++++++++++++++++++++++++++++--------------
 1 file changed, 41 insertions(+), 19 deletions(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 2d6e3b2..c350cfe 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1153,18 +1153,23 @@ static const struct option options[] = {
 	OPT_END()
 };
 
-static bool have_cmd(int argc, const char **argv)
+static int have_cmd(int argc, const char **argv)
 {
 	char **__argv = malloc(sizeof(const char *) * argc);
 
-	if (!__argv)
-		die("malloc");
+	if (!__argv) {
+		pr_err("malloc failed\n");
+		return -1;
+	}
+
 	memcpy(__argv, argv, sizeof(const char *) * argc);
 	argc = parse_options(argc, (const char **)__argv, record_options,
 			     NULL, PARSE_OPT_STOP_AT_NON_OPTION);
 	free(__argv);
 
-	return argc != 0;
+	system_wide = (argc == 0);
+
+	return 0;
 }
 
 int cmd_script(int argc, const char **argv, const char *prefix __used)
@@ -1231,13 +1236,13 @@ int cmd_script(int argc, const char **argv, const char *prefix __used)
 
 		if (pipe(live_pipe) < 0) {
 			perror("failed to create pipe");
-			exit(-1);
+			return -1;
 		}
 
 		pid = fork();
 		if (pid < 0) {
 			perror("failed to fork");
-			exit(-1);
+			return -1;
 		}
 
 		if (!pid) {
@@ -1249,13 +1254,18 @@ int cmd_script(int argc, const char **argv, const char *prefix __used)
 			if (is_top_script(argv[0])) {
 				system_wide = true;
 			} else if (!system_wide) {
-				system_wide = !have_cmd(argc - rep_args,
-							&argv[rep_args]);
+				if (have_cmd(argc - rep_args, &argv[rep_args]) != 0) {
+					err = -1;
+					goto out;
+				}
 			}
 
 			__argv = malloc((argc + 6) * sizeof(const char *));
-			if (!__argv)
-				die("malloc");
+			if (!__argv) {
+				pr_err("malloc failed\n");
+				err = -ENOMEM;
+				goto out;
+			}
 
 			__argv[j++] = "/bin/sh";
 			__argv[j++] = rec_script_path;
@@ -1277,8 +1287,12 @@ int cmd_script(int argc, const char **argv, const char *prefix __used)
 		close(live_pipe[1]);
 
 		__argv = malloc((argc + 4) * sizeof(const char *));
-		if (!__argv)
-			die("malloc");
+		if (!__argv) {
+			pr_err("malloc failed\n");
+			err = -ENOMEM;
+			goto out;
+		}
+
 		j = 0;
 		__argv[j++] = "/bin/sh";
 		__argv[j++] = rep_script_path;
@@ -1303,12 +1317,20 @@ int cmd_script(int argc, const char **argv, const char *prefix __used)
 
 		if (!rec_script_path)
 			system_wide = false;
-		else if (!system_wide)
-			system_wide = !have_cmd(argc - 1, &argv[1]);
+		else if (!system_wide) {
+			if (have_cmd(argc - 1, &argv[1]) != 0) {
+				err = -1;
+				goto out;
+			}
+		}
 
 		__argv = malloc((argc + 2) * sizeof(const char *));
-		if (!__argv)
-			die("malloc");
+		if (!__argv) {
+			pr_err("malloc failed\n");
+			err = -ENOMEM;
+			goto out;
+		}
+
 		__argv[j++] = "/bin/sh";
 		__argv[j++] = script_path;
 		if (system_wide)
@@ -1357,18 +1379,18 @@ int cmd_script(int argc, const char **argv, const char *prefix __used)
 		input = open(session->filename, O_RDONLY);	/* input_name */
 		if (input < 0) {
 			perror("failed to open file");
-			exit(-1);
+			return -1;
 		}
 
 		err = fstat(input, &perf_stat);
 		if (err < 0) {
 			perror("failed to stat file");
-			exit(-1);
+			return -1;
 		}
 
 		if (!perf_stat.st_size) {
 			fprintf(stderr, "zero-sized file, nothing to do!\n");
-			exit(0);
+			return 0;
 		}
 
 		scripting_ops = script_spec__lookup(generate_script_lang);
-- 
1.7.10.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 7/7] perf record: remove use of die/exit
  2012-08-26 18:24 [PATCH 0/7] perf: cleanups related to die/exit and error handling David Ahern
                   ` (5 preceding siblings ...)
  2012-08-26 18:24 ` [PATCH 6/7] perf script: remove use of die/exit David Ahern
@ 2012-08-26 18:24 ` David Ahern
  2012-09-07  6:01   ` [tip:perf/core] perf record: Remove " tip-bot for David Ahern
  6 siblings, 1 reply; 15+ messages in thread
From: David Ahern @ 2012-08-26 18:24 UTC (permalink / raw)
  To: acme, linux-kernel
  Cc: David Ahern, Ingo Molnar, Frederic Weisbecker, Peter Zijlstra

Allows perf to clean up properly on exit. If perf-record is exiting
due to failure, the on_exit should not run as the session has been
deleted.

Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 tools/perf/builtin-record.c |  158 +++++++++++++++++++++++++++++--------------
 1 file changed, 109 insertions(+), 49 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 479ff2a..7b8b891 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -71,19 +71,23 @@ static void advance_output(struct perf_record *rec, size_t size)
 	rec->bytes_written += size;
 }
 
-static void write_output(struct perf_record *rec, void *buf, size_t size)
+static int write_output(struct perf_record *rec, void *buf, size_t size)
 {
 	while (size) {
 		int ret = write(rec->output, buf, size);
 
-		if (ret < 0)
-			die("failed to write");
+		if (ret < 0) {
+			pr_err("failed to write\n");
+			return -1;
+		}
 
 		size -= ret;
 		buf += ret;
 
 		rec->bytes_written += ret;
 	}
+
+	return 0;
 }
 
 static int process_synthesized_event(struct perf_tool *tool,
@@ -92,11 +96,13 @@ static int process_synthesized_event(struct perf_tool *tool,
 				     struct machine *machine __used)
 {
 	struct perf_record *rec = container_of(tool, struct perf_record, tool);
-	write_output(rec, event, event->header.size);
+	if (write_output(rec, event, event->header.size) < 0)
+		return -1;
+
 	return 0;
 }
 
-static void perf_record__mmap_read(struct perf_record *rec,
+static int perf_record__mmap_read(struct perf_record *rec,
 				   struct perf_mmap *md)
 {
 	unsigned int head = perf_mmap__read_head(md);
@@ -104,9 +110,10 @@ static void perf_record__mmap_read(struct perf_record *rec,
 	unsigned char *data = md->base + rec->page_size;
 	unsigned long size;
 	void *buf;
+	int rc = 0;
 
 	if (old == head)
-		return;
+		return 0;
 
 	rec->samples++;
 
@@ -117,17 +124,26 @@ static void perf_record__mmap_read(struct perf_record *rec,
 		size = md->mask + 1 - (old & md->mask);
 		old += size;
 
-		write_output(rec, buf, size);
+		if (write_output(rec, buf, size) < 0) {
+			rc = -1;
+			goto out;
+		}
 	}
 
 	buf = &data[old & md->mask];
 	size = head - old;
 	old += size;
 
-	write_output(rec, buf, size);
+	if (write_output(rec, buf, size) < 0) {
+		rc = -1;
+		goto out;
+	}
 
 	md->prev = old;
 	perf_mmap__write_tail(md, old);
+
+out:
+	return rc;
 }
 
 static volatile int done = 0;
@@ -183,12 +199,13 @@ static bool perf_evlist__equal(struct perf_evlist *evlist,
 	return true;
 }
 
-static void perf_record__open(struct perf_record *rec)
+static int perf_record__open(struct perf_record *rec)
 {
 	struct perf_evsel *pos;
 	struct perf_evlist *evlist = rec->evlist;
 	struct perf_session *session = rec->session;
 	struct perf_record_opts *opts = &rec->opts;
+	int rc = 0;
 
 	perf_evlist__config_attrs(evlist, opts);
 
@@ -222,10 +239,13 @@ try_again:
 
 			if (err == EPERM || err == EACCES) {
 				ui__error_paranoid();
-				exit(EXIT_FAILURE);
+				rc = -err;
+				goto out;
 			} else if (err ==  ENODEV && opts->target.cpu_list) {
-				die("No such device - did you specify"
-					" an out-of-range profile CPU?\n");
+				pr_err("No such device - did you specify"
+				       " an out-of-range profile CPU?\n");
+				rc = -err;
+				goto out;
 			} else if (err == EINVAL) {
 				if (!opts->exclude_guest_missing &&
 				    (attr->exclude_guest || attr->exclude_host)) {
@@ -272,7 +292,8 @@ try_again:
 			if (err == ENOENT) {
 				ui__error("The %s event is not supported.\n",
 					  perf_evsel__name(pos));
-				exit(EXIT_FAILURE);
+				rc = -err;
+				goto out;
 			}
 
 			printf("\n");
@@ -280,34 +301,46 @@ try_again:
 			      err, strerror(err));
 
 #if defined(__i386__) || defined(__x86_64__)
-			if (attr->type == PERF_TYPE_HARDWARE && err == EOPNOTSUPP)
-				die("No hardware sampling interrupt available."
-				    " No APIC? If so then you can boot the kernel"
-				    " with the \"lapic\" boot parameter to"
-				    " force-enable it.\n");
+			if (attr->type == PERF_TYPE_HARDWARE &&
+			    err == EOPNOTSUPP) {
+				pr_err("No hardware sampling interrupt available."
+				       " No APIC? If so then you can boot the kernel"
+				       " with the \"lapic\" boot parameter to"
+				       " force-enable it.\n");
+				rc = -err;
+				goto out;
+			}
 #endif
 
-			die("No CONFIG_PERF_EVENTS=y kernel support configured?\n");
+			pr_err("No CONFIG_PERF_EVENTS=y kernel support configured?\n");
+			rc = -err;
+			goto out;
 		}
 	}
 
 	if (perf_evlist__set_filters(evlist)) {
 		error("failed to set filter with %d (%s)\n", errno,
 			strerror(errno));
-		exit(-1);
+		rc = -1;
+		goto out;
 	}
 
 	if (perf_evlist__mmap(evlist, opts->mmap_pages, false) < 0) {
-		if (errno == EPERM)
-			die("Permission error mapping pages.\n"
-			    "Consider increasing "
-			    "/proc/sys/kernel/perf_event_mlock_kb,\n"
-			    "or try again with a smaller value of -m/--mmap_pages.\n"
-			    "(current value: %d)\n", opts->mmap_pages);
-		else if (!is_power_of_2(opts->mmap_pages))
-			die("--mmap_pages/-m value must be a power of two.");
-
-		die("failed to mmap with %d (%s)\n", errno, strerror(errno));
+		if (errno == EPERM) {
+			pr_err("Permission error mapping pages.\n"
+			       "Consider increasing "
+			       "/proc/sys/kernel/perf_event_mlock_kb,\n"
+			       "or try again with a smaller value of -m/--mmap_pages.\n"
+			       "(current value: %d)\n", opts->mmap_pages);
+			rc = -errno;
+		} else if (!is_power_of_2(opts->mmap_pages)) {
+			pr_err("--mmap_pages/-m value must be a power of two.");
+			rc = -EINVAL;
+		} else {
+			pr_err("failed to mmap with %d (%s)\n", errno, strerror(errno));
+			rc = -errno;
+		}
+		goto out;
 	}
 
 	if (rec->file_new)
@@ -315,11 +348,14 @@ try_again:
 	else {
 		if (!perf_evlist__equal(session->evlist, evlist)) {
 			fprintf(stderr, "incompatible append\n");
-			exit(-1);
+			rc = -1;
+			goto out;
 		}
  	}
 
 	perf_session__set_id_hdr_size(session);
+out:
+	return rc;
 }
 
 static int process_buildids(struct perf_record *rec)
@@ -335,10 +371,13 @@ static int process_buildids(struct perf_record *rec)
 					      size, &build_id__mark_dso_hit_ops);
 }
 
-static void perf_record__exit(int status __used, void *arg)
+static void perf_record__exit(int status, void *arg)
 {
 	struct perf_record *rec = arg;
 
+	if (status != 0)
+		return;
+
 	if (!rec->opts.pipe_output) {
 		rec->session->header.data_size += rec->bytes_written;
 
@@ -393,17 +432,26 @@ static struct perf_event_header finished_round_event = {
 	.type = PERF_RECORD_FINISHED_ROUND,
 };
 
-static void perf_record__mmap_read_all(struct perf_record *rec)
+static int perf_record__mmap_read_all(struct perf_record *rec)
 {
 	int i;
+	int rc = 0;
 
 	for (i = 0; i < rec->evlist->nr_mmaps; i++) {
-		if (rec->evlist->mmap[i].base)
-			perf_record__mmap_read(rec, &rec->evlist->mmap[i]);
+		if (rec->evlist->mmap[i].base) {
+			if (perf_record__mmap_read(rec, &rec->evlist->mmap[i]) != 0) {
+				rc = -1;
+				goto out;
+			}
+		}
 	}
 
 	if (perf_header__has_feat(&rec->session->header, HEADER_TRACING_DATA))
-		write_output(rec, &finished_round_event, sizeof(finished_round_event));
+		rc = write_output(rec, &finished_round_event,
+				  sizeof(finished_round_event));
+
+out:
+	return rc;
 }
 
 static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
@@ -463,7 +511,7 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
 		output = open(output_name, flags, S_IRUSR | S_IWUSR);
 	if (output < 0) {
 		perror("failed to create output file");
-		exit(-1);
+		return -1;
 	}
 
 	rec->output = output;
@@ -503,7 +551,10 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
 		}
 	}
 
-	perf_record__open(rec);
+	if (perf_record__open(rec) != 0) {
+		err = -1;
+		goto out_delete_session;
+	}
 
 	/*
 	 * perf_session__delete(session) will be called at perf_record__exit()
@@ -513,19 +564,20 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
 	if (opts->pipe_output) {
 		err = perf_header__write_pipe(output);
 		if (err < 0)
-			return err;
+			goto out_delete_session;
 	} else if (rec->file_new) {
 		err = perf_session__write_header(session, evsel_list,
 						 output, false);
 		if (err < 0)
-			return err;
+			goto out_delete_session;
 	}
 
 	if (!rec->no_buildid
 	    && !perf_header__has_feat(&session->header, HEADER_BUILD_ID)) {
 		pr_err("Couldn't generate buildids. "
 		       "Use --no-buildid to profile anyway.\n");
-		return -1;
+		err = -1;
+		goto out_delete_session;
 	}
 
 	rec->post_processing_offset = lseek(output, 0, SEEK_CUR);
@@ -533,7 +585,8 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
 	machine = perf_session__find_host_machine(session);
 	if (!machine) {
 		pr_err("Couldn't find native kernel information.\n");
-		return -1;
+		err = -1;
+		goto out_delete_session;
 	}
 
 	if (opts->pipe_output) {
@@ -541,14 +594,14 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
 						   process_synthesized_event);
 		if (err < 0) {
 			pr_err("Couldn't synthesize attrs.\n");
-			return err;
+			goto out_delete_session;
 		}
 
 		err = perf_event__synthesize_event_types(tool, process_synthesized_event,
 							 machine);
 		if (err < 0) {
 			pr_err("Couldn't synthesize event_types.\n");
-			return err;
+			goto out_delete_session;
 		}
 
 		if (have_tracepoints(&evsel_list->entries)) {
@@ -564,7 +617,7 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
 								  process_synthesized_event);
 			if (err <= 0) {
 				pr_err("Couldn't record tracing data.\n");
-				return err;
+				goto out_delete_session;
 			}
 			advance_output(rec, err);
 		}
@@ -592,20 +645,24 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
 					       perf_event__synthesize_guest_os);
 
 	if (!opts->target.system_wide)
-		perf_event__synthesize_thread_map(tool, evsel_list->threads,
+		err = perf_event__synthesize_thread_map(tool, evsel_list->threads,
 						  process_synthesized_event,
 						  machine);
 	else
-		perf_event__synthesize_threads(tool, process_synthesized_event,
+		err = perf_event__synthesize_threads(tool, process_synthesized_event,
 					       machine);
 
+	if (err != 0)
+		goto out_delete_session;
+
 	if (rec->realtime_prio) {
 		struct sched_param param;
 
 		param.sched_priority = rec->realtime_prio;
 		if (sched_setscheduler(0, SCHED_FIFO, &param)) {
 			pr_err("Could not set realtime priority.\n");
-			exit(-1);
+			err = -1;
+			goto out_delete_session;
 		}
 	}
 
@@ -620,7 +677,10 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
 	for (;;) {
 		int hits = rec->samples;
 
-		perf_record__mmap_read_all(rec);
+		if (perf_record__mmap_read_all(rec) < 0) {
+			err = -1;
+			goto out_delete_session;
+		}
 
 		if (hits == rec->samples) {
 			if (done)
-- 
1.7.10.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [tip:perf/core] perf session: flush_sample_queue needs to handle errors from handlers
  2012-08-26 18:24 ` [PATCH 1/7] perf tool: flush_sample_queue needs to handle errors from handlers David Ahern
@ 2012-09-07  5:56   ` tip-bot for David Ahern
  0 siblings, 0 replies; 15+ messages in thread
From: tip-bot for David Ahern @ 2012-09-07  5:56 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, hpa, mingo, peterz, fweisbec, dsahern, tglx

Commit-ID:  d25380cd3be38baff4ab31935b9d19b7f58ba7ac
Gitweb:     http://git.kernel.org/tip/d25380cd3be38baff4ab31935b9d19b7f58ba7ac
Author:     David Ahern <dsahern@gmail.com>
AuthorDate: Sun, 26 Aug 2012 12:24:41 -0600
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 5 Sep 2012 17:17:30 -0300

perf session: flush_sample_queue needs to handle errors from handlers

Allows errors to propogate through event processing code and back to
commands.

Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1346005487-62961-2-git-send-email-dsahern@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/session.c |   24 +++++++++++++++---------
 1 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index f7bb7ae..9453758 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -692,7 +692,7 @@ static int perf_session_deliver_event(struct perf_session *session,
 				      struct perf_tool *tool,
 				      u64 file_offset);
 
-static void flush_sample_queue(struct perf_session *s,
+static int flush_sample_queue(struct perf_session *s,
 			       struct perf_tool *tool)
 {
 	struct ordered_samples *os = &s->ordered_samples;
@@ -705,7 +705,7 @@ static void flush_sample_queue(struct perf_session *s,
 	int ret;
 
 	if (!tool->ordered_samples || !limit)
-		return;
+		return 0;
 
 	list_for_each_entry_safe(iter, tmp, head, list) {
 		if (iter->timestamp > limit)
@@ -715,9 +715,12 @@ static void flush_sample_queue(struct perf_session *s,
 						s->header.needs_swap);
 		if (ret)
 			pr_err("Can't parse sample, err = %d\n", ret);
-		else
-			perf_session_deliver_event(s, iter->event, &sample, tool,
-						   iter->file_offset);
+		else {
+			ret = perf_session_deliver_event(s, iter->event, &sample, tool,
+							 iter->file_offset);
+			if (ret)
+				return ret;
+		}
 
 		os->last_flush = iter->timestamp;
 		list_del(&iter->list);
@@ -737,6 +740,8 @@ static void flush_sample_queue(struct perf_session *s,
 	}
 
 	os->nr_samples = 0;
+
+	return 0;
 }
 
 /*
@@ -782,10 +787,11 @@ static int process_finished_round(struct perf_tool *tool,
 				  union perf_event *event __used,
 				  struct perf_session *session)
 {
-	flush_sample_queue(session, tool);
-	session->ordered_samples.next_flush = session->ordered_samples.max_timestamp;
+	int ret = flush_sample_queue(session, tool);
+	if (!ret)
+		session->ordered_samples.next_flush = session->ordered_samples.max_timestamp;
 
-	return 0;
+	return ret;
 }
 
 /* The queue is ordered by time */
@@ -1443,7 +1449,7 @@ more:
 	err = 0;
 	/* do the final flush for ordered samples */
 	session->ordered_samples.next_flush = ULLONG_MAX;
-	flush_sample_queue(session, tool);
+	err = flush_sample_queue(session, tool);
 out_err:
 	perf_session__warn_about_errors(session, tool);
 	perf_session_free_sample_buffers(session);

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [tip:perf/core] perf tool: handle errors in synthesized event functions
  2012-08-26 18:24 ` [PATCH 2/7] perf tool: handle errors in synthesized event functions David Ahern
@ 2012-09-07  5:57   ` tip-bot for David Ahern
  0 siblings, 0 replies; 15+ messages in thread
From: tip-bot for David Ahern @ 2012-09-07  5:57 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, hpa, mingo, peterz, fweisbec, dsahern, tglx

Commit-ID:  1e6d53223884225f0c3f9f1a3ac54a224d97ab24
Gitweb:     http://git.kernel.org/tip/1e6d53223884225f0c3f9f1a3ac54a224d97ab24
Author:     David Ahern <dsahern@gmail.com>
AuthorDate: Sun, 26 Aug 2012 12:24:42 -0600
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 5 Sep 2012 17:17:30 -0300

perf tool: handle errors in synthesized event functions

Handle error from process callback and propagate back to caller.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1346005487-62961-3-git-send-email-dsahern@gmail.com
Signed-off-by: David Ahern <dsahern@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/event.c |   35 ++++++++++++++++++++++++++---------
 1 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 3a0f1a5..84ff6f16 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -120,7 +120,9 @@ static pid_t perf_event__synthesize_comm(struct perf_tool *tool,
 	if (!full) {
 		event->comm.tid = pid;
 
-		process(tool, event, &synth_sample, machine);
+		if (process(tool, event, &synth_sample, machine) != 0)
+			return -1;
+
 		goto out;
 	}
 
@@ -151,7 +153,10 @@ static pid_t perf_event__synthesize_comm(struct perf_tool *tool,
 
 		event->comm.tid = pid;
 
-		process(tool, event, &synth_sample, machine);
+		if (process(tool, event, &synth_sample, machine) != 0) {
+			tgid = -1;
+			break;
+		}
 	}
 
 	closedir(tasks);
@@ -167,6 +172,7 @@ static int perf_event__synthesize_mmap_events(struct perf_tool *tool,
 {
 	char filename[PATH_MAX];
 	FILE *fp;
+	int rc = 0;
 
 	snprintf(filename, sizeof(filename), "/proc/%d/maps", pid);
 
@@ -231,18 +237,22 @@ static int perf_event__synthesize_mmap_events(struct perf_tool *tool,
 			event->mmap.pid = tgid;
 			event->mmap.tid = pid;
 
-			process(tool, event, &synth_sample, machine);
+			if (process(tool, event, &synth_sample, machine) != 0) {
+				rc = -1;
+				break;
+			}
 		}
 	}
 
 	fclose(fp);
-	return 0;
+	return rc;
 }
 
 int perf_event__synthesize_modules(struct perf_tool *tool,
 				   perf_event__handler_t process,
 				   struct machine *machine)
 {
+	int rc = 0;
 	struct rb_node *nd;
 	struct map_groups *kmaps = &machine->kmaps;
 	union perf_event *event = zalloc((sizeof(event->mmap) +
@@ -284,11 +294,14 @@ int perf_event__synthesize_modules(struct perf_tool *tool,
 
 		memcpy(event->mmap.filename, pos->dso->long_name,
 		       pos->dso->long_name_len + 1);
-		process(tool, event, &synth_sample, machine);
+		if (process(tool, event, &synth_sample, machine) != 0) {
+			rc = -1;
+			break;
+		}
 	}
 
 	free(event);
-	return 0;
+	return rc;
 }
 
 static int __event__synthesize_thread(union perf_event *comm_event,
@@ -392,12 +405,16 @@ int perf_event__synthesize_threads(struct perf_tool *tool,
 		if (*end) /* only interested in proper numerical dirents */
 			continue;
 
-		__event__synthesize_thread(comm_event, mmap_event, pid, 1,
-					   process, tool, machine);
+		if (__event__synthesize_thread(comm_event, mmap_event, pid, 1,
+					   process, tool, machine) != 0) {
+			err = -1;
+			goto out_closedir;
+		}
 	}
 
-	closedir(proc);
 	err = 0;
+out_closedir:
+	closedir(proc);
 out_free_mmap:
 	free(mmap_event);
 out_free_comm:

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [tip:perf/core] perf lock: Remove use of die and handle errors
  2012-08-26 18:24 ` [PATCH 3/7] perf lock: remove use of die and handle errors David Ahern
@ 2012-09-07  5:58   ` tip-bot for David Ahern
  0 siblings, 0 replies; 15+ messages in thread
From: tip-bot for David Ahern @ 2012-09-07  5:58 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, hpa, mingo, peterz, fweisbec, dsahern, tglx

Commit-ID:  33d6aef5136075930f7e9a05175bf4f772d8428e
Gitweb:     http://git.kernel.org/tip/33d6aef5136075930f7e9a05175bf4f772d8428e
Author:     David Ahern <dsahern@gmail.com>
AuthorDate: Sun, 26 Aug 2012 12:24:43 -0600
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 5 Sep 2012 17:19:38 -0300

perf lock: Remove use of die and handle errors

Allows perf to clean up properly on exit.

Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1346005487-62961-4-git-send-email-dsahern@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-lock.c |  181 +++++++++++++++++++++++++++++++--------------
 1 files changed, 124 insertions(+), 57 deletions(-)

diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index 585aae2..75153c8 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -161,8 +161,10 @@ static struct thread_stat *thread_stat_findnew_after_first(u32 tid)
 		return st;
 
 	st = zalloc(sizeof(struct thread_stat));
-	if (!st)
-		die("memory allocation failed\n");
+	if (!st) {
+		pr_err("memory allocation failed\n");
+		return NULL;
+	}
 
 	st->tid = tid;
 	INIT_LIST_HEAD(&st->seq_list);
@@ -181,8 +183,10 @@ static struct thread_stat *thread_stat_findnew_first(u32 tid)
 	struct thread_stat *st;
 
 	st = zalloc(sizeof(struct thread_stat));
-	if (!st)
-		die("memory allocation failed\n");
+	if (!st) {
+		pr_err("memory allocation failed\n");
+		return NULL;
+	}
 	st->tid = tid;
 	INIT_LIST_HEAD(&st->seq_list);
 
@@ -248,18 +252,20 @@ struct lock_key keys[] = {
 	{ NULL, NULL }
 };
 
-static void select_key(void)
+static int select_key(void)
 {
 	int i;
 
 	for (i = 0; keys[i].name; i++) {
 		if (!strcmp(keys[i].name, sort_key)) {
 			compare = keys[i].key;
-			return;
+			return 0;
 		}
 	}
 
-	die("Unknown compare key:%s\n", sort_key);
+	pr_err("Unknown compare key: %s\n", sort_key);
+
+	return -1;
 }
 
 static void insert_to_result(struct lock_stat *st,
@@ -324,7 +330,8 @@ static struct lock_stat *lock_stat_findnew(void *addr, const char *name)
 	return new;
 
 alloc_failed:
-	die("memory allocation failed\n");
+	pr_err("memory allocation failed\n");
+	return NULL;
 }
 
 static const char *input_name;
@@ -356,16 +363,16 @@ struct trace_release_event {
 };
 
 struct trace_lock_handler {
-	void (*acquire_event)(struct trace_acquire_event *,
+	int (*acquire_event)(struct trace_acquire_event *,
 			      const struct perf_sample *sample);
 
-	void (*acquired_event)(struct trace_acquired_event *,
+	int (*acquired_event)(struct trace_acquired_event *,
 			       const struct perf_sample *sample);
 
-	void (*contended_event)(struct trace_contended_event *,
+	int (*contended_event)(struct trace_contended_event *,
 				const struct perf_sample *sample);
 
-	void (*release_event)(struct trace_release_event *,
+	int (*release_event)(struct trace_release_event *,
 			      const struct perf_sample *sample);
 };
 
@@ -379,8 +386,10 @@ static struct lock_seq_stat *get_seq(struct thread_stat *ts, void *addr)
 	}
 
 	seq = zalloc(sizeof(struct lock_seq_stat));
-	if (!seq)
-		die("Not enough memory\n");
+	if (!seq) {
+		pr_err("memory allocation failed\n");
+		return NULL;
+	}
 	seq->state = SEQ_STATE_UNINITIALIZED;
 	seq->addr = addr;
 
@@ -403,7 +412,7 @@ enum acquire_flags {
 	READ_LOCK = 2,
 };
 
-static void
+static int
 report_lock_acquire_event(struct trace_acquire_event *acquire_event,
 			  const struct perf_sample *sample)
 {
@@ -412,11 +421,18 @@ report_lock_acquire_event(struct trace_acquire_event *acquire_event,
 	struct lock_seq_stat *seq;
 
 	ls = lock_stat_findnew(acquire_event->addr, acquire_event->name);
+	if (!ls)
+		return -1;
 	if (ls->discard)
-		return;
+		return 0;
 
 	ts = thread_stat_findnew(sample->tid);
+	if (!ts)
+		return -1;
+
 	seq = get_seq(ts, acquire_event->addr);
+	if (!seq)
+		return -1;
 
 	switch (seq->state) {
 	case SEQ_STATE_UNINITIALIZED:
@@ -461,10 +477,10 @@ broken:
 	ls->nr_acquire++;
 	seq->prev_event_time = sample->time;
 end:
-	return;
+	return 0;
 }
 
-static void
+static int
 report_lock_acquired_event(struct trace_acquired_event *acquired_event,
 			   const struct perf_sample *sample)
 {
@@ -475,16 +491,23 @@ report_lock_acquired_event(struct trace_acquired_event *acquired_event,
 	u64 contended_term;
 
 	ls = lock_stat_findnew(acquired_event->addr, acquired_event->name);
+	if (!ls)
+		return -1;
 	if (ls->discard)
-		return;
+		return 0;
 
 	ts = thread_stat_findnew(sample->tid);
+	if (!ts)
+		return -1;
+
 	seq = get_seq(ts, acquired_event->addr);
+	if (!seq)
+		return -1;
 
 	switch (seq->state) {
 	case SEQ_STATE_UNINITIALIZED:
 		/* orphan event, do nothing */
-		return;
+		return 0;
 	case SEQ_STATE_ACQUIRING:
 		break;
 	case SEQ_STATE_CONTENDED:
@@ -515,10 +538,10 @@ report_lock_acquired_event(struct trace_acquired_event *acquired_event,
 	ls->nr_acquired++;
 	seq->prev_event_time = timestamp;
 end:
-	return;
+	return 0;
 }
 
-static void
+static int
 report_lock_contended_event(struct trace_contended_event *contended_event,
 			    const struct perf_sample *sample)
 {
@@ -527,16 +550,23 @@ report_lock_contended_event(struct trace_contended_event *contended_event,
 	struct lock_seq_stat *seq;
 
 	ls = lock_stat_findnew(contended_event->addr, contended_event->name);
+	if (!ls)
+		return -1;
 	if (ls->discard)
-		return;
+		return 0;
 
 	ts = thread_stat_findnew(sample->tid);
+	if (!ts)
+		return -1;
+
 	seq = get_seq(ts, contended_event->addr);
+	if (!seq)
+		return -1;
 
 	switch (seq->state) {
 	case SEQ_STATE_UNINITIALIZED:
 		/* orphan event, do nothing */
-		return;
+		return 0;
 	case SEQ_STATE_ACQUIRING:
 		break;
 	case SEQ_STATE_RELEASED:
@@ -559,10 +589,10 @@ report_lock_contended_event(struct trace_contended_event *contended_event,
 	ls->nr_contended++;
 	seq->prev_event_time = sample->time;
 end:
-	return;
+	return 0;
 }
 
-static void
+static int
 report_lock_release_event(struct trace_release_event *release_event,
 			  const struct perf_sample *sample)
 {
@@ -571,11 +601,18 @@ report_lock_release_event(struct trace_release_event *release_event,
 	struct lock_seq_stat *seq;
 
 	ls = lock_stat_findnew(release_event->addr, release_event->name);
+	if (!ls)
+		return -1;
 	if (ls->discard)
-		return;
+		return 0;
 
 	ts = thread_stat_findnew(sample->tid);
+	if (!ts)
+		return -1;
+
 	seq = get_seq(ts, release_event->addr);
+	if (!seq)
+		return -1;
 
 	switch (seq->state) {
 	case SEQ_STATE_UNINITIALIZED:
@@ -609,7 +646,7 @@ free_seq:
 	list_del(&seq->list);
 	free(seq);
 end:
-	return;
+	return 0;
 }
 
 /* lock oriented handlers */
@@ -623,13 +660,14 @@ static struct trace_lock_handler report_lock_ops  = {
 
 static struct trace_lock_handler *trace_handler;
 
-static void perf_evsel__process_lock_acquire(struct perf_evsel *evsel,
+static int perf_evsel__process_lock_acquire(struct perf_evsel *evsel,
 					     struct perf_sample *sample)
 {
 	struct trace_acquire_event acquire_event;
 	struct event_format *event = evsel->tp_format;
 	void *data = sample->raw_data;
 	u64 tmp;		/* this is required for casting... */
+	int rc = 0;
 
 	tmp = raw_field_value(event, "lockdep_addr", data);
 	memcpy(&acquire_event.addr, &tmp, sizeof(void *));
@@ -637,70 +675,84 @@ static void perf_evsel__process_lock_acquire(struct perf_evsel *evsel,
 	acquire_event.flag = (int)raw_field_value(event, "flag", data);
 
 	if (trace_handler->acquire_event)
-		trace_handler->acquire_event(&acquire_event, sample);
+		rc = trace_handler->acquire_event(&acquire_event, sample);
+
+	return rc;
 }
 
-static void perf_evsel__process_lock_acquired(struct perf_evsel *evsel,
+static int perf_evsel__process_lock_acquired(struct perf_evsel *evsel,
 					      struct perf_sample *sample)
 {
 	struct trace_acquired_event acquired_event;
 	struct event_format *event = evsel->tp_format;
 	void *data = sample->raw_data;
 	u64 tmp;		/* this is required for casting... */
+	int rc = 0;
 
 	tmp = raw_field_value(event, "lockdep_addr", data);
 	memcpy(&acquired_event.addr, &tmp, sizeof(void *));
 	acquired_event.name = (char *)raw_field_ptr(event, "name", data);
 
-	if (trace_handler->acquire_event)
-		trace_handler->acquired_event(&acquired_event, sample);
+	if (trace_handler->acquired_event)
+		rc = trace_handler->acquired_event(&acquired_event, sample);
+
+	return rc;
 }
 
-static void perf_evsel__process_lock_contended(struct perf_evsel *evsel,
+static int perf_evsel__process_lock_contended(struct perf_evsel *evsel,
 					       struct perf_sample *sample)
 {
 	struct trace_contended_event contended_event;
 	struct event_format *event = evsel->tp_format;
 	void *data = sample->raw_data;
 	u64 tmp;		/* this is required for casting... */
+	int rc = 0;
 
 	tmp = raw_field_value(event, "lockdep_addr", data);
 	memcpy(&contended_event.addr, &tmp, sizeof(void *));
 	contended_event.name = (char *)raw_field_ptr(event, "name", data);
 
-	if (trace_handler->acquire_event)
-		trace_handler->contended_event(&contended_event, sample);
+	if (trace_handler->contended_event)
+		rc = trace_handler->contended_event(&contended_event, sample);
+
+	return rc;
 }
 
-static void perf_evsel__process_lock_release(struct perf_evsel *evsel,
+static int perf_evsel__process_lock_release(struct perf_evsel *evsel,
 					     struct perf_sample *sample)
 {
 	struct trace_release_event release_event;
 	struct event_format *event = evsel->tp_format;
 	void *data = sample->raw_data;
 	u64 tmp;		/* this is required for casting... */
+	int rc = 0;
 
 	tmp = raw_field_value(event, "lockdep_addr", data);
 	memcpy(&release_event.addr, &tmp, sizeof(void *));
 	release_event.name = (char *)raw_field_ptr(event, "name", data);
 
-	if (trace_handler->acquire_event)
-		trace_handler->release_event(&release_event, sample);
+	if (trace_handler->release_event)
+		rc = trace_handler->release_event(&release_event, sample);
+
+	return rc;
 }
 
-static void perf_evsel__process_lock_event(struct perf_evsel *evsel,
+static int perf_evsel__process_lock_event(struct perf_evsel *evsel,
 					   struct perf_sample *sample)
 {
 	struct event_format *event = evsel->tp_format;
+	int rc = 0;
 
 	if (!strcmp(event->name, "lock_acquire"))
-		perf_evsel__process_lock_acquire(evsel, sample);
+		rc = perf_evsel__process_lock_acquire(evsel, sample);
 	if (!strcmp(event->name, "lock_acquired"))
-		perf_evsel__process_lock_acquired(evsel, sample);
+		rc = perf_evsel__process_lock_acquired(evsel, sample);
 	if (!strcmp(event->name, "lock_contended"))
-		perf_evsel__process_lock_contended(evsel, sample);
+		rc = perf_evsel__process_lock_contended(evsel, sample);
 	if (!strcmp(event->name, "lock_release"))
-		perf_evsel__process_lock_release(evsel, sample);
+		rc = perf_evsel__process_lock_release(evsel, sample);
+
+	return rc;
 }
 
 static void print_bad_events(int bad, int total)
@@ -802,14 +854,20 @@ static void dump_map(void)
 	}
 }
 
-static void dump_info(void)
+static int dump_info(void)
 {
+	int rc = 0;
+
 	if (info_threads)
 		dump_threads();
 	else if (info_map)
 		dump_map();
-	else
-		die("Unknown type of information\n");
+	else {
+		rc = -1;
+		pr_err("Unknown type of information\n");
+	}
+
+	return rc;
 }
 
 static int process_sample_event(struct perf_tool *tool __used,
@@ -826,8 +884,7 @@ static int process_sample_event(struct perf_tool *tool __used,
 		return -1;
 	}
 
-	perf_evsel__process_lock_event(evsel, sample);
-	return 0;
+	return perf_evsel__process_lock_event(evsel, sample);
 }
 
 static struct perf_tool eops = {
@@ -839,8 +896,10 @@ static struct perf_tool eops = {
 static int read_events(void)
 {
 	session = perf_session__new(input_name, O_RDONLY, 0, false, &eops);
-	if (!session)
-		die("Initializing perf session failed\n");
+	if (!session) {
+		pr_err("Initializing perf session failed\n");
+		return -1;
+	}
 
 	return perf_session__process_events(session, &eops);
 }
@@ -857,13 +916,18 @@ static void sort_result(void)
 	}
 }
 
-static void __cmd_report(void)
+static int __cmd_report(void)
 {
 	setup_pager();
-	select_key();
-	read_events();
+
+	if ((select_key() != 0) ||
+	    (read_events() != 0))
+		return -1;
+
 	sort_result();
 	print_result();
+
+	return 0;
 }
 
 static const char * const report_usage[] = {
@@ -959,6 +1023,7 @@ static int __cmd_record(int argc, const char **argv)
 int cmd_lock(int argc, const char **argv, const char *prefix __used)
 {
 	unsigned int i;
+	int rc = 0;
 
 	symbol__init();
 	for (i = 0; i < LOCKHASH_SIZE; i++)
@@ -993,11 +1058,13 @@ int cmd_lock(int argc, const char **argv, const char *prefix __used)
 		/* recycling report_lock_ops */
 		trace_handler = &report_lock_ops;
 		setup_pager();
-		read_events();
-		dump_info();
+		if (read_events() != 0)
+			rc = -1;
+		else
+			rc = dump_info();
 	} else {
 		usage_with_options(lock_usage, lock_options);
 	}
 
-	return 0;
+	return rc;
 }

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [tip:perf/core] perf stat: Remove use of die/ exit and handle errors
  2012-08-26 18:24 ` [PATCH 4/7] perf stat: remove use of die/exit " David Ahern
@ 2012-09-07  5:58   ` tip-bot for David Ahern
  0 siblings, 0 replies; 15+ messages in thread
From: tip-bot for David Ahern @ 2012-09-07  5:58 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, hpa, mingo, peterz, fweisbec, dsahern, tglx

Commit-ID:  fceda7feb4a7822feee9662bc64968230d8f37bf
Gitweb:     http://git.kernel.org/tip/fceda7feb4a7822feee9662bc64968230d8f37bf
Author:     David Ahern <dsahern@gmail.com>
AuthorDate: Sun, 26 Aug 2012 12:24:44 -0600
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 5 Sep 2012 17:20:24 -0300

perf stat: Remove use of die/exit and handle errors

Allows perf to clean up properly on program termination.

Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1346005487-62961-5-git-send-email-dsahern@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-stat.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index d53d8ab..02f49eb 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -428,7 +428,7 @@ static int run_perf_stat(int argc __used, const char **argv)
 
 	if (forks && (pipe(child_ready_pipe) < 0 || pipe(go_pipe) < 0)) {
 		perror("failed to create pipes");
-		exit(1);
+		return -1;
 	}
 
 	if (forks) {
@@ -510,7 +510,8 @@ static int run_perf_stat(int argc __used, const char **argv)
 			}
 			if (child_pid != -1)
 				kill(child_pid, SIGTERM);
-			die("Not all events could be opened.\n");
+
+			pr_err("Not all events could be opened.\n");
 			return -1;
 		}
 		counter->supported = true;
@@ -1189,7 +1190,7 @@ int cmd_stat(int argc, const char **argv, const char *prefix __used)
 		output = fopen(output_name, mode);
 		if (!output) {
 			perror("failed to create output file");
-			exit(-1);
+			return -1;
 		}
 		clock_gettime(CLOCK_REALTIME, &tm);
 		fprintf(output, "# started on %s\n", ctime(&tm.tv_sec));

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [tip:perf/core] perf help: Remove use of die and handle errors
  2012-08-26 18:24 ` [PATCH 5/7] perf help: remove remove use of die " David Ahern
@ 2012-09-07  5:59   ` tip-bot for David Ahern
  0 siblings, 0 replies; 15+ messages in thread
From: tip-bot for David Ahern @ 2012-09-07  5:59 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, hpa, mingo, peterz, fweisbec, dsahern, tglx

Commit-ID:  cc58482133296f52873be909a2795f6d934ecec9
Gitweb:     http://git.kernel.org/tip/cc58482133296f52873be909a2795f6d934ecec9
Author:     David Ahern <dsahern@gmail.com>
AuthorDate: Sun, 26 Aug 2012 12:24:45 -0600
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 5 Sep 2012 17:21:10 -0300

perf help: Remove use of die and handle errors

Allows perf to clean up properly on exit.

Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1346005487-62961-6-git-send-email-dsahern@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-help.c |   48 +++++++++++++++++++++++++++++++-------------
 1 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/tools/perf/builtin-help.c b/tools/perf/builtin-help.c
index 6d5a8a7..f9daae5 100644
--- a/tools/perf/builtin-help.c
+++ b/tools/perf/builtin-help.c
@@ -24,13 +24,14 @@ static struct man_viewer_info_list {
 } *man_viewer_info_list;
 
 enum help_format {
+	HELP_FORMAT_NONE,
 	HELP_FORMAT_MAN,
 	HELP_FORMAT_INFO,
 	HELP_FORMAT_WEB,
 };
 
 static bool show_all = false;
-static enum help_format help_format = HELP_FORMAT_MAN;
+static enum help_format help_format = HELP_FORMAT_NONE;
 static struct option builtin_help_options[] = {
 	OPT_BOOLEAN('a', "all", &show_all, "print all available commands"),
 	OPT_SET_UINT('m', "man", &help_format, "show man page", HELP_FORMAT_MAN),
@@ -54,7 +55,9 @@ static enum help_format parse_help_format(const char *format)
 		return HELP_FORMAT_INFO;
 	if (!strcmp(format, "web") || !strcmp(format, "html"))
 		return HELP_FORMAT_WEB;
-	die("unrecognized help format '%s'", format);
+
+	pr_err("unrecognized help format '%s'", format);
+	return HELP_FORMAT_NONE;
 }
 
 static const char *get_man_viewer_info(const char *name)
@@ -259,6 +262,8 @@ static int perf_help_config(const char *var, const char *value, void *cb)
 		if (!value)
 			return config_error_nonbool(var);
 		help_format = parse_help_format(value);
+		if (help_format == HELP_FORMAT_NONE)
+			return -1;
 		return 0;
 	}
 	if (!strcmp(var, "man.viewer")) {
@@ -352,7 +357,7 @@ static void exec_viewer(const char *name, const char *page)
 		warning("'%s': unknown man viewer.", name);
 }
 
-static void show_man_page(const char *perf_cmd)
+static int show_man_page(const char *perf_cmd)
 {
 	struct man_viewer_list *viewer;
 	const char *page = cmd_to_page(perf_cmd);
@@ -365,28 +370,35 @@ static void show_man_page(const char *perf_cmd)
 	if (fallback)
 		exec_viewer(fallback, page);
 	exec_viewer("man", page);
-	die("no man viewer handled the request");
+
+	pr_err("no man viewer handled the request");
+	return -1;
 }
 
-static void show_info_page(const char *perf_cmd)
+static int show_info_page(const char *perf_cmd)
 {
 	const char *page = cmd_to_page(perf_cmd);
 	setenv("INFOPATH", system_path(PERF_INFO_PATH), 1);
 	execlp("info", "info", "perfman", page, NULL);
+	return -1;
 }
 
-static void get_html_page_path(struct strbuf *page_path, const char *page)
+static int get_html_page_path(struct strbuf *page_path, const char *page)
 {
 	struct stat st;
 	const char *html_path = system_path(PERF_HTML_PATH);
 
 	/* Check that we have a perf documentation directory. */
 	if (stat(mkpath("%s/perf.html", html_path), &st)
-	    || !S_ISREG(st.st_mode))
-		die("'%s': not a documentation directory.", html_path);
+	    || !S_ISREG(st.st_mode)) {
+		pr_err("'%s': not a documentation directory.", html_path);
+		return -1;
+	}
 
 	strbuf_init(page_path, 0);
 	strbuf_addf(page_path, "%s/%s.html", html_path, page);
+
+	return 0;
 }
 
 /*
@@ -401,19 +413,23 @@ static void open_html(const char *path)
 }
 #endif
 
-static void show_html_page(const char *perf_cmd)
+static int show_html_page(const char *perf_cmd)
 {
 	const char *page = cmd_to_page(perf_cmd);
 	struct strbuf page_path; /* it leaks but we exec bellow */
 
-	get_html_page_path(&page_path, page);
+	if (get_html_page_path(&page_path, page) != 0)
+		return -1;
 
 	open_html(page_path.buf);
+
+	return 0;
 }
 
 int cmd_help(int argc, const char **argv, const char *prefix __used)
 {
 	const char *alias;
+	int rc = 0;
 
 	load_command_list("perf-", &main_cmds, &other_cmds);
 
@@ -444,16 +460,20 @@ int cmd_help(int argc, const char **argv, const char *prefix __used)
 
 	switch (help_format) {
 	case HELP_FORMAT_MAN:
-		show_man_page(argv[0]);
+		rc = show_man_page(argv[0]);
 		break;
 	case HELP_FORMAT_INFO:
-		show_info_page(argv[0]);
+		rc = show_info_page(argv[0]);
 		break;
 	case HELP_FORMAT_WEB:
-		show_html_page(argv[0]);
+		rc = show_html_page(argv[0]);
+		break;
+	case HELP_FORMAT_NONE:
+		/* fall-through */
 	default:
+		rc = -1;
 		break;
 	}
 
-	return 0;
+	return rc;
 }

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [tip:perf/core] perf script: Remove use of die/exit
  2012-08-26 18:24 ` [PATCH 6/7] perf script: remove use of die/exit David Ahern
@ 2012-09-07  6:00   ` tip-bot for David Ahern
  0 siblings, 0 replies; 15+ messages in thread
From: tip-bot for David Ahern @ 2012-09-07  6:00 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, hpa, mingo, peterz, fweisbec, dsahern, tglx

Commit-ID:  d54b1a9e0eaca92cde678d19bd82b9594ed00450
Gitweb:     http://git.kernel.org/tip/d54b1a9e0eaca92cde678d19bd82b9594ed00450
Author:     David Ahern <dsahern@gmail.com>
AuthorDate: Sun, 26 Aug 2012 12:24:46 -0600
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 5 Sep 2012 17:21:39 -0300

perf script: Remove use of die/exit

Allows perf to clean up properly on exit. Only exits left are exec
failures which are appropriate and usage callbacks that list available
options.

Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1346005487-62961-7-git-send-email-dsahern@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-script.c |   60 +++++++++++++++++++++++++++++-------------
 1 files changed, 41 insertions(+), 19 deletions(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 2d6e3b2..c350cfe 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1153,18 +1153,23 @@ static const struct option options[] = {
 	OPT_END()
 };
 
-static bool have_cmd(int argc, const char **argv)
+static int have_cmd(int argc, const char **argv)
 {
 	char **__argv = malloc(sizeof(const char *) * argc);
 
-	if (!__argv)
-		die("malloc");
+	if (!__argv) {
+		pr_err("malloc failed\n");
+		return -1;
+	}
+
 	memcpy(__argv, argv, sizeof(const char *) * argc);
 	argc = parse_options(argc, (const char **)__argv, record_options,
 			     NULL, PARSE_OPT_STOP_AT_NON_OPTION);
 	free(__argv);
 
-	return argc != 0;
+	system_wide = (argc == 0);
+
+	return 0;
 }
 
 int cmd_script(int argc, const char **argv, const char *prefix __used)
@@ -1231,13 +1236,13 @@ int cmd_script(int argc, const char **argv, const char *prefix __used)
 
 		if (pipe(live_pipe) < 0) {
 			perror("failed to create pipe");
-			exit(-1);
+			return -1;
 		}
 
 		pid = fork();
 		if (pid < 0) {
 			perror("failed to fork");
-			exit(-1);
+			return -1;
 		}
 
 		if (!pid) {
@@ -1249,13 +1254,18 @@ int cmd_script(int argc, const char **argv, const char *prefix __used)
 			if (is_top_script(argv[0])) {
 				system_wide = true;
 			} else if (!system_wide) {
-				system_wide = !have_cmd(argc - rep_args,
-							&argv[rep_args]);
+				if (have_cmd(argc - rep_args, &argv[rep_args]) != 0) {
+					err = -1;
+					goto out;
+				}
 			}
 
 			__argv = malloc((argc + 6) * sizeof(const char *));
-			if (!__argv)
-				die("malloc");
+			if (!__argv) {
+				pr_err("malloc failed\n");
+				err = -ENOMEM;
+				goto out;
+			}
 
 			__argv[j++] = "/bin/sh";
 			__argv[j++] = rec_script_path;
@@ -1277,8 +1287,12 @@ int cmd_script(int argc, const char **argv, const char *prefix __used)
 		close(live_pipe[1]);
 
 		__argv = malloc((argc + 4) * sizeof(const char *));
-		if (!__argv)
-			die("malloc");
+		if (!__argv) {
+			pr_err("malloc failed\n");
+			err = -ENOMEM;
+			goto out;
+		}
+
 		j = 0;
 		__argv[j++] = "/bin/sh";
 		__argv[j++] = rep_script_path;
@@ -1303,12 +1317,20 @@ int cmd_script(int argc, const char **argv, const char *prefix __used)
 
 		if (!rec_script_path)
 			system_wide = false;
-		else if (!system_wide)
-			system_wide = !have_cmd(argc - 1, &argv[1]);
+		else if (!system_wide) {
+			if (have_cmd(argc - 1, &argv[1]) != 0) {
+				err = -1;
+				goto out;
+			}
+		}
 
 		__argv = malloc((argc + 2) * sizeof(const char *));
-		if (!__argv)
-			die("malloc");
+		if (!__argv) {
+			pr_err("malloc failed\n");
+			err = -ENOMEM;
+			goto out;
+		}
+
 		__argv[j++] = "/bin/sh";
 		__argv[j++] = script_path;
 		if (system_wide)
@@ -1357,18 +1379,18 @@ int cmd_script(int argc, const char **argv, const char *prefix __used)
 		input = open(session->filename, O_RDONLY);	/* input_name */
 		if (input < 0) {
 			perror("failed to open file");
-			exit(-1);
+			return -1;
 		}
 
 		err = fstat(input, &perf_stat);
 		if (err < 0) {
 			perror("failed to stat file");
-			exit(-1);
+			return -1;
 		}
 
 		if (!perf_stat.st_size) {
 			fprintf(stderr, "zero-sized file, nothing to do!\n");
-			exit(0);
+			return 0;
 		}
 
 		scripting_ops = script_spec__lookup(generate_script_lang);

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [tip:perf/core] perf record: Remove use of die/exit
  2012-08-26 18:24 ` [PATCH 7/7] perf record: remove " David Ahern
@ 2012-09-07  6:01   ` tip-bot for David Ahern
  0 siblings, 0 replies; 15+ messages in thread
From: tip-bot for David Ahern @ 2012-09-07  6:01 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, hpa, mingo, peterz, fweisbec, dsahern, tglx

Commit-ID:  8d3eca20b9f31cf10088e283d704f6a71b9a4ee2
Gitweb:     http://git.kernel.org/tip/8d3eca20b9f31cf10088e283d704f6a71b9a4ee2
Author:     David Ahern <dsahern@gmail.com>
AuthorDate: Sun, 26 Aug 2012 12:24:47 -0600
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 5 Sep 2012 17:22:41 -0300

perf record: Remove use of die/exit

Allows perf to clean up properly on exit. If perf-record is exiting due
to failure, the on_exit should not run as the session has been deleted.

Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1346005487-62961-8-git-send-email-dsahern@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c |  158 +++++++++++++++++++++++++++++-------------
 1 files changed, 109 insertions(+), 49 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 479ff2a..7b8b891 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -71,19 +71,23 @@ static void advance_output(struct perf_record *rec, size_t size)
 	rec->bytes_written += size;
 }
 
-static void write_output(struct perf_record *rec, void *buf, size_t size)
+static int write_output(struct perf_record *rec, void *buf, size_t size)
 {
 	while (size) {
 		int ret = write(rec->output, buf, size);
 
-		if (ret < 0)
-			die("failed to write");
+		if (ret < 0) {
+			pr_err("failed to write\n");
+			return -1;
+		}
 
 		size -= ret;
 		buf += ret;
 
 		rec->bytes_written += ret;
 	}
+
+	return 0;
 }
 
 static int process_synthesized_event(struct perf_tool *tool,
@@ -92,11 +96,13 @@ static int process_synthesized_event(struct perf_tool *tool,
 				     struct machine *machine __used)
 {
 	struct perf_record *rec = container_of(tool, struct perf_record, tool);
-	write_output(rec, event, event->header.size);
+	if (write_output(rec, event, event->header.size) < 0)
+		return -1;
+
 	return 0;
 }
 
-static void perf_record__mmap_read(struct perf_record *rec,
+static int perf_record__mmap_read(struct perf_record *rec,
 				   struct perf_mmap *md)
 {
 	unsigned int head = perf_mmap__read_head(md);
@@ -104,9 +110,10 @@ static void perf_record__mmap_read(struct perf_record *rec,
 	unsigned char *data = md->base + rec->page_size;
 	unsigned long size;
 	void *buf;
+	int rc = 0;
 
 	if (old == head)
-		return;
+		return 0;
 
 	rec->samples++;
 
@@ -117,17 +124,26 @@ static void perf_record__mmap_read(struct perf_record *rec,
 		size = md->mask + 1 - (old & md->mask);
 		old += size;
 
-		write_output(rec, buf, size);
+		if (write_output(rec, buf, size) < 0) {
+			rc = -1;
+			goto out;
+		}
 	}
 
 	buf = &data[old & md->mask];
 	size = head - old;
 	old += size;
 
-	write_output(rec, buf, size);
+	if (write_output(rec, buf, size) < 0) {
+		rc = -1;
+		goto out;
+	}
 
 	md->prev = old;
 	perf_mmap__write_tail(md, old);
+
+out:
+	return rc;
 }
 
 static volatile int done = 0;
@@ -183,12 +199,13 @@ static bool perf_evlist__equal(struct perf_evlist *evlist,
 	return true;
 }
 
-static void perf_record__open(struct perf_record *rec)
+static int perf_record__open(struct perf_record *rec)
 {
 	struct perf_evsel *pos;
 	struct perf_evlist *evlist = rec->evlist;
 	struct perf_session *session = rec->session;
 	struct perf_record_opts *opts = &rec->opts;
+	int rc = 0;
 
 	perf_evlist__config_attrs(evlist, opts);
 
@@ -222,10 +239,13 @@ try_again:
 
 			if (err == EPERM || err == EACCES) {
 				ui__error_paranoid();
-				exit(EXIT_FAILURE);
+				rc = -err;
+				goto out;
 			} else if (err ==  ENODEV && opts->target.cpu_list) {
-				die("No such device - did you specify"
-					" an out-of-range profile CPU?\n");
+				pr_err("No such device - did you specify"
+				       " an out-of-range profile CPU?\n");
+				rc = -err;
+				goto out;
 			} else if (err == EINVAL) {
 				if (!opts->exclude_guest_missing &&
 				    (attr->exclude_guest || attr->exclude_host)) {
@@ -272,7 +292,8 @@ try_again:
 			if (err == ENOENT) {
 				ui__error("The %s event is not supported.\n",
 					  perf_evsel__name(pos));
-				exit(EXIT_FAILURE);
+				rc = -err;
+				goto out;
 			}
 
 			printf("\n");
@@ -280,34 +301,46 @@ try_again:
 			      err, strerror(err));
 
 #if defined(__i386__) || defined(__x86_64__)
-			if (attr->type == PERF_TYPE_HARDWARE && err == EOPNOTSUPP)
-				die("No hardware sampling interrupt available."
-				    " No APIC? If so then you can boot the kernel"
-				    " with the \"lapic\" boot parameter to"
-				    " force-enable it.\n");
+			if (attr->type == PERF_TYPE_HARDWARE &&
+			    err == EOPNOTSUPP) {
+				pr_err("No hardware sampling interrupt available."
+				       " No APIC? If so then you can boot the kernel"
+				       " with the \"lapic\" boot parameter to"
+				       " force-enable it.\n");
+				rc = -err;
+				goto out;
+			}
 #endif
 
-			die("No CONFIG_PERF_EVENTS=y kernel support configured?\n");
+			pr_err("No CONFIG_PERF_EVENTS=y kernel support configured?\n");
+			rc = -err;
+			goto out;
 		}
 	}
 
 	if (perf_evlist__set_filters(evlist)) {
 		error("failed to set filter with %d (%s)\n", errno,
 			strerror(errno));
-		exit(-1);
+		rc = -1;
+		goto out;
 	}
 
 	if (perf_evlist__mmap(evlist, opts->mmap_pages, false) < 0) {
-		if (errno == EPERM)
-			die("Permission error mapping pages.\n"
-			    "Consider increasing "
-			    "/proc/sys/kernel/perf_event_mlock_kb,\n"
-			    "or try again with a smaller value of -m/--mmap_pages.\n"
-			    "(current value: %d)\n", opts->mmap_pages);
-		else if (!is_power_of_2(opts->mmap_pages))
-			die("--mmap_pages/-m value must be a power of two.");
-
-		die("failed to mmap with %d (%s)\n", errno, strerror(errno));
+		if (errno == EPERM) {
+			pr_err("Permission error mapping pages.\n"
+			       "Consider increasing "
+			       "/proc/sys/kernel/perf_event_mlock_kb,\n"
+			       "or try again with a smaller value of -m/--mmap_pages.\n"
+			       "(current value: %d)\n", opts->mmap_pages);
+			rc = -errno;
+		} else if (!is_power_of_2(opts->mmap_pages)) {
+			pr_err("--mmap_pages/-m value must be a power of two.");
+			rc = -EINVAL;
+		} else {
+			pr_err("failed to mmap with %d (%s)\n", errno, strerror(errno));
+			rc = -errno;
+		}
+		goto out;
 	}
 
 	if (rec->file_new)
@@ -315,11 +348,14 @@ try_again:
 	else {
 		if (!perf_evlist__equal(session->evlist, evlist)) {
 			fprintf(stderr, "incompatible append\n");
-			exit(-1);
+			rc = -1;
+			goto out;
 		}
  	}
 
 	perf_session__set_id_hdr_size(session);
+out:
+	return rc;
 }
 
 static int process_buildids(struct perf_record *rec)
@@ -335,10 +371,13 @@ static int process_buildids(struct perf_record *rec)
 					      size, &build_id__mark_dso_hit_ops);
 }
 
-static void perf_record__exit(int status __used, void *arg)
+static void perf_record__exit(int status, void *arg)
 {
 	struct perf_record *rec = arg;
 
+	if (status != 0)
+		return;
+
 	if (!rec->opts.pipe_output) {
 		rec->session->header.data_size += rec->bytes_written;
 
@@ -393,17 +432,26 @@ static struct perf_event_header finished_round_event = {
 	.type = PERF_RECORD_FINISHED_ROUND,
 };
 
-static void perf_record__mmap_read_all(struct perf_record *rec)
+static int perf_record__mmap_read_all(struct perf_record *rec)
 {
 	int i;
+	int rc = 0;
 
 	for (i = 0; i < rec->evlist->nr_mmaps; i++) {
-		if (rec->evlist->mmap[i].base)
-			perf_record__mmap_read(rec, &rec->evlist->mmap[i]);
+		if (rec->evlist->mmap[i].base) {
+			if (perf_record__mmap_read(rec, &rec->evlist->mmap[i]) != 0) {
+				rc = -1;
+				goto out;
+			}
+		}
 	}
 
 	if (perf_header__has_feat(&rec->session->header, HEADER_TRACING_DATA))
-		write_output(rec, &finished_round_event, sizeof(finished_round_event));
+		rc = write_output(rec, &finished_round_event,
+				  sizeof(finished_round_event));
+
+out:
+	return rc;
 }
 
 static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
@@ -463,7 +511,7 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
 		output = open(output_name, flags, S_IRUSR | S_IWUSR);
 	if (output < 0) {
 		perror("failed to create output file");
-		exit(-1);
+		return -1;
 	}
 
 	rec->output = output;
@@ -503,7 +551,10 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
 		}
 	}
 
-	perf_record__open(rec);
+	if (perf_record__open(rec) != 0) {
+		err = -1;
+		goto out_delete_session;
+	}
 
 	/*
 	 * perf_session__delete(session) will be called at perf_record__exit()
@@ -513,19 +564,20 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
 	if (opts->pipe_output) {
 		err = perf_header__write_pipe(output);
 		if (err < 0)
-			return err;
+			goto out_delete_session;
 	} else if (rec->file_new) {
 		err = perf_session__write_header(session, evsel_list,
 						 output, false);
 		if (err < 0)
-			return err;
+			goto out_delete_session;
 	}
 
 	if (!rec->no_buildid
 	    && !perf_header__has_feat(&session->header, HEADER_BUILD_ID)) {
 		pr_err("Couldn't generate buildids. "
 		       "Use --no-buildid to profile anyway.\n");
-		return -1;
+		err = -1;
+		goto out_delete_session;
 	}
 
 	rec->post_processing_offset = lseek(output, 0, SEEK_CUR);
@@ -533,7 +585,8 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
 	machine = perf_session__find_host_machine(session);
 	if (!machine) {
 		pr_err("Couldn't find native kernel information.\n");
-		return -1;
+		err = -1;
+		goto out_delete_session;
 	}
 
 	if (opts->pipe_output) {
@@ -541,14 +594,14 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
 						   process_synthesized_event);
 		if (err < 0) {
 			pr_err("Couldn't synthesize attrs.\n");
-			return err;
+			goto out_delete_session;
 		}
 
 		err = perf_event__synthesize_event_types(tool, process_synthesized_event,
 							 machine);
 		if (err < 0) {
 			pr_err("Couldn't synthesize event_types.\n");
-			return err;
+			goto out_delete_session;
 		}
 
 		if (have_tracepoints(&evsel_list->entries)) {
@@ -564,7 +617,7 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
 								  process_synthesized_event);
 			if (err <= 0) {
 				pr_err("Couldn't record tracing data.\n");
-				return err;
+				goto out_delete_session;
 			}
 			advance_output(rec, err);
 		}
@@ -592,20 +645,24 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
 					       perf_event__synthesize_guest_os);
 
 	if (!opts->target.system_wide)
-		perf_event__synthesize_thread_map(tool, evsel_list->threads,
+		err = perf_event__synthesize_thread_map(tool, evsel_list->threads,
 						  process_synthesized_event,
 						  machine);
 	else
-		perf_event__synthesize_threads(tool, process_synthesized_event,
+		err = perf_event__synthesize_threads(tool, process_synthesized_event,
 					       machine);
 
+	if (err != 0)
+		goto out_delete_session;
+
 	if (rec->realtime_prio) {
 		struct sched_param param;
 
 		param.sched_priority = rec->realtime_prio;
 		if (sched_setscheduler(0, SCHED_FIFO, &param)) {
 			pr_err("Could not set realtime priority.\n");
-			exit(-1);
+			err = -1;
+			goto out_delete_session;
 		}
 	}
 
@@ -620,7 +677,10 @@ static int __cmd_record(struct perf_record *rec, int argc, const char **argv)
 	for (;;) {
 		int hits = rec->samples;
 
-		perf_record__mmap_read_all(rec);
+		if (perf_record__mmap_read_all(rec) < 0) {
+			err = -1;
+			goto out_delete_session;
+		}
 
 		if (hits == rec->samples) {
 			if (done)

^ permalink raw reply related	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2012-09-07  6:02 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-26 18:24 [PATCH 0/7] perf: cleanups related to die/exit and error handling David Ahern
2012-08-26 18:24 ` [PATCH 1/7] perf tool: flush_sample_queue needs to handle errors from handlers David Ahern
2012-09-07  5:56   ` [tip:perf/core] perf session: " tip-bot for David Ahern
2012-08-26 18:24 ` [PATCH 2/7] perf tool: handle errors in synthesized event functions David Ahern
2012-09-07  5:57   ` [tip:perf/core] " tip-bot for David Ahern
2012-08-26 18:24 ` [PATCH 3/7] perf lock: remove use of die and handle errors David Ahern
2012-09-07  5:58   ` [tip:perf/core] perf lock: Remove " tip-bot for David Ahern
2012-08-26 18:24 ` [PATCH 4/7] perf stat: remove use of die/exit " David Ahern
2012-09-07  5:58   ` [tip:perf/core] perf stat: Remove use of die/ exit " tip-bot for David Ahern
2012-08-26 18:24 ` [PATCH 5/7] perf help: remove remove use of die " David Ahern
2012-09-07  5:59   ` [tip:perf/core] perf help: Remove " tip-bot for David Ahern
2012-08-26 18:24 ` [PATCH 6/7] perf script: remove use of die/exit David Ahern
2012-09-07  6:00   ` [tip:perf/core] perf script: Remove " tip-bot for David Ahern
2012-08-26 18:24 ` [PATCH 7/7] perf record: remove " David Ahern
2012-09-07  6:01   ` [tip:perf/core] perf record: Remove " tip-bot for David Ahern

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).