| Message ID | 062b1dbee6747d8013ea91d7020216969aefb128.1418654436.git.naveen.n.rao@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Michael Ellerman |
| Headers | show |
Em Mon, Dec 15, 2014 at 08:20:35PM +0530, Naveen N. Rao escreveu: > Allow perf probe to work on powerpc ABIv1 without the need to specify > the leading dot '.' for functions. 'perf probe do_fork' works with this > patch. > > Introduce HAVE_ARCH_SYMBOL_HANDLING to indicate need for special > handling of symbols. In this patch, we override probe_function_filter() > on powerpc to account for dot symbols. This one looks better, does arch specific stuff in tools/perf/arch, good, some nits below. > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> > --- > Changes from the previous patchset: > Introduced arch helper to override the way probe function filter works. > > tools/perf/arch/powerpc/Makefile | 1 + > tools/perf/arch/powerpc/util/sym-handling.c | 28 ++++++++++++++++++++++++++++ > tools/perf/config/Makefile | 1 + > tools/perf/util/probe-event.c | 10 +++++----- > tools/perf/util/probe-event.h | 5 +++++ > 5 files changed, 40 insertions(+), 5 deletions(-) > create mode 100644 tools/perf/arch/powerpc/util/sym-handling.c > > diff --git a/tools/perf/arch/powerpc/Makefile b/tools/perf/arch/powerpc/Makefile > index 6f7782b..1c3d435 100644 > --- a/tools/perf/arch/powerpc/Makefile > +++ b/tools/perf/arch/powerpc/Makefile > @@ -3,4 +3,5 @@ PERF_HAVE_DWARF_REGS := 1 > LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/dwarf-regs.o > LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/skip-callchain-idx.o > endif > +LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/sym-handling.o > LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/header.o > diff --git a/tools/perf/arch/powerpc/util/sym-handling.c b/tools/perf/arch/powerpc/util/sym-handling.c > new file mode 100644 > index 0000000..0a77825 > --- /dev/null > +++ b/tools/perf/arch/powerpc/util/sym-handling.c > @@ -0,0 +1,28 @@ > +/* > + * Special symbol handling for PowerPC: > + * - Handle dot symbols on ABIv1 > + * > + * Copyright (C) 2014 Naveen N Rao, IBM Corporation. > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * as published by the Free Software Foundation; either version > + * 2 of the License, or (at your option) any later version. > + */ > + > +#include "map.h" > +#include "symbol.h" > +#include "probe-event.h" > + > +int probe_function_filter(struct map *map __maybe_unused, struct symbol *sym) > +{ > + if (sym->binding == STB_GLOBAL || sym->binding == STB_LOCAL) { > + if ((strcmp(looking_function_name, sym->name) == 0) || > + (sym->name[0] == '.' && looking_function_name[0] != '.' && > + strcmp(looking_function_name, sym->name+1) == 0)) { > + num_matched_functions++; > + return 0; > + } > + } > + return 1; > +} > diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile > index 5d4b039..35cf934 100644 > --- a/tools/perf/config/Makefile > +++ b/tools/perf/config/Makefile > @@ -383,6 +383,7 @@ ifeq ($(ARCH),powerpc) > ifndef NO_DWARF > CFLAGS += -DHAVE_SKIP_CALLCHAIN_IDX > endif > + CFLAGS += -DHAVE_ARCH_SYMBOL_HANDLING Dunno about this naming, looks too general: SYMBOL_HANDLING, but can't come to some better one now, anyone? > endif > > ifndef NO_LIBUNWIND > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c > index 74b7fef..7eb9b27 100644 > --- a/tools/perf/util/probe-event.c > +++ b/tools/perf/util/probe-event.c > @@ -50,6 +50,8 @@ > #define PERFPROBE_GROUP "probe" > > bool probe_event_dry_run; /* Dry run flag */ > +char *looking_function_name; > +int num_matched_functions; > > #define semantic_error(msg ...) pr_err("Semantic error :" msg) > > @@ -2210,11 +2212,8 @@ static int __add_probe_trace_events(struct perf_probe_event *pev, > return ret; > } > > -static char *looking_function_name; > -static int num_matched_functions; > - > -static int probe_function_filter(struct map *map __maybe_unused, > - struct symbol *sym) > +#ifndef HAVE_ARCH_SYMBOL_HANDLING > +int probe_function_filter(struct map *map __maybe_unused, struct symbol *sym) > { > if ((sym->binding == STB_GLOBAL || sym->binding == STB_LOCAL) && > strcmp(looking_function_name, sym->name) == 0) { > @@ -2223,6 +2222,7 @@ static int probe_function_filter(struct map *map __maybe_unused, > } > return 1; > } > +#endif /* HAVE_ARCH_SYMBOL_HANDLING */ Can't we do something like providing a weak function and let the linked to its work? I guess we have cases like this in tools/ already. I.e. not using the ifndef block. Minor nit tho. > > #define strdup_or_goto(str, label) \ > ({ char *__p = strdup(str); if (!__p) goto label; __p; }) > diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h > index e01e994..8564451 100644 > --- a/tools/perf/util/probe-event.h > +++ b/tools/perf/util/probe-event.h > @@ -7,6 +7,8 @@ > #include "strfilter.h" > > extern bool probe_event_dry_run; > +extern char *looking_function_name; > +extern int num_matched_functions; > > /* kprobe-tracer and uprobe-tracer tracing point */ > struct probe_trace_point { > @@ -136,6 +138,9 @@ extern int show_available_vars(struct perf_probe_event *pevs, int npevs, > extern int show_available_funcs(const char *module, struct strfilter *filter, > bool user); > > +extern int probe_function_filter(struct map *map __maybe_unused, > + struct symbol *sym); > + Please do not prefix function declarations with 'extern', even when we have one just before, its not needed, patches removing the existing ones would be accepted. > /* Maximum index number of event-name postfix */ > #define MAX_EVENT_INDEX 1024 > > -- > 2.1.3
On 2015/03/12 05:30PM, Arnaldo Carvalho de Melo wrote: > Em Mon, Dec 15, 2014 at 08:20:35PM +0530, Naveen N. Rao escreveu: > > Allow perf probe to work on powerpc ABIv1 without the need to specify > > the leading dot '.' for functions. 'perf probe do_fork' works with this > > patch. > > > > Introduce HAVE_ARCH_SYMBOL_HANDLING to indicate need for special > > handling of symbols. In this patch, we override probe_function_filter() > > on powerpc to account for dot symbols. > > This one looks better, does arch specific stuff in tools/perf/arch, > good, some nits below. > > > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> > > --- > > Changes from the previous patchset: > > Introduced arch helper to override the way probe function filter works. > > > > tools/perf/arch/powerpc/Makefile | 1 + > > tools/perf/arch/powerpc/util/sym-handling.c | 28 ++++++++++++++++++++++++++++ > > tools/perf/config/Makefile | 1 + > > tools/perf/util/probe-event.c | 10 +++++----- > > tools/perf/util/probe-event.h | 5 +++++ > > 5 files changed, 40 insertions(+), 5 deletions(-) > > create mode 100644 tools/perf/arch/powerpc/util/sym-handling.c > > > > diff --git a/tools/perf/arch/powerpc/Makefile b/tools/perf/arch/powerpc/Makefile > > index 6f7782b..1c3d435 100644 > > --- a/tools/perf/arch/powerpc/Makefile > > +++ b/tools/perf/arch/powerpc/Makefile > > @@ -3,4 +3,5 @@ PERF_HAVE_DWARF_REGS := 1 > > LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/dwarf-regs.o > > LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/skip-callchain-idx.o > > endif > > +LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/sym-handling.o > > LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/header.o > > diff --git a/tools/perf/arch/powerpc/util/sym-handling.c b/tools/perf/arch/powerpc/util/sym-handling.c > > new file mode 100644 > > index 0000000..0a77825 > > --- /dev/null > > +++ b/tools/perf/arch/powerpc/util/sym-handling.c > > @@ -0,0 +1,28 @@ > > +/* > > + * Special symbol handling for PowerPC: > > + * - Handle dot symbols on ABIv1 > > + * > > + * Copyright (C) 2014 Naveen N Rao, IBM Corporation. > > + * > > + * This program is free software; you can redistribute it and/or > > + * modify it under the terms of the GNU General Public License > > + * as published by the Free Software Foundation; either version > > + * 2 of the License, or (at your option) any later version. > > + */ > > + > > +#include "map.h" > > +#include "symbol.h" > > +#include "probe-event.h" > > + > > +int probe_function_filter(struct map *map __maybe_unused, struct symbol *sym) > > +{ > > + if (sym->binding == STB_GLOBAL || sym->binding == STB_LOCAL) { > > + if ((strcmp(looking_function_name, sym->name) == 0) || > > + (sym->name[0] == '.' && looking_function_name[0] != '.' && > > + strcmp(looking_function_name, sym->name+1) == 0)) { > > + num_matched_functions++; > > + return 0; > > + } > > + } > > + return 1; > > +} > > diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile > > index 5d4b039..35cf934 100644 > > --- a/tools/perf/config/Makefile > > +++ b/tools/perf/config/Makefile > > @@ -383,6 +383,7 @@ ifeq ($(ARCH),powerpc) > > ifndef NO_DWARF > > CFLAGS += -DHAVE_SKIP_CALLCHAIN_IDX > > endif > > + CFLAGS += -DHAVE_ARCH_SYMBOL_HANDLING > > > Dunno about this naming, looks too general: SYMBOL_HANDLING, but can't > come to some better one now, anyone? > > > endif > > > > ifndef NO_LIBUNWIND > > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c > > index 74b7fef..7eb9b27 100644 > > --- a/tools/perf/util/probe-event.c > > +++ b/tools/perf/util/probe-event.c > > @@ -50,6 +50,8 @@ > > #define PERFPROBE_GROUP "probe" > > > > bool probe_event_dry_run; /* Dry run flag */ > > +char *looking_function_name; > > +int num_matched_functions; > > > > #define semantic_error(msg ...) pr_err("Semantic error :" msg) > > > > @@ -2210,11 +2212,8 @@ static int __add_probe_trace_events(struct perf_probe_event *pev, > > return ret; > > } > > > > -static char *looking_function_name; > > -static int num_matched_functions; > > - > > -static int probe_function_filter(struct map *map __maybe_unused, > > - struct symbol *sym) > > +#ifndef HAVE_ARCH_SYMBOL_HANDLING > > +int probe_function_filter(struct map *map __maybe_unused, struct symbol *sym) > > { > > if ((sym->binding == STB_GLOBAL || sym->binding == STB_LOCAL) && > > strcmp(looking_function_name, sym->name) == 0) { > > @@ -2223,6 +2222,7 @@ static int probe_function_filter(struct map *map __maybe_unused, > > } > > return 1; > > } > > +#endif /* HAVE_ARCH_SYMBOL_HANDLING */ > > Can't we do something like providing a weak function and let the linked > to its work? I guess we have cases like this in tools/ already. I.e. not > using the ifndef block. Minor nit tho. That sounds like a good idea. I will move these over to use __weak functions. > > > > > #define strdup_or_goto(str, label) \ > > ({ char *__p = strdup(str); if (!__p) goto label; __p; }) > > diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h > > index e01e994..8564451 100644 > > --- a/tools/perf/util/probe-event.h > > +++ b/tools/perf/util/probe-event.h > > @@ -7,6 +7,8 @@ > > #include "strfilter.h" > > > > extern bool probe_event_dry_run; > > +extern char *looking_function_name; > > +extern int num_matched_functions; > > > > > /* kprobe-tracer and uprobe-tracer tracing point */ > > struct probe_trace_point { > > @@ -136,6 +138,9 @@ extern int show_available_vars(struct perf_probe_event *pevs, int npevs, > > extern int show_available_funcs(const char *module, struct strfilter *filter, > > bool user); > > > > +extern int probe_function_filter(struct map *map __maybe_unused, > > + struct symbol *sym); > > + > > Please do not prefix function declarations with 'extern', even when we > have one just before, its not needed, patches removing the existing ones > would be accepted. Sure. - Naveen > > > /* Maximum index number of event-name postfix */ > > #define MAX_EVENT_INDEX 1024 > > > > -- > > 2.1.3 >
diff --git a/tools/perf/arch/powerpc/Makefile b/tools/perf/arch/powerpc/Makefile index 6f7782b..1c3d435 100644 --- a/tools/perf/arch/powerpc/Makefile +++ b/tools/perf/arch/powerpc/Makefile @@ -3,4 +3,5 @@ PERF_HAVE_DWARF_REGS := 1 LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/dwarf-regs.o LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/skip-callchain-idx.o endif +LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/sym-handling.o LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/header.o diff --git a/tools/perf/arch/powerpc/util/sym-handling.c b/tools/perf/arch/powerpc/util/sym-handling.c new file mode 100644 index 0000000..0a77825 --- /dev/null +++ b/tools/perf/arch/powerpc/util/sym-handling.c @@ -0,0 +1,28 @@ +/* + * Special symbol handling for PowerPC: + * - Handle dot symbols on ABIv1 + * + * Copyright (C) 2014 Naveen N Rao, IBM Corporation. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version + * 2 of the License, or (at your option) any later version. + */ + +#include "map.h" +#include "symbol.h" +#include "probe-event.h" + +int probe_function_filter(struct map *map __maybe_unused, struct symbol *sym) +{ + if (sym->binding == STB_GLOBAL || sym->binding == STB_LOCAL) { + if ((strcmp(looking_function_name, sym->name) == 0) || + (sym->name[0] == '.' && looking_function_name[0] != '.' && + strcmp(looking_function_name, sym->name+1) == 0)) { + num_matched_functions++; + return 0; + } + } + return 1; +} diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile index 5d4b039..35cf934 100644 --- a/tools/perf/config/Makefile +++ b/tools/perf/config/Makefile @@ -383,6 +383,7 @@ ifeq ($(ARCH),powerpc) ifndef NO_DWARF CFLAGS += -DHAVE_SKIP_CALLCHAIN_IDX endif + CFLAGS += -DHAVE_ARCH_SYMBOL_HANDLING endif ifndef NO_LIBUNWIND diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c index 74b7fef..7eb9b27 100644 --- a/tools/perf/util/probe-event.c +++ b/tools/perf/util/probe-event.c @@ -50,6 +50,8 @@ #define PERFPROBE_GROUP "probe" bool probe_event_dry_run; /* Dry run flag */ +char *looking_function_name; +int num_matched_functions; #define semantic_error(msg ...) pr_err("Semantic error :" msg) @@ -2210,11 +2212,8 @@ static int __add_probe_trace_events(struct perf_probe_event *pev, return ret; } -static char *looking_function_name; -static int num_matched_functions; - -static int probe_function_filter(struct map *map __maybe_unused, - struct symbol *sym) +#ifndef HAVE_ARCH_SYMBOL_HANDLING +int probe_function_filter(struct map *map __maybe_unused, struct symbol *sym) { if ((sym->binding == STB_GLOBAL || sym->binding == STB_LOCAL) && strcmp(looking_function_name, sym->name) == 0) { @@ -2223,6 +2222,7 @@ static int probe_function_filter(struct map *map __maybe_unused, } return 1; } +#endif /* HAVE_ARCH_SYMBOL_HANDLING */ #define strdup_or_goto(str, label) \ ({ char *__p = strdup(str); if (!__p) goto label; __p; }) diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h index e01e994..8564451 100644 --- a/tools/perf/util/probe-event.h +++ b/tools/perf/util/probe-event.h @@ -7,6 +7,8 @@ #include "strfilter.h" extern bool probe_event_dry_run; +extern char *looking_function_name; +extern int num_matched_functions; /* kprobe-tracer and uprobe-tracer tracing point */ struct probe_trace_point { @@ -136,6 +138,9 @@ extern int show_available_vars(struct perf_probe_event *pevs, int npevs, extern int show_available_funcs(const char *module, struct strfilter *filter, bool user); +extern int probe_function_filter(struct map *map __maybe_unused, + struct symbol *sym); + /* Maximum index number of event-name postfix */ #define MAX_EVENT_INDEX 1024
Allow perf probe to work on powerpc ABIv1 without the need to specify the leading dot '.' for functions. 'perf probe do_fork' works with this patch. Introduce HAVE_ARCH_SYMBOL_HANDLING to indicate need for special handling of symbols. In this patch, we override probe_function_filter() on powerpc to account for dot symbols. Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> --- Changes from the previous patchset: Introduced arch helper to override the way probe function filter works. tools/perf/arch/powerpc/Makefile | 1 + tools/perf/arch/powerpc/util/sym-handling.c | 28 ++++++++++++++++++++++++++++ tools/perf/config/Makefile | 1 + tools/perf/util/probe-event.c | 10 +++++----- tools/perf/util/probe-event.h | 5 +++++ 5 files changed, 40 insertions(+), 5 deletions(-) create mode 100644 tools/perf/arch/powerpc/util/sym-handling.c