| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <1429687101-4360-9-git-send-email-namhyung@kernel.org>
Date: Wed, 22 Apr 2015 16:18:19 +0900
From: Namhyung Kim <namhyung@...nel.org>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: Ingo Molnar <mingo@...nel.org>,
Peter Zijlstra <a.p.zijlstra@...llo.nl>,
Jiri Olsa <jolsa@...hat.com>,
LKML <linux-kernel@...r.kernel.org>,
David Ahern <dsahern@...il.com>
Subject: [PATCH 08/10] perf hists browser: Split popup menu actions - part 2
Currently perf_evsel__hists_browse() function spins on a huge loop and
handles many key actions. Since it's hard to read and modify, let's
split it out into small helper functions.
The add_XXX_opt() functions are to register popup menu item on the
selected entry. When it adds an item, it also saves related data into
struct popup_action and returns 1 so that it can increase the number of
items (nr_options).
With this change, we can simplify the code just to call selected
callback function without considering various conditions. A callback
function named do_XXX is called with saved data when the item is
selected by user.
No functional change intended.
Signed-off-by: Namhyung Kim <namhyung@...nel.org>
---
tools/perf/ui/browsers/hists.c | 354 +++++++++++++++++++++++++----------------
1 file changed, 214 insertions(+), 140 deletions(-)
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 7d88a1cdf04b..9bd7b38de64c 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1402,8 +1402,16 @@ close_file_and_continue:
return ret;
}
+struct popup_action {
+ struct thread *thread;
+ struct dso *dso;
+ struct map_symbol ms;
+
+ int (*fn)(struct hist_browser *browser, struct popup_action *act);
+};
+
static int
-do_annotate(struct hist_browser *browser, struct map_symbol *ms)
+do_annotate(struct hist_browser *browser, struct popup_action *act)
{
struct perf_evsel *evsel;
struct annotation *notes;
@@ -1413,12 +1421,12 @@ do_annotate(struct hist_browser *browser, struct map_symbol *ms)
if (!objdump_path && perf_session_env__lookup_objdump(browser->env))
return 0;
- notes = symbol__annotation(ms->sym);
+ notes = symbol__annotation(act->ms.sym);
if (!notes->src)
return 0;
evsel = hists_to_evsel(browser->hists);
- err = map_symbol__tui_annotate(ms, evsel, browser->hbt);
+ err = map_symbol__tui_annotate(&act->ms, evsel, browser->hbt);
he = hist_browser__selected_entry(browser);
/*
* offer option to annotate the other branch source or target
@@ -1434,8 +1442,27 @@ do_annotate(struct hist_browser *browser, struct map_symbol *ms)
}
static int
-do_zoom_thread(struct hist_browser *browser, struct thread *thread)
+add_annotate_opt(struct hist_browser *browser __maybe_unused,
+ struct popup_action *act, char **optstr,
+ struct map *map, struct symbol *sym)
{
+ if (sym == NULL || map->dso->annotate_warned)
+ return 0;
+
+ if (asprintf(optstr, "Annotate %s", sym->name) < 0)
+ return 0;
+
+ act->ms.map = map;
+ act->ms.sym = sym;
+ act->fn = do_annotate;
+ return 1;
+}
+
+static int
+do_zoom_thread(struct hist_browser *browser, struct popup_action *act)
+{
+ struct thread *thread = act->thread;
+
if (browser->hists->thread_filter) {
pstack__remove(browser->pstack, &browser->hists->thread_filter);
perf_hpp__set_elide(HISTC_THREAD, false);
@@ -1456,8 +1483,28 @@ do_zoom_thread(struct hist_browser *browser, struct thread *thread)
}
static int
-do_zoom_dso(struct hist_browser *browser, struct dso *dso)
+add_thread_opt(struct hist_browser *browser, struct popup_action *act,
+ char **optstr, struct thread *thread)
+{
+ if (thread == NULL)
+ return 0;
+
+ if (asprintf(optstr, "Zoom %s %s(%d) thread",
+ browser->hists->thread_filter ? "out of" : "into",
+ thread->comm_set ? thread__comm_str(thread) : "",
+ thread->tid) < 0)
+ return 0;
+
+ act->thread = thread;
+ act->fn = do_zoom_thread;
+ return 1;
+}
+
+static int
+do_zoom_dso(struct hist_browser *browser, struct popup_action *act)
{
+ struct dso *dso = act->dso;
+
if (browser->hists->dso_filter) {
pstack__remove(browser->pstack, &browser->hists->dso_filter);
perf_hpp__set_elide(HISTC_DSO, false);
@@ -1479,25 +1526,58 @@ do_zoom_dso(struct hist_browser *browser, struct dso *dso)
}
static int
-do_browse_map(struct hist_browser *browser __maybe_unused, struct map *map)
+add_dso_opt(struct hist_browser *browser, struct popup_action *act,
+ char **optstr, struct dso *dso)
{
- map__browse(map);
+ if (dso == NULL)
+ return 0;
+
+ if (asprintf(optstr, "Zoom %s %s DSO",
+ browser->hists->dso_filter ? "out of" : "into",
+ dso->kernel ? "the Kernel" : dso->short_name) < 0)
+ return 0;
+
+ act->dso = dso;
+ act->fn = do_zoom_dso;
+ return 1;
+}
+
+static int
+do_browse_map(struct hist_browser *browser __maybe_unused,
+ struct popup_action *act)
+{
+ map__browse(act->ms.map);
return 0;
}
static int
+add_map_opt(struct hist_browser *browser __maybe_unused,
+ struct popup_action *act, char **optstr, struct map *map)
+{
+ if (map == NULL)
+ return 0;
+
+ if (asprintf(optstr, "Browse map details") < 0)
+ return 0;
+
+ act->ms.map = map;
+ act->fn = do_browse_map;
+ return 1;
+}
+
+static int
do_run_script(struct hist_browser *browser __maybe_unused,
- struct thread *thread, struct symbol *sym)
+ struct popup_action *act)
{
char script_opt[64];
memset(script_opt, 0, sizeof(script_opt));
- if (thread) {
+ if (act->thread) {
scnprintf(script_opt, sizeof(script_opt), " -c %s ",
- thread__comm_str(thread));
- } else if (sym) {
+ thread__comm_str(act->thread));
+ } else if (act->ms.sym) {
scnprintf(script_opt, sizeof(script_opt), " -S %s ",
- sym->name);
+ act->ms.sym->name);
}
script_browse(script_opt);
@@ -1505,17 +1585,74 @@ do_run_script(struct hist_browser *browser __maybe_unused,
}
static int
-do_switch_data(struct hist_browser *browser __maybe_unused, int key)
+add_script_opt(struct hist_browser *browser __maybe_unused,
+ struct popup_action *act, char **optstr,
+ struct thread *thread, struct symbol *sym)
+{
+ if (thread) {
+ if (asprintf(optstr, "Run scripts for samples of thread [%s]",
+ thread__comm_str(thread)) < 0)
+ return 0;
+ } else if (sym) {
+ if (asprintf(optstr, "Run scripts for samples of symbol [%s]",
+ sym->name) < 0)
+ return 0;
+ } else {
+ if (asprintf(optstr, "Run scripts for all samples") < 0)
+ return 0;
+ }
+
+ act->thread = thread;
+ act->ms.sym = sym;
+ act->fn = do_run_script;
+ return 1;
+}
+
+static int
+do_switch_data(struct hist_browser *browser __maybe_unused,
+ struct popup_action *act __maybe_unused)
{
if (switch_data_file()) {
ui__warning("Won't switch the data files due to\n"
"no valid data file get selected!\n");
- return key;
+ return 0;
}
return K_SWITCH_INPUT_DATA;
}
+static int
+add_switch_opt(struct hist_browser *browser,
+ struct popup_action *act, char **optstr)
+{
+ if (!is_report_browser(browser->hbt))
+ return 0;
+
+ if (asprintf(optstr, "Switch to another data file in PWD") < 0)
+ return 0;
+
+ act->fn = do_switch_data;
+ return 1;
+}
+
+static int
+do_exit_browser(struct hist_browser *browser __maybe_unused,
+ struct popup_action *act __maybe_unused)
+{
+ return 0;
+}
+
+static int
+add_exit_opt(struct hist_browser *browser __maybe_unused,
+ struct popup_action *act, char **optstr)
+{
+ if (asprintf(optstr, "Exit") < 0)
+ return 0;
+
+ act->fn = do_exit_browser;
+ return 1;
+}
+
static void hist_browser__update_nr_entries(struct hist_browser *hb)
{
u64 nr_entries = 0;
@@ -1546,6 +1683,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
struct branch_info *bi;
#define MAX_OPTIONS 16
char *options[MAX_OPTIONS];
+ struct popup_action actions[MAX_OPTIONS];
int nr_options = 0;
int key = -1;
char buf[64];
@@ -1600,6 +1738,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
ui_helpline__push(helpline);
memset(options, 0, sizeof(options));
+ memset(actions, 0, sizeof(actions));
perf_hpp__for_each_format(fmt)
perf_hpp__reset_width(fmt, hists);
@@ -1610,12 +1749,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
while (1) {
struct thread *thread = NULL;
struct dso *dso = NULL;
- struct map_symbol ms;
- 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, switch_data = -2;
+ int choice = 0;
nr_options = 0;
@@ -1648,22 +1782,23 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
browser->selection->map->dso->annotate_warned)
continue;
- ms.map = browser->selection->map;
- ms.sym = browser->selection->sym;
-
- do_annotate(browser, &ms);
+ actions->ms.map = browser->selection->map;
+ actions->ms.sym = browser->selection->sym;
+ do_annotate(browser, actions);
continue;
case 'P':
hist_browser__dump(browser);
continue;
case 'd':
- do_zoom_dso(browser, dso);
+ actions->dso = dso;
+ do_zoom_dso(browser, actions);
continue;
case 'V':
browser->show_dso = !browser->show_dso;
continue;
case 't':
- do_zoom_thread(browser, thread);
+ actions->thread = thread;
+ do_zoom_thread(browser, actions);
continue;
case '/':
if (ui_browser__input_window("Symbol to show",
@@ -1676,12 +1811,15 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
}
continue;
case 'r':
- if (is_report_browser(hbt))
- do_run_script(browser, NULL, NULL);
+ if (is_report_browser(hbt)) {
+ actions->thread = NULL;
+ actions->ms.sym = NULL;
+ do_run_script(browser, actions);
+ }
continue;
case 's':
if (is_report_browser(hbt)) {
- key = do_switch_data(browser, key);
+ key = do_switch_data(browser, actions);
if (key == K_SWITCH_INPUT_DATA)
goto out_free_stack;
}
@@ -1762,129 +1900,65 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
if (bi == NULL)
goto skip_annotation;
- if (bi->from.sym != NULL &&
- !bi->from.map->dso->annotate_warned &&
- asprintf(&options[nr_options], "Annotate %s", bi->from.sym->name) > 0) {
- annotate_f = nr_options++;
- }
-
- if (bi->to.sym != NULL &&
- !bi->to.map->dso->annotate_warned &&
- (bi->to.sym != bi->from.sym ||
- bi->to.map->dso != bi->from.map->dso) &&
- asprintf(&options[nr_options], "Annotate %s", bi->to.sym->name) > 0) {
- annotate_t = nr_options++;
- }
+ nr_options += add_annotate_opt(browser,
+ &actions[nr_options],
+ &options[nr_options],
+ bi->from.map,
+ bi->from.sym);
+ if (bi->to.sym != bi->from.sym)
+ nr_options += add_annotate_opt(browser,
+ &actions[nr_options],
+ &options[nr_options],
+ bi->to.map,
+ bi->to.sym);
} else {
- if (browser->selection->sym != NULL &&
- !browser->selection->map->dso->annotate_warned) {
- struct annotation *notes;
-
- notes = symbol__annotation(browser->selection->sym);
-
- if (notes->src &&
- asprintf(&options[nr_options], "Annotate %s",
- browser->selection->sym->name) > 0) {
- annotate = nr_options++;
- }
- }
+ nr_options += add_annotate_opt(browser,
+ &actions[nr_options],
+ &options[nr_options],
+ browser->selection->map,
+ browser->selection->sym);
}
skip_annotation:
- if (thread != NULL &&
- asprintf(&options[nr_options], "Zoom %s %s(%d) thread",
- (browser->hists->thread_filter ? "out of" : "into"),
- (thread->comm_set ? thread__comm_str(thread) : ""),
- thread->tid) > 0)
- zoom_thread = nr_options++;
-
- if (dso != NULL &&
- asprintf(&options[nr_options], "Zoom %s %s DSO",
- (browser->hists->dso_filter ? "out of" : "into"),
- (dso->kernel ? "the Kernel" : dso->short_name)) > 0)
- zoom_dso = nr_options++;
-
- if (browser->selection != NULL &&
- browser->selection->map != NULL &&
- asprintf(&options[nr_options], "Browse map details") > 0)
- browse_map = nr_options++;
+ nr_options += add_thread_opt(browser, &actions[nr_options],
+ &options[nr_options], thread);
+ nr_options += add_dso_opt(browser, &actions[nr_options],
+ &options[nr_options], dso);
+ nr_options += add_map_opt(browser, &actions[nr_options],
+ &options[nr_options],
+ browser->selection->map);
/* perf script support */
if (browser->he_selection) {
- struct symbol *sym;
-
- if (asprintf(&options[nr_options], "Run scripts for samples of thread [%s]",
- thread__comm_str(browser->he_selection->thread)) > 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++;
+ nr_options += add_script_opt(browser,
+ &actions[nr_options],
+ &options[nr_options],
+ thread, NULL);
+ nr_options += add_script_opt(browser,
+ &actions[nr_options],
+ &options[nr_options],
+ NULL, browser->selection->sym);
}
-
- if (asprintf(&options[nr_options], "Run scripts for all samples") > 0)
- scripts_all = nr_options++;
-
- if (is_report_browser(hbt) && asprintf(&options[nr_options],
- "Switch to another data file in PWD") > 0)
- switch_data = nr_options++;
+ nr_options += add_script_opt(browser, &actions[nr_options],
+ &options[nr_options], NULL, NULL);
+ nr_options += add_switch_opt(browser, &actions[nr_options],
+ &options[nr_options]);
add_exit_option:
- if (asprintf(&options[nr_options], "Exit") > 0)
- nr_options++;
-retry_popup_menu:
- choice = ui__popup_menu(nr_options, options);
+ nr_options += add_exit_opt(browser, &actions[nr_options],
+ &options[nr_options]);
- if (choice == nr_options - 1)
- break;
-
- if (choice == -1) {
- free_popup_options(options, nr_options - 1);
- continue;
- }
-
- if (choice == annotate || choice == annotate_t || choice == annotate_f) {
- struct hist_entry *he;
+ do {
+ struct popup_action *act;
- he = hist_browser__selected_entry(browser);
- if (he == NULL)
- continue;
+ choice = ui__popup_menu(nr_options, options);
+ if (choice == -1 || choice >= nr_options)
+ break;
- if (choice == annotate_f) {
- ms.map = he->branch_info->from.map;
- ms.sym = he->branch_info->from.sym;
- } else if (choice == annotate_t) {
- ms.map = he->branch_info->to.map;
- ms.sym = he->branch_info->to.sym;
- } else {
- ms = *browser->selection;
- }
+ act = &actions[choice];
+ key = act->fn(browser, act);
+ } while (key == 1);
- if (do_annotate(browser, &ms) == 1)
- goto retry_popup_menu;
- } else if (choice == browse_map) {
- do_browse_map(browser, browser->selection->map);
- } else if (choice == zoom_dso) {
- do_zoom_dso(browser, dso);
- } else if (choice == zoom_thread) {
- do_zoom_thread(browser, thread);
- }
- /* perf scripts support */
- else if (choice == scripts_all || choice == scripts_comm ||
- choice == scripts_symbol) {
- if (choice == scripts_comm)
- do_run_script(browser, browser->he_selection->thread, NULL);
- if (choice == scripts_symbol)
- do_run_script(browser, NULL, browser->he_selection->ms.sym);
- if (choice == scripts_all)
- do_run_script(browser, NULL, NULL);
- }
- /* Switch to another data file */
- else if (choice == switch_data) {
- key = do_switch_data(browser, key);
- if (key == K_SWITCH_INPUT_DATA)
- break;
- }
+ if (key == K_SWITCH_INPUT_DATA)
+ break;
}
out_free_stack:
pstack__delete(browser->pstack);
--
2.3.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists