From: duchangbin <changbin.du@huawei.com>
To: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: duchangbin <changbin.du@huawei.com>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Namhyung Kim <namhyung@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
"Alexander Shishkin" <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@kernel.org>, Ian Rogers <irogers@google.com>,
Adrian Hunter <adrian.hunter@intel.com>,
"linux-perf-users@vger.kernel.org"
<linux-perf-users@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 1/2] perf trace beauty: Always show param if show_zero is set
Date: Wed, 24 Apr 2024 02:43:25 +0000 [thread overview]
Message-ID: <1b548158ce844e239dbc1e42ceeb8d0a@huawei.com> (raw)
In-Reply-To: <ZigcEg1inS1JoSr5@x1>
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
next prev parent reply other threads:[~2024-04-24 2:43 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2024-04-23 1:53 ` [PATCH v2 2/2] perf trace beauty: Always show mmap prot even though PROT_NONE Changbin Du
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1b548158ce844e239dbc1e42ceeb8d0a@huawei.com \
--to=changbin.du@huawei.com \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=irogers@google.com \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).