2024-01-19 01:36:31

by yebin (H)

[permalink] [raw]
Subject: [PATCH 0/3] 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.

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



2024-01-19 01:36:35

by yebin (H)

[permalink] [raw]
Subject: [PATCH 1/3] 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_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


2024-01-19 01:36:36

by yebin (H)

[permalink] [raw]
Subject: [PATCH 3/3] Documentation: tracing: add new type 'pd' and 'pD' for kprobe

Similar to printk() 'pd' is for print dentry's name, and 'pD' is
for print file's name.

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


2024-01-19 01:36:53

by yebin (H)

[permalink] [raw]
Subject: [PATCH 2/3] 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 | 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


2024-01-19 14:44:14

by Masami Hiramatsu

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

On Fri, 19 Jan 2024 09:38:45 +0800
Ye Bin <[email protected]> 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) <[email protected]>

2024-01-19 14:58:40

by Masami Hiramatsu

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

On Fri, 19 Jan 2024 09:38:46 +0800
Ye Bin <[email protected]> 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 <[email protected]>
> ---
> 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) <[email protected]>

2024-01-19 15:52:01

by Steven Rostedt

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

On Fri, 19 Jan 2024 23:43:56 +0900
Masami Hiramatsu (Google) <[email protected]> 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.

2024-01-20 01:00:37

by kernel test robot

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

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/[email protected]/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/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

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

2024-01-20 06:27:01

by yebin (H)

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



On 2024/1/19 22:43, Masami Hiramatsu (Google) wrote:
> On Fri, 19 Jan 2024 09:38:45 +0800
> Ye Bin <[email protected]> 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
>>
>


2024-01-20 06:27:42

by kernel test robot

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

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/[email protected]/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/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

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

2024-01-20 07:15:47

by Masami Hiramatsu

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

On Fri, 19 Jan 2024 10:52:43 -0500
Steven Rostedt <[email protected]> wrote:

> On Fri, 19 Jan 2024 23:43:56 +0900
> Masami Hiramatsu (Google) <[email protected]> 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) <[email protected]>