From: Andi Kleen <[email protected]>
By default perf currently links with the GTK2 gui. This pulls
in a lot of external libraries. It also causes dependency
problems for distribution packages: simply installing perf
requires pulling in GTK2 with all its dependencies.
I think the UI is valuable, but it shouldn't be everywhere.
The interfaces between the main perf and the GTK2 perf are
already quite clean, so it's very straight forward to just
add a few weak stubs and then generate two executables:
perf and perfgtk
The only difference is that the gtk version links in the
GTK code and overrides the weak stubs.
(so everything is still only compiled once)
I currently gave it the preliminary name "perfgtk".
This cuts down the library dependencies on the main perf
dramatically. It also completely eliminates the GTK2_SUPPORT
ifdef.
% ldd ./perf | wc -l
18
% ldd ./perfgtk | wc -l
53
Cc: Namhyung Kim <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andi Kleen <[email protected]>
---
tools/perf/Makefile | 34 +++++++++++++++++++++-------------
tools/perf/config/Makefile | 4 ++--
tools/perf/ui/gtkstub.c | 37 +++++++++++++++++++++++++++++++++++++
tools/perf/ui/ui.h | 8 --------
tools/perf/util/annotate.h | 11 -----------
tools/perf/util/hist.h | 11 -----------
6 files changed, 60 insertions(+), 45 deletions(-)
create mode 100644 tools/perf/ui/gtkstub.c
diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 641fccd..25116fc 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -113,6 +113,7 @@ SPARSE_FLAGS = -D__BIG_ENDIAN__ -D__powerpc__
BUILTIN_OBJS =
LIB_H =
LIB_OBJS =
+GUI_OBJS =
PYRF_OBJS =
SCRIPT_SH =
@@ -161,11 +162,12 @@ $(OUTPUT)python/perf.so: $(PYTHON_EXT_SRCS) $(PYTHON_EXT_DEPS)
SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH))
-#
-# Single 'perf' binary right now:
-#
PROGRAMS += $(OUTPUT)perf
+ifndef NO_GTK2
+ PROGRAMS += $(OUTPUT)perfgtk
+endif
+
# what 'all' will build and 'install' will install, in perfexecdir
ALL_PROGRAMS = $(PROGRAMS) $(SCRIPTS)
@@ -366,6 +368,7 @@ LIB_OBJS += $(OUTPUT)ui/helpline.o
LIB_OBJS += $(OUTPUT)ui/progress.o
LIB_OBJS += $(OUTPUT)ui/util.o
LIB_OBJS += $(OUTPUT)ui/hist.o
+LIB_OBJS += $(OUTPUT)ui/gtkstub.o
LIB_OBJS += $(OUTPUT)ui/stdio/hist.o
LIB_OBJS += $(OUTPUT)arch/common.o
@@ -481,13 +484,13 @@ ifndef NO_SLANG
endif
ifndef NO_GTK2
- LIB_OBJS += $(OUTPUT)ui/gtk/browser.o
- LIB_OBJS += $(OUTPUT)ui/gtk/hists.o
- LIB_OBJS += $(OUTPUT)ui/gtk/setup.o
- LIB_OBJS += $(OUTPUT)ui/gtk/util.o
- LIB_OBJS += $(OUTPUT)ui/gtk/helpline.o
- LIB_OBJS += $(OUTPUT)ui/gtk/progress.o
- LIB_OBJS += $(OUTPUT)ui/gtk/annotate.o
+ GUI_OBJS += $(OUTPUT)ui/gtk/browser.o
+ GUI_OBJS += $(OUTPUT)ui/gtk/hists.o
+ GUI_OBJS += $(OUTPUT)ui/gtk/setup.o
+ GUI_OBJS += $(OUTPUT)ui/gtk/util.o
+ GUI_OBJS += $(OUTPUT)ui/gtk/helpline.o
+ GUI_OBJS += $(OUTPUT)ui/gtk/progress.o
+ GUI_OBJS += $(OUTPUT)ui/gtk/annotate.o
endif
ifndef NO_LIBPERL
@@ -541,6 +544,10 @@ $(OUTPUT)perf: $(OUTPUT)perf.o $(BUILTIN_OBJS) $(PERFLIBS)
$(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $(OUTPUT)perf.o \
$(BUILTIN_OBJS) $(LIBS) -o $@
+$(OUTPUT)perfgtk: $(OUTPUT)perf.o $(BUILTIN_OBJS) $(PERFLIBS) $(GUI_OBJS)
+ $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $(OUTPUT)perf.o \
+ $(BUILTIN_OBJS) $(GUI_OBJS) $(LIBS) $(GUI_EXTLIBS) -o $@
+
$(OUTPUT)builtin-help.o: builtin-help.c $(OUTPUT)common-cmds.h $(OUTPUT)PERF-CFLAGS
$(QUIET_CC)$(CC) -o $@ -c $(CFLAGS) \
'-DPERF_HTML_PATH="$(htmldir_SQ)"' \
@@ -645,12 +652,12 @@ $(OUTPUT)scripts/python/Perf-Trace-Util/Context.o: scripts/python/Perf-Trace-Uti
$(OUTPUT)perf-%: %.o $(PERFLIBS)
$(QUIET_LINK)$(CC) $(CFLAGS) -o $@ $(LDFLAGS) $(filter %.o,$^) $(LIBS)
-$(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H)
+$(LIB_OBJS) $(BUILTIN_OBJS) $(GUI_OBJS): $(LIB_H)
$(patsubst perf-%,%.o,$(PROGRAMS)): $(LIB_H) $(wildcard */*.h)
# we compile into subdirectories. if the target directory is not the source directory, they might not exists. So
# we depend the various files onto their directories.
-DIRECTORY_DEPS = $(LIB_OBJS) $(BUILTIN_OBJS) $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)common-cmds.h
+DIRECTORY_DEPS = $(LIB_OBJS) $(GUI_OBJS) $(BUILTIN_OBJS) $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)common-cmds.h
$(DIRECTORY_DEPS): | $(sort $(dir $(DIRECTORY_DEPS)))
# In the second step, we make a rule to actually create these directories
$(sort $(dir $(DIRECTORY_DEPS))):
@@ -792,7 +799,8 @@ $(INSTALL_DOC_TARGETS):
### Cleaning rules
clean: $(LIBTRACEEVENT)-clean $(LIBLK)-clean
- $(RM) $(LIB_OBJS) $(BUILTIN_OBJS) $(LIB_FILE) $(OUTPUT)perf-archive $(OUTPUT)perf.o $(LANG_BINDINGS)
+ $(RM) $(LIB_OBJS) $(GUI_OBJS) $(BUILTIN_OBJS) $(LIB_FILE)
+ $(RM) $(OUTPUT)perf-archive $(OUTPUT)perf.o $(LANG_BINDINGS)
$(RM) $(ALL_PROGRAMS) perf
$(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo $(OUTPUT)common-cmds.h TAGS tags cscope*
$(QUIET_SUBDIR0)Documentation $(QUIET_SUBDIR1) clean
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index b5d9238..d718961 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -86,6 +86,7 @@ CFLAGS += -Wextra
CFLAGS += -std=gnu99
EXTLIBS = -lelf -lpthread -lrt -lm
+GUI_EXTLIBS =
ifeq ($(call try-cc,$(SOURCE_HELLO),$(CFLAGS) -Werror -fstack-protector-all,-fstack-protector-all),y)
CFLAGS += -fstack-protector-all
@@ -268,9 +269,8 @@ ifndef NO_GTK2
ifeq ($(call try-cc,$(SOURCE_GTK2_INFOBAR),$(FLAGS_GTK2),-DHAVE_GTK_INFO_BAR),y)
CFLAGS += -DHAVE_GTK_INFO_BAR
endif
- CFLAGS += -DGTK2_SUPPORT
CFLAGS += $(shell pkg-config --cflags gtk+-2.0 2>/dev/null)
- EXTLIBS += $(shell pkg-config --libs gtk+-2.0 2>/dev/null)
+ GUI_EXTLIBS += $(shell pkg-config --libs gtk+-2.0 2>/dev/null)
endif
endif
diff --git a/tools/perf/ui/gtkstub.c b/tools/perf/ui/gtkstub.c
new file mode 100644
index 0000000..cb58d31
--- /dev/null
+++ b/tools/perf/ui/gtkstub.c
@@ -0,0 +1,37 @@
+#include "ui/ui.h"
+#include "util/annotate.h"
+
+/* Stubs used when the gtk2 code is not linked in */
+
+#define __weak __attribute__((weak))
+
+__weak int perf_gtk__init(void)
+{
+ return -1;
+}
+
+__weak void perf_gtk__exit(bool wait_for_ok __maybe_unused)
+{
+}
+
+__weak int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist __maybe_unused,
+ const char *help __maybe_unused,
+ struct hist_browser_timer *hbt __maybe_unused,
+ float min_pcnt __maybe_unused)
+{
+ return 0;
+}
+
+
+__weak void perf_gtk__show_annotations(void)
+{
+}
+
+__weak int symbol__gtk_annotate(struct symbol *sym __maybe_unused,
+ struct map *map __maybe_unused,
+ struct perf_evsel *evsel __maybe_unused,
+ struct hist_browser_timer *hbt __maybe_unused)
+{
+ return 0;
+}
+
diff --git a/tools/perf/ui/ui.h b/tools/perf/ui/ui.h
index 70cb0d4..3a6ca86 100644
--- a/tools/perf/ui/ui.h
+++ b/tools/perf/ui/ui.h
@@ -23,16 +23,8 @@ static inline int ui__init(void)
static inline void ui__exit(bool wait_for_ok __maybe_unused) {}
#endif
-#ifdef GTK2_SUPPORT
int perf_gtk__init(void);
void perf_gtk__exit(bool wait_for_ok);
-#else
-static inline int perf_gtk__init(void)
-{
- return -1;
-}
-static inline void perf_gtk__exit(bool wait_for_ok __maybe_unused) {}
-#endif
void ui__refresh_dimensions(bool force);
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index af75515..ac70cc6 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -165,7 +165,6 @@ static inline int symbol__tui_annotate(struct symbol *sym __maybe_unused,
}
#endif
-#ifdef GTK2_SUPPORT
int symbol__gtk_annotate(struct symbol *sym, struct map *map,
struct perf_evsel *evsel,
struct hist_browser_timer *hbt);
@@ -178,16 +177,6 @@ static inline int hist_entry__gtk_annotate(struct hist_entry *he,
}
void perf_gtk__show_annotations(void);
-#else
-static inline int hist_entry__gtk_annotate(struct hist_entry *he __maybe_unused,
- struct perf_evsel *evsel __maybe_unused,
- struct hist_browser_timer *hbt __maybe_unused)
-{
- return 0;
-}
-
-static inline void perf_gtk__show_annotations(void) {}
-#endif
extern const char *disassembler_style;
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 2d3790f..9fa8b2d 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -229,20 +229,9 @@ static inline int script_browse(const char *script_opt __maybe_unused)
#define K_SWITCH_INPUT_DATA -3000
#endif
-#ifdef GTK2_SUPPORT
int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist, const char *help,
struct hist_browser_timer *hbt __maybe_unused,
float min_pcnt);
-#else
-static inline
-int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist __maybe_unused,
- const char *help __maybe_unused,
- struct hist_browser_timer *hbt __maybe_unused,
- float min_pcnt __maybe_unused)
-{
- return 0;
-}
-#endif
unsigned int hists__sort_list_width(struct hists *self);
--
1.8.3.1
* Andi Kleen <[email protected]> wrote:
> From: Andi Kleen <[email protected]>
>
> By default perf currently links with the GTK2 gui. This pulls
> in a lot of external libraries. It also causes dependency
> problems for distribution packages: simply installing perf
> requires pulling in GTK2 with all its dependencies.
>
> I think the UI is valuable, but it shouldn't be everywhere.
>
> The interfaces between the main perf and the GTK2 perf are
> already quite clean, so it's very straight forward to just
> add a few weak stubs and then generate two executables:
> perf and perfgtk
>
> The only difference is that the gtk version links in the
> GTK code and overrides the weak stubs.
> (so everything is still only compiled once)
>
> I currently gave it the preliminary name "perfgtk".
>
> This cuts down the library dependencies on the main perf
> dramatically. It also completely eliminates the GTK2_SUPPORT
> ifdef.
>
> % ldd ./perf | wc -l
> 18
> % ldd ./perfgtk | wc -l
> 53
If you want fewer dependencies then build with 'make NO_GTK=1'.
Furthermore, what really matters in practice is binary size - and the GKT
UI frontend code isn't really big:
comet:~/tip/tools/perf> ls -l perf.gtk perf.nogtk
-rwxrwxr-x 1 mingo mingo 2525416 Aug 5 10:09 perf.gtk.stripped
-rwxrwxr-x 1 mingo mingo 2497480 Aug 5 10:09 perf.nogtk.stripped
that's only a 1% difference ...
So this is not a good idea, as it breaks the single binary structure of
perf, which is a rather powerful concept that has served us really well in
the past.
Thanks,
Ingo
On Mon, Aug 05, 2013 at 10:16:41AM +0200, Ingo Molnar wrote:
> If you want fewer dependencies then build with 'make NO_GTK=1'.
Doesn't help the distros. Installing perf and pulling all the graphics
libraries in is highly annoying, especially in size constrained VM or
Cloud images. Having a separate binary or at least an ldopen()able
plugin that can go into a separate package is highly desirable.
* Christoph Hellwig <[email protected]> wrote:
> On Mon, Aug 05, 2013 at 10:16:41AM +0200, Ingo Molnar wrote:
>
> > If you want fewer dependencies then build with 'make NO_GTK=1'.
>
> Doesn't help the distros. Installing perf and pulling all the graphics
> libraries in is highly annoying, especially in size constrained VM or
> Cloud images. Having a separate binary or at least an ldopen()able
> plugin that can go into a separate package is highly desirable.
Nonsense, a distro, if it truly worried about this, could create two
packages already, there's no need to expose configuration options in the
binary name itself and burden users with the separation. I sometimes
switch the UI frontend of perf depending on the workflow and the terminal,
it would be highly annoying if the binary name was changed to expose
configuration options.
The thing is, you strongly objected to perf itself when we offered it up
for an upstream merge and I'm not surprised you still don't like it.
Thanks,
Ingo
On Mon, Aug 05, 2013 at 10:31:32AM +0200, Ingo Molnar wrote:
> Nonsense, a distro, if it truly worried about this, could create two
> packages already, there's no need to expose configuration options in the
> binary name itself and burden users with the separation. I sometimes
> switch the UI frontend of perf depending on the workflow and the terminal,
> it would be highly annoying if the binary name was changed to expose
> configuration options.
Which means you'd have to use a different tool name or have incompatible
packages, both of which aren't desirable.
> The thing is, you strongly objected to perf itself when we offered it up
> for an upstream merge and I'm not surprised you still don't like it.
I strongly objected to adding it to the kernel tree, and I still stand
to that opinion because it makes using perf much more painful than it
needs to be. I never disliked perf itself and use it frequently now
that I can bypass some of the pains by just using an older distro
package.
But I'd much rather get this back to technical discussions than personal
attacks..
Hi,
On Mon, 5 Aug 2013 01:23:20 -0700, Christoph Hellwig wrote:
> On Mon, Aug 05, 2013 at 10:16:41AM +0200, Ingo Molnar wrote:
>> If you want fewer dependencies then build with 'make NO_GTK=1'.
>
> Doesn't help the distros. Installing perf and pulling all the graphics
> libraries in is highly annoying, especially in size constrained VM or
> Cloud images. Having a separate binary or at least an ldopen()able
> plugin that can go into a separate package is highly desirable.
I wrote a patch series [1] separating gtk code to a dso and use it with
libdl last year. But I didn't get much feedback probably due to the
mistake of not installing the dso to a proper place. It'd be great if
you guys take a look at it and give some comments.
Thanks,
Namhyung
[1] https://lkml.org/lkml/2012/11/14/510
* Christoph Hellwig <[email protected]> wrote:
> On Mon, Aug 05, 2013 at 10:31:32AM +0200, Ingo Molnar wrote:
>
> > Nonsense, a distro, if it truly worried about this, could create two
> > packages already, there's no need to expose configuration options in
> > the binary name itself and burden users with the separation. I
> > sometimes switch the UI frontend of perf depending on the workflow and
> > the terminal, it would be highly annoying if the binary name was
> > changed to expose configuration options.
>
> Which means you'd have to use a different tool name or have incompatible
> packages, both of which aren't desirable.
You'd have alternative packages - i.e. the configuration and dependency
difference is exposed in the packaging space, not in the user interface,
command name space.
(and yes, gtk linking in 20+ libraries is suboptimal, hopefully that will
eventually be fixed by the GTK project. If a leaner project with similarly
good UI elements comes around we might switch to it - without having to
rename the binary yet again.)
I.e. put the burden on packagers for too high library dependency
complexity, not on end users. A fair deal by any count.
> > The thing is, you strongly objected to perf itself when we offered it
> > up for an upstream merge and I'm not surprised you still don't like
> > it.
>
> I strongly objected to adding it to the kernel tree, and I still stand
> to that opinion because it makes using perf much more painful than it
> needs to be. [...]
That's still a red herring - 'using' perf for 99% of the users is to
install the perf package or to type 'make install' ...
> [...] I never disliked perf itself and use it frequently now that I can
> bypass some of the pains by just using an older distro package.
If you have special needs you could lobby your distro for different
versions - or you could build it from source.
Your solution, to split the binary into two parts, just to expose a
configuration option you don't want to enable in certain uses, burdens the
regular user of perf.
> But I'd much rather get this back to technical discussions than personal
> attacks..
You never replied to the original counter-arguments, such as this one from
Linus:
http://article.gmane.org/gmane.linux.kernel/849965
Or this one from Andrew:
http://article.gmane.org/gmane.linux.kernel/850067
so I assumed your (still arguably dishonest) objections are still valid
and still broad - and are reflected in this thread.
That's not a personal attack by any means - we met before and I actually
like you as a person, I just don't like your opinion here and I don't like
your occasionally dishonest discussion style: because it only focuses on
the narrow issue of packaging complexity and does not look at the bigger
picture such as health of development and end user ease of use.
Thanks,
Ingo
* Namhyung Kim <[email protected]> wrote:
> Hi,
>
> On Mon, 5 Aug 2013 01:23:20 -0700, Christoph Hellwig wrote:
> > On Mon, Aug 05, 2013 at 10:16:41AM +0200, Ingo Molnar wrote:
> >> If you want fewer dependencies then build with 'make NO_GTK=1'.
> >
> > Doesn't help the distros. Installing perf and pulling all the
> > graphics libraries in is highly annoying, especially in size
> > constrained VM or Cloud images. Having a separate binary or at least
> > an ldopen()able plugin that can go into a separate package is highly
> > desirable.
>
> I wrote a patch series [1] separating gtk code to a dso and use it with
> libdl last year. But I didn't get much feedback probably due to the
> mistake of not installing the dso to a proper place. It'd be great if
> you guys take a look at it and give some comments.
>
> Thanks,
> Namhyung
>
>
> [1] https://lkml.org/lkml/2012/11/14/510
Assuming that in the normal case it can be made to work like a full build
of perf today (i.e. without forcing LD_PRELOAD) then splitting the
libraries away looks like a much better solution than splitting the
binary.
Thanks,
Ingo
On Mon, Aug 5, 2013 at 12:12 PM, Ingo Molnar <[email protected]> wrote:
> Assuming that in the normal case it can be made to work like a full build
> of perf today (i.e. without forcing LD_PRELOAD) then splitting the
> libraries away looks like a much better solution than splitting the
> binary.
Yup, if we can make it work, it's probably the best option. I'm not very
keen on splitting the binary but pulling all the GTK dependencies
like we do now is definitely a problem.
Pekka
Namhyung Kim <[email protected]> writes:
>
> I wrote a patch series [1] separating gtk code to a dso and use it with
> libdl last year. But I didn't get much feedback probably due to the
> mistake of not installing the dso to a proper place. It'd be great if
> you guys take a look at it and give some comments.
It looks good as a replacement of this patch
Except for the requirement to build libperf as -fPIC
In some setups the PIC penalty can be high, and libperf
has some very hot code (e.g. callstack merging)
-Andi
--
[email protected] -- Speaking for myself only
On Mon, Aug 05, 2013 at 11:08:57AM +0200, Ingo Molnar wrote:
> You never replied to the original counter-arguments, such as this one from
> Linus:
>
> http://article.gmane.org/gmane.linux.kernel/849965
The only thing Linus sais is that it's trivial to generate a subpackage,
and that opofile is a desaster. Both of them are 100% correct but at
the same time entirely miss the point.
Yes, oprofile was and is a desaster, but that has aboslutely nothing to
do with where the code lives.
And yes, it's easy to generate a subpackage, but you still need all the
source tree first. There's a reason why things like X.org got split up
(too fine grained in my opinion, but that's another story).
As said I very much disagree with having the userspace perf tree in the
kernel still, but I've also given up on the fight as I have more
important things to do.
And as said before it has nothing to do with the issue discussed here
right now.
* Christoph Hellwig <[email protected]> wrote:
> On Mon, Aug 05, 2013 at 11:08:57AM +0200, Ingo Molnar wrote:
> > You never replied to the original counter-arguments, such as this one from
> > Linus:
> >
> > http://article.gmane.org/gmane.linux.kernel/849965
>
> The only thing Linus sais is that it's trivial to generate a subpackage,
> and that opofile is a desaster. Both of them are 100% correct but at
> the same time entirely miss the point.
>
> Yes, oprofile was and is a desaster, but that has aboslutely nothing to
> do with where the code lives.
>
> And yes, it's easy to generate a subpackage, but you still need all the
> source tree first. [...]
And the kernel source tree is not particularly hard to get so what's your
point? ...
> [...] There's a reason why things like X.org got split up (too fine
> grained in my opinion, but that's another story).
X.org got split up for all the wrong reasons, it's still a unified project
by and large, and the different components only work reliably when going
in lockstep so it's not like there's a true ABI between them.
So I really hope you don't advocate for that.
perf is the exact opposite: no split-up the development culture because
they are closely related, yet a relatively disciplined ABI between the
components. In fact the ABI is higher quality exactly because development
is more integrated and allows for ABI problems to be resolved before they
leak out. It also allows for faster iteration of development, without
nonsensical ABI steps pulluting the way.
> As said I very much disagree with having the userspace perf tree in the
> kernel still, but I've also given up on the fight as I have more
> important things to do.
>
> And as said before it has nothing to do with the issue discussed here
> right now.
My point is that it's a very similar meta argument: splitting up perf
usage space would be nonsensical and harmful in a similar fashion as it
would be harmful to split up the perf development space.
Put differently: there's strong benefits to having a unified perf
development environment and there's equally strong benefits on the user
side to have a unified perf usage environment that a single command
represents.
The benefits are not absolute and not unconditional, and any costs of
integration should be minimized to the best of our ability, but I hope you
get the drift of my argument ...
Thanks,
Ingo
On Mon, 12 Aug 2013, Ingo Molnar wrote:
> perf is the exact opposite: no split-up the development culture because
> they are closely related, yet a relatively disciplined ABI between the
> components. In fact the ABI is higher quality exactly because development
> is more integrated and allows for ABI problems to be resolved before they
> leak out. It also allows for faster iteration of development, without
> nonsensical ABI steps pulluting the way.
I don't know if I'd use "quality" and "perf ABI" in the same sentence.
It's a horrible ABI; it has the honor of having the longest syscall
manpage, beating out even ptrace.
It also really isn't that stable; I've had perf ABI changes break programs
I maintain at least three times in the last 2 kernel releases. Part of
this is due to the tight coupling into the kernel, in fact the only ABI
anyone seems to care about is that presented by the perf-tool CLI
interface; the _actual_ kernel ABI seems like an afterthought.
Vince
* Vince Weaver <[email protected]> wrote:
> On Mon, 12 Aug 2013, Ingo Molnar wrote:
>
> > perf is the exact opposite: no split-up the development culture
> > because they are closely related, yet a relatively disciplined ABI
> > between the components. In fact the ABI is higher quality exactly
> > because development is more integrated and allows for ABI problems to
> > be resolved before they leak out. It also allows for faster iteration
> > of development, without nonsensical ABI steps pulluting the way.
>
> I don't know if I'd use "quality" and "perf ABI" in the same sentence.
> It's a horrible ABI; it has the honor of having the longest syscall
> manpage, beating out even ptrace.
The functionality it provides is useful, and comprehensive documentation
of it is useful as well.
> It also really isn't that stable; I've had perf ABI changes break
> programs I maintain at least three times in the last 2 kernel releases.
> Part of this is due to the tight coupling into the kernel, in fact the
> only ABI anyone seems to care about is that presented by the perf-tool
> CLI interface; the _actual_ kernel ABI seems like an afterthought.
It's certainly complex, but the main root cause for your problems is what
I pointed out to you in previous, similar discussions: I'm not aware of
*any* tester using your library on devel kernels, so regressions in seldom
used functionality that you rely on simply doesn't get reported.
In the past you used to only test your library once the -stable kernel was
released - has this changed recently by any chance? I remember that in one
particular case I got a regression bugreport from you essentially on the
day of a -stable release.
If you tested -rc2 or so that would give us a much larger window to fix
any breakages that affect your library. (I'm not even asking for
linux-next testing.)
tools/perf is used much more prominently and breakages do get reported
reasonably early, typically before the merge window even opens.
Once we receive a report we do fix your regressions as well and mark them
for -stable.
To resolve this situation you could help us out by doing either of these:
1) integrate your tests into tools/, there's 'perf test' that has a ton
of testcases already
2) run your testsuite more frequently - instead of waiting for a stable
kernel to be released and then complain about breakage.
So far you refused to do any of this and blamed others for non-reported
breakages :-/
Thanks,
Ingo
Em Tue, Aug 13, 2013 at 12:48:21PM +0200, Ingo Molnar escreveu:
> To resolve this situation you could help us out by doing either of these:
> 1) integrate your tests into tools/, there's 'perf test' that has a ton
> of testcases already
I think Jiri is working on merging some of those tests, Jiri?
Yeah, these at least:
commit 06933e3a732bb305b0721f1051a45264588e0519
Author: Jiri Olsa <[email protected]>
Date: Sun Mar 10 19:41:11 2013 +0100
perf tests: Test breakpoint overflow signal handler counts
commit 5a6bef47b418676546ab86d25631c3cfb9ffaf2a
Author: Jiri Olsa <[email protected]>
Date: Sun Mar 10 19:41:10 2013 +0100
perf tests: Test breakpoint overflow signal handler
> 2) run your testsuite more frequently - instead of waiting for a stable
> kernel to be released and then complain about breakage.
My feeling is that he increased the frequency of such tests.
> So far you refused to do any of this and blamed others for non-reported
> breakages :-/
He has been using the perf trinity fuzzer and reporting problems, thanks
Vince!
- Arnaldo
On Tue, 13 Aug 2013, Ingo Molnar wrote:
> * Vince Weaver <[email protected]> wrote:
> In the past you used to only test your library once the -stable kernel was
> released - has this changed recently by any chance? I remember that in one
> particular case I got a regression bugreport from you essentially on the
> day of a -stable release.
>
> If you tested -rc2 or so that would give us a much larger window to fix
> any breakages that affect your library. (I'm not even asking for
> linux-next testing.)
I wasn't aware that the Linux "no-ABI breakage guarantee" only applied to
people who ran -rc kernels.
I've spent a lot of time enhancing trinity and writing my own perf syscall
fuzzer just to try to keep on top of ABI breaks as per your rules and they
still slip in and they still don't get reverted.
In any case, a list of perf-related ABI breakages is here.
http://web.eece.maine.edu/~vweaver/projects/perf_events/abi_breakage.html
I guess whether it's a lot or just a few depends on your perspective.
> To resolve this situation you could help us out by doing either of these:
>
> 1) integrate your tests into tools/, there's 'perf test' that has a ton
> of testcases already
I have enough trouble keeping my code up to date let alone wasting weeks
of time re-sending patches. Some things are just simpler to maintain
outside the kernel.
> 2) run your testsuite more frequently - instead of waiting for a stable
> kernel to be released and then complain about breakage.
I only have so many machines and so much time, and no one is funding me
for this work. I run -rc kernels when I can, but perf can be very
architecture and even CPU dependent so it is hard to get full coverage.
For example it's hard to test full SNB-EP uncore support w/o such a
machine.
Vince
* Arnaldo Carvalho de Melo <[email protected]> wrote:
> Em Tue, Aug 13, 2013 at 12:48:21PM +0200, Ingo Molnar escreveu:
> > To resolve this situation you could help us out by doing either of these:
>
> > 1) integrate your tests into tools/, there's 'perf test' that has a ton
> > of testcases already
>
> I think Jiri is working on merging some of those tests, Jiri?
>
> Yeah, these at least:
>
> commit 06933e3a732bb305b0721f1051a45264588e0519
> Author: Jiri Olsa <[email protected]>
> Date: Sun Mar 10 19:41:11 2013 +0100
>
> perf tests: Test breakpoint overflow signal handler counts
>
> commit 5a6bef47b418676546ab86d25631c3cfb9ffaf2a
> Author: Jiri Olsa <[email protected]>
> Date: Sun Mar 10 19:41:10 2013 +0100
>
> perf tests: Test breakpoint overflow signal handler
>
> > 2) run your testsuite more frequently - instead of waiting for a stable
> > kernel to be released and then complain about breakage.
>
> My feeling is that he increased the frequency of such tests.
>
> > So far you refused to do any of this and blamed others for non-reported
> > breakages :-/
>
> He has been using the perf trinity fuzzer and reporting problems, thanks
> Vince!
Absolutely, it's much appreciated, thanks Vince!
I was reacting to this claim:
> > I've had perf ABI changes break programs I maintain at least three
> > times in the last 2 kernel releases.
that can only be addressed by either extending 'perf test' or by testing
libpfm et al sooner. The upstream kernel can only address regressions that
get reported.
Thanks,
Ingo
* Vince Weaver <[email protected]> wrote:
> On Tue, 13 Aug 2013, Ingo Molnar wrote:
> > * Vince Weaver <[email protected]> wrote:
>
> > In the past you used to only test your library once the -stable kernel was
> > released - has this changed recently by any chance? I remember that in one
> > particular case I got a regression bugreport from you essentially on the
> > day of a -stable release.
> >
> > If you tested -rc2 or so that would give us a much larger window to
> > fix any breakages that affect your library. (I'm not even asking for
> > linux-next testing.)
>
> I wasn't aware that the Linux "no-ABI breakage guarantee" only applied
> to people who ran -rc kernels.
It obviously does not, and nowhere did I claim the opposite.
> I've spent a lot of time enhancing trinity and writing my own perf
> syscall fuzzer just to try to keep on top of ABI breaks as per your
> rules and they still slip in and they still don't get reverted.
Your fuzzing effort is much appreciated, but please don't mix this up with
the other ABI bugs you were talking about:
> > > > I've had perf ABI changes break programs I maintain at least three
> > > > times in the last 2 kernel releases.
We can only fix regressions after they get reported.
Thanks,
Ingo
On Tue, 13 Aug 2013, Ingo Molnar wrote:
>
> that can only be addressed by either extending 'perf test' or by testing
> libpfm et al sooner. The upstream kernel can only address regressions that
> get reported.
Most of the tests in my test-suite are reactive. Meaning, I wrote them
after an ABI-breaking change was reported elsewhere, and I needed a small
test case for bisection purposes. Thus they are good for finding if a
corner of the perf ABI re-breaks but they're not great at spotting new
breakages.
Writing a complete test suite for something as complicated as the
perf-event ABI is impractical. One thing you can do is require anyone
submitting new functionality also provide a regression test, but
I don't see that happening.
Another issue is that despite having some ABI definitions for files in
/sys, these are broken with impunity. And I've yet to have an
ABI-breaking changeset reverted based on my bug reports. So you can see
why I'm not really motivated to even bother trying, as it seems like it
would be pointless busy work at this point.
It would just be nice if we just straight out say "the ABI is whatever
lets the perf tool run. Anything else is undefined behavior and shouldn't
be relied on".
Vince
On Tue, Aug 13, 2013 at 05:57:16PM -0400, Vince Weaver wrote:
> On Tue, 13 Aug 2013, Ingo Molnar wrote:
>
> >
> > that can only be addressed by either extending 'perf test' or by testing
> > libpfm et al sooner. The upstream kernel can only address regressions that
> > get reported.
>
> Most of the tests in my test-suite are reactive. Meaning, I wrote them
> after an ABI-breaking change was reported elsewhere, and I needed a small
> test case for bisection purposes. Thus they are good for finding if a
> corner of the perf ABI re-breaks but they're not great at spotting new
> breakages.
I guess best would be to just run the major other users (PAPI, perf, trinity,
numatop, ...?) once around rc1 time.
If it's reported at rc1 things can usually be fixed.
So would just need a easy script to do a quick test of all of these.
-Andi
Em Tue, Aug 13, 2013 at 05:57:16PM -0400, Vince Weaver escreveu:
> On Tue, 13 Aug 2013, Ingo Molnar wrote:
> > that can only be addressed by either extending 'perf test' or by testing
> > libpfm et al sooner. The upstream kernel can only address regressions that
> > get reported.
> Most of the tests in my test-suite are reactive. Meaning, I wrote them
> after an ABI-breaking change was reported elsewhere, and I needed a small
> test case for bisection purposes. Thus they are good for finding if a
> corner of the perf ABI re-breaks but they're not great at spotting new
> breakages.
> Writing a complete test suite for something as complicated as the
> perf-event ABI is impractical. One thing you can do is require anyone
> submitting new functionality also provide a regression test, but
Agreed.
> I don't see that happening.
See some of Namhyung, Adrian and Jiri recent patchsets, they came with
'perf test' regression tests.
- ARnaldo
* Arnaldo Carvalho de Melo <[email protected]> wrote:
> Em Tue, Aug 13, 2013 at 05:57:16PM -0400, Vince Weaver escreveu:
> > On Tue, 13 Aug 2013, Ingo Molnar wrote:
> >
> > > that can only be addressed by either extending 'perf test' or by
> > > testing libpfm et al sooner. The upstream kernel can only address
> > > regressions that get reported.
>
> > Most of the tests in my test-suite are reactive. Meaning, I wrote
> > them after an ABI-breaking change was reported elsewhere, and I needed
> > a small test case for bisection purposes. Thus they are good for
> > finding if a corner of the perf ABI re-breaks but they're not great at
> > spotting new breakages.
Would be nice to merge those in into 'perf test' - reactive tests are
useful as well IMO.
> > Writing a complete test suite for something as complicated as the
> > perf-event ABI is impractical. One thing you can do is require anyone
> > submitting new functionality also provide a regression test, but
>
> Agreed.
>
> > I don't see that happening.
>
> See some of Namhyung, Adrian and Jiri recent patchsets, they came with
> 'perf test' regression tests.
That's good progress indeed! If we merge in the cases that Vince found
then we'd have good practical coverage and we could also start requiring
good testcases for every new ABI extension.
That method distributes the overhead of having to write it to those who
want to extend (and, statistically speaking, inadvertantlybreak!) the ABI.
Thanks,
Ingo