LKML Archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/8] support '%pd' and '%pD' for print file name
@ 2024-03-15  6:55 Ye Bin
  2024-03-15  6:55 ` [PATCH v6 1/8] tracing/probes: add traceprobe_expand_dentry_args() helper Ye Bin
                   ` (8 more replies)
  0 siblings, 9 replies; 13+ messages in thread
From: Ye Bin @ 2024-03-15  6:55 UTC (permalink / raw
  To: rostedt, mhiramat, mathieu.desnoyers, linux-trace-kernel
  Cc: linux-kernel, yebin10

Sorry for taking so long to post the V6 version. I thought my email was
sent successfully. I checked the patchwork and it was not sent successfully.

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 v6 vs v5:
1. Add const for 'bufsize' in PATCH [1];
2. Move PATCH 'tracing/probes: support '%pd/%pD' type for fprobe' after
PATCH "tracing/probes: support '%pd' type for print struct dentry's name".
3. Add requires '"%pd/%pD":README' for testcase.

Diff v5 vs v4:
1. Use glob_match() instead of str_has_suffix(), so remove the first patch;
2. Allocate buffer from heap for expand dentry;
3. Support "%pd/%pD" print type for fprobe;
4. Use $arg1 instead of origin register in test case;
5. Add test case for fprobe;

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 (8):
  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 fprobe
  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 kprobe test cases for VFS type "%pd" and "%pD"
  selftests/ftrace: add fprobe test cases for VFS type "%pd" and "%pD"

 Documentation/trace/kprobetrace.rst           |  8 ++-
 kernel/trace/trace.c                          |  2 +-
 kernel/trace/trace_fprobe.c                   |  6 ++
 kernel/trace/trace_kprobe.c                   |  6 ++
 kernel/trace/trace_probe.c                    | 63 +++++++++++++++++++
 kernel/trace/trace_probe.h                    |  2 +
 .../ftrace/test.d/dynevent/fprobe_args_vfs.tc | 40 ++++++++++++
 .../ftrace/test.d/kprobe/kprobe_args_vfs.tc   | 43 +++++++++++++
 8 files changed, 167 insertions(+), 3 deletions(-)
 create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/fprobe_args_vfs.tc
 create mode 100644 tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_vfs.tc

-- 
2.31.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v6 1/8] tracing/probes: add traceprobe_expand_dentry_args() helper
  2024-03-15  6:55 [PATCH v6 0/8] support '%pd' and '%pD' for print file name Ye Bin
@ 2024-03-15  6:55 ` Ye Bin
  2024-03-15  6:55 ` [PATCH v6 2/8] tracing/probes: support '%pd' type for print struct dentry's name Ye Bin
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Ye Bin @ 2024-03-15  6:55 UTC (permalink / raw
  To: rostedt, mhiramat, mathieu.desnoyers, linux-trace-kernel
  Cc: linux-kernel, yebin10

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 | 50 ++++++++++++++++++++++++++++++++++++++
 kernel/trace/trace_probe.h |  2 ++
 2 files changed, 52 insertions(+)

diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 34289f9c6707..a27567e16c36 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -1569,6 +1569,56 @@ const char **traceprobe_expand_meta_args(int argc, const char *argv[],
 	return ERR_PTR(ret);
 }
 
+/* @buf: *buf must be equal to NULL. Caller must to free *buf */
+int traceprobe_expand_dentry_args(int argc, const char *argv[], char **buf)
+{
+	int i, used, ret;
+	const int bufsize = MAX_DENTRY_ARGS_LEN;
+	char *tmpbuf = NULL;
+
+	if (*buf)
+		return -EINVAL;
+
+	used = 0;
+	for (i = 0; i < argc; i++) {
+		if (glob_match("*:%pd", argv[i])) {
+			char *tmp;
+			char *equal;
+
+			if (!tmpbuf) {
+				tmpbuf = kmalloc(bufsize, GFP_KERNEL);
+				if (!tmpbuf)
+					return -ENOMEM;
+			}
+
+			tmp = kstrdup(argv[i], GFP_KERNEL);
+			if (!tmp)
+				goto nomem;
+
+			equal = strchr(tmp, '=');
+			if (equal)
+				*equal = '\0';
+			tmp[strlen(argv[i]) - 4] = '\0';
+			ret = snprintf(tmpbuf + 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)
+				goto nomem;
+			argv[i] = tmpbuf + used;
+			used += ret + 1;
+		}
+	}
+
+	*buf = tmpbuf;
+	return 0;
+nomem:
+	kfree(tmpbuf);
+	return -ENOMEM;
+}
+
 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 c1877d018269..3d22fba4b63f 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)
 
@@ -402,6 +403,7 @@ 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);
 
 extern int traceprobe_update_arg(struct probe_arg *arg);
 extern void traceprobe_free_probe_arg(struct probe_arg *arg);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v6 2/8] tracing/probes: support '%pd' type for print struct dentry's name
  2024-03-15  6:55 [PATCH v6 0/8] support '%pd' and '%pD' for print file name Ye Bin
  2024-03-15  6:55 ` [PATCH v6 1/8] tracing/probes: add traceprobe_expand_dentry_args() helper Ye Bin
@ 2024-03-15  6:55 ` Ye Bin
  2024-03-15  6:55 ` [PATCH v6 3/8] tracing/probes: support '%pd' type for fprobe Ye Bin
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Ye Bin @ 2024-03-15  6:55 UTC (permalink / raw
  To: rostedt, mhiramat, mathieu.desnoyers, linux-trace-kernel
  Cc: linux-kernel, yebin10

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 ++++++
 1 file changed, 6 insertions(+)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index c4c6e0e0068b..7cbb43740b4f 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 = NULL;
 	struct traceprobe_parse_context ctx = { .flags = TPARG_FL_KERNEL };
 
 	switch (argv[0][0]) {
@@ -930,6 +931,10 @@ static int __trace_kprobe_create(int argc, const char *argv[])
 		argv = new_argv;
 	}
 
+	ret = traceprobe_expand_dentry_args(argc, argv, &dbuf);
+	if (ret)
+		goto out;
+
 	/* setup a probe */
 	tk = alloc_trace_kprobe(group, event, addr, symbol, offset, maxactive,
 				argc, is_return);
@@ -971,6 +976,7 @@ static int __trace_kprobe_create(int argc, const char *argv[])
 	trace_probe_log_clear();
 	kfree(new_argv);
 	kfree(symbol);
+	kfree(dbuf);
 	return ret;
 
 parse_error:
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v6 3/8] tracing/probes: support '%pd' type for fprobe
  2024-03-15  6:55 [PATCH v6 0/8] support '%pd' and '%pD' for print file name Ye Bin
  2024-03-15  6:55 ` [PATCH v6 1/8] tracing/probes: add traceprobe_expand_dentry_args() helper Ye Bin
  2024-03-15  6:55 ` [PATCH v6 2/8] tracing/probes: support '%pd' type for print struct dentry's name Ye Bin
@ 2024-03-15  6:55 ` Ye Bin
  2024-03-20 12:46   ` Masami Hiramatsu
  2024-03-15  6:55 ` [PATCH v6 4/8] tracing/probes: support '%pD' type for print struct file's name Ye Bin
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 13+ messages in thread
From: Ye Bin @ 2024-03-15  6:55 UTC (permalink / raw
  To: rostedt, mhiramat, mathieu.desnoyers, linux-trace-kernel
  Cc: linux-kernel, yebin10

Support print type '%pd' for print dentry's or file's name.

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

diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c
index 7d2ddbcfa377..988d68e906ad 100644
--- a/kernel/trace/trace_fprobe.c
+++ b/kernel/trace/trace_fprobe.c
@@ -976,6 +976,7 @@ static int __trace_fprobe_create(int argc, const char *argv[])
 	char gbuf[MAX_EVENT_NAME_LEN];
 	char sbuf[KSYM_NAME_LEN];
 	char abuf[MAX_BTF_ARGS_LEN];
+	char *dbuf = NULL;
 	bool is_tracepoint = false;
 	struct tracepoint *tpoint = NULL;
 	struct traceprobe_parse_context ctx = {
@@ -1086,6 +1087,10 @@ static int __trace_fprobe_create(int argc, const char *argv[])
 		argv = new_argv;
 	}
 
+	ret = traceprobe_expand_dentry_args(argc, argv, &dbuf);
+	if (ret)
+		goto out;
+
 	/* setup a probe */
 	tf = alloc_trace_fprobe(group, event, symbol, tpoint, maxactive,
 				argc, is_return);
@@ -1131,6 +1136,7 @@ static int __trace_fprobe_create(int argc, const char *argv[])
 	trace_probe_log_clear();
 	kfree(new_argv);
 	kfree(symbol);
+	kfree(dbuf);
 	return ret;
 
 parse_error:
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v6 4/8] tracing/probes: support '%pD' type for print struct file's name
  2024-03-15  6:55 [PATCH v6 0/8] support '%pd' and '%pD' for print file name Ye Bin
                   ` (2 preceding siblings ...)
  2024-03-15  6:55 ` [PATCH v6 3/8] tracing/probes: support '%pd' type for fprobe Ye Bin
