From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756646AbcECVSX (ORCPT ); Tue, 3 May 2016 17:18:23 -0400 Received: from e28smtp05.in.ibm.com ([125.16.236.5]:53332 "EHLO e28smtp05.in.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756624AbcECVSV (ORCPT ); Tue, 3 May 2016 17:18:21 -0400 X-IBM-Helo: d28dlp02.in.ibm.com X-IBM-MailFrom: hemant@linux.vnet.ibm.com X-IBM-RcptTo: linux-kernel@vger.kernel.org Message-ID: <57291589.2040302@linux.vnet.ibm.com> Date: Wed, 04 May 2016 02:48:01 +0530 From: Hemant Kumar User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Masami Hiramatsu CC: linux-kernel@vger.kernel.org, acme@kernel.org, peterz@infradead.org, namhyung@kernel.org, mingo@redhat.com, ananth@linux.vnet.ibm.com Subject: Re: [PATCH] perf/sdt: Directly record cached SDT events References: <1461937241-347-1-git-send-email-hemant@linux.vnet.ibm.com> <20160430213633.36a0292f0c3eb25926c62d91@kernel.org> <5727E478.6030208@linux.vnet.ibm.com> <20160503093526.12bd508cd8df8b09197c5100@kernel.org> In-Reply-To: <20160503093526.12bd508cd8df8b09197c5100@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: 16050321-0017-0000-0000-00001E820B7D Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/03/2016 06:05 AM, Masami Hiramatsu wrote: > On Tue, 03 May 2016 05:06:24 +0530 > Hemant Kumar wrote: > >> 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 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? > Yes, but for the symbols or lines etc., user can not directly specify > it via perf record. And as you showed below, perf-probe expresses > there are 2 events on the probe point. So user is forced to aware of it. Right. >> # 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. > Not only the different address, but also they will see the different > event names. That may be no good for making a script on it. > > My point is, if the user only uses "perf record -e sdt_something:sdtevent", > they will think that there is one event recorded. it can easily misleading > them. Ok. Makes sense. With a warning message then, we can make the user aware in this case. >>> 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 least this should be noticed to users carefully. (e.g. warn if > there are more than two SDTs defined) Ok. I have made the changes and also added a warning message if the user tries to record on an sdt event, which has multiple occurences with the same event and group name. I have sent a v2 for this patch. -- Thanks, Hemant Kumar