Re: [PATCH v4 3.0-rc2-tip 20/22] 20: perf: perf interface foruprobes

[Srikar Dronamraju]

Hey Masami,
[ I have restricted the number of people in CC to those people who are
closely following perf. Please feel free to add people].

Thanks for the review.

> > Enhances perf probe to user space executables and libraries.
> > Provides very basic support for uprobes.
> > 
> > [ Probing a function in the executable using function name  ]
> > -------------------------------------------------------------
> > [root@localhost ~]# perf probe -u zfree@/bin/zsh
> 
> Hmm, here, I have concern about the interface inconsistency
> of the probe point syntax.
> 
> Since perf probe already supports debuginfo analysis,
> it accepts following syntax;
> 
> [EVENT=]FUNC[@SRC][:RLN|+OFFS|%return|;PTN] [ARG ...]
> 
> Thus, The "@" should take a source file path, not binary path.
> 

All the discussions in this forum happened with respect to
<function>@<executable> form (esp since we started with the inode based
uprobes approach). Thats why I chose the
perf probe -u func@execfile. This also gave us a flexibility to define 
two probes in two different executables in the same session/command.
Something like 
perf probe -u func1@exec1 func2@exec2.

However I am okay to change to your suggested syntax which is 
"perf probe -u /bin/zsh -a zfree"

> I think -u option should have a path of the target binary, as below
> 
> # perf probe -u /bin/zsh -a zfree

Will --uprobe work as the long name option for -u or do you suggest
something else?

> 
> This will allow perf-probe to support user-space debuginfo
> analysis. With it, we can do as below;
> 

While we discuss this I would also like to get your thoughts on couple
of issues.

1. Its okay to use a switch like "-u" that restricts the probe
definition session to just the user space tracing. i.e We wont be able
to define a probe in user space executable and also a kernel probe in
one single session.

2. Its okay to have just "target" and a flag to identify if the session
is a userspace probing session, instead of having separate fields for
userspace executable path and the module name.
	i.e you are okay with patch "perf: rename target_module to target"

3. Currently perf_probe_point has one field "file" that was only used to
refer to source file. Uprobes overrides it to use it for executable file
too. One approach would be to add another field something like
"execfile" that we use to refer to executable files and use the current
field "file" for source files. 
	Also do you suggest that we rename the current
file field to "srcfile?

Based on the outcomes of this discussion, I will modify the code and
Documentation.

-- 
Thanks and Regards
Srikar
--
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/