Re: [PATCH 4/4] perf tools: Complete support for dynamic strings

From: Frederic Weisbecker
Date: Mon Aug 31 2009 - 13:01:37 EST


On Mon, Aug 31, 2009 at 10:11:33AM +0200, Ingo Molnar wrote:
>
> * Frederic Weisbecker <fweisbec@xxxxxxxxx> wrote:
>
> > Complete support for __str_loc type strings of ftrace events which
> > have dynamic offsets values set for each of them inside their sammples.
> >
> > Before:
> > geany-5759 [000] 0.000000: lock_release: name
> > geany-5759 [000] 0.000000: lock_release: name
> > geany-5759 [000] 0.000000: lock_release: name
> > kondemand/0-362 [000] 0.000000: lock_release: name
> > pdflush-421 [000] 0.000000: lock_release: name
> >
> > After:
> > geany-5759 [000] 0.000000: lock_release: &u->lock
> > geany-5759 [000] 0.000000: lock_release: key
> > geany-5759 [000] 0.000000: lock_release: &group->notification_mutex
> > kondemand/0-362 [000] 0.000000: lock_release: &rq->lock
> > pdflush-421 [000] 0.000000: lock_release: &rq->lock
>
> I've applied all five patches, thanks Frederic!
>
> Also, i think this makes perf trace quite functional already so i
> merged that topic (and these commits) into tracing/core, for v2.6.32
> merging.



Ok.



> One thing that i noticed which is still quite quirky are the
> parameters to perf record. (needing -R and the :r postfix to events)
> Could something more intuitive be done here?


I hope. It's not that easy, at least for me, to find something intuitive
there.

I've thought about

-e *sys:event (because the * suggests a deref that takes the content)
But that's a bit convoluted. And moreover we may want
to support basic regex later for event selection.

If someone has an idea, please tell me.

Another thing. We could, by default display a small comment while
selecting an event without its content:

"Opening counter sys:event. Type -R if you want raw sampling."

The user could shut that up with a -q option.

> Also, i think people would like to use wildcards in event
> specifiers, such as:
>
> perf record -e timer/*
>
> To capture all timer events:
>
> hrtimer_cancel
> hrtimer_expire_entry
> hrtimer_expire_exit
> hrtimer_init
> hrtimer_start
> itimer_expire
> itimer_state
> timer_cancel
> timer_expire_entry
> timer_expire_exit
> timer_init
> timer_start
>
> Instead of having to type:
>
> perf record -e hrtimer_cancel -e hrtimer_expire_entry \
> -e hrtimer_expire_exit -e hrtimer_init -e hrtimer_start \
> -e itimer_expire -e itimer_state -e timer_cancel \
> -e timer_expire_entry -e timer_expire_exit -e timer_init \
> -e timer_start
>
> which is not quite realistic.


Totally agreed, it was in my todo list.

But don't you think
perf record -e timer:*
is more intuitive and follows the current logic?

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