2024-01-23 09:21:32

by yebin (H)

Subject: [PATCH v4 7/7] selftests/ftrace: add test cases for VFS type "%pd" and "%pD"

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 <[email protected]>
---
.../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
+;;
+s390*)
+ ARG1=%r2
+;;
+mips*)
+ ARG1=%r4
+;;
+loongarch*)
+ ARG1=%r4
+;;
+riscv*)
+ ARG1=%a0
+;;
+*)
+ 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

2024-01-24 01:32:15

by Masami Hiramatsu

Subject: Re: [PATCH v4 7/7] selftests/ftrace: add test cases for VFS type "%pd" and "%pD"

On Tue, 23 Jan 2024 17:21:39 +0800
Ye Bin <[email protected]> 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 <[email protected]>
> ---
> .../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,

> +;;
> +*)
> + 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) <[email protected]>

2024-01-24 01:53:55

by yebin (H)

Subject: Re: [PATCH v4 7/7] selftests/ftrace: add test cases for VFS type "%pd" and "%pD"

On 2024/1/24 9:32, Masami Hiramatsu (Google) wrote:
> On Tue, 23 Jan 2024 17:21:39 +0800
> Ye Bin <[email protected]> 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 <[email protected]>
>> ---
>> .../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,
Thank you for your advice.
Actually, I wrote the test case by referring to "
kprobe_args_string.tc". I'll modify it
according to your suggestion.

>> +;;
>> +*)
>> + 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
>>
>

2024-01-24 03:22:20

by yebin (H)

Subject: Re: [PATCH v4 7/7] selftests/ftrace: add test cases for VFS type "%pd" and "%pD"

On 2024/1/24 9:32, Masami Hiramatsu (Google) wrote:
> On Tue, 23 Jan 2024 17:21:39 +0800
> Ye Bin <[email protected]> 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 <[email protected]>
>> ---
>> .../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.
>> +;;
>> +*)
>> + 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
>>
>

2024-01-24 23:31:02

by Masami Hiramatsu

Subject: Re: [PATCH v4 7/7] selftests/ftrace: add test cases for VFS type "%pd" and "%pD"

On Wed, 24 Jan 2024 11:21:45 +0800
"yebin (H)" <[email protected]> wrote:

>
>
> On 2024/1/24 9:32, Masami Hiramatsu (Google) wrote:
> > On Tue, 23 Jan 2024 17:21:39 +0800
> > Ye Bin <[email protected]> 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 <[email protected]>
> >> ---
> >> .../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) <[email protected]>