From: Masami Hiramatsu on
Srikar Dronamraju wrote:
> perf: perf interface for uprobes
>
> Changelog from v4: Merged to 2.6.35-rc3-tip.
>
> Changelog from v3: (addressed comments from Masami Hiramatsu)
> * Every process id has a different group name.
> * event name starts with function name.
> * If vaddr is specified, event name has vaddr appended
> along with function name, (this is to avoid subsequent probes
> using same event name.)
> * warning if -p and --list options are used together.
>
> Also dso can either be a short name or absolute path.
>
> Enhances perf probe to accept pid and user vaddr.
> Provides very basic support for uprobes.

It's unsure what will happen if user sets a probe by line number...
I'd like to suggest you if need_dwarf && uid, it should show an error message.
But other parts are OK for me.

> TODO:
> Update perf-probes.txt.
> Global tracing.

Debuginfo support too ?:)

Thank you,
--
Masami HIRAMATSU
2nd Research Dept.
Hitachi, Ltd., Systems Development Laboratory
E-mail: masami.hiramatsu.pt(a)hitachi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Masami Hiramatsu on
Masami Hiramatsu wrote:
> Srikar Dronamraju wrote:
>> perf: perf interface for uprobes
>>
>> Changelog from v4: Merged to 2.6.35-rc3-tip.
>>
>> Changelog from v3: (addressed comments from Masami Hiramatsu)
>> * Every process id has a different group name.
>> * event name starts with function name.
>> * If vaddr is specified, event name has vaddr appended
>> along with function name, (this is to avoid subsequent probes
>> using same event name.)
>> * warning if -p and --list options are used together.
>>
>> Also dso can either be a short name or absolute path.
>>
>> Enhances perf probe to accept pid and user vaddr.
>> Provides very basic support for uprobes.
>
> It's unsure what will happen if user sets a probe by line number...
> I'd like to suggest you if need_dwarf && uid, it should show an error message.
> But other parts are OK for me.

I've checked that perf probe was stopped by SEGV in convert_name_to_addr() with
below options.

# perf probe -u 2403 hoge.c:100

because pp->function == NULL.

Thank you,
--
Masami HIRAMATSU
2nd Research Dept.
Hitachi, Ltd., Systems Development Laboratory
E-mail: masami.hiramatsu.pt(a)hitachi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Masami Hiramatsu on
Srikar Dronamraju wrote:
>> Masami Hiramatsu wrote:
>>> Srikar Dronamraju wrote:
>>>> perf: perf interface for uprobes
>>>>
>>>> Changelog from v4: Merged to 2.6.35-rc3-tip.
>>>>
>>>> Changelog from v3: (addressed comments from Masami Hiramatsu)
>>>> * Every process id has a different group name.
>>>> * event name starts with function name.
>>>> * If vaddr is specified, event name has vaddr appended
>>>> along with function name, (this is to avoid subsequent probes
>>>> using same event name.)
>>>> * warning if -p and --list options are used together.
>>>>
>>>> Also dso can either be a short name or absolute path.
>>>>
>>>> Enhances perf probe to accept pid and user vaddr.
>>>> Provides very basic support for uprobes.
>>> It's unsure what will happen if user sets a probe by line number...
>>> I'd like to suggest you if need_dwarf && uid, it should show an error message.
>>> But other parts are OK for me.
>> I've checked that perf probe was stopped by SEGV in convert_name_to_addr() with
>> below options.
>>
>> # perf probe -u 2403 hoge.c:100
>
> It should have been perf probe -p 2403 hode.c:100
>
> However I have taken your comment and fixed that part of the code.
> So if it uprobe based probes and need_dwarf is set, it errors out.

Ah, that's great:)

Thank you,

>
>> because pp->function == NULL.
>>
>> Thank you,

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Christoph Hellwig on
> Here is a terminal snapshot of placing, using and removing a probe on a
> process with pid 3591 (corresponding to zsh)

Btw, I think this interface is not the most the best one for typical
uses cases. To make perf probe support more useful we'll need at least
a way to create a probe in all intances of a given binary/DSO, and
a way to run a binary with pre-defined breakpoints.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Arnaldo Carvalho de Melo on
Before some more comments: this is getting really nice! Kudos!

Em Mon, Jul 12, 2010 at 04:04:22PM +0530, Srikar Dronamraju escreveu:
<SNIP>
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 09cf546..ef7c2d5 100644
<SNIP>
> @@ -403,6 +426,115 @@ static bool check_event_name(const char *name)
> return true;
> }
>
> +/*
> + * uprobe_events only accepts address:
> + * Convert function and any offset to address
> + */
> +static int convert_name_to_addr(struct perf_probe_event *pev)
> +{
> + struct perf_probe_point *pp = &pev->point;
> + struct perf_session *session;
> + struct thread *thread;
> + struct symbol *sym;
> + struct map *map;
> + char *name;
> + int ret = -EINVAL;
> + unsigned long long vaddr;
> +
> + /* check if user has specifed a virtual address */
> + vaddr = strtoul(pp->function, NULL, 0);
> + session = perf_session__new(NULL, O_WRONLY, false, false);

At first creating a session here looks too much, lets see below...

> + if (!session) {
> + pr_warning("Cannot initialize perf session.\n");
> + return -ENOMEM;
> + }
> +
> + symbol_conf.try_vmlinux_path = false;
> + if (!vaddr)
> + symbol_conf.sort_by_name = true;
> + if (symbol__init() < 0) {
> + pr_warning("Cannot initialize symbols.\n");
> + goto free_session;
> + }

Configuring the symbol lib on a library function is a no-go, this
function (symbol__init()) should be marked with the equivalent of
"module_init()" on tools that need the symbol library, i.e. be called
from the cmd_{top,report,probe,etc} level.

> + event__synthesize_thread(pev->upid, event__process, session);
> + thread = perf_session__findnew(session, pev->upid);
> + if (!thread) {
> + pr_warning("Cannot initialize perf session.\n");
> + goto free_session;
> + }

Got it, you want to read an existing thread, get it into the
perf_session threads rb_tree to then use what was parsed from /proc.

I think you should change event__synthesize_thread somehow to achieve
taht same goal instead of going in such a roundabout way, unless you
need the session for some other need.

Probably we could change it to create a thread instance that then would
be used to synthesize the MMAP and COMM events... but then for the
existing use case we would be creating such events just to trow those
objects away right after synthesizing the PERF_RECORD_{MMAP,COMM}
events... perhaps duplicate them after all :-\

If I don't come with something for this quickly we can go on using what
you coded and later refactor it to remove the fat.

<SNIP>

> @@ -712,16 +858,17 @@ bool perf_probe_event_need_dwarf(struct perf_probe_event *pev)
> return false;
> }
>
> -/* Parse kprobe_events event into struct probe_point */
> -int parse_kprobe_trace_command(const char *cmd, struct kprobe_trace_event *tev)
> +/* Parse probe_events (uprobe_events) event into struct probe_point */
> +static int parse_probe_trace_command(const char *cmd,
> + struct probe_trace_event *tev)
> {
> - struct kprobe_trace_point *tp = &tev->point;
> + struct probe_trace_point *tp = &tev->point;
> char pr;
> char *p;
> int ret, i, argc;
> char **argv;
>
> - pr_debug("Parsing kprobe_events: %s\n", cmd);
> + pr_debug("Parsing probe_events: %s\n", cmd);

I suggest you put these s/kprobe/probe/g parts in a separate patch for
easing review :)

<SNIP>

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