2024-01-23 02:55:50

by yebin (H)

[permalink] [raw]
Subject: [PATCH v3 0/7] support '%pd' and '%pD' for print file name

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



2024-01-23 02:56:25

by yebin (H)

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

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

Signed-off-by: Ye Bin <[email protected]>
---
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..6215b9573793 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][strlen(argv[i]) - 1] == '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


2024-01-23 02:56:25

by yebin (H)

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

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

Signed-off-by: Ye Bin <[email protected]>
---
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


2024-01-23 02:56:28

by yebin (H)

[permalink] [raw]
Subject: [PATCH v3 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-23 03:02:38

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH v3 4/7] tracing/probes: support '%pD' type for print struct file's name

On Tue, 23 Jan 2024 10:56:05 +0800
Ye Bin <[email protected]> wrote:

> Similar to '%pD' for printk, use '%pD' for print struct file's name.
>
> Signed-off-by: Ye Bin <[email protected]>
> ---
> 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..6215b9573793 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][strlen(argv[i]) - 1] == 'd')

You can use idx again here:

if (argv[i][idx + 3] == 'd')

and save from doing the strlen(argv[i]);

idx will point to ':', and + 3 would point to either 'd' or 'D'

-- Steve

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


2024-01-23 03:18:59

by yebin (H)

[permalink] [raw]
Subject: [PATCH v3 6/7] Documentation: tracing: add new type '%pd' and '%pD' for kprobe

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 <[email protected]>
---
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