Re: [BUGFIX PATCH 1/6] perf/probe: Fix wrong address verification

[Arnaldo Carvalho de Melo]

Em Fri, Oct 25, 2019 at 09:36:33PM +0900, Masami Hiramatsu escreveu:
> Hi,
> 
> On Fri, 25 Oct 2019 09:14:48 -0300
> Arnaldo Carvalho de Melo <acme@xxxxxxxxxx> wrote:
> 
> > Em Fri, Oct 25, 2019 at 05:46:25PM +0900, Masami Hiramatsu escreveu:
> > > Since there are some DIE which has only ranges instead of the
> > > combination of entrypc/highpc, address verification must use
> > > dwarf_haspc() instead of dwarf_entrypc/dwarf_highpc.
> > > 
> > > Also, the ranges only DIE will have a partial code in different
> > > section (e.g. unlikely code will be in text.unlikely as "FUNC.cold"
> > > symbol). In that case, we can not use dwarf_entrypc() or
> > > die_entrypc(), because the offset from original DIE can be
> > > a minus value.
> > > 
> > > Instead, this simply gets the symbol and offset from symtab.
> > > 
> > > Without this patch;
> > >   # tools/perf/perf probe -D clear_tasks_mm_cpumask:1
> > >   Failed to get entry address of clear_tasks_mm_cpumask
> > >     Error: Failed to add events.
> > > 
> > > And with this patch
> > >   # tools/perf/perf probe -D clear_tasks_mm_cpumask:1
> > >   p:probe/clear_tasks_mm_cpumask clear_tasks_mm_cpumask+0
> > >   p:probe/clear_tasks_mm_cpumask_1 clear_tasks_mm_cpumask+5
> > >   p:probe/clear_tasks_mm_cpumask_2 clear_tasks_mm_cpumask+8
> > >   p:probe/clear_tasks_mm_cpumask_3 clear_tasks_mm_cpumask+16
> > >   p:probe/clear_tasks_mm_cpumask_4 clear_tasks_mm_cpumask+82
> > 
> > Ok, so this just asks for the definition, but doesn't try to actually
> > _use_ it, which I did and it fails:
> > 
> > [root@quaco tracebuffer]# perf probe -D clear_tasks_mm_cpumask:1
> > p:probe/clear_tasks_mm_cpumask _text+919968
> > p:probe/clear_tasks_mm_cpumask_1 _text+919973
> > p:probe/clear_tasks_mm_cpumask_2 _text+919976
> > [root@quaco tracebuffer]#
> > [root@quaco tracebuffer]# perf probe clear_tasks_mm_cpumask
> > Probe point 'clear_tasks_mm_cpumask' not found.
> >   Error: Failed to add events.
> > [root@quaco tracebuffer]#
> > 
> > So I'll tentatively continue to apply the other patches in this series,
> > maybe one of them will fix this.
> 
> Yes, it should be fixed by [2/6] :)

Yeah, it is, works there, but unfortunately I couldn't see it in action
even having put it successfuly into place:

[root@quaco ~]# perf probe clear_tasks_mm_cpumask:0
Added new event:
  probe:clear_tasks_mm_cpumask (on clear_tasks_mm_cpumask)

You can now use it in all perf tools, such as:

	perf record -e probe:clear_tasks_mm_cpumask -aR sleep 1

[root@quaco ~]# perf trace -e probe:clear_tasks_mm_cpumask/max-stack=16/

Doesn't seem to be used in x86-64...

[acme@quaco perf]$ find . -name "*.c" | xargs grep clear_tasks_mm_cpumask
./kernel/cpu.c: * clear_tasks_mm_cpumask - Safely clear tasks' mm_cpumask for a CPU
./kernel/cpu.c:void clear_tasks_mm_cpumask(int cpu)
./arch/xtensa/kernel/smp.c:	clear_tasks_mm_cpumask(cpu);
./arch/csky/kernel/smp.c:	clear_tasks_mm_cpumask(cpu);
./arch/sh/kernel/smp.c:	clear_tasks_mm_cpumask(cpu);
./arch/arm/kernel/smp.c:	clear_tasks_mm_cpumask(cpu);
./arch/powerpc/mm/nohash/mmu_context.c:	clear_tasks_mm_cpumask(cpu);
[acme@quaco perf]$ find . -name "*.h" | xargs grep clear_tasks_mm_cpumask
./include/linux/cpu.h:void clear_tasks_mm_cpumask(int cpu);
[acme@quaco perf]$

:-)
 
> Actually, a probe point with offset line and no-offset are handled
> a bit differently.

What is the difference? Anyway, things seem to work as advertised, so
I'm happy ;-)

- Arnaldo