* Re: [PATCH v5 7/8] perf hists browser: Add option for runtime switching perf data file
2012-10-30 3:56 ` [PATCH v5 7/8] perf hists browser: Add option for runtime switching perf data file Feng Tang
@ 2012-10-29 14:06 ` Arnaldo Carvalho de Melo
2012-10-30 16:01 ` Feng Tang
0 siblings, 1 reply; 19+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-10-29 14:06 UTC (permalink / raw)
To: Feng Tang
Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Andi Kleen, linux-kernel
Em Tue, Oct 30, 2012 at 11:56:08AM +0800, Feng Tang escreveu:
> + pwd_dir = opendir(pwd);
> + if (!pwd_dir)
> + return ret;
> +
> + memset(options, 0, sizeof(options));
> + memset(options, 0, sizeof(abs_path));
> +
> + while ((dent = readdir(pwd_dir))) {
> + char path[PATH_MAX];
> + u64 magic;
> + char *name = dent->d_name;
> + FILE *file;
> +
> + if (!(dent->d_type == DT_REG))
> + continue;
> +
> + snprintf(path, PATH_MAX, "%s/%s", pwd, name);
> +
> + file = fopen(path, "r");
> + if (!file)
> + continue;
> +
> + if (fread(&magic, 1, 8, file) < 8)
> + goto close_file_and_continue;
> +
> + if (is_perf_magic(magic)) {
> + options[nr_options] = strdup(name);
> + if (!options[nr_options])
> + goto close_file_and_continue;
Silently not offering a valid file? At this point I think you should
warn the user and bail out.
> +
> + abs_path[nr_options] = strdup(path);
> + if (!abs_path[nr_options]) {
> + free(options[nr_options]);
> + goto close_file_and_continue;
> + }
> +
> + nr_options++;
> + }
> +
> +close_file_and_continue:
> + fclose(file);
> + if (nr_options >= 256)
> + break;
Why is this? At the very least a warning has to be given to the user
that way too many perf.data files are present, so only the first N are
in the menu.
> + }
> + closedir(pwd_dir);
> +
> + if (nr_options) {
> + choice = ui__popup_menu(nr_options, options);
> + if (choice < nr_options && choice >= 0) {
> + tmp = strdup(abs_path[choice]);
> + if (tmp) {
> + if (is_input_name_malloced)
> + free((void *)input_name);
> + input_name = tmp;
> + is_input_name_malloced = true;
> + ret = 0;
> + }
Here you return an error, but will the user get any warning on the
caller of this function if it returns -1?
> + /* Switch to another data file */
> + else if (choice == switch_data) {
> +do_data_switch:
> + if (!switch_data_file()) {
> + key = K_SWITCH_INPUT_DATA;
> + break;
> + }
> + }
... no, so it will silently continue and the user will get confused.
> }
> out_free_stack:
> pstack__delete(fstack);
> @@ -1563,6 +1666,7 @@ browse_hists:
> "Do you really want to exit?"))
> continue;
> /* Fall thru */
> + case K_SWITCH_INPUT_DATA:
> case 'q':
> case CTRL('c'):
> goto out;
> diff --git a/tools/perf/ui/keysyms.h b/tools/perf/ui/keysyms.h
> index 809eca5..65092d5 100644
> --- a/tools/perf/ui/keysyms.h
> +++ b/tools/perf/ui/keysyms.h
> @@ -23,5 +23,6 @@
> #define K_TIMER -1
> #define K_ERROR -2
> #define K_RESIZE -3
> +#define K_SWITCH_INPUT_DATA -4
>
> #endif /* _PERF_KEYSYMS_H_ */
> --
> 1.7.9.5
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 5/8] perf ui/browser: Integrate script browser into main hists browser
2012-10-30 3:56 ` [PATCH v5 5/8] perf ui/browser: Integrate script browser into main hists browser Feng Tang
@ 2012-10-29 14:54 ` Arnaldo Carvalho de Melo
2012-10-30 16:05 ` Feng Tang
2012-10-30 12:09 ` [tip:perf/core] perf hists browser: " tip-bot for Feng Tang
1 sibling, 1 reply; 19+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-10-29 14:54 UTC (permalink / raw)
To: Feng Tang
Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Andi Kleen, linux-kernel
Em Tue, Oct 30, 2012 at 11:56:06AM +0800, Feng Tang escreveu:
> Integrate the script browser into "perf report" framework, users can
> use function key 'r' or the drop down menu to list all perf scripts
> and select one of them, just like they did for the annotation.
>
> Signed-off-by: Feng Tang <feng.tang@intel.com>
I noticed this comment on another patch:
--------------------------
This initial version only enables it for 'perf report', by checking
the "timer" parameter of perf_evsel__hists_browser() equals NULL.
--------------------------
Yeah, one can say that if a timer is provided, we can say its 'top' and
not 'report', at least as things stand now.
So please add a 'is_top(void *timer)' so that we know the intent more
clearly and use it to avoid showing the 'r' key when in 'top' mode, ok?
- Arnaldo
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v5 0/8] perf tools: Add script browser and runtime data file switch
@ 2012-10-30 3:56 Feng Tang
2012-10-30 3:56 ` [PATCH v5 1/8] perf tool: Add a global variable "const char *input_name" Feng Tang
` (7 more replies)
0 siblings, 8 replies; 19+ messages in thread
From: Feng Tang @ 2012-10-30 3:56 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Namhyung Kim, Andi Kleen, linux-kernel
Cc: Feng Tang
Hi Arnaldo and all,
This patch set make 2 changes to perf tool:
1. Add a browser for perf script, which will be integrated into the
main hists and annotation browser.
2. Add the inital support for runtime perf data file switch in the
'perf report' window.
Patch 1 Add the global variable "input_name"
patch 2-5 Introduce the script browser and integrate it to
hists/annotation browser
patch 6-8 Add the runtime data file switch for 'perf report'
The patches are on top of current perf/core branch of your git tree.
Please help to review.
Thanks to Andi/Arnaldo/Namhyung for the great suggestions/reviews.
Changelog:
Since v4:
* Rebase againt post-3.7 ACME's perf tree
Since v3:
* Fix memory leak for input_name
* Add return value check for strdup
* Fix some small bugs in scripts browser
Since v2:
* add more filter to find_scripts()
* add runtime data file switch for 'perf report'
* emphasize the script browser doesn't cover 'record'
in commit log
Since v1:
* Add filter for scripts can't be run in script browser
* Fix some bugs about buffer handling and error check
Thanks,
Feng
--------------
Feng Tang (8):
perf tool: Add a global variable "const char *input_name"
perf script: Add more filter to find_scripts()
perf ui/browser: Add a browser for perf script
perf ui/browser: Integrate script browser into annotation browser
perf ui/browser: Integrate script browser into main hists browser
perf header: Add is_perf_magic() func
perf hists browser: Add option for runtime switching perf data file
perf report: Enable the runtime switching of perf data file
tools/perf/Makefile | 4 +
tools/perf/builtin-annotate.c | 5 +-
tools/perf/builtin-buildid-list.c | 6 +-
tools/perf/builtin-evlist.c | 5 +-
tools/perf/builtin-kmem.c | 5 +-
tools/perf/builtin-lock.c | 2 -
tools/perf/builtin-report.c | 31 ++++--
tools/perf/builtin-sched.c | 5 +-
tools/perf/builtin-script.c | 83 +++++++++++++++-
tools/perf/builtin-timechart.c | 5 +-
tools/perf/perf.c | 1 +
tools/perf/perf.h | 1 +
tools/perf/ui/browsers/annotate.c | 6 ++
tools/perf/ui/browsers/hists.c | 142 ++++++++++++++++++++++++++++
tools/perf/ui/browsers/scripts.c | 189 +++++++++++++++++++++++++++++++++++++
tools/perf/ui/keysyms.h | 1 +
tools/perf/util/header.c | 10 ++
tools/perf/util/header.h | 1 +
tools/perf/util/hist.h | 7 ++
19 files changed, 475 insertions(+), 34 deletions(-)
create mode 100644 tools/perf/ui/browsers/scripts.c
--
1.7.9.5
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v5 1/8] perf tool: Add a global variable "const char *input_name"
2012-10-30 3:56 [PATCH v5 0/8] perf tools: Add script browser and runtime data file switch Feng Tang
@ 2012-10-30 3:56 ` Feng Tang
2012-10-30 12:05 ` [tip:perf/core] perf tools: Add a global variable " const " tip-bot for Feng Tang
2012-10-30 3:56 ` [PATCH v5 2/8] perf script: Add more filter to find_scripts() Feng Tang
` (6 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Feng Tang @ 2012-10-30 3:56 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Namhyung Kim, Andi Kleen, linux-kernel
Cc: Feng Tang
Currently many perf commands annotate/evlist/report/script/lock etc
all support "-i" option to chose a specific perf data, and all
of them create a local "input_name" to save the file name for
that perf data.
Since most of these commands need it, we can add a global variable
for it, also it can some other benefits:
1. When calling script browser inside hists/annotation browser, it
needs to know the perf data file name to run that script.
2. For further feature like runtime switching to another perf data
file, this variable can also help.
Signed-off-by: Feng Tang <feng.tang@intel.com>
---
tools/perf/builtin-annotate.c | 5 ++---
tools/perf/builtin-buildid-list.c | 6 ++----
tools/perf/builtin-evlist.c | 5 ++---
tools/perf/builtin-kmem.c | 5 ++---
tools/perf/builtin-lock.c | 2 --
tools/perf/builtin-report.c | 13 ++++++-------
tools/perf/builtin-sched.c | 5 ++---
tools/perf/builtin-script.c | 1 -
tools/perf/builtin-timechart.c | 5 ++---
tools/perf/perf.c | 1 +
tools/perf/perf.h | 1 +
11 files changed, 20 insertions(+), 29 deletions(-)
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index c4bb645..cb23476 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -34,7 +34,6 @@
struct perf_annotate {
struct perf_tool tool;
- char const *input_name;
bool force, use_tui, use_stdio;
bool full_paths;
bool print_line;
@@ -175,7 +174,7 @@ static int __cmd_annotate(struct perf_annotate *ann)
struct perf_evsel *pos;
u64 total_nr_samples;
- session = perf_session__new(ann->input_name, O_RDONLY,
+ session = perf_session__new(input_name, O_RDONLY,
ann->force, false, &ann->tool);
if (session == NULL)
return -ENOMEM;
@@ -260,7 +259,7 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused)
},
};
const struct option options[] = {
- OPT_STRING('i', "input", &annotate.input_name, "file",
+ OPT_STRING('i', "input", &input_name, "file",
"input file name"),
OPT_STRING('d', "dsos", &symbol_conf.dso_list_str, "dso[,dso...]",
"only consider symbols in these dsos"),
diff --git a/tools/perf/builtin-buildid-list.c b/tools/perf/builtin-buildid-list.c
index a0e94ff..a82d99f 100644
--- a/tools/perf/builtin-buildid-list.c
+++ b/tools/perf/builtin-buildid-list.c
@@ -44,8 +44,7 @@ static int filename__fprintf_build_id(const char *name, FILE *fp)
return fprintf(fp, "%s\n", sbuild_id);
}
-static int perf_session__list_build_ids(const char *input_name,
- bool force, bool with_hits)
+static int perf_session__list_build_ids(bool force, bool with_hits)
{
struct perf_session *session;
@@ -81,7 +80,6 @@ int cmd_buildid_list(int argc, const char **argv,
bool show_kernel = false;
bool with_hits = false;
bool force = false;
- const char *input_name = NULL;
const struct option options[] = {
OPT_BOOLEAN('H', "with-hits", &with_hits, "Show only DSOs with hits"),
OPT_STRING('i', "input", &input_name, "file", "input file name"),
@@ -101,5 +99,5 @@ int cmd_buildid_list(int argc, const char **argv,
if (show_kernel)
return sysfs__fprintf_build_id(stdout);
- return perf_session__list_build_ids(input_name, force, with_hits);
+ return perf_session__list_build_ids(force, with_hits);
}
diff --git a/tools/perf/builtin-evlist.c b/tools/perf/builtin-evlist.c
index 997afb8..c20f1dc 100644
--- a/tools/perf/builtin-evlist.c
+++ b/tools/perf/builtin-evlist.c
@@ -48,12 +48,12 @@ static int __if_print(bool *first, const char *field, u64 value)
#define if_print(field) __if_print(&first, #field, pos->attr.field)
-static int __cmd_evlist(const char *input_name, struct perf_attr_details *details)
+static int __cmd_evlist(const char *file_name, struct perf_attr_details *details)
{
struct perf_session *session;
struct perf_evsel *pos;
- session = perf_session__new(input_name, O_RDONLY, 0, false, NULL);
+ session = perf_session__new(file_name, O_RDONLY, 0, false, NULL);
if (session == NULL)
return -ENOMEM;
@@ -111,7 +111,6 @@ static int __cmd_evlist(const char *input_name, struct perf_attr_details *detail
int cmd_evlist(int argc, const char **argv, const char *prefix __maybe_unused)
{
struct perf_attr_details details = { .verbose = false, };
- const char *input_name = NULL;
const struct option options[] = {
OPT_STRING('i', "input", &input_name, "file", "Input file name"),
OPT_BOOLEAN('F', "freq", &details.freq, "Show the sample frequency"),
diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index 14bf82f..0b4b796 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -477,7 +477,7 @@ static void sort_result(void)
__sort_result(&root_caller_stat, &root_caller_sorted, &caller_sort);
}
-static int __cmd_kmem(const char *input_name)
+static int __cmd_kmem(void)
{
int err = -EINVAL;
struct perf_session *session;
@@ -743,7 +743,6 @@ static int __cmd_record(int argc, const char **argv)
int cmd_kmem(int argc, const char **argv, const char *prefix __maybe_unused)
{
const char * const default_sort_order = "frag,hit,bytes";
- const char *input_name = NULL;
const struct option kmem_options[] = {
OPT_STRING('i', "input", &input_name, "file", "input file name"),
OPT_CALLBACK_NOOPT(0, "caller", NULL, NULL,
@@ -779,7 +778,7 @@ int cmd_kmem(int argc, const char **argv, const char *prefix __maybe_unused)
if (list_empty(&alloc_sort))
setup_sorting(&alloc_sort, default_sort_order);
- return __cmd_kmem(input_name);
+ return __cmd_kmem();
} else
usage_with_options(kmem_usage, kmem_options);
diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index 6f5f328..4258300 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -335,8 +335,6 @@ alloc_failed:
return NULL;
}
-static const char *input_name;
-
struct trace_lock_handler {
int (*acquire_event)(struct perf_evsel *evsel,
struct perf_sample *sample);
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 90d1162..f07eae7 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -40,7 +40,6 @@
struct perf_report {
struct perf_tool tool;
struct perf_session *session;
- char const *input_name;
bool force, use_tui, use_gtk, use_stdio;
bool hide_unresolved;
bool dont_use_callchains;
@@ -571,7 +570,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
.pretty_printing_style = "normal",
};
const struct option options[] = {
- OPT_STRING('i', "input", &report.input_name, "file",
+ OPT_STRING('i', "input", &input_name, "file",
"input file name"),
OPT_INCR('v', "verbose", &verbose,
"be more verbose (show symbol address, etc)"),
@@ -657,13 +656,13 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
if (report.inverted_callchain)
callchain_param.order = ORDER_CALLER;
- if (!report.input_name || !strlen(report.input_name)) {
+ if (!input_name || !strlen(input_name)) {
if (!fstat(STDIN_FILENO, &st) && S_ISFIFO(st.st_mode))
- report.input_name = "-";
+ input_name = "-";
else
- report.input_name = "perf.data";
+ input_name = "perf.data";
}
- session = perf_session__new(report.input_name, O_RDONLY,
+ session = perf_session__new(input_name, O_RDONLY,
report.force, false, &report.tool);
if (session == NULL)
return -ENOMEM;
@@ -694,7 +693,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
}
- if (strcmp(report.input_name, "-") != 0)
+ if (strcmp(input_name, "-") != 0)
setup_browser(true);
else {
use_browser = 0;
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 30e5336..cc28b85 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -120,7 +120,6 @@ struct trace_sched_handler {
struct perf_sched {
struct perf_tool tool;
- const char *input_name;
const char *sort_order;
unsigned long nr_tasks;
struct task_desc *pid_to_task[MAX_PID];
@@ -1460,7 +1459,7 @@ static int perf_sched__read_events(struct perf_sched *sched, bool destroy,
};
struct perf_session *session;
- session = perf_session__new(sched->input_name, O_RDONLY, 0, false, &sched->tool);
+ session = perf_session__new(input_name, O_RDONLY, 0, false, &sched->tool);
if (session == NULL) {
pr_debug("No Memory for session\n");
return -1;
@@ -1708,7 +1707,7 @@ int cmd_sched(int argc, const char **argv, const char *prefix __maybe_unused)
OPT_END()
};
const struct option sched_options[] = {
- OPT_STRING('i', "input", &sched.input_name, "file",
+ OPT_STRING('i', "input", &input_name, "file",
"input file name"),
OPT_INCR('v', "verbose", &verbose,
"be more verbose (show symbol address, etc)"),
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 04ceb07..7c6e4b2 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1175,7 +1175,6 @@ static int have_cmd(int argc, const char **argv)
int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused)
{
bool show_full_info = false;
- const char *input_name = NULL;
char *rec_script_path = NULL;
char *rep_script_path = NULL;
struct perf_session *session;
diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index f251b61..ab4cf232 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -965,7 +965,7 @@ static void write_svg_file(const char *filename)
svg_close();
}
-static int __cmd_timechart(const char *input_name, const char *output_name)
+static int __cmd_timechart(const char *output_name)
{
struct perf_tool perf_timechart = {
.comm = process_comm_event,
@@ -1061,7 +1061,6 @@ parse_process(const struct option *opt __maybe_unused, const char *arg,
int cmd_timechart(int argc, const char **argv,
const char *prefix __maybe_unused)
{
- const char *input_name;
const char *output_name = "output.svg";
const struct option options[] = {
OPT_STRING('i', "input", &input_name, "file", "input file name"),
@@ -1092,5 +1091,5 @@ int cmd_timechart(int argc, const char **argv,
setup_pager();
- return __cmd_timechart(input_name, output_name);
+ return __cmd_timechart(output_name);
}
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index d480d8a..e968373 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -24,6 +24,7 @@ const char perf_more_info_string[] =
int use_browser = -1;
static int use_pager = -1;
+const char *input_name;
struct cmd_struct {
const char *cmd;
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index fd116be..3998498 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -208,6 +208,7 @@ struct branch_stack {
struct branch_entry entries[0];
};
+extern const char *input_name;
extern bool perf_host, perf_guest;
extern const char perf_version_string[];
--
1.7.9.5
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v5 2/8] perf script: Add more filter to find_scripts()
2012-10-30 3:56 [PATCH v5 0/8] perf tools: Add script browser and runtime data file switch Feng Tang
2012-10-30 3:56 ` [PATCH v5 1/8] perf tool: Add a global variable "const char *input_name" Feng Tang
@ 2012-10-30 3:56 ` Feng Tang
2012-10-30 12:06 ` [tip:perf/core] " tip-bot for Feng Tang
2012-10-30 3:56 ` [PATCH v5 3/8] perf ui/browser: Add a browser for perf script Feng Tang
` (5 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Feng Tang @ 2012-10-30 3:56 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Namhyung Kim, Andi Kleen, linux-kernel
Cc: Feng Tang
As suggested by Arnaldo, many scripts have their own usages and need
capture specific events or tracepoints, so only those scripts whose
target events match the events in current perf data file should be
listed in the script browser menu.
This patch will add the event match checking, by opening "xxx-record"
script to cherry pick out all events name and comparing them with
those inside the perf data file.
Signed-off-by: Feng Tang <feng.tang@intel.com>
---
tools/perf/builtin-script.c | 82 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 79 insertions(+), 3 deletions(-)
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 7c6e4b2..b363e7b 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1030,6 +1030,68 @@ static int list_available_scripts(const struct option *opt __maybe_unused,
}
/*
+ * Some scripts specify the required events in their "xxx-record" file,
+ * this function will check if the events in perf.data match those
+ * mentioned in the "xxx-record".
+ *
+ * Fixme: All existing "xxx-record" are all in good formats "-e event ",
+ * which is covered well now. And new parsing code should be added to
+ * cover the future complexing formats like event groups etc.
+ */
+static int check_ev_match(char *dir_name, char *scriptname,
+ struct perf_session *session)
+{
+ char filename[MAXPATHLEN], evname[128];
+ char line[BUFSIZ], *p;
+ struct perf_evsel *pos;
+ int match, len;
+ FILE *fp;
+
+ sprintf(filename, "%s/bin/%s-record", dir_name, scriptname);
+
+ fp = fopen(filename, "r");
+ if (!fp)
+ return -1;
+
+ while (fgets(line, sizeof(line), fp)) {
+ p = ltrim(line);
+ if (*p == '#')
+ continue;
+
+ while (strlen(p)) {
+ p = strstr(p, "-e");
+ if (!p)
+ break;
+
+ p += 2;
+ p = ltrim(p);
+ len = strcspn(p, " \t");
+ if (!len)
+ break;
+
+ snprintf(evname, len + 1, "%s", p);
+
+ match = 0;
+ list_for_each_entry(pos,
+ &session->evlist->entries, node) {
+ if (!strcmp(perf_evsel__name(pos), evname)) {
+ match = 1;
+ break;
+ }
+ }
+
+ if (!match) {
+ fclose(fp);
+ return -1;
+ }
+ }
+ }
+
+ fclose(fp);
+ return 0;
+}
+
+/*
* Return -1 if none is found, otherwise the actual scripts number.
*
* Currently the only user of this function is the script browser, which
@@ -1039,17 +1101,23 @@ static int list_available_scripts(const struct option *opt __maybe_unused,
int find_scripts(char **scripts_array, char **scripts_path_array)
{
struct dirent *script_next, *lang_next, script_dirent, lang_dirent;
- char scripts_path[MAXPATHLEN];
+ char scripts_path[MAXPATHLEN], lang_path[MAXPATHLEN];
DIR *scripts_dir, *lang_dir;
- char lang_path[MAXPATHLEN];
+ struct perf_session *session;
char *temp;
int i = 0;
+ session = perf_session__new(input_name, O_RDONLY, 0, false, NULL);
+ if (!session)
+ return -1;
+
snprintf(scripts_path, MAXPATHLEN, "%s/scripts", perf_exec_path());
scripts_dir = opendir(scripts_path);
- if (!scripts_dir)
+ if (!scripts_dir) {
+ perf_session__delete(session);
return -1;
+ }
for_each_lang(scripts_path, scripts_dir, lang_dirent, lang_next) {
snprintf(lang_path, MAXPATHLEN, "%s/%s", scripts_path,
@@ -1077,10 +1145,18 @@ int find_scripts(char **scripts_array, char **scripts_path_array)
snprintf(scripts_array[i],
(temp - script_dirent.d_name) + 1,
"%s", script_dirent.d_name);
+
+ if (check_ev_match(lang_path,
+ scripts_array[i], session))
+ continue;
+
i++;
}
+ closedir(lang_dir);
}
+ closedir(scripts_dir);
+ perf_session__delete(session);
return i;
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v5 3/8] perf ui/browser: Add a browser for perf script
2012-10-30 3:56 [PATCH v5 0/8] perf tools: Add script browser and runtime data file switch Feng Tang
2012-10-30 3:56 ` [PATCH v5 1/8] perf tool: Add a global variable "const char *input_name" Feng Tang
2012-10-30 3:56 ` [PATCH v5 2/8] perf script: Add more filter to find_scripts() Feng Tang
@ 2012-10-30 3:56 ` Feng Tang
2012-10-30 12:07 ` [tip:perf/core] perf scripts browser: " tip-bot for Feng Tang
2012-10-30 3:56 ` [PATCH v5 4/8] perf ui/browser: Integrate script browser into annotation browser Feng Tang
` (4 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Feng Tang @ 2012-10-30 3:56 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Namhyung Kim, Andi Kleen, linux-kernel
Cc: Feng Tang
Create a script browser, so that user can check all the available
scripts for current perf data file and run them inside the main perf
report or annotation browsers, for all perf samples or for samples
belong to one thread/symbol.
Please be noted: current script browser is only for report use, and
doesn't cover the record phase, IOW it must run against one existing
perf data file.
The work flow is, users can use function key to list all the available
scripts for current perf data file in system and chose one, which will
be executed with popen("perf script -s xxx.xx",) and all the output
lines are put into one ui browser, pressing 'q' or left arrow key will
make it return to previous browser.
Signed-off-by: Feng Tang <feng.tang@intel.com>
---
tools/perf/Makefile | 4 +
tools/perf/ui/browsers/scripts.c | 189 ++++++++++++++++++++++++++++++++++++++
tools/perf/util/hist.h | 7 ++
3 files changed, 200 insertions(+)
create mode 100644 tools/perf/ui/browsers/scripts.c
diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 629fc6a..ae56940 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -591,6 +591,7 @@ ifndef NO_NEWT
LIB_OBJS += $(OUTPUT)ui/browsers/annotate.o
LIB_OBJS += $(OUTPUT)ui/browsers/hists.o
LIB_OBJS += $(OUTPUT)ui/browsers/map.o
+ LIB_OBJS += $(OUTPUT)ui/browsers/scripts.o
LIB_OBJS += $(OUTPUT)ui/progress.o
LIB_OBJS += $(OUTPUT)ui/util.o
LIB_OBJS += $(OUTPUT)ui/tui/setup.o
@@ -907,6 +908,9 @@ $(OUTPUT)ui/browsers/hists.o: ui/browsers/hists.c $(OUTPUT)PERF-CFLAGS
$(OUTPUT)ui/browsers/map.o: ui/browsers/map.c $(OUTPUT)PERF-CFLAGS
$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DENABLE_SLFUTURE_CONST $<
+$(OUTPUT)ui/browsers/scripts.o: ui/browsers/scripts.c $(OUTPUT)PERF-CFLAGS
+ $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DENABLE_SLFUTURE_CONST $<
+
$(OUTPUT)util/rbtree.o: ../../lib/rbtree.c $(OUTPUT)PERF-CFLAGS
$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -Wno-unused-parameter -DETC_PERFCONFIG='"$(ETC_PERFCONFIG_SQ)"' $<
diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
new file mode 100644
index 0000000..cbbd44b
--- /dev/null
+++ b/tools/perf/ui/browsers/scripts.c
@@ -0,0 +1,189 @@
+#include <elf.h>
+#include <newt.h>
+#include <inttypes.h>
+#include <sys/ttydefaults.h>
+#include <string.h>
+#include "../../util/sort.h"
+#include "../../util/util.h"
+#include "../../util/hist.h"
+#include "../../util/debug.h"
+#include "../../util/symbol.h"
+#include "../browser.h"
+#include "../helpline.h"
+#include "../libslang.h"
+
+/* 2048 lines should be enough for a script output */
+#define MAX_LINES 2048
+
+/* 160 bytes for one output line */
+#define AVERAGE_LINE_LEN 160
+
+struct script_line {
+ struct list_head node;
+ char line[AVERAGE_LINE_LEN];
+};
+
+struct perf_script_browser {
+ struct ui_browser b;
+ struct list_head entries;
+ const char *script_name;
+ int nr_lines;
+};
+
+#define SCRIPT_NAMELEN 128
+#define SCRIPT_MAX_NO 64
+/*
+ * Usually the full path for a script is:
+ * /home/username/libexec/perf-core/scripts/python/xxx.py
+ * /home/username/libexec/perf-core/scripts/perl/xxx.pl
+ * So 256 should be long enough to contain the full path.
+ */
+#define SCRIPT_FULLPATH_LEN 256
+
+/*
+ * When success, will copy the full path of the selected script
+ * into the buffer pointed by script_name, and return 0.
+ * Return -1 on failure.
+ */
+static int list_scripts(char *script_name)
+{
+ char *buf, *names[SCRIPT_MAX_NO], *paths[SCRIPT_MAX_NO];
+ int i, num, choice, ret = -1;
+
+ /* Preset the script name to SCRIPT_NAMELEN */
+ buf = malloc(SCRIPT_MAX_NO * (SCRIPT_NAMELEN + SCRIPT_FULLPATH_LEN));
+ if (!buf)
+ return ret;
+
+ for (i = 0; i < SCRIPT_MAX_NO; i++) {
+ names[i] = buf + i * (SCRIPT_NAMELEN + SCRIPT_FULLPATH_LEN);
+ paths[i] = names[i] + SCRIPT_NAMELEN;
+ }
+
+ num = find_scripts(names, paths);
+ if (num > 0) {
+ choice = ui__popup_menu(num, names);
+ if (choice < num && choice >= 0) {
+ strcpy(script_name, paths[choice]);
+ ret = 0;
+ }
+ }
+
+ free(buf);
+ return ret;
+}
+
+static void script_browser__write(struct ui_browser *browser,
+ void *entry, int row)
+{
+ struct script_line *sline = list_entry(entry, struct script_line, node);
+ bool current_entry = ui_browser__is_current_entry(browser, row);
+
+ ui_browser__set_color(browser, current_entry ? HE_COLORSET_SELECTED :
+ HE_COLORSET_NORMAL);
+
+ slsmg_write_nstring(sline->line, browser->width);
+}
+
+static int script_browser__run(struct perf_script_browser *self)
+{
+ int key;
+
+ if (ui_browser__show(&self->b, self->script_name,
+ "Press <- or ESC to exit") < 0)
+ return -1;
+
+ while (1) {
+ key = ui_browser__run(&self->b, 0);
+
+ /* We can add some special key handling here if needed */
+ break;
+ }
+
+ ui_browser__hide(&self->b);
+ return key;
+}
+
+
+int script_browse(const char *script_opt)
+{
+ char cmd[SCRIPT_FULLPATH_LEN*2], script_name[SCRIPT_FULLPATH_LEN];
+ char *line = NULL;
+ size_t len = 0;
+ ssize_t retlen;
+ int ret = -1, nr_entries = 0;
+ FILE *fp;
+ void *buf;
+ struct script_line *sline;
+
+ struct perf_script_browser script = {
+ .b = {
+ .refresh = ui_browser__list_head_refresh,
+ .seek = ui_browser__list_head_seek,
+ .write = script_browser__write,
+ },
+ .script_name = script_name,
+ };
+
+ INIT_LIST_HEAD(&script.entries);
+
+ /* Save each line of the output in one struct script_line object. */
+ buf = zalloc((sizeof(*sline)) * MAX_LINES);
+ if (!buf)
+ return -1;
+ sline = buf;
+
+ memset(script_name, 0, SCRIPT_FULLPATH_LEN);
+ if (list_scripts(script_name))
+ goto exit;
+
+ sprintf(cmd, "perf script -s %s ", script_name);
+
+ if (script_opt)
+ strcat(cmd, script_opt);
+
+ if (input_name) {
+ strcat(cmd, " -i ");
+ strcat(cmd, input_name);
+ }
+
+ strcat(cmd, " 2>&1");
+
+ fp = popen(cmd, "r");
+ if (!fp)
+ goto exit;
+
+ while ((retlen = getline(&line, &len, fp)) != -1) {
+ strncpy(sline->line, line, AVERAGE_LINE_LEN);
+
+ /* If one output line is very large, just cut it short */
+ if (retlen >= AVERAGE_LINE_LEN) {
+ sline->line[AVERAGE_LINE_LEN - 1] = '\0';
+ sline->line[AVERAGE_LINE_LEN - 2] = '\n';
+ }
+ list_add_tail(&sline->node, &script.entries);
+
+ if (script.b.width < retlen)
+ script.b.width = retlen;
+
+ if (nr_entries++ >= MAX_LINES - 1)
+ break;
+ sline++;
+ }
+
+ if (script.b.width > AVERAGE_LINE_LEN)
+ script.b.width = AVERAGE_LINE_LEN;
+
+ if (line)
+ free(line);
+ pclose(fp);
+
+ script.nr_lines = nr_entries;
+ script.b.nr_entries = nr_entries;
+ script.b.entries = &script.entries;
+
+ ret = script_browser__run(&script);
+exit:
+ free(buf);
+ return ret;
+}
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index c751624..b874609 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -165,6 +165,7 @@ int hist_entry__tui_annotate(struct hist_entry *he, int evidx,
int perf_evlist__tui_browse_hists(struct perf_evlist *evlist, const char *help,
void(*timer)(void *arg), void *arg,
int refresh);
+int script_browse(const char *script_opt);
#else
static inline
int perf_evlist__tui_browse_hists(struct perf_evlist *evlist __maybe_unused,
@@ -186,6 +187,12 @@ static inline int hist_entry__tui_annotate(struct hist_entry *self
{
return 0;
}
+
+static inline int script_browse(const char *script_opt)
+{
+ return 0;
+}
+
#define K_LEFT -1
#define K_RIGHT -2
#endif
--
1.7.9.5
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v5 4/8] perf ui/browser: Integrate script browser into annotation browser
2012-10-30 3:56 [PATCH v5 0/8] perf tools: Add script browser and runtime data file switch Feng Tang
` (2 preceding siblings ...)
2012-10-30 3:56 ` [PATCH v5 3/8] perf ui/browser: Add a browser for perf script Feng Tang
@ 2012-10-30 3:56 ` Feng Tang
2012-10-30 12:08 ` [tip:perf/core] perf annotate browser: " tip-bot for Feng Tang
2012-10-30 3:56 ` [PATCH v5 5/8] perf ui/browser: Integrate script browser into main hists browser Feng Tang
` (3 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Feng Tang @ 2012-10-30 3:56 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Namhyung Kim, Andi Kleen, linux-kernel
Cc: Feng Tang
Integrate the script browser into annotation, users can press function
key 'r' to list all perf scripts and select one of them to run that
script, the output will be shown in a separate browser.
Signed-off-by: Feng Tang <feng.tang@intel.com>
---
tools/perf/ui/browsers/annotate.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 8f8cd2d..28f8aab 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -676,8 +676,14 @@ static int annotate_browser__run(struct annotate_browser *browser, int evidx,
"o Toggle disassembler output/simplified view\n"
"s Toggle source code view\n"
"/ Search string\n"
+ "r Run available scripts\n"
"? Search previous string\n");
continue;
+ case 'r':
+ {
+ script_browse(NULL);
+ continue;
+ }
case 'H':
nd = browser->curr_hot;
break;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v5 5/8] perf ui/browser: Integrate script browser into main hists browser
2012-10-30 3:56 [PATCH v5 0/8] perf tools: Add script browser and runtime data file switch Feng Tang
` (3 preceding siblings ...)
2012-10-30 3:56 ` [PATCH v5 4/8] perf ui/browser: Integrate script browser into annotation browser Feng Tang
@ 2012-10-30 3:56 ` Feng Tang
2012-10-29 14:54 ` Arnaldo Carvalho de Melo
2012-10-30 12:09 ` [tip:perf/core] perf hists browser: " tip-bot for Feng Tang
2012-10-30 3:56 ` [PATCH v5 6/8] perf header: Add is_perf_magic() func Feng Tang
` (2 subsequent siblings)
7 siblings, 2 replies; 19+ messages in thread
From: Feng Tang @ 2012-10-30 3:56 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Namhyung Kim, Andi Kleen, linux-kernel
Cc: Feng Tang
Integrate the script browser into "perf report" framework, users can
use function key 'r' or the drop down menu to list all perf scripts
and select one of them, just like they did for the annotation.
Signed-off-by: Feng Tang <feng.tang@intel.com>
---
tools/perf/ui/browsers/hists.c | 38 ++++++++++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index ef2f93c..fe62284 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1141,6 +1141,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
int nr_options = 0;
int key = -1;
char buf[64];
+ char script_opt[64];
if (browser == NULL)
return -1;
@@ -1159,6 +1160,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
int choice = 0,
annotate = -2, zoom_dso = -2, zoom_thread = -2,
annotate_f = -2, annotate_t = -2, browse_map = -2;
+ int scripts_comm = -2, scripts_symbol = -2, scripts_all = -2;
nr_options = 0;
@@ -1211,6 +1213,8 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
hist_browser__reset(browser);
}
continue;
+ case 'r':
+ goto do_scripts;
case K_F1:
case 'h':
case '?':
@@ -1229,6 +1233,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
"E Expand all callchains\n"
"d Zoom into current DSO\n"
"t Zoom into current Thread\n"
+ "r Run available scripts\n"
"P Print histograms to perf.hist.N\n"
"V Verbose (DSO names in callchains, etc)\n"
"/ Filter symbol by name");
@@ -1317,6 +1322,25 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
browser->selection->map != NULL &&
asprintf(&options[nr_options], "Browse map details") > 0)
browse_map = nr_options++;
+
+ /* perf script support */
+ if (browser->he_selection) {
+ struct symbol *sym;
+
+ if (asprintf(&options[nr_options], "Run scripts for samples of thread [%s]",
+ browser->he_selection->thread->comm) > 0)
+ scripts_comm = nr_options++;
+
+ sym = browser->he_selection->ms.sym;
+ if (sym && sym->namelen &&
+ asprintf(&options[nr_options], "Run scripts for samples of symbol [%s]",
+ sym->name) > 0)
+ scripts_symbol = nr_options++;
+ }
+
+ if (asprintf(&options[nr_options], "Run scripts for all samples") > 0)
+ scripts_all = nr_options++;
+
add_exit_option:
options[nr_options++] = (char *)"Exit";
retry_popup_menu:
@@ -1411,6 +1435,20 @@ zoom_out_thread:
hists__filter_by_thread(hists);
hist_browser__reset(browser);
}
+ /* perf scripts support */
+ else if (choice == scripts_all || choice == scripts_comm ||
+ choice == scripts_symbol) {
+do_scripts:
+ memset(script_opt, 0, 64);
+
+ if (choice == scripts_comm)
+ sprintf(script_opt, " -c %s ", browser->he_selection->thread->comm);
+
+ if (choice == scripts_symbol)
+ sprintf(script_opt, " -S %s ", browser->he_selection->ms.sym->name);
+
+ script_browse(script_opt);
+ }
}
out_free_stack:
pstack__delete(fstack);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v5 6/8] perf header: Add is_perf_magic() func
2012-10-30 3:56 [PATCH v5 0/8] perf tools: Add script browser and runtime data file switch Feng Tang
` (4 preceding siblings ...)
2012-10-30 3:56 ` [PATCH v5 5/8] perf ui/browser: Integrate script browser into main hists browser Feng Tang
@ 2012-10-30 3:56 ` Feng Tang
2012-10-30 12:10 ` [tip:perf/core] " tip-bot for Feng Tang
2012-10-30 3:56 ` [PATCH v5 7/8] perf hists browser: Add option for runtime switching perf data file Feng Tang
2012-10-30 3:56 ` [PATCH v5 8/8] perf report: Enable the runtime switching of " Feng Tang
7 siblings, 1 reply; 19+ messages in thread
From: Feng Tang @ 2012-10-30 3:56 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Namhyung Kim, Andi Kleen, linux-kernel
Cc: Feng Tang
With this function, other modules can basically check whether a
file is a legal perf data file by checking its first 8 bytes
against all possible perf magic numbers.
Change the function name from check_perf_magic to more meaningful
is_perf_magic as suggested by acme.
Signed-off-by: Feng Tang <feng.tang@intel.com>
---
| 10 ++++++++++
| 1 +
2 files changed, 11 insertions(+)
--git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 7daad23..87e679e 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2340,6 +2340,16 @@ static int try_all_pipe_abis(uint64_t hdr_sz, struct perf_header *ph)
return -1;
}
+bool is_perf_magic(u64 magic)
+{
+ if (!memcmp(&magic, __perf_magic1, sizeof(magic))
+ || magic == __perf_magic2
+ || magic == __perf_magic2_sw)
+ return true;
+
+ return false;
+}
+
static int check_magic_endian(u64 magic, uint64_t hdr_sz,
bool is_pipe, struct perf_header *ph)
{
--git a/tools/perf/util/header.h b/tools/perf/util/header.h
index 879d215..5f1cd68 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -154,6 +154,7 @@ int perf_event__synthesize_build_id(struct perf_tool *tool,
int perf_event__process_build_id(struct perf_tool *tool,
union perf_event *event,
struct perf_session *session);
+bool is_perf_magic(u64 magic);
/*
* arch specific callback
--
1.7.9.5
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v5 7/8] perf hists browser: Add option for runtime switching perf data file
2012-10-30 3:56 [PATCH v5 0/8] perf tools: Add script browser and runtime data file switch Feng Tang
` (5 preceding siblings ...)
2012-10-30 3:56 ` [PATCH v5 6/8] perf header: Add is_perf_magic() func Feng Tang
@ 2012-10-30 3:56 ` Feng Tang
2012-10-29 14:06 ` Arnaldo Carvalho de Melo
2012-10-30 3:56 ` [PATCH v5 8/8] perf report: Enable the runtime switching of " Feng Tang
7 siblings, 1 reply; 19+ messages in thread
From: Feng Tang @ 2012-10-30 3:56 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Namhyung Kim, Andi Kleen, linux-kernel
Cc: Feng Tang
Based on perf report/top/scripts browser integration idea from acme.
This will enable user to runtime switch the data file, when this option
is selected, it will popup all the legal data files in current working
directory, and the filename selected by user is saved in the global
variable "input_name", and a new key 'K_SWITCH_INPUT_DATA' will be
passed back to the built-in command which will perform the switch.
This initial version only enables it for 'perf report', by checking
the "timer" parameter of perf_evsel__hists_browser() equals NULL.
Signed-off-by: Feng Tang <feng.tang@intel.com>
---
tools/perf/ui/browsers/hists.c | 106 +++++++++++++++++++++++++++++++++++++++-
tools/perf/ui/keysyms.h | 1 +
2 files changed, 106 insertions(+), 1 deletion(-)
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index fe62284..671b0fb 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1127,6 +1127,91 @@ static inline void free_popup_options(char **options, int n)
}
}
+/*
+ * Only runtime switching of perf data file will make "input_name" point
+ * to a malloced buffer. So add "is_input_name_malloced" flag to decide
+ * whether we need to call free() for current "input_name" during the switch.
+ */
+static bool is_input_name_malloced = false;
+
+static int switch_data_file(void)
+{
+ char *pwd, *options[256], *abs_path[256], *tmp;
+ DIR *pwd_dir;
+ int nr_options = 0, choice = -1, ret = -1;
+
+ struct dirent *dent;
+
+ pwd = getenv("PWD");
+ if (!pwd)
+ return ret;
+
+ pwd_dir = opendir(pwd);
+ if (!pwd_dir)
+ return ret;
+
+ memset(options, 0, sizeof(options));
+ memset(options, 0, sizeof(abs_path));
+
+ while ((dent = readdir(pwd_dir))) {
+ char path[PATH_MAX];
+ u64 magic;
+ char *name = dent->d_name;
+ FILE *file;
+
+ if (!(dent->d_type == DT_REG))
+ continue;
+
+ snprintf(path, PATH_MAX, "%s/%s", pwd, name);
+
+ file = fopen(path, "r");
+ if (!file)
+ continue;
+
+ if (fread(&magic, 1, 8, file) < 8)
+ goto close_file_and_continue;
+
+ if (is_perf_magic(magic)) {
+ options[nr_options] = strdup(name);
+ if (!options[nr_options])
+ goto close_file_and_continue;
+
+ abs_path[nr_options] = strdup(path);
+ if (!abs_path[nr_options]) {
+ free(options[nr_options]);
+ goto close_file_and_continue;
+ }
+
+ nr_options++;
+ }
+
+close_file_and_continue:
+ fclose(file);
+ if (nr_options >= 256)
+ break;
+ }
+ closedir(pwd_dir);
+
+ if (nr_options) {
+ choice = ui__popup_menu(nr_options, options);
+ if (choice < nr_options && choice >= 0) {
+ tmp = strdup(abs_path[choice]);
+ if (tmp) {
+ if (is_input_name_malloced)
+ free((void *)input_name);
+ input_name = tmp;
+ is_input_name_malloced = true;
+ ret = 0;
+ }
+ }
+ }
+
+ free_popup_options(options, nr_options);
+ free_popup_options(abs_path, nr_options);
+ return ret;
+}
+
+
static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
const char *helpline, const char *ev_name,
bool left_exits,
@@ -1160,7 +1245,8 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
int choice = 0,
annotate = -2, zoom_dso = -2, zoom_thread = -2,
annotate_f = -2, annotate_t = -2, browse_map = -2;
- int scripts_comm = -2, scripts_symbol = -2, scripts_all = -2;
+ int scripts_comm = -2, scripts_symbol = -2,
+ scripts_all = -2, switch_data = -2;
nr_options = 0;
@@ -1215,6 +1301,11 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
continue;
case 'r':
goto do_scripts;
+ case 's':
+ /* input_name for 'perf top' is NULL */
+ if (input_name)
+ goto do_data_switch;
+ continue;
case K_F1:
case 'h':
case '?':
@@ -1234,6 +1325,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
"d Zoom into current DSO\n"
"t Zoom into current Thread\n"
"r Run available scripts\n"
+ "s Switch to another data file in PWD\n"
"P Print histograms to perf.hist.N\n"
"V Verbose (DSO names in callchains, etc)\n"
"/ Filter symbol by name");
@@ -1341,6 +1433,9 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
if (asprintf(&options[nr_options], "Run scripts for all samples") > 0)
scripts_all = nr_options++;
+ if (!timer && asprintf(&options[nr_options],
+ "Switch to another data file in PWD") > 0)
+ switch_data = nr_options++;
add_exit_option:
options[nr_options++] = (char *)"Exit";
retry_popup_menu:
@@ -1449,6 +1544,14 @@ do_scripts:
script_browse(script_opt);
}
+ /* Switch to another data file */
+ else if (choice == switch_data) {
+do_data_switch:
+ if (!switch_data_file()) {
+ key = K_SWITCH_INPUT_DATA;
+ break;
+ }
+ }
}
out_free_stack:
pstack__delete(fstack);
@@ -1563,6 +1666,7 @@ browse_hists:
"Do you really want to exit?"))
continue;
/* Fall thru */
+ case K_SWITCH_INPUT_DATA:
case 'q':
case CTRL('c'):
goto out;
diff --git a/tools/perf/ui/keysyms.h b/tools/perf/ui/keysyms.h
index 809eca5..65092d5 100644
--- a/tools/perf/ui/keysyms.h
+++ b/tools/perf/ui/keysyms.h
@@ -23,5 +23,6 @@
#define K_TIMER -1
#define K_ERROR -2
#define K_RESIZE -3
+#define K_SWITCH_INPUT_DATA -4
#endif /* _PERF_KEYSYMS_H_ */
--
1.7.9.5
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v5 8/8] perf report: Enable the runtime switching of perf data file
2012-10-30 3:56 [PATCH v5 0/8] perf tools: Add script browser and runtime data file switch Feng Tang
` (6 preceding siblings ...)
2012-10-30 3:56 ` [PATCH v5 7/8] perf hists browser: Add option for runtime switching perf data file Feng Tang
@ 2012-10-30 3:56 ` Feng Tang
7 siblings, 0 replies; 19+ messages in thread
From: Feng Tang @ 2012-10-30 3:56 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Namhyung Kim, Andi Kleen, linux-kernel
Cc: Feng Tang
This is for tui browser only. This patch will check the returned
key of tui hists browser, if it's K_SWITH_INPUT_DATA, then recreate
a session for the new selected data file.
Signed-off-by: Feng Tang <feng.tang@intel.com>
---
tools/perf/builtin-report.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index f07eae7..3665a9a 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -427,8 +427,14 @@ static int __cmd_report(struct perf_report *rep)
if (use_browser > 0) {
if (use_browser == 1) {
- perf_evlist__tui_browse_hists(session->evlist, help,
- NULL, NULL, 0);
+ ret = perf_evlist__tui_browse_hists(session->evlist,
+ help, NULL, NULL, 0);
+ /*
+ * Usually "ret" is the last pressed key, and we only
+ * care if the key notifies us to switch data file.
+ */
+ if (ret != K_SWITCH_INPUT_DATA)
+ ret = 0;
} else if (use_browser == 2) {
perf_evlist__gtk_browse_hists(session->evlist, help,
NULL, NULL, 0);
@@ -662,6 +668,8 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
else
input_name = "perf.data";
}
+
+repeat:
session = perf_session__new(input_name, O_RDONLY,
report.force, false, &report.tool);
if (session == NULL)
@@ -768,6 +776,12 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
}
ret = __cmd_report(&report);
+ if (ret == K_SWITCH_INPUT_DATA) {
+ perf_session__delete(session);
+ goto repeat;
+ } else
+ ret = 0;
+
error:
perf_session__delete(session);
return ret;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [tip:perf/core] perf tools: Add a global variable " const char *input_name"
2012-10-30 3:56 ` [PATCH v5 1/8] perf tool: Add a global variable "const char *input_name" Feng Tang
@ 2012-10-30 12:05 ` tip-bot for Feng Tang
0 siblings, 0 replies; 19+ messages in thread
From: tip-bot for Feng Tang @ 2012-10-30 12:05 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, hpa, mingo, andi, peterz, namhyung, tglx,
feng.tang, mingo
Commit-ID: 70cb4e963f77dae90ae2aa3dd9385a43737c469f
Gitweb: http://git.kernel.org/tip/70cb4e963f77dae90ae2aa3dd9385a43737c469f
Author: Feng Tang <feng.tang@intel.com>
AuthorDate: Tue, 30 Oct 2012 11:56:02 +0800
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 29 Oct 2012 11:45:34 -0200
perf tools: Add a global variable "const char *input_name"
Currently many perf commands annotate/evlist/report/script/lock etc all
support "-i" option to chose a specific perf data, and all of them
create a local "input_name" to save the file name for that perf data.
Since most of these commands need it, we can add a global variable for
it, also it can some other benefits:
1. When calling script browser inside hists/annotation browser, it needs
to know the perf data file name to run that script.
2. For further feature like runtime switching to another perf data file,
this variable can also help.
Signed-off-by: Feng Tang <feng.tang@intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1351569369-26732-2-git-send-email-feng.tang@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-annotate.c | 5 ++---
tools/perf/builtin-buildid-list.c | 6 ++----
tools/perf/builtin-evlist.c | 5 ++---
tools/perf/builtin-kmem.c | 5 ++---
tools/perf/builtin-lock.c | 2 --
tools/perf/builtin-report.c | 13 ++++++-------
tools/perf/builtin-sched.c | 5 ++---
tools/perf/builtin-script.c | 1 -
tools/perf/builtin-timechart.c | 5 ++---
tools/perf/perf.c | 1 +
tools/perf/perf.h | 1 +
11 files changed, 20 insertions(+), 29 deletions(-)
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index c4bb645..cb23476 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -34,7 +34,6 @@
struct perf_annotate {
struct perf_tool tool;
- char const *input_name;
bool force, use_tui, use_stdio;
bool full_paths;
bool print_line;
@@ -175,7 +174,7 @@ static int __cmd_annotate(struct perf_annotate *ann)
struct perf_evsel *pos;
u64 total_nr_samples;
- session = perf_session__new(ann->input_name, O_RDONLY,
+ session = perf_session__new(input_name, O_RDONLY,
ann->force, false, &ann->tool);
if (session == NULL)
return -ENOMEM;
@@ -260,7 +259,7 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused)
},
};
const struct option options[] = {
- OPT_STRING('i', "input", &annotate.input_name, "file",
+ OPT_STRING('i', "input", &input_name, "file",
"input file name"),
OPT_STRING('d', "dsos", &symbol_conf.dso_list_str, "dso[,dso...]",
"only consider symbols in these dsos"),
diff --git a/tools/perf/builtin-buildid-list.c b/tools/perf/builtin-buildid-list.c
index a0e94ff..a82d99f 100644
--- a/tools/perf/builtin-buildid-list.c
+++ b/tools/perf/builtin-buildid-list.c
@@ -44,8 +44,7 @@ static int filename__fprintf_build_id(const char *name, FILE *fp)
return fprintf(fp, "%s\n", sbuild_id);
}
-static int perf_session__list_build_ids(const char *input_name,
- bool force, bool with_hits)
+static int perf_session__list_build_ids(bool force, bool with_hits)
{
struct perf_session *session;
@@ -81,7 +80,6 @@ int cmd_buildid_list(int argc, const char **argv,
bool show_kernel = false;
bool with_hits = false;
bool force = false;
- const char *input_name = NULL;
const struct option options[] = {
OPT_BOOLEAN('H', "with-hits", &with_hits, "Show only DSOs with hits"),
OPT_STRING('i', "input", &input_name, "file", "input file name"),
@@ -101,5 +99,5 @@ int cmd_buildid_list(int argc, const char **argv,
if (show_kernel)
return sysfs__fprintf_build_id(stdout);
- return perf_session__list_build_ids(input_name, force, with_hits);
+ return perf_session__list_build_ids(force, with_hits);
}
diff --git a/tools/perf/builtin-evlist.c b/tools/perf/builtin-evlist.c
index 997afb8..c20f1dc 100644
--- a/tools/perf/builtin-evlist.c
+++ b/tools/perf/builtin-evlist.c
@@ -48,12 +48,12 @@ static int __if_print(bool *first, const char *field, u64 value)
#define if_print(field) __if_print(&first, #field, pos->attr.field)
-static int __cmd_evlist(const char *input_name, struct perf_attr_details *details)
+static int __cmd_evlist(const char *file_name, struct perf_attr_details *details)
{
struct perf_session *session;
struct perf_evsel *pos;
- session = perf_session__new(input_name, O_RDONLY, 0, false, NULL);
+ session = perf_session__new(file_name, O_RDONLY, 0, false, NULL);
if (session == NULL)
return -ENOMEM;
@@ -111,7 +111,6 @@ static int __cmd_evlist(const char *input_name, struct perf_attr_details *detail
int cmd_evlist(int argc, const char **argv, const char *prefix __maybe_unused)
{
struct perf_attr_details details = { .verbose = false, };
- const char *input_name = NULL;
const struct option options[] = {
OPT_STRING('i', "input", &input_name, "file", "Input file name"),
OPT_BOOLEAN('F', "freq", &details.freq, "Show the sample frequency"),
diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index 14bf82f..0b4b796 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -477,7 +477,7 @@ static void sort_result(void)
__sort_result(&root_caller_stat, &root_caller_sorted, &caller_sort);
}
-static int __cmd_kmem(const char *input_name)
+static int __cmd_kmem(void)
{
int err = -EINVAL;
struct perf_session *session;
@@ -743,7 +743,6 @@ static int __cmd_record(int argc, const char **argv)
int cmd_kmem(int argc, const char **argv, const char *prefix __maybe_unused)
{
const char * const default_sort_order = "frag,hit,bytes";
- const char *input_name = NULL;
const struct option kmem_options[] = {
OPT_STRING('i', "input", &input_name, "file", "input file name"),
OPT_CALLBACK_NOOPT(0, "caller", NULL, NULL,
@@ -779,7 +778,7 @@ int cmd_kmem(int argc, const char **argv, const char *prefix __maybe_unused)
if (list_empty(&alloc_sort))
setup_sorting(&alloc_sort, default_sort_order);
- return __cmd_kmem(input_name);
+ return __cmd_kmem();
} else
usage_with_options(kmem_usage, kmem_options);
diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index 6f5f328..4258300 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -335,8 +335,6 @@ alloc_failed:
return NULL;
}
-static const char *input_name;
-
struct trace_lock_handler {
int (*acquire_event)(struct perf_evsel *evsel,
struct perf_sample *sample);
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 90d1162..f07eae7 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -40,7 +40,6 @@
struct perf_report {
struct perf_tool tool;
struct perf_session *session;
- char const *input_name;
bool force, use_tui, use_gtk, use_stdio;
bool hide_unresolved;
bool dont_use_callchains;
@@ -571,7 +570,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
.pretty_printing_style = "normal",
};
const struct option options[] = {
- OPT_STRING('i', "input", &report.input_name, "file",
+ OPT_STRING('i', "input", &input_name, "file",
"input file name"),
OPT_INCR('v', "verbose", &verbose,
"be more verbose (show symbol address, etc)"),
@@ -657,13 +656,13 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
if (report.inverted_callchain)
callchain_param.order = ORDER_CALLER;
- if (!report.input_name || !strlen(report.input_name)) {
+ if (!input_name || !strlen(input_name)) {
if (!fstat(STDIN_FILENO, &st) && S_ISFIFO(st.st_mode))
- report.input_name = "-";
+ input_name = "-";
else
- report.input_name = "perf.data";
+ input_name = "perf.data";
}
- session = perf_session__new(report.input_name, O_RDONLY,
+ session = perf_session__new(input_name, O_RDONLY,
report.force, false, &report.tool);
if (session == NULL)
return -ENOMEM;
@@ -694,7 +693,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
}
- if (strcmp(report.input_name, "-") != 0)
+ if (strcmp(input_name, "-") != 0)
setup_browser(true);
else {
use_browser = 0;
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 30e5336..cc28b85 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -120,7 +120,6 @@ struct trace_sched_handler {
struct perf_sched {
struct perf_tool tool;
- const char *input_name;
const char *sort_order;
unsigned long nr_tasks;
struct task_desc *pid_to_task[MAX_PID];
@@ -1460,7 +1459,7 @@ static int perf_sched__read_events(struct perf_sched *sched, bool destroy,
};
struct perf_session *session;
- session = perf_session__new(sched->input_name, O_RDONLY, 0, false, &sched->tool);
+ session = perf_session__new(input_name, O_RDONLY, 0, false, &sched->tool);
if (session == NULL) {
pr_debug("No Memory for session\n");
return -1;
@@ -1708,7 +1707,7 @@ int cmd_sched(int argc, const char **argv, const char *prefix __maybe_unused)
OPT_END()
};
const struct option sched_options[] = {
- OPT_STRING('i', "input", &sched.input_name, "file",
+ OPT_STRING('i', "input", &input_name, "file",
"input file name"),
OPT_INCR('v', "verbose", &verbose,
"be more verbose (show symbol address, etc)"),
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 04ceb07..7c6e4b2 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1175,7 +1175,6 @@ static int have_cmd(int argc, const char **argv)
int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused)
{
bool show_full_info = false;
- const char *input_name = NULL;
char *rec_script_path = NULL;
char *rep_script_path = NULL;
struct perf_session *session;
diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index f251b61..ab4cf232 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -965,7 +965,7 @@ static void write_svg_file(const char *filename)
svg_close();
}
-static int __cmd_timechart(const char *input_name, const char *output_name)
+static int __cmd_timechart(const char *output_name)
{
struct perf_tool perf_timechart = {
.comm = process_comm_event,
@@ -1061,7 +1061,6 @@ parse_process(const struct option *opt __maybe_unused, const char *arg,
int cmd_timechart(int argc, const char **argv,
const char *prefix __maybe_unused)
{
- const char *input_name;
const char *output_name = "output.svg";
const struct option options[] = {
OPT_STRING('i', "input", &input_name, "file", "input file name"),
@@ -1092,5 +1091,5 @@ int cmd_timechart(int argc, const char **argv,
setup_pager();
- return __cmd_timechart(input_name, output_name);
+ return __cmd_timechart(output_name);
}
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index d480d8a..e968373 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -24,6 +24,7 @@ const char perf_more_info_string[] =
int use_browser = -1;
static int use_pager = -1;
+const char *input_name;
struct cmd_struct {
const char *cmd;
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index c50985e..469fbf2 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -208,6 +208,7 @@ struct branch_stack {
struct branch_entry entries[0];
};
+extern const char *input_name;
extern bool perf_host, perf_guest;
extern const char perf_version_string[];
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [tip:perf/core] perf script: Add more filter to find_scripts()
2012-10-30 3:56 ` [PATCH v5 2/8] perf script: Add more filter to find_scripts() Feng Tang
@ 2012-10-30 12:06 ` tip-bot for Feng Tang
0 siblings, 0 replies; 19+ messages in thread
From: tip-bot for Feng Tang @ 2012-10-30 12:06 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, hpa, mingo, andi, peterz, namhyung, tglx,
feng.tang, mingo
Commit-ID: 49e639e256ea18fb92f609dd6be09883cd9d05aa
Gitweb: http://git.kernel.org/tip/49e639e256ea18fb92f609dd6be09883cd9d05aa
Author: Feng Tang <feng.tang@intel.com>
AuthorDate: Tue, 30 Oct 2012 11:56:03 +0800
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 29 Oct 2012 11:46:23 -0200
perf script: Add more filter to find_scripts()
As suggested by Arnaldo, many scripts have their own usages and need
capture specific events or tracepoints, so only those scripts whose
target events match the events in current perf data file should be
listed in the script browser menu.
This patch will add the event match checking, by opening "xxx-record"
script to cherry pick out all events name and comparing them with
those inside the perf data file.
Signed-off-by: Feng Tang <feng.tang@intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1351569369-26732-3-git-send-email-feng.tang@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-script.c | 82 +++++++++++++++++++++++++++++++++++++++++--
1 files changed, 79 insertions(+), 3 deletions(-)
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 7c6e4b2..b363e7b 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1030,6 +1030,68 @@ static int list_available_scripts(const struct option *opt __maybe_unused,
}
/*
+ * Some scripts specify the required events in their "xxx-record" file,
+ * this function will check if the events in perf.data match those
+ * mentioned in the "xxx-record".
+ *
+ * Fixme: All existing "xxx-record" are all in good formats "-e event ",
+ * which is covered well now. And new parsing code should be added to
+ * cover the future complexing formats like event groups etc.
+ */
+static int check_ev_match(char *dir_name, char *scriptname,
+ struct perf_session *session)
+{
+ char filename[MAXPATHLEN], evname[128];
+ char line[BUFSIZ], *p;
+ struct perf_evsel *pos;
+ int match, len;
+ FILE *fp;
+
+ sprintf(filename, "%s/bin/%s-record", dir_name, scriptname);
+
+ fp = fopen(filename, "r");
+ if (!fp)
+ return -1;
+
+ while (fgets(line, sizeof(line), fp)) {
+ p = ltrim(line);
+ if (*p == '#')
+ continue;
+
+ while (strlen(p)) {
+ p = strstr(p, "-e");
+ if (!p)
+ break;
+
+ p += 2;
+ p = ltrim(p);
+ len = strcspn(p, " \t");
+ if (!len)
+ break;
+
+ snprintf(evname, len + 1, "%s", p);
+
+ match = 0;
+ list_for_each_entry(pos,
+ &session->evlist->entries, node) {
+ if (!strcmp(perf_evsel__name(pos), evname)) {
+ match = 1;
+ break;
+ }
+ }
+
+ if (!match) {
+ fclose(fp);
+ return -1;
+ }
+ }
+ }
+
+ fclose(fp);
+ return 0;
+}
+
+/*
* Return -1 if none is found, otherwise the actual scripts number.
*
* Currently the only user of this function is the script browser, which
@@ -1039,17 +1101,23 @@ static int list_available_scripts(const struct option *opt __maybe_unused,
int find_scripts(char **scripts_array, char **scripts_path_array)
{
struct dirent *script_next, *lang_next, script_dirent, lang_dirent;
- char scripts_path[MAXPATHLEN];
+ char scripts_path[MAXPATHLEN], lang_path[MAXPATHLEN];
DIR *scripts_dir, *lang_dir;
- char lang_path[MAXPATHLEN];
+ struct perf_session *session;
char *temp;
int i = 0;
+ session = perf_session__new(input_name, O_RDONLY, 0, false, NULL);
+ if (!session)
+ return -1;
+
snprintf(scripts_path, MAXPATHLEN, "%s/scripts", perf_exec_path());
scripts_dir = opendir(scripts_path);
- if (!scripts_dir)
+ if (!scripts_dir) {
+ perf_session__delete(session);
return -1;
+ }
for_each_lang(scripts_path, scripts_dir, lang_dirent, lang_next) {
snprintf(lang_path, MAXPATHLEN, "%s/%s", scripts_path,
@@ -1077,10 +1145,18 @@ int find_scripts(char **scripts_array, char **scripts_path_array)
snprintf(scripts_array[i],
(temp - script_dirent.d_name) + 1,
"%s", script_dirent.d_name);
+
+ if (check_ev_match(lang_path,
+ scripts_array[i], session))
+ continue;
+
i++;
}
+ closedir(lang_dir);
}
+ closedir(scripts_dir);
+ perf_session__delete(session);
return i;
}
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [tip:perf/core] perf scripts browser: Add a browser for perf script
2012-10-30 3:56 ` [PATCH v5 3/8] perf ui/browser: Add a browser for perf script Feng Tang
@ 2012-10-30 12:07 ` tip-bot for Feng Tang
0 siblings, 0 replies; 19+ messages in thread
From: tip-bot for Feng Tang @ 2012-10-30 12:07 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, hpa, mingo, andi, peterz, namhyung, tglx,
feng.tang, mingo
Commit-ID: 66517826664fa910d4bc5f32a5abff6bcd8657c5
Gitweb: http://git.kernel.org/tip/66517826664fa910d4bc5f32a5abff6bcd8657c5
Author: Feng Tang <feng.tang@intel.com>
AuthorDate: Tue, 30 Oct 2012 11:56:04 +0800
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 29 Oct 2012 11:52:53 -0200
perf scripts browser: Add a browser for perf script
Create a script browser, so that user can check all the available
scripts for current perf data file and run them inside the main perf
report or annotation browsers, for all perf samples or for samples
belong to one thread/symbol.
Please be noted: current script browser is only for report use, and
doesn't cover the record phase, IOW it must run against one existing
perf data file.
The work flow is, users can use function key to list all the available
scripts for current perf data file in system and chose one, which will
be executed with popen("perf script -s xxx.xx",) and all the output
lines are put into one ui browser, pressing 'q' or left arrow key will
make it return to previous browser.
Signed-off-by: Feng Tang <feng.tang@intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1351569369-26732-4-git-send-email-feng.tang@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/Makefile | 4 +
tools/perf/ui/browsers/scripts.c | 189 ++++++++++++++++++++++++++++++++++++++
tools/perf/util/hist.h | 7 ++
3 files changed, 200 insertions(+), 0 deletions(-)
diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index ec63d53..7e25f59 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -593,6 +593,7 @@ ifndef NO_NEWT
LIB_OBJS += $(OUTPUT)ui/browsers/annotate.o
LIB_OBJS += $(OUTPUT)ui/browsers/hists.o
LIB_OBJS += $(OUTPUT)ui/browsers/map.o
+ LIB_OBJS += $(OUTPUT)ui/browsers/scripts.o
LIB_OBJS += $(OUTPUT)ui/progress.o
LIB_OBJS += $(OUTPUT)ui/util.o
LIB_OBJS += $(OUTPUT)ui/tui/setup.o
@@ -909,6 +910,9 @@ $(OUTPUT)ui/browsers/hists.o: ui/browsers/hists.c $(OUTPUT)PERF-CFLAGS
$(OUTPUT)ui/browsers/map.o: ui/browsers/map.c $(OUTPUT)PERF-CFLAGS
$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DENABLE_SLFUTURE_CONST $<
+$(OUTPUT)ui/browsers/scripts.o: ui/browsers/scripts.c $(OUTPUT)PERF-CFLAGS
+ $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DENABLE_SLFUTURE_CONST $<
+
$(OUTPUT)util/rbtree.o: ../../lib/rbtree.c $(OUTPUT)PERF-CFLAGS
$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -Wno-unused-parameter -DETC_PERFCONFIG='"$(ETC_PERFCONFIG_SQ)"' $<
diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
new file mode 100644
index 0000000..cbbd44b
--- /dev/null
+++ b/tools/perf/ui/browsers/scripts.c
@@ -0,0 +1,189 @@
+#include <elf.h>
+#include <newt.h>
+#include <inttypes.h>
+#include <sys/ttydefaults.h>
+#include <string.h>
+#include "../../util/sort.h"
+#include "../../util/util.h"
+#include "../../util/hist.h"
+#include "../../util/debug.h"
+#include "../../util/symbol.h"
+#include "../browser.h"
+#include "../helpline.h"
+#include "../libslang.h"
+
+/* 2048 lines should be enough for a script output */
+#define MAX_LINES 2048
+
+/* 160 bytes for one output line */
+#define AVERAGE_LINE_LEN 160
+
+struct script_line {
+ struct list_head node;
+ char line[AVERAGE_LINE_LEN];
+};
+
+struct perf_script_browser {
+ struct ui_browser b;
+ struct list_head entries;
+ const char *script_name;
+ int nr_lines;
+};
+
+#define SCRIPT_NAMELEN 128
+#define SCRIPT_MAX_NO 64
+/*
+ * Usually the full path for a script is:
+ * /home/username/libexec/perf-core/scripts/python/xxx.py
+ * /home/username/libexec/perf-core/scripts/perl/xxx.pl
+ * So 256 should be long enough to contain the full path.
+ */
+#define SCRIPT_FULLPATH_LEN 256
+
+/*
+ * When success, will copy the full path of the selected script
+ * into the buffer pointed by script_name, and return 0.
+ * Return -1 on failure.
+ */
+static int list_scripts(char *script_name)
+{
+ char *buf, *names[SCRIPT_MAX_NO], *paths[SCRIPT_MAX_NO];
+ int i, num, choice, ret = -1;
+
+ /* Preset the script name to SCRIPT_NAMELEN */
+ buf = malloc(SCRIPT_MAX_NO * (SCRIPT_NAMELEN + SCRIPT_FULLPATH_LEN));
+ if (!buf)
+ return ret;
+
+ for (i = 0; i < SCRIPT_MAX_NO; i++) {
+ names[i] = buf + i * (SCRIPT_NAMELEN + SCRIPT_FULLPATH_LEN);
+ paths[i] = names[i] + SCRIPT_NAMELEN;
+ }
+
+ num = find_scripts(names, paths);
+ if (num > 0) {
+ choice = ui__popup_menu(num, names);
+ if (choice < num && choice >= 0) {
+ strcpy(script_name, paths[choice]);
+ ret = 0;
+ }
+ }
+
+ free(buf);
+ return ret;
+}
+
+static void script_browser__write(struct ui_browser *browser,
+ void *entry, int row)
+{
+ struct script_line *sline = list_entry(entry, struct script_line, node);
+ bool current_entry = ui_browser__is_current_entry(browser, row);
+
+ ui_browser__set_color(browser, current_entry ? HE_COLORSET_SELECTED :
+ HE_COLORSET_NORMAL);
+
+ slsmg_write_nstring(sline->line, browser->width);
+}
+
+static int script_browser__run(struct perf_script_browser *self)
+{
+ int key;
+
+ if (ui_browser__show(&self->b, self->script_name,
+ "Press <- or ESC to exit") < 0)
+ return -1;
+
+ while (1) {
+ key = ui_browser__run(&self->b, 0);
+
+ /* We can add some special key handling here if needed */
+ break;
+ }
+
+ ui_browser__hide(&self->b);
+ return key;
+}
+
+
+int script_browse(const char *script_opt)
+{
+ char cmd[SCRIPT_FULLPATH_LEN*2], script_name[SCRIPT_FULLPATH_LEN];
+ char *line = NULL;
+ size_t len = 0;
+ ssize_t retlen;
+ int ret = -1, nr_entries = 0;
+ FILE *fp;
+ void *buf;
+ struct script_line *sline;
+
+ struct perf_script_browser script = {
+ .b = {
+ .refresh = ui_browser__list_head_refresh,
+ .seek = ui_browser__list_head_seek,
+ .write = script_browser__write,
+ },
+ .script_name = script_name,
+ };
+
+ INIT_LIST_HEAD(&script.entries);
+
+ /* Save each line of the output in one struct script_line object. */
+ buf = zalloc((sizeof(*sline)) * MAX_LINES);
+ if (!buf)
+ return -1;
+ sline = buf;
+
+ memset(script_name, 0, SCRIPT_FULLPATH_LEN);
+ if (list_scripts(script_name))
+ goto exit;
+
+ sprintf(cmd, "perf script -s %s ", script_name);
+
+ if (script_opt)
+ strcat(cmd, script_opt);
+
+ if (input_name) {
+ strcat(cmd, " -i ");
+ strcat(cmd, input_name);
+ }
+
+ strcat(cmd, " 2>&1");
+
+ fp = popen(cmd, "r");
+ if (!fp)
+ goto exit;
+
+ while ((retlen = getline(&line, &len, fp)) != -1) {
+ strncpy(sline->line, line, AVERAGE_LINE_LEN);
+
+ /* If one output line is very large, just cut it short */
+ if (retlen >= AVERAGE_LINE_LEN) {
+ sline->line[AVERAGE_LINE_LEN - 1] = '\0';
+ sline->line[AVERAGE_LINE_LEN - 2] = '\n';
+ }
+ list_add_tail(&sline->node, &script.entries);
+
+ if (script.b.width < retlen)
+ script.b.width = retlen;
+
+ if (nr_entries++ >= MAX_LINES - 1)
+ break;
+ sline++;
+ }
+
+ if (script.b.width > AVERAGE_LINE_LEN)
+ script.b.width = AVERAGE_LINE_LEN;
+
+ if (line)
+ free(line);
+ pclose(fp);
+
+ script.nr_lines = nr_entries;
+ script.b.nr_entries = nr_entries;
+ script.b.entries = &script.entries;
+
+ ret = script_browser__run(&script);
+exit:
+ free(buf);
+ return ret;
+}
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index c751624..b874609 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -165,6 +165,7 @@ int hist_entry__tui_annotate(struct hist_entry *he, int evidx,
int perf_evlist__tui_browse_hists(struct perf_evlist *evlist, const char *help,
void(*timer)(void *arg), void *arg,
int refresh);
+int script_browse(const char *script_opt);
#else
static inline
int perf_evlist__tui_browse_hists(struct perf_evlist *evlist __maybe_unused,
@@ -186,6 +187,12 @@ static inline int hist_entry__tui_annotate(struct hist_entry *self
{
return 0;
}
+
+static inline int script_browse(const char *script_opt)
+{
+ return 0;
+}
+
#define K_LEFT -1
#define K_RIGHT -2
#endif
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [tip:perf/core] perf annotate browser: Integrate script browser into annotation browser
2012-10-30 3:56 ` [PATCH v5 4/8] perf ui/browser: Integrate script browser into annotation browser Feng Tang
@ 2012-10-30 12:08 ` tip-bot for Feng Tang
0 siblings, 0 replies; 19+ messages in thread
From: tip-bot for Feng Tang @ 2012-10-30 12:08 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, hpa, mingo, andi, peterz, namhyung, tglx,
feng.tang, mingo
Commit-ID: 79ee47faa77706f568f0329b7475c123b67a3b4a
Gitweb: http://git.kernel.org/tip/79ee47faa77706f568f0329b7475c123b67a3b4a
Author: Feng Tang <feng.tang@intel.com>
AuthorDate: Tue, 30 Oct 2012 11:56:05 +0800
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 29 Oct 2012 11:54:45 -0200
perf annotate browser: Integrate script browser into annotation browser
Integrate the script browser into annotation, users can press function
key 'r' to list all perf scripts and select one of them to run that
script, the output will be shown in a separate browser.
Signed-off-by: Feng Tang <feng.tang@intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1351569369-26732-5-git-send-email-feng.tang@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/ui/browsers/annotate.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 8f8cd2d..28f8aab 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -676,8 +676,14 @@ static int annotate_browser__run(struct annotate_browser *browser, int evidx,
"o Toggle disassembler output/simplified view\n"
"s Toggle source code view\n"
"/ Search string\n"
+ "r Run available scripts\n"
"? Search previous string\n");
continue;
+ case 'r':
+ {
+ script_browse(NULL);
+ continue;
+ }
case 'H':
nd = browser->curr_hot;
break;
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [tip:perf/core] perf hists browser: Integrate script browser into main hists browser
2012-10-30 3:56 ` [PATCH v5 5/8] perf ui/browser: Integrate script browser into main hists browser Feng Tang
2012-10-29 14:54 ` Arnaldo Carvalho de Melo
@ 2012-10-30 12:09 ` tip-bot for Feng Tang
1 sibling, 0 replies; 19+ messages in thread
From: tip-bot for Feng Tang @ 2012-10-30 12:09 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, hpa, mingo, andi, peterz, namhyung, tglx,
feng.tang, mingo
Commit-ID: cdbab7c201ab38f7b8d248ebf289025381166526
Gitweb: http://git.kernel.org/tip/cdbab7c201ab38f7b8d248ebf289025381166526
Author: Feng Tang <feng.tang@intel.com>
AuthorDate: Tue, 30 Oct 2012 11:56:06 +0800
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 29 Oct 2012 11:56:19 -0200
perf hists browser: Integrate script browser into main hists browser
Integrate the script browser into "perf report" framework, users can use
function key 'r' or the drop down menu to list all perf scripts and
select one of them, just like they did for the annotation.
Signed-off-by: Feng Tang <feng.tang@intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1351569369-26732-6-git-send-email-feng.tang@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/ui/browsers/hists.c | 38 ++++++++++++++++++++++++++++++++++++++
1 files changed, 38 insertions(+), 0 deletions(-)
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index ef2f93c..fe62284 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1141,6 +1141,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
int nr_options = 0;
int key = -1;
char buf[64];
+ char script_opt[64];
if (browser == NULL)
return -1;
@@ -1159,6 +1160,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
int choice = 0,
annotate = -2, zoom_dso = -2, zoom_thread = -2,
annotate_f = -2, annotate_t = -2, browse_map = -2;
+ int scripts_comm = -2, scripts_symbol = -2, scripts_all = -2;
nr_options = 0;
@@ -1211,6 +1213,8 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
hist_browser__reset(browser);
}
continue;
+ case 'r':
+ goto do_scripts;
case K_F1:
case 'h':
case '?':
@@ -1229,6 +1233,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
"E Expand all callchains\n"
"d Zoom into current DSO\n"
"t Zoom into current Thread\n"
+ "r Run available scripts\n"
"P Print histograms to perf.hist.N\n"
"V Verbose (DSO names in callchains, etc)\n"
"/ Filter symbol by name");
@@ -1317,6 +1322,25 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
browser->selection->map != NULL &&
asprintf(&options[nr_options], "Browse map details") > 0)
browse_map = nr_options++;
+
+ /* perf script support */
+ if (browser->he_selection) {
+ struct symbol *sym;
+
+ if (asprintf(&options[nr_options], "Run scripts for samples of thread [%s]",
+ browser->he_selection->thread->comm) > 0)
+ scripts_comm = nr_options++;
+
+ sym = browser->he_selection->ms.sym;
+ if (sym && sym->namelen &&
+ asprintf(&options[nr_options], "Run scripts for samples of symbol [%s]",
+ sym->name) > 0)
+ scripts_symbol = nr_options++;
+ }
+
+ if (asprintf(&options[nr_options], "Run scripts for all samples") > 0)
+ scripts_all = nr_options++;
+
add_exit_option:
options[nr_options++] = (char *)"Exit";
retry_popup_menu:
@@ -1411,6 +1435,20 @@ zoom_out_thread:
hists__filter_by_thread(hists);
hist_browser__reset(browser);
}
+ /* perf scripts support */
+ else if (choice == scripts_all || choice == scripts_comm ||
+ choice == scripts_symbol) {
+do_scripts:
+ memset(script_opt, 0, 64);
+
+ if (choice == scripts_comm)
+ sprintf(script_opt, " -c %s ", browser->he_selection->thread->comm);
+
+ if (choice == scripts_symbol)
+ sprintf(script_opt, " -S %s ", browser->he_selection->ms.sym->name);
+
+ script_browse(script_opt);
+ }
}
out_free_stack:
pstack__delete(fstack);
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [tip:perf/core] perf header: Add is_perf_magic() func
2012-10-30 3:56 ` [PATCH v5 6/8] perf header: Add is_perf_magic() func Feng Tang
@ 2012-10-30 12:10 ` tip-bot for Feng Tang
0 siblings, 0 replies; 19+ messages in thread
From: tip-bot for Feng Tang @ 2012-10-30 12:10 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, hpa, mingo, andi, peterz, namhyung, tglx,
feng.tang, mingo
Commit-ID: e84ba4e26833991d1c1c15a592b1474ee2b6dfdb
Gitweb: http://git.kernel.org/tip/e84ba4e26833991d1c1c15a592b1474ee2b6dfdb
Author: Feng Tang <feng.tang@intel.com>
AuthorDate: Tue, 30 Oct 2012 11:56:07 +0800
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 29 Oct 2012 11:56:59 -0200
perf header: Add is_perf_magic() func
With this function, other modules can basically check whether a file is
a legal perf data file by checking its first 8 bytes against all
possible perf magic numbers.
Change the function name from check_perf_magic to more meaningful
is_perf_magic as suggested by acme.
Signed-off-by: Feng Tang <feng.tang@intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1351569369-26732-7-git-send-email-feng.tang@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
| 10 ++++++++++
| 1 +
2 files changed, 11 insertions(+), 0 deletions(-)
--git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 514ed1b..195a47a 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2341,6 +2341,16 @@ static int try_all_pipe_abis(uint64_t hdr_sz, struct perf_header *ph)
return -1;
}
+bool is_perf_magic(u64 magic)
+{
+ if (!memcmp(&magic, __perf_magic1, sizeof(magic))
+ || magic == __perf_magic2
+ || magic == __perf_magic2_sw)
+ return true;
+
+ return false;
+}
+
static int check_magic_endian(u64 magic, uint64_t hdr_sz,
bool is_pipe, struct perf_header *ph)
{
--git a/tools/perf/util/header.h b/tools/perf/util/header.h
index 879d215..5f1cd68 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -154,6 +154,7 @@ int perf_event__synthesize_build_id(struct perf_tool *tool,
int perf_event__process_build_id(struct perf_tool *tool,
union perf_event *event,
struct perf_session *session);
+bool is_perf_magic(u64 magic);
/*
* arch specific callback
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v5 7/8] perf hists browser: Add option for runtime switching perf data file
2012-10-29 14:06 ` Arnaldo Carvalho de Melo
@ 2012-10-30 16:01 ` Feng Tang
0 siblings, 0 replies; 19+ messages in thread
From: Feng Tang @ 2012-10-30 16:01 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Andi Kleen, linux-kernel
Hi Arnaldo,
Thanks for the review.
On Mon, Oct 29, 2012 at 12:06:20PM -0200, Arnaldo Carvalho de Melo wrote:
> Em Tue, Oct 30, 2012 at 11:56:08AM +0800, Feng Tang escreveu:
> > + pwd_dir = opendir(pwd);
> > + if (!pwd_dir)
> > + return ret;
> > +
> > + memset(options, 0, sizeof(options));
> > + memset(options, 0, sizeof(abs_path));
> > +
> > + while ((dent = readdir(pwd_dir))) {
> > + char path[PATH_MAX];
> > + u64 magic;
> > + char *name = dent->d_name;
> > + FILE *file;
> > +
> > + if (!(dent->d_type == DT_REG))
> > + continue;
> > +
> > + snprintf(path, PATH_MAX, "%s/%s", pwd, name);
> > +
> > + file = fopen(path, "r");
> > + if (!file)
> > + continue;
> > +
> > + if (fread(&magic, 1, 8, file) < 8)
> > + goto close_file_and_continue;
> > +
> > + if (is_perf_magic(magic)) {
> > + options[nr_options] = strdup(name);
> > + if (!options[nr_options])
> > + goto close_file_and_continue;
>
> Silently not offering a valid file? At this point I think you should
> warn the user and bail out.
Yeah, we should give a warning message and go on with the valid data
files already got.
> > +
> > + abs_path[nr_options] = strdup(path);
> > + if (!abs_path[nr_options]) {
> > + free(options[nr_options]);
> > + goto close_file_and_continue;
> > + }
> > +
> > + nr_options++;
> > + }
> > +
> > +close_file_and_continue:
> > + fclose(file);
> > + if (nr_options >= 256)
> > + break;
>
> Why is this? At the very least a warning has to be given to the user
> that way too many perf.data files are present, so only the first N are
> in the menu.
Ok.
> > + }
> > + closedir(pwd_dir);
> > +
> > + if (nr_options) {
> > + choice = ui__popup_menu(nr_options, options);
> > + if (choice < nr_options && choice >= 0) {
> > + tmp = strdup(abs_path[choice]);
> > + if (tmp) {
> > + if (is_input_name_malloced)
> > + free((void *)input_name);
> > + input_name = tmp;
> > + is_input_name_malloced = true;
> > + ret = 0;
> > + }
>
> Here you return an error, but will the user get any warning on the
> caller of this function if it returns -1?
>
>
> > + /* Switch to another data file */
> > + else if (choice == switch_data) {
> > +do_data_switch:
> > + if (!switch_data_file()) {
> > + key = K_SWITCH_INPUT_DATA;
> > + break;
> > + }
> > + }
>
> ... no, so it will silently continue and the user will get confused.
>
Yes, I will add more ui_warning() in these error/problematic path
Thanks,
Feng
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 5/8] perf ui/browser: Integrate script browser into main hists browser
2012-10-29 14:54 ` Arnaldo Carvalho de Melo
@ 2012-10-30 16:05 ` Feng Tang
0 siblings, 0 replies; 19+ messages in thread
From: Feng Tang @ 2012-10-30 16:05 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Andi Kleen, linux-kernel
On Mon, Oct 29, 2012 at 12:54:23PM -0200, Arnaldo Carvalho de Melo wrote:
> Em Tue, Oct 30, 2012 at 11:56:06AM +0800, Feng Tang escreveu:
> > Integrate the script browser into "perf report" framework, users can
> > use function key 'r' or the drop down menu to list all perf scripts
> > and select one of them, just like they did for the annotation.
> >
> > Signed-off-by: Feng Tang <feng.tang@intel.com>
>
> I noticed this comment on another patch:
>
> --------------------------
> This initial version only enables it for 'perf report', by checking
> the "timer" parameter of perf_evsel__hists_browser() equals NULL.
> --------------------------
>
> Yeah, one can say that if a timer is provided, we can say its 'top' and
> not 'report', at least as things stand now.
>
> So please add a 'is_top(void *timer)' so that we know the intent more
> clearly and use it to avoid showing the 'r' key when in 'top' mode, ok?
Will do and thanks for the catch. I once thought about hiding script option
for 'top' mode, but forgot to follow it on.
Thanks,
Feng
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2012-10-30 12:11 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-30 3:56 [PATCH v5 0/8] perf tools: Add script browser and runtime data file switch Feng Tang
2012-10-30 3:56 ` [PATCH v5 1/8] perf tool: Add a global variable "const char *input_name" Feng Tang
2012-10-30 12:05 ` [tip:perf/core] perf tools: Add a global variable " const " tip-bot for Feng Tang
2012-10-30 3:56 ` [PATCH v5 2/8] perf script: Add more filter to find_scripts() Feng Tang
2012-10-30 12:06 ` [tip:perf/core] " tip-bot for Feng Tang
2012-10-30 3:56 ` [PATCH v5 3/8] perf ui/browser: Add a browser for perf script Feng Tang
2012-10-30 12:07 ` [tip:perf/core] perf scripts browser: " tip-bot for Feng Tang
2012-10-30 3:56 ` [PATCH v5 4/8] perf ui/browser: Integrate script browser into annotation browser Feng Tang
2012-10-30 12:08 ` [tip:perf/core] perf annotate browser: " tip-bot for Feng Tang
2012-10-30 3:56 ` [PATCH v5 5/8] perf ui/browser: Integrate script browser into main hists browser Feng Tang
2012-10-29 14:54 ` Arnaldo Carvalho de Melo
2012-10-30 16:05 ` Feng Tang
2012-10-30 12:09 ` [tip:perf/core] perf hists browser: " tip-bot for Feng Tang
2012-10-30 3:56 ` [PATCH v5 6/8] perf header: Add is_perf_magic() func Feng Tang
2012-10-30 12:10 ` [tip:perf/core] " tip-bot for Feng Tang
2012-10-30 3:56 ` [PATCH v5 7/8] perf hists browser: Add option for runtime switching perf data file Feng Tang
2012-10-29 14:06 ` Arnaldo Carvalho de Melo
2012-10-30 16:01 ` Feng Tang
2012-10-30 3:56 ` [PATCH v5 8/8] perf report: Enable the runtime switching of " Feng Tang
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).