Re: [RFC PATCH v2 2/3] tracefs: add instances support for uprobe events

[Hari Bathini]

Hi Steve,


Thanks for the review


On Tuesday 02 August 2016 03:15 AM, Steven Rostedt wrote:
> On Thu, 28 Jul 2016 02:57:38 +0530
> Hari Bathini <hbathini@xxxxxxxxxxxxxxxxxx> wrote:
>
> > If a uprobe event is set on a library function, and if a similar uprobe
> > event trace is needed for a container, a duplicate is created leaving
> > the uprobe list with multiple entries of the same function:
> >
> >    $ perf probe --list
> >      probe_libc:malloc    (on 0x80490 in /lib64/libc.so.6)
> >      probe_libc:malloc_1  (on __libc_malloc in /lib64/libc.so.6)
> >    $
> >
> > This can soon get out of hand if multiple containers want to probe the
> > same function/address in their libraries. This patch tries to resolve this
> > by adding uprobe event trace files to every new instance. Currently, perf
> > tool can leverage this by using --debugfs-dir option - something like
> > (assuming instance dir name is 'tracing'):
> >
> >    $ perf --debugfs-dir=$MOUNT_PNT/instances probe /lib64/libc.so.6 malloc
> >    $
> >    $
> >    $ perf --debugfs-dir=$MOUNT_PNT/instances probe --list
> >      probe_libc:malloc    (on __libc_malloc in /lib64/libc.so.6)
> >    $
> >
> > New uprobe events can be added to the uprobe_events file under the instance
> > directory and the profile information for these events will be available in
> > uprobe_profile file in the same instance directory.
> Hmm, this does change the behavior of normal instances.
>
> # cd /sys/kernel/debug/tracing
> # echo 'p /bin/bash:0x41adf0' > uprobe_events
> # ls events/uprobes
> enable filter p_bash_0x41adf0
>
> # mkdir instances/foo
> # ls instances/foo/events/uprobes
> ls: cannot access instances/foo/events/uprobes: No such file or directory
>
> Usually, instances will have the same events as the top level
> directory. This will make uprobes, and only uprobes different. I'm not
> sure if this is a bad thing or not, I'll have to think about it more.

Hmmm. I think making uprobes an exception is worth considering.

> But what would it take to have this only differ for containers, and not
> normal instances?

With the current approach, instances created in instances directory and
the ones created with newinstance mount option (patch 3 of 3) are similar.
Each instance corresponds to a trace_array structure.
An alternate approach I could think of is something like below:

struct trace_instance {
    struct trace_array tr;
    struct mutex uprobe_lock;
    struct list_head uprobe_list;
    /* any other new data specific to a mount instance */
};

where a mountable instance is more than a trace array.
This may need addition of new flags for trace array saying
whether it is a global trace or directory instance or mountable instance.
Also, the helper functions that add/remove events need to be tweaked 
accordingly.


Thanks
Hari