Message-ID: <20091020065055.GL8550@elte.hu>
Date:	Tue, 20 Oct 2009 08:50:55 +0200
From:	Ingo Molnar <mingo@...e.hu>
To:	Masami Hiramatsu <mhiramat@...hat.com>
Cc:	Frederic Weisbecker <fweisbec@...il.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	lkml <linux-kernel@...r.kernel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Arnaldo Carvalho de Melo <acme@...hat.com>,
	Mike Galbraith <efault@....de>,
	Paul Mackerras <paulus@...ba.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Christoph Hellwig <hch@...radead.org>,
	Ananth N Mavinakayanahalli <ananth@...ibm.com>,
	Jim Keniston <jkenisto@...ibm.com>,
	"Frank Ch. Eigler" <fche@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	systemtap <systemtap@...rces.redhat.com>,
	DLE <dle-develop@...ts.sourceforge.net>
Subject: Re: [PATCH -tip tracing/kprobes 0/9] tracing/kprobes, perf: perf
	probe and kprobe-tracer bugfixes


* Masami Hiramatsu <mhiramat@...hat.com> wrote:

> Ah, I've gotten what you said, and many lines will be optimized out it 
> would be better to show line numbers which can be probed, as below.
> 
> 000	asmlinkage void __sched schedule(void)
> 	{
> 		struct task_struct *prev, *next;
> 		unsigned long *switch_count;
> 		struct rq *rq;
> 		int cpu;
> 
> 007	need_resched:
> 008		preempt_disable();
> 009		cpu = smp_processor_id();
> 010		rq = cpu_rq(cpu);
> 011		rcu_sched_qs(cpu);
> 012		prev = rq->curr;
> 013		switch_count = &prev->nivcsw;
> 
> 015		release_kernel_lock(prev);

Agreed, that's a very good idea!

( You could also make use of color_fprintf() to color-code some of the 
  numbers - for example lines with existing probes might be marked red. )

> > That way the following two are equivalent:
> >
> >   perf probe schedule
> >   perf probe schedule+0
> >
> > The advantage of relative line numbers is that they are much more
> > version invariant than absolute line numbers. Relative line numbers into
> > schedule() will only change if the function itself changes.
> >
> > This means that expressions like 'schedule+16' will have a lot longer
> > life-time than absolute line number driven probes. You can quote it in
> > an email and chances are that it will still be valid even a few kernel
> > releases down the road.
> 
> Hmm, I imagines 'schedule() + 16 byte offset' from 'schedule+16', 
> because many in-kernel(and oops) messages means that. :-) So I'd like 
> to use 'schedule:16'.

kernel symbol addresses are in the form of schedule+0x16/0x510.

I like the plus sign because it also allows the negative positions: 
relative position from the _end_ of the function. So for example 
'schedule-1' would probe the (last) return line of the function. 
[admittedly this is not as useful as the + operation though.]

But schedule:16 [and schedule:+16] would be fine too i suspect.

> >>> We also want to have functionality that helps people find probe
> >>> spots within a function:
> >>>
> >>>   perf probe --list-lines schedule
> >>>
> >>> Would list the line numbers and source code of the schedule()
> >>> function. (similar to how GDB 'list' works) That way someone can
> >>> have an ad-hoc session of deciding what place to probe, and the line
> >>> numbers make for an easy ID of the statement to probe.
> >>
> >> Agreed!
> >
> > Furthermore - to answer another question you raised above - the
> > following syntax:
> >
> >    perf probe schedule:'switch_count = &prev->nivcsw'
> >
> > is basically a 'fuzzy string match' based probe to within a function.
> >
> > For example you might want to probe the point within schedule that calls
> > switch_mm() - this could be done via:
> >
> >    perf probe schedule@...tch_mm
> >
> > Or the point where 'next' gets assigned? Sure, you dont need to even
> > open the editor, if you know the rough outline of the function you can
> > probe it via:
> >
> >    perf probe schedule@...xt ='
> >
> > Note that i was able to specify both probes without having opened an
> > editor - just based on the general knowledge of the scheduler.
> 
> It may be useful for return probe too :-)
> 
>  perf probe schedule@...urn
> 
> > The point is to prefer intuitive, non-mechanic, fundamentally human
> > expressions of information above mechanic ones (absolute line numbers,
> > addresses, ways of probing, etc.) - and to have a rich variety of them.
> >
> > String based pattern matching and intuitive syntax that reuses existing
> > paradigms of arithmetics and pattern-matching is good - limited syntax
> > and extra, arbitrary syntactic hoops to jump through is bad.
> >
> > If we provide all that, people will start using this stuff - and i'd
> > only like to merge this upstream once it's clear that people like me
> > will (be able to) use this facility for ad-hoc probe insertion.
> >
> > In other words: this facility has to 'live within' our source code and
> > has to be able to interact with it on a very broad basis - for it to be
> > maximally useful for everyday development.
> 
> Hmm, so you mean perf-probe should work with source-code? Without 
> source code (but with debuginfo), maybe we can't use string matching, 
> is that OK?

Well most forms of debuginfo embedd the full source code in the 
debuginfo, right? If it's not there (or we dont know where it is) then 
we cannot use it, obviously.

But we obviously want the whole 'perf probe' workflow to primarily 
operate on source code - we are humans.

	Ingo
--
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