* [PATCH 0/3] support '%pd' and '%pD' for print file name
@ 2024-01-19 1:38 Ye Bin
2024-01-19 1:38 ` [PATCH 1/3] tracing/probes: support '%pd' type for print struct dentry's name Ye Bin
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Ye Bin @ 2024-01-19 1:38 UTC (permalink / raw)
To: rostedt, mhiramat, mathieu.desnoyers, linux-trace-kernel
Cc: linux-kernel, yebin10
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.
Ye Bin (3):
tracing/probes: support '%pd' type for print struct dentry's name
tracing/probes: support '%pD' type for print struct file's name
Documentation: tracing: add new type 'pd' and 'pD' for kprobe
Documentation/trace/kprobetrace.rst | 3 +-
kernel/trace/trace_probe.c | 50 +++++++++++++++++++++++++++++
2 files changed, 52 insertions(+), 1 deletion(-)
--
2.31.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] tracing/probes: support '%pd' type for print struct dentry's name
2024-01-19 1:38 [PATCH 0/3] support '%pd' and '%pD' for print file name Ye Bin
@ 2024-01-19 1:38 ` Ye Bin
2024-01-19 14:57 ` Masami Hiramatsu
` (2 more replies)
2024-01-19 1:38 ` [PATCH 2/3] tracing/probes: support '%pD' type for print struct file's name Ye Bin
` (2 subsequent siblings)
3 siblings, 3 replies; 11+ messages in thread
From: Ye Bin @ 2024-01-19 1:38 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_probe.c | 38 ++++++++++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 4dc74d73fc1d..460f98b85b1c 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -11,6 +11,7 @@
*/
#define pr_fmt(fmt) "trace_probe: " fmt
+#include <linux/dcache.h>
#include <linux/bpf.h>
#include "trace_btf.h"
@@ -86,6 +87,8 @@ static const struct fetch_type probe_fetch_types[] = {
"__data_loc char[]"),
__ASSIGN_FETCH_TYPE("symstr", string, string, sizeof(u32), 1, 1,
"__data_loc char[]"),
+ __ASSIGN_FETCH_TYPE("pd", string, string, sizeof(u32), 1, 1,
+ "__data_loc char[]"),
/* Basic types */
ASSIGN_FETCH_TYPE(u8, u8, 0),
ASSIGN_FETCH_TYPE(u16, u16, 0),
@@ -1090,6 +1093,25 @@ static int __parse_bitfield_probe_arg(const char *bf,
return (BYTES_TO_BITS(t->size) < (bw + bo)) ? -EINVAL : 0;
}
+static char* traceprobe_expand_dentry(const char *argv)
+{
+ #define DENTRY_EXPAND_LEN 7 /* +0xXX() */
+ char *new_argv;
+ int len = strlen(argv) + 1 + DENTRY_EXPAND_LEN;
+
+ new_argv = kmalloc(len, GFP_KERNEL);
+ if (!new_argv)
+ return NULL;
+
+ if (snprintf(new_argv, len, "+0x%lx(%s)",
+ offsetof(struct dentry, d_name.name), argv) >= len) {
+ kfree(new_argv);
+ return NULL;
+ }
+
+ return new_argv;
+}
+
/* String length checking wrapper */
static int traceprobe_parse_probe_arg_body(const char *argv, ssize_t *size,
struct probe_arg *parg,
@@ -1099,6 +1121,7 @@ static int traceprobe_parse_probe_arg_body(const char *argv, ssize_t *size,
char *t, *t2, *t3;
int ret, len;
char *arg;
+ char *org_arg = NULL;
arg = kstrdup(argv, GFP_KERNEL);
if (!arg)
@@ -1182,6 +1205,16 @@ static int traceprobe_parse_probe_arg_body(const char *argv, ssize_t *size,
parg->count);
}
+ if (!strcmp("pd", parg->type->name)) {
+ char *temp;
+
+ temp = traceprobe_expand_dentry(arg);
+ if (!temp)
+ goto out;
+ org_arg = arg;
+ arg = temp;
+ }
+
code = tmp = kcalloc(FETCH_INSN_MAX, sizeof(*code), GFP_KERNEL);
if (!code)
goto out;
@@ -1243,6 +1276,10 @@ static int traceprobe_parse_probe_arg_body(const char *argv, ssize_t *size,
goto fail;
}
}
+
+ if (!strcmp(parg->type->name, "pd"))
+ code++;
+
/* If op == DEREF, replace it with STRING */
if (!strcmp(parg->type->name, "ustring") ||
code->op == FETCH_OP_UDEREF)
@@ -1321,6 +1358,7 @@ static int traceprobe_parse_probe_arg_body(const char *argv, ssize_t *size,
kfree(tmp);
out:
kfree(arg);
+ kfree(org_arg);
return ret;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/3] tracing/probes: support '%pD' type for print struct file's name
2024-01-19 1:38 [PATCH 0/3] support '%pd' and '%pD' for print file name Ye Bin
2024-01-19 1:38 ` [PATCH 1/3] tracing/probes: support '%pd' type for print struct dentry's name Ye Bin
@ 2024-01-19 1:38 ` Ye Bin
2024-01-19 1:38 ` [PATCH 3/3] Documentation: tracing: add new type 'pd' and 'pD' for kprobe Ye Bin
2024-01-19 14:43 ` [PATCH 0/3] support '%pd' and '%pD' for print file name Masami Hiramatsu
3 siblings, 0 replies; 11+ messages in thread
From: Ye Bin @ 2024-01-19 1:38 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 | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 460f98b85b1c..400a1dd52c39 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/dcache.h>
+#include <linux/fs.h>
#include <linux/bpf.h>
#include "trace_btf.h"
@@ -89,6 +90,8 @@ static const struct fetch_type probe_fetch_types[] = {
"__data_loc char[]"),
__ASSIGN_FETCH_TYPE("pd", string, string, sizeof(u32), 1, 1,
"__data_loc char[]"),
+ __ASSIGN_FETCH_TYPE("pD", string, string, sizeof(u32), 1, 1,
+ "__data_loc char[]"),
/* Basic types */
ASSIGN_FETCH_TYPE(u8, u8, 0),
ASSIGN_FETCH_TYPE(u16, u16, 0),
@@ -1093,18 +1096,26 @@ static int __parse_bitfield_probe_arg(const char *bf,
return (BYTES_TO_BITS(t->size) < (bw + bo)) ? -EINVAL : 0;
}
-static char* traceprobe_expand_dentry(const char *argv)
+static char* traceprobe_expand_dentry_file(const char *argv, bool is_dentry)
{
#define DENTRY_EXPAND_LEN 7 /* +0xXX() */
+ #define FILE_EXPAND_LEN 15 /* +0xXX(+0xXXX()) */
char *new_argv;
- int len = strlen(argv) + 1 + DENTRY_EXPAND_LEN;
+ int len, ret;
+ len = strlen(argv) + 1 + (is_dentry ? DENTRY_EXPAND_LEN : FILE_EXPAND_LEN);
new_argv = kmalloc(len, GFP_KERNEL);
if (!new_argv)
return NULL;
- if (snprintf(new_argv, len, "+0x%lx(%s)",
- offsetof(struct dentry, d_name.name), argv) >= len) {
+ if (is_dentry)
+ ret = snprintf(new_argv, len, "+0x%lx(%s)",
+ offsetof(struct dentry, d_name.name), argv);
+ else
+ ret = snprintf(new_argv, len, "+0x%lx(+0x%lx(%s))",
+ offsetof(struct dentry, d_name.name),
+ offsetof(struct file, f_path.dentry), argv);
+ if (ret >= len) {
kfree(new_argv);
return NULL;
}
@@ -1205,10 +1216,11 @@ static int traceprobe_parse_probe_arg_body(const char *argv, ssize_t *size,
parg->count);
}
- if (!strcmp("pd", parg->type->name)) {
+ if (!strcasecmp("pd", parg->type->name)) {
char *temp;
- temp = traceprobe_expand_dentry(arg);
+ temp = traceprobe_expand_dentry_file(arg,
+ parg->type->name[1] == 'd');
if (!temp)
goto out;
org_arg = arg;
@@ -1277,7 +1289,7 @@ static int traceprobe_parse_probe_arg_body(const char *argv, ssize_t *size,
}
}
- if (!strcmp(parg->type->name, "pd"))
+ if (!strcasecmp(parg->type->name, "pd"))
code++;
/* If op == DEREF, replace it with STRING */
--
2.31.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] Documentation: tracing: add new type 'pd' and 'pD' for kprobe
2024-01-19 1:38 [PATCH 0/3] support '%pd' and '%pD' for print file name Ye Bin
2024-01-19 1:38 ` [PATCH 1/3] tracing/probes: support '%pd' type for print struct dentry's name Ye Bin
2024-01-19 1:38 ` [PATCH 2/3] tracing/probes: support '%pD' type for print struct file's name Ye Bin
@ 2024-01-19 1:38 ` Ye Bin
2024-01-19 14:43 ` [PATCH 0/3] support '%pd' and '%pD' for print file name Masami Hiramatsu
3 siblings, 0 replies; 11+ messages in thread
From: Ye Bin @ 2024-01-19 1:38 UTC (permalink / raw)
To: rostedt, mhiramat, mathieu.desnoyers, linux-trace-kernel
Cc: linux-kernel, yebin10
Similar to printk() 'pd' is for print dentry's name, and 'pD' is
for print file's name.
Signed-off-by: Ye Bin <yebin10@huawei.com>
---
Documentation/trace/kprobetrace.rst | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/trace/kprobetrace.rst b/Documentation/trace/kprobetrace.rst
index bf9cecb69fc9..c46c5d0a25e6 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
--
2.31.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 0/3] support '%pd' and '%pD' for print file name
2024-01-19 1:38 [PATCH 0/3] support '%pd' and '%pD' for print file name Ye Bin
` (2 preceding siblings ...)
2024-01-19 1:38 ` [PATCH 3/3] Documentation: tracing: add new type 'pd' and 'pD' for kprobe Ye Bin
@ 2024-01-19 14:43 ` Masami Hiramatsu
2024-01-19 15:52 ` Steven Rostedt
2024-01-20 6:26 ` yebin (H)
3 siblings, 2 replies; 11+ messages in thread
From: Masami Hiramatsu @ 2024-01-19 14:43 UTC (permalink / raw)
To: Ye Bin; +Cc: rostedt, mathieu.desnoyers, linux-trace-kernel, linux-kernel
On Fri, 19 Jan 2024 09:38:45 +0800
Ye Bin <yebin10@huawei.com> wrote:
> 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.
Hi Ye,
Thanks for your proposal!
Generically, I think this type of hack is not good for the tracing
because there are already some ways to do that. e.g.
- Use perf probe to specify dentry->name:string or file->name:string
- Use BTF to specify in the same way (but only for function entry)
And those are more obvious what it does.
However, if this is implemented in more generic syntax, it will be
acceptable.
For example, type specifying with "arg1:printfmt(%pD)" will be
more generic because it is apparently one of the printfmt and output
string. Or, maybe we can just allow to use ":%pD" as a fetch type
(start with '%' means the printfmt)
Also, could you update readme_msg[] in kernel/trace/trace.c if
you add a type, and add a testcase of selftests/ftrace, for this
feature? Documentation should also be updated with more syntax
information.
Thank you,
>
> Ye Bin (3):
> tracing/probes: support '%pd' type for print struct dentry's name
> tracing/probes: support '%pD' type for print struct file's name
> Documentation: tracing: add new type 'pd' and 'pD' for kprobe
>
> Documentation/trace/kprobetrace.rst | 3 +-
> kernel/trace/trace_probe.c | 50 +++++++++++++++++++++++++++++
> 2 files changed, 52 insertions(+), 1 deletion(-)
>
> --
> 2.31.1
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] tracing/probes: support '%pd' type for print struct dentry's name
2024-01-19 1:38 ` [PATCH 1/3] tracing/probes: support '%pd' type for print struct dentry's name Ye Bin
@ 2024-01-19 14:57 ` Masami Hiramatsu
2024-01-20 0:59 ` kernel test robot
2024-01-20 6:27 ` kernel test robot
2 siblings, 0 replies; 11+ messages in thread
From: Masami Hiramatsu @ 2024-01-19 14:57 UTC (permalink / raw)
To: Ye Bin; +Cc: rostedt, mathieu.desnoyers, linux-trace-kernel, linux-kernel
On Fri, 19 Jan 2024 09:38:46 +0800
Ye Bin <yebin10@huawei.com> wrote:
> Similar to '%pd' for printk, use 'pd' for print struct dentry's name.
No, please use '%pd' as a type instead.
>
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
> kernel/trace/trace_probe.c | 38 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
>
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index 4dc74d73fc1d..460f98b85b1c 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -11,6 +11,7 @@
> */
> #define pr_fmt(fmt) "trace_probe: " fmt
>
> +#include <linux/dcache.h>
> #include <linux/bpf.h>
> #include "trace_btf.h"
>
> @@ -86,6 +87,8 @@ static const struct fetch_type probe_fetch_types[] = {
> "__data_loc char[]"),
> __ASSIGN_FETCH_TYPE("symstr", string, string, sizeof(u32), 1, 1,
> "__data_loc char[]"),
> + __ASSIGN_FETCH_TYPE("pd", string, string, sizeof(u32), 1, 1,
> + "__data_loc char[]"),
And you don't need to add a new type.
Similar to the preprocessor macro, this should be done before parsing the
args. See traceprobe_expand_meta_args(), you can scan the args and search
argument end with ':%pd'. If you find that, you can replace it with
"+0x%lx(%s):string". Then, you don't need any of these complex fetch type.
Thank you,
> /* Basic types */
> ASSIGN_FETCH_TYPE(u8, u8, 0),
> ASSIGN_FETCH_TYPE(u16, u16, 0),
> @@ -1090,6 +1093,25 @@ static int __parse_bitfield_probe_arg(const char *bf,
> return (BYTES_TO_BITS(t->size) < (bw + bo)) ? -EINVAL : 0;
> }
>
> +static char* traceprobe_expand_dentry(const char *argv)
> +{
> + #define DENTRY_EXPAND_LEN 7 /* +0xXX() */
> + char *new_argv;
> + int len = strlen(argv) + 1 + DENTRY_EXPAND_LEN;
> +
> + new_argv = kmalloc(len, GFP_KERNEL);
> + if (!new_argv)
> + return NULL;
> +
> + if (snprintf(new_argv, len, "+0x%lx(%s)",
> + offsetof(struct dentry, d_name.name), argv) >= len) {
> + kfree(new_argv);
> + return NULL;
> + }
> +
> + return new_argv;
> +}
> +
> /* String length checking wrapper */
> static int traceprobe_parse_probe_arg_body(const char *argv, ssize_t *size,
> struct probe_arg *parg,
> @@ -1099,6 +1121,7 @@ static int traceprobe_parse_probe_arg_body(const char *argv, ssize_t *size,
> char *t, *t2, *t3;
> int ret, len;
> char *arg;
> + char *org_arg = NULL;
>
> arg = kstrdup(argv, GFP_KERNEL);
> if (!arg)
> @@ -1182,6 +1205,16 @@ static int traceprobe_parse_probe_arg_body(const char *argv, ssize_t *size,
> parg->count);
> }
>
> + if (!strcmp("pd", parg->type->name)) {
> + char *temp;
> +
> + temp = traceprobe_expand_dentry(arg);
> + if (!temp)
> + goto out;
> + org_arg = arg;
> + arg = temp;
> + }
> +
> code = tmp = kcalloc(FETCH_INSN_MAX, sizeof(*code), GFP_KERNEL);
> if (!code)
> goto out;
> @@ -1243,6 +1276,10 @@ static int traceprobe_parse_probe_arg_body(const char *argv, ssize_t *size,
> goto fail;
> }
> }
> +
> + if (!strcmp(parg->type->name, "pd"))
> + code++;
> +
> /* If op == DEREF, replace it with STRING */
> if (!strcmp(parg->type->name, "ustring") ||
> code->op == FETCH_OP_UDEREF)
> @@ -1321,6 +1358,7 @@ static int traceprobe_parse_probe_arg_body(const char *argv, ssize_t *size,
> kfree(tmp);
> out:
> kfree(arg);
> + kfree(org_arg);
>
> return ret;
> }
> --
> 2.31.1
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/3] support '%pd' and '%pD' for print file name
2024-01-19 14:43 ` [PATCH 0/3] support '%pd' and '%pD' for print file name Masami Hiramatsu
@ 2024-01-19 15:52 ` Steven Rostedt
2024-01-20 7:15 ` Masami Hiramatsu
2024-01-20 6:26 ` yebin (H)
1 sibling, 1 reply; 11+ messages in thread
From: Steven Rostedt @ 2024-01-19 15:52 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Ye Bin, mathieu.desnoyers, linux-trace-kernel, linux-kernel
On Fri, 19 Jan 2024 23:43:56 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> Thanks for your proposal!
>
> Generically, I think this type of hack is not good for the tracing
> because there are already some ways to do that. e.g.
> - Use perf probe to specify dentry->name:string or file->name:string
> - Use BTF to specify in the same way (but only for function entry)
> And those are more obvious what it does.
>
> However, if this is implemented in more generic syntax, it will be
> acceptable.
> For example, type specifying with "arg1:printfmt(%pD)" will be
> more generic because it is apparently one of the printfmt and output
> string. Or, maybe we can just allow to use ":%pD" as a fetch type
> (start with '%' means the printfmt)
Yes, I like this idea a lot. Please add the '%' keyword/token to change how
to display this in the print format.
We may need to add more than one token though. Is that supported?
$arg1:u32:%08x
or that could also be:
$arg1:%08x:u32
That is, the order should not be important.
Thoughts?
-- Steve
>
> Also, could you update readme_msg[] in kernel/trace/trace.c if
> you add a type, and add a testcase of selftests/ftrace, for this
> feature? Documentation should also be updated with more syntax
> information.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] tracing/probes: support '%pd' type for print struct dentry's name
2024-01-19 1:38 ` [PATCH 1/3] tracing/probes: support '%pd' type for print struct dentry's name Ye Bin
2024-01-19 14:57 ` Masami Hiramatsu
@ 2024-01-20 0:59 ` kernel test robot
2024-01-20 6:27 ` kernel test robot
2 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2024-01-20 0:59 UTC (permalink / raw)
To: Ye Bin, rostedt, mhiramat, mathieu.desnoyers, linux-trace-kernel
Cc: oe-kbuild-all, linux-kernel, yebin10
Hi Ye,
kernel test robot noticed the following build warnings:
[auto build test WARNING on linus/master]
[also build test WARNING on v6.7 next-20240119]
[cannot apply to rostedt-trace/for-next rostedt-trace/for-next-urgent]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Ye-Bin/tracing-probes-support-pd-type-for-print-struct-dentry-s-name/20240119-093912
base: linus/master
patch link: https://lore.kernel.org/r/20240119013848.3111364-2-yebin10%40huawei.com
patch subject: [PATCH 1/3] tracing/probes: support '%pd' type for print struct dentry's name
config: i386-defconfig (https://download.01.org/0day-ci/archive/20240120/202401200837.b9DEGdBT-lkp@intel.com/config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240120/202401200837.b9DEGdBT-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202401200837.b9DEGdBT-lkp@intel.com/
All warnings (new ones prefixed by >>):
kernel/trace/trace_probe.c: In function 'traceprobe_expand_dentry':
>> kernel/trace/trace_probe.c:1106:36: warning: format '%lx' expects argument of type 'long unsigned int', but argument 4 has type 'unsigned int' [-Wformat=]
if (snprintf(new_argv, len, "+0x%lx(%s)",
~~^
%x
vim +1106 kernel/trace/trace_probe.c
1095
1096 static char* traceprobe_expand_dentry(const char *argv)
1097 {
1098 #define DENTRY_EXPAND_LEN 7 /* +0xXX() */
1099 char *new_argv;
1100 int len = strlen(argv) + 1 + DENTRY_EXPAND_LEN;
1101
1102 new_argv = kmalloc(len, GFP_KERNEL);
1103 if (!new_argv)
1104 return NULL;
1105
> 1106 if (snprintf(new_argv, len, "+0x%lx(%s)",
1107 offsetof(struct dentry, d_name.name), argv) >= len) {
1108 kfree(new_argv);
1109 return NULL;
1110 }
1111
1112 return new_argv;
1113 }
1114
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/3] support '%pd' and '%pD' for print file name
2024-01-19 14:43 ` [PATCH 0/3] support '%pd' and '%pD' for print file name Masami Hiramatsu
2024-01-19 15:52 ` Steven Rostedt
@ 2024-01-20 6:26 ` yebin (H)
1 sibling, 0 replies; 11+ messages in thread
From: yebin (H) @ 2024-01-20 6:26 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: rostedt, mathieu.desnoyers, linux-trace-kernel, linux-kernel
On 2024/1/19 22:43, Masami Hiramatsu (Google) wrote:
> On Fri, 19 Jan 2024 09:38:45 +0800
> Ye Bin <yebin10@huawei.com> wrote:
>
>> 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.
> Hi Ye,
>
> Thanks for your proposal!
>
> Generically, I think this type of hack is not good for the tracing
> because there are already some ways to do that. e.g.
> - Use perf probe to specify dentry->name:string or file->name:string
> - Use BTF to specify in the same way (but only for function entry)
> And those are more obvious what it does.
>
> However, if this is implemented in more generic syntax, it will be
> acceptable.
> For example, type specifying with "arg1:printfmt(%pD)" will be
> more generic because it is apparently one of the printfmt and output
> string. Or, maybe we can just allow to use ":%pD" as a fetch type
> (start with '%' means the printfmt)
>
> Also, could you update readme_msg[] in kernel/trace/trace.c if
> you add a type, and add a testcase of selftests/ftrace, for this
> feature? Documentation should also be updated with more syntax
> information.
>
> Thank you,
Thank you very much for your suggestion.
I will re-implement this function according to your suggestion.
>> Ye Bin (3):
>> tracing/probes: support '%pd' type for print struct dentry's name
>> tracing/probes: support '%pD' type for print struct file's name
>> Documentation: tracing: add new type 'pd' and 'pD' for kprobe
>>
>> Documentation/trace/kprobetrace.rst | 3 +-
>> kernel/trace/trace_probe.c | 50 +++++++++++++++++++++++++++++
>> 2 files changed, 52 insertions(+), 1 deletion(-)
>>
>> --
>> 2.31.1
>>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] tracing/probes: support '%pd' type for print struct dentry's name
2024-01-19 1:38 ` [PATCH 1/3] tracing/probes: support '%pd' type for print struct dentry's name Ye Bin
2024-01-19 14:57 ` Masami Hiramatsu
2024-01-20 0:59 ` kernel test robot
@ 2024-01-20 6:27 ` kernel test robot
2 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2024-01-20 6:27 UTC (permalink / raw)
To: Ye Bin, rostedt, mhiramat, mathieu.desnoyers, linux-trace-kernel
Cc: oe-kbuild-all, linux-kernel, yebin10
Hi Ye,
kernel test robot noticed the following build warnings:
[auto build test WARNING on linus/master]
[also build test WARNING on v6.7 next-20240119]
[cannot apply to rostedt-trace/for-next rostedt-trace/for-next-urgent]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Ye-Bin/tracing-probes-support-pd-type-for-print-struct-dentry-s-name/20240119-093912
base: linus/master
patch link: https://lore.kernel.org/r/20240119013848.3111364-2-yebin10%40huawei.com
patch subject: [PATCH 1/3] tracing/probes: support '%pd' type for print struct dentry's name
config: i386-randconfig-051-20240120 (https://download.01.org/0day-ci/archive/20240120/202401201403.aTEbo79S-lkp@intel.com/config)
compiler: ClangBuiltLinux clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240120/202401201403.aTEbo79S-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202401201403.aTEbo79S-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> kernel/trace/trace_probe.c:1107:8: warning: format specifies type 'unsigned long' but the argument has type 'unsigned int' [-Wformat]
1106 | if (snprintf(new_argv, len, "+0x%lx(%s)",
| ~~~
| %x
1107 | offsetof(struct dentry, d_name.name), argv) >= len) {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/stddef.h:16:32: note: expanded from macro 'offsetof'
16 | #define offsetof(TYPE, MEMBER) __builtin_offsetof(TYPE, MEMBER)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
vim +1107 kernel/trace/trace_probe.c
1095
1096 static char* traceprobe_expand_dentry(const char *argv)
1097 {
1098 #define DENTRY_EXPAND_LEN 7 /* +0xXX() */
1099 char *new_argv;
1100 int len = strlen(argv) + 1 + DENTRY_EXPAND_LEN;
1101
1102 new_argv = kmalloc(len, GFP_KERNEL);
1103 if (!new_argv)
1104 return NULL;
1105
1106 if (snprintf(new_argv, len, "+0x%lx(%s)",
> 1107 offsetof(struct dentry, d_name.name), argv) >= len) {
1108 kfree(new_argv);
1109 return NULL;
1110 }
1111
1112 return new_argv;
1113 }
1114
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/3] support '%pd' and '%pD' for print file name
2024-01-19 15:52 ` Steven Rostedt
@ 2024-01-20 7:15 ` Masami Hiramatsu
0 siblings, 0 replies; 11+ messages in thread
From: Masami Hiramatsu @ 2024-01-20 7:15 UTC (permalink / raw)
To: Steven Rostedt
Cc: Ye Bin, mathieu.desnoyers, linux-trace-kernel, linux-kernel
On Fri, 19 Jan 2024 10:52:43 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Fri, 19 Jan 2024 23:43:56 +0900
> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
>
> > Thanks for your proposal!
> >
> > Generically, I think this type of hack is not good for the tracing
> > because there are already some ways to do that. e.g.
> > - Use perf probe to specify dentry->name:string or file->name:string
> > - Use BTF to specify in the same way (but only for function entry)
> > And those are more obvious what it does.
> >
> > However, if this is implemented in more generic syntax, it will be
> > acceptable.
> > For example, type specifying with "arg1:printfmt(%pD)" will be
> > more generic because it is apparently one of the printfmt and output
> > string. Or, maybe we can just allow to use ":%pD" as a fetch type
> > (start with '%' means the printfmt)
>
> Yes, I like this idea a lot. Please add the '%' keyword/token to change how
> to display this in the print format.
>
> We may need to add more than one token though. Is that supported?
>
> $arg1:u32:%08x
>
> or that could also be:
>
> $arg1:%08x:u32
No, not yet. But I rather like comma separated.
$arg1:u32,%08x
Hm, this needs more changes, like a new type parser. And it will be
a option of the default type.
Thank you,
>
> That is, the order should not be important.
>
> Thoughts?
>
> -- Steve
>
>
> >
> > Also, could you update readme_msg[] in kernel/trace/trace.c if
> > you add a type, and add a testcase of selftests/ftrace, for this
> > feature? Documentation should also be updated with more syntax
> > information.
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-01-20 7:15 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-19 1:38 [PATCH 0/3] support '%pd' and '%pD' for print file name Ye Bin
2024-01-19 1:38 ` [PATCH 1/3] tracing/probes: support '%pd' type for print struct dentry's name Ye Bin
2024-01-19 14:57 ` Masami Hiramatsu
2024-01-20 0:59 ` kernel test robot
2024-01-20 6:27 ` kernel test robot
2024-01-19 1:38 ` [PATCH 2/3] tracing/probes: support '%pD' type for print struct file's name Ye Bin
2024-01-19 1:38 ` [PATCH 3/3] Documentation: tracing: add new type 'pd' and 'pD' for kprobe Ye Bin
2024-01-19 14:43 ` [PATCH 0/3] support '%pd' and '%pD' for print file name Masami Hiramatsu
2024-01-19 15:52 ` Steven Rostedt
2024-01-20 7:15 ` Masami Hiramatsu
2024-01-20 6:26 ` yebin (H)
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 NNTP newsgroup(s).