@ 2024-03-15  6:55 ` Ye Bin
  2024-03-15  6:55 ` [PATCH v6 5/8] tracing: add new type "%pd/%pD" in readme_msg[] Ye Bin
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Ye Bin @ 2024-03-15  6:55 UTC (permalink / raw
  To: rostedt, mhiramat, mathieu.desnoyers, linux-trace-kernel
  Cc: linux-kernel, yebin10

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 | 57 +++++++++++++++++++++++---------------
 1 file changed, 35 insertions(+), 22 deletions(-)

diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index a27567e16c36..7bfc6c0d5436 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"
@@ -1581,35 +1582,47 @@ int traceprobe_expand_dentry_args(int argc, const char *argv[], char **buf)
 
 	used = 0;
 	for (i = 0; i < argc; i++) {
-		if (glob_match("*:%pd", argv[i])) {
-			char *tmp;
-			char *equal;
-
-			if (!tmpbuf) {
-				tmpbuf = kmalloc(bufsize, GFP_KERNEL);
-				if (!tmpbuf)
-					return -ENOMEM;
-			}
+		char *tmp;
+		char *equal;
+		size_t arg_len;
 
-			tmp = kstrdup(argv[i], GFP_KERNEL);
-			if (!tmp)
-				goto nomem;
+		if (!glob_match("*:%p[dD]", argv[i]))
+			continue;
 
-			equal = strchr(tmp, '=');
-			if (equal)
-				*equal = '\0';
-			tmp[strlen(argv[i]) - 4] = '\0';
+		if (!tmpbuf) {
+			tmpbuf = kmalloc(bufsize, GFP_KERNEL);
+			if (!tmpbuf)
+				return -ENOMEM;
+		}
+
+		tmp = kstrdup(argv[i], GFP_KERNEL);
+		if (!tmp)
+			goto nomem;
+
+		equal = strchr(tmp, '=');
+		if (equal)
+			*equal = '\0';
+		arg_len = strlen(argv[i]);
+		tmp[arg_len - 4] = '\0';
+		if (argv[i][arg_len - 1] == 'd')
 			ret = snprintf(tmpbuf + 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)
-				goto nomem;
-			argv[i] = tmpbuf + used;
-			used += ret + 1;
-		}
+		else
+			ret = snprintf(tmpbuf + 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)
+			goto nomem;
+		argv[i] = tmpbuf + used;
+		used += ret + 1;
 	}
 
 	*buf = tmpbuf;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v6 5/8] tracing: add new type "%pd/%pD" in readme_msg[]
  2024-03-15  6:55 [PATCH v6 0/8] support '%pd' and '%pD' for print file name Ye Bin
                   ` (3 preceding siblings ...)
  2024-03-15  6:55 ` [PATCH v6 4/8] tracing/probes: support '%pD' type for print struct file's name Ye Bin
