Received-SPF: pass (google.com: domain of linux-kernel+bounces-37799-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Thu, 25 Jan 2024 08:30:50 +0900
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
To: "yebin (H)" <yebin10@huawei.com>
Cc: <rostedt@goodmis.org>, <mathieu.desnoyers@efficios.com>,
 <linux-trace-kernel@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 7/7] selftests/ftrace: add test cases for VFS type
 "%pd" and "%pD"
Message-Id: <20240125083050.5aca0f58c3936206ec1827f9@kernel.org>
In-Reply-To: <65B08249.6050704@huawei.com>
References: <20240123092139.3698375-1-yebin10@huawei.com>
	<20240123092139.3698375-8-yebin10@huawei.com>
	<20240124103201.6dd8d466b1b734796418c1c4@kernel.org>
	<65B08249.6050704@huawei.com>
Precedence: bulk
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

On Wed, 24 Jan 2024 11:21:45 +0800
"yebin (H)" <yebin10@huawei.com> wrote:

> 
> 
> On 2024/1/24 9:32, Masami Hiramatsu (Google) wrote:
> > On Tue, 23 Jan 2024 17:21:39 +0800
> > Ye Bin <yebin10@huawei.com> wrote:
> >
> >> This patch adds test cases for new print format type "%pd/%pD".The test cases
> >> test the following items:
> >> 1. Test README if add "%pd/%pD" type;
> >> 2. Test "%pd" type for dput();
> >> 3. Test "%pD" type for vfs_read();
> >>
> >> Signed-off-by: Ye Bin <yebin10@huawei.com>
> >> ---
> >>   .../ftrace/test.d/kprobe/kprobe_args_vfs.tc   | 79 +++++++++++++++++++
> >>   1 file changed, 79 insertions(+)
> >>   create mode 100644 tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_vfs.tc
> >>
> >> diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_vfs.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_vfs.tc
> >> new file mode 100644
> >> index 000000000000..1d8edd294dd6
> >> --- /dev/null
> >> +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_vfs.tc
> >> @@ -0,0 +1,79 @@
> >> +#!/bin/sh
> >> +# SPDX-License-Identifier: GPL-2.0
> >> +# description: Kprobe event VFS type argument
> >> +# requires: kprobe_events
> >> +
> >> +case `uname -m` in
> >> +x86_64)
> >> +  ARG1=%di
> >> +;;
> >> +i[3456]86)
> >> +  ARG1=%ax
> >> +;;
> >> +aarch64)
> >> +  ARG1=%x0
> >> +;;
> >> +arm*)
> >> +  ARG1=%r0
> >> +;;
> >> +ppc64*)
> >> +  ARG1=%r3
> >> +;;
> >> +ppc*)
> >> +  ARG1=%r3
> > You can merge this ppc* and ppc64* cases :)
> >
> >> +;;
> >> +s390*)
> >> +  ARG1=%r2
> >> +;;
> >> +mips*)
> >> +  ARG1=%r4
> >> +;;
> >> +loongarch*)
> >> +  ARG1=%r4
> >> +;;
> >> +riscv*)
> >> +  ARG1=%a0
> > Anyway, I wonder why don't you use '$arg1' instead of these registers.
> > Is there any reason?
> >
> > Thank you,
> I looked at the parameter parsing code again, and using "$arg1" requires 
> the kernel to
> enable the CONFIG_HAVE_FUNCTION_ARG_ACCESS_API configuration.

Yes, and it is recommended (required) for supporting kprobe event
via ftrace. So, if you see any error on this test, that machine should
implement it.

Thank you,

> >> +;;
> >> +*)
> >> +  echo "Please implement other architecture here"
> >> +  exit_untested
> >> +esac
> >> +
> >> +: "Test argument %pd/%pD in README"
> >> +grep -q "%pd/%pD" README
> >> +
> >> +: "Test argument %pd with name"
> >> +echo "p:testprobe dput name=${ARG1}:%pd" > kprobe_events
> >> +echo 1 > events/kprobes/testprobe/enable
> >> +grep -q "1" events/kprobes/testprobe/enable
> >> +echo 0 > events/kprobes/testprobe/enable
> >> +grep "dput" trace | grep -q "enable"
> >> +echo "" > kprobe_events
> >> +echo "" > trace
> >> +
> >> +: "Test argument %pd without name"
> >> +echo "p:testprobe dput ${ARG1}:%pd" > kprobe_events
> >> +echo 1 > events/kprobes/testprobe/enable
> >> +grep -q "1" events/kprobes/testprobe/enable
> >> +echo 0 > events/kprobes/testprobe/enable
> >> +grep "dput" trace | grep -q "enable"
> >> +echo "" > kprobe_events
> >> +echo "" > trace
> >> +
> >> +: "Test argument %pD with name"
> >> +echo "p:testprobe vfs_read name=${ARG1}:%pD" > kprobe_events
> >> +echo 1 > events/kprobes/testprobe/enable
> >> +grep -q "1" events/kprobes/testprobe/enable
> >> +echo 0 > events/kprobes/testprobe/enable
> >> +grep "vfs_read" trace | grep -q "enable"
> >> +echo "" > kprobe_events
> >> +echo "" > trace
> >> +
> >> +: "Test argument %pD without name"
> >> +echo "p:testprobe vfs_read ${ARG1}:%pD" > kprobe_events
> >> +echo 1 > events/kprobes/testprobe/enable
> >> +grep -q "1"  events/kprobes/testprobe/enable
> >> +echo 0 > events/kprobes/testprobe/enable
> >> +grep "vfs_read" trace | grep -q "enable"
> >> +echo "" > kprobe_events
> >> +echo "" > trace
> >> -- 
> >> 2.31.1
> >>
> >
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>