linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] perf trace beautify: Beautify flags of mount(2) and umount(2).
@ 2018-08-28  3:53 Benjamin Peterson
  2018-08-30 18:28 ` Arnaldo Carvalho de Melo
  2018-09-06 13:44 ` [tip:perf/core] perf trace beauty: Alias 'umount' to 'umount2' tip-bot for Benjamin Peterson
  0 siblings, 2 replies; 10+ messages in thread
From: Benjamin Peterson @ 2018-08-28  3:53 UTC (permalink / raw)
  To: peterz, mingo, acme, alexander.shishkin, jolsa, namhyung; +Cc: linux-kernel

Example output of perf trace -e mount,umount2:

  6243.930 ( 0.052 ms): mount/30976 mount(dev_name: 0x55dc541bb920, dir_name: 0x55dc541bc960, type: 0x55dc541b9c40, flags: BIND) = 0
  7851.821 (26.015 ms): umount/30983 umount2(name: 0x558daa82cf50, flags: MNT_FORCE                        ) = 0

Signed-off-by: Benjamin Peterson <benjamin@python.org>
---
 tools/perf/builtin-trace.c            |  5 ++
 tools/perf/trace/beauty/mount_flags.c | 79 +++++++++++++++++++++++++++
 2 files changed, 84 insertions(+)
 create mode 100644 tools/perf/trace/beauty/mount_flags.c

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 22ab8e67c760..3cffb25d1928 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -603,6 +603,7 @@ static size_t syscall_arg__scnprintf_getrandom_flags(char *bf, size_t size,
 #include "trace/beauty/futex_val3.c"
 #include "trace/beauty/mmap.c"
 #include "trace/beauty/mode_t.c"
+#include "trace/beauty/mount_flags.c"
 #include "trace/beauty/msg_flags.c"
 #include "trace/beauty/open_flags.c"
 #include "trace/beauty/perf_event_open.c"
@@ -722,6 +723,8 @@ static struct syscall_fmt {
 	  .arg = { [0] = { .scnprintf = SCA_HEX,	/* addr */ },
 		   [2] = { .scnprintf = SCA_MMAP_PROT,	/* prot */ },
 		   [3] = { .scnprintf = SCA_MMAP_FLAGS,	/* flags */ }, }, },
+	{ .name     = "mount",
+	  .arg = { [3] = { .scnprintf = SCA_MOUNT_FLAGS, /* flags */ }, }, },
 	{ .name	    = "mprotect",
 	  .arg = { [0] = { .scnprintf = SCA_HEX,	/* start */ },
 		   [2] = { .scnprintf = SCA_MMAP_PROT,	/* prot */ }, }, },
@@ -830,6 +833,8 @@ static struct syscall_fmt {
 	  .arg = { [2] = { .scnprintf = SCA_SIGNUM, /* sig */ }, }, },
 	{ .name	    = "tkill",
 	  .arg = { [1] = { .scnprintf = SCA_SIGNUM, /* sig */ }, }, },
+	{ .name     = "umount2", .alias = "umount",
+	  .arg = { [1] = { .scnprintf = SCA_UMOUNT_FLAGS, /* flags */ }, }, },
 	{ .name	    = "uname", .alias = "newuname", },
 	{ .name	    = "unlinkat",
 	  .arg = { [0] = { .scnprintf = SCA_FDAT, /* dfd */ }, }, },
diff --git a/tools/perf/trace/beauty/mount_flags.c b/tools/perf/trace/beauty/mount_flags.c
new file mode 100644
index 000000000000..86790b82c869
--- /dev/null
+++ b/tools/perf/trace/beauty/mount_flags.c
@@ -0,0 +1,79 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <sys/mount.h>
+
+static size_t syscall_arg__scnprintf_mount_flags(char *bf, size_t size, struct syscall_arg *arg)
+{
+	size_t printed = 0;
+	int flags = arg->val;
+
+	if ((flags & MS_MGC_MSK) == MS_MGC_VAL)
+		flags &= ~MS_MGC_MSK;
+
+#define	P_FLAG(n) \
+	if (flags & MS_##n) { \
+		printed += scnprintf(bf + printed, size - printed, "%s%s", printed ? "|" : "", #n); \
+		flags &= ~MS_##n; \
+	}
+
+	P_FLAG(RDONLY);
+	P_FLAG(NOSUID);
+	P_FLAG(NODEV);
+	P_FLAG(NOEXEC);
+	P_FLAG(SYNCHRONOUS);
+	P_FLAG(REMOUNT);
+	P_FLAG(MANDLOCK);
+	P_FLAG(DIRSYNC);
+	P_FLAG(NOATIME);
+	P_FLAG(NODIRATIME);
+	P_FLAG(BIND);
+	P_FLAG(MOVE);
+	P_FLAG(REC);
+	P_FLAG(SILENT);
+	P_FLAG(POSIXACL);
+	P_FLAG(UNBINDABLE);
+	P_FLAG(PRIVATE);
+	P_FLAG(SLAVE);
+	P_FLAG(SHARED);
+	P_FLAG(RELATIME);
+	P_FLAG(KERNMOUNT);
+	P_FLAG(I_VERSION);
+	P_FLAG(STRICTATIME);
+	P_FLAG(LAZYTIME);
+	P_FLAG(ACTIVE);
+	P_FLAG(NOUSER);
+
+#undef P_FLAG
+
+	if (flags)
+		printed += scnprintf(bf + printed, size - printed, "%s%#x", printed ? "|" : "", flags);
+
+	return printed;
+}
+
+#define SCA_MOUNT_FLAGS syscall_arg__scnprintf_mount_flags
+
+static size_t syscall_arg__scnprintf_umount_flags(char *bf, size_t size, struct syscall_arg *arg)
+{
+	size_t printed = 0;
+	int flags = arg->val;
+
+#define	P_FLAG(n) \
+	if (flags & n) { \
+		printed += scnprintf(bf + printed, size - printed, "%s%s", printed ? "|" : "", #n); \
+		flags &= ~n; \
+	}
+
+	P_FLAG(MNT_FORCE);
+	P_FLAG(MNT_DETACH);
+	P_FLAG(MNT_EXPIRE);
+	P_FLAG(UMOUNT_NOFOLLOW);
+
+#undef P_FLAG
+
+	if (flags)
+		printed += scnprintf(bf + printed, size - printed, "%s%#x", printed ? "|" : "", flags);
+
+	return printed;
+}
+
+#define SCA_UMOUNT_FLAGS syscall_arg__scnprintf_umount_flags
-- 
2.17.1


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

* Re: [PATCH] perf trace beautify: Beautify flags of mount(2) and umount(2).
  2018-08-28  3:53 [PATCH] perf trace beautify: Beautify flags of mount(2) and umount(2) Benjamin Peterson
@ 2018-08-30 18:28 ` Arnaldo Carvalho de Melo
  2018-08-30 21:50   ` Benjamin Peterson
  2018-09-06 13:44 ` [tip:perf/core] perf trace beauty: Alias 'umount' to 'umount2' tip-bot for Benjamin Peterson
  1 sibling, 1 reply; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-08-30 18:28 UTC (permalink / raw)
  To: Benjamin Peterson
  Cc: peterz, mingo, alexander.shishkin, jolsa, namhyung, linux-kernel

Em Mon, Aug 27, 2018 at 08:53:44PM -0700, Benjamin Peterson escreveu:
> Example output of perf trace -e mount,umount2:
> 
>   6243.930 ( 0.052 ms): mount/30976 mount(dev_name: 0x55dc541bb920, dir_name: 0x55dc541bc960, type: 0x55dc541b9c40, flags: BIND) = 0
>   7851.821 (26.015 ms): umount/30983 umount2(name: 0x558daa82cf50, flags: MNT_FORCE                        ) = 0
> 
> Signed-off-by: Benjamin Peterson <benjamin@python.org>

So, I tried:

Before your patch:

  # perf trace -e *mount* mount /dev/mapper/fedora-home /s
     0.000 ( 4.335 ms): mount/3126 mount(dev_name: 0x55deecc4d480, dir_name: 0x55deecc4d4a0, type: 0x55deecc53880, flags: 3236757504) = 0
  #
  # perf trace -e *mount* umount /dev/mapper/fedora-home /s
         ? (         ): umount/3138  ... [continued]: umount2()) = 0
    11.576 (umount:  0.004 ms): /s: not mounted.umount/
3138 umount2(arg0: 94501956754656, arg1: 0, arg2: 1, arg3: 140051050083104, arg4: 4, arg5: 94501956755136) = -1 EINVAL Invalid argument
  #

After:

[root@jouet ~]# perf trace -e *mount* mount /dev/mapper/fedora-home /s
     0.000 ( 1.213 ms): mount/5244 mount(dev_name: 0x5558c1169480, dir_name: 0x5558c11694a0, type: 0x5558c116f880, flags: ) = 0
[root@jouet ~]# perf trace -e *mount* umount /s
     0.000 ( 9.241 ms): umount/5251 umount2(name: 0x55f74a986480                                          ) = 0

The flags for mount got empty? Can you pleaes check that? I.e. using the
default mount options, not specifying a 'bind' mount like you did.

I'm splitting this patch so that the aliasing of 'umount2' to 'umount'
gets merged now,

Check my perf/core branch at:

git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git

- Arnaldo

> ---
>  tools/perf/builtin-trace.c            |  5 ++
>  tools/perf/trace/beauty/mount_flags.c | 79 +++++++++++++++++++++++++++
>  2 files changed, 84 insertions(+)
>  create mode 100644 tools/perf/trace/beauty/mount_flags.c
> 
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index 22ab8e67c760..3cffb25d1928 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -603,6 +603,7 @@ static size_t syscall_arg__scnprintf_getrandom_flags(char *bf, size_t size,
>  #include "trace/beauty/futex_val3.c"
>  #include "trace/beauty/mmap.c"
>  #include "trace/beauty/mode_t.c"
> +#include "trace/beauty/mount_flags.c"
>  #include "trace/beauty/msg_flags.c"
>  #include "trace/beauty/open_flags.c"
>  #include "trace/beauty/perf_event_open.c"
> @@ -722,6 +723,8 @@ static struct syscall_fmt {
>  	  .arg = { [0] = { .scnprintf = SCA_HEX,	/* addr */ },
>  		   [2] = { .scnprintf = SCA_MMAP_PROT,	/* prot */ },
>  		   [3] = { .scnprintf = SCA_MMAP_FLAGS,	/* flags */ }, }, },
> +	{ .name     = "mount",
> +	  .arg = { [3] = { .scnprintf = SCA_MOUNT_FLAGS, /* flags */ }, }, },
>  	{ .name	    = "mprotect",
>  	  .arg = { [0] = { .scnprintf = SCA_HEX,	/* start */ },
>  		   [2] = { .scnprintf = SCA_MMAP_PROT,	/* prot */ }, }, },
> @@ -830,6 +833,8 @@ static struct syscall_fmt {
>  	  .arg = { [2] = { .scnprintf = SCA_SIGNUM, /* sig */ }, }, },
>  	{ .name	    = "tkill",
>  	  .arg = { [1] = { .scnprintf = SCA_SIGNUM, /* sig */ }, }, },
> +	{ .name     = "umount2", .alias = "umount",
> +	  .arg = { [1] = { .scnprintf = SCA_UMOUNT_FLAGS, /* flags */ }, }, },
>  	{ .name	    = "uname", .alias = "newuname", },
>  	{ .name	    = "unlinkat",
>  	  .arg = { [0] = { .scnprintf = SCA_FDAT, /* dfd */ }, }, },
> diff --git a/tools/perf/trace/beauty/mount_flags.c b/tools/perf/trace/beauty/mount_flags.c
> new file mode 100644
> index 000000000000..86790b82c869
> --- /dev/null
> +++ b/tools/perf/trace/beauty/mount_flags.c
> @@ -0,0 +1,79 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <sys/mount.h>
> +
> +static size_t syscall_arg__scnprintf_mount_flags(char *bf, size_t size, struct syscall_arg *arg)
> +{
> +	size_t printed = 0;
> +	int flags = arg->val;
> +
> +	if ((flags & MS_MGC_MSK) == MS_MGC_VAL)
> +		flags &= ~MS_MGC_MSK;
> +
> +#define	P_FLAG(n) \
> +	if (flags & MS_##n) { \
> +		printed += scnprintf(bf + printed, size - printed, "%s%s", printed ? "|" : "", #n); \
> +		flags &= ~MS_##n; \
> +	}
> +
> +	P_FLAG(RDONLY);
> +	P_FLAG(NOSUID);
> +	P_FLAG(NODEV);
> +	P_FLAG(NOEXEC);
> +	P_FLAG(SYNCHRONOUS);
> +	P_FLAG(REMOUNT);
> +	P_FLAG(MANDLOCK);
> +	P_FLAG(DIRSYNC);
> +	P_FLAG(NOATIME);
> +	P_FLAG(NODIRATIME);
> +	P_FLAG(BIND);
> +	P_FLAG(MOVE);
> +	P_FLAG(REC);
> +	P_FLAG(SILENT);
> +	P_FLAG(POSIXACL);
> +	P_FLAG(UNBINDABLE);
> +	P_FLAG(PRIVATE);
> +	P_FLAG(SLAVE);
> +	P_FLAG(SHARED);
> +	P_FLAG(RELATIME);
> +	P_FLAG(KERNMOUNT);
> +	P_FLAG(I_VERSION);
> +	P_FLAG(STRICTATIME);
> +	P_FLAG(LAZYTIME);
> +	P_FLAG(ACTIVE);
> +	P_FLAG(NOUSER);
> +
> +#undef P_FLAG
> +
> +	if (flags)
> +		printed += scnprintf(bf + printed, size - printed, "%s%#x", printed ? "|" : "", flags);
> +
> +	return printed;
> +}
> +
> +#define SCA_MOUNT_FLAGS syscall_arg__scnprintf_mount_flags
> +
> +static size_t syscall_arg__scnprintf_umount_flags(char *bf, size_t size, struct syscall_arg *arg)
> +{
> +	size_t printed = 0;
> +	int flags = arg->val;
> +
> +#define	P_FLAG(n) \
> +	if (flags & n) { \
> +		printed += scnprintf(bf + printed, size - printed, "%s%s", printed ? "|" : "", #n); \
> +		flags &= ~n; \
> +	}
> +
> +	P_FLAG(MNT_FORCE);
> +	P_FLAG(MNT_DETACH);
> +	P_FLAG(MNT_EXPIRE);
> +	P_FLAG(UMOUNT_NOFOLLOW);
> +
> +#undef P_FLAG
> +
> +	if (flags)
> +		printed += scnprintf(bf + printed, size - printed, "%s%#x", printed ? "|" : "", flags);
> +
> +	return printed;
> +}
> +
> +#define SCA_UMOUNT_FLAGS syscall_arg__scnprintf_umount_flags
> -- 
> 2.17.1

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

* Re: [PATCH] perf trace beautify: Beautify flags of mount(2) and umount(2).
  2018-08-30 18:28 ` Arnaldo Carvalho de Melo
@ 2018-08-30 21:50   ` Benjamin Peterson
  2018-10-10  3:52     ` Benjamin Peterson
  0 siblings, 1 reply; 10+ messages in thread
From: Benjamin Peterson @ 2018-08-30 21:50 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: peterz, mingo, alexander.shishkin, jolsa, namhyung, linux-kernel

Thanks for the review.

On Thu, Aug 30, 2018, at 11:28, Arnaldo Carvalho de Melo wrote:
> Em Mon, Aug 27, 2018 at 08:53:44PM -0700, Benjamin Peterson escreveu:
> > Example output of perf trace -e mount,umount2:
> > 
> >   6243.930 ( 0.052 ms): mount/30976 mount(dev_name: 0x55dc541bb920, dir_name: 0x55dc541bc960, type: 0x55dc541b9c40, flags: BIND) = 0
> >   7851.821 (26.015 ms): umount/30983 umount2(name: 0x558daa82cf50, flags: MNT_FORCE                        ) = 0
> > 
> > Signed-off-by: Benjamin Peterson <benjamin@python.org>
> 
> So, I tried:
> 
> Before your patch:
> 
>   # perf trace -e *mount* mount /dev/mapper/fedora-home /s
>      0.000 ( 4.335 ms): mount/3126 mount(dev_name: 0x55deecc4d480, 
> dir_name: 0x55deecc4d4a0, type: 0x55deecc53880, flags: 3236757504) = 0
>   #
>   # perf trace -e *mount* umount /dev/mapper/fedora-home /s
>          ? (         ): umount/3138  ... [continued]: umount2()) = 0
>     11.576 (umount:  0.004 ms): /s: not mounted.umount/
> 3138 umount2(arg0: 94501956754656, arg1: 0, arg2: 1, arg3: 
> 140051050083104, arg4: 4, arg5: 94501956755136) = -1 EINVAL Invalid 
> argument
>   #
> 
> After:
> 
> [root@jouet ~]# perf trace -e *mount* mount /dev/mapper/fedora-home /s
>      0.000 ( 1.213 ms): mount/5244 mount(dev_name: 0x5558c1169480, 
> dir_name: 0x5558c11694a0, type: 0x5558c116f880, flags: ) = 0
> [root@jouet ~]# perf trace -e *mount* umount /s
>      0.000 ( 9.241 ms): umount/5251 umount2(name: 
> 0x55f74a986480                                          ) = 0
> 
> The flags for mount got empty? Can you pleaes check that? I.e. using the
> default mount options, not specifying a 'bind' mount like you did.

That comes from these lines in syscall_arg__scnprintf_mount_flags:

if ((flags & MS_MGC_MSK) == MS_MGC_VAL)
	flags &= ~MS_MGC_MSK;

The reason for this is explained in the comment for do_mount in fs/namespace.c:

/*
 * Pre-0.97 versions of mount() didn't have a flags word.
 * When the flags word was introduced its top half was required
 * to have the magic value 0xC0ED, and this remained so until 2.4.0-test9.
 * Therefore, if this magic number is present, it carries no information
 * and must be discarded.
*/

(I used a bind mount for my commit message example because a "default" mount() doesn't use any flags.)

> 
> I'm splitting this patch so that the aliasing of 'umount2' to 'umount'
> gets merged now,
> 
> Check my perf/core branch at:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git

LGTM. Thanks


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

* [tip:perf/core] perf trace beauty: Alias 'umount' to 'umount2'
  2018-08-28  3:53 [PATCH] perf trace beautify: Beautify flags of mount(2) and umount(2) Benjamin Peterson
  2018-08-30 18:28 ` Arnaldo Carvalho de Melo
@ 2018-09-06 13:44 ` tip-bot for Benjamin Peterson
  1 sibling, 0 replies; 10+ messages in thread
From: tip-bot for Benjamin Peterson @ 2018-09-06 13:44 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: hpa, tglx, jolsa, benjamin, peterz, acme, alexander.shishkin,
	mingo, linux-kernel, namhyung

Commit-ID:  3de3e8bbf302545ef9acebb9f900939ac5c3820f
Gitweb:     https://git.kernel.org/tip/3de3e8bbf302545ef9acebb9f900939ac5c3820f
Author:     Benjamin Peterson <benjamin@python.org>
AuthorDate: Mon, 27 Aug 2018 20:53:44 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 30 Aug 2018 15:52:25 -0300

perf trace beauty: Alias 'umount' to 'umount2'

Before:

  # perf trace -e *mount* umount /dev/mapper/fedora-home /s
    11.576 ( 0.004 ms) umount/3138 umount2(arg0: 94501956754656, arg1: 0, arg2: 1, arg3: 140051050083104, arg4: 4, arg5: 94501956755136) = -1 EINVAL Invalid argument
  #

After:

  # perf trace -e *mount* umount /s
     0.000 ( 9.241 ms): umount/5251 umount2(name: 0x55f74a986480) = 0

Signed-off-by: Benjamin Peterson <benjamin@python.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20180828035344.31500-1-benjamin@python.org
[ split from a larger patch ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-trace.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 97ace635bed8..c106189f4066 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -825,6 +825,7 @@ static struct syscall_fmt {
 	  .arg = { [2] = { .scnprintf = SCA_SIGNUM, /* sig */ }, }, },
 	{ .name	    = "tkill",
 	  .arg = { [1] = { .scnprintf = SCA_SIGNUM, /* sig */ }, }, },
+	{ .name     = "umount2", .alias = "umount", },
 	{ .name	    = "uname", .alias = "newuname", },
 	{ .name	    = "unlinkat",
 	  .arg = { [0] = { .scnprintf = SCA_FDAT, /* dfd */ }, }, },

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

* Re: [PATCH] perf trace beautify: Beautify flags of mount(2) and umount(2).
  2018-08-30 21:50   ` Benjamin Peterson
@ 2018-10-10  3:52     ` Benjamin Peterson
  2018-10-10 12:55       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 10+ messages in thread
From: Benjamin Peterson @ 2018-10-10  3:52 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: peterz, mingo, alexander.shishkin, jolsa, namhyung, linux-kernel

Hi Arnaldo,
Did you get a chance to look at this again?

On Thu, Aug 30, 2018, at 14:50, Benjamin Peterson wrote:
> Thanks for the review.
> 
> On Thu, Aug 30, 2018, at 11:28, Arnaldo Carvalho de Melo wrote:
> > Em Mon, Aug 27, 2018 at 08:53:44PM -0700, Benjamin Peterson escreveu:
> > > Example output of perf trace -e mount,umount2:
> > > 
> > >   6243.930 ( 0.052 ms): mount/30976 mount(dev_name: 0x55dc541bb920, dir_name: 0x55dc541bc960, type: 0x55dc541b9c40, flags: BIND) = 0
> > >   7851.821 (26.015 ms): umount/30983 umount2(name: 0x558daa82cf50, flags: MNT_FORCE                        ) = 0
> > > 
> > > Signed-off-by: Benjamin Peterson <benjamin@python.org>
> > 
> > So, I tried:
> > 
> > Before your patch:
> > 
> >   # perf trace -e *mount* mount /dev/mapper/fedora-home /s
> >      0.000 ( 4.335 ms): mount/3126 mount(dev_name: 0x55deecc4d480, 
> > dir_name: 0x55deecc4d4a0, type: 0x55deecc53880, flags: 3236757504) = 0
> >   #
> >   # perf trace -e *mount* umount /dev/mapper/fedora-home /s
> >          ? (         ): umount/3138  ... [continued]: umount2()) = 0
> >     11.576 (umount:  0.004 ms): /s: not mounted.umount/
> > 3138 umount2(arg0: 94501956754656, arg1: 0, arg2: 1, arg3: 
> > 140051050083104, arg4: 4, arg5: 94501956755136) = -1 EINVAL Invalid 
> > argument
> >   #
> > 
> > After:
> > 
> > [root@jouet ~]# perf trace -e *mount* mount /dev/mapper/fedora-home /s
> >      0.000 ( 1.213 ms): mount/5244 mount(dev_name: 0x5558c1169480, 
> > dir_name: 0x5558c11694a0, type: 0x5558c116f880, flags: ) = 0
> > [root@jouet ~]# perf trace -e *mount* umount /s
> >      0.000 ( 9.241 ms): umount/5251 umount2(name: 
> > 0x55f74a986480                                          ) = 0
> > 
> > The flags for mount got empty? Can you pleaes check that? I.e. using the
> > default mount options, not specifying a 'bind' mount like you did.
> 
> That comes from these lines in syscall_arg__scnprintf_mount_flags:
> 
> if ((flags & MS_MGC_MSK) == MS_MGC_VAL)
> 	flags &= ~MS_MGC_MSK;
> 
> The reason for this is explained in the comment for do_mount in fs/namespace.c:
> 
> /*
>  * Pre-0.97 versions of mount() didn't have a flags word.
>  * When the flags word was introduced its top half was required
>  * to have the magic value 0xC0ED, and this remained so until 2.4.0-test9.
>  * Therefore, if this magic number is present, it carries no information
>  * and must be discarded.
> */
> 
> (I used a bind mount for my commit message example because a "default" 
> mount() doesn't use any flags.)
> 
> > 
> > I'm splitting this patch so that the aliasing of 'umount2' to 'umount'
> > gets merged now,
> > 
> > Check my perf/core branch at:
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
> 
> LGTM. Thanks
> 

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

* Re: [PATCH] perf trace beautify: Beautify flags of mount(2) and umount(2).
  2018-10-10  3:52     ` Benjamin Peterson
@ 2018-10-10 12:55       ` Arnaldo Carvalho de Melo
  2018-10-10 12:56         ` Arnaldo Carvalho de Melo
  2018-10-23  2:40         ` Benjamin Peterson
  0 siblings, 2 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-10-10 12:55 UTC (permalink / raw)
  To: Benjamin Peterson
  Cc: peterz, mingo, alexander.shishkin, jolsa, namhyung, linux-kernel

Em Tue, Oct 09, 2018 at 08:52:26PM -0700, Benjamin Peterson escreveu:
> Hi Arnaldo,
> Did you get a chance to look at this again?

Thanks, for pinging me about it, will check.

- Arnaldo
 
> On Thu, Aug 30, 2018, at 14:50, Benjamin Peterson wrote:
> > Thanks for the review.
> > 
> > On Thu, Aug 30, 2018, at 11:28, Arnaldo Carvalho de Melo wrote:
> > > Em Mon, Aug 27, 2018 at 08:53:44PM -0700, Benjamin Peterson escreveu:
> > > > Example output of perf trace -e mount,umount2:
> > > > 
> > > >   6243.930 ( 0.052 ms): mount/30976 mount(dev_name: 0x55dc541bb920, dir_name: 0x55dc541bc960, type: 0x55dc541b9c40, flags: BIND) = 0
> > > >   7851.821 (26.015 ms): umount/30983 umount2(name: 0x558daa82cf50, flags: MNT_FORCE                        ) = 0
> > > > 
> > > > Signed-off-by: Benjamin Peterson <benjamin@python.org>
> > > 
> > > So, I tried:
> > > 
> > > Before your patch:
> > > 
> > >   # perf trace -e *mount* mount /dev/mapper/fedora-home /s
> > >      0.000 ( 4.335 ms): mount/3126 mount(dev_name: 0x55deecc4d480, 
> > > dir_name: 0x55deecc4d4a0, type: 0x55deecc53880, flags: 3236757504) = 0
> > >   #
> > >   # perf trace -e *mount* umount /dev/mapper/fedora-home /s
> > >          ? (         ): umount/3138  ... [continued]: umount2()) = 0
> > >     11.576 (umount:  0.004 ms): /s: not mounted.umount/
> > > 3138 umount2(arg0: 94501956754656, arg1: 0, arg2: 1, arg3: 
> > > 140051050083104, arg4: 4, arg5: 94501956755136) = -1 EINVAL Invalid 
> > > argument
> > >   #
> > > 
> > > After:
> > > 
> > > [root@jouet ~]# perf trace -e *mount* mount /dev/mapper/fedora-home /s
> > >      0.000 ( 1.213 ms): mount/5244 mount(dev_name: 0x5558c1169480, 
> > > dir_name: 0x5558c11694a0, type: 0x5558c116f880, flags: ) = 0
> > > [root@jouet ~]# perf trace -e *mount* umount /s
> > >      0.000 ( 9.241 ms): umount/5251 umount2(name: 
> > > 0x55f74a986480                                          ) = 0
> > > 
> > > The flags for mount got empty? Can you pleaes check that? I.e. using the
> > > default mount options, not specifying a 'bind' mount like you did.
> > 
> > That comes from these lines in syscall_arg__scnprintf_mount_flags:
> > 
> > if ((flags & MS_MGC_MSK) == MS_MGC_VAL)
> > 	flags &= ~MS_MGC_MSK;
> > 
> > The reason for this is explained in the comment for do_mount in fs/namespace.c:
> > 
> > /*
> >  * Pre-0.97 versions of mount() didn't have a flags word.
> >  * When the flags word was introduced its top half was required
> >  * to have the magic value 0xC0ED, and this remained so until 2.4.0-test9.
> >  * Therefore, if this magic number is present, it carries no information
> >  * and must be discarded.
> > */
> > 
> > (I used a bind mount for my commit message example because a "default" 
> > mount() doesn't use any flags.)
> > 
> > > 
> > > I'm splitting this patch so that the aliasing of 'umount2' to 'umount'
> > > gets merged now,
> > > 
> > > Check my perf/core branch at:
> > > 
> > > git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
> > 
> > LGTM. Thanks
> > 

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

* Re: [PATCH] perf trace beautify: Beautify flags of mount(2) and umount(2).
  2018-10-10 12:55       ` Arnaldo Carvalho de Melo
@ 2018-10-10 12:56         ` Arnaldo Carvalho de Melo
  2018-10-23  2:40         ` Benjamin Peterson
  1 sibling, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-10-10 12:56 UTC (permalink / raw)
  To: Benjamin Peterson
  Cc: peterz, mingo, alexander.shishkin, jolsa, namhyung, linux-kernel

Em Wed, Oct 10, 2018 at 09:55:06AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Oct 09, 2018 at 08:52:26PM -0700, Benjamin Peterson escreveu:
> > Hi Arnaldo,
> > Did you get a chance to look at this again?
> 
> Thanks, for pinging me about it, will check.

So, this is already upstream, I'll check the other bit.

commit 3de3e8bbf302545ef9acebb9f900939ac5c3820f
Author: Benjamin Peterson <benjamin@python.org>
Date:   Mon Aug 27 20:53:44 2018 -0700

    perf trace beauty: Alias 'umount' to 'umount2'
    
    Before:
    
      # perf trace -e *mount* umount /dev/mapper/fedora-home /s
        11.576 ( 0.004 ms) umount/3138 umount2(arg0: 94501956754656, arg1: 0, arg2: 1, arg3: 140051050083104, arg4: 4, arg5: 94501956755136) = -1 EINVAL Invalid argument
      #
    
    After:
    
      # perf trace -e *mount* umount /s
         0.000 ( 9.241 ms): umount/5251 umount2(name: 0x55f74a986480) = 0
    
    Signed-off-by: Benjamin Peterson <benjamin@python.org>
    Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
    Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
    Cc: Jiri Olsa <jolsa@redhat.com>
    Cc: Namhyung Kim <namhyung@kernel.org>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Link: http://lkml.kernel.org/r/20180828035344.31500-1-benjamin@python.org
    [ split from a larger patch ]
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 97ace635bed8..c106189f4066 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -825,6 +825,7 @@ static struct syscall_fmt {
 	  .arg = { [2] = { .scnprintf = SCA_SIGNUM, /* sig */ }, }, },
 	{ .name	    = "tkill",
 	  .arg = { [1] = { .scnprintf = SCA_SIGNUM, /* sig */ }, }, },
+	{ .name     = "umount2", .alias = "umount", },
 	{ .name	    = "uname", .alias = "newuname", },
 	{ .name	    = "unlinkat",
 	  .arg = { [0] = { .scnprintf = SCA_FDAT, /* dfd */ }, }, },
 
> - Arnaldo
>  
> > On Thu, Aug 30, 2018, at 14:50, Benjamin Peterson wrote:
> > > Thanks for the review.
> > > 
> > > On Thu, Aug 30, 2018, at 11:28, Arnaldo Carvalho de Melo wrote:
> > > > Em Mon, Aug 27, 2018 at 08:53:44PM -0700, Benjamin Peterson escreveu:
> > > > > Example output of perf trace -e mount,umount2:
> > > > > 
> > > > >   6243.930 ( 0.052 ms): mount/30976 mount(dev_name: 0x55dc541bb920, dir_name: 0x55dc541bc960, type: 0x55dc541b9c40, flags: BIND) = 0
> > > > >   7851.821 (26.015 ms): umount/30983 umount2(name: 0x558daa82cf50, flags: MNT_FORCE                        ) = 0
> > > > > 
> > > > > Signed-off-by: Benjamin Peterson <benjamin@python.org>
> > > > 
> > > > So, I tried:
> > > > 
> > > > Before your patch:
> > > > 
> > > >   # perf trace -e *mount* mount /dev/mapper/fedora-home /s
> > > >      0.000 ( 4.335 ms): mount/3126 mount(dev_name: 0x55deecc4d480, 
> > > > dir_name: 0x55deecc4d4a0, type: 0x55deecc53880, flags: 3236757504) = 0
> > > >   #
> > > >   # perf trace -e *mount* umount /dev/mapper/fedora-home /s
> > > >          ? (         ): umount/3138  ... [continued]: umount2()) = 0
> > > >     11.576 (umount:  0.004 ms): /s: not mounted.umount/
> > > > 3138 umount2(arg0: 94501956754656, arg1: 0, arg2: 1, arg3: 
> > > > 140051050083104, arg4: 4, arg5: 94501956755136) = -1 EINVAL Invalid 
> > > > argument
> > > >   #
> > > > 
> > > > After:
> > > > 
> > > > [root@jouet ~]# perf trace -e *mount* mount /dev/mapper/fedora-home /s
> > > >      0.000 ( 1.213 ms): mount/5244 mount(dev_name: 0x5558c1169480, 
> > > > dir_name: 0x5558c11694a0, type: 0x5558c116f880, flags: ) = 0
> > > > [root@jouet ~]# perf trace -e *mount* umount /s
> > > >      0.000 ( 9.241 ms): umount/5251 umount2(name: 
> > > > 0x55f74a986480                                          ) = 0
> > > > 
> > > > The flags for mount got empty? Can you pleaes check that? I.e. using the
> > > > default mount options, not specifying a 'bind' mount like you did.
> > > 
> > > That comes from these lines in syscall_arg__scnprintf_mount_flags:
> > > 
> > > if ((flags & MS_MGC_MSK) == MS_MGC_VAL)
> > > 	flags &= ~MS_MGC_MSK;
> > > 
> > > The reason for this is explained in the comment for do_mount in fs/namespace.c:
> > > 
> > > /*
> > >  * Pre-0.97 versions of mount() didn't have a flags word.
> > >  * When the flags word was introduced its top half was required
> > >  * to have the magic value 0xC0ED, and this remained so until 2.4.0-test9.
> > >  * Therefore, if this magic number is present, it carries no information
> > >  * and must be discarded.
> > > */
> > > 
> > > (I used a bind mount for my commit message example because a "default" 
> > > mount() doesn't use any flags.)
> > > 
> > > > 
> > > > I'm splitting this patch so that the aliasing of 'umount2' to 'umount'
> > > > gets merged now,
> > > > 
> > > > Check my perf/core branch at:
> > > > 
> > > > git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
> > > 
> > > LGTM. Thanks
> > > 

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

* Re: [PATCH] perf trace beautify: Beautify flags of mount(2) and umount(2).
  2018-10-10 12:55       ` Arnaldo Carvalho de Melo
  2018-10-10 12:56         ` Arnaldo Carvalho de Melo
@ 2018-10-23  2:40         ` Benjamin Peterson
  2018-10-24 20:18           ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 10+ messages in thread
From: Benjamin Peterson @ 2018-10-23  2:40 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: peterz, mingo, alexander.shishkin, jolsa, namhyung, linux-kernel



On Wed, Oct 10, 2018, at 05:55, Arnaldo Carvalho de Melo wrote:
> Em Tue, Oct 09, 2018 at 08:52:26PM -0700, Benjamin Peterson escreveu:
> > Hi Arnaldo,
> > Did you get a chance to look at this again?
> 
> Thanks, for pinging me about it, will check.

Will the main part of my patch manage to make the cut for 4.20?

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

* Re: [PATCH] perf trace beautify: Beautify flags of mount(2) and umount(2).
  2018-10-23  2:40         ` Benjamin Peterson
@ 2018-10-24 20:18           ` Arnaldo Carvalho de Melo
  2018-10-26  6:23             ` Benjamin Peterson
  0 siblings, 1 reply; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-10-24 20:18 UTC (permalink / raw)
  To: Benjamin Peterson
  Cc: peterz, mingo, alexander.shishkin, jolsa, namhyung, linux-kernel

Em Mon, Oct 22, 2018 at 07:40:35PM -0700, Benjamin Peterson escreveu:
> 
> 
> On Wed, Oct 10, 2018, at 05:55, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Oct 09, 2018 at 08:52:26PM -0700, Benjamin Peterson escreveu:
> > > Hi Arnaldo,
> > > Did you get a chance to look at this again?

> > Thanks, for pinging me about it, will check.
 
> Will the main part of my patch manage to make the cut for 4.20?

The functionality will, but I'm implementing it differently, the model
you used to create the patch is being phased out, now we're unsing a
model that create tables from the header files where those flags are
defined, so that when a new one gets added, we get it included
automatically, please take a look at:

https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=tmp.perf/trace.mount_flags

More specifically this one:

https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=tmp.perf/trace.mount_flags&id=341628af5643adbaa78097d528f34b4b16aa908f

That will be used by one like tools/perf/trace/beauty/pkey_alloc.c, that
goes looking if the bits are set and if so, get the strings from the
table, etc.

In time pkey_alloc__scnprintf_access_rights() will be further
generalized, as all it needs is a strarray + the flags integer to
produce a simbolic or-ed flags string.

Sorry for taking so long, kept being sidetracked with other stuff...

- Arnaldo

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

* Re: [PATCH] perf trace beautify: Beautify flags of mount(2) and umount(2).
  2018-10-24 20:18           ` Arnaldo Carvalho de Melo
@ 2018-10-26  6:23             ` Benjamin Peterson
  0 siblings, 0 replies; 10+ messages in thread
From: Benjamin Peterson @ 2018-10-26  6:23 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: peterz, mingo, alexander.shishkin, jolsa, namhyung, linux-kernel



On Wed, Oct 24, 2018, at 13:18, Arnaldo Carvalho de Melo wrote:
> Em Mon, Oct 22, 2018 at 07:40:35PM -0700, Benjamin Peterson escreveu:
> > 
> > 
> > On Wed, Oct 10, 2018, at 05:55, Arnaldo Carvalho de Melo wrote:
> > > Em Tue, Oct 09, 2018 at 08:52:26PM -0700, Benjamin Peterson escreveu:
> > > > Hi Arnaldo,
> > > > Did you get a chance to look at this again?
> 
> > > Thanks, for pinging me about it, will check.
>  
> > Will the main part of my patch manage to make the cut for 4.20?
> 
> The functionality will, but I'm implementing it differently, the model
> you used to create the patch is being phased out, now we're unsing a
> model that create tables from the header files where those flags are
> defined, so that when a new one gets added, we get it included
> automatically, please take a look at:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=tmp.perf/trace.mount_flags
> 
> More specifically this one:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=tmp.perf/trace.mount_flags&id=341628af5643adbaa78097d528f34b4b16aa908f
> 
> That will be used by one like tools/perf/trace/beauty/pkey_alloc.c, that
> goes looking if the bits are set and if so, get the strings from the
> table, etc.
> 
> In time pkey_alloc__scnprintf_access_rights() will be further
> generalized, as all it needs is a strarray + the flags integer to
> produce a simbolic or-ed flags string.
> 
> Sorry for taking so long, kept being sidetracked with other stuff...

Thanks for explaining the new implementation. I'm excited to see these improvements to perf trace.

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

end of thread, other threads:[~2018-10-26  6:23 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-28  3:53 [PATCH] perf trace beautify: Beautify flags of mount(2) and umount(2) Benjamin Peterson
2018-08-30 18:28 ` Arnaldo Carvalho de Melo
2018-08-30 21:50   ` Benjamin Peterson
2018-10-10  3:52     ` Benjamin Peterson
2018-10-10 12:55       ` Arnaldo Carvalho de Melo
2018-10-10 12:56         ` Arnaldo Carvalho de Melo
2018-10-23  2:40         ` Benjamin Peterson
2018-10-24 20:18           ` Arnaldo Carvalho de Melo
2018-10-26  6:23             ` Benjamin Peterson
2018-09-06 13:44 ` [tip:perf/core] perf trace beauty: Alias 'umount' to 'umount2' tip-bot for Benjamin Peterson

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).