Re: [PATCH 1/2] perf/powerpc: Fix kprobe and kretprobe handling with kallsyms

[Naveen N. Rao]

On 2016/04/07 10:00AM, Ananth N wrote:
> On Wed, Apr 06, 2016 at 06:02:57PM +0530, Naveen N. Rao wrote:
> 
> > +   if (!pev->uprobes && map->dso->symtab_type == DSO_BINARY_TYPE__KALLSYMS)
> > tev->point.offset += PPC64LE_LEP_OFFSET;
> 
> uprobes check against kallsysms? Am I missing something here?

Ah yes. That check shouldn't be necessary since symtab_type would be 
different anyway. I will remove that check.

Thanks for the review!
- Naveen

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 1/2] perf/powerpc: Fix kprobe and kretprobe handling with kallsyms

[Ananth N Mavinakayanahalli]

On Wed, Apr 06, 2016 at 06:02:57PM +0530, Naveen N. Rao wrote:

> + if (!pev->uprobes && map->dso->symtab_type == DSO_BINARY_TYPE__KALLSYMS)
>   tev->point.offset += PPC64LE_LEP_OFFSET;

uprobes check against kallsysms? Am I missing something here?

Ananth

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 1/2] perf/powerpc: Fix kprobe and kretprobe handling with kallsyms

[Naveen N. Rao]

So far, we used to treat probe point offsets as being offset from the
LEP. However, userspace applications (objdump/readelf) always show
disassembly and offsets from the function GEP. This is confusing to the
user as we will end up probing at an address different from what the
user expects when looking at the function disassembly with
readelf/objdump. Fix this by changing how we modify probe address with
perf.

If only the function name is provided, we assume the user needs the LEP.
Otherwise, if an offset is specified, we assume that the user knows the
exact address to probe based on function disassembly, and so we just
place the probe from the GEP offset.

Finally, kretprobe was also broken with kallsyms as we were trying to
specify an offset. This patch also fixes that issue.

Cc: Mark Wielaard 
Cc: Thiago Jung Bauermann 
Cc: Arnaldo Carvalho de Melo 
Cc: Masami Hiramatsu 
Cc: Michael Ellerman 
Reported-by: Ananth N Mavinakayanahalli 
Signed-off-by: Naveen N. Rao 
---
 tools/perf/arch/powerpc/util/sym-handling.c | 19 ++-
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/tools/perf/arch/powerpc/util/sym-handling.c 
b/tools/perf/arch/powerpc/util/sym-handling.c
index bbc1a50..c5b4756 100644
--- a/tools/perf/arch/powerpc/util/sym-handling.c
+++ b/tools/perf/arch/powerpc/util/sym-handling.c
@@ -71,12 +71,21 @@ void arch__fix_tev_from_maps(struct perf_probe_event *pev,
 struct probe_trace_event *tev, struct map *map)
 {
/*
-* ppc64 ABIv2 local entry point is currently always 2 instructions
-* (8 bytes) after the global entry point.
+* When probing at a function entry point, we normally always want the
+* LEP since that catches calls to the function through both the GEP and
+* the LEP. Hence, we would like to probe at an offset of 8 bytes if
+* the user only specified the function entry.
+*
+* However, if the user specifies an offset, we fall back to using the
+* GEP since all userspace applications (objdump/readelf) show function
+* disassembly with offsets from the GEP.
+*
+* In addition, we shouldn't specify an offset for kretprobes.
 */
-   if (!pev->uprobes && map->dso->symtab_type == 
DSO_BINARY_TYPE__KALLSYMS) {
-   tev->point.address += PPC64LE_LEP_OFFSET;
+   if (pev->point.offset || pev->point.retprobe)
+   return;
+
+   if (!pev->uprobes && map->dso->symtab_type == DSO_BINARY_TYPE__KALLSYMS)
tev->point.offset += PPC64LE_LEP_OFFSET;
-   }
 }
 #endif
-- 
2.7.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev