linux-kernel - Re: [PATCH] perf/sdt: Directly record cached SDT events

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 03 May 2016 05:06:24 +0530
From:	Hemant Kumar <hemant@...ux.vnet.ibm.com>
To:	Masami Hiramatsu <mhiramat@...nel.org>
CC:	linux-kernel@...r.kernel.org, acme@...nel.org,
	peterz@...radead.org, namhyung@...nel.org, mingo@...hat.com,
	ananth@...ux.vnet.ibm.com
Subject: Re: [PATCH] perf/sdt: Directly record cached SDT events

Hi Masami,

On 04/30/2016 06:06 PM, Masami Hiramatsu wrote:
> Hi Hemant,
>
> On Fri, 29 Apr 2016 19:10:41 +0530
> Hemant Kumar <hemant@...ux.vnet.ibm.com> wrote:
>
>> This patch adds support for directly recording SDT events which are
>> present in the probe cache. This patch is based on current SDT
>> enablement patchset (v5) by Masami :
>> https://lkml.org/lkml/2016/4/27/828
>> and it implements two points in the TODO list mentioned in the
>> cover note :
>> "- (perf record) Support SDT event recording directly"
>> "- (perf record) Try to unregister SDT events after record."
>>
>> Without this patch, we could probe into SDT events using
>> "perf probe" and "perf record". With this patch, we can probe
>> the SDT events directly using "perf record".
> Thanks! However, before looking over each part of this patch,
> I think this is not enough for supporting SDT for perf record.

Hmm.

> If there are several SDTs which have same eventname but differnt
> addresses (e.g. libc:memory_memalign_retry), how are those handled?
> Currently, to support this, we'll need to enable those events
> in different names, or just pick one of them. It could confuse
> users in each case.

Right. But now, its the same case with a binary having multiple
symbols with same names, isn't it?

# nm ./multi | grep foo
0000000000400530 t foo
0000000000400560 t foo

# perf probe -x ./multi foo
Added new events:
   probe_multi:foo      (on foo in /home/hemant/work/linux/tools/perf/multi)
   probe_multi:foo_1    (on foo in /home/hemant/work/linux/tools/perf/multi)

You can now use it in all perf tools, such as:

     perf record -e probe_multi:foo_1 -aR sleep 1


My point being, the user can still know, if its shown that there are two or
more probes being placed and the o/p of perf report/script shows that
the probes are placed at two or more different addresses.

> To solve this issue, we need to introduce multiple SDTs on single
> ftrace event. Please read my comment on v3 patch (https://lkml.org/lkml/2015/8/15/52)

Ok. But, I think, for initial direct recording support, we can go with 
this IMHO.

> So, at this point, I've decided to introduce only perf-probe side.
> If user want to record SDT, they can use perf-probe to add SDT events
> and see what events are generated by SDT, so that they can specify those
> events via perf-record.
> e.g.
>
> # perf probe -a %sdt_libc:*
> ...
>    sdt_libc:lll_futex_wake_1 (on %* in /usr/lib64/libc-2.20.so)
>    sdt_libc:lll_lock_wait_private (on %* in /usr/lib64/libc-2.20.so)
>
> You can now use it in all perf tools, such as:
>
> 	perf record -e sdt_libc:lll_lock_wait_private -aR sleep 1
>
> # perf record -e sdt_libc:* -a
> ^C[ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 1.461 MB perf.data (6195 samples) ]
>
> # perf script
>   plugin_audio_th 11119 [000] 16059.864654:   sdt_libc:setjmp: (7f37bf55b6c1)
>            chrome  4650 [000] 16059.881345:   sdt_libc:setjmp: (7f37bf55b6c1)
>            chrome  4650 [000] 16059.881350:   sdt_libc:setjmp: (7f37bf55b6c1)
>            chrome  4650 [000] 16059.881411:   sdt_libc:setjmp: (7f37bf55b6c1)
> ...
>
> BTW, see below comments on the code for implementation issues.

Will fix the issues and send a v2.

Thanks a lot for the review.

-- 
Thanks,
Hemant Kumar