All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf test shell: Fix check open filename arg using 'perf trace'
@ 2017-11-29 17:27 Michael Petlan
  2017-11-30 10:28 ` Thomas-Mich Richter
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Petlan @ 2017-11-29 17:27 UTC (permalink / raw)
  To: linux-perf-users; +Cc: Jiri Olsa, Arnaldo de Melo

[-- Attachment #1: Type: text/plain, Size: 1382 bytes --]

Hi Arnaldo, Jiri and others!

Posting a fix for perf test "Check open filename arg using perf trace + vfs_getname".

The commit f231af789b11a2f1a3795acc3228a3e178a80c21 adds an exception for s390x to
use openat() syscall instead of open(). This exception is not s390x-only, thus I
adjusted the test to accept both open and openat syscalls, no matter which arch it
runs on. Does it sound reasonable to you?


When testing on 4.15.0-rc1, I also hit the following issue:

# perf probe "vfs_getname=getname_flags:72 pathname=result->name:string"
Failed to find 'result' in this function.
  Error: Failed to add events.

# perf probe -L getname_flags 
[...]
     72         result->uptr = filename;
     73         result->aname = NULL;
[...]

# perf probe "vfs_getname=getname_flags:72 pathname=result->uptr:string"
Failed to find 'result' in this function.
  Error: Failed to add events.

... When the probed var is changed to "filename", it seems to work:

# perf probe "vfs_getname=getname_flags:72 pathname=filename:string"
Added new event:
  probe:vfs_getname    (on getname_flags:72 with pathname=filename:string)

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

	perf record -e probe:vfs_getname -aR sleep 1


So maybe the second attached patch is necessary too, not sure. Just thinking
that "filename" might be less change-prone, as a func. arg... ?

Thank you.

Cheers,
Michael

[-- Attachment #2: Type: text/plain, Size: 1675 bytes --]

commit 526f2201254f8da750e722b893af19b7d4f2640d
Author: Michael Petlan <mpetlan@redhat.com>
Date:   Tue Nov 28 17:47:49 2017 +0100

    perf test shell: Fix check open filename arg using 'perf trace'
    
    The following commit added an exception for s390x to use openat()
    instead of open() in the test:
    
    commit f231af789b11a2f1a3795acc3228a3e178a80c21
    Author: Thomas Richter <tmricht@linux.vnet.ibm.com>
    Date:   Tue Nov 14 08:18:46 2017 +0100
    
    Since the problem is not s390x-specific, this patch makes it more
    generic, so the test handles both open() and openat() no matter
    which architecture it is running on.
    
    Signed-off-by: Michael Petlan <mpetlan@redhat.com>

diff --git a/tools/perf/tests/shell/trace+probe_vfs_getname.sh b/tools/perf/tests/shell/trace+probe_vfs_getname.sh
index 2a9ef08..33212da 100755
--- a/tools/perf/tests/shell/trace+probe_vfs_getname.sh
+++ b/tools/perf/tests/shell/trace+probe_vfs_getname.sh
@@ -17,10 +17,8 @@ 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]+$"
+	perf trace -e 'open*' touch $file 2>&1 | \
+	egrep " +[0-9]+\.[0-9]+ +\( +[0-9]+\.[0-9]+ ms\): +touch\/[0-9]+ open(at)?\((dfd: +CWD, +)?filename: +${file}, +flags: CREAT\|NOCTTY\|NONBLOCK\|WRONLY, +mode: +IRUGO\|IWUGO\) += +[0-9]+$"
 }
 
 

[-- Attachment #3: Type: text/plain, Size: 691 bytes --]

diff --git a/tools/perf/tests/shell/lib/probe_vfs_getname.sh b/tools/perf/tests/shell/lib/probe_vfs_getname.sh
index 30a950c..3759582 100644
--- a/tools/perf/tests/shell/lib/probe_vfs_getname.sh
+++ b/tools/perf/tests/shell/lib/probe_vfs_getname.sh
@@ -13,7 +13,7 @@ add_probe_vfs_getname() {
 	local verbose=$1
 	if [ $had_vfs_getname -eq 1 ] ; then
 		line=$(perf probe -L getname_flags 2>&1 | egrep 'result.*=.*filename;' | sed -r 's/[[:space:]]+([[:digit:]]+)[[:space:]]+result->uptr.*/\1/')
-		perf probe $verbose "vfs_getname=getname_flags:${line} pathname=result->name:string"
+		perf probe $verbose "vfs_getname=getname_flags:${line} pathname=filename:string"
 	fi
 }
 

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] perf test shell: Fix check open filename arg using 'perf trace'
  2017-11-29 17:27 [PATCH] perf test shell: Fix check open filename arg using 'perf trace' Michael Petlan
@ 2017-11-30 10:28 ` Thomas-Mich Richter
  2017-11-30 15:56   ` Arnaldo de Melo
  0 siblings, 1 reply; 12+ messages in thread
From: Thomas-Mich Richter @ 2017-11-30 10:28 UTC (permalink / raw)
  To: Michael Petlan, linux-perf-users; +Cc: Jiri Olsa, Arnaldo de Melo

On 11/29/2017 06:27 PM, Michael Petlan wrote:
> Hi Arnaldo, Jiri and others!
> 
> Posting a fix for perf test "Check open filename arg using perf trace + vfs_getname".
> 
> The commit f231af789b11a2f1a3795acc3228a3e178a80c21 adds an exception for s390x to
> use openat() syscall instead of open(). This exception is not s390x-only, thus I
> adjusted the test to accept both open and openat syscalls, no matter which arch it
> runs on. Does it sound reasonable to you?
> 
> 
> When testing on 4.15.0-rc1, I also hit the following issue:
> 
> # perf probe "vfs_getname=getname_flags:72 pathname=result->name:string"
> Failed to find 'result' in this function.
>   Error: Failed to add events.
> 
> # perf probe -L getname_flags 
> [...]
>      72         result->uptr = filename;
>      73         result->aname = NULL;
> [...]
> 
> # perf probe "vfs_getname=getname_flags:72 pathname=result->uptr:string"
> Failed to find 'result' in this function.
>   Error: Failed to add events.
> 
> ... When the probed var is changed to "filename", it seems to work:
> 
> # perf probe "vfs_getname=getname_flags:72 pathname=filename:string"
> Added new event:
>   probe:vfs_getname    (on getname_flags:72 with pathname=filename:string)
> 
> You can now use it in all perf tools, such as:
> 
> 	perf record -e probe:vfs_getname -aR sleep 1
> 
> 
> So maybe the second attached patch is necessary too, not sure. Just thinking
> that "filename" might be less change-prone, as a func. arg... ?
> 
> Thank you.
> 
> Cheers,
> Michael
> 

Maybe I have done something wrong when I tried your patch. I have download
latest 4.15.0rc1 but your patch does not work for me.
My perf tool does not handle wild cards on events:

root@s35lp76 shell]# /root/linux/tools/perf/perf  trace -e 'open' touch /tmp/xxx
     0.000 ( 0.013 ms): touch/28615 open(filename: 0xbc990692, flags: CLOEXEC                             ) = 3
[root@s35lp76 shell]# /root/linux/tools/perf/perf  trace -e 'openat' touch /tmp/xxx
     0.000 ( 0.011 ms): touch/28617 openat(dfd: CWD, filename: 0xa7324328, flags: CLOEXEC                 ) = 3
     0.037 ( 0.009 ms): touch/28617 openat(dfd: CWD, filename: 0xa737de60, flags: CLOEXEC                 ) = 3
     0.454 ( 0.015 ms): touch/28617 openat(dfd: CWD, filename: 0xeebff57c, flags: CREAT|NOCTTY|NONBLOCK|WRONLY, mode: IRUGO|IWUGO) = 3
[root@s35lp76 shell]# /root/linux/tools/perf/perf  trace -e 'open*' touch /tmp/xxx
event syntax error: 'open*'
                     \___ Cannot find PMU `open*'. Missing kernel support?
