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 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 | 20 +++++
kernel/trace/trace.c | 2 +-
kernel/trace/trace_kprobe.c | 6 ++
kernel/trace/trace_probe.c | 45 +++++++++++
kernel/trace/trace_probe.h | 3 +
.../ftrace/test.d/kprobe/kprobe_args_vfs.tc | 79 +++++++++++++++++++
7 files changed, 159 insertions(+), 2 deletions(-)
create mode 100644 tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_vfs.tc
--
2.31.1
str_has_suffix() is test string if ends with specify string.
Signed-off-by: Ye Bin <[email protected]>
---
include/linux/string.h | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/include/linux/string.h b/include/linux/string.h
index 433c207a01da..e47e9597af27 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -405,4 +405,24 @@ 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
+ *
+ * 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 len = strlen(suffix);
+ size_t str_len = strlen(str);
+
+ if (len > str_len)
+ return 0;
+
+ return strncmp(str + str_len - len, suffix, len) == 0 ? len : 0;
+}
+
#endif /* _LINUX_STRING_H_ */
--
2.31.1
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
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
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 1599c0c3e6b7..f9819625de58 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"
@@ -1572,28 +1573,38 @@ int traceprobe_expand_dentry_args(int argc, const char *argv[], char *buf,
used = 0;
for (i = 0; i < argc; i++) {
- if (str_has_suffix(argv[i], ":%pd")) {
- char *tmp = kstrdup(argv[i], GFP_KERNEL);
- char *equal;
+ if (!str_has_suffix(argv[i], ":%pd") &&
+ !str_has_suffix(argv[i], ":%pD"))
+ 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[strlen(argv[i]) - 4] = '\0';
+ equal = strchr(tmp, '=');
+ if (equal)
+ *equal = '\0';
+ tmp[strlen(argv[i]) - 4] = '\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
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
Signed-off-by: Ye Bin <[email protected]>
---
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
On Mon, 22 Jan 2024 15:40:12 +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 1599c0c3e6b7..f9819625de58 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"
> @@ -1572,28 +1573,38 @@ int traceprobe_expand_dentry_args(int argc, const char *argv[], char *buf,
>
> used = 0;
> for (i = 0; i < argc; i++) {
> - if (str_has_suffix(argv[i], ":%pd")) {
> - char *tmp = kstrdup(argv[i], GFP_KERNEL);
> - char *equal;
> + if (!str_has_suffix(argv[i], ":%pd") &&
> + !str_has_suffix(argv[i], ":%pD"))
> + continue;
And here too:
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[strlen(argv[i]) - 4] = '\0';
> + equal = strchr(tmp, '=');
> + if (equal)
> + *equal = '\0';
> + tmp[strlen(argv[i]) - 4] = '\0';
tmp[idx] = '\0';
> + if (argv[i][strlen(argv[i]) - 1] == 'd')
To avoid another strlen() call.
if (tmp[idx + 3] == '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;
On Mon, 22 Jan 2024 15:40:09 +0800
Ye Bin <[email protected]> wrote:
> str_has_suffix() is test string if ends with specify string.
>
> Signed-off-by: Ye Bin <[email protected]>
> ---
> include/linux/string.h | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/include/linux/string.h b/include/linux/string.h
> index 433c207a01da..e47e9597af27 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -405,4 +405,24 @@ 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
> + *
> + * 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 len = strlen(suffix);
> + size_t str_len = strlen(str);
> +
> + if (len > str_len)
> + return 0;
> +
> + return strncmp(str + str_len - len, suffix, len) == 0 ? len : 0;
> +}
I understand why you are returning the length of the suffix, as it matches
str_has_prefix(). But as the user of this will likely need the length of
the "str" too, we should either return the length of the string, or the
index of where the suffix was found.
/**
[..]
@index: The index into @str of where @suffix is if found (NULL to ignore)
[..]
*/
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 (index)
*index = str_len - len;
return strncmp(str + str_len - len, suffix, len) == 0 ? len : 0;
}
-- Steve
> +
> #endif /* _LINUX_STRING_H_ */
On 2024/1/23 0:07, Steven Rostedt wrote:
> On Mon, 22 Jan 2024 15:40:09 +0800
> Ye Bin <[email protected]> wrote:
>
>> str_has_suffix() is test string if ends with specify string.
>>
>> Signed-off-by: Ye Bin <[email protected]>
>> ---
>> include/linux/string.h | 20 ++++++++++++++++++++
>> 1 file changed, 20 insertions(+)
>>
>> diff --git a/include/linux/string.h b/include/linux/string.h
>> index 433c207a01da..e47e9597af27 100644
>> --- a/include/linux/string.h
>> +++ b/include/linux/string.h
>> @@ -405,4 +405,24 @@ 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
>> + *
>> + * 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 len = strlen(suffix);
>> + size_t str_len = strlen(str);
>> +
>> + if (len > str_len)
>> + return 0;
>> +
>> + return strncmp(str + str_len - len, suffix, len) == 0 ? len : 0;
>> +}
> I understand why you are returning the length of the suffix, as it matches
> str_has_prefix(). But as the user of this will likely need the length of
> the "str" too, we should either return the length of the string, or the
> index of where the suffix was found.
>
> /**
> [..]
> @index: The index into @str of where @suffix is if found (NULL to ignore)
> [..]
> */
> 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 (index)
> *index = str_len - len;
>
> return strncmp(str + str_len - len, suffix, len) == 0 ? len : 0;
> }
>
> -- Steve
Thank you for your advice.
Return the index of where the suffix was found is useful. I'll modify it
according to your suggestion.
-- Ye Bin
>> +
>> #endif /* _LINUX_STRING_H_ */
> .
>