* Re: [PATCH 03/13] perf annotate: Move session handling out of __cmd_annotate()
[not found] <5413EFA5.4020706@gmail.com>
@ 2014-09-13 7:52 ` taeung
0 siblings, 0 replies; 4+ messages in thread
From: taeung @ 2014-09-13 7:52 UTC (permalink / raw)
To: linux-kernel, linux-perf-users; +Cc: Namhyung Kim
Hi,
I modified error code for the requirement as below.
Author: taeung <treeze.taeung@gmail.com>
Date: Sat Sep 13 16:22:53 2014 +0900
modified error code when perf_session__new() fail
Because perf_session__new() could fail
for more reasons than just ENOMEM,
I modified error code(ENOMEM or EINVAL)
into -1.
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 1ec429f..81c9fda 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -199,7 +199,7 @@ static int __cmd_annotate(struct perf_annotate *ann)
session = perf_session__new(&file, false, &ann->tool);
if (session == NULL)
- return -ENOMEM;
+ return -1;
machines__set_symbol_filter(&session->machines,
symbol__annotate_init);
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 9a5a035..3363dce 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -683,7 +683,7 @@ static int __cmd_diff(void)
d->session = perf_session__new(&d->file, false, &tool);
if (!d->session) {
pr_err("Failed to open %s\n", d->file.path);
- ret = -ENOMEM;
+ ret = -1;
goto out_delete;
}
diff --git a/tools/perf/builtin-evlist.c b/tools/perf/builtin-evlist.c
index 66e12f5..0f93f85 100644
--- a/tools/perf/builtin-evlist.c
+++ b/tools/perf/builtin-evlist.c
@@ -28,7 +28,7 @@ static int __cmd_evlist(const char *file_name, struct
perf_attr_details *details
session = perf_session__new(&file, 0, NULL);
if (session == NULL)
- return -ENOMEM;
+ return -1;
evlist__for_each(session->evlist, pos)
perf_evsel__fprintf(pos, details, stdout);
diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 9a02807..8dbdd13 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -359,7 +359,7 @@ static int __cmd_inject(struct perf_inject *inject)
session = perf_session__new(&file, true, &inject->tool);
if (session == NULL)
- return -ENOMEM;
+ return -1;
if (inject->build_ids) {
inject->tool.sample = perf_event__inject_buildid;
diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index bef3376..b518f4b 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -422,7 +422,7 @@ static int __cmd_kmem(void)
session = perf_session__new(&file, false, &perf_kmem);
if (session == NULL)
- return -ENOMEM;
+ return -1;
if (perf_session__create_kernel_maps(session) < 0)
goto out_delete;
diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index 43367eb..828e706 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -1060,7 +1060,7 @@ static int read_events(struct perf_kvm_stat *kvm)
.comm = perf_event__process_comm,
.ordered_samples = true,
};
- struct perf_data_file file = {
+ struct perf_data_file file = {
.path = kvm->file_name,
.mode = PERF_DATA_MODE_READ,
};
@@ -1069,7 +1069,7 @@ static int read_events(struct perf_kvm_stat *kvm)
kvm->session = perf_session__new(&file, false, &kvm->tool);
if (!kvm->session) {
pr_err("Initializing perf session failed\n");
- return -EINVAL;
+ return -1;
}
if (!perf_session__has_traces(kvm->session, "kvm record"))
@@ -1369,7 +1369,7 @@ static int kvm_events_live(struct perf_kvm_stat *kvm,
*/
kvm->session = perf_session__new(&file, false, &kvm->tool);
if (kvm->session == NULL) {
- err = -ENOMEM;
+ err = -1;
goto out;
}
kvm->session->evlist = kvm->evlist;
diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index 6148afc..49f1e30 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -862,7 +862,7 @@ static int __cmd_report(bool display_info)
session = perf_session__new(&file, false, &eops);
if (!session) {
pr_err("Initializing perf session failed\n");
- return -ENOMEM;
+ return -1;
}
if (!perf_session__has_traces(session, "lock record"))
diff --git a/tools/perf/builtin-mem.c b/tools/perf/builtin-mem.c
index 4a1a6c9..75976ce 100644
--- a/tools/perf/builtin-mem.c
+++ b/tools/perf/builtin-mem.c
@@ -124,7 +124,7 @@ static int report_raw_events(struct perf_mem *mem)
&mem->tool);
if (session == NULL)
- return -ENOMEM;
+ return -1;
if (mem->cpu_list) {
ret = perf_session__cpu_bitmap(session, mem->cpu_list,
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 21d830b..1a7abac 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -712,7 +712,7 @@ int cmd_report(int argc, const char **argv, const
char *prefix __maybe_unused)
repeat:
session = perf_session__new(&file, false, &report.tool);
if (session == NULL)
- return -ENOMEM;
+ return -1;
report.session = session;
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index f57035b..77cef60 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1725,7 +1725,7 @@ int cmd_script(int argc, const char **argv, const
char *prefix __maybe_unused)
session = perf_session__new(&file, false, &script.tool);
if (session == NULL)
- return -ENOMEM;
+ return -1;
if (header || header_only) {
perf_session__fprintf_info(session, stdout, show_full_info);
diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index 2f1a522..dc9a465 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -1605,7 +1605,7 @@ static int __cmd_timechart(struct timechart
*tchart, const char *output_name)
int ret = -EINVAL;
if (session == NULL)
- return -ENOMEM;
+ return -1;
(void)perf_header__process_sections(&session->header,
perf_data_file__fd(session->file),
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 377971d..8217b5d 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -911,7 +911,7 @@ static int __cmd_top(struct perf_top *top)
top->session = perf_session__new(NULL, false, NULL);
if (top->session == NULL)
- return -ENOMEM;
+ return -1;
machines__set_symbol_filter(&top->session->machines, symbol_filter);
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index a6c3752..9e23293 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -2220,7 +2220,7 @@ static int trace__replay(struct trace *trace)
session = perf_session__new(&file, false, &trace->tool);
if (session == NULL)
- return -ENOMEM;
+ return -1;
trace->host = &session->machines.host;
Thanks,
--
taeung
> On Tue, Aug 12, 2014 at 03:40:35PM +0900, Namhyung Kim wrote:
>
> SNIP
>
> >
> > @@ -301,6 +281,10 @@ int cmd_annotate(int argc, const char **argv,
> const char *prefix __maybe_unused)
> > .ordering_requires_timestamps = true,
> > },
> > };
> > + struct perf_data_file file = {
> > + .path = input_name,
> > + .mode = PERF_DATA_MODE_READ,
> > + };
> > const struct option options[] = {
> > OPT_STRING('i', "input", &input_name, "file",
> > "input file name"),
> > @@ -308,7 +292,7 @@ int cmd_annotate(int argc, const char **argv,
> const char *prefix __maybe_unused)
> > "only consider symbols in these dsos"),
> > OPT_STRING('s', "symbol", &annotate.sym_hist_filter, "symbol",
> > "symbol to annotate"),
> > - OPT_BOOLEAN('f', "force", &annotate.force, "don't complain, do it"),
> > + OPT_BOOLEAN('f', "force", &file.force, "don't complain, do it"),
> > OPT_INCR('v', "verbose", &verbose,
> > "be more verbose (show symbol address, etc)"),
> > OPT_BOOLEAN('D', "dump-raw-trace", &dump_trace,
> > @@ -341,6 +325,7 @@ int cmd_annotate(int argc, const char **argv,
> const char *prefix __maybe_unused)
> > "Show event group information together"),
> > OPT_END()
> > };
> > + int ret;
> >
> > argc = parse_options(argc, argv, options, annotate_usage, 0);
> >
> > @@ -353,11 +338,16 @@ int cmd_annotate(int argc, const char **argv,
> const char *prefix __maybe_unused)
> >
> > setup_browser(true);
> >
> > + annotate.session = perf_session__new(&file, false, &annotate.tool);
> > + if (annotate.session == NULL)
> > + return -ENOMEM;
>
> I know you're just moving code, but perf_session__new could
> fail for more reasons than just ENOMEM, which is the most
> unlikely case ;-) -1 is probably better option here
>
> jirka
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 03/13] perf annotate: Move session handling out of __cmd_annotate()
2014-08-13 11:48 ` Jiri Olsa
@ 2014-08-19 6:03 ` Namhyung Kim
0 siblings, 0 replies; 4+ messages in thread
From: Namhyung Kim @ 2014-08-19 6:03 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Namhyung Kim, LKML, David Ahern, Adrian Hunter,
Minchan Kim
On Wed, 13 Aug 2014 13:48:22 +0200, Jiri Olsa wrote:
> On Tue, Aug 12, 2014 at 03:40:35PM +0900, Namhyung Kim wrote:
>> + annotate.session = perf_session__new(&file, false, &annotate.tool);
>> + if (annotate.session == NULL)
>> + return -ENOMEM;
>
> I know you're just moving code, but perf_session__new could
> fail for more reasons than just ENOMEM, which is the most
> unlikely case ;-) -1 is probably better option here
Okay, will change as a separate fix later.
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 03/13] perf annotate: Move session handling out of __cmd_annotate()
2014-08-12 6:40 ` [PATCH 03/13] perf annotate: Move session handling out of __cmd_annotate() Namhyung Kim
@ 2014-08-13 11:48 ` Jiri Olsa
2014-08-19 6:03 ` Namhyung Kim
0 siblings, 1 reply; 4+ messages in thread
From: Jiri Olsa @ 2014-08-13 11:48 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Namhyung Kim, LKML, David Ahern, Adrian Hunter,
Minchan Kim
On Tue, Aug 12, 2014 at 03:40:35PM +0900, Namhyung Kim wrote:
SNIP
>
> @@ -301,6 +281,10 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused)
> .ordering_requires_timestamps = true,
> },
> };
> + struct perf_data_file file = {
> + .path = input_name,
> + .mode = PERF_DATA_MODE_READ,
> + };
> const struct option options[] = {
> OPT_STRING('i', "input", &input_name, "file",
> "input file name"),
> @@ -308,7 +292,7 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused)
> "only consider symbols in these dsos"),
> OPT_STRING('s', "symbol", &annotate.sym_hist_filter, "symbol",
> "symbol to annotate"),
> - OPT_BOOLEAN('f', "force", &annotate.force, "don't complain, do it"),
> + OPT_BOOLEAN('f', "force", &file.force, "don't complain, do it"),
> OPT_INCR('v', "verbose", &verbose,
> "be more verbose (show symbol address, etc)"),
> OPT_BOOLEAN('D', "dump-raw-trace", &dump_trace,
> @@ -341,6 +325,7 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused)
> "Show event group information together"),
> OPT_END()
> };
> + int ret;
>
> argc = parse_options(argc, argv, options, annotate_usage, 0);
>
> @@ -353,11 +338,16 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused)
>
> setup_browser(true);
>
> + annotate.session = perf_session__new(&file, false, &annotate.tool);
> + if (annotate.session == NULL)
> + return -ENOMEM;
I know you're just moving code, but perf_session__new could
fail for more reasons than just ENOMEM, which is the most
unlikely case ;-) -1 is probably better option here
jirka
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 03/13] perf annotate: Move session handling out of __cmd_annotate()
2014-08-12 6:40 [PATCHSET 00/13] perf tools: Fix vmlinux search path initialization Namhyung Kim
@ 2014-08-12 6:40 ` Namhyung Kim
2014-08-13 11:48 ` Jiri Olsa
0 siblings, 1 reply; 4+ messages in thread
From: Namhyung Kim @ 2014-08-12 6:40 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Ingo Molnar, Paul Mackerras, Namhyung Kim,
Namhyung Kim, LKML, Jiri Olsa, David Ahern, Adrian Hunter,
Minchan Kim
This is a preparation of fixing dso__load_kernel_sym(). It needs a
session info before calling symbol__init().
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-annotate.c | 75 +++++++++++++++++++++++--------------------
1 file changed, 40 insertions(+), 35 deletions(-)
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 952b5ece6740..c0464dc38057 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -36,7 +36,8 @@
struct perf_annotate {
struct perf_tool tool;
- bool force, use_tui, use_stdio, use_gtk;
+ struct perf_session *session;
+ bool use_tui, use_stdio, use_gtk;
bool full_paths;
bool print_line;
bool skip_missing;
@@ -188,18 +189,9 @@ find_next:
static int __cmd_annotate(struct perf_annotate *ann)
{
int ret;
- struct perf_session *session;
+ struct perf_session *session = ann->session;
struct perf_evsel *pos;
u64 total_nr_samples;
- struct perf_data_file file = {
- .path = input_name,
- .mode = PERF_DATA_MODE_READ,
- .force = ann->force,
- };
-
- session = perf_session__new(&file, false, &ann->tool);
- if (session == NULL)
- return -ENOMEM;
machines__set_symbol_filter(&session->machines, symbol__annotate_init);
@@ -207,22 +199,22 @@ static int __cmd_annotate(struct perf_annotate *ann)
ret = perf_session__cpu_bitmap(session, ann->cpu_list,
ann->cpu_bitmap);
if (ret)
- goto out_delete;
+ goto out;
}
if (!objdump_path) {
ret = perf_session_env__lookup_objdump(&session->header.env);
if (ret)
- goto out_delete;
+ goto out;
}
ret = perf_session__process_events(session, &ann->tool);
if (ret)
- goto out_delete;
+ goto out;
if (dump_trace) {
perf_session__fprintf_nr_events(session, stdout);
- goto out_delete;
+ goto out;
}
if (verbose > 3)
@@ -250,8 +242,8 @@ static int __cmd_annotate(struct perf_annotate *ann)
}
if (total_nr_samples == 0) {
- ui__error("The %s file has no samples!\n", file.path);
- goto out_delete;
+ ui__error("The %s file has no samples!\n", session->file->path);
+ goto out;
}
if (use_browser == 2) {
@@ -261,24 +253,12 @@ static int __cmd_annotate(struct perf_annotate *ann)
"perf_gtk__show_annotations");
if (show_annotations == NULL) {
ui__error("GTK browser not found!\n");
- goto out_delete;
+ goto out;
}
show_annotations();
}
-out_delete:
- /*
- * Speed up the exit process, for large files this can
- * take quite a while.
- *
- * XXX Enable this when using valgrind or if we ever
- * librarize this command.
- *
- * Also experiment with obstacks to see how much speed
- * up we'll get here.
- *
- * perf_session__delete(session);
- */
+out:
return ret;
}
@@ -301,6 +281,10 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused)
.ordering_requires_timestamps = true,
},
};
+ struct perf_data_file file = {
+ .path = input_name,
+ .mode = PERF_DATA_MODE_READ,
+ };
const struct option options[] = {
OPT_STRING('i', "input", &input_name, "file",
"input file name"),
@@ -308,7 +292,7 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused)
"only consider symbols in these dsos"),
OPT_STRING('s', "symbol", &annotate.sym_hist_filter, "symbol",
"symbol to annotate"),
- OPT_BOOLEAN('f', "force", &annotate.force, "don't complain, do it"),
+ OPT_BOOLEAN('f', "force", &file.force, "don't complain, do it"),
OPT_INCR('v', "verbose", &verbose,
"be more verbose (show symbol address, etc)"),
OPT_BOOLEAN('D', "dump-raw-trace", &dump_trace,
@@ -341,6 +325,7 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused)
"Show event group information together"),
OPT_END()
};
+ int ret;
argc = parse_options(argc, argv, options, annotate_usage, 0);
@@ -353,11 +338,16 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused)
setup_browser(true);
+ annotate.session = perf_session__new(&file, false, &annotate.tool);
+ if (annotate.session == NULL)
+ return -ENOMEM;
+
symbol_conf.priv_size = sizeof(struct annotation);
symbol_conf.try_vmlinux_path = true;
- if (symbol__init() < 0)
- return -1;
+ ret = symbol__init();
+ if (ret < 0)
+ goto out_delete;
if (setup_sorting() < 0)
usage_with_options(annotate_usage, options);
@@ -373,5 +363,20 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused)
annotate.sym_hist_filter = argv[0];
}
- return __cmd_annotate(&annotate);
+ ret = __cmd_annotate(&annotate);
+
+out_delete:
+ /*
+ * Speed up the exit process, for large files this can
+ * take quite a while.
+ *
+ * XXX Enable this when using valgrind or if we ever
+ * librarize this command.
+ *
+ * Also experiment with obstacks to see how much speed
+ * up we'll get here.
+ *
+ * perf_session__delete(session);
+ */
+ return ret;
}
--
2.0.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-09-13 7:52 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <5413EFA5.4020706@gmail.com>
2014-09-13 7:52 ` [PATCH 03/13] perf annotate: Move session handling out of __cmd_annotate() taeung
2014-08-12 6:40 [PATCHSET 00/13] perf tools: Fix vmlinux search path initialization Namhyung Kim
2014-08-12 6:40 ` [PATCH 03/13] perf annotate: Move session handling out of __cmd_annotate() Namhyung Kim
2014-08-13 11:48 ` Jiri Olsa
2014-08-19 6:03 ` Namhyung Kim
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.