* [PATCH] perf tools: Support bpf prologue for arm64 @ 2015-08-29 3:16 He Kuang 2015-08-31 20:16 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 5+ messages in thread From: He Kuang @ 2015-08-29 3:16 UTC (permalink / raw) To: wangnan0, a.p.zijlstra, mingo, acme, jolsa; +Cc: linux-kernel This patch implements arch_get_reg_info() for arm64 to enable HAVE_BPF_PROLOGUE feature. For arm64, structure pt_regs is not composed by fields of register names but an array of regs, so here we simply multiply fixed register size by index number to get the byte offset. Signed-off-by: He Kuang <hekuang@huawei.com> --- tools/perf/arch/arm64/Makefile | 1 + tools/perf/arch/arm64/util/dwarf-regs.c | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/tools/perf/arch/arm64/Makefile b/tools/perf/arch/arm64/Makefile index 7fbca17..1256e6e 100644 --- a/tools/perf/arch/arm64/Makefile +++ b/tools/perf/arch/arm64/Makefile @@ -1,3 +1,4 @@ ifndef NO_DWARF PERF_HAVE_DWARF_REGS := 1 endif +PERF_HAVE_ARCH_GET_REG_INFO := 1 diff --git a/tools/perf/arch/arm64/util/dwarf-regs.c b/tools/perf/arch/arm64/util/dwarf-regs.c index d49efeb..cb2c50a 100644 --- a/tools/perf/arch/arm64/util/dwarf-regs.c +++ b/tools/perf/arch/arm64/util/dwarf-regs.c @@ -10,6 +10,10 @@ #include <stddef.h> #include <dwarf-regs.h> +#include <string.h> +#include <linux/ptrace.h> + +#define PT_REG_SIZE (sizeof(((struct user_pt_regs *)0)->regs[0])) struct pt_regs_dwarfnum { const char *name; @@ -78,3 +82,25 @@ const char *get_arch_regstr(unsigned int n) return roff->name; return NULL; } + +#ifdef HAVE_BPF_PROLOGUE +int arch_get_reg_info(const char *name, int *offset) +{ + const struct pt_regs_dwarfnum *roff; + + if (!name || !offset) + return -1; + + for (roff = regdwarfnum_table; roff->name != NULL; roff++) { + if (!strcmp(roff->name, name)) { + if (roff->dwarfnum < 0) + return -1; + + *offset = roff->dwarfnum * PT_REG_SIZE; + return 0; + } + } + + return -1; +} +#endif -- 1.8.5.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] perf tools: Support bpf prologue for arm64 2015-08-29 3:16 [PATCH] perf tools: Support bpf prologue for arm64 He Kuang @ 2015-08-31 20:16 ` Arnaldo Carvalho de Melo 2015-09-01 2:04 ` He Kuang 2015-09-02 10:34 ` Will Deacon 0 siblings, 2 replies; 5+ messages in thread From: Arnaldo Carvalho de Melo @ 2015-08-31 20:16 UTC (permalink / raw) To: Jean Pihet, Will Deacon Cc: He Kuang, wangnan0, a.p.zijlstra, mingo, jolsa, linux-kernel Em Sat, Aug 29, 2015 at 03:16:52AM +0000, He Kuang escreveu: > This patch implements arch_get_reg_info() for arm64 to enable > HAVE_BPF_PROLOGUE feature. For arm64, structure pt_regs is not composed > by fields of register names but an array of regs, so here we simply > multiply fixed register size by index number to get the byte offset. Hi Jean, Will, are you ok with this? Can I have Acked-by or Reviewed-by tags from you? He, please try to add the authors of the files you change in the CC list. - Arnaldo > Signed-off-by: He Kuang <hekuang@huawei.com> > --- > tools/perf/arch/arm64/Makefile | 1 + > tools/perf/arch/arm64/util/dwarf-regs.c | 26 ++++++++++++++++++++++++++ > 2 files changed, 27 insertions(+) > > diff --git a/tools/perf/arch/arm64/Makefile b/tools/perf/arch/arm64/Makefile > index 7fbca17..1256e6e 100644 > --- a/tools/perf/arch/arm64/Makefile > +++ b/tools/perf/arch/arm64/Makefile > @@ -1,3 +1,4 @@ > ifndef NO_DWARF > PERF_HAVE_DWARF_REGS := 1 > endif > +PERF_HAVE_ARCH_GET_REG_INFO := 1 > diff --git a/tools/perf/arch/arm64/util/dwarf-regs.c b/tools/perf/arch/arm64/util/dwarf-regs.c > index d49efeb..cb2c50a 100644 > --- a/tools/perf/arch/arm64/util/dwarf-regs.c > +++ b/tools/perf/arch/arm64/util/dwarf-regs.c > @@ -10,6 +10,10 @@ > > #include <stddef.h> > #include <dwarf-regs.h> > +#include <string.h> > +#include <linux/ptrace.h> > + > +#define PT_REG_SIZE (sizeof(((struct user_pt_regs *)0)->regs[0])) > > struct pt_regs_dwarfnum { > const char *name; > @@ -78,3 +82,25 @@ const char *get_arch_regstr(unsigned int n) > return roff->name; > return NULL; > } > + > +#ifdef HAVE_BPF_PROLOGUE > +int arch_get_reg_info(const char *name, int *offset) > +{ > + const struct pt_regs_dwarfnum *roff; > + > + if (!name || !offset) > + return -1; > + > + for (roff = regdwarfnum_table; roff->name != NULL; roff++) { > + if (!strcmp(roff->name, name)) { > + if (roff->dwarfnum < 0) > + return -1; > + > + *offset = roff->dwarfnum * PT_REG_SIZE; > + return 0; > + } > + } > + > + return -1; > +} > +#endif > -- > 1.8.5.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] perf tools: Support bpf prologue for arm64 2015-08-31 20:16 ` Arnaldo Carvalho de Melo @ 2015-09-01 2:04 ` He Kuang 2015-09-02 10:34 ` Will Deacon 1 sibling, 0 replies; 5+ messages in thread From: He Kuang @ 2015-09-01 2:04 UTC (permalink / raw) To: Arnaldo Carvalho de Melo, Jean Pihet, Will Deacon Cc: wangnan0, a.p.zijlstra, mingo, jolsa, linux-kernel Hi, Arnaldo On 2015/9/1 4:16, Arnaldo Carvalho de Melo wrote: > Em Sat, Aug 29, 2015 at 03:16:52AM +0000, He Kuang escreveu: >> This patch implements arch_get_reg_info() for arm64 to enable >> HAVE_BPF_PROLOGUE feature. For arm64, structure pt_regs is not composed >> by fields of register names but an array of regs, so here we simply >> multiply fixed register size by index number to get the byte offset. > > Hi Jean, Will, are you ok with this? Can I have Acked-by or Reviewed-by > tags from you? > > He, please try to add the authors of the files you change in the CC > list. Ok, usually I use the get_maintainers.pl script, but it seems that the output do not include file authors: ./scripts/get_maintainer.pl 0001-perf-tools-Support-bpf-prologue-for-arm64.patch Peter Zijlstra <a.p.zijlstra@chello.nl> (supporter:PERFORMANCE EVENTS SUBSYSTEM) Ingo Molnar <mingo@redhat.com> (supporter:PERFORMANCE EVENTS SUBSYSTEM) Arnaldo Carvalho de Melo <acme@kernel.org> (supporter:PERFORMANCE EVENTS SUBSYSTEM,commit_signer:1/2=50%) Jiri Olsa <jolsa@kernel.org> (commit_signer:1/2=50%,authored:1/2=50%,removed_lines:4/4=100%) He Kuang <hekuang@huawei.com> (commit_signer:1/2=50%,authored:1/2=50%,added_lines:1/1=100%,commit_signer:1/1=100%,authored:1/1=100%,added_lines:23/23=100%) linux-kernel@vger.kernel.org (open list:PERFORMANCE EVENTS SUBSYSTEM) get_maintainers.pl bug? > > - Arnaldo > >> Signed-off-by: He Kuang <hekuang@huawei.com> >> --- >> tools/perf/arch/arm64/Makefile | 1 + >> tools/perf/arch/arm64/util/dwarf-regs.c | 26 ++++++++++++++++++++++++++ >> 2 files changed, 27 insertions(+) >> >> diff --git a/tools/perf/arch/arm64/Makefile b/tools/perf/arch/arm64/Makefile >> index 7fbca17..1256e6e 100644 >> --- a/tools/perf/arch/arm64/Makefile >> +++ b/tools/perf/arch/arm64/Makefile >> @@ -1,3 +1,4 @@ >> ifndef NO_DWARF >> PERF_HAVE_DWARF_REGS := 1 >> endif >> +PERF_HAVE_ARCH_GET_REG_INFO := 1 >> diff --git a/tools/perf/arch/arm64/util/dwarf-regs.c b/tools/perf/arch/arm64/util/dwarf-regs.c >> index d49efeb..cb2c50a 100644 >> --- a/tools/perf/arch/arm64/util/dwarf-regs.c >> +++ b/tools/perf/arch/arm64/util/dwarf-regs.c >> @@ -10,6 +10,10 @@ >> >> #include <stddef.h> >> #include <dwarf-regs.h> >> +#include <string.h> >> +#include <linux/ptrace.h> >> + >> +#define PT_REG_SIZE (sizeof(((struct user_pt_regs *)0)->regs[0])) >> >> struct pt_regs_dwarfnum { >> const char *name; >> @@ -78,3 +82,25 @@ const char *get_arch_regstr(unsigned int n) >> return roff->name; >> return NULL; >> } >> + >> +#ifdef HAVE_BPF_PROLOGUE >> +int arch_get_reg_info(const char *name, int *offset) >> +{ >> + const struct pt_regs_dwarfnum *roff; >> + >> + if (!name || !offset) >> + return -1; >> + >> + for (roff = regdwarfnum_table; roff->name != NULL; roff++) { >> + if (!strcmp(roff->name, name)) { >> + if (roff->dwarfnum < 0) >> + return -1; Here's a compile error. error: comparison of unsigned expression < 0 is always false [-Werror=type-limits] So remove this useless comparison: --- tools/perf/arch/arm64/Makefile | 1 + tools/perf/arch/arm64/util/dwarf-regs.c | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/tools/perf/arch/arm64/Makefile b/tools/perf/arch/arm64/Makefile index 7fbca17..1256e6e 100644 --- a/tools/perf/arch/arm64/Makefile +++ b/tools/perf/arch/arm64/Makefile @@ -1,3 +1,4 @@ ifndef NO_DWARF PERF_HAVE_DWARF_REGS := 1 endif +PERF_HAVE_ARCH_GET_REG_INFO := 1 diff --git a/tools/perf/arch/arm64/util/dwarf-regs.c b/tools/perf/arch/arm64/util/dwarf-regs.c index d49efeb..cb935c4 100644 --- a/tools/perf/arch/arm64/util/dwarf-regs.c +++ b/tools/perf/arch/arm64/util/dwarf-regs.c @@ -10,6 +10,10 @@ #include <stddef.h> #include <dwarf-regs.h> +#include <string.h> +#include <linux/ptrace.h> + +#define PT_REG_SIZE (sizeof(((struct user_pt_regs *)0)->regs[0])) struct pt_regs_dwarfnum { const char *name; @@ -78,3 +82,22 @@ const char *get_arch_regstr(unsigned int n) return roff->name; return NULL; } + +#ifdef HAVE_BPF_PROLOGUE +int arch_get_reg_info(const char *name, int *offset) +{ + const struct pt_regs_dwarfnum *roff; + + if (!name || !offset) + return -1; + + for (roff = regdwarfnum_table; roff->name != NULL; roff++) { + if (!strcmp(roff->name, name)) { + *offset = roff->dwarfnum * PT_REG_SIZE; + return 0; + } + } + + return -1; +} +#endif -- 1.8.5.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] perf tools: Support bpf prologue for arm64 2015-08-31 20:16 ` Arnaldo Carvalho de Melo 2015-09-01 2:04 ` He Kuang @ 2015-09-02 10:34 ` Will Deacon 2015-09-02 10:50 ` Wangnan (F) 1 sibling, 1 reply; 5+ messages in thread From: Will Deacon @ 2015-09-02 10:34 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Jean Pihet, He Kuang, wangnan0@huawei.com, a.p.zijlstra@chello.nl, mingo@redhat.com, jolsa@kernel.org, linux-kernel@vger.kernel.org On Mon, Aug 31, 2015 at 09:16:28PM +0100, Arnaldo Carvalho de Melo wrote: > Em Sat, Aug 29, 2015 at 03:16:52AM +0000, He Kuang escreveu: > > This patch implements arch_get_reg_info() for arm64 to enable > > HAVE_BPF_PROLOGUE feature. For arm64, structure pt_regs is not composed > > by fields of register names but an array of regs, so here we simply > > multiply fixed register size by index number to get the byte offset. > > Hi Jean, Will, are you ok with this? Can I have Acked-by or Reviewed-by > tags from you? Any idea what this applies against? It's difficult to review it without knowing what PERF_HAVE_ARCH_GET_REG_INFO or HAVE_BPF_PROLOGUE are. Anyway, a couple of small comments below. > He, please try to add the authors of the files you change in the CC > list. > > - Arnaldo > > > Signed-off-by: He Kuang <hekuang@huawei.com> > > --- > > tools/perf/arch/arm64/Makefile | 1 + > > tools/perf/arch/arm64/util/dwarf-regs.c | 26 ++++++++++++++++++++++++++ > > 2 files changed, 27 insertions(+) Any plan to do arch/arm/ too? > > diff --git a/tools/perf/arch/arm64/Makefile b/tools/perf/arch/arm64/Makefile > > index 7fbca17..1256e6e 100644 > > --- a/tools/perf/arch/arm64/Makefile > > +++ b/tools/perf/arch/arm64/Makefile > > @@ -1,3 +1,4 @@ > > ifndef NO_DWARF > > PERF_HAVE_DWARF_REGS := 1 > > endif > > +PERF_HAVE_ARCH_GET_REG_INFO := 1 > > diff --git a/tools/perf/arch/arm64/util/dwarf-regs.c b/tools/perf/arch/arm64/util/dwarf-regs.c > > index d49efeb..cb2c50a 100644 > > --- a/tools/perf/arch/arm64/util/dwarf-regs.c > > +++ b/tools/perf/arch/arm64/util/dwarf-regs.c > > @@ -10,6 +10,10 @@ > > > > #include <stddef.h> > > #include <dwarf-regs.h> > > +#include <string.h> > > +#include <linux/ptrace.h> > > + > > +#define PT_REG_SIZE (sizeof(((struct user_pt_regs *)0)->regs[0])) Why not add an "offset" field to pt_regs_dwarfnum and just calculate that using offsetof in the GPR_DWARFNUM_NAME macro? > > > > struct pt_regs_dwarfnum { > > const char *name; > > @@ -78,3 +82,25 @@ const char *get_arch_regstr(unsigned int n) > > return roff->name; > > return NULL; > > } > > + > > +#ifdef HAVE_BPF_PROLOGUE > > +int arch_get_reg_info(const char *name, int *offset) > > +{ > > + const struct pt_regs_dwarfnum *roff; > > + > > + if (!name || !offset) > > + return -1; > > + > > + for (roff = regdwarfnum_table; roff->name != NULL; roff++) { > > + if (!strcmp(roff->name, name)) { > > + if (roff->dwarfnum < 0) When is this ever true? Do we need up update REG_DWARFNUM_END to use a negative index? Will ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] perf tools: Support bpf prologue for arm64 2015-09-02 10:34 ` Will Deacon @ 2015-09-02 10:50 ` Wangnan (F) 0 siblings, 0 replies; 5+ messages in thread From: Wangnan (F) @ 2015-09-02 10:50 UTC (permalink / raw) To: Will Deacon, Arnaldo Carvalho de Melo Cc: Jean Pihet, He Kuang, a.p.zijlstra@chello.nl, mingo@redhat.com, jolsa@kernel.org, linux-kernel@vger.kernel.org On 2015/9/2 18:34, Will Deacon wrote: > On Mon, Aug 31, 2015 at 09:16:28PM +0100, Arnaldo Carvalho de Melo wrote: >> Em Sat, Aug 29, 2015 at 03:16:52AM +0000, He Kuang escreveu: >>> This patch implements arch_get_reg_info() for arm64 to enable >>> HAVE_BPF_PROLOGUE feature. For arm64, structure pt_regs is not composed >>> by fields of register names but an array of regs, so here we simply >>> multiply fixed register size by index number to get the byte offset. >> Hi Jean, Will, are you ok with this? Can I have Acked-by or Reviewed-by >> tags from you? > Any idea what this applies against? It's difficult to review it without > knowing what PERF_HAVE_ARCH_GET_REG_INFO or HAVE_BPF_PROLOGUE are. > > Anyway, a couple of small comments below. > >> He, please try to add the authors of the files you change in the CC >> list. >> >> - Arnaldo >> >>> Signed-off-by: He Kuang <hekuang@huawei.com> >>> --- >>> tools/perf/arch/arm64/Makefile | 1 + >>> tools/perf/arch/arm64/util/dwarf-regs.c | 26 ++++++++++++++++++++++++++ >>> 2 files changed, 27 insertions(+) > Any plan to do arch/arm/ too? > >>> diff --git a/tools/perf/arch/arm64/Makefile b/tools/perf/arch/arm64/Makefile >>> index 7fbca17..1256e6e 100644 >>> --- a/tools/perf/arch/arm64/Makefile >>> +++ b/tools/perf/arch/arm64/Makefile >>> @@ -1,3 +1,4 @@ >>> ifndef NO_DWARF >>> PERF_HAVE_DWARF_REGS := 1 >>> endif >>> +PERF_HAVE_ARCH_GET_REG_INFO := 1 >>> diff --git a/tools/perf/arch/arm64/util/dwarf-regs.c b/tools/perf/arch/arm64/util/dwarf-regs.c >>> index d49efeb..cb2c50a 100644 >>> --- a/tools/perf/arch/arm64/util/dwarf-regs.c >>> +++ b/tools/perf/arch/arm64/util/dwarf-regs.c >>> @@ -10,6 +10,10 @@ >>> >>> #include <stddef.h> >>> #include <dwarf-regs.h> >>> +#include <string.h> >>> +#include <linux/ptrace.h> >>> + >>> +#define PT_REG_SIZE (sizeof(((struct user_pt_regs *)0)->regs[0])) > Why not add an "offset" field to pt_regs_dwarfnum and just calculate > that using offsetof in the GPR_DWARFNUM_NAME macro? > >>> >>> struct pt_regs_dwarfnum { >>> const char *name; >>> @@ -78,3 +82,25 @@ const char *get_arch_regstr(unsigned int n) >>> return roff->name; >>> return NULL; >>> } >>> + >>> +#ifdef HAVE_BPF_PROLOGUE >>> +int arch_get_reg_info(const char *name, int *offset) >>> +{ >>> + const struct pt_regs_dwarfnum *roff; >>> + >>> + if (!name || !offset) >>> + return -1; >>> + >>> + for (roff = regdwarfnum_table; roff->name != NULL; roff++) { >>> + if (!strcmp(roff->name, name)) { >>> + if (roff->dwarfnum < 0) > When is this ever true? Do we need up update REG_DWARFNUM_END to use > a negative index? > > Will Thank you for your review. Yesterday Arnaldo gave some comment on corresponding function on x86_64, please see discussion in following two threads: http//lkml.kernel.org/n/20150831204328.GE2443@redhat.com http://lkml.kernel.org/n/1441090789-108382-2-git-send-email-wangnan0@huawei.com The conclusion is we should try out best to reuse kernel's regs_query_register_offset() function, including providing same name and API and reusing its code. However, we should avoid use it directly from kernel directory. Instead, we should copy them to perf's directory. Therefore, He Kuang needs to redo his work after a while. Any suggestion about code reusing? For ARM: I don't think this is a hard work. In fact we have tested our perf BPF code on an ARM board. Haven't provided this because we haven't meet a real profiling task on ARM which requires fetching argument from BPF program. Thank you. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-09-02 10:58 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-08-29 3:16 [PATCH] perf tools: Support bpf prologue for arm64 He Kuang 2015-08-31 20:16 ` Arnaldo Carvalho de Melo 2015-09-01 2:04 ` He Kuang 2015-09-02 10:34 ` Will Deacon 2015-09-02 10:50 ` Wangnan (F)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox