linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Michael Petlan <mpetlan@redhat.com>
Cc: Arnaldo de Melo <acme@redhat.com>,
	linux-perf-users@vger.kernel.org, Jiri Olsa <jolsa@redhat.com>,
	Thomas-Mich Richter <tmricht@linux.vnet.ibm.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] perf test shell: Fix check open filename arg using 'perf trace
Date: Tue, 12 Dec 2017 11:57:07 -0300	[thread overview]
Message-ID: <20171212145707.GL3958@kernel.org> (raw)
In-Reply-To: <alpine.LRH.2.20.1712120133380.9416@Diego>

Em Tue, Dec 12, 2017 at 04:05:23AM +0100, Michael Petlan escreveu:
> On Mon, 11 Dec 2017, Arnaldo Carvalho de Melo wrote:
> > It is not working here:
> 
> Hmm, I think I got it.
> 
> The following construction:
> 
> evts=`perf list syscalls:sys_enter_open* |& egrep 'open(at)? ' | sed -r 's/.*sys_enter_(.*) +\[.*/-e \1/'`
> 
> ... expands to:
> 
> "-e open -e openat"
> 
> ... which is a format that perf-trace does not seem to accept.
> It simply overrides the first event with the second, thus it
> only traces "openat". It worked for me, since I tested it on
> aarch64 where $evts expanded to only one "-e" parameter.

Right, in this case 'perf trace -e' doesn't behave like -e in other
tools, which is unfortunate and should be fixed at some point.

> Attaching a patch for perf-trace that should address it. It
> seems to work, so perf-trace accepts "-e open -e openat".
> However, one weak point is there:
> 
> "-e open,openat -e stat" works, while
> "-e open -e openat,stat" does not.
> 
> In case you don't like the patch for perf-trace, the test's
> patch would need to expand events to something else (which is
> of course possible).
> 
> Also, on my system (x86_64, 4.15.0-rc1), I needed another
> patch for the test (perf_test_shell_fix_filename_arg.patch),
> because the variable name changed...

the point here is not to add any new patch for perf trace, etc, but find
a way to fix just this test, so I think this works:

# evts=$(echo $(perf list syscalls:sys_enter_open* |& egrep 'open(at)? ' | sed -r 's/.*sys_enter_([a-z]+) +\[.*$/\1/') | sed 's/ /,/')
# echo $evts
open,openat
[root@jouet ~]#

I'll work on a 'perf list -x,'  that will print all events matching as a
CSV with the informed delimiter, seems handy :-)

So the patch below does the trick for me, can you please check if does
for you?

With regards to the other patches, please consider submitting them in
separate messages, stating their purpose in a commit log, with example
usage, etc. Then people will be able to review it on its own.

Running it here I get:

[root@jouet perf]# sh -x tools/perf/tests/shell/trace+probe_vfs_getname.sh 
++ dirname tools/perf/tests/shell/trace+probe_vfs_getname.sh
+ . tools/perf/tests/shell/lib/probe.sh
+ skip_if_no_perf_probe
+ perf probe
+ grep -q 'is not a perf-command'
+ return 0
++ dirname tools/perf/tests/shell/trace+probe_vfs_getname.sh
+ . tools/perf/tests/shell/lib/probe_vfs_getname.sh
++ perf probe -l
++ grep -q probe:vfs_getname
++ had_vfs_getname=1
++ mktemp /tmp/temporary_file.XXXXX
+ file=/tmp/temporary_file.pSzKC
+ add_probe_vfs_getname
+ local verbose=
+ '[' 1 -eq 1 ']'
++ perf probe -L getname_flags
++ egrep 'result.*=.*filename;'
++ sed -r 's/[[:space:]]+([[:digit:]]+)[[:space:]]+result->uptr.*/\1/'
+ line=72
+ perf probe 'vfs_getname=getname_flags:72 pathname=result->name:string'
Added new event:
  probe:vfs_getname    (on getname_flags:72 with pathname=result->name:string)

You can now use it in all perf tools, such as:

	perf record -e probe:vfs_getname -aR sleep 1

+ err=0
+ '[' 0 -ne 0 ']'
+ trace_open_vfs_getname
++ uname -m
+ test x86_64 = s390x
++ sed 's/ /,/'
+++ egrep 'open(at)? '
+++ perf list 'syscalls:sys_enter_open*'
+++ sed -r 's/.*sys_enter_([a-z]+) +\[.*$/\1/'
++ echo open openat
+ svc=open,openat
+ perf trace -e open,openat touch /tmp/temporary_file.pSzKC
+ egrep ' +[0-9]+\.[0-9]+ +\( +[0-9]+\.[0-9]+ ms\): +touch\/[0-9]+ open(at)?\(filename: +/tmp/temporary_file.pSzKC, +flags: CREAT\|NOCTTY\|NONBLOCK\|WRONLY, +mode: +IRUGO\|IWUGO\) += +[0-9]+$'
     1.479 ( 0.073 ms): touch/27884 open(filename: /tmp/temporary_file.pSzKC, flags: CREAT|NOCTTY|NONBLOCK|WRONLY, mode: IRUGO|IWUGO) = 3
+ err=0
+ rm -f /tmp/temporary_file.pSzKC
+ cleanup_probe_vfs_getname
+ '[' 1 -eq 1 ']'
+ perf probe -q -d probe:vfs_getname
+ exit 0
[root@jouet perf]#


- Arnaldo

diff --git a/tools/perf/tests/shell/trace+probe_vfs_getname.sh b/tools/perf/tests/shell/trace+probe_vfs_getname.sh
index 2a9ef080efd0..d22f08568226 100755
--- a/tools/perf/tests/shell/trace+probe_vfs_getname.sh
+++ b/tools/perf/tests/shell/trace+probe_vfs_getname.sh
@@ -17,10 +17,10 @@ skip_if_no_perf_probe || exit 2
 file=$(mktemp /tmp/temporary_file.XXXXX)
 
 trace_open_vfs_getname() {
-	test "$(uname -m)" = s390x && { svc="openat"; txt="dfd: +CWD, +"; }
-
-	perf trace -e ${svc:-open} touch $file 2>&1 | \
-	egrep " +[0-9]+\.[0-9]+ +\( +[0-9]+\.[0-9]+ ms\): +touch\/[0-9]+ ${svc:-open}\(${txt}filename: +${file}, +flags: CREAT\|NOCTTY\|NONBLOCK\|WRONLY, +mode: +IRUGO\|IWUGO\) += +[0-9]+$"
+	test "$(uname -m)" = s390x && { txt="dfd: +CWD, +"; }
+	svc=$(echo $(perf list syscalls:sys_enter_open* |& egrep 'open(at)? ' | sed -r 's/.*sys_enter_([a-z]+) +\[.*$/\1/') | sed 's/ /,/')
+	perf trace -e ${svc} touch $file 2>&1 | \
+	egrep " +[0-9]+\.[0-9]+ +\( +[0-9]+\.[0-9]+ ms\): +touch\/[0-9]+ open(at)?\(${txt}filename: +${file}, +flags: CREAT\|NOCTTY\|NONBLOCK\|WRONLY, +mode: +IRUGO\|IWUGO\) += +[0-9]+$"
 }
 
 

  reply	other threads:[~2017-12-12 14:57 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-08 17:48 [PATCH v2] perf test shell: Fix check open filename arg using 'perf trace Michael Petlan
2017-12-11 15:43 ` Arnaldo de Melo
2017-12-11 17:07   ` Arnaldo Carvalho de Melo
2017-12-12  3:05     ` Michael Petlan
2017-12-12 14:57       ` Arnaldo Carvalho de Melo [this message]
2017-12-12 15:19         ` Arnaldo Carvalho de Melo
2017-12-12 16:24           ` Michael Petlan
2017-12-12 17:12             ` Arnaldo Carvalho de Melo
2017-12-14 12:08               ` Thomas-Mich Richter
2017-12-14 14:03                 ` Arnaldo Carvalho de Melo
2017-12-12 16:30         ` Michael Petlan

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=20171212145707.GL3958@kernel.org \
    --to=acme@kernel.org \
    --cc=acme@redhat.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mpetlan@redhat.com \
    --cc=tmricht@linux.vnet.ibm.com \
    /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 NNTP newsgroup(s).