Run 'perf list' for a list of valid events

 Usage: perf trace [<options>] [<command>]
    or: perf trace [<options>] -- <command> [<options>]
    or: perf trace record [<options>] [<command>]
    or: perf trace record [<options>] -- <command> [<options>]

    -e, --event <event>   event/syscall selector. use 'perf list' to list available events
[root@s35lp76 shell]# 

Is this a configuration error on my side?
-- 
Thomas Richter, Dept 3303, IBM LTC Boeblingen Germany
--
Vorsitzende des Aufsichtsrats: Martina Koederitz 
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] perf test shell: Fix check open filename arg using 'perf trace'
  2017-11-30 10:28 ` Thomas-Mich Richter
@ 2017-11-30 15:56   ` Arnaldo de Melo
  2017-11-30 15:59     ` Arnaldo de Melo
  2017-12-01  2:33     ` Namhyung Kim
  0 siblings, 2 replies; 12+ messages in thread
From: Arnaldo de Melo @ 2017-11-30 15:56 UTC (permalink / raw)
  To: Thomas-Mich Richter; +Cc: Michael Petlan, linux-perf-users, Jiri Olsa

Em Thu, Nov 30, 2017 at 11:28:33AM +0100, Thomas-Mich Richter escreveu:
> On 11/29/2017 06:27 PM, Michael Petlan wrote:
> > Hi Arnaldo, Jiri and others!
> > 
> > Posting a fix for perf test "Check open filename arg using perf trace + vfs_getname".
> > 
> > The commit f231af789b11a2f1a3795acc3228a3e178a80c21 adds an exception for s390x to
> > use openat() syscall instead of open(). This exception is not s390x-only, thus I
> > adjusted the test to accept both open and openat syscalls, no matter which arch it
> > runs on. Does it sound reasonable to you?
> > 
> > 
> > When testing on 4.15.0-rc1, I also hit the following issue:
> > 
> > # perf probe "vfs_getname=getname_flags:72 pathname=result->name:string"
> > Failed to find 'result' in this function.
> >   Error: Failed to add events.
> > 
> > # perf probe -L getname_flags 
> > [...]
> >      72         result->uptr = filename;
> >      73         result->aname = NULL;
> > [...]
> > 
> > # perf probe "vfs_getname=getname_flags:72 pathname=result->uptr:string"
> > Failed to find 'result' in this function.
> >   Error: Failed to add events.
> > 
> > ... When the probed var is changed to "filename", it seems to work:
> > 
> > # perf probe "vfs_getname=getname_flags:72 pathname=filename:string"
> > Added new event:
> >   probe:vfs_getname    (on getname_flags:72 with pathname=filename:string)
> > 
> > You can now use it in all perf tools, such as:
> > 
> > 	perf record -e probe:vfs_getname -aR sleep 1
> > 
> > 
> > So maybe the second attached patch is necessary too, not sure. Just thinking
> > that "filename" might be less change-prone, as a func. arg... ?
> > 
> > Thank you.
> > 
> > Cheers,
> > Michael
> > 
> 
> Maybe I have done something wrong when I tried your patch. I have download
> latest 4.15.0rc1 but your patch does not work for me.
> My perf tool does not handle wild cards on events:
> 
> root@s35lp76 shell]# /root/linux/tools/perf/perf  trace -e 'open' touch /tmp/xxx
>      0.000 ( 0.013 ms): touch/28615 open(filename: 0xbc990692, flags: CLOEXEC                             ) = 3
> [root@s35lp76 shell]# /root/linux/tools/perf/perf  trace -e 'openat' touch /tmp/xxx
>      0.000 ( 0.011 ms): touch/28617 openat(dfd: CWD, filename: 0xa7324328, flags: CLOEXEC                 ) = 3
>      0.037 ( 0.009 ms): touch/28617 openat(dfd: CWD, filename: 0xa737de60, flags: CLOEXEC                 ) = 3
>      0.454 ( 0.015 ms): touch/28617 openat(dfd: CWD, filename: 0xeebff57c, flags: CREAT|NOCTTY|NONBLOCK|WRONLY, mode: IRUGO|IWUGO) = 3
> [root@s35lp76 shell]# /root/linux/tools/perf/perf  trace -e 'open*' touch /tmp/xxx
> event syntax error: 'open*'
>                      \___ Cannot find PMU `open*'. Missing kernel support?
> Run 'perf list' for a list of valid events
> 
>  Usage: perf trace [<options>] [<command>]
>     or: perf trace [<options>] -- <command> [<options>]
>     or: perf trace record [<options>] [<command>]
>     or: perf trace record [<options>] -- <command> [<options>]
> 
>     -e, --event <event>   event/syscall selector. use 'perf list' to list available events
> [root@s35lp76 shell]# 
> 
> Is this a configuration error on my side?

[acme@jouet linux]$ git log -1 27702bcfe8a125a1feeeb5f07526d63b20cac47f --oneline
27702bcfe8a1 perf trace: Support syscall name globbing
[acme@jouet linux]$

Is in v4.14 final.

Testing with/without those quotes:

[root@jouet ~]# perf trace -e open* touch /etc/passwd
     0.000 ( 0.007 ms): touch/17246 open(filename: 0x99747e37, flags: CLOEXEC                             ) = 3
     0.022 ( 0.004 ms): touch/17246 open(filename: 0x9994b640, flags: CLOEXEC                             ) = 3
     0.189 ( 0.004 ms): touch/17246 open(filename: 0x994f1c70, flags: CLOEXEC                             ) = 3
     0.224 ( 0.056 ms): touch/17246 open(filename: 0xaa80a32a, flags: CREAT|NOCTTY|NONBLOCK|WRONLY, mode: IRUGO|IWUGO) = 3
[root@jouet ~]# perf trace -e 'open*' touch /etc/passwd
     0.000 ( 0.039 ms): touch/17250 open(filename: 0xf1445e37, flags: CLOEXEC                             ) = 3
     0.107 ( 0.030 ms): touch/17250 open(filename: 0xf1649640, flags: CLOEXEC                             ) = 3
     0.976 ( 0.009 ms): touch/17250 open(filename: 0xf11efc70, flags: CLOEXEC                             ) = 3
     1.032 ( 0.008 ms): touch/17250 open(filename: 0x9357432a, flags: CREAT|NOCTTY|NONBLOCK|WRONLY, mode: IRUGO|IWUGO) = 3
[root@jouet ~]#

What differs from x86 to others is that x86 uses syscalltbl, not requiring
audit-libs to map syscall numbers to names, so perhaps it is something in there...

- Arnaldo

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] perf test shell: Fix check open filename arg using 'perf trace'
  2017-11-30 15:56   ` Arnaldo de Melo
@ 2017-11-30 15:59     ` Arnaldo de Melo
  2017-12-05 15:39       ` Hendrik Brueckner
  2017-12-01  2:33     ` Namhyung Kim
  1 sibling, 1 reply; 12+ messages in thread
From: Arnaldo de Melo @ 2017-11-30 15:59 UTC (permalink / raw)
  To: Thomas-Mich Richter; +Cc: Michael Petlan, linux-perf-users, Jiri Olsa

Em Thu, Nov 30, 2017 at 01:56:42PM -0200, Arnaldo de Melo escreveu:
> Em Thu, Nov 30, 2017 at 11:28:33AM +0100, Thomas-Mich Richter escreveu:
> > On 11/29/2017 06:27 PM, Michael Petlan wrote:
> > > Hi Arnaldo, Jiri and others!
> > > 
> > > Posting a fix for perf test "Check open filename arg using perf trace + vfs_getname".
> > > 
> > > The commit f231af789b11a2f1a3795acc3228a3e178a80c21 adds an exception for s390x to
> > > use openat() syscall instead of open(). This exception is not s390x-only, thus I
> > > adjusted the test to accept both open and openat syscalls, no matter which arch it
> > > runs on. Does it sound reasonable to you?
> > > 
> > > 
> > > When testing on 4.15.0-rc1, I also hit the following issue:
> > > 
> > > # perf probe "vfs_getname=getname_flags:72 pathname=result->name:string"
> > > Failed to find 'result' in this function.
> > >   Error: Failed to add events.
> > > 
> > > # perf probe -L getname_flags 
> > > [...]
> > >      72         result->uptr = filename;
> > >      73         result->aname = NULL;
> > > [...]
> > > 
> > > # perf probe "vfs_getname=getname_flags:72 pathname=result->uptr:string"
> > > Failed to find 'result' in this function.
> > >   Error: Failed to add events.
> > > 
> > > ... When the probed var is changed to "filename", it seems to work:
> > > 
> > > # perf probe "vfs_getname=getname_flags:72 pathname=filename:string"
> > > Added new event:
> > >   probe:vfs_getname    (on getname_flags:72 with pathname=filename:string)
> > > 
> > > You can now use it in all perf tools, such as:
> > > 
> > > 	perf record -e probe:vfs_getname -aR sleep 1
> > > 
> > > 
> > > So maybe the second attached patch is necessary too, not sure. Just thinking
> > > that "filename" might be less change-prone, as a func. arg... ?
> > > 
> > > Thank you.
> > > 
> > > Cheers,
> > > Michael
> > > 
> > 
> > Maybe I have done something wrong when I tried your patch. I have download
> > latest 4.15.0rc1 but your patch does not work for me.
> > My perf tool does not handle wild cards on events:
> > 
> > root@s35lp76 shell]# /root/linux/tools/perf/perf  trace -e 'open' touch /tmp/xxx
> >      0.000 ( 0.013 ms): touch/28615 open(filename: 0xbc990692, flags: CLOEXEC                             ) = 3
> > [root@s35lp76 shell]# /root/linux/tools/perf/perf  trace -e 'openat' touch /tmp/xxx
> >      0.000 ( 0.011 ms): touch/28617 openat(dfd: CWD, filename: 0xa7324328, flags: CLOEXEC                 ) = 3
> >      0.037 ( 0.009 ms): touch/28617 openat(dfd: CWD, filename: 0xa737de60, flags: CLOEXEC                 ) = 3
> >      0.454 ( 0.015 ms): touch/28617 openat(dfd: CWD, filename: 0xeebff57c, flags: CREAT|NOCTTY|NONBLOCK|WRONLY, mode: IRUGO|IWUGO) = 3
> > [root@s35lp76 shell]# /root/linux/tools/perf/perf  trace -e 'open*' touch /tmp/xxx
> > event syntax error: 'open*'
> >                      \___ Cannot find PMU `open*'. Missing kernel support?
> > Run 'perf list' for a list of valid events
> > 
> >  Usage: perf trace [<options>] [<command>]
> >     or: perf trace [<options>] -- <command> [<options>]
> >     or: perf trace record [<options>] [<command>]
> >     or: perf trace record [<options>] -- <command> [<options>]
> > 
> >     -e, --event <event>   event/syscall selector. use 'perf list' to list available events
> > [root@s35lp76 shell]# 
> > 
> > Is this a configuration error on my side?
> 
> [acme@jouet linux]$ git log -1 27702bcfe8a125a1feeeb5f07526d63b20cac47f --oneline
> 27702bcfe8a1 perf trace: Support syscall name globbing
> [acme@jouet linux]$
> 
> Is in v4.14 final.
> 
> Testing with/without those quotes:
> 
> [root@jouet ~]# perf trace -e open* touch /etc/passwd
>      0.000 ( 0.007 ms): touch/17246 open(filename: 0x99747e37, flags: CLOEXEC                             ) = 3
>      0.022 ( 0.004 ms): touch/17246 open(filename: 0x9994b640, flags: CLOEXEC                             ) = 3
>      0.189 ( 0.004 ms): touch/17246 open(filename: 0x994f1c70, flags: CLOEXEC                             ) = 3
>      0.224 ( 0.056 ms): touch/17246 open(filename: 0xaa80a32a, flags: CREAT|NOCTTY|NONBLOCK|WRONLY, mode: IRUGO|IWUGO) = 3
> [root@jouet ~]# perf trace -e 'open*' touch /etc/passwd
>      0.000 ( 0.039 ms): touch/17250 open(filename: 0xf1445e37, flags: CLOEXEC                             ) = 3
>      0.107 ( 0.030 ms): touch/17250 open(filename: 0xf1649640, flags: CLOEXEC                             ) = 3
>      0.976 ( 0.009 ms): touch/17250 open(filename: 0xf11efc70, flags: CLOEXEC                             ) = 3
>      1.032 ( 0.008 ms): touch/17250 open(filename: 0x9357432a, flags: CREAT|NOCTTY|NONBLOCK|WRONLY, mode: IRUGO|IWUGO) = 3
> [root@jouet ~]#
> 
> What differs from x86 to others is that x86 uses syscalltbl, not requiring
> audit-libs to map syscall numbers to names, so perhaps it is something in there...

Yeah :-\

int syscalltbl__strglobmatch_next(struct syscalltbl *tbl __maybe_unused,
                                  const char *syscall_glob __maybe_unused, int *idx __maybe_unused)
{
        return -1;
}

int syscalltbl__strglobmatch_first(struct syscalltbl *tbl, const char
*syscall_glob, int *idx)
{
        return syscalltbl__strglobmatch_next(tbl, syscall_glob, idx);
}
#endif /* HAVE_SYSCALL_TABLE */

-----

So someone needs to implement syscalltbl__strglobmatch_next() for arches
that don't HAVE_SYSCALL_TABLE, which is basically at this stage !x86.

Shouldn't be that difficult, but remains to be done.

- Arnaldo

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] perf test shell: Fix check open filename arg using 'perf trace'
  2017-11-30 15:56   ` Arnaldo de Melo
  2017-11-30 15:59     ` Arnaldo de Melo
@ 2017-12-01  2:33     ` Namhyung Kim
  2017-12-01 15:16       ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 12+ messages in thread
From: Namhyung Kim @ 2017-12-01  2:33 UTC (permalink / raw)
  To: Arnaldo de Melo
  Cc: Thomas-Mich Richter, Michael Petlan, linux-perf-users, Jiri Olsa,
	kernel-team

Hello,

On Thu, Nov 30, 2017 at 01:56:42PM -0200, Arnaldo de Melo wrote:
> Em Thu, Nov 30, 2017 at 11:28:33AM +0100, Thomas-Mich Richter escreveu:
> > On 11/29/2017 06:27 PM, Michael Petlan wrote:
> > > Hi Arnaldo, Jiri and others!
> > > 
> > > Posting a fix for perf test "Check open filename arg using perf trace + vfs_getname".
> > > 
> > > The commit f231af789b11a2f1a3795acc3228a3e178a80c21 adds an exception for s390x to
> > > use openat() syscall instead of open(). This exception is not s390x-only, thus I
> > > adjusted the test to accept both open and openat syscalls, no matter which arch it
> > > runs on. Does it sound reasonable to you?

It seems glibc 2.26 changed the behavior:

  https://lwn.net/Articles/738694/


> > > 
> > > 
> > > When testing on 4.15.0-rc1, I also hit the following issue:
> > > 
> > > # perf probe "vfs_getname=getname_flags:72 pathname=result->name:string"
> > > Failed to find 'result' in this function.
> > >   Error: Failed to add events.
> > > 
> > > # perf probe -L getname_flags 
> > > [...]
> > >      72         result->uptr = filename;
> > >      73         result->aname = NULL;
> > > [...]
> > > 
> > > # perf probe "vfs_getname=getname_flags:72 pathname=result->uptr:string"
> > > Failed to find 'result' in this function.
> > >   Error: Failed to add events.
> > > 
> > > ... When the probed var is changed to "filename", it seems to work:
> > > 
> > > # perf probe "vfs_getname=getname_flags:72 pathname=filename:string"
> > > Added new event:
> > >   probe:vfs_getname    (on getname_flags:72 with pathname=filename:string)
> > > 
> > > You can now use it in all perf tools, such as:
> > > 
> > > 	perf record -e probe:vfs_getname -aR sleep 1
> > > 
> > > 
> > > So maybe the second attached patch is necessary too, not sure. Just thinking
> > > that "filename" might be less change-prone, as a func. arg... ?
> > > 
> > > Thank you.
> > > 
> > > Cheers,
> > > Michael
> > > 
> > 
> > Maybe I have done something wrong when I tried your patch. I have download
> > latest 4.15.0rc1 but your patch does not work for me.
> > My perf tool does not handle wild cards on events:
> > 
> > root@s35lp76 shell]# /root/linux/tools/perf/perf  trace -e 'open' touch /tmp/xxx
> >      0.000 ( 0.013 ms): touch/28615 open(filename: 0xbc990692, flags: CLOEXEC                             ) = 3
> > [root@s35lp76 shell]# /root/linux/tools/perf/perf  trace -e 'openat' touch /tmp/xxx
> >      0.000 ( 0.011 ms): touch/28617 openat(dfd: CWD, filename: 0xa7324328, flags: CLOEXEC                 ) = 3
> >      0.037 ( 0.009 ms): touch/28617 openat(dfd: CWD, filename: 0xa737de60, flags: CLOEXEC                 ) = 3
> >      0.454 ( 0.015 ms): touch/28617 openat(dfd: CWD, filename: 0xeebff57c, flags: CREAT|NOCTTY|NONBLOCK|WRONLY, mode: IRUGO|IWUGO) = 3
> > [root@s35lp76 shell]# /root/linux/tools/perf/perf  trace -e 'open*' touch /tmp/xxx
> > event syntax error: 'open*'
> >                      \___ Cannot find PMU `open*'. Missing kernel support?
> > Run 'perf list' for a list of valid events
> > 
> >  Usage: perf trace [<options>] [<command>]
> >     or: perf trace [<options>] -- <command> [<options>]
> >     or: perf trace record [<options>] [<command>]
> >     or: perf trace record [<options>] -- <command> [<options>]
> > 
> >     -e, --event <event>   event/syscall selector. use 'perf list' to list available events
> > [root@s35lp76 shell]# 
> > 
> > Is this a configuration error on my side?
> 
> [acme@jouet linux]$ git log -1 27702bcfe8a125a1feeeb5f07526d63b20cac47f --oneline
> 27702bcfe8a1 perf trace: Support syscall name globbing
> [acme@jouet linux]$
> 
> Is in v4.14 final.
> 
> Testing with/without those quotes:
> 
> [root@jouet ~]# perf trace -e open* touch /etc/passwd
>      0.000 ( 0.007 ms): touch/17246 open(filename: 0x99747e37, flags: CLOEXEC                             ) = 3
>      0.022 ( 0.004 ms): touch/17246 open(filename: 0x9994b640, flags: CLOEXEC                             ) = 3
>      0.189 ( 0.004 ms): touch/17246 open(filename: 0x994f1c70, flags: CLOEXEC                             ) = 3
>      0.224 ( 0.056 ms): touch/17246 open(filename: 0xaa80a32a, flags: CREAT|NOCTTY|NONBLOCK|WRONLY, mode: IRUGO|IWUGO) = 3
> [root@jouet ~]# perf trace -e 'open*' touch /etc/passwd
>      0.000 ( 0.039 ms): touch/17250 open(filename: 0xf1445e37, flags: CLOEXEC                             ) = 3
>      0.107 ( 0.030 ms): touch/17250 open(filename: 0xf1649640, flags: CLOEXEC                             ) = 3
>      0.976 ( 0.009 ms): touch/17250 open(filename: 0xf11efc70, flags: CLOEXEC                             ) = 3
>      1.032 ( 0.008 ms): touch/17250 open(filename: 0x9357432a, flags: CREAT|NOCTTY|NONBLOCK|WRONLY, mode: IRUGO|IWUGO) = 3
> [root@jouet ~]#
> 
> What differs from x86 to others is that x86 uses syscalltbl, not requiring
> audit-libs to map syscall numbers to names, so perhaps it is something in there...

If it only needs to consider open or openat, why not specifying the
both directly?

  # perf trace -e open -e openat touch /etc/passwd


Thanks,
Namhyung

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] perf test shell: Fix check open filename arg using 'perf trace'
  2017-12-01  2:33     ` Namhyung Kim
@ 2017-12-01 15:16       ` Arnaldo Carvalho de Melo
  2017-12-02 23:21         ` Michael Petlan
  2017-12-05 23:18         ` Michael Petlan
  0 siblings, 2 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-12-01 15:16 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo de Melo, Thomas-Mich Richter, Michael Petlan,
	linux-perf-users, Jiri Olsa, kernel-team

Em Fri, Dec 01, 2017 at 11:33:14AM +0900, Namhyung Kim escreveu:
> On Thu, Nov 30, 2017 at 01:56:42PM -0200, Arnaldo de Melo wrote:
> > Em Thu, Nov 30, 2017 at 11:28:33AM +0100, Thomas-Mich Richter escreveu:
> > > On 11/29/2017 06:27 PM, Michael Petlan wrote:
> > > > The commit f231af789b11a2f1a3795acc3228a3e178a80c21 adds an exception for s390x to
> > > > use openat() syscall instead of open(). This exception is not s390x-only, thus I
> > > > adjusted the test to accept both open and openat syscalls, no matter which arch it
> > > > runs on. Does it sound reasonable to you?
 
> It seems glibc 2.26 changed the behavior:
 
>   https://lwn.net/Articles/738694/

interesting
 
> > What differs from x86 to others is that x86 uses syscalltbl, not requiring
> > audit-libs to map syscall numbers to names, so perhaps it is something in there...
 
> If it only needs to consider open or openat, why not specifying the
> both directly?
 
>   # perf trace -e open -e openat touch /etc/passwd

Same effect, works with other perf trace versions, but one suggestion to
make it shorter:

	perf trace -e open,openat touch /etc/passwd

Does the same thing :-)

Michael, can you please take that into account and resubmit the patch?

- Arnaldo

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] perf test shell: Fix check open filename arg using 'perf trace'
  2017-12-01 15:16       ` Arnaldo Carvalho de Melo
@ 2017-12-02 23:21         ` Michael Petlan
  2017-12-05 23:18         ` Michael Petlan
  1 sibling, 0 replies; 12+ messages in thread
From: Michael Petlan @ 2017-12-02 23:21 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Namhyung Kim, Arnaldo de Melo, Thomas-Mich Richter,
	Michael Petlan, linux-perf-users, Jiri Olsa, kernel-team

On Fri, 1 Dec 2017, Arnaldo Carvalho de Melo wrote:
> Em Fri, Dec 01, 2017 at 11:33:14AM +0900, Namhyung Kim escreveu:
> > On Thu, Nov 30, 2017 at 01:56:42PM -0200, Arnaldo de Melo wrote:
> > > Em Thu, Nov 30, 2017 at 11:28:33AM +0100, Thomas-Mich Richter escreveu:
> > > > On 11/29/2017 06:27 PM, Michael Petlan wrote:
> > > > > The commit f231af789b11a2f1a3795acc3228a3e178a80c21 adds an exception for s390x to
> > > > > use openat() syscall instead of open(). This exception is not s390x-only, thus I
> > > > > adjusted the test to accept both open and openat syscalls, no matter which arch it
> > > > > runs on. Does it sound reasonable to you?
>  
> > It seems glibc 2.26 changed the behavior:
>  
> >   https://lwn.net/Articles/738694/
> 
> interesting
>  
> > > What differs from x86 to others is that x86 uses syscalltbl, not requiring
> > > audit-libs to map syscall numbers to names, so perhaps it is something in there...
>  
> > If it only needs to consider open or openat, why not specifying the
> > both directly?
>  
> >   # perf trace -e open -e openat touch /etc/passwd
> 
> Same effect, works with other perf trace versions, but one suggestion to
> make it shorter:
> 
> 	perf trace -e open,openat touch /etc/passwd
> 
> Does the same thing :-)
> 
> Michael, can you please take that into account and resubmit the patch?
> 

Sure, I will retry that again. However, as I can remember last time trying that,
it was failing in case one of them was missing on the arch... I have thought that
-e open,openat may succeed only in case both are successfully traced... Maybe I
am wrong. I will retry that on various kernels on Monday and see.

> - Arnaldo
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] perf test shell: Fix check open filename arg using 'perf trace'
  2017-11-30 15:59     ` Arnaldo de Melo
