Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1753740AbcD0URY (ORCPT <rfc822;w@1wt.eu>);
	Wed, 27 Apr 2016 16:17:24 -0400
Received: from e23smtp09.au.ibm.com ([202.81.31.142]:53414 "EHLO
	e23smtp09.au.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1753682AbcD0URU (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Wed, 27 Apr 2016 16:17:20 -0400
X-IBM-Helo: d23dlp01.au.ibm.com
X-IBM-MailFrom: hemant@linux.vnet.ibm.com
X-IBM-RcptTo: linux-kernel@vger.kernel.org
Message-ID: <57211E0C.4020205@linux.vnet.ibm.com>
Date: Thu, 28 Apr 2016 01:46:12 +0530
From: Hemant Kumar <hemant@linux.vnet.ibm.com>
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0
MIME-Version: 1.0
To: Arnaldo Carvalho de Melo <acme@kernel.org>
CC: Masami Hiramatsu <mhiramat@kernel.org>, linux-kernel@vger.kernel.org,
        Namhyung Kim <namhyung@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>,
        Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>
Subject: Re: [PATCH perf/core v4 14/19] perf buildid-cache: Scan and import
 user SDT events to probe cache
References: <20160426090200.11891.43944.stgit@devbox> <20160426090422.11891.92396.stgit@devbox> <5720D86C.3010209@linux.vnet.ibm.com> <20160427152816.GP11033@kernel.org>
In-Reply-To: <20160427152816.GP11033@kernel.org>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
X-TM-AS-MML: disable
X-Content-Scanned: Fidelis XPS MAILER
x-cbid: 16042720-0033-0000-0000-000005AB141E
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 3711
Lines: 99



On 04/27/2016 08:58 PM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Apr 27, 2016 at 08:49:08PM +0530, Hemant Kumar escreveu:
>>
>> On 04/26/2016 02:34 PM, Masami Hiramatsu wrote:
>>> From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>>>
>>> perf buildid-cache --add <binary> scans given binary and add
>>> the SDT events to probe cache. "sdt_" prefix is appended for
>>> all SDT providers to avoid event-name clash with other pre-defined
>>> events. It is possible to use the cached SDT events as other cached
>>> events, via perf probe --add "sdt_<provider>:<event>=<event>".
>>>
>>> e.g.
>>>    ----
>>>    # perf buildid-cache --add /lib/libc-2.17.so
>>>    # perf probe --cache --list | head -n 5
>>>    /usr/lib/libc-2.17.so (a6fb821bdf53660eb2c29f778757aef294d3d392):
>>>    sdt_libc:setjmp=setjmp
>>>    sdt_libc:longjmp=longjmp
>>>    sdt_libc:longjmp_target=longjmp_target
>>>    sdt_libc:memory_heap_new=memory_heap_new
>>>    # perf probe -x /usr/lib/libc-2.17.so \
>>>      -a sdt_libc:memory_heap_new=memory_heap_new
>>>    Added new event:
>>>      sdt_libc:memory_heap_new (on memory_heap_new
>>>     in /usr/lib/libc-2.17.so)
>>>
>>>    You can now use it in all perf tools, such as:
>>>
>>>            perf record -e sdt_libc:memory_heap_new -aR sleep 1
>>>
>>>    # perf probe -l
>>>      sdt_libc:memory_heap_new (on new_heap+183 in /usr/lib/libc-2.17.so)
>>>    ----
>> Patch looks good to me. Have a few questions below :
>>
>> What about the same binary path having different build-ids. For e.g
>> a binary say, "test" has some markers, we add it to the cache. And,
>> then the file gets rebuilt again with different build-id now. And we try
>> to add to the cache again. It shows multiple entries in the cache :
>> # perf probe --cache --list
>> /home/hemant/test (157380727e2b3854395aa915dfc91dbccc02058b):
>> sdt_user_test:marker1=marker1
>> /home/hemant/test (64c0a018636e6d5145b09fc65839c1a4a7899f18):
>> sdt_user_test:marker1=marker1
>> /home/hemant/test (c9e34759ae95b68fa385831041c5d9e0dd1697fb):
>> sdt_user_test:marker1=marker1
>> ...
>>
>> But, perf list sdt shows only one entry (which it should) :
>> # perf list sdt
>>
>> List of pre-defined events (to be used in -e):
>>
>>    sdt_user_test:marker1                                [SDT event]
>>
>> perf probe also works as expected :
>> # perf probe -x /home/hemant/test %sdt_user_test:marker1
>> Added new event:
>>    sdt_user_test:marker1  (on %marker1 in /home/hemant/test)
>>
>> You can now use it in all perf tools, such as:
>>
>>      perf record -e sdt_user_test:marker1 -aR sleep 1
>>
>> So, the question is, do we delete the previous entries for "test" from
>> the cache once we get a newer version of "test"?
> No, we shouldn't, since those entries may be used for other tasks that
> involves using the exact DSO used for a particular perf.data session.
>
> Humm, but you are talking about what cache? The "probe cache" or the
> "build-id cache"? My previous statement was about the build-id cache.

I was talking about the "probe cache".

> For the probe cache, humm, probably we want to keep it as well, we may
> have moved that 'test' file to some other place, renamed it, etc, but it
> continues being accessible by its content-based identifier (the
> build-id) and could be used in ways we don't envision right now.

Agreed. Makes sense.

> I.e. the same principle used for the build-id cache should be used for
> this probe cache, where we store things by build-id.
>
> We need to prune this from time to time and for this we have:
>
>    perf buildid-cache purge
>
> But that right now is unflexible, we should have a way to ask to control
> how much is purged :-\

Right.

-- 
Thanks,
Hemant Kumar