* [PATCH 1/5] perf ui: Make setup_browser() generic
2012-03-26 8:51 [RFC PATCHSET] perf ui: Small preparation on further UI work Namhyung Kim
@ 2012-03-26 8:51 ` Namhyung Kim
2012-03-26 8:51 ` [PATCH 2/5] perf ui: Drop arg[cv] arguments from perf_gtk_setup_browser() Namhyung Kim
` (5 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Namhyung Kim @ 2012-03-26 8:51 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
Pekka Enberg
The setup_browser contained newt-related codes in it.
As gtk front-end added recently, it should be more
generic to handle both cases properly. So move newt
codes to the ui__init() for now.
Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
tools/perf/util/ui/setup.c | 44 +++++++++++++++++++++++++-------------------
1 files changed, 25 insertions(+), 19 deletions(-)
diff --git a/tools/perf/util/ui/setup.c b/tools/perf/util/ui/setup.c
index 85a69faa09aa..becdcd0d9ce7 100644
--- a/tools/perf/util/ui/setup.c
+++ b/tools/perf/util/ui/setup.c
@@ -93,14 +93,37 @@ static void newt_suspend(void *d __used)
newtResume();
}
+static void ui__exit(void);
+
+static void ui__signal(int sig)
+{
+ ui__exit();
+ psignal(sig, "perf");
+ exit(0);
+}
+
static int ui__init(void)
{
- int err = SLkp_init();
+ int err;
- if (err < 0)
+ newtInit();
+ err = SLkp_init();
+ if (err < 0) {
+ pr_err("TUI initialization failed.\n");
goto out;
+ }
SLkp_define_keysym((char *)"^(kB)", SL_KEY_UNTAB);
+
+ newtSetSuspendCallback(newt_suspend, NULL);
+ ui_helpline__init();
+ ui_browser__init();
+
+ signal(SIGSEGV, ui__signal);
+ signal(SIGFPE, ui__signal);
+ signal(SIGINT, ui__signal);
+ signal(SIGQUIT, ui__signal);
+ signal(SIGTERM, ui__signal);
out:
return err;
}
@@ -113,13 +136,6 @@ static void ui__exit(void)
SLang_reset_tty();
}
-static void ui__signal(int sig)
-{
- ui__exit();
- psignal(sig, "perf");
- exit(0);
-}
-
void setup_browser(bool fallback_to_pager)
{
if (!isatty(1) || !use_browser || dump_trace) {
@@ -130,17 +146,7 @@ void setup_browser(bool fallback_to_pager)
}
use_browser = 1;
- newtInit();
ui__init();
- newtSetSuspendCallback(newt_suspend, NULL);
- ui_helpline__init();
- ui_browser__init();
-
- signal(SIGSEGV, ui__signal);
- signal(SIGFPE, ui__signal);
- signal(SIGINT, ui__signal);
- signal(SIGQUIT, ui__signal);
- signal(SIGTERM, ui__signal);
}
void exit_browser(bool wait_for_ok)
--
1.7.7.6
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 2/5] perf ui: Drop arg[cv] arguments from perf_gtk_setup_browser()
2012-03-26 8:51 [RFC PATCHSET] perf ui: Small preparation on further UI work Namhyung Kim
2012-03-26 8:51 ` [PATCH 1/5] perf ui: Make setup_browser() generic Namhyung Kim
@ 2012-03-26 8:51 ` Namhyung Kim
2012-03-26 8:51 ` [PATCH 3/5] perf ui: Add gtk2 support into setup_browser() Namhyung Kim
` (4 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Namhyung Kim @ 2012-03-26 8:51 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
Pekka Enberg
As perf doesn't allow to specify gtk command-line option,
drop the arguments and pass NULL to gtk_init(). This makes
the function easier to be called from setup_browser().
Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
tools/perf/builtin-report.c | 2 +-
tools/perf/util/cache.h | 4 ++--
tools/perf/util/gtk/browser.c | 5 ++---
3 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 2e317438980b..50a05739091d 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -675,7 +675,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __used)
if (strcmp(report.input_name, "-") != 0) {
if (report.use_gtk)
- perf_gtk_setup_browser(argc, argv, true);
+ perf_gtk_setup_browser(true);
else
setup_browser(true);
} else {
diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h
index 8dd224df3e54..d22ca689fb15 100644
--- a/tools/perf/util/cache.h
+++ b/tools/perf/util/cache.h
@@ -46,14 +46,14 @@ void exit_browser(bool wait_for_ok);
#endif
#ifdef NO_GTK2_SUPPORT
-static inline void perf_gtk_setup_browser(int argc __used, const char *argv[] __used, bool fallback_to_pager)
+static inline void perf_gtk_setup_browser(bool fallback_to_pager)
{
if (fallback_to_pager)
setup_pager();
}
static inline void perf_gtk_exit_browser(bool wait_for_ok __used) {}
#else
-void perf_gtk_setup_browser(int argc, const char *argv[], bool fallback_to_pager);
+void perf_gtk_setup_browser(bool fallback_to_pager);
void perf_gtk_exit_browser(bool wait_for_ok);
#endif
diff --git a/tools/perf/util/gtk/browser.c b/tools/perf/util/gtk/browser.c
index 258352a2356c..a1a83de3f459 100644
--- a/tools/perf/util/gtk/browser.c
+++ b/tools/perf/util/gtk/browser.c
@@ -9,10 +9,9 @@
#define MAX_COLUMNS 32
-void perf_gtk_setup_browser(int argc, const char *argv[],
- bool fallback_to_pager __used)
+void perf_gtk_setup_browser(bool fallback_to_pager __used)
{
- gtk_init(&argc, (char ***)&argv);
+ gtk_init(NULL, NULL);
}
void perf_gtk_exit_browser(bool wait_for_ok __used)
--
1.7.7.6
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 3/5] perf ui: Add gtk2 support into setup_browser()
2012-03-26 8:51 [RFC PATCHSET] perf ui: Small preparation on further UI work Namhyung Kim
2012-03-26 8:51 ` [PATCH 1/5] perf ui: Make setup_browser() generic Namhyung Kim
2012-03-26 8:51 ` [PATCH 2/5] perf ui: Drop arg[cv] arguments from perf_gtk_setup_browser() Namhyung Kim
@ 2012-03-26 8:51 ` Namhyung Kim
2012-03-26 8:51 ` [PATCH 4/5] perf gtk: Rename functions for consistency Namhyung Kim
` (3 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Namhyung Kim @ 2012-03-26 8:51 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
Pekka Enberg
Now setup_browser can handle gtk2 front-end so move the
code to a new location ui/setup.c in order to remove
dependency on TUI support. To this end, make ui__init/exit
global symbols and take an argument.
Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
tools/perf/Makefile | 7 ++++++-
tools/perf/builtin-report.c | 5 +----
tools/perf/ui/setup.c | 42 ++++++++++++++++++++++++++++++++++++++++++
tools/perf/util/cache.h | 15 ++++++++++++++-
tools/perf/util/gtk/gtk.h | 4 ++++
tools/perf/util/ui/setup.c | 37 ++++++++-----------------------------
6 files changed, 75 insertions(+), 35 deletions(-)
create mode 100644 tools/perf/ui/setup.c
diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index b492e3a51268..3885ad061db9 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -498,6 +498,7 @@ else
LIB_OBJS += $(OUTPUT)util/ui/helpline.o
LIB_OBJS += $(OUTPUT)util/ui/progress.o
LIB_OBJS += $(OUTPUT)util/ui/util.o
+ LIB_OBJS += $(OUTPUT)ui/setup.o
LIB_H += util/ui/browser.h
LIB_H += util/ui/browsers/map.h
LIB_H += util/ui/helpline.h
@@ -510,7 +511,7 @@ else
endif
ifdef NO_GTK2
- BASIC_CFLAGS += -DNO_GTK2
+ BASIC_CFLAGS += -DNO_GTK2_SUPPORT
else
FLAGS_GTK2=$(ALL_CFLAGS) $(ALL_LDFLAGS) $(EXTLIBS) $(shell pkg-config --libs --cflags gtk+-2.0)
ifneq ($(call try-cc,$(SOURCE_GTK2),$(FLAGS_GTK2)),y)
@@ -520,6 +521,10 @@ else
BASIC_CFLAGS += $(shell pkg-config --cflags gtk+-2.0)
EXTLIBS += $(shell pkg-config --libs gtk+-2.0)
LIB_OBJS += $(OUTPUT)util/gtk/browser.o
+ # Make sure that it'd be included only once.
+ ifneq ($(findstring -DNO_NEWT_SUPPORT,$(BASIC_CFLAGS)),)
+ LIB_OBJS += $(OUTPUT)ui/setup.o
+ endif
endif
endif
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 50a05739091d..815b57001d04 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -674,10 +674,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __used)
}
if (strcmp(report.input_name, "-") != 0) {
- if (report.use_gtk)
- perf_gtk_setup_browser(true);
- else
- setup_browser(true);
+ setup_browser(true);
} else {
use_browser = 0;
}
diff --git a/tools/perf/ui/setup.c b/tools/perf/ui/setup.c
new file mode 100644
index 000000000000..228221a11830
--- /dev/null
+++ b/tools/perf/ui/setup.c
@@ -0,0 +1,42 @@
+#include "../util/cache.h"
+#include "../util/debug.h"
+#include "../util/ui/ui.h"
+#include "../util/ui/util.h"
+#include "../util/gtk/gtk.h"
+
+void setup_browser(bool fallback_to_pager)
+{
+ if (!isatty(1) || dump_trace)
+ use_browser = 0;
+
+ switch (use_browser) {
+ case 2:
+ perf_gtk_setup_browser(fallback_to_pager);
+ break;
+
+ case 1:
+ ui__init(fallback_to_pager);
+ break;
+
+ default:
+ if (fallback_to_pager)
+ setup_pager();
+ break;
+ }
+}
+
+void exit_browser(bool wait_for_ok)
+{
+ switch (use_browser) {
+ case 2:
+ perf_gtk_exit_browser(wait_for_ok);
+ break;
+
+ case 1:
+ ui__exit(wait_for_ok);
+ break;
+
+ default:
+ break;
+ }
+}
diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h
index d22ca689fb15..60ad454e9a8b 100644
--- a/tools/perf/util/cache.h
+++ b/tools/perf/util/cache.h
@@ -33,7 +33,7 @@ extern int pager_use_color;
extern int use_browser;
-#ifdef NO_NEWT_SUPPORT
+#if defined(NO_NEWT_SUPPORT) && defined(NO_GTK2_SUPPORT)
static inline void setup_browser(bool fallback_to_pager)
{
if (fallback_to_pager)
@@ -43,6 +43,18 @@ static inline void exit_browser(bool wait_for_ok __used) {}
#else
void setup_browser(bool fallback_to_pager);
void exit_browser(bool wait_for_ok);
+
+#ifdef NO_NEWT_SUPPORT
+static inline int ui__init(bool fallback_to_pager)
+{
+ if (fallback_to_pager)
+ setup_pager();
+ return 0;
+}
+static inline void ui__exit(bool wait_for_ok __used) {}
+#else
+int ui__init(bool fallback_to_pager);
+void ui__exit(bool wait_for_ok);
#endif
#ifdef NO_GTK2_SUPPORT
@@ -56,6 +68,7 @@ static inline void perf_gtk_exit_browser(bool wait_for_ok __used) {}
void perf_gtk_setup_browser(bool fallback_to_pager);
void perf_gtk_exit_browser(bool wait_for_ok);
#endif
+#endif /* NO_NEWT_SUPPORT && NO_GTK2_SUPPORT */
char *alias_lookup(const char *alias);
int split_cmdline(char *cmdline, const char ***argv);
diff --git a/tools/perf/util/gtk/gtk.h b/tools/perf/util/gtk/gtk.h
index 75177ee04032..f5ac81832704 100644
--- a/tools/perf/util/gtk/gtk.h
+++ b/tools/perf/util/gtk/gtk.h
@@ -1,8 +1,12 @@
#ifndef _PERF_GTK_H_
#define _PERF_GTK_H_ 1
+#ifndef NO_GTK2_SUPPORT
+
#pragma GCC diagnostic ignored "-Wstrict-prototypes"
#include <gtk/gtk.h>
#pragma GCC diagnostic error "-Wstrict-prototypes"
+#endif /* NO_GTK2_SUPPORT */
+
#endif /* _PERF_GTK_H_ */
diff --git a/tools/perf/util/ui/setup.c b/tools/perf/util/ui/setup.c
index becdcd0d9ce7..907adfb4e99c 100644
--- a/tools/perf/util/ui/setup.c
+++ b/tools/perf/util/ui/setup.c
@@ -93,16 +93,14 @@ static void newt_suspend(void *d __used)
newtResume();
}
-static void ui__exit(void);
-
static void ui__signal(int sig)
{
- ui__exit();
+ ui__exit(false);
psignal(sig, "perf");
exit(0);
}
-static int ui__init(void)
+int ui__init(bool fallback_to_pager __used)
{
int err;
@@ -128,34 +126,15 @@ out:
return err;
}
-static void ui__exit(void)
+void ui__exit(bool wait_for_ok)
{
+ if (wait_for_ok)
+ ui__question_window("Fatal Error",
+ ui_helpline__last_msg,
+ "Press any key...", 0);
+
SLtt_set_cursor_visibility(1);
SLsmg_refresh();
SLsmg_reset_smg();
SLang_reset_tty();
}
-
-void setup_browser(bool fallback_to_pager)
-{
- if (!isatty(1) || !use_browser || dump_trace) {
- use_browser = 0;
- if (fallback_to_pager)
- setup_pager();
- return;
- }
-
- use_browser = 1;
- ui__init();
-}
-
-void exit_browser(bool wait_for_ok)
-{
- if (use_browser > 0) {
- if (wait_for_ok)
- ui__question_window("Fatal Error",
- ui_helpline__last_msg,
- "Press any key...", 0);
- ui__exit();
- }
-}
--
1.7.7.6
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 4/5] perf gtk: Rename functions for consistency
2012-03-26 8:51 [RFC PATCHSET] perf ui: Small preparation on further UI work Namhyung Kim
` (2 preceding siblings ...)
2012-03-26 8:51 ` [PATCH 3/5] perf ui: Add gtk2 support into setup_browser() Namhyung Kim
@ 2012-03-26 8:51 ` Namhyung Kim
2012-03-26 8:51 ` [PATCH 5/5] perf ui: Change fallback policy of setup_browser() Namhyung Kim
` (2 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Namhyung Kim @ 2012-03-26 8:51 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
Pekka Enberg
We use double underscore characters to distinguish its subsystem
and actual function name.
Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
tools/perf/ui/setup.c | 4 ++--
tools/perf/util/cache.h | 8 ++++----
tools/perf/util/gtk/browser.c | 24 ++++++++++++------------
3 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/tools/perf/ui/setup.c b/tools/perf/ui/setup.c
index 228221a11830..ee4916fc8157 100644
--- a/tools/perf/ui/setup.c
+++ b/tools/perf/ui/setup.c
@@ -11,7 +11,7 @@ void setup_browser(bool fallback_to_pager)
switch (use_browser) {
case 2:
- perf_gtk_setup_browser(fallback_to_pager);
+ perf_gtk__setup_browser(fallback_to_pager);
break;
case 1:
@@ -29,7 +29,7 @@ void exit_browser(bool wait_for_ok)
{
switch (use_browser) {
case 2:
- perf_gtk_exit_browser(wait_for_ok);
+ perf_gtk__exit_browser(wait_for_ok);
break;
case 1:
diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h
index 60ad454e9a8b..845571f53121 100644
--- a/tools/perf/util/cache.h
+++ b/tools/perf/util/cache.h
@@ -58,15 +58,15 @@ void ui__exit(bool wait_for_ok);
#endif
#ifdef NO_GTK2_SUPPORT
-static inline void perf_gtk_setup_browser(bool fallback_to_pager)
+static inline void perf_gtk__setup_browser(bool fallback_to_pager)
{
if (fallback_to_pager)
setup_pager();
}
-static inline void perf_gtk_exit_browser(bool wait_for_ok __used) {}
+static inline void perf_gtk__exit_browser(bool wait_for_ok __used) {}
#else
-void perf_gtk_setup_browser(bool fallback_to_pager);
-void perf_gtk_exit_browser(bool wait_for_ok);
+void perf_gtk__setup_browser(bool fallback_to_pager);
+void perf_gtk__exit_browser(bool wait_for_ok);
#endif
#endif /* NO_NEWT_SUPPORT && NO_GTK2_SUPPORT */
diff --git a/tools/perf/util/gtk/browser.c b/tools/perf/util/gtk/browser.c
index a1a83de3f459..5eafd9b3b783 100644
--- a/tools/perf/util/gtk/browser.c
+++ b/tools/perf/util/gtk/browser.c
@@ -9,23 +9,23 @@
#define MAX_COLUMNS 32
-void perf_gtk_setup_browser(bool fallback_to_pager __used)
+void perf_gtk__setup_browser(bool fallback_to_pager __used)
{
gtk_init(NULL, NULL);
}
-void perf_gtk_exit_browser(bool wait_for_ok __used)
+void perf_gtk__exit_browser(bool wait_for_ok __used)
{
gtk_main_quit();
}
-static void perf_gtk_signal(int sig)
+static void perf_gtk__signal(int sig)
{
psignal(sig, "perf");
gtk_main_quit();
}
-static void perf_gtk_resize_window(GtkWidget *window)
+static void perf_gtk__resize_window(GtkWidget *window)
{
GdkRectangle rect;
GdkScreen *screen;
@@ -45,7 +45,7 @@ static void perf_gtk_resize_window(GtkWidget *window)
gtk_window_resize(GTK_WINDOW(window), width, height);
}
-static void perf_gtk_show_hists(GtkWidget *window, struct hists *hists)
+static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
{
GType col_types[MAX_COLUMNS];
GtkCellRenderer *renderer;
@@ -141,11 +141,11 @@ int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist,
GtkWidget *notebook;
GtkWidget *window;
- signal(SIGSEGV, perf_gtk_signal);
- signal(SIGFPE, perf_gtk_signal);
- signal(SIGINT, perf_gtk_signal);
- signal(SIGQUIT, perf_gtk_signal);
- signal(SIGTERM, perf_gtk_signal);
+ signal(SIGSEGV, perf_gtk__signal);
+ signal(SIGFPE, perf_gtk__signal);
+ signal(SIGINT, perf_gtk__signal);
+ signal(SIGQUIT, perf_gtk__signal);
+ signal(SIGTERM, perf_gtk__signal);
window = gtk_window_new(GTK_WINDOW_TOPLEVEL);
@@ -167,7 +167,7 @@ int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist,
GTK_POLICY_AUTOMATIC,
GTK_POLICY_AUTOMATIC);
- perf_gtk_show_hists(scrolled_window, hists);
+ perf_gtk__show_hists(scrolled_window, hists);
tab_label = gtk_label_new(evname);
@@ -178,7 +178,7 @@ int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist,
gtk_widget_show_all(window);
- perf_gtk_resize_window(window);
+ perf_gtk__resize_window(window);
gtk_window_set_position(GTK_WINDOW(window), GTK_WIN_POS_CENTER);
--
1.7.7.6
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 5/5] perf ui: Change fallback policy of setup_browser()
2012-03-26 8:51 [RFC PATCHSET] perf ui: Small preparation on further UI work Namhyung Kim
` (3 preceding siblings ...)
2012-03-26 8:51 ` [PATCH 4/5] perf gtk: Rename functions for consistency Namhyung Kim
@ 2012-03-26 8:51 ` Namhyung Kim
2012-03-26 9:46 ` [RFC PATCHSET] perf ui: Small preparation on further UI work Pekka Enberg
2012-03-26 17:38 ` Arnaldo Carvalho de Melo
6 siblings, 0 replies; 13+ messages in thread
From: Namhyung Kim @ 2012-03-26 8:51 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
Pekka Enberg
If gtk2 support is not enabled (or failed for some reason) try TUI
again instead of falling directly back to the stdio interface.
While at it, rename setup functions properly and remove unused
unused arguments.
Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
tools/perf/ui/setup.c | 18 ++++++++++--------
tools/perf/util/cache.h | 23 ++++++++++-------------
tools/perf/util/gtk/browser.c | 6 +++---
tools/perf/util/ui/setup.c | 6 +++---
4 files changed, 26 insertions(+), 27 deletions(-)
diff --git a/tools/perf/ui/setup.c b/tools/perf/ui/setup.c
index ee4916fc8157..5cd36675e6fb 100644
--- a/tools/perf/ui/setup.c
+++ b/tools/perf/ui/setup.c
@@ -11,14 +11,16 @@ void setup_browser(bool fallback_to_pager)
switch (use_browser) {
case 2:
- perf_gtk__setup_browser(fallback_to_pager);
- break;
-
+ if (perf_gtk__init() == 0)
+ break;
+ /* fall through */
case 1:
- ui__init(fallback_to_pager);
- break;
-
+ use_browser = 1;
+ if (perf_tui__init() == 0)
+ break;
+ /* fall through */
default:
+ use_browser = 0;
if (fallback_to_pager)
setup_pager();
break;
@@ -29,11 +31,11 @@ void exit_browser(bool wait_for_ok)
{
switch (use_browser) {
case 2:
- perf_gtk__exit_browser(wait_for_ok);
+ perf_gtk__exit(wait_for_ok);
break;
case 1:
- ui__exit(wait_for_ok);
+ perf_tui__exit(wait_for_ok);
break;
default:
diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h
index 845571f53121..d50b9f627d9d 100644
--- a/tools/perf/util/cache.h
+++ b/tools/perf/util/cache.h
@@ -45,28 +45,25 @@ void setup_browser(bool fallback_to_pager);
void exit_browser(bool wait_for_ok);
#ifdef NO_NEWT_SUPPORT
-static inline int ui__init(bool fallback_to_pager)
+static inline int perf_tui__init(void)
{
- if (fallback_to_pager)
- setup_pager();
- return 0;
+ return -1;
}
-static inline void ui__exit(bool wait_for_ok __used) {}
+static inline void perf_tui__exit(bool wait_for_ok __used) {}
#else
-int ui__init(bool fallback_to_pager);
-void ui__exit(bool wait_for_ok);
+int perf_tui__init(void);
+void perf_tui__exit(bool wait_for_ok);
#endif
#ifdef NO_GTK2_SUPPORT
-static inline void perf_gtk__setup_browser(bool fallback_to_pager)
+static inline int perf_gtk__init(void)
{
- if (fallback_to_pager)
- setup_pager();
+ return -1;
}
-static inline void perf_gtk__exit_browser(bool wait_for_ok __used) {}
+static inline void perf_gtk__exit(bool wait_for_ok __used) {}
#else
-void perf_gtk__setup_browser(bool fallback_to_pager);
-void perf_gtk__exit_browser(bool wait_for_ok);
+int perf_gtk__init(void);
+void perf_gtk__exit(bool wait_for_ok);
#endif
#endif /* NO_NEWT_SUPPORT && NO_GTK2_SUPPORT */
diff --git a/tools/perf/util/gtk/browser.c b/tools/perf/util/gtk/browser.c
index 5eafd9b3b783..17520310ea3c 100644
--- a/tools/perf/util/gtk/browser.c
+++ b/tools/perf/util/gtk/browser.c
@@ -9,12 +9,12 @@
#define MAX_COLUMNS 32
-void perf_gtk__setup_browser(bool fallback_to_pager __used)
+int perf_gtk__init(void)
{
- gtk_init(NULL, NULL);
+ return gtk_init_check(NULL, NULL) ? 0 : -1;
}
-void perf_gtk__exit_browser(bool wait_for_ok __used)
+void perf_gtk__exit(bool wait_for_ok __used)
{
gtk_main_quit();
}
diff --git a/tools/perf/util/ui/setup.c b/tools/perf/util/ui/setup.c
index 907adfb4e99c..dd33b7b65d3b 100644
--- a/tools/perf/util/ui/setup.c
+++ b/tools/perf/util/ui/setup.c
@@ -95,12 +95,12 @@ static void newt_suspend(void *d __used)
static void ui__signal(int sig)
{
- ui__exit(false);
+ perf_tui__exit(false);
psignal(sig, "perf");
exit(0);
}
-int ui__init(bool fallback_to_pager __used)
+int perf_tui__init(void)
{
int err;
@@ -126,7 +126,7 @@ out:
return err;
}
-void ui__exit(bool wait_for_ok)
+void perf_tui__exit(bool wait_for_ok)
{
if (wait_for_ok)
ui__question_window("Fatal Error",
--
1.7.7.6
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [RFC PATCHSET] perf ui: Small preparation on further UI work
2012-03-26 8:51 [RFC PATCHSET] perf ui: Small preparation on further UI work Namhyung Kim
` (4 preceding siblings ...)
2012-03-26 8:51 ` [PATCH 5/5] perf ui: Change fallback policy of setup_browser() Namhyung Kim
@ 2012-03-26 9:46 ` Pekka Enberg
2012-03-26 17:38 ` Arnaldo Carvalho de Melo
6 siblings, 0 replies; 13+ messages in thread
From: Pekka Enberg @ 2012-03-26 9:46 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Paul Mackerras,
Ingo Molnar, Namhyung Kim, LKML
On Mon, Mar 26, 2012 at 11:51 AM, Namhyung Kim <namhyung.kim@lge.com> wrote:
> This is my initial work of UI generalization. As we now get GTK2 support on
> perf report, improve setup_browser() to handle it properly so that we can
> add various UI specific initialization codes to the function. At least, we
> need basic error/warning handlers ASAP to see what's going on when an error
> occurred, IMHO.
>
> I put new file setup.c under ui directory (not under util/ui) and it may or
> may not be compiled depending on the system configuration. I think it'd be
> better moving generic UI codes to the directory and TUI specific codes to
> ui/tui, and so on, so that the util directory doesn't contain any UI codes.
> But before proceeding, I'd like to listen to your opinions :).
The series looks good to me:
Acked-by: Pekka Enberg <penberg@kernel.org>
Thanks!
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [RFC PATCHSET] perf ui: Small preparation on further UI work
2012-03-26 8:51 [RFC PATCHSET] perf ui: Small preparation on further UI work Namhyung Kim
` (5 preceding siblings ...)
2012-03-26 9:46 ` [RFC PATCHSET] perf ui: Small preparation on further UI work Pekka Enberg
@ 2012-03-26 17:38 ` Arnaldo Carvalho de Melo
2012-03-27 10:44 ` Pekka Enberg
2012-03-28 2:14 ` Namhyung Kim
6 siblings, 2 replies; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-03-26 17:38 UTC (permalink / raw)
To: Namhyung Kim
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
Pekka Enberg
Em Mon, Mar 26, 2012 at 05:51:31PM +0900, Namhyung Kim escreveu:
> This is my initial work of UI generalization. As we now get GTK2 support on
> perf report, improve setup_browser() to handle it properly so that we can
> add various UI specific initialization codes to the function. At least, we
> need basic error/warning handlers ASAP to see what's going on when an error
> occurred, IMHO.
>
> I put new file setup.c under ui directory (not under util/ui) and it may or
> may not be compiled depending on the system configuration. I think it'd be
> better moving generic UI codes to the directory and TUI specific codes to
> ui/tui, and so on, so that the util directory doesn't contain any UI codes.
> But before proceeding, I'd like to listen to your opinions :).
My plans were for util/ui/ to be this generalization you want to, hence
I didn't use the 't', i.e. I didn't call it util/tui/, because tui would
be just one of the possible backends.
But that still needs more work that I haven't be able to pursue. Pekka
thinks that doing it that way is not OK as it would limit what one could
do with a more featureful UI like GTK+.
I would like to still pursue a simple GUI using a function table for
the simple operations we use in the hists browser in
tools/perf/util/ui/ but if others want to pursue it the way the GTK+
browser is being worked on, more power to them.
So I would just leave things in tools/perf/util/ui/ and do what you did
in moving the TUI specific bits to a separate function, even ui__init()
would be ok for now, and then at setup_browser() check what kind of
interface is being used and call ui__init() if it is the TUI and the
gtk init one if GTK+ was chosen.
But wouldn't introduce tools/perf/ui/setup.c for that, no need for new
directory trees, I think :-)
- Arnaldo
> Any comments are welcome.
> Thanks.
>
> Namhyung Kim (5):
> perf ui: Make setup_browser() generic
> perf ui: Drop arg[cv] arguments from perf_gtk_setup_browser()
> perf ui: Add gtk2 support into setup_browser()
> perf gtk: Rename functions for consistency
> perf ui: Change fallback policy of setup_browser()
>
> tools/perf/Makefile | 7 ++++-
> tools/perf/builtin-report.c | 5 +---
> tools/perf/ui/setup.c | 44 +++++++++++++++++++++++++++++
> tools/perf/util/cache.h | 24 +++++++++++-----
> tools/perf/util/gtk/browser.c | 27 +++++++++---------
> tools/perf/util/gtk/gtk.h | 4 +++
> tools/perf/util/ui/setup.c | 61 +++++++++++++++-------------------------
> 7 files changed, 108 insertions(+), 64 deletions(-)
> create mode 100644 tools/perf/ui/setup.c
>
> --
> 1.7.7.6
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [RFC PATCHSET] perf ui: Small preparation on further UI work
2012-03-26 17:38 ` Arnaldo Carvalho de Melo
@ 2012-03-27 10:44 ` Pekka Enberg
2012-03-27 13:55 ` Arnaldo Carvalho de Melo
2012-03-28 2:14 ` Namhyung Kim
1 sibling, 1 reply; 13+ messages in thread
From: Pekka Enberg @ 2012-03-27 10:44 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Namhyung Kim, Peter Zijlstra, Paul Mackerras, Ingo Molnar,
Namhyung Kim, LKML
On Mon, 26 Mar 2012, Arnaldo Carvalho de Melo wrote:
> My plans were for util/ui/ to be this generalization you want to, hence
> I didn't use the 't', i.e. I didn't call it util/tui/, because tui would
> be just one of the possible backends.
>
> But that still needs more work that I haven't be able to pursue. Pekka
> thinks that doing it that way is not OK as it would limit what one could
> do with a more featureful UI like GTK+.
>
> I would like to still pursue a simple GUI using a function table for
> the simple operations we use in the hists browser in
> tools/perf/util/ui/ but if others want to pursue it the way the GTK+
> browser is being worked on, more power to them.
>
> So I would just leave things in tools/perf/util/ui/ and do what you did
> in moving the TUI specific bits to a separate function, even ui__init()
> would be ok for now, and then at setup_browser() check what kind of
> interface is being used and call ui__init() if it is the TUI and the
> gtk init one if GTK+ was chosen.
>
> But wouldn't introduce tools/perf/ui/setup.c for that, no need for new
> directory trees, I think :-)
I actually completely agree that we should aggressively consolidate code
between TUI and GTK back-ends. I also do think going for function table
for simple operations make sense.
What I think we disagreed about is that I don't want to make a totally new
abstraction for perf that hides the differences between TUI and GTK
completely. That is, I think both UI implementations should be allowed to
have different UI layouts and use all the toolkit specific extensions if
necessary.
I completely missed the newly added tools/perf/ui directory which doesn't
make much sense. We definitely ought to consolidate all this code under
tools/perf/util/ui and gradually move the tui code out of it instead.
[ I would, however, like to make the directory nesting less deep by
dropping the "util" part from the directory name. But that's a
completely different topic. ]
Pekka
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCHSET] perf ui: Small preparation on further UI work
2012-03-27 10:44 ` Pekka Enberg
@ 2012-03-27 13:55 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-03-27 13:55 UTC (permalink / raw)
To: Pekka Enberg
Cc: Namhyung Kim, Peter Zijlstra, Paul Mackerras, Ingo Molnar,
Namhyung Kim, LKML
Em Tue, Mar 27, 2012 at 01:44:42PM +0300, Pekka Enberg escreveu:
> I completely missed the newly added tools/perf/ui directory which doesn't
> make much sense. We definitely ought to consolidate all this code under
> tools/perf/util/ui and gradually move the tui code out of it instead.
>
> [ I would, however, like to make the directory nesting less deep by
> dropping the "util" part from the directory name. But that's a
> completely different topic. ]
Yeah, the tools/perf/util/ should go away at some point, Borislav's
patches are a step in that direction, he is again at it, this time
carving out subsets so that we can go on gradually doing this
separation, see the tools top level Makefile discussion.
- Arnaldo
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCHSET] perf ui: Small preparation on further UI work
2012-03-26 17:38 ` Arnaldo Carvalho de Melo
2012-03-27 10:44 ` Pekka Enberg
@ 2012-03-28 2:14 ` Namhyung Kim
2012-03-28 14:28 ` Arnaldo Carvalho de Melo
1 sibling, 1 reply; 13+ messages in thread
From: Namhyung Kim @ 2012-03-28 2:14 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, LKML, Pekka Enberg
Hi,
Arnaldo Carvalho de Melo <acme@ghostprotocols.net> wrote:
> Em Mon, Mar 26, 2012 at 05:51:31PM +0900, Namhyung Kim escreveu:
>> This is my initial work of UI generalization. As we now get GTK2 support on
>> perf report, improve setup_browser() to handle it properly so that we can
>> add various UI specific initialization codes to the function. At least, we
>> need basic error/warning handlers ASAP to see what's going on when an error
>> occurred, IMHO.
>>
>> I put new file setup.c under ui directory (not under util/ui) and it may or
>> may not be compiled depending on the system configuration. I think it'd be
>> better moving generic UI codes to the directory and TUI specific codes to
>> ui/tui, and so on, so that the util directory doesn't contain any UI codes.
>> But before proceeding, I'd like to listen to your opinions :).
>
> My plans were for util/ui/ to be this generalization you want to, hence
> I didn't use the 't', i.e. I didn't call it util/tui/, because tui would
> be just one of the possible backends.
>
> But that still needs more work that I haven't be able to pursue. Pekka
> thinks that doing it that way is not OK as it would limit what one could
> do with a more featureful UI like GTK+.
>
> I would like to still pursue a simple GUI using a function table for
> the simple operations we use in the hists browser in
> tools/perf/util/ui/ but if others want to pursue it the way the GTK+
> browser is being worked on, more power to them.
>
Yeah, I was thinking about it too. I think the warning/error reporting
utilties seem to fit for that direction.
> So I would just leave things in tools/perf/util/ui/ and do what you did
> in moving the TUI specific bits to a separate function, even ui__init()
> would be ok for now, and then at setup_browser() check what kind of
> interface is being used and call ui__init() if it is the TUI and the
> gtk init one if GTK+ was chosen.
>
I think it'd be better moving the TUI specific codes to a separate file
(under a separate directory, like tools/perf/util/ui/tui - but it looks
like so deep nesting) rather than a function since the generic
code (setup_browser) should be compiled without the TUI support.
Otherwise we'll see some #ifdef's in the source file(s).
Or, we can put those codes under the same directory (tools/perf/util/ui)
and have different suffixes - say, if generic code were XXX.c, TUI one
would be XXX-tui.c and GTK+ one would be XXX-gtk.c.
> But wouldn't introduce tools/perf/ui/setup.c for that, no need for new
> directory trees, I think :-)
>
OK.
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC PATCHSET] perf ui: Small preparation on further UI work
2012-03-28 2:14 ` Namhyung Kim
@ 2012-03-28 14:28 ` Arnaldo Carvalho de Melo
2012-03-29 1:03 ` Namhyung Kim
0 siblings, 1 reply; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-03-28 14:28 UTC (permalink / raw)
To: Namhyung Kim
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, LKML, Pekka Enberg
Em Wed, Mar 28, 2012 at 11:14:25AM +0900, Namhyung Kim escreveu:
> Em Tue, Mar 27, 2012 at 01:44:42PM +0300, Pekka Enberg escreveu:
> > Arnaldo Carvalho de Melo <acme@ghostprotocols.net> wrote:
> > So I would just leave things in tools/perf/util/ui/ and do what you did
> > in moving the TUI specific bits to a separate function, even ui__init()
> > would be ok for now, and then at setup_browser() check what kind of
> > interface is being used and call ui__init() if it is the TUI and the
> > gtk init one if GTK+ was chosen.
>
> I think it'd be better moving the TUI specific codes to a separate file
> (under a separate directory, like tools/perf/util/ui/tui - but it looks
> like so deep nesting) rather than a function since the generic
What do you have against directory trees? :P We need to chop off the
/util/ part, but apart from that...
> code (setup_browser) should be compiled without the TUI support.
> Otherwise we'll see some #ifdef's in the source file(s).
>
> Or, we can put those codes under the same directory (tools/perf/util/ui)
> and have different suffixes - say, if generic code were XXX.c, TUI one
> would be XXX-tui.c and GTK+ one would be XXX-gtk.c.
... what is the difference of using:
tools/perf/util/ui/tui/init.c
instead of:
tools/perf/util/ui/tui-init.c
?
Since we'll be introducing new files, it seems the first step would be
to just do a simple, no changes in the files, move of
tools/perf/util/ui/ to tools/perf/ui/ and then introduce
tools/perf/ui/tui/{init,etc}.c
Gtk would as well be moved to tools/perf/ui/gtk/.
At some point we would then introduce some sort of plugin mechanism
where files found in /usr/lib/perf/ui/ or some other suitable directory
would be loaded in a modprobe like way, i.e. if the user asks for --gtk,
it would try to find /usr/lib/perf/ui/gtk.so and try to load it, etc.
That way we start to reduce the miriad libraries that we have linked in
the main perf binary, making it easier for packaging, etc.
- Arnaldo
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [RFC PATCHSET] perf ui: Small preparation on further UI work
2012-03-28 14:28 ` Arnaldo Carvalho de Melo
@ 2012-03-29 1:03 ` Namhyung Kim
0 siblings, 0 replies; 13+ messages in thread
From: Namhyung Kim @ 2012-03-29 1:03 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, LKML, Pekka Enberg
Hi,
On Wed, 28 Mar 2012 11:28:08 -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Mar 28, 2012 at 11:14:25AM +0900, Namhyung Kim escreveu:
>> Em Tue, Mar 27, 2012 at 01:44:42PM +0300, Pekka Enberg escreveu:
>> > Arnaldo Carvalho de Melo <acme@ghostprotocols.net> wrote:
>
>> > So I would just leave things in tools/perf/util/ui/ and do what you did
>> > in moving the TUI specific bits to a separate function, even ui__init()
>> > would be ok for now, and then at setup_browser() check what kind of
>> > interface is being used and call ui__init() if it is the TUI and the
>> > gtk init one if GTK+ was chosen.
>>
>> I think it'd be better moving the TUI specific codes to a separate file
>> (under a separate directory, like tools/perf/util/ui/tui - but it looks
>> like so deep nesting) rather than a function since the generic
>
> What do you have against directory trees? :P We need to chop off the
> /util/ part, but apart from that...
>
>> code (setup_browser) should be compiled without the TUI support.
>> Otherwise we'll see some #ifdef's in the source file(s).
>>
>> Or, we can put those codes under the same directory (tools/perf/util/ui)
>> and have different suffixes - say, if generic code were XXX.c, TUI one
>> would be XXX-tui.c and GTK+ one would be XXX-gtk.c.
>
> ... what is the difference of using:
>
> tools/perf/util/ui/tui/init.c
>
> instead of:
>
> tools/perf/util/ui/tui-init.c
>
> ?
>
Nothing. I just want to remain the directory nesting level if you care. :)
> Since we'll be introducing new files, it seems the first step would be
> to just do a simple, no changes in the files, move of
> tools/perf/util/ui/ to tools/perf/ui/ and then introduce
> tools/perf/ui/tui/{init,etc}.c
>
> Gtk would as well be moved to tools/perf/ui/gtk/.
>
Yeah, This is what I wanted to have from the begining. I'll revise my
patches after moving the files to tools/perf/ui.
> At some point we would then introduce some sort of plugin mechanism
> where files found in /usr/lib/perf/ui/ or some other suitable directory
> would be loaded in a modprobe like way, i.e. if the user asks for --gtk,
> it would try to find /usr/lib/perf/ui/gtk.so and try to load it, etc.
>
> That way we start to reduce the miriad libraries that we have linked in
> the main perf binary, making it easier for packaging, etc.
>
> - Arnaldo
That would be great!
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 13+ messages in thread