@ 2024-03-15  6:55 ` Ye Bin
  2024-03-20 10:09   ` Masami Hiramatsu
  2024-03-15  6:55 ` [PATCH v6 6/8] Documentation: tracing: add new type '%pd' and '%pD' for kprobe Ye Bin
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 13+ messages in thread
From: Ye Bin @ 2024-03-15  6:55 UTC (permalink / raw
  To: rostedt, mhiramat, mathieu.desnoyers, linux-trace-kernel
  Cc: linux-kernel, yebin10

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 b12f8384a36a..831dfd0773a4 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -5510,7 +5510,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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v6 6/8] Documentation: tracing: add new type '%pd' and '%pD' for kprobe
  2024-03-15  6:55 [PATCH v6 0/8] support '%pd' and '%pD' for print file name Ye Bin
                   ` (4 preceding siblings ...)
  2024-03-15  6:55 ` [PATCH v6 5/8] tracing: add new type "%pd/%pD" in readme_msg[] Ye Bin
@ 2024-03-15  6:55 ` Ye Bin
  2024-03-15  6:55 ` [PATCH v6 7/8] selftests/ftrace: add kprobe test cases for VFS type "%pd" and "%pD" Ye Bin
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Ye Bin @ 2024-03-15  6:55 UTC (permalink / raw
  To: rostedt, mhiramat, mathieu.desnoyers, linux-trace-kernel
  Cc: linux-kernel, yebin10

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 | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/trace/kprobetrace.rst b/Documentation/trace/kprobetrace.rst
index bf9cecb69fc9..f13f0fc11251 100644
--- a/Documentation/trace/kprobetrace.rst
+++ b/Documentation/trace/kprobetrace.rst
@@ -58,8 +58,9 @@ 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"
-                  and bitfield are supported.
+		  (x8/x16/x32/x64), VFS layer common type(%pd/%pD), "char",
+                  "string", "ustring", "symbol", "symstr" and bitfield are
+                  supported.
 
   (\*1) only for the probe on function entry (offs == 0). Note, this argument access
         is best effort, because depending on the argument type, it may be passed on
@@ -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 address or struct file's address.
+
 .. _user_mem_access:
 
 User Memory Access
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v6 7/8] selftests/ftrace: add kprobe test cases for VFS type "%pd" and "%pD"
  2024-03-15  6:55 [PATCH v6 0/8] support '%pd' and '%pD' for print file name Ye Bin
                   ` (5 preceding siblings ...)
  2024-03-15  6:55 ` [PATCH v6 6/8] Documentation: tracing: add new type '%pd' and '%pD' for kprobe Ye Bin
@ 2024-03-15  6:55 ` Ye Bin
  2024-03-20  3:28   ` Masami Hiramatsu
  2024-03-15  6:55 ` [PATCH v6 8/8] selftests/ftrace: add fprobe " Ye Bin
  2024-03-20 10:20 ` [PATCH v6 0/8] support '%pd' and '%pD' for print file name Masami Hiramatsu
  8 siblings, 1 reply; 13+ messages in thread
From: Ye Bin @ 2024-03-15  6:55 UTC (permalink / raw
  To: rostedt, mhiramat, mathieu.desnoyers, linux-trace-kernel
  Cc: linux-kernel, yebin10

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();

This test case require enable CONFIG_HAVE_FUNCTION_ARG_ACCESS_API configuration.

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 .../ftrace/test.d/kprobe/kprobe_args_vfs.tc   | 43 +++++++++++++++++++
 1 file changed, 43 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..8bea9a75a331
--- /dev/null
+++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_vfs.tc
@@ -0,0 +1,43 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# description: Kprobe event VFS type argument
+# requires: kprobe_events "%pd/%pD":README
+
+: "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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v6 8/8] selftests/ftrace: add fprobe test cases for VFS type "%pd" and "%pD"
  2024-03-15  6:55 [PATCH v6 0/8] support '%pd' and '%pD' for print file name Ye Bin
                   ` (6 preceding siblings ...)
  2024-03-15  6:55 ` [PATCH v6 7/8] selftests/ftrace: add kprobe test cases for VFS type "%pd" and "%pD" Ye Bin
@ 2024-03-15  6:55 ` Ye Bin
  2024-03-20 10:20 ` [PATCH v6 0/8] support '%pd' and '%pD' for print file name Masami Hiramatsu
  8 siblings, 0 replies; 13+ messages in thread
From: Ye Bin @ 2024-03-15  6:55 UTC (permalink / raw
  To: rostedt, mhiramat, mathieu.desnoyers, linux-trace-kernel
  Cc: linux-kernel, yebin10

This patch adds fprobe test cases for new print format type "%pd/%pD".The
test cases test the following items:
1. Test "%pd" type for dput();
2. Test "%pD" type for vfs_read();

This test case require enable CONFIG_HAVE_FUNCTION_ARG_ACCESS_API configuration.

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 .../ftrace/test.d/dynevent/fprobe_args_vfs.tc | 40 +++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/fprobe_args_vfs.tc

diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_args_vfs.tc b/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_args_vfs.tc
new file mode 100644
index 000000000000..49a833bf334c
--- /dev/null
+++ b/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_args_vfs.tc
@@ -0,0 +1,40 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# description: Fprobe event VFS type argument
+# requires: kprobe_events "%pd/%pD":README
+
+: "Test argument %pd with name for fprobe"
+echo 'f:testprobe dput name=$arg1:%pd' > dynamic_events
+echo 1 > events/fprobes/testprobe/enable
+grep -q "1" events/fprobes/testprobe/enable
+echo 0 > events/fprobes/testprobe/enable
+grep "dput" trace | grep -q "enable"
+echo "" > dynamic_events
+echo "" > trace
+
+: "Test argument %pd without name for fprobe"
+echo 'f:testprobe dput $arg1:%pd' > dynamic_events
+echo 1 > events/fprobes/testprobe/enable
+grep -q "1" events/fprobes/testprobe/enable
+echo 0 > events/fprobes/testprobe/enable
+grep "dput" trace | grep -q "enable"
+echo "" > dynamic_events
+echo "" > trace
+
+: "Test argument %pD with name for fprobe"
+echo 'f:testprobe vfs_read name=$arg1:%pD' > dynamic_events
+echo 1 > events/fprobes/testprobe/enable
+grep -q "1" events/fprobes/testprobe/enable
+echo 0 > events/fprobes/testprobe/enable
+grep "vfs_read" trace | grep -q "enable"
+echo "" > dynamic_events
+echo "" > trace
+
+: "Test argument %pD without name for fprobe"
+echo 'f:testprobe vfs_read $arg1:%pD' > dynamic_events
+echo 1 > events/fprobes/testprobe/enable
+grep -q "1"  events/fprobes/testprobe/enable
+echo 0 > events/fprobes/testprobe/enable
+grep "vfs_read" trace | grep -q "enable"
+echo "" > dynamic_events
+echo "" > trace
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 7/8] selftests/ftrace: add kprobe test cases for VFS type "%pd" and "%pD"
  2024-03-15  6:55 ` [PATCH v6 7/8] selftests/ftrace: add kprobe test cases for VFS type "%pd" and "%pD" Ye Bin
