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

2018-02-28 Thread Ravi Bangoria 


On 02/28/2018 07:55 PM, Masami Hiramatsu wrote:
> Hi Ravi,
>
> Thank you for your great work!

Thanks Masami.

> On Wed, 28 Feb 2018 13:23:41 +0530
> Ravi Bangoria  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:
...
>>
>> 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 /uprobe_events file.
> Ok, it is easy to pass semaphore address via perf probe :)

Yes, it should be fairly easy to parse semaphore at buildid-cache time.
Will add a patch for that.

>
>> 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

This is great suggestion. Will change it.

Please review patch 3 and 4 which contains actual implementation.

Thanks for the review,
Ravi

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

2018-02-28 Thread Ravi Bangoria 


On 02/28/2018 07:55 PM, Masami Hiramatsu wrote:
> Hi Ravi,
>
> Thank you for your great work!

Thanks Masami.

> On Wed, 28 Feb 2018 13:23:41 +0530
> Ravi Bangoria  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:
...
>>
>> 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 /uprobe_events file.
> Ok, it is easy to pass semaphore address via perf probe :)

Yes, it should be fairly easy to parse semaphore at buildid-cache time.
Will add a patch for that.

>
>> 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

This is great suggestion. Will change it.

Please review patch 3 and 4 which contains actual implementation.

Thanks for the review,
Ravi

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

2018-02-28 Thread Ravi Bangoria 


On 02/28/2018 05:36 PM, Srikar Dronamraju wrote:
> * Ravi Bangoria  [2018-02-28 13:23:41]:
>
>>   # readelf -n ./tick
>>   Provider: tick
>>   Name: loop2
>>   ... Semaphore: 0x10020036
>>
>>   # readelf -SW ./tick | grep probes
>>   [25] .probes   PROGBITS10020034 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 /uprobe_events file.
>> Special char * denotes semaphore offset.
>>
>>
>>   # echo "p:sdt_tick/loop2 /tmp/tick:0x6e4 *0x10036" > uprobe_events
>>
>>   # 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.
>>
> Is it possible to extend perf buildcache with a new option to work with
> semaphore?

Yes, that should be fairly easy to do. Will add a patch for that.

Thanks for the review,
Ravi

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

2018-02-28 Thread Ravi Bangoria 


On 02/28/2018 05:36 PM, Srikar Dronamraju wrote:
> * Ravi Bangoria  [2018-02-28 13:23:41]:
>
>>   # readelf -n ./tick
>>   Provider: tick
>>   Name: loop2
>>   ... Semaphore: 0x10020036
>>
>>   # readelf -SW ./tick | grep probes
>>   [25] .probes   PROGBITS10020034 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 /uprobe_events file.
>> Special char * denotes semaphore offset.
>>
>>
>>   # echo "p:sdt_tick/loop2 /tmp/tick:0x6e4 *0x10036" > uprobe_events
>>
>>   # 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.
>>
> Is it possible to extend perf buildcache with a new option to work with
> semaphore?

Yes, that should be fairly easy to do. Will add a patch for that.

Thanks for the review,
Ravi

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

2018-02-28 Thread Masami Hiramatsu 
Hi Ravi,

Thank you for your great work!

On Wed, 28 Feb 2018 13:23:41 +0530
Ravi Bangoria  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: 0x10020036
> 
>   # readelf -SW ./tick | grep probes
>   [25] .probes   PROGBITS10020034 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 /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

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

2018-02-28 Thread Masami Hiramatsu 
Hi Ravi,

Thank you for your great work!

On Wed, 28 Feb 2018 13:23:41 +0530
Ravi Bangoria  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: 0x10020036
> 
>   # readelf -SW ./tick | grep probes
>   [25] .probes   PROGBITS10020034 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 /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

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

2018-02-28 Thread Srikar Dronamraju 
* Ravi Bangoria  [2018-02-28 13:23:41]:

>   # readelf -n ./tick
>   Provider: tick
>   Name: loop2
>   ... Semaphore: 0x10020036
> 
>   # readelf -SW ./tick | grep probes
>   [25] .probes   PROGBITS10020034 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 /uprobe_events file.
> Special char * denotes semaphore offset.
> 
> 
>   # echo "p:sdt_tick/loop2 /tmp/tick:0x6e4 *0x10036" > uprobe_events
> 
>   # 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.
> 

Is it possible to extend perf buildcache with a new option to work with
semaphore?

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

2018-02-28 Thread Srikar Dronamraju 
* Ravi Bangoria  [2018-02-28 13:23:41]:

>   # readelf -n ./tick
>   Provider: tick
>   Name: loop2
>   ... Semaphore: 0x10020036
> 
>   # readelf -SW ./tick | grep probes
>   [25] .probes   PROGBITS10020034 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 /uprobe_events file.
> Special char * denotes semaphore offset.
> 
> 
>   # echo "p:sdt_tick/loop2 /tmp/tick:0x6e4 *0x10036" > uprobe_events
> 
>   # 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.
> 

Is it possible to extend perf buildcache with a new option to work with
semaphore?