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

[Hari Bathini <hbathini@linux.vnet.ibm.com>]

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@linux.vnet.ibm.com> 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