Message-ID: <4B1FE022.1030704@redhat.com>
Date:	Wed, 09 Dec 2009 12:36:34 -0500
From:	Masami Hiramatsu <mhiramat@...hat.com>
To:	Ingo Molnar <mingo@...e.hu>
CC:	Frederic Weisbecker <fweisbec@...il.com>,
	lkml <linux-kernel@...r.kernel.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	Jim Keniston <jkenisto@...ibm.com>,
	Ananth N Mavinakayanahalli <ananth@...ibm.com>,
	Christoph Hellwig <hch@...radead.org>,
	"Frank Ch. Eigler" <fche@...hat.com>,
	Jason Baron <jbaron@...hat.com>,
	"K.Prasad" <prasad@...ux.vnet.ibm.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Srikar Dronamraju <srikar@...ux.vnet.ibm.com>,
	Arnaldo Carvalho de Melo <acme@...hat.com>,
	systemtap <systemtap@...rces.redhat.com>,
	DLE <dle-develop@...ts.sourceforge.net>
Subject: Re: [PATCH -tip 0/8] perf-probe updates

Hi Ingo,

Thank you for error reporting :-)

Ingo Molnar wrote:
> There's another small hickup i had - when i typoed 'perf probe -', it
> gave me:
>
>   # perf probe -
>   No dwarf info found in the vmlinux - please rebuild with CONFIG_DEBUG_INFO.
>   An error occurred in debuginfo analysis. Try to use symbols.
>     Fatal: Failed to write event: Invalid argument

OK, perhaps, you are expecting "perf probe -" works similar to "perf record -",
but that's not implemented yet...
Anyway, I think "perf probe -" should show a usage and return.


> Similar thing happens if i try to probe a non-existent symbol:
>
>   # perf probe test
>   No dwarf info found in the vmlinux - please rebuild with CONFIG_DEBUG_INFO.
>   An error occurred in debuginfo analysis. Try to use symbols.
>     Fatal: Failed to write event: Invalid argument
>
> I think we should print something more helpful, such as:
>
>   # perf probe test
>   Fatal: Kernel symbol 'test' not found - probe not added.
>
> the debuginfo printout is not helpful in this case - we should fall back
> to symbols silently, unless the nature of the error indicates that we
> fail _because_ there's no debuginfo.

OK.

> Here the failure was because the symbol does not exist.

Yeah, so that's what I'm expecting to be implemented with below item :-)

   - Symbol search by libelf/kallsyms

I guess it will be done by using symbol.c.
(Actually, current

781 221 7109

>
> There's similar problems in most other failure cases. Trying to remove a
> non-existent probe gives:
>
>   # perf probe -d test
>   Warning: event "probe:test" is not found.
>
> It should say something like:
>
>   # perf probe -d test
>   Info: event "probe:test" does not exist, could not remove it.

Sure.

> Also, it's possible to add multiple probes to the same function, using
> 'perf probe schedule' + 'perf probe schedule', etc. While in general it
> makes sense to allow it, by default we should refuse the second,
> identical probe on the symbol - and add a -f/--force option to force
> duplicate probes. I.e. the second probe should print:
>
>   # perf probe schedule
>   Info: event "probe:schedule" already exists. (Use -f to force a duplicate.)
>
> etc.

Hmm, if you mean 'refusing the second issued command',
it might be good, because it can avoid to add events with
unwilling names.

However, I don't think it's so useful generally, because
some code lines can be expanded to several addresses.
In that case, we need to add several events at once,
and each event has different names.
e.g.

# ./perf probe schedule:10
Added new event:
   probe:schedule                           (on schedule+1)
Added new event:
   probe:schedule_1                         (on schedule+19)
Added new event:
   probe:schedule_2                         (on schedule+19)
Added new event:
   probe:schedule_3                         (on schedule+28)
Added new event:
   probe:schedule_4                         (on schedule+38)

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

         perf record -e probe:schedule_4 -a sleep 1
---
Same things happens when probing inlined functions.

I guess this issue might be solved by below items;
   - Support glob expression with --del option (like --del "*")
   - Support event/group name specifying for new events

e.g.
Adding new events with another name.

# perf probe --add mygroup:sched_enter=schedule
Added new event:
   mygroup:sched_enter                      (on schedule+0)

# perf probe --add mygroup:sched_event=schedule:10
Added new event:
   mygroup:sched_event                      (on schedule+1)
Added new event:
   mygroup:sched_event_1                    (on schedule+19)
Added new event:
   mygroup:sched_event_2                    (on schedule+19)
Added new event:
   mygroup:sched_event_3                    (on schedule+28)
Added new event:
   mygroup:sched_event_4                    (on schedule+38)

And record it by glob specifying.

# perf record -e mygroup:sched_event* -a sleep 1

or

# perf record -e mygroup:sched_enter* -a sleep 1

What would you think about it?

> Please try out various sensible and also less sensible options of
> this tool and try to make it break - and see whether the behavior is
> intuitive and obvious to users - whether the messages are consistent,
> etc. etc.

OK, actually, perf-probe needs to be brushed up. Any comments, reports
and opinions are welcome. I'd like to update todo list, based on what
is useful and which is important for users.

Thank you,

>
> 	Ingo

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhiramat@...hat.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists