Re: uprobes && pre-filtering

[Oleg Nesterov <oleg@redhat.com>]

On 11/06, Josh Stone wrote:
>
> On 11/06/2012 09:02 AM, Oleg Nesterov wrote:
> >>> - Perhaps we should extend the API. We can add
> >>>
> >>> 	uprobe_apply(consumer, task, bool add_remove);
> >>>
> >>>   which adds/removes breakpoints to task->mm.
> >>>
> >>>   This way consumer can probe every task it wants to trace after
> >>>   uprobe_register().
> >>>
> >>>   Its ->filter(UPROBE_FILTER_REGISTER) should simply return false. Or,
> >>>   better, we can split uprobe_register() into 2 functions,
> >>>   __uprobe_register() and uprobe_apply_all() which actually does
> >>>   register_for_each_vma().
> >>>
> >>>   ***** QUESTION *****: perhaps this is all systemtap needs? ignoring
> >>>   UPROBE_FILTER_MMAP.
> >>>
> >> So in this case, would uprobe_register() just add a consumer to a
> >> new/existing uprobe. The actual probe insertion is done by the
> >> uprobe_apply()/uprobe_apply_all().
> >
> > Yes. Not sure we really need this, but to me this extension looks natural.
> >
> > Frank, Josh, do you think it can help systemtap ?
>
> Yes, I think this sounds closer to systemtap's model of probing.  We
> already track tasks that come and go to see which are "interesting", so
> we could easily call apply() at that time.  We actually watch mmaps too,
> so I think we could apply() for that case as well.

OK, thanks.

(just in case, mmap is different, but lets ignore this now).

> We wouldn't even need filtering functions at all in this mode.  But
> maybe other consumers could still use it, like perf.

Of course, we need ->filter() anyway.

> However, it's not clear to me what value there is in uprobe_register, if
> you always have to apply it too.  The modes are something like:
>
> 1. uprobe_register(); uprobe_apply_all();
> 2. uprobe_register(); uprobe_apply(); [...]

No, no, sorry for confusion.

I meant we could add __uprobe_register() (or whatever) which doesn't actually
insert the breakpoint. So if the tracer relies on uprobe_apply() it can avoid
the costly register_for_each_vma/filter and do __uprobe_register + apply.

This is not strictly necessary even if we add uprobe_apply*, and you can
always use uprobe_register() (or uprobe_register_all as you denoted it
below).

> first applicable task to come around.  So why not instead:
>
> 1. uprobe_register_all();
> 2. uprobe_register_task(); [...]
>
> In this case, the second would have to allow the same consumer to be
> repeated on different tasks, but it feels more natural to me.

This can work too.

But uprobe_unregister_task() doesn't look very clear. What should it
do? IOW, you still need uprobe_unregister_all() and this doesn't look
symmetrical.

Oleg.