1. Patchew
  2. linux
  3. [PATCH v2 00/10] perf trace: Enhanced augmentation for pointer arguments
  4. View patch

[PATCH v2 04/10] perf trace: Add some string arguments' name in syscall_arg_fmt__init_array()

Howard Chu posted 10 patches 1 year, 5 months ago
There is a newer version of this series
[PATCH v2 04/10] perf trace: Add some string arguments' name in syscall_arg_fmt__init_array()
Posted by Howard Chu 1 year, 5 months ago 
Add them so that we can augment more strings (which is a file path)

Signed-off-by: Howard Chu <howardchu95@gmail.com>
---
 tools/perf/builtin-trace.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index e7e8c89d9538..84c7398312d8 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -1918,7 +1918,13 @@ syscall_arg_fmt__init_array(struct syscall_arg_fmt *arg, struct tep_format_field
 
 		if (strcmp(field->type, "const char *") == 0 &&
 		    ((len >= 4 && strcmp(field->name + len - 4, "name") == 0) ||
-		     strstr(field->name, "path") != NULL))
+		     strstr(field->name, "path") ||
+		     strstr(field->name, "file") ||
+		     strstr(field->name, "root") ||
+		     strstr(field->name, "key") ||
+		     strstr(field->name, "special") ||
+		     strstr(field->name, "type") ||
+		     strstr(field->name, "description")))
 			arg->scnprintf = SCA_FILENAME;
 		else if ((field->flags & TEP_FIELD_IS_POINTER) || strstr(field->name, "addr"))
 			arg->scnprintf = SCA_PTR;
-- 
2.45.2
Re: [PATCH v2 04/10] perf trace: Add some string arguments' name in syscall_arg_fmt__init_array()
Posted by Arnaldo Carvalho de Melo 1 year, 5 months ago 
On Thu, Aug 15, 2024 at 09:36:20AM +0800, Howard Chu wrote:
> Add them so that we can augment more strings (which is a file path)
> 
> Signed-off-by: Howard Chu <howardchu95@gmail.com>
> ---
>  tools/perf/builtin-trace.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index e7e8c89d9538..84c7398312d8 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -1918,7 +1918,13 @@ syscall_arg_fmt__init_array(struct syscall_arg_fmt *arg, struct tep_format_field
>  
>  		if (strcmp(field->type, "const char *") == 0 &&
>  		    ((len >= 4 && strcmp(field->name + len - 4, "name") == 0) ||
> -		     strstr(field->name, "path") != NULL))
> +		     strstr(field->name, "path") ||
> +		     strstr(field->name, "file") ||
> +		     strstr(field->name, "root") ||
> +		     strstr(field->name, "key") ||
> +		     strstr(field->name, "special") ||
> +		     strstr(field->name, "type") ||
> +		     strstr(field->name, "description")))
>  			arg->scnprintf = SCA_FILENAME;
>  		else if ((field->flags & TEP_FIELD_IS_POINTER) || strstr(field->name, "addr"))
>  			arg->scnprintf = SCA_PTR;

Humm?

root@number:~# for field_name in file root key special type description ; do grep "field:.* $field_name\>" /sys/kernel/tracing/events/syscalls/sys_enter_*/format ; done

/sys/kernel/tracing/events/syscalls/sys_enter_msgget/format:	field:key_t key;	offset:16;	size:8;	signed:0;
/sys/kernel/tracing/events/syscalls/sys_enter_semget/format:	field:key_t key;	offset:16;	size:8;	signed:0;
/sys/kernel/tracing/events/syscalls/sys_enter_shmget/format:	field:key_t key;	offset:16;	size:8;	signed:0;
/sys/kernel/tracing/events/syscalls/sys_enter_quotactl/format:	field:const char * special;	offset:24;	size:8;	signed:0;
/sys/kernel/tracing/events/syscalls/sys_enter_kcmp/format:	field:int type;	offset:32;	size:8;	signed:0;
/sys/kernel/tracing/events/syscalls/sys_enter_mount/format:	field:char * type;	offset:32;	size:8;	signed:0;
/sys/kernel/tracing/events/syscalls/sys_enter_socket/format:	field:int type;	offset:24;	size:8;	signed:0;
/sys/kernel/tracing/events/syscalls/sys_enter_socketpair/format:	field:int type;	offset:24;	size:8;	signed:0;
/sys/kernel/tracing/events/syscalls/sys_enter_syslog/format:	field:int type;	offset:16;	size:8;	signed:0;
root@number:~#

Skipping this one. Please ellaborate, what am I missing?

