All of lore.kernel.org
 help / color / mirror / Atom feed
From: Frederic Weisbecker <fweisbec@gmail.com>
To: Ingo Molnar <mingo@elte.hu>
Cc: Masami Hiramatsu <mhiramat@redhat.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	lkml <linux-kernel@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Arnaldo Carvalho de Melo <acme@redhat.com>,
	Mike Galbraith <efault@gmx.de>, Paul Mackerras <paulus@samba.org>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Christoph Hellwig <hch@infradead.org>,
	Ananth N Mavinakayanahalli <ananth@in.ibm.com>,
	Jim Keniston <jkenisto@us.ibm.com>,
	"Frank Ch. Eigler" <fche@redhat.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	systemtap <systemtap@sources.redhat.com>,
	DLE <dle-develop@lists.sourceforge.net>
Subject: Re: [PATCH -tip tracing/kprobes 0/9] tracing/kprobes, perf: perf probe and kprobe-tracer bugfixes
Date: Mon, 19 Oct 2009 21:32:51 +0200	[thread overview]
Message-ID: <20091019193248.GB4880@nowhere> (raw)
In-Reply-To: <20091019112125.GA12829@elte.hu>

On Mon, Oct 19, 2009 at 01:21:25PM +0200, Ingo Molnar wrote:
> 
> * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> > I personally don't imagine common easy usecases that imply relative 
> > line offsets but rather absolute lines.
> 
> Absolute line numbers could be expressed too, via:
> 
>    perf schedule@5396
> 
> There's no immediate need to express 'sched.c'. (you can do it, or you 
> can do it for extra clarity or for the case of local scope functions of 
> which multiple instances exist)



Ok. As long as there is an easy way to do that, like in the above
example, then I think it's fine.

