[PATCH v4 0/7] support '%pd' and '%pD' for print file name

[PATCH v4 0/7] support '%pd' and '%pD' for print file name

[Ye Bin]

During fault locating, the file name needs to be printed based on the
dentry/file address. The offset needs to be calculated each time, which
is troublesome. Similar to printk, kprobe supports printing file names
for dentry/file addresses.

Diff v4 vs v3:
1. Use "argv[i][idx + 3] == 'd'" instead of "argv[i][strlen(argv[i]) - 1] == 'd'"
to judge print format in PATCH[4/7];

Diff v3 vs v2:
1. Return the index of where the suffix was found in str_has_suffix();

Diff v2 vs v1:
1. Use "%pd/%pD" print format instead of "pd/pD" print format;
2. Add "%pd/%pD" in README;
3. Expand "%pd/%pD" argument before parameter parsing;
4. Add more detail information in ftrace documentation;
5. Add test cases for new print format in selftests/ftrace;

Ye Bin (7):
  string.h: add str_has_suffix() helper for test string ends with
    specify string
  tracing/probes: add traceprobe_expand_dentry_args() helper
  tracing/probes: support '%pd' type for print struct dentry's name
  tracing/probes: support '%pD' type for print struct file's name
  tracing: add new type "%pd/%pD" in readme_msg[]
  Documentation: tracing: add new type '%pd' and '%pD' for kprobe
  selftests/ftrace: add test cases for VFS type "%pd" and "%pD"

 Documentation/trace/kprobetrace.rst           |  6 +-
 include/linux/string.h                        | 28 +++++++
 kernel/trace/trace.c                          |  2 +-
 kernel/trace/trace_kprobe.c                   |  6 ++
 kernel/trace/trace_probe.c                    | 47 +++++++++++
 kernel/trace/trace_probe.h                    |  3 +
 .../ftrace/test.d/kprobe/kprobe_args_vfs.tc   | 79 +++++++++++++++++++
 7 files changed, 169 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_vfs.tc

-- 
2.31.1

[PATCH v4 1/7] string.h: add str_has_suffix() helper for test string ends with specify string

[Ye Bin]

str_has_suffix() is test string if ends with specify string, and also
this API may return the index of where the suffix was found.

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 include/linux/string.h | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/include/linux/string.h b/include/linux/string.h
index 433c207a01da..2fb0f22237fe 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -405,4 +405,32 @@ static __always_inline size_t str_has_prefix(const char *str, const char *prefix
 	return strncmp(str, prefix, len) == 0 ? len : 0;
 }
 
+/**
+ * str_has_suffix - Test if a string has a given suffix
+ * @str: The string to test
+ * @suffix: The string to see if @str ends with
+ * @index: The index into @str of where @suffix is if found (NULL to ignore)
+ *
+ * Returns:
+ * * strlen(@suffix) if @str ends with @suffix
+ * * 0 if @str does not end with @suffix
+ */
+static __always_inline size_t str_has_suffix(const char *str, const char *suffix,
+					     size_t *index)
+{
+	size_t len = strlen(suffix);
+	size_t str_len = strlen(str);
+
+	if (len > str_len)
+		return 0;
+
+	if (strncmp(str + str_len - len, suffix, len))
+		return 0;
+
+	if (index)
+		*index = str_len - len;
+
+	return len;
+}
+
 #endif /* _LINUX_STRING_H_ */
-- 
2.31.1

[PATCH v4 2/7] tracing/probes: add traceprobe_expand_dentry_args() helper

[Ye Bin]

Add traceprobe_expand_dentry_args() to expand dentry args. this API is
prepare to support "%pd" print format for kprobe.

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 kernel/trace/trace_probe.c | 36 ++++++++++++++++++++++++++++++++++++
 kernel/trace/trace_probe.h |  2 ++
 2 files changed, 38 insertions(+)

diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 4dc74d73fc1d..cc8bd7ea5341 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -1565,6 +1565,42 @@ const char **traceprobe_expand_meta_args(int argc, const char *argv[],
 	return ERR_PTR(ret);
 }
 
