perf trace beautify: Beautify flags of mount(2) and umount(2).

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 <[email protected]>

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

tools/perf/builtin-trace.c +5 -0

index 22ab8e67c760..3cffb25d1928 100644
@@ -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 */ }, }, },

tools/perf/trace/beauty/mount_flags.c new file +79 -0

new file mode 100644
index 000000000000..86790b82c869
@@ -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

Signature · 1 line

2.17.1

Em Mon, Aug 27, 2018 at 08:53:44PM -0700, Benjamin Peterson escreveu:

Quoted 6 lines

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 <[email protected]>

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

Thanks for the review.

Quoted 27 lines · Arnaldo Carvalho de Melo

Em Mon, Aug 27, 2018 at 08:53:44PM -0700, Benjamin Peterson escreveu:

Quoted 6 lines

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 <[email protected]>

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

Quoted 8 lines

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

Quoted 7 lines

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

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

Quoted 67 lines

Thanks for the review.

Quoted 27 lines · Arnaldo Carvalho de Melo

Em Mon, Aug 27, 2018 at 08:53:44PM -0700, Benjamin Peterson escreveu:

Quoted 6 lines

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 <[email protected]>

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

Quoted 8 lines

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

Quoted 7 lines

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

Em Tue, Oct 09, 2018 at 08:52:26PM -0700, Benjamin Peterson escreveu:

Quoted 2 lines

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

Thanks, for pinging me about it, will check.

- Arnaldo

Quoted 68 lines · Benjamin Peterson

Quoted 67 lines

Thanks for the review.

Quoted 27 lines · Arnaldo Carvalho de Melo

Em Mon, Aug 27, 2018 at 08:53:44PM -0700, Benjamin Peterson escreveu:

Quoted 6 lines

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 <[email protected]>

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

Quoted 8 lines

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

Quoted 7 lines

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

Em Tue, Oct 09, 2018 at 08:52:26PM -0700, Benjamin Peterson escreveu:

Quoted 2 lines

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

Thanks, for pinging me about it, will check.

Quoted 5 lines

Em Tue, Oct 09, 2018 at 08:52:26PM -0700, Benjamin Peterson escreveu:

Quoted 2 lines

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?

Em Mon, Oct 22, 2018 at 07:40:35PM -0700, Benjamin Peterson escreveu:

Quoted 10 lines · Arnaldo Carvalho de Melo

Quoted 5 lines

Em Tue, Oct 09, 2018 at 08:52:26PM -0700, Benjamin Peterson escreveu:

Quoted 2 lines

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