(More argument below)

 
> > I don't understand your point. If your editor is opened and you have 
> > the source code in front of you, why would you cut'n'paste a line 
> > instead of actually write the line number?
> 
> The 'perf probe --list schedule' sub-tool i outlined would display 
> relative line numbers for the function - starting at 0:
> 
> 000	/*
> 001	 * schedule() is the main scheduler function.
> 002	 */
> 003	asmlinkage void __sched schedule(void)
> 004	{
> 005		struct task_struct *prev, *next;
> 006		unsigned long *switch_count;
> 007		struct rq *rq;
> 008		int cpu;
> 009
> 010	need_resched:
> 011		preempt_disable();
> 012		cpu = smp_processor_id();
> 013		rq = cpu_rq(cpu);
> 014		rcu_sched_qs(cpu);
> 015		prev = rq->curr;
> 016		switch_count = &prev->nivcsw;
> 017
> 018		release_kernel_lock(prev);
> 
> 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.


I haven't thought that relative lines would make it less short-lived.
So yeah that's a good point.

I think absolute and relative line modes are not colliding/contending
at all but actually fit two different needs.

- absolute is nice when you are lonely doing kernel debugging.
  (can be expanded at will once you imagine user probes)
  You are stuck in your code editor, trying to figure out the
  origin of your problem and then you think it would be nice
  to set a probe in branch 1 and in branch 2 inside func_foo().
  Then you already have absolute lines and relying in
  perf probe --list func_foo() to resolve an absolute line into
  a relative one is a very undesired middle step.

- relative is nice in some other cases. When you already have
  the function target in mind, you even don't need to check your
  editor, just a quick check to this command and get the relative
  line. But also when you want to transmit a probe reference
  in a mailing list because of its better lifetime.


Both are important IMO.


> 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@switch_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@'next ='



Yeah, that's a nice advanced usage.


> 
> Note that i was able to specify both probes without having opened an 
> editor - just based on the general knowledge of the scheduler.


Yeah, that a good workflow for someone who knows well the targeted
code.

That can't cover every needs though, but I guess you agree that
abs_line/rel_line/smart_resolution methods need to coexist anyway,
all of them fit different level of needs.


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


Agreed!

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


Ok.

Thanks.


  reply	other threads:[~2009-10-19 19:32 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-10-17  0:07 [PATCH -tip tracing/kprobes 0/9] tracing/kprobes, perf: perf probe and kprobe-tracer bugfixes Masami Hiramatsu
2009-10-17  0:07 ` [PATCH -tip tracing/kprobes 1/9] tracing/kprobes: Update kprobe-tracer selftest against new syntax Masami Hiramatsu
2009-10-17 10:04   ` [tip:perf/probes] " tip-bot for Masami Hiramatsu
2009-10-17  0:07 ` [PATCH -tip tracing/kprobes 2/9] tracing/kprobes: Add failure messages for debugging Masami Hiramatsu
2009-10-17 10:05   ` [tip:perf/probes] " tip-bot for Masami Hiramatsu
2009-10-17  0:07 ` [PATCH -tip tracing/kprobes 3/9] x86: Add MMX/SSE opcode groups to opcode map Masami Hiramatsu
2009-10-17 10:05   ` [tip:perf/probes] " tip-bot for Masami Hiramatsu
2009-10-17  0:07 ` [PATCH -tip tracing/kprobes 4/9] x86: Add AMD prefetch and 3DNow! opcodes " Masami Hiramatsu
2009-10-17 10:05   ` [tip:perf/probes] " tip-bot for Masami Hiramatsu
2009-10-17  0:07 ` [PATCH -tip tracing/kprobes 5/9] perf: Check libdwarf APIs for perf probe Masami Hiramatsu
2009-10-17 10:05   ` [tip:perf/probes] " tip-bot for Masami Hiramatsu
2009-10-17  0:08 ` [PATCH -tip tracing/kprobes 6/9] perf: Use die() for error cases in perf-probe Masami Hiramatsu
2009-10-17 10:06   ` [tip:perf/probes] " tip-bot for Masami Hiramatsu
2009-10-17  0:08 ` [PATCH -tip tracing/kprobes 7/9] perf: Use eprintf() for debug messages " Masami Hiramatsu
2009-10-17 10:06   ` [tip:perf/probes] " tip-bot for Masami Hiramatsu
2009-10-17  0:08 ` [PATCH -tip tracing/kprobes 8/9] perf: Add DIE_IF() macro for error checking Masami Hiramatsu
2009-10-17 10:06   ` [tip:perf/probes] " tip-bot for Masami Hiramatsu
2009-10-17  0:08 ` [PATCH -tip tracing/kprobes 9/9] perf: Add perf-probe document Masami Hiramatsu
2009-10-17 10:06   ` [tip:perf/probes] " tip-bot for Masami Hiramatsu
2009-10-17  8:02 ` [PATCH -tip tracing/kprobes 0/9] tracing/kprobes, perf: perf probe and kprobe-tracer bugfixes Ingo Molnar
2009-10-17 10:34   ` Ingo Molnar
2009-10-17 10:37     ` Ingo Molnar
2009-10-18  6:01     ` Masami Hiramatsu
2009-10-19  7:51       ` Ingo Molnar
2009-10-19 11:00         ` Frederic Weisbecker
2009-10-19 11:21           ` Ingo Molnar
2009-10-19 19:32             ` Frederic Weisbecker [this message]
2009-10-20  6:43               ` Ingo Molnar
2009-10-20 17:51                 ` Frederic Weisbecker
2009-10-19 19:51             ` Masami Hiramatsu
2009-10-19 22:54               ` Masami Hiramatsu
2009-10-20  6:51                 ` Ingo Molnar
2009-10-21  0:05                   ` Masami Hiramatsu
2009-10-20  6:50               ` Ingo Molnar
2009-10-20 14:38                 ` Masami Hiramatsu
2009-10-19 16:18           ` Arnaldo Carvalho de Melo
2009-10-20 17:45             ` Frederic Weisbecker
2009-10-19 18:56         ` Masami Hiramatsu
2009-10-20  6:54           ` Ingo Molnar
2009-10-20 14:27             ` Masami Hiramatsu
2009-10-19 23:10 ` Ashwin Chaugule
2009-10-20  0:30   ` Masami Hiramatsu
2009-10-20  1:59     ` Ashwin Chaugule
2009-10-20  6:55       ` Ingo Molnar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20091019193248.GB4880@nowhere \
    --to=fweisbec@gmail.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@redhat.com \
    --cc=ananth@in.ibm.com \
    --cc=dle-develop@lists.sourceforge.net \
    --cc=efault@gmx.de \
    --cc=fche@redhat.com \
    --cc=hch@infradead.org \
    --cc=hpa@zytor.com \
    --cc=jkenisto@us.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhiramat@redhat.com \
    --cc=mingo@elte.hu \
    --cc=paulus@samba.org \
    --cc=rostedt@goodmis.org \
    --cc=systemtap@sources.redhat.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.