+int traceprobe_expand_dentry_args(int argc, const char *argv[], char *buf,
+				  int bufsize)
+{
+	int i, used, ret;
+
+	used = 0;
+	for (i = 0; i < argc; i++) {
+		size_t idx;
+
+		if (str_has_suffix(argv[i], ":%pd", &idx)) {
+			char *tmp = kstrdup(argv[i], GFP_KERNEL);
+			char *equal;
+
+			if (!tmp)
+				return -ENOMEM;
+
+			equal = strchr(tmp, '=');
+			if (equal)
+				*equal = '\0';
+			tmp[idx] = '\0';
+			ret = snprintf(buf + used, bufsize - used,
+				       "%s%s+0x0(+0x%zx(%s)):string",
+				       equal ? tmp : "", equal ? "=" : "",
+				       offsetof(struct dentry, d_name.name),
+				       equal ? equal + 1 : tmp);
+			kfree(tmp);
+			if (ret >= bufsize - used)
+				return -ENOMEM;
+			argv[i] = buf + used;
+			used += ret + 1;
+		}
+	}
+
+	return 0;
+}
+
 void traceprobe_finish_parse(struct traceprobe_parse_context *ctx)
 {
 	clear_btf_context(ctx);
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 850d9ecb6765..553371a4e0b1 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -402,6 +402,8 @@ extern int traceprobe_parse_probe_arg(struct trace_probe *tp, int i,
 const char **traceprobe_expand_meta_args(int argc, const char *argv[],
 					 int *new_argc, char *buf, int bufsize,
 					 struct traceprobe_parse_context *ctx);
+extern int traceprobe_expand_dentry_args(int argc, const char *argv[], char *buf,
+					 int bufsize);
 
 extern int traceprobe_update_arg(struct probe_arg *arg);
 extern void traceprobe_free_probe_arg(struct probe_arg *arg);
-- 
2.31.1

[PATCH v4 3/7] tracing/probes: support '%pd' type for print struct dentry's name

[Ye Bin]

Similar to '%pd' for printk, use '%pd' for print struct dentry's name.

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 kernel/trace/trace_kprobe.c | 6 ++++++
 kernel/trace/trace_probe.h  | 1 +
 2 files changed, 7 insertions(+)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index c4c6e0e0068b..00b74530fbad 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -779,6 +779,7 @@ static int __trace_kprobe_create(int argc, const char *argv[])
 	char buf[MAX_EVENT_NAME_LEN];
 	char gbuf[MAX_EVENT_NAME_LEN];
 	char abuf[MAX_BTF_ARGS_LEN];
+	char dbuf[MAX_DENTRY_ARGS_LEN];
 	struct traceprobe_parse_context ctx = { .flags = TPARG_FL_KERNEL };
 
 	switch (argv[0][0]) {
@@ -930,6 +931,11 @@ static int __trace_kprobe_create(int argc, const char *argv[])
 		argv = new_argv;
 	}
 
+	ret = traceprobe_expand_dentry_args(argc, argv, dbuf,
+					    MAX_DENTRY_ARGS_LEN);
+	if (ret)
+		goto out;
+
 	/* setup a probe */
 	tk = alloc_trace_kprobe(group, event, addr, symbol, offset, maxactive,
 				argc, is_return);
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 553371a4e0b1..d9c053824975 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -34,6 +34,7 @@
 #define MAX_ARRAY_LEN		64
 #define MAX_ARG_NAME_LEN	32
 #define MAX_BTF_ARGS_LEN	128
+#define MAX_DENTRY_ARGS_LEN	256
 #define MAX_STRING_SIZE		PATH_MAX
 #define MAX_ARG_BUF_LEN		(MAX_TRACE_ARGS * MAX_ARG_NAME_LEN)
 
-- 
2.31.1

[PATCH v4 4/7] tracing/probes: support '%pD' type for print struct file's name

[Ye Bin]

Similar to '%pD' for printk, use '%pD' for print struct file's name.

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 kernel/trace/trace_probe.c | 41 ++++++++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index cc8bd7ea5341..a664633137a0 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -12,6 +12,7 @@
 #define pr_fmt(fmt)	"trace_probe: " fmt
 
 #include <linux/bpf.h>
+#include <linux/fs.h>
 #include "trace_btf.h"
 
 #include "trace_probe.h"
@@ -1574,28 +1575,38 @@ int traceprobe_expand_dentry_args(int argc, const char *argv[], char *buf,
 	for (i = 0; i < argc; i++) {
 		size_t idx;
 
-		if (str_has_suffix(argv[i], ":%pd", &idx)) {
-			char *tmp = kstrdup(argv[i], GFP_KERNEL);
-			char *equal;
+		if (!str_has_suffix(argv[i], ":%pd", &idx) &&
+		    !str_has_suffix(argv[i], ":%pD", &idx))
+			continue;
 
-			if (!tmp)
-				return -ENOMEM;
+		char *tmp = kstrdup(argv[i], GFP_KERNEL);
+		char *equal;
+
+		if (!tmp)
+			return -ENOMEM;
 
-			equal = strchr(tmp, '=');
-			if (equal)
-				*equal = '\0';
-			tmp[idx] = '\0';
+		equal = strchr(tmp, '=');
+		if (equal)
+			*equal = '\0';
+		tmp[idx] = '\0';
+		if (argv[i][idx + 3] == 'd')
 			ret = snprintf(buf + used, bufsize - used,
 				       "%s%s+0x0(+0x%zx(%s)):string",
 				       equal ? tmp : "", equal ? "=" : "",
 				       offsetof(struct dentry, d_name.name),
 				       equal ? equal + 1 : tmp);
-			kfree(tmp);
-			if (ret >= bufsize - used)
-				return -ENOMEM;
-			argv[i] = buf + used;
-			used += ret + 1;
-		}
+		else
+			ret = snprintf(buf + used, bufsize - used,
+				       "%s%s+0x0(+0x%zx(+0x%zx(%s))):string",
+				       equal ? tmp : "", equal ? "=" : "",
+				       offsetof(struct dentry, d_name.name),
+				       offsetof(struct file, f_path.dentry),
+				       equal ? equal + 1 : tmp);
+		kfree(tmp);
+		if (ret >= bufsize - used)
+			return -ENOMEM;
+		argv[i] = buf + used;
+		used += ret + 1;
 	}
 
 	return 0;
-- 
2.31.1

[PATCH v4 5/7] tracing: add new type "%pd/%pD" in readme_msg[]

[Ye Bin]

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 kernel/trace/trace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 2a7c6fd934e9..13197d3b86bd 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -5745,7 +5745,7 @@ static const char readme_msg[] =
 	"\t           +|-[u]<offset>(<fetcharg>), \\imm-value, \\\"imm-string\"\n"
 	"\t     type: s8/16/32/64, u8/16/32/64, x8/16/32/64, char, string, symbol,\n"
 	"\t           b<bit-width>@<bit-offset>/<container-size>, ustring,\n"
-	"\t           symstr, <type>\\[<array-size>\\]\n"
+	"\t           symstr, %pd/%pD <type>\\[<array-size>\\]\n"
 #ifdef CONFIG_HIST_TRIGGERS
 	"\t    field: <stype> <name>;\n"
 	"\t    stype: u8/u16/u32/u64, s8/s16/s32/s64, pid_t,\n"
-- 
2.31.1

[PATCH v4 6/7] Documentation: tracing: add new type '%pd' and '%pD' for kprobe

[Ye Bin]

Similar to printk() '%pd' is for fetch dentry's name from struct dentry's
pointer, and '%pD' is for fetch file's name from struct file's pointer.

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 Documentation/trace/kprobetrace.rst | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/trace/kprobetrace.rst b/Documentation/trace/kprobetrace.rst
index bf9cecb69fc9..a1d12d65a8dc 100644
--- a/Documentation/trace/kprobetrace.rst
+++ b/Documentation/trace/kprobetrace.rst
@@ -58,7 +58,8 @@ Synopsis of kprobe_events
   NAME=FETCHARG : Set NAME as the argument name of FETCHARG.
   FETCHARG:TYPE : Set TYPE as the type of FETCHARG. Currently, basic types
 		  (u8/u16/u32/u64/s8/s16/s32/s64), hexadecimal types
-		  (x8/x16/x32/x64), "char", "string", "ustring", "symbol", "symstr"
+		  (x8/x16/x32/x64), VFS layer common type(%pd/%pD) for print
+                  file name, "char", "string", "ustring", "symbol", "symstr"
                   and bitfield are supported.
 
   (\*1) only for the probe on function entry (offs == 0). Note, this argument access
@@ -113,6 +114,9 @@ With 'symstr' type, you can filter the event with wildcard pattern of the
 symbols, and you don't need to solve symbol name by yourself.
 For $comm, the default type is "string"; any other type is invalid.
 
+VFS layer common type(%pd/%pD) is a special type, which fetches dentry's or
+file's name from struct dentry's pointer or struct file's pointer.
+
 .. _user_mem_access:
 
 User Memory Access
-- 
2.31.1

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

[Ye Bin]

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

Re: [PATCH v4 6/7] Documentation: tracing: add new type '%pd' and '%pD' for kprobe

[Masami Hiramatsu]

On Tue, 23 Jan 2024 17:21:38 +0800
Ye Bin <yebin10@huawei.com> wrote:

> Similar to printk() '%pd' is for fetch dentry's name from struct dentry's
> pointer, and '%pD' is for fetch file's name from struct file's pointer.
> 
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
>  Documentation/trace/kprobetrace.rst | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/trace/kprobetrace.rst b/Documentation/trace/kprobetrace.rst
> index bf9cecb69fc9..a1d12d65a8dc 100644
> --- a/Documentation/trace/kprobetrace.rst
> +++ b/Documentation/trace/kprobetrace.rst
> @@ -58,7 +58,8 @@ Synopsis of kprobe_events
>    NAME=FETCHARG : Set NAME as the argument name of FETCHARG.
>    FETCHARG:TYPE : Set TYPE as the type of FETCHARG. Currently, basic types
>  		  (u8/u16/u32/u64/s8/s16/s32/s64), hexadecimal types
> -		  (x8/x16/x32/x64), "char", "string", "ustring", "symbol", "symstr"
> +		  (x8/x16/x32/x64), VFS layer common type(%pd/%pD) for print
> +                  file name, "char", "string", "ustring", "symbol", "symstr"

Could you remove the "for print file name" here?  Since this is not the place
where such precise information describes. I think following hunk is enough to
explain it.

Thank you,

>                    and bitfield are supported.
>  
>    (\*1) only for the probe on function entry (offs == 0). Note, this argument access
> @@ -113,6 +114,9 @@ With 'symstr' type, you can filter the event with wildcard pattern of the
>  symbols, and you don't need to solve symbol name by yourself.
>  For $comm, the default type is "string"; any other type is invalid.
>  
> +VFS layer common type(%pd/%pD) is a special type, which fetches dentry's or
> +file's name from struct dentry's pointer or struct file's pointer.
> +
>  .. _user_mem_access:
>  
>  User Memory Access
> -- 
> 2.31.1
> 
> 


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

Re: [PATCH v4 2/7] tracing/probes: add traceprobe_expand_dentry_args() helper

[Masami Hiramatsu]

Hi Ye,

On Tue, 23 Jan 2024 17:21:34 +0800
Ye Bin <yebin10@huawei.com> wrote:

> Add traceprobe_expand_dentry_args() to expand dentry args. this API is
> prepare to support "%pd" print format for kprobe.
> 
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
>  kernel/trace/trace_probe.c | 36 ++++++++++++++++++++++++++++++++++++
>  kernel/trace/trace_probe.h |  2 ++
>  2 files changed, 38 insertions(+)
> 
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index 4dc74d73fc1d..cc8bd7ea5341 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -1565,6 +1565,42 @@ const char **traceprobe_expand_meta_args(int argc, const char *argv[],
>  	return ERR_PTR(ret);
>  }
>  
> +int traceprobe_expand_dentry_args(int argc, const char *argv[], char *buf,
> +				  int bufsize)
> +{
> +	int i, used, ret;
> +
> +	used = 0;
> +	for (i = 0; i < argc; i++) {
> +		size_t idx;
> +
> +		if (str_has_suffix(argv[i], ":%pd", &idx)) {

Instead of using this, I recommend to use `glob_match("*:%pd", argv[i])`
so that you can simply expand the pattern as `glob_match("*:%p[dD]",...)`
(glob_match means wildcard match like shell does)

Thank you,

> +			char *tmp = kstrdup(argv[i], GFP_KERNEL);
> +			char *equal;
> +
> +			if (!tmp)
> +				return -ENOMEM;
> +
> +			equal = strchr(tmp, '=');
> +			if (equal)
> +				*equal = '\0';
> +			tmp[idx] = '\0';
> +			ret = snprintf(buf + used, bufsize - used,
nb> +				       "%s%s+0x0(+0x%zx(%s)):string",
> +				       equal ? tmp : "", equal ? "=" : "",
> +				       offsetof(struct dentry, d_name.name),
> +				       equal ? equal + 1 : tmp);
> +			kfree(tmp);
> +			if (ret >= bufsize - used)
> +				return -ENOMEM;
> +			argv[i] = buf + used;
> +			used += ret + 1;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  void traceprobe_finish_parse(struct traceprobe_parse_context *ctx)
>  {
>  	clear_btf_context(ctx);
> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index 850d9ecb6765..553371a4e0b1 100644
> --- a/kernel/trace/trace_probe.h
> +++ b/kernel/trace/trace_probe.h
> @@ -402,6 +402,8 @@ extern int traceprobe_parse_probe_arg(struct trace_probe *tp, int i,
>  const char **traceprobe_expand_meta_args(int argc, const char *argv[],
>  					 int *new_argc, char *buf, int bufsize,
>  					 struct traceprobe_parse_context *ctx);
> +extern int traceprobe_expand_dentry_args(int argc, const char *argv[], char *buf,
> +					 int bufsize);
>  
>  extern int traceprobe_update_arg(struct probe_arg *arg);
>  extern void traceprobe_free_probe_arg(struct probe_arg *arg);
> -- 
> 2.31.1
> 
> 


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

Re: [PATCH v4 3/7] tracing/probes: support '%pd' type for print struct dentry's name

[Masami Hiramatsu]

On Tue, 23 Jan 2024 17:21:35 +0800
Ye Bin <yebin10@huawei.com> wrote:

> Similar to '%pd' for printk, use '%pd' for print struct dentry's name.
> 
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
>  kernel/trace/trace_kprobe.c | 6 ++++++
>  kernel/trace/trace_probe.h  | 1 +
>  2 files changed, 7 insertions(+)
> 
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index c4c6e0e0068b..00b74530fbad 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -779,6 +779,7 @@ static int __trace_kprobe_create(int argc, const char *argv[])
>  	char buf[MAX_EVENT_NAME_LEN];
>  	char gbuf[MAX_EVENT_NAME_LEN];
>  	char abuf[MAX_BTF_ARGS_LEN];
> +	char dbuf[MAX_DENTRY_ARGS_LEN];

Hmm, no, I don't like to expand stack anymore. Please allocate it
from heap.

>  	struct traceprobe_parse_context ctx = { .flags = TPARG_FL_KERNEL };
>  
>  	switch (argv[0][0]) {
> @@ -930,6 +931,11 @@ static int __trace_kprobe_create(int argc, const char *argv[])
>  		argv = new_argv;
>  	}
>  
> +	ret = traceprobe_expand_dentry_args(argc, argv, dbuf,
> +					    MAX_DENTRY_ARGS_LEN);
> +	if (ret)
> +		goto out;

And calling this here will not cover the trace_fprobe. 

Could you call this from traceprobe_expand_meta_args() instead of
calling it directly from trace_kprobe? Then it can be used from
fprobe_event too.

Thank you,

> +
>  	/* setup a probe */
>  	tk = alloc_trace_kprobe(group, event, addr, symbol, offset, maxactive,
>  				argc, is_return);
> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index 553371a4e0b1..d9c053824975 100644
> --- a/kernel/trace/trace_probe.h
> +++ b/kernel/trace/trace_probe.h
> @@ -34,6 +34,7 @@
>  #define MAX_ARRAY_LEN		64
>  #define MAX_ARG_NAME_LEN	32
>  #define MAX_BTF_ARGS_LEN	128
> +#define MAX_DENTRY_ARGS_LEN	256

Why do you think 

>  #define MAX_STRING_SIZE		PATH_MAX
>  #define MAX_ARG_BUF_LEN		(MAX_TRACE_ARGS * MAX_ARG_NAME_LEN)
>  
> -- 
> 2.31.1
> 
> 


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

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

[Masami Hiramatsu]

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,

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

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

[yebin (H)]

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,
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
>>
>

Re: [PATCH v4 3/7] tracing/probes: support '%pd' type for print struct dentry's name

[yebin (H)]

On 2024/1/23 22:40, Masami Hiramatsu (Google) wrote:
> On Tue, 23 Jan 2024 17:21:35 +0800
> Ye Bin <yebin10@huawei.com> wrote:
>
>> Similar to '%pd' for printk, use '%pd' for print struct dentry's name.
>>
>> Signed-off-by: Ye Bin <yebin10@huawei.com>
>> ---
>>   kernel/trace/trace_kprobe.c | 6 ++++++
>>   kernel/trace/trace_probe.h  | 1 +
>>   2 files changed, 7 insertions(+)
>>
>> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
>> index c4c6e0e0068b..00b74530fbad 100644
>> --- a/kernel/trace/trace_kprobe.c
>> +++ b/kernel/trace/trace_kprobe.c
>> @@ -779,6 +779,7 @@ static int __trace_kprobe_create(int argc, const char *argv[])
>>   	char buf[MAX_EVENT_NAME_LEN];
>>   	char gbuf[MAX_EVENT_NAME_LEN];
>>   	char abuf[MAX_BTF_ARGS_LEN];
>> +	char dbuf[MAX_DENTRY_ARGS_LEN];
> Hmm, no, I don't like to expand stack anymore. Please allocate it
> from heap.
Do I need to change the other buffers on the stacks to allocate memory 
from heap?
>>   	struct traceprobe_parse_context ctx = { .flags = TPARG_FL_KERNEL };
>>   
>>   	switch (argv[0][0]) {
>> @@ -930,6 +931,11 @@ static int __trace_kprobe_create(int argc, const char *argv[])
>>   		argv = new_argv;
>>   	}
>>   
>> +	ret = traceprobe_expand_dentry_args(argc, argv, dbuf,
>> +					    MAX_DENTRY_ARGS_LEN);
>> +	if (ret)
>> +		goto out;
> And calling this here will not cover the trace_fprobe.
>
> Could you call this from traceprobe_expand_meta_args() instead of
> calling it directly from trace_kprobe? Then it can be used from
> fprobe_event too.
>
> Thank you,
At first I wanted to implement the extension logic in 
traceprobe_expand_meta_args(),
but I found that the code was difficult to understand when I started 
writing. If fprobe_event
wants to support this function, is traceprobe_expand_dentry_args() also 
called? Or re-encapsulate
an interface to include the logic of different extensions. In this way, 
the same buffer is used for
the entire extension process, and the extension function needs to return 
the information about
the length of the buffer.

>> +
>>   	/* setup a probe */
>>   	tk = alloc_trace_kprobe(group, event, addr, symbol, offset, maxactive,
>>   				argc, is_return);
>> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
>> index 553371a4e0b1..d9c053824975 100644
>> --- a/kernel/trace/trace_probe.h
>> +++ b/kernel/trace/trace_probe.h
>> @@ -34,6 +34,7 @@
>>   #define MAX_ARRAY_LEN		64
>>   #define MAX_ARG_NAME_LEN	32
>>   #define MAX_BTF_ARGS_LEN	128
>> +#define MAX_DENTRY_ARGS_LEN	256
> Why do you think
I determined this value according to the extreme case that a parameter 
is expanded to occupy
64 bytes, and a maximum of four such large parameters are supported.
>>   #define MAX_STRING_SIZE		PATH_MAX
>>   #define MAX_ARG_BUF_LEN		(MAX_TRACE_ARGS * MAX_ARG_NAME_LEN)
>>   
>> -- 
>> 2.31.1
>>
>>
>

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

[yebin (H)]

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

Re: [PATCH v4 3/7] tracing/probes: support '%pd' type for print struct dentry's name

[Masami Hiramatsu]

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

> 
> 
> On 2024/1/23 22:40, Masami Hiramatsu (Google) wrote:
> > On Tue, 23 Jan 2024 17:21:35 +0800
> > Ye Bin <yebin10@huawei.com> wrote:
> >
> >> Similar to '%pd' for printk, use '%pd' for print struct dentry's name.
> >>
> >> Signed-off-by: Ye Bin <yebin10@huawei.com>
> >> ---
> >>   kernel/trace/trace_kprobe.c | 6 ++++++
> >>   kernel/trace/trace_probe.h  | 1 +
> >>   2 files changed, 7 insertions(+)
> >>
> >> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> >> index c4c6e0e0068b..00b74530fbad 100644
> >> --- a/kernel/trace/trace_kprobe.c
> >> +++ b/kernel/trace/trace_kprobe.c
> >> @@ -779,6 +779,7 @@ static int __trace_kprobe_create(int argc, const char *argv[])
> >>   	char buf[MAX_EVENT_NAME_LEN];
> >>   	char gbuf[MAX_EVENT_NAME_LEN];
> >>   	char abuf[MAX_BTF_ARGS_LEN];
> >> +	char dbuf[MAX_DENTRY_ARGS_LEN];
> > Hmm, no, I don't like to expand stack anymore. Please allocate it
> > from heap.
> Do I need to change the other buffers on the stacks to allocate memory 
> from heap?

No, that is not needed for this series, but if you want, you can :)

> >>   	struct traceprobe_parse_context ctx = { .flags = TPARG_FL_KERNEL };
> >>   
> >>   	switch (argv[0][0]) {
> >> @@ -930,6 +931,11 @@ static int __trace_kprobe_create(int argc, const char *argv[])
> >>   		argv = new_argv;
> >>   	}
> >>   
> >> +	ret = traceprobe_expand_dentry_args(argc, argv, dbuf,
> >> +					    MAX_DENTRY_ARGS_LEN);
> >> +	if (ret)
> >> +		goto out;
> > And calling this here will not cover the trace_fprobe.
> >
> > Could you call this from traceprobe_expand_meta_args() instead of
> > calling it directly from trace_kprobe? Then it can be used from
> > fprobe_event too.
> >
> > Thank you,
> At first I wanted to implement the extension logic in 
> traceprobe_expand_meta_args(),
> but I found that the code was difficult to understand when I started 
> writing.

Yeah, I also found that is a bit different usage.

> If fprobe_event
> wants to support this function, is traceprobe_expand_dentry_args() also 
> called?

Yes, it is for expanding '$arg*' into '$arg1 $arg2 ...'

> Or re-encapsulate
> an interface to include the logic of different extensions. In this way, 
> the same buffer is used for
> the entire extension process, and the extension function needs to return 
> the information about
> the length of the buffer.

OK, I confirmed that will be too much complicated. Then can you just call
it from where the traceprobe_expand_meta_args() is called, which is 
__trace_fprobe_create()@trace_fprobe.c ?

But those should be simplified later.

Thank you,

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

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

[Masami Hiramatsu]

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>