@ 2017-12-05 15:39       ` Hendrik Brueckner
  2017-12-06 16:34         ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 12+ messages in thread
From: Hendrik Brueckner @ 2017-12-05 15:39 UTC (permalink / raw)
  To: Arnaldo de Melo
  Cc: Thomas-Mich Richter, Michael Petlan, linux-perf-users, Jiri Olsa

Hi Arnaldo,

On Thu, Nov 30, 2017 at 01:59:22PM -0200, Arnaldo de Melo wrote:
> Em Thu, Nov 30, 2017 at 01:56:42PM -0200, Arnaldo de Melo escreveu:
> > Em Thu, Nov 30, 2017 at 11:28:33AM +0100, Thomas-Mich Richter escreveu:
> > > On 11/29/2017 06:27 PM, Michael Petlan wrote:
> > [acme@jouet linux]$ git log -1 27702bcfe8a125a1feeeb5f07526d63b20cac47f --oneline
> > 27702bcfe8a1 perf trace: Support syscall name globbing
> > [acme@jouet linux]$
> > 
> > Is in v4.14 final.
> > 
> > Testing with/without those quotes:
> > 
> > [root@jouet ~]# perf trace -e open* touch /etc/passwd
> >      0.000 ( 0.007 ms): touch/17246 open(filename: 0x99747e37, flags: CLOEXEC                             ) = 3
> >      0.022 ( 0.004 ms): touch/17246 open(filename: 0x9994b640, flags: CLOEXEC                             ) = 3
> >      0.189 ( 0.004 ms): touch/17246 open(filename: 0x994f1c70, flags: CLOEXEC                             ) = 3
> >      0.224 ( 0.056 ms): touch/17246 open(filename: 0xaa80a32a, flags: CREAT|NOCTTY|NONBLOCK|WRONLY, mode: IRUGO|IWUGO) = 3
> > [root@jouet ~]# perf trace -e 'open*' touch /etc/passwd
> >      0.000 ( 0.039 ms): touch/17250 open(filename: 0xf1445e37, flags: CLOEXEC                             ) = 3
> >      0.107 ( 0.030 ms): touch/17250 open(filename: 0xf1649640, flags: CLOEXEC                             ) = 3
> >      0.976 ( 0.009 ms): touch/17250 open(filename: 0xf11efc70, flags: CLOEXEC                             ) = 3
> >      1.032 ( 0.008 ms): touch/17250 open(filename: 0x9357432a, flags: CREAT|NOCTTY|NONBLOCK|WRONLY, mode: IRUGO|IWUGO) = 3
> > [root@jouet ~]#
> > 
> > What differs from x86 to others is that x86 uses syscalltbl, not requiring
> > audit-libs to map syscall numbers to names, so perhaps it is something in there...
> 
> Yeah :-\
> 
> int syscalltbl__strglobmatch_next(struct syscalltbl *tbl __maybe_unused,
>                                   const char *syscall_glob __maybe_unused, int *idx __maybe_unused)
> {
>         return -1;
> }
> 
> int syscalltbl__strglobmatch_first(struct syscalltbl *tbl, const char
> *syscall_glob, int *idx)
> {
>         return syscalltbl__strglobmatch_next(tbl, syscall_glob, idx);
> }
> #endif /* HAVE_SYSCALL_TABLE */
> 
> -----
> 
> So someone needs to implement syscalltbl__strglobmatch_next() for arches
> that don't HAVE_SYSCALL_TABLE, which is basically at this stage !x86.
> 
> Shouldn't be that difficult, but remains to be done.

Thanks for pointing in that direction.  I looked at it and will post a
patch to add syscall tables for s390 too.

Kind regards,
  Hendrik

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] perf test shell: Fix check open filename arg using 'perf trace'
  2017-12-01 15:16       ` Arnaldo Carvalho de Melo
  2017-12-02 23:21         ` Michael Petlan
@ 2017-12-05 23:18         ` Michael Petlan
  2017-12-06 14:28           ` Arnaldo de Melo
  1 sibling, 1 reply; 12+ messages in thread
From: Michael Petlan @ 2017-12-05 23:18 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Namhyung Kim, Arnaldo de Melo, Thomas-Mich Richter,
	linux-perf-users, Jiri Olsa, kernel-team

On Fri, 1 Dec 2017, Arnaldo Carvalho de Melo wrote:
> Em Fri, Dec 01, 2017 at 11:33:14AM +0900, Namhyung Kim escreveu:
[...]
>  
> > > What differs from x86 to others is that x86 uses syscalltbl, not requiring
> > > audit-libs to map syscall numbers to names, so perhaps it is something in there...
>  
> > If it only needs to consider open or openat, why not specifying the
> > both directly?
>  
> >   # perf trace -e open -e openat touch /etc/passwd
> 
> Same effect, works with other perf trace versions, but one suggestion to
> make it shorter:
> 
> 	perf trace -e open,openat touch /etc/passwd
> 
> Does the same thing :-)

Well, do I understand it correctly, that we prefer the "-e open,openat" over
the "-e 'open*'" because there are issues with the globbing feature on some
architectures?

I see a disadvantage of "-e open,openat" concept in that it does not work
in case any of them is unavailable. So in case some kernel does not contain
e.g. open in favor of openat, perf-trace fails to trace them. This can be
easily simulated like `perf trace -e open,openit`:

event syntax error: 'openit'
                     \___ Cannot find PMU `openit'. Missing kernel support?
Run 'perf list' for a list of valid events

Since it is possible that open is not even supported, having both of them
there is fragile. The "-e 'open*'" approach should expand only to supported
syscalls.

The '' are necessary there, since just open* can expand to some filename.

.....

I also see problems with globbing on my aarch64 with 4.14, so that makes
the "-e 'open*'" fragile too...

Ideas?

> 
> Michael, can you please take that into account and resubmit the patch?
> 
> - Arnaldo
> --
> To unsubscribe from this list: send the line "unsubscribe linux-perf-users" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] perf test shell: Fix check open filename arg using 'perf trace'
  2017-12-05 23:18         ` Michael Petlan
@ 2017-12-06 14:28           ` Arnaldo de Melo
  0 siblings, 0 replies; 12+ messages in thread
From: Arnaldo de Melo @ 2017-12-06 14:28 UTC (permalink / raw)
  To: Michael Petlan
  Cc: Arnaldo Carvalho de Melo, Namhyung Kim, Thomas-Mich Richter,
	linux-perf-users, Jiri Olsa, kernel-team

Em Wed, Dec 06, 2017 at 12:18:59AM +0100, Michael Petlan escreveu:
> On Fri, 1 Dec 2017, Arnaldo Carvalho de Melo wrote:
> > Em Fri, Dec 01, 2017 at 11:33:14AM +0900, Namhyung Kim escreveu:
> [...]
> >  
> > > > What differs from x86 to others is that x86 uses syscalltbl, not requiring
> > > > audit-libs to map syscall numbers to names, so perhaps it is something in there...
> >  
> > > If it only needs to consider open or openat, why not specifying the
> > > both directly?
> >  
> > >   # perf trace -e open -e openat touch /etc/passwd
> > 
> > Same effect, works with other perf trace versions, but one suggestion to
> > make it shorter:
> > 
> > 	perf trace -e open,openat touch /etc/passwd
> > 
> > Does the same thing :-)
> 
> Well, do I understand it correctly, that we prefer the "-e open,openat" over
> the "-e 'open*'" because there are issues with the globbing feature on some
> architectures?