@ 2024-03-20  3:28   ` Masami Hiramatsu
  0 siblings, 0 replies; 13+ messages in thread
From: Masami Hiramatsu @ 2024-03-20  3:28 UTC (permalink / raw
  To: Ye Bin; +Cc: rostedt, mathieu.desnoyers, linux-trace-kernel, linux-kernel

Hi Ye,

Sorry for replying late.

On Fri, 15 Mar 2024 14:55: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();
> 
> This test case require enable CONFIG_HAVE_FUNCTION_ARG_ACCESS_API configuration.
> 
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
>  .../ftrace/test.d/kprobe/kprobe_args_vfs.tc   | 43 +++++++++++++++++++
>  1 file changed, 43 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..8bea9a75a331
> --- /dev/null
> +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_vfs.tc
> @@ -0,0 +1,43 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +# description: Kprobe event VFS type argument
> +# requires: kprobe_events "%pd/%pD":README
> +
> +: "Test argument %pd/%pD in README"
> +grep -q "%pd/%pD" README

If you put this check in "# requires" line, you don't need to check it again.

> +
> +: "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

So accessing "enable" file here,

> +echo 0 > events/kprobes/testprobe/enable
> +grep "dput" trace | grep -q "enable"

and check it.

OK, the test case itself looks good to me.

Thanks,

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 5/8] tracing: add new type "%pd/%pD" in readme_msg[]
  2024-03-15  6:55 ` [PATCH v6 5/8] tracing: add new type "%pd/%pD" in readme_msg[] Ye Bin
