Re: [PATCH v9 3.2 0/9] Uprobes patchset with perf probe support

[Ingo Molnar]

* Srikar Dronamraju <srikar@xxxxxxxxxxxxxxxxxx> wrote:

> > We already have some other vma tracking goodies in perf 
> > itself (see perf_event_mmap() et al) - would it make sense 
> > to merge the two vma instrumentation facilities and not 
> > burden mm/ with two separate sets of callbacks?
> 
> Atleast for file backed vmas, perf_event_mmap seems to be 
> interested in just the new vma creations. Uprobes would also 
> be interested in the size changes like the vma 
> growing/shrinking/remap. Is perf_event_mmap interested in such 
> changes? From what i could see, perf_event_mmap seems to be 
> interested in stack vma size changes but not file vma size 
> changes.

I don't think perf_event_mmap() would be hurt from getting more 
callbacks, as long as there's enough metadata to differentiate 
the various events from each other.

( In the long run we really want all such callbacks to be
  tracepoints so that we can make them even less intrusive via
  jump label patching optimizations, etc. - but we are not there
  yet. )

> Also mmap_uprobe gets called in fork path. Currently we have a 
> hook in copy_mm/dup_mm so that we get to know the context of 
> each vma that gets added to the child and add its breakpoints. 
> At dup_mm/dup_mmap we would have taken mmap_sem for both 
> parent and child so there is no way we could have missed a 
> register/unregister in the parent not reflected in the child.
> 
> I see the perf_event_fork but that would have to enhanced to 
> do a lot more to help us do a mmap_uprobe.

Andrew's call i guess.

I did not suggest anything complex or intrusive: just basically 
unify the namespace, have a single set of callbacks, and call 
into the uprobes and perf code from those callbacks - out of the 
sight of MM code.

That unified namespace could be called:

    event_mmap(...);
    event_fork(...);

etc. - and from event_mmap() you could do a simple:

	perf_event_mmap(...)
	uprobes_event_mmap(...)

[ Once all this is updated to use tracepoints it would turn into 
  a notification callback chain kind of thing. ]

> > If all such issues are resolved then i guess we could queue 
> > up uprobes in -tip, conditional on it remaining sufficiently 
> > regression-, problem- and NAK-free.
> 
> Okay. Accepting uprobes into tip, would provide more 
> testing/feedback.
> 
> > Also, it would be nice to hear Arnaldo's opinion about the 
> > tools/perf/ bits.
> 
> Whatever comments Arnaldo/Masami have given till now have been 
> resolved.

Please see the probe installation syntax feedback i've sent in 
the previous mail - that needs to be addressed too.

You should think with the head of an ordinary user-space 
developer who wants to say debug a library and wants to use 
uprobes for that. What would he have to type to achieve that and 
is what he types the minimum number of characters to reasonably 
achieve that goal?

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/