linux-trace-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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).