@ 2024-03-20 10:09   ` Masami Hiramatsu
  0 siblings, 0 replies; 13+ messages in thread
From: Masami Hiramatsu @ 2024-03-20 10:09 UTC (permalink / raw
  To: Ye Bin; +Cc: rostedt, mathieu.desnoyers, linux-trace-kernel, linux-kernel

Hi Ye,

Can you split this into 

(1)
> -	"\t           symstr, <type>\\[<array-size>\\]\n"
> +	"\t           symstr, %pd, <type>\\[<array-size>\\]\n"

and 

(2)
> -	"\t           symstr, %pd, <type>\\[<array-size>\\]\n"
> +	"\t           symstr, %pd/%pD, <type>\\[<array-size>\\]\n"

And merge (1) into [PATCH 3/8], (2) into [PATCH 5/8], so that
the feature patch itself updates the readme text?

This is because even if one of the patch drops, others can
show its feature on readme.

Thank you,

On Fri, 15 Mar 2024 14:55:37 +0800
Ye Bin <yebin10@huawei.com> wrote:

> 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 b12f8384a36a..831dfd0773a4 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -5510,7 +5510,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
> 


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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 0/8] support '%pd' and '%pD' for print file name
  2024-03-15  6:55 [PATCH v6 0/8] support '%pd' and '%pD' for print file name Ye Bin
                   ` (7 preceding siblings ...)
  2024-03-15  6:55 ` [PATCH v6 8/8] selftests/ftrace: add fprobe " Ye Bin
@ 2024-03-20 10:20 ` Masami Hiramatsu
  8 siblings, 0 replies; 13+ messages in thread
From: Masami Hiramatsu @ 2024-03-20 10:20 UTC (permalink / raw
  To: Ye Bin; +Cc: rostedt, mathieu.desnoyers, linux-trace-kernel, linux-kernel

Hi Ye,

On Fri, 15 Mar 2024 14:55:32 +0800
Ye Bin <yebin10@huawei.com> wrote:

> Sorry for taking so long to post the V6 version. I thought my email was
> sent successfully. I checked the patchwork and it was not sent successfully.
> 
> 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.

Thanks for updating the series.

There are some nits, but basically this is good to me.
Could you update this series?

Thank you,

> 
> Diff v6 vs v5:
> 1. Add const for 'bufsize' in PATCH [1];
> 2. Move PATCH 'tracing/probes: support '%pd/%pD' type for fprobe' after
> PATCH "tracing/probes: support '%pd' type for print struct dentry's name".
> 3. Add requires '"%pd/%pD":README' for testcase.
> 
> Diff v5 vs v4:
> 1. Use glob_match() instead of str_has_suffix(), so remove the first patch;
> 2. Allocate buffer from heap for expand dentry;
> 3. Support "%pd/%pD" print type for fprobe;
> 4. Use $arg1 instead of origin register in test case;
> 5. Add test case for fprobe;
> 
> 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 (8):
>   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 fprobe
>   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 kprobe test cases for VFS type "%pd" and "%pD"
>   selftests/ftrace: add fprobe test cases for VFS type "%pd" and "%pD"
> 
>  Documentation/trace/kprobetrace.rst           |  8 ++-
>  kernel/trace/trace.c                          |  2 +-
>  kernel/trace/trace_fprobe.c                   |  6 ++
>  kernel/trace/trace_kprobe.c                   |  6 ++
>  kernel/trace/trace_probe.c                    | 63 +++++++++++++++++++
>  kernel/trace/trace_probe.h                    |  2 +
>  .../ftrace/test.d/dynevent/fprobe_args_vfs.tc | 40 ++++++++++++
>  .../ftrace/test.d/kprobe/kprobe_args_vfs.tc   | 43 +++++++++++++
>  8 files changed, 167 insertions(+), 3 deletions(-)
>  create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/fprobe_args_vfs.tc
>  create mode 100644 tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_vfs.tc
> 
> -- 
> 2.31.1
> 


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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 3/8] tracing/probes: support '%pd' type for fprobe
  2024-03-15  6:55 ` [PATCH v6 3/8] tracing/probes: support '%pd' type for fprobe Ye Bin
@ 2024-03-20 12:46   ` Masami Hiramatsu
  0 siblings, 0 replies; 13+ messages in thread
