[GIT PULL] uprobes: pre-filtering

[GIT PULL] uprobes: pre-filtering

[Oleg Nesterov]

Ingo, please pull from

  git://git.kernel.org/pub/scm/linux/kernel/git/oleg/misc uprobes/core

Mostly pre-filtering. This needs more work and perhaps more functionality.
In particular, perhaps dup_mmap() should remove the unwanted breakpoints.
And we can add more ->filter() hooks to, say, speedup uprobe_register().
Plus we can do some optimizations to avoid register_for_each_vma() in
case when we know that all mm's were previously acked/nacked.

Srikar, the only patch you did not ack explicitely is 1fecb96d
"Do not allocate current->utask unnecessary", but afaics you do not
object.

And the patch from Josh which exports uprobe_register/unregister for modules.
Christoph (cc'ed) doesn't like this change, but I disagree. Whatever you
think about systemtap it is the widely used tool, and uprobes can have other
out-of-tree users. This is like kprobes, kprobe_register() is exported but
it doesn't have a modular in-kernel user too. I do not see why should we
limit the usage of uprobes.



Josh Stone (1):
      uprobes: Add exports for module use

Oleg Nesterov (26):
      uprobes: Move __set_bit(UPROBE_SKIP_SSTEP) into alloc_uprobe()
      uprobes: Kill the "uprobe != NULL" check in uprobe_unregister()
      uprobes: Kill the pointless inode/uc checks in register/unregister
      uprobes: Kill uprobe_consumer->filter()
      uprobes: Introduce filter_chain()
      uprobes: _unregister() should always do register_for_each_vma(false)
      uprobes: _register() should always do register_for_each_vma(true)
      uprobes: Introduce uprobe->register_rwsem
      uprobes: Change filter_chain() to iterate ->consumers list
      uprobes: Kill UPROBE_RUN_HANDLER flag
      uprobes: Kill uprobe->copy_mutex
      uprobes: Kill uprobe_events, use RB_EMPTY_ROOT() instead
      uprobes: Introduce uprobe_is_active()
      uprobes: Kill uprobes_mutex[], separate alloc_uprobe() and __uprobe_register()
      uprobes: Rationalize the usage of filter_chain()
      uprobes: Reintroduce uprobe_consumer->filter()
      uprobes: Teach handler_chain() to filter out the probed task
      uprobes/x86: Change __skip_sstep() to actually skip the whole insn
      uprobes: Change handle_swbp() to expose bp_vaddr to handler_chain()
      uprobes: Move alloc_page() from xol_add_vma() to xol_alloc_area()
      uprobes: Fold xol_alloc_area() into get_xol_area()
      uprobes: Turn add_utask() into get_utask()
      uprobes: Do not play with utask in xol_get_insn_slot()
      uprobes: Fix utask->xol_vaddr leak in pre_ssout()
      uprobes: Do not allocate current->utask unnecessary
      uprobes: Kill the bogus IS_ERR_VALUE(xol_vaddr) check

 arch/x86/kernel/uprobes.c   |    4 +-
 include/linux/uprobes.h     |   17 ++-
 kernel/events/uprobes.c     |  433 ++++++++++++++++++++++---------------------
 kernel/ptrace.c             |    6 +
 kernel/trace/trace_uprobe.c |    5 +-
 5 files changed, 243 insertions(+), 222 deletions(-)

Re: [GIT PULL] uprobes: pre-filtering

[Ingo Molnar]

* Oleg Nesterov <oleg@redhat.com> wrote:

> Ingo, please pull from
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/oleg/misc uprobes/core
> 
> Mostly pre-filtering. This needs more work and perhaps more functionality.
> In particular, perhaps dup_mmap() should remove the unwanted breakpoints.
> And we can add more ->filter() hooks to, say, speedup uprobe_register().
> Plus we can do some optimizations to avoid register_for_each_vma() in
> case when we know that all mm's were previously acked/nacked.
> 
> Srikar, the only patch you did not ack explicitely is 1fecb96d
> "Do not allocate current->utask unnecessary", but afaics you do not
> object.
> 
> And the patch from Josh which exports uprobe_register/unregister for modules.
> Christoph (cc'ed) doesn't like this change, but I disagree. Whatever you
> think about systemtap it is the widely used tool, and uprobes can have other
> out-of-tree users. This is like kprobes, kprobe_register() is exported but
> it doesn't have a modular in-kernel user too. I do not see why should we
> limit the usage of uprobes.
> 
> 
> 
> Josh Stone (1):
>       uprobes: Add exports for module use
> 
> Oleg Nesterov (26):
>       uprobes: Move __set_bit(UPROBE_SKIP_SSTEP) into alloc_uprobe()
>       uprobes: Kill the "uprobe != NULL" check in uprobe_unregister()
>       uprobes: Kill the pointless inode/uc checks in register/unregister
>       uprobes: Kill uprobe_consumer->filter()
>       uprobes: Introduce filter_chain()
>       uprobes: _unregister() should always do register_for_each_vma(false)
>       uprobes: _register() should always do register_for_each_vma(true)
>       uprobes: Introduce uprobe->register_rwsem
>       uprobes: Change filter_chain() to iterate ->consumers list
>       uprobes: Kill UPROBE_RUN_HANDLER flag
>       uprobes: Kill uprobe->copy_mutex
>       uprobes: Kill uprobe_events, use RB_EMPTY_ROOT() instead
>       uprobes: Introduce uprobe_is_active()
>       uprobes: Kill uprobes_mutex[], separate alloc_uprobe() and __uprobe_register()
>       uprobes: Rationalize the usage of filter_chain()
>       uprobes: Reintroduce uprobe_consumer->filter()
>       uprobes: Teach handler_chain() to filter out the probed task
>       uprobes/x86: Change __skip_sstep() to actually skip the whole insn
>       uprobes: Change handle_swbp() to expose bp_vaddr to handler_chain()
>       uprobes: Move alloc_page() from xol_add_vma() to xol_alloc_area()
>       uprobes: Fold xol_alloc_area() into get_xol_area()
>       uprobes: Turn add_utask() into get_utask()
>       uprobes: Do not play with utask in xol_get_insn_slot()
>       uprobes: Fix utask->xol_vaddr leak in pre_ssout()
>       uprobes: Do not allocate current->utask unnecessary
>       uprobes: Kill the bogus IS_ERR_VALUE(xol_vaddr) check
> 
>  arch/x86/kernel/uprobes.c   |    4 +-
>  include/linux/uprobes.h     |   17 ++-
>  kernel/events/uprobes.c     |  433 ++++++++++++++++++++++---------------------
>  kernel/ptrace.c             |    6 +
>  kernel/trace/trace_uprobe.c |    5 +-
>  5 files changed, 243 insertions(+), 222 deletions(-)

The kernel side looks good to me - but how does 'perf uprobe' 
make use of it in practice, how can I test it?

Thanks,

	Ingo

Re: [GIT PULL] uprobes: pre-filtering

[Anton Arapov]

Hello Ingo,

On Thu, Jan 24, 2013 at 11:17 PM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Oleg Nesterov <oleg@redhat.com> wrote:
>
>> Ingo, please pull from
>>
>>   git://git.kernel.org/pub/scm/linux/kernel/git/oleg/misc uprobes/core
>>
>> Mostly pre-filtering. This needs more work and perhaps more functionality.
>> In particular, perhaps dup_mmap() should remove the unwanted breakpoints.
>> And we can add more ->filter() hooks to, say, speedup uprobe_register().
>> Plus we can do some optimizations to avoid register_for_each_vma() in
>> case when we know that all mm's were previously acked/nacked.
>>
>> Srikar, the only patch you did not ack explicitely is 1fecb96d
>> "Do not allocate current->utask unnecessary", but afaics you do not
>> object.
>>
>> And the patch from Josh which exports uprobe_register/unregister for modules.
>> Christoph (cc'ed) doesn't like this change, but I disagree. Whatever you
>> think about systemtap it is the widely used tool, and uprobes can have other
>> out-of-tree users. This is like kprobes, kprobe_register() is exported but
>> it doesn't have a modular in-kernel user too. I do not see why should we
>> limit the usage of uprobes.
>>
>>
>>
>> Josh Stone (1):
>>       uprobes: Add exports for module use
>>
>> Oleg Nesterov (26):
>>       uprobes: Move __set_bit(UPROBE_SKIP_SSTEP) into alloc_uprobe()
>>       uprobes: Kill the "uprobe != NULL" check in uprobe_unregister()
>>       uprobes: Kill the pointless inode/uc checks in register/unregister
>>       uprobes: Kill uprobe_consumer->filter()
>>       uprobes: Introduce filter_chain()
>>       uprobes: _unregister() should always do register_for_each_vma(false)
>>       uprobes: _register() should always do register_for_each_vma(true)
>>       uprobes: Introduce uprobe->register_rwsem
>>       uprobes: Change filter_chain() to iterate ->consumers list
>>       uprobes: Kill UPROBE_RUN_HANDLER flag
>>       uprobes: Kill uprobe->copy_mutex
>>       uprobes: Kill uprobe_events, use RB_EMPTY_ROOT() instead
>>       uprobes: Introduce uprobe_is_active()
>>       uprobes: Kill uprobes_mutex[], separate alloc_uprobe() and __uprobe_register()
>>       uprobes: Rationalize the usage of filter_chain()
>>       uprobes: Reintroduce uprobe_consumer->filter()
>>       uprobes: Teach handler_chain() to filter out the probed task
>>       uprobes/x86: Change __skip_sstep() to actually skip the whole insn
>>       uprobes: Change handle_swbp() to expose bp_vaddr to handler_chain()
>>       uprobes: Move alloc_page() from xol_add_vma() to xol_alloc_area()
>>       uprobes: Fold xol_alloc_area() into get_xol_area()
>>       uprobes: Turn add_utask() into get_utask()
>>       uprobes: Do not play with utask in xol_get_insn_slot()
>>       uprobes: Fix utask->xol_vaddr leak in pre_ssout()
>>       uprobes: Do not allocate current->utask unnecessary
>>       uprobes: Kill the bogus IS_ERR_VALUE(xol_vaddr) check
>>
>>  arch/x86/kernel/uprobes.c   |    4 +-
>>  include/linux/uprobes.h     |   17 ++-
>>  kernel/events/uprobes.c     |  433 ++++++++++++++++++++++---------------------
>>  kernel/ptrace.c             |    6 +
>>  kernel/trace/trace_uprobe.c |    5 +-
>>  5 files changed, 243 insertions(+), 222 deletions(-)
>
> The kernel side looks good to me - but how does 'perf uprobe'
> make use of it in practice, how can I test it?

I'm not sure whether you looking into testing specific changes in this
pull, but in general, syntax is:
  perf probe -x /lib64/libc.so.6 malloc
  perf record -e probe_libc:p_malloc -aR sleep 30

hope this is what you was looking for,
Anton.

Re: [GIT PULL] uprobes: pre-filtering

[Ingo Molnar]

* Anton Arapov <anton@redhat.com> wrote:

> Hello Ingo,
> 
> On Thu, Jan 24, 2013 at 11:17 PM, Ingo Molnar <mingo@kernel.org> wrote:
> >
> > * Oleg Nesterov <oleg@redhat.com> wrote:
> >
> >> Ingo, please pull from
> >>
> >>   git://git.kernel.org/pub/scm/linux/kernel/git/oleg/misc uprobes/core
> >>
> >> Mostly pre-filtering. This needs more work and perhaps more functionality.
> >> In particular, perhaps dup_mmap() should remove the unwanted breakpoints.
> >> And we can add more ->filter() hooks to, say, speedup uprobe_register().
> >> Plus we can do some optimizations to avoid register_for_each_vma() in
> >> case when we know that all mm's were previously acked/nacked.
> >>
> >> Srikar, the only patch you did not ack explicitely is 1fecb96d
> >> "Do not allocate current->utask unnecessary", but afaics you do not
> >> object.
> >>
> >> And the patch from Josh which exports uprobe_register/unregister for modules.
> >> Christoph (cc'ed) doesn't like this change, but I disagree. Whatever you
> >> think about systemtap it is the widely used tool, and uprobes can have other
> >> out-of-tree users. This is like kprobes, kprobe_register() is exported but
> >> it doesn't have a modular in-kernel user too. I do not see why should we
> >> limit the usage of uprobes.
> >>
> >>
> >>
> >> Josh Stone (1):
> >>       uprobes: Add exports for module use
> >>
> >> Oleg Nesterov (26):
> >>       uprobes: Move __set_bit(UPROBE_SKIP_SSTEP) into alloc_uprobe()
> >>       uprobes: Kill the "uprobe != NULL" check in uprobe_unregister()
> >>       uprobes: Kill the pointless inode/uc checks in register/unregister
> >>       uprobes: Kill uprobe_consumer->filter()
> >>       uprobes: Introduce filter_chain()
> >>       uprobes: _unregister() should always do register_for_each_vma(false)
> >>       uprobes: _register() should always do register_for_each_vma(true)
> >>       uprobes: Introduce uprobe->register_rwsem
> >>       uprobes: Change filter_chain() to iterate ->consumers list
> >>       uprobes: Kill UPROBE_RUN_HANDLER flag
> >>       uprobes: Kill uprobe->copy_mutex
> >>       uprobes: Kill uprobe_events, use RB_EMPTY_ROOT() instead
> >>       uprobes: Introduce uprobe_is_active()
> >>       uprobes: Kill uprobes_mutex[], separate alloc_uprobe() and __uprobe_register()
> >>       uprobes: Rationalize the usage of filter_chain()
> >>       uprobes: Reintroduce uprobe_consumer->filter()
> >>       uprobes: Teach handler_chain() to filter out the probed task
> >>       uprobes/x86: Change __skip_sstep() to actually skip the whole insn
> >>       uprobes: Change handle_swbp() to expose bp_vaddr to handler_chain()
> >>       uprobes: Move alloc_page() from xol_add_vma() to xol_alloc_area()
> >>       uprobes: Fold xol_alloc_area() into get_xol_area()
> >>       uprobes: Turn add_utask() into get_utask()
> >>       uprobes: Do not play with utask in xol_get_insn_slot()
> >>       uprobes: Fix utask->xol_vaddr leak in pre_ssout()
> >>       uprobes: Do not allocate current->utask unnecessary
> >>       uprobes: Kill the bogus IS_ERR_VALUE(xol_vaddr) check
> >>
> >>  arch/x86/kernel/uprobes.c   |    4 +-
> >>  include/linux/uprobes.h     |   17 ++-
> >>  kernel/events/uprobes.c     |  433 ++++++++++++++++++++++---------------------
> >>  kernel/ptrace.c             |    6 +
> >>  kernel/trace/trace_uprobe.c |    5 +-
> >>  5 files changed, 243 insertions(+), 222 deletions(-)
> >
> > The kernel side looks good to me - but how does 'perf uprobe'
> > make use of it in practice, how can I test it?
> 
> I'm not sure whether you looking into testing specific changes in this
> pull, [...]

Yes, I was curious about specifically testing the filtering 
callback changes in this pull request.

Thanks,

	Ingo

Re: [GIT PULL] uprobes: pre-filtering

[Oleg Nesterov]

On 01/24, Ingo Molnar wrote:
>
> * Oleg Nesterov <oleg@redhat.com> wrote:
>
> > Ingo, please pull from
> >
> >   git://git.kernel.org/pub/scm/linux/kernel/git/oleg/misc uprobes/core
> >
> > Mostly pre-filtering. This needs more work and perhaps more functionality.
> > In particular, perhaps dup_mmap() should remove the unwanted breakpoints.
> > And we can add more ->filter() hooks to, say, speedup uprobe_register().
> > Plus we can do some optimizations to avoid register_for_each_vma() in
> > case when we know that all mm's were previously acked/nacked.
>
> The kernel side looks good to me - but how does 'perf uprobe'
> make use of it in practice, how can I test it?

Unfortunately, currently there is no in-kernel user of pre-filtering.

I'll try to implement the pid-base filtering at least for
tracing/uprobe_events, but this needs a time. Not only I am not familiar
with this code, I am not sure how this interface should actually look.
And I agree, perf should be able to use it somehow, perhaps at least
to allow to probe a single task/mm.

Oleg.

Re: [GIT PULL] uprobes: pre-filtering

[Ingo Molnar]

* Oleg Nesterov <oleg@redhat.com> wrote:

> On 01/24, Ingo Molnar wrote:
> >
> > * Oleg Nesterov <oleg@redhat.com> wrote:
> >
> > > Ingo, please pull from
> > >
> > >   git://git.kernel.org/pub/scm/linux/kernel/git/oleg/misc uprobes/core
> > >
> > > Mostly pre-filtering. This needs more work and perhaps more functionality.
> > > In particular, perhaps dup_mmap() should remove the unwanted breakpoints.
> > > And we can add more ->filter() hooks to, say, speedup uprobe_register().
> > > Plus we can do some optimizations to avoid register_for_each_vma() in
> > > case when we know that all mm's were previously acked/nacked.
> >
> > The kernel side looks good to me - but how does 'perf 
> > uprobe' make use of it in practice, how can I test it?
> 
> Unfortunately, currently there is no in-kernel user of 
> pre-filtering.
> 
> I'll try to implement the pid-base filtering at least for 
> tracing/uprobe_events, but this needs a time. Not only I am 
> not familiar with this code, I am not sure how this interface 
> should actually look. And I agree, perf should be able to use 
> it somehow, perhaps at least to allow to probe a single 
> task/mm.

Would be nice to get something minimal/simple going, so that it 
can be tested, etc.

Thanks,

	Ingo

Re: [GIT PULL] uprobes: pre-filtering

[Josh Stone]

On 01/24/2013 07:40 AM, Oleg Nesterov wrote:
> I'll try to implement the pid-base filtering at least for
> tracing/uprobe_events, but this needs a time. Not only I am not familiar
> with this code, I am not sure how this interface should actually look.
> And I agree, perf should be able to use it somehow, perhaps at least
> to allow to probe a single task/mm.

Even without changing perf's interface, it already has constraints for
its child processes (versus --all-cpus) or for specific pid/tid/uid.
Pre-filtering could help enforce and optimize those constraints.

Re: [GIT PULL] uprobes: pre-filtering

[Oleg Nesterov]

On 01/24, Ingo Molnar wrote:
>
> * Oleg Nesterov <oleg@redhat.com> wrote:
>
> > Unfortunately, currently there is no in-kernel user of
> > pre-filtering.
> >
> > I'll try to implement the pid-base filtering at least for
> > tracing/uprobe_events, but this needs a time. Not only I am
> > not familiar with this code, I am not sure how this interface
> > should actually look. And I agree, perf should be able to use
> > it somehow, perhaps at least to allow to probe a single
> > task/mm.
>
> Would be nice to get something minimal/simple going, so that it
> can be tested, etc.

Heh, I understand.

I do not see anything simple to implement... I'll try to think.
Srikar, do you have any idea?

All I can say right now: I'll send you the patches when I have them ;)
Can't promise this will be soon.

_Perhaps_, as a first step, we can simply change create_trace_uprobe()
so that it would be possible to specify list-of-pids at creation time...
Not sure this actually makes sense.

Oleg.

Re: [GIT PULL] uprobes: pre-filtering

[Oleg Nesterov]

On 01/24, Josh Stone wrote:
>
> On 01/24/2013 07:40 AM, Oleg Nesterov wrote:
> > I'll try to implement the pid-base filtering at least for
> > tracing/uprobe_events, but this needs a time. Not only I am not familiar
> > with this code, I am not sure how this interface should actually look.
> > And I agree, perf should be able to use it somehow, perhaps at least
> > to allow to probe a single task/mm.
>
> Even without changing perf's interface, it already has constraints for
> its child processes (versus --all-cpus) or for specific pid/tid/uid.
> Pre-filtering could help enforce and optimize those constraints.

This is what I meant. The question is how can I do this.

Oleg.

Re: [GIT PULL] uprobes: pre-filtering

[Srikar Dronamraju]

* Oleg Nesterov <oleg@redhat.com> [2013-01-24 18:06:12]:

> On 01/24, Ingo Molnar wrote:
> >
> > * Oleg Nesterov <oleg@redhat.com> wrote:
> >
> > > Unfortunately, currently there is no in-kernel user of
> > > pre-filtering.
> > >
> > > I'll try to implement the pid-base filtering at least for
> > > tracing/uprobe_events, but this needs a time. Not only I am
> > > not familiar with this code, I am not sure how this interface
> > > should actually look. And I agree, perf should be able to use
> > > it somehow, perhaps at least to allow to probe a single
> > > task/mm.
> >
> > Would be nice to get something minimal/simple going, so that it
> > can be tested, etc.
> 
> Heh, I understand.
> 
> I do not see anything simple to implement... I'll try to think.
> Srikar, do you have any idea?
> 
> All I can say right now: I'll send you the patches when I have them ;)
> Can't promise this will be soon.
> 
> _Perhaps_, as a first step, we can simply change create_trace_uprobe()
> so that it would be possible to specify list-of-pids at creation time...
> Not sure this actually makes sense.
> 

[ Adding Masami and Steven since they have other ideas / inputs ]

I can see two not-so-easy alternatives

Reuse the event's post filtering meta data and convert it to a
prefiltering handler. Convertion looks very tricky 

probe_event_enable() seems to be the right place to do this.
tu->call.filter would have the post filtering data
utc->filter needs to be set to the pre-filtering handler.

The other alternative is to extend the current abi and pass the
prefilter option. Should we extend the abi for userspace tracing is
obviously debatable.

> Oleg.
> 

-- 
Thanks and Regards
Srikar Dronamraju

Re: [GIT PULL] uprobes: pre-filtering

[Ingo Molnar]

* Srikar Dronamraju <srikar@linux.vnet.ibm.com> wrote:

> The other alternative is to extend the current abi and pass 
> the prefilter option. Should we extend the abi for userspace 
> tracing is obviously debatable.

That's the obvious path to go - why add something to the kernel 
if user-space cannot make use of it?

Thanks,

	Ingo

Re: Re: [GIT PULL] uprobes: pre-filtering

[Masami Hiramatsu]

(2013/01/25 15:46), Srikar Dronamraju wrote:
> The other alternative is to extend the current abi and pass the
> prefilter option. Should we extend the abi for userspace tracing is
> obviously debatable.

Hmm, afaik, ftrace has set_ftrace_pid and perf record
also has --pid option. I think you can hook those
to set pre-filter option at first. Of course, since
those are universal options, we may need other per-event
pid filter.

Thank you,

-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

Re: [GIT PULL] uprobes: pre-filtering

[Oleg Nesterov]

On 01/25, Ingo Molnar wrote:
>
> * Srikar Dronamraju <srikar@linux.vnet.ibm.com> wrote:
>
> > The other alternative is to extend the current abi and pass
> > the prefilter option. Should we extend the abi for userspace
> > tracing is obviously debatable.
>
> That's the obvious path to go - why add something to the kernel
> if user-space cannot make use of it?

This is what I am going to (try to) do, but I am not sure if this makes
sense...

For the start, can't we teach 'uprobe_events' file to accept, say,

	'p file:0x1234 pid=1 other-opts'

for the start? This looks simple enough, and I after looked into tools/perf
it seems that perf can be changed too.

What do you think?

Then we can extend 'pid=' option to accept the list of pids, perhaps.

In the long term we probably need uprobes/pid_filter or something like this,
it should allow to add/del pid dynamically. I really do not know.

Oleg.

Re: [GIT PULL] uprobes: pre-filtering

[Ingo Molnar]

* Oleg Nesterov <oleg@redhat.com> wrote:

> On 01/25, Ingo Molnar wrote:
> >
> > * Srikar Dronamraju <srikar@linux.vnet.ibm.com> wrote:
> >
> > > The other alternative is to extend the current abi and pass
> > > the prefilter option. Should we extend the abi for userspace
> > > tracing is obviously debatable.
> >
> > That's the obvious path to go - why add something to the kernel
> > if user-space cannot make use of it?
> 
> This is what I am going to (try to) do, but I am not sure if this makes
> sense...
> 
> For the start, can't we teach 'uprobe_events' file to accept, say,
> 
> 	'p file:0x1234 pid=1 other-opts'
> 
> for the start? This looks simple enough, and I after looked 
> into tools/perf it seems that perf can be changed too.
> 
> What do you think?

Sounds sensible and functional to me.

> Then we can extend 'pid=' option to accept the list of pids, 
> perhaps.
> 
> In the long term we probably need uprobes/pid_filter or 
> something like this, it should allow to add/del pid 
> dynamically. I really do not know.

For now removing+adding a new one should be enough to 'change' a 
uprobe, right?

Thanks,

	Ingo

Re: [GIT PULL] uprobes: pre-filtering

[Oleg Nesterov]

On 01/25, Ingo Molnar wrote:
>
> * Oleg Nesterov <oleg@redhat.com> wrote:
>
> > On 01/25, Ingo Molnar wrote:
> > >
> > > * Srikar Dronamraju <srikar@linux.vnet.ibm.com> wrote:
> > >
> > > > The other alternative is to extend the current abi and pass
> > > > the prefilter option. Should we extend the abi for userspace
> > > > tracing is obviously debatable.
> > >
> > > That's the obvious path to go - why add something to the kernel
> > > if user-space cannot make use of it?
> >
> > This is what I am going to (try to) do, but I am not sure if this makes
> > sense...
> >
> > For the start, can't we teach 'uprobe_events' file to accept, say,
> >
> > 	'p file:0x1234 pid=1 other-opts'
> >
> > for the start? This looks simple enough, and I after looked
> > into tools/perf it seems that perf can be changed too.
> >
> > What do you think?
>
> Sounds sensible and functional to me.

Great, thanks.

> > Then we can extend 'pid=' option to accept the list of pids,
> > perhaps.
> >
> > In the long term we probably need uprobes/pid_filter or
> > something like this, it should allow to add/del pid
> > dynamically. I really do not know.
>
> For now removing+adding a new one should be enough to 'change' a
> uprobe, right?

Yes, yes. I meant that obviously we can do more/better to filter-out
the tasks we do not want to probe. But this need more changes, and more
importantly this needs more discussion about API/ABI/etc.

Thanks.

Oleg.

Re: [GIT PULL] uprobes: pre-filtering

[Srikar Dronamraju]

* Oleg Nesterov <oleg@redhat.com> [2013-01-25 17:17:28]:

> On 01/25, Ingo Molnar wrote:
> >
> > * Srikar Dronamraju <srikar@linux.vnet.ibm.com> wrote:
> >
> > > The other alternative is to extend the current abi and pass
> > > the prefilter option. Should we extend the abi for userspace
> > > tracing is obviously debatable.
> >
> > That's the obvious path to go - why add something to the kernel
> > if user-space cannot make use of it?
> 
> This is what I am going to (try to) do, but I am not sure if this makes
> sense...
> 
> For the start, can't we teach 'uprobe_events' file to accept, say,
> 
> 	'p file:0x1234 pid=1 other-opts'


I think this would be a very good start


The only downside, I see is we would have add and remove to change the
filter. Something like perf record will not be able to dynamically
change the filter parameter.


> 
> for the start? This looks simple enough, and I after looked into tools/perf
> it seems that perf can be changed too.
> 
> What do you think?
> 
> Then we can extend 'pid=' option to accept the list of pids, perhaps.
> 
> In the long term we probably need uprobes/pid_filter or something like this,
> it should allow to add/del pid dynamically. I really do not know.

Yes, I think having a file like uprobes/pid_filter would be able to
dynamically change the filter.  Probably when we code this up should we
make this something more generic otherwise, we might end up with
uid_filter, sid_filter ..

Steven, 
Are you still pursuing multiple ftrace buffers that you proposed at LPC 2012?

In which case, this new filter should also be per buffer.

-- 
Thanks and Regards
Srikar Dronamraju

Re: [GIT PULL] uprobes: pre-filtering

[Oleg Nesterov]

On 01/28, Srikar Dronamraju wrote:
>
> * Oleg Nesterov <oleg@redhat.com> [2013-01-25 17:17:28]:
>
> > This is what I am going to (try to) do, but I am not sure if this makes
> > sense...
> >
> > For the start, can't we teach 'uprobe_events' file to accept, say,
> >
> > 	'p file:0x1234 pid=1 other-opts'
>
>
> I think this would be a very good start

Good.

So will you agree with 3/4 and 3/4 ?

> The only downside, I see is we would have add and remove to change the
> filter. Something like perf record will not be able to dynamically
> change the filter parameter.

Yes, this is only the first step.

Oleg.