Re: [RFC 0/4] trace_uprobe: Support SDT markers having semaphore

From: Masami Hiramatsu
Date: Wed Feb 28 2018 - 09:26:02 EST


Hi Ravi,

Thank you for your great work!

On Wed, 28 Feb 2018 13:23:41 +0530
Ravi Bangoria <ravi.bangoria@xxxxxxxxxxxxxxxxxx> wrote:

> Userspace Statically Defined Tracepoints[1] are dtrace style markers
> inside userspace applications. These markers are added by developer at
> important places in the code. Each marker source expands to a single
> nop instruction in the compiled code but there may be additional
> overhead for computing the marker arguments which expands to couple of
> instructions. If this computaion is quite more, execution of it can be
> ommited by runtime if() condition when no one is tracing on the marker:
>
> if (semaphore > 0) {
> Execute marker instructions;
> }
>
> Default value of semaphore is 0. Tracer has to increment the semaphore
> before recording on a marker and decrement it at the end of tracing.
>
> Currently, perf tool has limited supports for SDT markers. I.e. it
> can not trace markers surrounded by semaphore. Also, it's not easy
> to add semaphore flip logic in userspace tool like perf, so basic
> idea for this patchset is to add semaphore flip logic in the
> trace_uprobe infrastructure. Ex,[2]
>
> # cat tick.c
> ...
> for (i = 0; i < 100; i++) {
> DTRACE_PROBE1(tick, loop1, i);
> if (TICK_LOOP2_ENABLED()) {
> DTRACE_PROBE1(tick, loop2, i);
> }
> printf("hi: %d\n", i);
> sleep(1);
> }
> ...
>
> Here tick:loop1 is marker without semaphore where as tick:loop2 is
> surrounded by semaphore.
>
>
> # perf buildid-cache --add /tmp/tick
> # perf probe sdt_tick:loop1
> # perf probe sdt_tick:loop2
>
> # perf stat -e sdt_tick:loop1,sdt_tick:loop2 -- /tmp/tick
> hi: 0
> hi: 1
> hi: 2
> ^C
> Performance counter stats for '/tmp/tick':
> 3 sdt_tick:loop1
> 0 sdt_tick:loop2
> 2.747086086 seconds time elapsed
>
>
> Perf failed to record data for tick:loop2. Same experiment with this
> patch series:
>
>
> # readelf -n ./tick
> Provider: tick
> Name: loop2
> ... Semaphore: 0x0000000010020036
>
> # readelf -SW ./tick | grep probes
> [25] .probes PROGBITS 0000000010020034 010034
>
>
> Semaphore offset is 0x10036. I don't have educated 'perf probe'
> about semaphore. So instead of using 'perf probe' command, I'm
> manually adding entry in the <tracefs>/uprobe_events file.

Ok, it is easy to pass semaphore address via perf probe :)

> Special char * denotes semaphore offset.
>
>
> # echo "p:sdt_tick/loop2 /tmp/tick:0x6e4 *0x10036" > uprobe_events

IMHO, this syntax is no good, separate with space is only for arguments.
Since the semaphore is per-probe-point based, that should be specified with probe point.
(there are no 2 or more semaphores on 1 event, are there?)
So something like

# echo "p:sdt_tick/loop2 /tmp/tick:0x6e4(0x10036)" > uprobe_events

would be better to me.

Thank you,

>
> # perf stat -e sdt_tick:loop2 -- /tmp/tick
> hi: 0
> hi: 1
> hi: 2
> hi: 3
> ^C
> Performance counter stats for '/tmp/tick':
> 4 sdt_tick:loop2
> 3.359047827 seconds time elapsed
>
>
> Feedback?
>
> TODO:
> - Educate perf tool about semaphore.
> - perf_event_open() now suppoers {k,u}probe event creation[3]. If we
> can supply semaphore offset in perf_event_attr, perf_event_open()
> can be educated to probe SDT marker having semaphore. Though, both
> config1 and config2 are already being used for uprobe and I don't
> see any other attribute which I can use for semaphore offset. Can
> we introduce one more config there? config3?
>
> [1] https://sourceware.org/systemtap/wiki/UserSpaceProbeImplementation
> [2] https://github.com/iovisor/bcc/issues/327#issuecomment-200576506
> [3] https://lkml.org/lkml/2017/12/6/976
>
>
> Ravi Bangoria (4):
> Uprobe: Rename map_info to uprobe_map_info
> Uprobe: Export few functions / data structures
> trace_uprobe: Support SDT markers having semaphore
> trace_uprobe: Fix multiple update of same semaphores
>
> include/linux/uprobes.h | 25 +++++
> kernel/events/uprobes.c | 43 ++++----
> kernel/trace/trace_uprobe.c | 244 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 290 insertions(+), 22 deletions(-)
>
> --
> 1.8.3.1
>


--
Masami Hiramatsu <mhiramat@xxxxxxxxxx>