From: Masami Hiramatsu @ 2024-03-20 12:46 UTC (permalink / raw
  To: Ye Bin; +Cc: rostedt, mathieu.desnoyers, linux-trace-kernel, linux-kernel

Hi Ye,

BTW, if you have a chance, please squash [1/8] to [3/8] patches
into 1 patch. There is no reason to split these since these are
a local(= the same subsystem) function implementation and callers.

Thank you,

On Fri, 15 Mar 2024 14:55:35 +0800
Ye Bin <yebin10@huawei.com> wrote:

> Support print type '%pd' for print dentry's or file's name.
> 
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
>  kernel/trace/trace_fprobe.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c
> index 7d2ddbcfa377..988d68e906ad 100644
> --- a/kernel/trace/trace_fprobe.c
> +++ b/kernel/trace/trace_fprobe.c
> @@ -976,6 +976,7 @@ static int __trace_fprobe_create(int argc, const char *argv[])
>  	char gbuf[MAX_EVENT_NAME_LEN];
>  	char sbuf[KSYM_NAME_LEN];
>  	char abuf[MAX_BTF_ARGS_LEN];
> +	char *dbuf = NULL;
>  	bool is_tracepoint = false;
>  	struct tracepoint *tpoint = NULL;
>  	struct traceprobe_parse_context ctx = {
> @@ -1086,6 +1087,10 @@ static int __trace_fprobe_create(int argc, const char *argv[])
>  		argv = new_argv;
>  	}
>  
> +	ret = traceprobe_expand_dentry_args(argc, argv, &dbuf);
> +	if (ret)
> +		goto out;
> +
>  	/* setup a probe */
>  	tf = alloc_trace_fprobe(group, event, symbol, tpoint, maxactive,
>  				argc, is_return);
> @@ -1131,6 +1136,7 @@ static int __trace_fprobe_create(int argc, const char *argv[])
>  	trace_probe_log_clear();
>  	kfree(new_argv);
>  	kfree(symbol);
> +	kfree(dbuf);
>  	return ret;
>  
>  parse_error:
> -- 
> 2.31.1
> 


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

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2024-03-20 12:46 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-15  6:55 [PATCH v6 0/8] support '%pd' and '%pD' for print file name Ye Bin
2024-03-15  6:55 ` [PATCH v6 1/8] tracing/probes: add traceprobe_expand_dentry_args() helper Ye Bin
2024-03-15  6:55 ` [PATCH v6 2/8] tracing/probes: support '%pd' type for print struct dentry's name Ye Bin
2024-03-15  6:55 ` [PATCH v6 3/8] tracing/probes: support '%pd' type for fprobe Ye Bin
2024-03-20 12:46   ` Masami Hiramatsu
2024-03-15  6:55 ` [PATCH v6 4/8] tracing/probes: support '%pD' type for print struct file's name Ye Bin
2024-03-15  6:55 ` [PATCH v6 5/8] tracing: add new type "%pd/%pD" in readme_msg[] Ye Bin
2024-03-20 10:09   ` Masami Hiramatsu
2024-03-15  6:55 ` [PATCH v6 6/8] Documentation: tracing: add new type '%pd' and '%pD' for kprobe Ye Bin
2024-03-15  6:55 ` [PATCH v6 7/8] selftests/ftrace: add kprobe test cases for VFS type "%pd" and "%pD" Ye Bin
2024-03-20  3:28   ` Masami Hiramatsu
2024-03-15  6:55 ` [PATCH v6 8/8] selftests/ftrace: add fprobe " Ye Bin
2024-03-20 10:20 ` [PATCH v6 0/8] support '%pd' and '%pD' for print file name Masami Hiramatsu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).