- Arnaldo
Re: [PATCH v2 04/10] perf trace: Add some string arguments' name in syscall_arg_fmt__init_array()
Posted by Howard Chu 1 year, 5 months ago 
On Fri, Aug 23, 2024 at 6:14 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> On Thu, Aug 15, 2024 at 09:36:20AM +0800, Howard Chu wrote:
> > Add them so that we can augment more strings (which is a file path)
> >
> > Signed-off-by: Howard Chu <howardchu95@gmail.com>
> > ---
> >  tools/perf/builtin-trace.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> > index e7e8c89d9538..84c7398312d8 100644
> > --- a/tools/perf/builtin-trace.c
> > +++ b/tools/perf/builtin-trace.c
> > @@ -1918,7 +1918,13 @@ syscall_arg_fmt__init_array(struct syscall_arg_fmt *arg, struct tep_format_field
> >
> >               if (strcmp(field->type, "const char *") == 0 &&
> >                   ((len >= 4 && strcmp(field->name + len - 4, "name") == 0) ||
> > -                  strstr(field->name, "path") != NULL))
> > +                  strstr(field->name, "path") ||
> > +                  strstr(field->name, "file") ||
> > +                  strstr(field->name, "root") ||
> > +                  strstr(field->name, "key") ||
> > +                  strstr(field->name, "special") ||
> > +                  strstr(field->name, "type") ||
> > +                  strstr(field->name, "description")))
> >                       arg->scnprintf = SCA_FILENAME;
> >               else if ((field->flags & TEP_FIELD_IS_POINTER) || strstr(field->name, "addr"))
> >                       arg->scnprintf = SCA_PTR;
>
> Humm?
>
> root@number:~# for field_name in file root key special type description ; do grep "field:.* $field_name\>" /sys/kernel/tracing/events/syscalls/sys_enter_*/format ; done
>
> /sys/kernel/tracing/events/syscalls/sys_enter_msgget/format:    field:key_t key;        offset:16;      size:8; signed:0;
> /sys/kernel/tracing/events/syscalls/sys_enter_semget/format:    field:key_t key;        offset:16;      size:8; signed:0;
> /sys/kernel/tracing/events/syscalls/sys_enter_shmget/format:    field:key_t key;        offset:16;      size:8; signed:0;
> /sys/kernel/tracing/events/syscalls/sys_enter_quotactl/format:  field:const char * special;     offset:24;      size:8; signed:0;
> /sys/kernel/tracing/events/syscalls/sys_enter_kcmp/format:      field:int type; offset:32;      size:8; signed:0;
> /sys/kernel/tracing/events/syscalls/sys_enter_mount/format:     field:char * type;      offset:32;      size:8; signed:0;
> /sys/kernel/tracing/events/syscalls/sys_enter_socket/format:    field:int type; offset:24;      size:8; signed:0;
> /sys/kernel/tracing/events/syscalls/sys_enter_socketpair/format:        field:int type; offset:24;      size:8; signed:0;
> /sys/kernel/tracing/events/syscalls/sys_enter_syslog/format:    field:int type; offset:16;      size:8; signed:0;
> root@number:~#
>
> Skipping this one. Please ellaborate, what am I missing?

Hello, just some minor changes on your command, if I do:
```
perf $ for field_name in file root key special type description ; do
grep "field:.*char \* .*$field_name\>"
/sys/kernel/tracing/events/syscalls/sys_enter_*/format ; done
/sys/kernel/tracing/events/syscalls/sys_enter_swapoff/format:
field:const char * specialfile;   offset:16;      size:8; signed:0;
/sys/kernel/tracing/events/syscalls/sys_enter_swapon/format:
field:const char * specialfile;   offset:16;      size:8; signed:0;
/sys/kernel/tracing/events/syscalls/sys_enter_pivot_root/format:
field:const char * new_root;     offset:16;      size:8; signed:0;
/sys/kernel/tracing/events/syscalls/sys_enter_fsconfig/format:
field:const char * _key;  offset:32;      size:8; signed:0;
/sys/kernel/tracing/events/syscalls/sys_enter_quotactl/format:
field:const char * special;       offset:24;      size:8; signed:0;
/sys/kernel/tracing/events/syscalls/sys_enter_add_key/format:
field:const char * _type; offset:16;      size:8; signed:0;
/sys/kernel/tracing/events/syscalls/sys_enter_mount/format:
field:char * type;        offset:32;      size:8; signed:0;
/sys/kernel/tracing/events/syscalls/sys_enter_request_key/format:field:const
char * _type;        offset:16;      size:8; signed:0;
/sys/kernel/tracing/events/syscalls/sys_enter_add_key/format:
field:const char * _description;  offset:24;      size:8; signed:0;
/sys/kernel/tracing/events/syscalls/sys_enter_request_key/format:field:const
char * _description; offset:24;      size:8; signed:0;
```

They pop up.

Because it's strstr(), not strcmp(). Do you think we should use
"strstr(field->name, "description") ||" or "strstr(field->name,
"_description") ||"? Please let me know.

Thanks,
Howard

