From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752042AbdF1ORB (ORCPT ); Wed, 28 Jun 2017 10:17:01 -0400 Received: from mail.kernel.org ([198.145.29.99]:53068 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751583AbdF1OQz (ORCPT ); Wed, 28 Jun 2017 10:16:55 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C7D39214D7 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=mhiramat@kernel.org Date: Wed, 28 Jun 2017 23:16:50 +0900 From: Masami Hiramatsu To: "Naveen N. Rao" Cc: Steven Rostedt , Ananth N Mavinakayanahalli , Michael Ellerman , linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH 2/2] selftests/ftrace: Update multiple kprobes test for powerpc Message-Id: <20170628231650.6fb5d14fb00923d29bd313d7@kernel.org> In-Reply-To: <20170628092846.uwc55zuq7dsbrzho@naverao1-tp.localdomain> References: <20170622180738.e97092d4fa6458b9199a03d7@kernel.org> <20170622170325.qznheui7eydkdnmv@naverao1-tp.localdomain> <20170624023021.92f2f90f0e09314ac9bdb87a@kernel.org> <20170624200610.f3aee818b8f6ff2631e0d56a@kernel.org> <20170628092846.uwc55zuq7dsbrzho@naverao1-tp.localdomain> X-Mailer: Sylpheed 3.5.0 (GTK+ 2.24.31; x86_64-redhat-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 28 Jun 2017 14:58:46 +0530 "Naveen N. Rao" wrote: > On 2017/06/24 08:06PM, Masami Hiramatsu wrote: > > On Sat, 24 Jun 2017 02:30:21 +0900 > > Masami Hiramatsu wrote: > > > > > On Thu, 22 Jun 2017 22:33:25 +0530 > > > "Naveen N. Rao" wrote: > > > > > > > On 2017/06/22 06:07PM, Masami Hiramatsu wrote: > > > > > On Thu, 22 Jun 2017 00:20:28 +0530 > > > > > "Naveen N. Rao" wrote: > > > > > > > > > > > KPROBES_ON_FTRACE is only available on powerpc64le. Update comment to > > > > > > clarify this. > > > > > > > > > > > > Also, we should use an offset of 8 to ensure that the probe does not > > > > > > fall on ftrace location. The current offset of 4 will fall before the > > > > > > function local entry point and won't fire, while an offset of 12 or 16 > > > > > > will fall on ftrace location. Offset 8 is currently guaranteed to not be > > > > > > the ftrace location. > > > > > > > > > > OK, these part seems good to me. > > > > > > > > > > > > > > > > > Finally, do not filter out symbols with a dot. Powerpc Elfv1 uses dot > > > > > > prefix for all functions and this prevents us from testing some of those > > > > > > symbols. Furthermore, with the patch to derive event names properly in > > > > > > the presence of ':' and '.', such names are accepted by kprobe_events > > > > > > and constitutes a good test for those symbols. > > > > > > > > > > Hmm, the reason why I added such filter was to avoid symbols including > > > > > gcc-generated suffixes like as .constprop or .isra etc. > > > > > > > > I see. > > > > > > > > I do wonder -- is there a problem if we try probing those symbols? On my > > > > local x86 vm, I don't see an issue probing it especially with the > > > > previous patch to enable probing with symbols having a '.' or ':'. > > > > > > > > Furthermore, since this is for testing kprobe_events, I feel it is good > > > > to try probing those symbols too to catch any weird errors we may hit. > > > > > > Yes, and that is not what this testcase is aiming to. That testcase should > > > be a separated one, with correct error handling. > > Ok, I will re-send the patch by changing the pattern. > > > > > Hi Naveen, > > > > Here is the testcase which I meant above. This may help if there is any > > regression related to this specific issue. > > Nice! > I tested this and there are a few small issues on powerpc... > > > > > Thank you, > > > > ----- > > > > selftests: ftrace: Add a testcase for kprobe event naming > > > > From: Masami Hiramatsu > > > > Add a testcase for kprobe event naming. This testcase > > checks whether the kprobe events can automatically ganerate > > its event name on normal function and dot-suffixed function. > > Also it checks whether the kprobe events can correctly > > define new event with given event name and group name. > > > > Signed-off-by: Masami Hiramatsu > > --- > > .../ftrace/test.d/kprobe/kprobe_eventname.tc | 28 ++++++++++++++++++++ > > 1 file changed, 28 insertions(+) > > create mode 100644 tools/testing/selftests/ftrace/test.d/kprobe/kprobe_eventname.tc > > > > diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_eventname.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_eventname.tc > > new file mode 100644 > > index 0000000..d259031 > > --- /dev/null > > +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_eventname.tc > > @@ -0,0 +1,28 @@ > > +#!/bin/sh > > +# description: Kprobe event auto/manual naming > > + > > +disable_events > > +echo > kprobe_events > > + > > +:;: "Add an event on function without name" ;: > > + > > +FUNC=`grep -m 10 " [tT] [^.]*$" /proc/kallsyms | tail -n 1 | cut -f 3 -d " "` > > On powerpc, this always ends up using a blacklisted function. So, I > think we need a way to find a function that is not black listed. Hmm, if we increase the -m argument, like -m 100, is that still in a blacklisted function? > However, one of the issues is that debugfs does not show all the address > ranges that are blacklisted. I am coming up with a way to address that > and will post patches once I have it working. Would you find that is only on powerpc or generic issue? > > With those patches, we should be able to select symbols that are not > blacklisted. Or, maybe we can use generic function, like "schedule" or "vfs_read" etc. > > > +echo p $FUNC > kprobe_events > > +test -d events/kprobes/p_${FUNC}_0 || exit_failure > > + > > +:;: "Add an event on function with new name" ;: > > + > > +echo p:event1 $FUNC > kprobe_events > > +test -d events/kprobes/event1 || exit_failure > > + > > +:;: "Add an event on function with new name and group" ;: > > + > > +echo p:kprobes2/event2 $FUNC > kprobe_events > > +test -d events/kprobes2/event2 || exit_failure > > + > > +:;: "Add an event on dot function without name" ;: > > + > > +FUNC=`grep -m 10 " [tT] .*\..*$" /proc/kallsyms | tail -n 1 | cut -f 3 -d " "` > > +echo p $FUNC > kprobe_events > > +EVENT=`grep $FUNC kprobe_events | cut -f 1 -d " " | cut -f 2 -d:` || exit_failure > > +test -d events/$EVENT || exit_failure > > Probably good to add 'echo > kprobe_events' at the end just to clean up? Ah, good catch! Thanks! > > > Thanks, > Naveen > -- Masami Hiramatsu