The best one is '-e open*', but then wildcard support is recent, so not
all backports have it, so I thought about '-e open,openat', but that
shouldn't be used due to reasons you pointed out, so probably we should
try to one that works most of the time and then check if it worked, if
not, try the other one
 
> I see a disadvantage of "-e open,openat" concept in that it does not work
> in case any of them is unavailable. So in case some kernel does not contain
> e.g. open in favor of openat, perf-trace fails to trace them. This can be
> easily simulated like `perf trace -e open,openit`:
> 
> event syntax error: 'openit'
>                      \___ Cannot find PMU `openit'. Missing kernel support?
> Run 'perf list' for a list of valid events
> 
> Since it is possible that open is not even supported, having both of them
> there is fragile. The "-e 'open*'" approach should expand only to supported
> syscalls.
> 
> The '' are necessary there, since just open* can expand to some filename.
> 
> .....
> 
> I also see problems with globbing on my aarch64 with 4.14, so that makes

Right, only x86 has support for globbing, a fallback method for using
audit-libs to get the list of syscall names to apply the glob and get
the result (open, openat) from open* remains to be done.

For s/390 it is being worked on by Hendrik, IIRC without using
audit-libs.

So, for the tiem being, it seems the best way is to do something like:

[root@jouet acme]# perf list syscalls:sys_enter_open* |& egrep 'open(at)? ' | sed -r 's/.*sys_enter_(.*) +\[.*/-e \1/'
-e open                           
-e openat                         
[root@jouet acme]# 

Because 'perf list' should have glob support for long enough for use to
use it here?

- Arnaldo

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] perf test shell: Fix check open filename arg using 'perf trace'
  2017-12-05 15:39       ` Hendrik Brueckner
@ 2017-12-06 16:34         ` Arnaldo Carvalho de Melo
  2017-12-07  7:39           ` Hendrik Brueckner
  0 siblings, 1 reply; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-12-06 16:34 UTC (permalink / raw)
  To: Hendrik Brueckner
  Cc: Arnaldo de Melo, Thomas-Mich Richter, Michael Petlan,
	linux-perf-users, Jiri Olsa

Em Tue, Dec 05, 2017 at 04:39:49PM +0100, Hendrik Brueckner escreveu:
> On Thu, Nov 30, 2017 at 01:59:22PM -0200, Arnaldo de Melo wrote:
> > So someone needs to implement syscalltbl__strglobmatch_next() for arches
> > that don't HAVE_SYSCALL_TABLE, which is basically at this stage !x86.

> > Shouldn't be that difficult, but remains to be done.
 
> Thanks for pointing in that direction.  I looked at it and will post a
> patch to add syscall tables for s390 too.

Will you do it the way I did for x86? That would be the best, as we
would then reduce the need for audit-libs to be linked with perf.

We really need to reduce the number of libraries that perf links to :-\

- Arnaldo

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] perf test shell: Fix check open filename arg using 'perf trace'
  2017-12-06 16:34         ` Arnaldo Carvalho de Melo
@ 2017-12-07  7:39           ` Hendrik Brueckner
  0 siblings, 0 replies; 12+ messages in thread
From: Hendrik Brueckner @ 2017-12-07  7:39 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Hendrik Brueckner, Arnaldo de Melo, Thomas-Mich Richter,
	Michael Petlan, linux-perf-users, Jiri Olsa

On Wed, Dec 06, 2017 at 01:34:43PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Dec 05, 2017 at 04:39:49PM +0100, Hendrik Brueckner escreveu:
> > On Thu, Nov 30, 2017 at 01:59:22PM -0200, Arnaldo de Melo wrote:
> > > So someone needs to implement syscalltbl__strglobmatch_next() for arches
> > > that don't HAVE_SYSCALL_TABLE, which is basically at this stage !x86.
> 
> > > Shouldn't be that difficult, but remains to be done.
> 
> > Thanks for pointing in that direction.  I looked at it and will post a
> > patch to add syscall tables for s390 too.
> 
> Will you do it the way I did for x86? That would be the best, as we
> would then reduce the need for audit-libs to be linked with perf.

It looks similar to x86 but at the time being, s390 does not have a
system call table like x86.  So the input is different, the output
will follow what x86 provides to perf.

> 
> We really need to reduce the number of libraries that perf links to :-\

I have prepared two patches to get rid of the auditlibs dependency for
those arch's that have system call tables.  Going to post this today.

Thanks and kind regards,
  Hendrik

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-12-07  7:39 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-29 17:27 [PATCH] perf test shell: Fix check open filename arg using 'perf trace' Michael Petlan
2017-11-30 10:28 ` Thomas-Mich Richter
2017-11-30 15:56   ` Arnaldo de Melo
2017-11-30 15:59     ` Arnaldo de Melo
2017-12-05 15:39       ` Hendrik Brueckner
2017-12-06 16:34         ` Arnaldo Carvalho de Melo
2017-12-07  7:39           ` Hendrik Brueckner
2017-12-01  2:33     ` Namhyung Kim
2017-12-01 15:16       ` Arnaldo Carvalho de Melo
2017-12-02 23:21         ` Michael Petlan
2017-12-05 23:18         ` Michael Petlan
2017-12-06 14:28           ` Arnaldo de Melo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.