>
> - Arnaldo
Re: [PATCH v2 04/10] perf trace: Add some string arguments' name in syscall_arg_fmt__init_array()
Posted by Arnaldo Carvalho de Melo 1 year, 5 months ago 
On Fri, Aug 23, 2024 at 12:37:01PM +0800, Howard Chu wrote:
> On Fri, Aug 23, 2024 at 6:14 AM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> >
> > On Thu, Aug 15, 2024 at 09:36:20AM +0800, Howard Chu wrote:
> > > Add them so that we can augment more strings (which is a file path)
> > >
> > > Signed-off-by: Howard Chu <howardchu95@gmail.com>
> > > ---
> > >  tools/perf/builtin-trace.c | 8 +++++++-
> > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> > > index e7e8c89d9538..84c7398312d8 100644
> > > --- a/tools/perf/builtin-trace.c
> > > +++ b/tools/perf/builtin-trace.c
> > > @@ -1918,7 +1918,13 @@ syscall_arg_fmt__init_array(struct syscall_arg_fmt *arg, struct tep_format_field
> > >
> > >               if (strcmp(field->type, "const char *") == 0 &&
> > >                   ((len >= 4 && strcmp(field->name + len - 4, "name") == 0) ||
> > > -                  strstr(field->name, "path") != NULL))
> > > +                  strstr(field->name, "path") ||
> > > +                  strstr(field->name, "file") ||
> > > +                  strstr(field->name, "root") ||
> > > +                  strstr(field->name, "key") ||
> > > +                  strstr(field->name, "special") ||
> > > +                  strstr(field->name, "type") ||
> > > +                  strstr(field->name, "description")))
> > >                       arg->scnprintf = SCA_FILENAME;
> > >               else if ((field->flags & TEP_FIELD_IS_POINTER) || strstr(field->name, "addr"))
> > >                       arg->scnprintf = SCA_PTR;
> >
> > Humm?
> >
> > root@number:~# for field_name in file root key special type description ; do grep "field:.* $field_name\>" /sys/kernel/tracing/events/syscalls/sys_enter_*/format ; done
> >
> > /sys/kernel/tracing/events/syscalls/sys_enter_msgget/format:    field:key_t key;        offset:16;      size:8; signed:0;
> > /sys/kernel/tracing/events/syscalls/sys_enter_semget/format:    field:key_t key;        offset:16;      size:8; signed:0;
> > /sys/kernel/tracing/events/syscalls/sys_enter_shmget/format:    field:key_t key;        offset:16;      size:8; signed:0;
> > /sys/kernel/tracing/events/syscalls/sys_enter_quotactl/format:  field:const char * special;     offset:24;      size:8; signed:0;
> > /sys/kernel/tracing/events/syscalls/sys_enter_kcmp/format:      field:int type; offset:32;      size:8; signed:0;
> > /sys/kernel/tracing/events/syscalls/sys_enter_mount/format:     field:char * type;      offset:32;      size:8; signed:0;
> > /sys/kernel/tracing/events/syscalls/sys_enter_socket/format:    field:int type; offset:24;      size:8; signed:0;
> > /sys/kernel/tracing/events/syscalls/sys_enter_socketpair/format:        field:int type; offset:24;      size:8; signed:0;
> > /sys/kernel/tracing/events/syscalls/sys_enter_syslog/format:    field:int type; offset:16;      size:8; signed:0;
> > root@number:~#
> >
> > Skipping this one. Please ellaborate, what am I missing?
> 
> Hello, just some minor changes on your command, if I do:
> ```
> perf $ for field_name in file root key special type description ; do
> grep "field:.*char \* .*$field_name\>"
> /sys/kernel/tracing/events/syscalls/sys_enter_*/format ; done
> /sys/kernel/tracing/events/syscalls/sys_enter_swapoff/format:
> field:const char * specialfile;   offset:16;      size:8; signed:0;
> /sys/kernel/tracing/events/syscalls/sys_enter_swapon/format:
> field:const char * specialfile;   offset:16;      size:8; signed:0;
> /sys/kernel/tracing/events/syscalls/sys_enter_pivot_root/format:
> field:const char * new_root;     offset:16;      size:8; signed:0;
> /sys/kernel/tracing/events/syscalls/sys_enter_fsconfig/format:
> field:const char * _key;  offset:32;      size:8; signed:0;
> /sys/kernel/tracing/events/syscalls/sys_enter_quotactl/format:
> field:const char * special;       offset:24;      size:8; signed:0;
> /sys/kernel/tracing/events/syscalls/sys_enter_add_key/format:
> field:const char * _type; offset:16;      size:8; signed:0;
> /sys/kernel/tracing/events/syscalls/sys_enter_mount/format:
> field:char * type;        offset:32;      size:8; signed:0;
> /sys/kernel/tracing/events/syscalls/sys_enter_request_key/format:field:const
> char * _type;        offset:16;      size:8; signed:0;
> /sys/kernel/tracing/events/syscalls/sys_enter_add_key/format:
> field:const char * _description;  offset:24;      size:8; signed:0;
> /sys/kernel/tracing/events/syscalls/sys_enter_request_key/format:field:const
> char * _description; offset:24;      size:8; signed:0;
> ```
> 
> They pop up.
> 
> Because it's strstr(), not strcmp(). Do you think we should use

Sure, that was my mistake, I was looking for those exact words, and its
being used as substrings.

> "strstr(field->name, "description") ||" or "strstr(field->name,
> "_description") ||"? Please let me know.

But I think using "key" as a a substring is way too generic to think
that a syscall arg with that in its name is necessarily a string.

We better look at the tracepoint _type_ instead since we have it in the
first place :-)

- Arnaldo

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.