* [PATCH v2 0/2] Always show mmap prot even though PROT_NONE @ 2024-04-23 1:53 Changbin Du 2024-04-23 1:53 ` [PATCH v2 1/2] perf trace beauty: Always show param if show_zero is set Changbin Du 2024-04-23 1:53 ` [PATCH v2 2/2] perf trace beauty: Always show mmap prot even though PROT_NONE Changbin Du 0 siblings, 2 replies; 5+ messages in thread From: Changbin Du @ 2024-04-23 1:53 UTC (permalink / raw) To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim Cc: Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter, linux-perf-users, linux-kernel, Changbin Du v2: - Remove bugfix since the code is refactored in perf-tools-next. - Also apply to mmap, pkey_mprotect. Changbin Du (2): perf trace beauty: Always show param if show_zero is set perf trace beauty: Always show mmap prot even though PROT_NONE tools/perf/builtin-trace.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] perf trace beauty: Always show param if show_zero is set 2024-04-23 1:53 [PATCH v2 0/2] Always show mmap prot even though PROT_NONE Changbin Du @ 2024-04-23 1:53 ` Changbin Du 2024-04-23 20:37 ` Arnaldo Carvalho de Melo 2024-04-23 1:53 ` [PATCH v2 2/2] perf trace beauty: Always show mmap prot even though PROT_NONE Changbin Du 1 sibling, 1 reply; 5+ messages in thread From: Changbin Du @ 2024-04-23 1:53 UTC (permalink / raw) To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim Cc: Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter, linux-perf-users, linux-kernel, Changbin Du For some parameters, it is best to also display them when they are 0, e.g. flags. Signed-off-by: Changbin Du <changbin.du@huawei.com> --- tools/perf/builtin-trace.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c index e5fef39c34bf..a8407eee58a3 100644 --- a/tools/perf/builtin-trace.c +++ b/tools/perf/builtin-trace.c @@ -2099,9 +2099,9 @@ static size_t syscall__scnprintf_args(struct syscall *sc, char *bf, size_t size, !trace->show_zeros && !(sc->arg_fmt && (sc->arg_fmt[arg.idx].show_zero || - sc->arg_fmt[arg.idx].scnprintf == SCA_STRARRAY || - sc->arg_fmt[arg.idx].scnprintf == SCA_STRARRAYS) && - sc->arg_fmt[arg.idx].parm)) + ((sc->arg_fmt[arg.idx].scnprintf == SCA_STRARRAY || + sc->arg_fmt[arg.idx].scnprintf == SCA_STRARRAYS) && + sc->arg_fmt[arg.idx].parm)))) continue; printed += scnprintf(bf + printed, size - printed, "%s", printed ? ", " : ""); @@ -2803,8 +2803,8 @@ static size_t trace__fprintf_tp_fields(struct trace *trace, struct evsel *evsel, */ if (val == 0 && !trace->show_zeros && - !((arg->show_zero || - arg->scnprintf == SCA_STRARRAY || + !arg->show_zero && + !((arg->scnprintf == SCA_STRARRAY || arg->scnprintf == SCA_STRARRAYS) && arg->parm)) continue; -- 2.34.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] perf trace beauty: Always show param if show_zero is set 2024-04-23 1:53 ` [PATCH v2 1/2] perf trace beauty: Always show param if show_zero is set Changbin Du @ 2024-04-23 20:37 ` Arnaldo Carvalho de Melo 2024-04-24 2:43 ` duchangbin 0 siblings, 1 reply; 5+ messages in thread From: Arnaldo Carvalho de Melo @ 2024-04-23 20:37 UTC (permalink / raw) To: Changbin Du Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter, linux-perf-users, linux-kernel On Tue, Apr 23, 2024 at 09:53:29AM +0800, Changbin Du wrote: > For some parameters, it is best to also display them when they are 0, > e.g. flags. Please provide examples of what you're changing, to understand your change one has to know what are strarrays and what they handle in 'perf trace', i.e. if the value is zero but the argument has a string array associated, it probably will translate zero into some string. So I did: root@x1:~# perf trace -e syscalls:sys_enter_mmap --filter prot==0 0.000 gnome-shell/2293 syscalls:sys_enter_mmap(addr: 0x10afec7e1000, len: 65536, flags: PRIVATE|FIXED|ANONYMOUS) ^Croot@x1:~# And this is _before_ this patch, after this patch we get: root@x1:~# perf trace -e syscalls:sys_enter_mmap --filter prot==0 0.000 Isolated Web C/25530 syscalls:sys_enter_mmap(addr: 0x7f27df529000, len: 4096, flags: PRIVATE|FIXED|ANONYMOUS) 40.267 DOM Worker/1105511 syscalls:sys_enter_mmap(addr: 0x1c9faec48000, len: 65536, flags: PRIVATE|FIXED|ANONYMOUS) 270.145 firefox/3447 syscalls:sys_enter_mmap(addr: 0x7fa0ed343000, len: 4096, flags: PRIVATE|FIXED|ANONYMOUS) 2194.686 firefox/3447 syscalls:sys_enter_mmap(addr: 0x7fa0ed39f000, len: 4096, flags: PRIVATE|FIXED|ANONYMOUS) 2461.709 Isolated Web C/21794 syscalls:sys_enter_mmap(addr: 0x7fdc3e100000, len: 1048576, flags: PRIVATE|FIXED|ANONYMOUS) 4476.053 firefox/3447 syscalls:sys_enter_mmap(addr: 0x7fa0ed3a1000, len: 4096, flags: PRIVATE|FIXED|ANONYMOUS) ^Croot@x1:~# Because 'mmap's 'prot' is not set directly as an strarray, see: { .name = "mmap", .hexret = true, /* The standard mmap maps to old_mmap on s390x */ #if defined(__s390x__) .alias = "old_mmap", #endif .arg = { [2] = { .scnprintf = SCA_MMAP_PROT, /* prot */ }, [3] = { .scnprintf = SCA_MMAP_FLAGS, /* flags */ .strtoul = STUL_STRARRAY_FLAGS, .parm = &strarray__mmap_flags, }, [5] = { .scnprintf = SCA_HEX, /* offset */ }, }, }, static size_t syscall_arg__scnprintf_mmap_prot(char *bf, size_t size, struct syscall_arg *arg) { unsigned long prot = arg->val; if (prot == 0) return scnprintf(bf, size, "%sNONE", arg->show_string_prefix ? strarray__mmap_prot.prefix : ""); return mmap__scnprintf_prot(prot, bf, size, arg->show_string_prefix); } #define SCA_MMAP_PROT syscall_arg__scnprintf_mmap_prot So can you please provide an example that shows before/after your patch? - Arnaldo > Signed-off-by: Changbin Du <changbin.du@huawei.com> > --- > tools/perf/builtin-trace.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c > index e5fef39c34bf..a8407eee58a3 100644 > --- a/tools/perf/builtin-trace.c > +++ b/tools/perf/builtin-trace.c > @@ -2099,9 +2099,9 @@ static size_t syscall__scnprintf_args(struct syscall *sc, char *bf, size_t size, > !trace->show_zeros && > !(sc->arg_fmt && > (sc->arg_fmt[arg.idx].show_zero || > - sc->arg_fmt[arg.idx].scnprintf == SCA_STRARRAY || > - sc->arg_fmt[arg.idx].scnprintf == SCA_STRARRAYS) && > - sc->arg_fmt[arg.idx].parm)) > + ((sc->arg_fmt[arg.idx].scnprintf == SCA_STRARRAY || > + sc->arg_fmt[arg.idx].scnprintf == SCA_STRARRAYS) && > + sc->arg_fmt[arg.idx].parm)))) > continue; > > printed += scnprintf(bf + printed, size - printed, "%s", printed ? ", " : ""); > @@ -2803,8 +2803,8 @@ static size_t trace__fprintf_tp_fields(struct trace *trace, struct evsel *evsel, > */ > if (val == 0 && > !trace->show_zeros && > - !((arg->show_zero || > - arg->scnprintf == SCA_STRARRAY || > + !arg->show_zero && > + !((arg->scnprintf == SCA_STRARRAY || > arg->scnprintf == SCA_STRARRAYS) && > arg->parm)) > continue; > -- > 2.34.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] perf trace beauty: Always show param if show_zero is set 2024-04-23 20:37 ` Arnaldo Carvalho de Melo @ 2024-04-24 2:43 ` duchangbin 0 siblings, 0 replies; 5+ messages in thread From: duchangbin @ 2024-04-24 2:43 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: duchangbin, Peter Zijlstra, Ingo Molnar, Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter, linux-perf-users, linux-kernel On Tue, Apr 23, 2024 at 05:37:38PM -0300, Arnaldo Carvalho de Melo wrote: > On Tue, Apr 23, 2024 at 09:53:29AM +0800, Changbin Du wrote: > > For some parameters, it is best to also display them when they are 0, > > e.g. flags. > > Please provide examples of what you're changing, to understand your > change one has to know what are strarrays and what they handle in 'perf > trace', i.e. if the value is zero but the argument has a string array > associated, it probably will translate zero into some string. > How about only check the show_zero property? The field itself knows exactly what it wants. if (val == 0 && !trace->show_zeros && !(sc->arg_fmt && sc->arg_fmt[arg.idx].show_zero)) continue; > So I did: > > root@x1:~# perf trace -e syscalls:sys_enter_mmap --filter prot==0 > 0.000 gnome-shell/2293 syscalls:sys_enter_mmap(addr: 0x10afec7e1000, len: 65536, flags: PRIVATE|FIXED|ANONYMOUS) > ^Croot@x1:~# > > And this is _before_ this patch, after this patch we get: > > > root@x1:~# perf trace -e syscalls:sys_enter_mmap --filter prot==0 > 0.000 Isolated Web C/25530 syscalls:sys_enter_mmap(addr: 0x7f27df529000, len: 4096, flags: PRIVATE|FIXED|ANONYMOUS) > 40.267 DOM Worker/1105511 syscalls:sys_enter_mmap(addr: 0x1c9faec48000, len: 65536, flags: PRIVATE|FIXED|ANONYMOUS) > 270.145 firefox/3447 syscalls:sys_enter_mmap(addr: 0x7fa0ed343000, len: 4096, flags: PRIVATE|FIXED|ANONYMOUS) > 2194.686 firefox/3447 syscalls:sys_enter_mmap(addr: 0x7fa0ed39f000, len: 4096, flags: PRIVATE|FIXED|ANONYMOUS) > 2461.709 Isolated Web C/21794 syscalls:sys_enter_mmap(addr: 0x7fdc3e100000, len: 1048576, flags: PRIVATE|FIXED|ANONYMOUS) > 4476.053 firefox/3447 syscalls:sys_enter_mmap(addr: 0x7fa0ed3a1000, len: 4096, flags: PRIVATE|FIXED|ANONYMOUS) > ^Croot@x1:~# > It doesn't seem to be effective on your perf. Here is my example. Before: PROT_NONE is not shown. $ sudo perf trace -e syscalls:sys_enter_mmap --filter prot==0 -- ls 0.000 ls/2979231 syscalls:sys_enter_mmap(len: 4220888, flags: PRIVATE|ANONYMOUS) After: PROT_NONE is displayed. $ sudo perf trace -e syscalls:sys_enter_mmap --filter prot==0 -- ls 0.000 ls/2975708 syscalls:sys_enter_mmap(len: 4220888, prot: NONE, flags: PRIVATE|ANONYMOUS) > Because 'mmap's 'prot' is not set directly as an strarray, see: > > { .name = "mmap", .hexret = true, > /* The standard mmap maps to old_mmap on s390x */ > #if defined(__s390x__) > .alias = "old_mmap", > #endif > .arg = { [2] = { .scnprintf = SCA_MMAP_PROT, /* prot */ }, > [3] = { .scnprintf = SCA_MMAP_FLAGS, /* flags */ > .strtoul = STUL_STRARRAY_FLAGS, > .parm = &strarray__mmap_flags, }, > [5] = { .scnprintf = SCA_HEX, /* offset */ }, }, }, > > static size_t syscall_arg__scnprintf_mmap_prot(char *bf, size_t size, struct syscall_arg *arg) > { > unsigned long prot = arg->val; > > if (prot == 0) > return scnprintf(bf, size, "%sNONE", arg->show_string_prefix ? strarray__mmap_prot.prefix : ""); > > return mmap__scnprintf_prot(prot, bf, size, arg->show_string_prefix); > } > > #define SCA_MMAP_PROT syscall_arg__scnprintf_mmap_prot > > So can you please provide an example that shows before/after your patch? > > - Arnaldo > > > Signed-off-by: Changbin Du <changbin.du@huawei.com> > > --- > > tools/perf/builtin-trace.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c > > index e5fef39c34bf..a8407eee58a3 100644 > > --- a/tools/perf/builtin-trace.c > > +++ b/tools/perf/builtin-trace.c > > @@ -2099,9 +2099,9 @@ static size_t syscall__scnprintf_args(struct syscall *sc, char *bf, size_t size, > > !trace->show_zeros && > > !(sc->arg_fmt && > > (sc->arg_fmt[arg.idx].show_zero || > > - sc->arg_fmt[arg.idx].scnprintf == SCA_STRARRAY || > > - sc->arg_fmt[arg.idx].scnprintf == SCA_STRARRAYS) && > > - sc->arg_fmt[arg.idx].parm)) > > + ((sc->arg_fmt[arg.idx].scnprintf == SCA_STRARRAY || > > + sc->arg_fmt[arg.idx].scnprintf == SCA_STRARRAYS) && > > + sc->arg_fmt[arg.idx].parm)))) > > continue; > > > > printed += scnprintf(bf + printed, size - printed, "%s", printed ? ", " : ""); > > @@ -2803,8 +2803,8 @@ static size_t trace__fprintf_tp_fields(struct trace *trace, struct evsel *evsel, > > */ > > if (val == 0 && > > !trace->show_zeros && > > - !((arg->show_zero || > > - arg->scnprintf == SCA_STRARRAY || > > + !arg->show_zero && > > + !((arg->scnprintf == SCA_STRARRAY || > > arg->scnprintf == SCA_STRARRAYS) && > > arg->parm)) > > continue; > > -- > > 2.34.1 -- Cheers, Changbin Du ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] perf trace beauty: Always show mmap prot even though PROT_NONE 2024-04-23 1:53 [PATCH v2 0/2] Always show mmap prot even though PROT_NONE Changbin Du 2024-04-23 1:53 ` [PATCH v2 1/2] perf trace beauty: Always show param if show_zero is set Changbin Du @ 2024-04-23 1:53 ` Changbin Du 1 sibling, 0 replies; 5+ messages in thread From: Changbin Du @ 2024-04-23 1:53 UTC (permalink / raw) To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim Cc: Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter, linux-perf-users, linux-kernel, Changbin Du PROT_NONE is also useful information, so do not omit the mmap prot even though it is 0. syscall_arg__scnprintf_mmap_prot() could print PROT_NONE for prot 0. Signed-off-by: Changbin Du <changbin.du@huawei.com> --- tools/perf/builtin-trace.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c index a8407eee58a3..e82ed002890f 100644 --- a/tools/perf/builtin-trace.c +++ b/tools/perf/builtin-trace.c @@ -1033,7 +1033,7 @@ static const struct syscall_fmt syscall_fmts[] = { #if defined(__s390x__) .alias = "old_mmap", #endif - .arg = { [2] = { .scnprintf = SCA_MMAP_PROT, /* prot */ }, + .arg = { [2] = { .scnprintf = SCA_MMAP_PROT, .show_zero = true, /* prot */ }, [3] = { .scnprintf = SCA_MMAP_FLAGS, /* flags */ .strtoul = STUL_STRARRAY_FLAGS, .parm = &strarray__mmap_flags, }, @@ -1050,7 +1050,7 @@ static const struct syscall_fmt syscall_fmts[] = { [4] = { .scnprintf = SCA_MOVE_MOUNT_FLAGS, /* flags */ }, }, }, { .name = "mprotect", .arg = { [0] = { .scnprintf = SCA_HEX, /* start */ }, - [2] = { .scnprintf = SCA_MMAP_PROT, /* prot */ }, }, }, + [2] = { .scnprintf = SCA_MMAP_PROT, .show_zero = true, /* prot */ }, }, }, { .name = "mq_unlink", .arg = { [0] = { .scnprintf = SCA_FILENAME, /* u_name */ }, }, }, { .name = "mremap", .hexret = true, @@ -1084,7 +1084,7 @@ static const struct syscall_fmt syscall_fmts[] = { .arg = { [0] = { .scnprintf = SCA_INT, /* key */ }, }, }, { .name = "pkey_mprotect", .arg = { [0] = { .scnprintf = SCA_HEX, /* start */ }, - [2] = { .scnprintf = SCA_MMAP_PROT, /* prot */ }, + [2] = { .scnprintf = SCA_MMAP_PROT, .show_zero = true, /* prot */ }, [3] = { .scnprintf = SCA_INT, /* pkey */ }, }, }, { .name = "poll", .timeout = true, }, { .name = "ppoll", .timeout = true, }, -- 2.34.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-04-24 2:43 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-04-23 1:53 [PATCH v2 0/2] Always show mmap prot even though PROT_NONE Changbin Du 2024-04-23 1:53 ` [PATCH v2 1/2] perf trace beauty: Always show param if show_zero is set Changbin Du 2024-04-23 20:37 ` Arnaldo Carvalho de Melo 2024-04-24 2:43 ` duchangbin 2024-04-23 1:53 ` [PATCH v2 2/2] perf trace beauty: Always show mmap prot even though PROT_NONE Changbin Du
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).