Re: [3/3 PATCH] Kprobes: User space probes support- single stepping out-of-line

[Andrew Morton <akpm@osdl.org>]

Prasanna S Panchamukhi <prasanna@in.ibm.com> wrote:
>
> > 
>  > > > > +	addr = (kprobe_opcode_t *)kmap_atomic(page, KM_USER1);
>  > > > > +	addr = (kprobe_opcode_t *)((unsigned long)addr +
>  > > > > +				 (unsigned long)(uprobe->offset & ~PAGE_MASK));
>  > > > > +	*addr = opcode;
>  > > > > +	/*TODO: flush vma ? */
>  > > > 
>  > > > flush_dcache_page() would be needed.
>  > > > 
>  > > > But then, what happens if the page is shared by other processes?  Do they
>  > > > all start taking debug traps?
>  > > 
>  > > Yes, you are right. I think single stepping inline was a bad idea, disarming
>  > > the probe looks to be a better option
>  > > 
>  > 
>  > You skipped my second question?
> 
>  There is a small window in which other processor will not be able to see
>  the breakpoint if we are single step inline. But since single stepping inline
>  is a bad idea, we will disarm the probe forever (replace with original instrcution) if we cannot single step out-of-line.
>  Any suggestions?

This doesn't appear to be working.

Let's go back in time and pretend that these patches were never written,
OK?  We're standing around the water cooler saying "hey, wouldn't it be
cool if someone did X".  You guys are way too far into this and you keep on
leaving everyone else behind.  When I try to drag you up, you resist ;)

So let me rephrase, and thrash around in the dark a little more.

>From my reading of the code (and thus far that's my _only_ source of this
information) it appears that a design decision has been made (for reasons
which have yet to be disclosed) that the way to implement this (yet to be
described) requirement is to set user breakpoints upon particular
instructions within executables.  System-wide, for all processes and
threads.

There are other things that could have been done.  For example, you might
have chosen to set breakpoints upon a particular virtual address within a
heavyweight process.  That's a process-oriented viewpoint, rather than a
file-oriented one, if you like.

This raises interesting questions, like

- How come that decision was made?  Why _is_ this an executable-oriented
  rather than process-oriented thing?  Richard has covered that somewhat.

- What happens if the executable is writeably mapped?

- What happens if someone writes to the executable?  (I think both
  of these were disallowed in the implementation-which-is-not-to-be-named).

- What happens if different processes map the executable at different
  addresses?

- Various other things which you've thought of and I haven't and which it
  would be REALLY interesting to hear about.

But this is just one example.  I don't think I'm being too picky here - my
reaction on seeing all this stuff was, basically, "wtf is all this code
for?".  References to dprobes won't help, sorry - I've never looked at
dprobes and I don't know anyone apart from you guys who has.

What I'm asking you for is a description of what problem we're trying to
solve and how this code solves that problem.  It is hard, it is inefficient
and, worse, it is error-prone for us to try to work all that out from a
particular implementation.