linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] More perf trace syscall pretty printing improvements
@ 2024-03-20 19:31 Arnaldo Carvalho de Melo
  2024-03-20 19:31 ` [PATCH 1/5] perf beauty: Introduce scrape script for various fs syscalls 'flags' arguments Arnaldo Carvalho de Melo
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-03-20 19:31 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Jiri Olsa, Namhyung Kim, Adrian Hunter, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Hi,

	Here are some more improvements for 'perf trace', please review,

Thanks,

- Arnaldo

Arnaldo Carvalho de Melo (5):
  perf beauty: Introduce scrape script for various fs syscalls 'flags' arguments
  perf beauty: Introduce scrape script for the 'statx' syscall 'mask' argument
  perf beauty: Introduce faccessat2 flags scnprintf routine
  perf trace: Beautify the 'flags' arg of unlinkat
  perf trace: Fix 'newfstatat'/'fstatat' argument pretty printing

 tools/perf/Makefile.perf                      |  14 ++
 tools/perf/builtin-trace.c                    |  22 +-
 tools/perf/check-headers.sh                   |   2 +
 tools/perf/trace/beauty/Build                 |   1 +
 tools/perf/trace/beauty/beauty.h              |   7 +-
 tools/perf/trace/beauty/fs_at_flags.c         |  50 +++++
 tools/perf/trace/beauty/fs_at_flags.sh        |  21 ++
 .../trace/beauty/include/uapi/linux/fcntl.h   | 123 +++++++++++
 .../trace/beauty/include/uapi/linux/stat.h    | 195 ++++++++++++++++++
 tools/perf/trace/beauty/statx.c               |  77 +------
 tools/perf/trace/beauty/statx_mask.sh         |  23 +++
 11 files changed, 458 insertions(+), 77 deletions(-)
 create mode 100644 tools/perf/trace/beauty/fs_at_flags.c
 create mode 100755 tools/perf/trace/beauty/fs_at_flags.sh
 create mode 100644 tools/perf/trace/beauty/include/uapi/linux/fcntl.h
 create mode 100644 tools/perf/trace/beauty/include/uapi/linux/stat.h
 create mode 100755 tools/perf/trace/beauty/statx_mask.sh

-- 
2.44.0


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

* [PATCH 1/5] perf beauty: Introduce scrape script for various fs syscalls 'flags' arguments
  2024-03-20 19:31 [PATCH 0/5] More perf trace syscall pretty printing improvements Arnaldo Carvalho de Melo
@ 2024-03-20 19:31 ` Arnaldo Carvalho de Melo
  2024-03-21  1:47   ` Ian Rogers
  2024-03-20 19:31 ` [PATCH 2/5] perf beauty: Introduce scrape script for the 'statx' syscall 'mask' argument Arnaldo Carvalho de Melo
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-03-20 19:31 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Jiri Olsa, Namhyung Kim, Adrian Hunter, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo

From: Arnaldo Carvalho de Melo <acme@redhat.com>

It was using the first variation on producing a string representation
for a binary flag, one that used the system's fcntl.h and preprocessor
tricks that had to be updated everytime a new flag was introduced.

Use the more recent scrape script + strarray + strarray__scnprintf_flags() combo.

  $ tools/perf/trace/beauty/fs_at_flags.sh
  static const char *fs_at_flags[] = {
  	[ilog2(0x100) + 1] = "SYMLINK_NOFOLLOW",
  	[ilog2(0x200) + 1] = "REMOVEDIR",
  	[ilog2(0x400) + 1] = "SYMLINK_FOLLOW",
  	[ilog2(0x800) + 1] = "NO_AUTOMOUNT",
  	[ilog2(0x1000) + 1] = "EMPTY_PATH",
  	[ilog2(0x0000) + 1] = "STATX_SYNC_AS_STAT",
  	[ilog2(0x2000) + 1] = "STATX_FORCE_SYNC",
  	[ilog2(0x4000) + 1] = "STATX_DONT_SYNC",
  	[ilog2(0x8000) + 1] = "RECURSIVE",
  	[ilog2(0x80000000) + 1] = "GETATTR_NOSEC",
  };
  $

Now we need a copy of uapi/linux/fcntl.h from tools/include/ in the
scrape only directory tools/perf/trace/beauty/include and will use that
fs_at_flags array for other fs syscalls.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Ian Rogers <irogers@google.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Makefile.perf                      |   7 +
 tools/perf/builtin-trace.c                    |   2 +-
 tools/perf/check-headers.sh                   |   1 +
 tools/perf/trace/beauty/Build                 |   1 +
 tools/perf/trace/beauty/beauty.h              |   4 +-
 tools/perf/trace/beauty/fs_at_flags.c         |  26 ++++
 tools/perf/trace/beauty/fs_at_flags.sh        |  21 +++
 .../trace/beauty/include/uapi/linux/fcntl.h   | 123 ++++++++++++++++++
 tools/perf/trace/beauty/statx.c               |  31 -----
 9 files changed, 182 insertions(+), 34 deletions(-)
 create mode 100644 tools/perf/trace/beauty/fs_at_flags.c
 create mode 100755 tools/perf/trace/beauty/fs_at_flags.sh
 create mode 100644 tools/perf/trace/beauty/include/uapi/linux/fcntl.h

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index ccd2dcbc64f720d2..73d5603450b0a547 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -489,6 +489,12 @@ beauty_ioctl_outdir := $(beauty_outdir)/ioctl
 # Create output directory if not already present
 $(shell [ -d '$(beauty_ioctl_outdir)' ] || mkdir -p '$(beauty_ioctl_outdir)')
 
+fs_at_flags_array := $(beauty_outdir)/fs_at_flags_array.c
+fs_at_flags_tbl := $(srctree)/tools/perf/trace/beauty/fs_at_flags.sh
+
+$(fs_at_flags_array): $(beauty_uapi_linux_dir)/fcntl.h $(fs_at_flags_tbl)
+	$(Q)$(SHELL) '$(fs_at_flags_tbl)' $(beauty_uapi_linux_dir) > $@
+
 clone_flags_array := $(beauty_outdir)/clone_flags_array.c
 clone_flags_tbl := $(srctree)/tools/perf/trace/beauty/clone.sh
 
@@ -772,6 +778,7 @@ build-dir   = $(or $(__build-dir),.)
 
 prepare: $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)common-cmds.h archheaders \
 	arm64-sysreg-defs \
+	$(fs_at_flags_array) \
 	$(clone_flags_array) \
 	$(drm_ioctl_array) \
 	$(fadvise_advice_array) \
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 8fb032caeaf53288..8417387aafa8295d 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -1144,7 +1144,7 @@ static const struct syscall_fmt syscall_fmts[] = {
 	{ .name	    = "stat", .alias = "newstat", },
 	{ .name	    = "statx",
 	  .arg = { [0] = { .scnprintf = SCA_FDAT,	 /* fdat */ },
-		   [2] = { .scnprintf = SCA_STATX_FLAGS, /* flags */ } ,
+		   [2] = { .scnprintf = SCA_FS_AT_FLAGS, /* flags */ } ,
 		   [3] = { .scnprintf = SCA_STATX_MASK,	 /* mask */ }, }, },
 	{ .name	    = "swapoff",
 	  .arg = { [0] = { .scnprintf = SCA_FILENAME, /* specialfile */ }, }, },
diff --git a/tools/perf/check-headers.sh b/tools/perf/check-headers.sh
index 413c9b747216020f..d23a84fdf3efef78 100755
--- a/tools/perf/check-headers.sh
+++ b/tools/perf/check-headers.sh
@@ -89,6 +89,7 @@ BEAUTY_FILES=(
   "arch/x86/include/asm/irq_vectors.h"
   "arch/x86/include/uapi/asm/prctl.h"
   "include/linux/socket.h"
+  "include/uapi/linux/fcntl.h"
   "include/uapi/linux/fs.h"
   "include/uapi/linux/mount.h"
   "include/uapi/linux/prctl.h"
diff --git a/tools/perf/trace/beauty/Build b/tools/perf/trace/beauty/Build
index d11ce256f5114034..d8ce1b6989832134 100644
--- a/tools/perf/trace/beauty/Build
+++ b/tools/perf/trace/beauty/Build
@@ -1,6 +1,7 @@
 perf-y += clone.o
 perf-y += fcntl.o
 perf-y += flock.o
+perf-y += fs_at_flags.o
 perf-y += fsmount.o
 perf-y += fspick.o
 ifeq ($(SRCARCH),$(filter $(SRCARCH),x86))
diff --git a/tools/perf/trace/beauty/beauty.h b/tools/perf/trace/beauty/beauty.h
index 9feb794f5c6e15f4..c94ae8701bc65a2f 100644
--- a/tools/perf/trace/beauty/beauty.h
+++ b/tools/perf/trace/beauty/beauty.h
@@ -234,8 +234,8 @@ size_t syscall_arg__scnprintf_socket_protocol(char *bf, size_t size, struct sysc
 size_t syscall_arg__scnprintf_socket_level(char *bf, size_t size, struct syscall_arg *arg);
 #define SCA_SK_LEVEL syscall_arg__scnprintf_socket_level
 
-size_t syscall_arg__scnprintf_statx_flags(char *bf, size_t size, struct syscall_arg *arg);
-#define SCA_STATX_FLAGS syscall_arg__scnprintf_statx_flags
+size_t syscall_arg__scnprintf_fs_at_flags(char *bf, size_t size, struct syscall_arg *arg);
+#define SCA_FS_AT_FLAGS syscall_arg__scnprintf_fs_at_flags
 
 size_t syscall_arg__scnprintf_statx_mask(char *bf, size_t size, struct syscall_arg *arg);
 #define SCA_STATX_MASK syscall_arg__scnprintf_statx_mask
diff --git a/tools/perf/trace/beauty/fs_at_flags.c b/tools/perf/trace/beauty/fs_at_flags.c
new file mode 100644
index 0000000000000000..2a099953d9935782
--- /dev/null
+++ b/tools/perf/trace/beauty/fs_at_flags.c
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: LGPL-2.1
+/*
+ * trace/beauty/fs_at_flags.c
+ *
+ *  Copyright (C) 2017, Red Hat Inc, Arnaldo Carvalho de Melo <acme@redhat.com>
+ */
+
+#include "trace/beauty/beauty.h"
+#include <sys/types.h>
+#include <linux/log2.h>
+
+#include "trace/beauty/generated/fs_at_flags_array.c"
+static DEFINE_STRARRAY(fs_at_flags, "AT_");
+
+static size_t fs_at__scnprintf_flags(unsigned long flags, char *bf, size_t size, bool show_prefix)
+{
+	return strarray__scnprintf_flags(&strarray__fs_at_flags, bf, size, show_prefix, flags);
+}
+
+size_t syscall_arg__scnprintf_fs_at_flags(char *bf, size_t size, struct syscall_arg *arg)
+{
+	bool show_prefix = arg->show_string_prefix;
+	int flags = arg->val;
+
+	return fs_at__scnprintf_flags(flags, bf, size, show_prefix);
+}
diff --git a/tools/perf/trace/beauty/fs_at_flags.sh b/tools/perf/trace/beauty/fs_at_flags.sh
new file mode 100755
index 0000000000000000..456f59addf741062
--- /dev/null
+++ b/tools/perf/trace/beauty/fs_at_flags.sh
@@ -0,0 +1,21 @@
+#!/bin/sh
+# SPDX-License-Identifier: LGPL-2.1
+
+if [ $# -ne 1 ] ; then
+	beauty_uapi_linux_dir=tools/perf/trace/beauty/include/uapi/linux/
+else
+	beauty_uapi_linux_dir=$1
+fi
+
+linux_fcntl=${beauty_uapi_linux_dir}/fcntl.h
+
+printf "static const char *fs_at_flags[] = {\n"
+regex='^[[:space:]]*#[[:space:]]*define[[:space:]]+AT_([^_]+[[:alnum:]_]+)[[:space:]]+(0x[[:xdigit:]]+)[[:space:]]*.*'
+# AT_EACCESS is only meaningful to faccessat, so we will special case it there...
+# AT_STATX_SYNC_TYPE is not a bit, its a mask of AT_STATX_SYNC_AS_STAT, AT_STATX_FORCE_SYNC and AT_STATX_DONT_SYNC
+grep -E $regex ${linux_fcntl} | \
+	grep -v AT_EACCESS | \
+	grep -v AT_STATX_SYNC_TYPE | \
+	sed -r "s/$regex/\2 \1/g"	| \
+	xargs printf "\t[ilog2(%s) + 1] = \"%s\",\n"
+printf "};\n"
diff --git a/tools/perf/trace/beauty/include/uapi/linux/fcntl.h b/tools/perf/trace/beauty/include/uapi/linux/fcntl.h
new file mode 100644
index 0000000000000000..282e90aeb163c028
--- /dev/null
+++ b/tools/perf/trace/beauty/include/uapi/linux/fcntl.h
@@ -0,0 +1,123 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_LINUX_FCNTL_H
+#define _UAPI_LINUX_FCNTL_H
+
+#include <asm/fcntl.h>
+#include <linux/openat2.h>
+
+#define F_SETLEASE	(F_LINUX_SPECIFIC_BASE + 0)
+#define F_GETLEASE	(F_LINUX_SPECIFIC_BASE + 1)
+
+/*
+ * Cancel a blocking posix lock; internal use only until we expose an
+ * asynchronous lock api to userspace:
+ */
+#define F_CANCELLK	(F_LINUX_SPECIFIC_BASE + 5)
+
+/* Create a file descriptor with FD_CLOEXEC set. */
+#define F_DUPFD_CLOEXEC	(F_LINUX_SPECIFIC_BASE + 6)
+
+/*
+ * Request nofications on a directory.
+ * See below for events that may be notified.
+ */
+#define F_NOTIFY	(F_LINUX_SPECIFIC_BASE+2)
+
+/*
+ * Set and get of pipe page size array
+ */
+#define F_SETPIPE_SZ	(F_LINUX_SPECIFIC_BASE + 7)
+#define F_GETPIPE_SZ	(F_LINUX_SPECIFIC_BASE + 8)
+
+/*
+ * Set/Get seals
+ */
+#define F_ADD_SEALS	(F_LINUX_SPECIFIC_BASE + 9)
+#define F_GET_SEALS	(F_LINUX_SPECIFIC_BASE + 10)
+
+/*
+ * Types of seals
+ */
+#define F_SEAL_SEAL	0x0001	/* prevent further seals from being set */
+#define F_SEAL_SHRINK	0x0002	/* prevent file from shrinking */
+#define F_SEAL_GROW	0x0004	/* prevent file from growing */
+#define F_SEAL_WRITE	0x0008	/* prevent writes */
+#define F_SEAL_FUTURE_WRITE	0x0010  /* prevent future writes while mapped */
+#define F_SEAL_EXEC	0x0020  /* prevent chmod modifying exec bits */
+/* (1U << 31) is reserved for signed error codes */
+
+/*
+ * Set/Get write life time hints. {GET,SET}_RW_HINT operate on the
+ * underlying inode, while {GET,SET}_FILE_RW_HINT operate only on
+ * the specific file.
+ */
+#define F_GET_RW_HINT		(F_LINUX_SPECIFIC_BASE + 11)
+#define F_SET_RW_HINT		(F_LINUX_SPECIFIC_BASE + 12)
+#define F_GET_FILE_RW_HINT	(F_LINUX_SPECIFIC_BASE + 13)
+#define F_SET_FILE_RW_HINT	(F_LINUX_SPECIFIC_BASE + 14)
+
+/*
+ * Valid hint values for F_{GET,SET}_RW_HINT. 0 is "not set", or can be
+ * used to clear any hints previously set.
+ */
+#define RWH_WRITE_LIFE_NOT_SET	0
+#define RWH_WRITE_LIFE_NONE	1
+#define RWH_WRITE_LIFE_SHORT	2
+#define RWH_WRITE_LIFE_MEDIUM	3
+#define RWH_WRITE_LIFE_LONG	4
+#define RWH_WRITE_LIFE_EXTREME	5
+
+/*
+ * The originally introduced spelling is remained from the first
+ * versions of the patch set that introduced the feature, see commit
+ * v4.13-rc1~212^2~51.
+ */
+#define RWF_WRITE_LIFE_NOT_SET	RWH_WRITE_LIFE_NOT_SET
+
+/*
+ * Types of directory notifications that may be requested.
+ */
+#define DN_ACCESS	0x00000001	/* File accessed */
+#define DN_MODIFY	0x00000002	/* File modified */
+#define DN_CREATE	0x00000004	/* File created */
+#define DN_DELETE	0x00000008	/* File removed */
+#define DN_RENAME	0x00000010	/* File renamed */
+#define DN_ATTRIB	0x00000020	/* File changed attibutes */
+#define DN_MULTISHOT	0x80000000	/* Don't remove notifier */
+
+/*
+ * The constants AT_REMOVEDIR and AT_EACCESS have the same value.  AT_EACCESS is
+ * meaningful only to faccessat, while AT_REMOVEDIR is meaningful only to
+ * unlinkat.  The two functions do completely different things and therefore,
+ * the flags can be allowed to overlap.  For example, passing AT_REMOVEDIR to
+ * faccessat would be undefined behavior and thus treating it equivalent to
+ * AT_EACCESS is valid undefined behavior.
+ */
+#define AT_FDCWD		-100    /* Special value used to indicate
+                                           openat should use the current
+                                           working directory. */
+#define AT_SYMLINK_NOFOLLOW	0x100   /* Do not follow symbolic links.  */
+#define AT_EACCESS		0x200	/* Test access permitted for
+                                           effective IDs, not real IDs.  */
+#define AT_REMOVEDIR		0x200   /* Remove directory instead of
+                                           unlinking file.  */
+#define AT_SYMLINK_FOLLOW	0x400   /* Follow symbolic links.  */
+#define AT_NO_AUTOMOUNT		0x800	/* Suppress terminal automount traversal */
+#define AT_EMPTY_PATH		0x1000	/* Allow empty relative pathname */
+
+#define AT_STATX_SYNC_TYPE	0x6000	/* Type of synchronisation required from statx() */
+#define AT_STATX_SYNC_AS_STAT	0x0000	/* - Do whatever stat() does */
+#define AT_STATX_FORCE_SYNC	0x2000	/* - Force the attributes to be sync'd with the server */
+#define AT_STATX_DONT_SYNC	0x4000	/* - Don't sync attributes with the server */
+
+#define AT_RECURSIVE		0x8000	/* Apply to the entire subtree */
+
+/* Flags for name_to_handle_at(2). We reuse AT_ flag space to save bits... */
+#define AT_HANDLE_FID		AT_REMOVEDIR	/* file handle is needed to
+					compare object identity and may not
+					be usable to open_by_handle_at(2) */
+#if defined(__KERNEL__)
+#define AT_GETATTR_NOSEC	0x80000000
+#endif
+
+#endif /* _UAPI_LINUX_FCNTL_H */
diff --git a/tools/perf/trace/beauty/statx.c b/tools/perf/trace/beauty/statx.c
index 1f7e34ed4e02be86..4e0059fd02118f9c 100644
--- a/tools/perf/trace/beauty/statx.c
+++ b/tools/perf/trace/beauty/statx.c
@@ -8,7 +8,6 @@
 #include "trace/beauty/beauty.h"
 #include <linux/kernel.h>
 #include <sys/types.h>
-#include <linux/fcntl.h>
 #include <linux/stat.h>
 
 #ifndef STATX_MNT_ID
@@ -21,36 +20,6 @@
 #define STATX_MNT_ID_UNIQUE	0x00004000U
 #endif
 
-size_t syscall_arg__scnprintf_statx_flags(char *bf, size_t size, struct syscall_arg *arg)
-{
-	bool show_prefix = arg->show_string_prefix;
-	const char *prefix = "AT_";
-	int printed = 0, flags = arg->val;
-
-	if (flags == 0)
-		return scnprintf(bf, size, "%s%s", show_prefix ? "AT_STATX_" : "", "SYNC_AS_STAT");
-#define	P_FLAG(n) \
-	if (flags & AT_##n) { \
-		printed += scnprintf(bf + printed, size - printed, "%s%s", printed ? "|" : "", show_prefix ? prefix : "", #n); \
-		flags &= ~AT_##n; \
-	}
-
-	P_FLAG(SYMLINK_NOFOLLOW);
-	P_FLAG(REMOVEDIR);
-	P_FLAG(SYMLINK_FOLLOW);
-	P_FLAG(NO_AUTOMOUNT);
-	P_FLAG(EMPTY_PATH);
-	P_FLAG(STATX_FORCE_SYNC);
-	P_FLAG(STATX_DONT_SYNC);
-
-#undef P_FLAG
-
-	if (flags)
-		printed += scnprintf(bf + printed, size - printed, "%s%#x", printed ? "|" : "", flags);
-
-	return printed;
-}
-
 size_t syscall_arg__scnprintf_statx_mask(char *bf, size_t size, struct syscall_arg *arg)
 {
 	bool show_prefix = arg->show_string_prefix;
-- 
2.44.0


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

* [PATCH 2/5] perf beauty: Introduce scrape script for the 'statx' syscall 'mask' argument
  2024-03-20 19:31 [PATCH 0/5] More perf trace syscall pretty printing improvements Arnaldo Carvalho de Melo
  2024-03-20 19:31 ` [PATCH 1/5] perf beauty: Introduce scrape script for various fs syscalls 'flags' arguments Arnaldo Carvalho de Melo
@ 2024-03-20 19:31 ` Arnaldo Carvalho de Melo
  2024-03-21  1:49   ` Ian Rogers
  2024-03-20 19:31 ` [PATCH 3/5] perf beauty: Introduce faccessat2 flags scnprintf routine Arnaldo Carvalho de Melo
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-03-20 19:31 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Jiri Olsa, Namhyung Kim, Adrian Hunter, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo

From: Arnaldo Carvalho de Melo <acme@redhat.com>

It was using the first variation on producing a string representation
for a binary flag, one that used the system's stat.h and preprocessor
tricks that had to be updated everytime a new flag was introduced.

Use the more recent scrape script + strarray +
strarray__scnprintf_flags() combo.

  $ tools/perf/trace/beauty/statx_mask.sh
  static const char *statx_mask[] = {
  	[ilog2(0x00000001) + 1] = "TYPE",
  	[ilog2(0x00000002) + 1] = "MODE",
  	[ilog2(0x00000004) + 1] = "NLINK",
  	[ilog2(0x00000008) + 1] = "UID",
  	[ilog2(0x00000010) + 1] = "GID",
  	[ilog2(0x00000020) + 1] = "ATIME",
  	[ilog2(0x00000040) + 1] = "MTIME",
  	[ilog2(0x00000080) + 1] = "CTIME",
  	[ilog2(0x00000100) + 1] = "INO",
  	[ilog2(0x00000200) + 1] = "SIZE",
  	[ilog2(0x00000400) + 1] = "BLOCKS",
  	[ilog2(0x00000800) + 1] = "BTIME",
  	[ilog2(0x00001000) + 1] = "MNT_ID",
  	[ilog2(0x00002000) + 1] = "DIOALIGN",
  	[ilog2(0x00004000) + 1] = "MNT_ID_UNIQUE",
  };
  $

Now we need a copy of uapi/linux/stat.h from tools/include/ in the
scrape only directory tools/perf/trace/beauty/include.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Ian Rogers <irogers@google.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Makefile.perf                      |   7 +
 tools/perf/check-headers.sh                   |   1 +
 .../trace/beauty/include/uapi/linux/stat.h    | 195 ++++++++++++++++++
 tools/perf/trace/beauty/statx.c               |  50 +----
 tools/perf/trace/beauty/statx_mask.sh         |  23 +++
 5 files changed, 235 insertions(+), 41 deletions(-)
 create mode 100644 tools/perf/trace/beauty/include/uapi/linux/stat.h
 create mode 100755 tools/perf/trace/beauty/statx_mask.sh

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 73d5603450b0a547..0d2dbdfc44df3019 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -673,6 +673,12 @@ arch_errno_tbl := $(srctree)/tools/perf/trace/beauty/arch_errno_names.sh
 $(arch_errno_name_array): $(arch_errno_tbl)
 	$(Q)$(SHELL) '$(arch_errno_tbl)' '$(patsubst -%,,$(CC))' $(arch_errno_hdr_dir) > $@
 
+statx_mask_array := $(beauty_outdir)/statx_mask_array.c
+statx_mask_tbl := $(srctree)/tools/perf/trace/beauty/statx_mask.sh
+
+$(statx_mask_array): $(beauty_uapi_linux_dir)/stat.h $(statx_mask_tbl)
+	$(Q)$(SHELL) '$(statx_mask_tbl)' $(beauty_uapi_linux_dir) > $@
+
 sync_file_range_arrays := $(beauty_outdir)/sync_file_range_arrays.c
 sync_file_range_tbls := $(srctree)/tools/perf/trace/beauty/sync_file_range.sh
 
@@ -807,6 +813,7 @@ prepare: $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)common-cmds.h archheaders \
 	$(x86_arch_prctl_code_array) \
 	$(rename_flags_array) \
 	$(arch_errno_name_array) \
+	$(statx_mask_array) \
 	$(sync_file_range_arrays) \
 	$(LIBAPI) \
 	$(LIBPERF) \
diff --git a/tools/perf/check-headers.sh b/tools/perf/check-headers.sh
index d23a84fdf3efef78..76726a5a7c789273 100755
--- a/tools/perf/check-headers.sh
+++ b/tools/perf/check-headers.sh
@@ -94,6 +94,7 @@ BEAUTY_FILES=(
   "include/uapi/linux/mount.h"
   "include/uapi/linux/prctl.h"
   "include/uapi/linux/sched.h"
+  "include/uapi/linux/stat.h"
   "include/uapi/linux/usbdevice_fs.h"
   "include/uapi/sound/asound.h"
 )
diff --git a/tools/perf/trace/beauty/include/uapi/linux/stat.h b/tools/perf/trace/beauty/include/uapi/linux/stat.h
new file mode 100644
index 0000000000000000..2f2ee82d55175d05
--- /dev/null
+++ b/tools/perf/trace/beauty/include/uapi/linux/stat.h
@@ -0,0 +1,195 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_LINUX_STAT_H
+#define _UAPI_LINUX_STAT_H
+
+#include <linux/types.h>
+
+#if defined(__KERNEL__) || !defined(__GLIBC__) || (__GLIBC__ < 2)
+
+#define S_IFMT  00170000
+#define S_IFSOCK 0140000
+#define S_IFLNK	 0120000
+#define S_IFREG  0100000
+#define S_IFBLK  0060000
+#define S_IFDIR  0040000
+#define S_IFCHR  0020000
+#define S_IFIFO  0010000
+#define S_ISUID  0004000
+#define S_ISGID  0002000
+#define S_ISVTX  0001000
+
+#define S_ISLNK(m)	(((m) & S_IFMT) == S_IFLNK)
+#define S_ISREG(m)	(((m) & S_IFMT) == S_IFREG)
+#define S_ISDIR(m)	(((m) & S_IFMT) == S_IFDIR)
+#define S_ISCHR(m)	(((m) & S_IFMT) == S_IFCHR)
+#define S_ISBLK(m)	(((m) & S_IFMT) == S_IFBLK)
+#define S_ISFIFO(m)	(((m) & S_IFMT) == S_IFIFO)
+#define S_ISSOCK(m)	(((m) & S_IFMT) == S_IFSOCK)
+
+#define S_IRWXU 00700
+#define S_IRUSR 00400
+#define S_IWUSR 00200
+#define S_IXUSR 00100
+
+#define S_IRWXG 00070
+#define S_IRGRP 00040
+#define S_IWGRP 00020
+#define S_IXGRP 00010
+
+#define S_IRWXO 00007
+#define S_IROTH 00004
+#define S_IWOTH 00002
+#define S_IXOTH 00001
+
+#endif
+
+/*
+ * Timestamp structure for the timestamps in struct statx.
+ *
+ * tv_sec holds the number of seconds before (negative) or after (positive)
+ * 00:00:00 1st January 1970 UTC.
+ *
+ * tv_nsec holds a number of nanoseconds (0..999,999,999) after the tv_sec time.
+ *
+ * __reserved is held in case we need a yet finer resolution.
+ */
+struct statx_timestamp {
+	__s64	tv_sec;
+	__u32	tv_nsec;
+	__s32	__reserved;
+};
+
+/*
+ * Structures for the extended file attribute retrieval system call
+ * (statx()).
+ *
+ * The caller passes a mask of what they're specifically interested in as a
+ * parameter to statx().  What statx() actually got will be indicated in
+ * st_mask upon return.
+ *
+ * For each bit in the mask argument:
+ *
+ * - if the datum is not supported:
+ *
+ *   - the bit will be cleared, and
+ *
+ *   - the datum will be set to an appropriate fabricated value if one is
+ *     available (eg. CIFS can take a default uid and gid), otherwise
+ *
+ *   - the field will be cleared;
+ *
+ * - otherwise, if explicitly requested:
+ *
+ *   - the datum will be synchronised to the server if AT_STATX_FORCE_SYNC is
+ *     set or if the datum is considered out of date, and
+ *
+ *   - the field will be filled in and the bit will be set;
+ *
+ * - otherwise, if not requested, but available in approximate form without any
+ *   effort, it will be filled in anyway, and the bit will be set upon return
+ *   (it might not be up to date, however, and no attempt will be made to
+ *   synchronise the internal state first);
+ *
+ * - otherwise the field and the bit will be cleared before returning.
+ *
+ * Items in STATX_BASIC_STATS may be marked unavailable on return, but they
+ * will have values installed for compatibility purposes so that stat() and
+ * co. can be emulated in userspace.
+ */
+struct statx {
+	/* 0x00 */
+	__u32	stx_mask;	/* What results were written [uncond] */
+	__u32	stx_blksize;	/* Preferred general I/O size [uncond] */
+	__u64	stx_attributes;	/* Flags conveying information about the file [uncond] */
+	/* 0x10 */
+	__u32	stx_nlink;	/* Number of hard links */
+	__u32	stx_uid;	/* User ID of owner */
+	__u32	stx_gid;	/* Group ID of owner */
+	__u16	stx_mode;	/* File mode */
+	__u16	__spare0[1];
+	/* 0x20 */
+	__u64	stx_ino;	/* Inode number */
+	__u64	stx_size;	/* File size */
+	__u64	stx_blocks;	/* Number of 512-byte blocks allocated */
+	__u64	stx_attributes_mask; /* Mask to show what's supported in stx_attributes */
+	/* 0x40 */
+	struct statx_timestamp	stx_atime;	/* Last access time */
+	struct statx_timestamp	stx_btime;	/* File creation time */
+	struct statx_timestamp	stx_ctime;	/* Last attribute change time */
+	struct statx_timestamp	stx_mtime;	/* Last data modification time */
+	/* 0x80 */
+	__u32	stx_rdev_major;	/* Device ID of special file [if bdev/cdev] */
+	__u32	stx_rdev_minor;
+	__u32	stx_dev_major;	/* ID of device containing file [uncond] */
+	__u32	stx_dev_minor;
+	/* 0x90 */
+	__u64	stx_mnt_id;
+	__u32	stx_dio_mem_align;	/* Memory buffer alignment for direct I/O */
+	__u32	stx_dio_offset_align;	/* File offset alignment for direct I/O */
+	/* 0xa0 */
+	__u64	__spare3[12];	/* Spare space for future expansion */
+	/* 0x100 */
+};
+
+/*
+ * Flags to be stx_mask
+ *
+ * Query request/result mask for statx() and struct statx::stx_mask.
+ *
+ * These bits should be set in the mask argument of statx() to request
+ * particular items when calling statx().
+ */
+#define STATX_TYPE		0x00000001U	/* Want/got stx_mode & S_IFMT */
+#define STATX_MODE		0x00000002U	/* Want/got stx_mode & ~S_IFMT */
+#define STATX_NLINK		0x00000004U	/* Want/got stx_nlink */
+#define STATX_UID		0x00000008U	/* Want/got stx_uid */
+#define STATX_GID		0x00000010U	/* Want/got stx_gid */
+#define STATX_ATIME		0x00000020U	/* Want/got stx_atime */
+#define STATX_MTIME		0x00000040U	/* Want/got stx_mtime */
+#define STATX_CTIME		0x00000080U	/* Want/got stx_ctime */
+#define STATX_INO		0x00000100U	/* Want/got stx_ino */
+#define STATX_SIZE		0x00000200U	/* Want/got stx_size */
+#define STATX_BLOCKS		0x00000400U	/* Want/got stx_blocks */
+#define STATX_BASIC_STATS	0x000007ffU	/* The stuff in the normal stat struct */
+#define STATX_BTIME		0x00000800U	/* Want/got stx_btime */
+#define STATX_MNT_ID		0x00001000U	/* Got stx_mnt_id */
+#define STATX_DIOALIGN		0x00002000U	/* Want/got direct I/O alignment info */
+#define STATX_MNT_ID_UNIQUE	0x00004000U	/* Want/got extended stx_mount_id */
+
+#define STATX__RESERVED		0x80000000U	/* Reserved for future struct statx expansion */
+
+#ifndef __KERNEL__
+/*
+ * This is deprecated, and shall remain the same value in the future.  To avoid
+ * confusion please use the equivalent (STATX_BASIC_STATS | STATX_BTIME)
+ * instead.
+ */
+#define STATX_ALL		0x00000fffU
+#endif
+
+/*
+ * Attributes to be found in stx_attributes and masked in stx_attributes_mask.
+ *
+ * These give information about the features or the state of a file that might
+ * be of use to ordinary userspace programs such as GUIs or ls rather than
+ * specialised tools.
+ *
+ * Note that the flags marked [I] correspond to the FS_IOC_SETFLAGS flags
+ * semantically.  Where possible, the numerical value is picked to correspond
+ * also.  Note that the DAX attribute indicates that the file is in the CPU
+ * direct access state.  It does not correspond to the per-inode flag that
+ * some filesystems support.
+ *
+ */
+#define STATX_ATTR_COMPRESSED		0x00000004 /* [I] File is compressed by the fs */
+#define STATX_ATTR_IMMUTABLE		0x00000010 /* [I] File is marked immutable */
+#define STATX_ATTR_APPEND		0x00000020 /* [I] File is append-only */
+#define STATX_ATTR_NODUMP		0x00000040 /* [I] File is not to be dumped */
+#define STATX_ATTR_ENCRYPTED		0x00000800 /* [I] File requires key to decrypt in fs */
+#define STATX_ATTR_AUTOMOUNT		0x00001000 /* Dir: Automount trigger */
+#define STATX_ATTR_MOUNT_ROOT		0x00002000 /* Root of a mount */
+#define STATX_ATTR_VERITY		0x00100000 /* [I] Verity protected file */
+#define STATX_ATTR_DAX			0x00200000 /* File is currently in DAX state */
+
+
+#endif /* _UAPI_LINUX_STAT_H */
diff --git a/tools/perf/trace/beauty/statx.c b/tools/perf/trace/beauty/statx.c
index 4e0059fd02118f9c..24843e614b935f3a 100644
--- a/tools/perf/trace/beauty/statx.c
+++ b/tools/perf/trace/beauty/statx.c
@@ -6,52 +6,20 @@
  */
 
 #include "trace/beauty/beauty.h"
-#include <linux/kernel.h>
 #include <sys/types.h>
-#include <linux/stat.h>
+#include <linux/log2.h>
 
-#ifndef STATX_MNT_ID
-#define STATX_MNT_ID		0x00001000U
-#endif
-#ifndef STATX_DIOALIGN
-#define STATX_DIOALIGN		0x00002000U
-#endif
-#ifndef STATX_MNT_ID_UNIQUE
-#define STATX_MNT_ID_UNIQUE	0x00004000U
-#endif
+static size_t statx__scnprintf_mask(unsigned long mask, char *bf, size_t size, bool show_prefix)
+{
+	#include "trace/beauty/generated/statx_mask_array.c"
+	static DEFINE_STRARRAY(statx_mask, "STATX_");
+	return strarray__scnprintf_flags(&strarray__statx_mask, bf, size, show_prefix, mask);
+}
 
 size_t syscall_arg__scnprintf_statx_mask(char *bf, size_t size, struct syscall_arg *arg)
 {
 	bool show_prefix = arg->show_string_prefix;
-	const char *prefix = "STATX_";
-	int printed = 0, flags = arg->val;
-
-#define	P_FLAG(n) \
-	if (flags & STATX_##n) { \
-		printed += scnprintf(bf + printed, size - printed, "%s%s", printed ? "|" : "", show_prefix ? prefix : "", #n); \
-		flags &= ~STATX_##n; \
-	}
-
-	P_FLAG(TYPE);
-	P_FLAG(MODE);
-	P_FLAG(NLINK);
-	P_FLAG(UID);
-	P_FLAG(GID);
-	P_FLAG(ATIME);
-	P_FLAG(MTIME);
-	P_FLAG(CTIME);
-	P_FLAG(INO);
-	P_FLAG(SIZE);
-	P_FLAG(BLOCKS);
-	P_FLAG(BTIME);
-	P_FLAG(MNT_ID);
-	P_FLAG(DIOALIGN);
-	P_FLAG(MNT_ID_UNIQUE);
-
-#undef P_FLAG
-
-	if (flags)
-		printed += scnprintf(bf + printed, size - printed, "%s%#x", printed ? "|" : "", flags);
+	int mask = arg->val;
 
-	return printed;
+	return statx__scnprintf_mask(mask, bf, size, show_prefix);
 }
diff --git a/tools/perf/trace/beauty/statx_mask.sh b/tools/perf/trace/beauty/statx_mask.sh
new file mode 100755
index 0000000000000000..18c802ed0c71578f
--- /dev/null
+++ b/tools/perf/trace/beauty/statx_mask.sh
@@ -0,0 +1,23 @@
+#!/bin/sh
+# SPDX-License-Identifier: LGPL-2.1
+
+if [ $# -ne 1 ] ; then
+	beauty_uapi_linux_dir=tools/perf/trace/beauty/include/uapi/linux/
+else
+	beauty_uapi_linux_dir=$1
+fi
+
+linux_stat=${beauty_uapi_linux_dir}/stat.h
+
+printf "static const char *statx_mask[] = {\n"
+regex='^[[:space:]]*#[[:space:]]*define[[:space:]]+STATX_([^_]+[[:alnum:]_]+)[[:space:]]+(0x[[:xdigit:]]+)[[:space:]]*.*'
+# STATX_BASIC_STATS its a bitmask formed by the mask in the normal stat struct
+# STATX_ALL is another bitmask and deprecated
+# STATX_ATTR_*: Attributes to be found in stx_attributes and masked in stx_attributes_mask
+grep -E $regex ${linux_stat} | \
+	grep -v STATX_ALL | \
+	grep -v STATX_BASIC_STATS | \
+	grep -v '\<STATX_ATTR_' | \
+	sed -r "s/$regex/\2 \1/g"	| \
+	xargs printf "\t[ilog2(%s) + 1] = \"%s\",\n"
+printf "};\n"
-- 
2.44.0


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

* [PATCH 3/5] perf beauty: Introduce faccessat2 flags scnprintf routine
  2024-03-20 19:31 [PATCH 0/5] More perf trace syscall pretty printing improvements Arnaldo Carvalho de Melo
  2024-03-20 19:31 ` [PATCH 1/5] perf beauty: Introduce scrape script for various fs syscalls 'flags' arguments Arnaldo Carvalho de Melo
  2024-03-20 19:31 ` [PATCH 2/5] perf beauty: Introduce scrape script for the 'statx' syscall 'mask' argument Arnaldo Carvalho de Melo
@ 2024-03-20 19:31 ` Arnaldo Carvalho de Melo
  2024-03-21  1:50   ` Ian Rogers
  2024-03-20 19:31 ` [PATCH 4/5] perf trace: Beautify the 'flags' arg of unlinkat Arnaldo Carvalho de Melo
  2024-03-20 19:31 ` [PATCH 5/5] perf trace: Fix 'newfstatat'/'fstatat' argument pretty printing Arnaldo Carvalho de Melo
  4 siblings, 1 reply; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-03-20 19:31 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Jiri Olsa, Namhyung Kim, Adrian Hunter, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo

From: Arnaldo Carvalho de Melo <acme@redhat.com>

The fsaccessat and fsaccessat2 now have beautifiers for its arguments.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Ian Rogers <irogers@google.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-trace.c            |  9 +++++++++
 tools/perf/trace/beauty/beauty.h      |  3 +++
 tools/perf/trace/beauty/fs_at_flags.c | 24 ++++++++++++++++++++++++
 3 files changed, 36 insertions(+)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 8417387aafa8295d..58546e8af9fcf481 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -947,6 +947,15 @@ static const struct syscall_fmt syscall_fmts[] = {
 	  .arg = { [1] = STRARRAY(op, epoll_ctl_ops), }, },
 	{ .name	    = "eventfd2",
 	  .arg = { [1] = { .scnprintf = SCA_EFD_FLAGS, /* flags */ }, }, },
+	{ .name     = "faccessat",
+	  .arg = { [0] = { .scnprintf = SCA_FDAT,	  /* dirfd */ },
+		   [1] = { .scnprintf = SCA_FILENAME,	  /* pathname */ },
+		   [2] = { .scnprintf = SCA_ACCMODE,	  /* mode */ }, }, },
+	{ .name     = "faccessat2",
+	  .arg = { [0] = { .scnprintf = SCA_FDAT,	  /* dirfd */ },
+		   [1] = { .scnprintf = SCA_FILENAME,	  /* pathname */ },
+		   [2] = { .scnprintf = SCA_ACCMODE,	  /* mode */ },
+		   [3] = { .scnprintf = SCA_FACCESSAT2_FLAGS, /* flags */ }, }, },
 	{ .name	    = "fchmodat",
 	  .arg = { [0] = { .scnprintf = SCA_FDAT, /* fd */ }, }, },
 	{ .name	    = "fchownat",
diff --git a/tools/perf/trace/beauty/beauty.h b/tools/perf/trace/beauty/beauty.h
index c94ae8701bc65a2f..78d10d92d351f8e2 100644
--- a/tools/perf/trace/beauty/beauty.h
+++ b/tools/perf/trace/beauty/beauty.h
@@ -237,6 +237,9 @@ size_t syscall_arg__scnprintf_socket_level(char *bf, size_t size, struct syscall
 size_t syscall_arg__scnprintf_fs_at_flags(char *bf, size_t size, struct syscall_arg *arg);
 #define SCA_FS_AT_FLAGS syscall_arg__scnprintf_fs_at_flags
 
+size_t syscall_arg__scnprintf_faccessat2_flags(char *bf, size_t size, struct syscall_arg *arg);
+#define SCA_FACCESSAT2_FLAGS syscall_arg__scnprintf_faccessat2_flags
+
 size_t syscall_arg__scnprintf_statx_mask(char *bf, size_t size, struct syscall_arg *arg);
 #define SCA_STATX_MASK syscall_arg__scnprintf_statx_mask
 
diff --git a/tools/perf/trace/beauty/fs_at_flags.c b/tools/perf/trace/beauty/fs_at_flags.c
index 2a099953d9935782..c1365e8f0b96ef43 100644
--- a/tools/perf/trace/beauty/fs_at_flags.c
+++ b/tools/perf/trace/beauty/fs_at_flags.c
@@ -7,6 +7,7 @@
 
 #include "trace/beauty/beauty.h"
 #include <sys/types.h>
+#include <linux/fcntl.h>
 #include <linux/log2.h>
 
 #include "trace/beauty/generated/fs_at_flags_array.c"
@@ -24,3 +25,26 @@ size_t syscall_arg__scnprintf_fs_at_flags(char *bf, size_t size, struct syscall_
 
 	return fs_at__scnprintf_flags(flags, bf, size, show_prefix);
 }
+
+static size_t faccessat2__scnprintf_flags(unsigned long flags, char *bf, size_t size, bool show_prefix)
+{
+	int printed = 0;
+
+	// AT_EACCESS is the same as AT_REMOVEDIR, that is in fs_at_flags_array,
+	// special case it here.
+	if (flags & AT_EACCESS) {
+		flags &= ~AT_EACCESS;
+		printed += scnprintf(bf + printed, size - printed, "%sEACCESS%s",
+				     show_prefix ? strarray__fs_at_flags.prefix : "", flags ? "|" : "");
+	}
+
+	return strarray__scnprintf_flags(&strarray__fs_at_flags, bf + printed, size - printed, show_prefix, flags);
+}
+
+size_t syscall_arg__scnprintf_faccessat2_flags(char *bf, size_t size, struct syscall_arg *arg)
+{
+	bool show_prefix = arg->show_string_prefix;
+	int flags = arg->val;
+
+	return faccessat2__scnprintf_flags(flags, bf, size, show_prefix);
+}
-- 
2.44.0


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

* [PATCH 4/5] perf trace: Beautify the 'flags' arg of unlinkat
  2024-03-20 19:31 [PATCH 0/5] More perf trace syscall pretty printing improvements Arnaldo Carvalho de Melo
                   ` (2 preceding siblings ...)
  2024-03-20 19:31 ` [PATCH 3/5] perf beauty: Introduce faccessat2 flags scnprintf routine Arnaldo Carvalho de Melo
@ 2024-03-20 19:31 ` Arnaldo Carvalho de Melo
  2024-03-21  1:50   ` Ian Rogers
  2024-03-20 19:31 ` [PATCH 5/5] perf trace: Fix 'newfstatat'/'fstatat' argument pretty printing Arnaldo Carvalho de Melo
  4 siblings, 1 reply; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-03-20 19:31 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Jiri Olsa, Namhyung Kim, Adrian Hunter, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Reusing the fs_at_flags array done for the 'stat' syscall.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Ian Rogers <irogers@google.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-trace.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 58546e8af9fcf481..ef0dfffd99fdf3cc 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -1171,7 +1171,9 @@ static const struct syscall_fmt syscall_fmts[] = {
 	  .arg = { [0] = { .scnprintf = SCA_FILENAME, /* name */ }, }, },
 	{ .name	    = "uname", .alias = "newuname", },
 	{ .name	    = "unlinkat",
-	  .arg = { [0] = { .scnprintf = SCA_FDAT, /* dfd */ }, }, },
+	  .arg = { [0] = { .scnprintf = SCA_FDAT,	  /* dfd */ },
+		   [1] = { .scnprintf = SCA_FILENAME,	  /* pathname */ },
+		   [2] = { .scnprintf = SCA_FS_AT_FLAGS,  /* flags */ }, }, },
 	{ .name	    = "utimensat",
 	  .arg = { [0] = { .scnprintf = SCA_FDAT, /* dirfd */ }, }, },
 	{ .name	    = "wait4",	    .errpid = true,
-- 
2.44.0


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

* [PATCH 5/5] perf trace: Fix 'newfstatat'/'fstatat' argument pretty printing
  2024-03-20 19:31 [PATCH 0/5] More perf trace syscall pretty printing improvements Arnaldo Carvalho de Melo
                   ` (3 preceding siblings ...)
  2024-03-20 19:31 ` [PATCH 4/5] perf trace: Beautify the 'flags' arg of unlinkat Arnaldo Carvalho de Melo
@ 2024-03-20 19:31 ` Arnaldo Carvalho de Melo
  2024-03-21  1:50   ` Ian Rogers
  4 siblings, 1 reply; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-03-20 19:31 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Jiri Olsa, Namhyung Kim, Adrian Hunter, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo

From: Arnaldo Carvalho de Melo <acme@redhat.com>

There were needless two entries, one for 'newfstatat' and another for
'fstatat', keep just one and pretty print its 'flags' argument using the
fs_at_flags scnprintf that is also used by other FS syscalls such as
'stat', now:

  root@number:~# perf trace -e newfstatat --max-events=5
       0.000 ( 0.010 ms): abrt-dump-jour/1400 newfstatat(dfd: 7, filename: "", statbuf: 0x7fff0d127000, flag: EMPTY_PATH) = 0
       0.020 ( 0.003 ms): abrt-dump-jour/1400 newfstatat(dfd: 9, filename: "", statbuf: 0x55752507b0e8, flag: EMPTY_PATH) = 0
       0.039 ( 0.004 ms): abrt-dump-jour/1400 newfstatat(dfd: 19, filename: "", statbuf: 0x557525061378, flag: EMPTY_PATH) = 0
       0.047 ( 0.003 ms): abrt-dump-jour/1400 newfstatat(dfd: 20, filename: "", statbuf: 0x5575250b8cc8, flag: EMPTY_PATH) = 0
       0.053 ( 0.003 ms): abrt-dump-jour/1400 newfstatat(dfd: 22, filename: "", statbuf: 0x5575250535d8, flag: EMPTY_PATH) = 0
  root@number:~#

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Ian Rogers <irogers@google.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-trace.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index ef0dfffd99fdf3cc..d3ec244e692a415e 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -978,7 +978,6 @@ static const struct syscall_fmt syscall_fmts[] = {
 		   [1] = { .scnprintf = SCA_FILENAME,	  /* path */ },
 		   [2] = { .scnprintf = SCA_FSPICK_FLAGS, /* flags */ }, }, },
 	{ .name	    = "fstat", .alias = "newfstat", },
-	{ .name	    = "fstatat", .alias = "newfstatat", },
 	{ .name	    = "futex",
 	  .arg = { [1] = { .scnprintf = SCA_FUTEX_OP, /* op */ },
 		   [5] = { .scnprintf = SCA_FUTEX_VAL3, /* val3 */ }, }, },
@@ -1060,8 +1059,10 @@ static const struct syscall_fmt syscall_fmts[] = {
 	  .arg = { [0] = { .scnprintf = SCA_FDAT, /* dfd */ }, }, },
 	{ .name	    = "nanosleep",
 	  .arg = { [0] = { .scnprintf = SCA_TIMESPEC,  /* req */ }, }, },
-	{ .name	    = "newfstatat",
-	  .arg = { [0] = { .scnprintf = SCA_FDAT, /* dfd */ }, }, },
+	{ .name	    = "newfstatat", .alias = "fstatat",
+	  .arg = { [0] = { .scnprintf = SCA_FDAT,	  /* dirfd */ },
+		   [1] = { .scnprintf = SCA_FILENAME,	  /* pathname */ },
+		   [3] = { .scnprintf = SCA_FS_AT_FLAGS, /* flags */ }, }, },
 	{ .name	    = "open",
 	  .arg = { [1] = { .scnprintf = SCA_OPEN_FLAGS, /* flags */ }, }, },
 	{ .name	    = "open_by_handle_at",
-- 
2.44.0


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

* Re: [PATCH 1/5] perf beauty: Introduce scrape script for various fs syscalls 'flags' arguments
  2024-03-20 19:31 ` [PATCH 1/5] perf beauty: Introduce scrape script for various fs syscalls 'flags' arguments Arnaldo Carvalho de Melo
@ 2024-03-21  1:47   ` Ian Rogers
  2024-03-21 13:35     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 12+ messages in thread
From: Ian Rogers @ 2024-03-21  1:47 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Adrian Hunter, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo

On Wed, Mar 20, 2024 at 12:31 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> It was using the first variation on producing a string representation
> for a binary flag, one that used the system's fcntl.h and preprocessor
> tricks that had to be updated everytime a new flag was introduced.
>
> Use the more recent scrape script + strarray + strarray__scnprintf_flags() combo.
>
>   $ tools/perf/trace/beauty/fs_at_flags.sh
>   static const char *fs_at_flags[] = {
>         [ilog2(0x100) + 1] = "SYMLINK_NOFOLLOW",
>         [ilog2(0x200) + 1] = "REMOVEDIR",
>         [ilog2(0x400) + 1] = "SYMLINK_FOLLOW",
>         [ilog2(0x800) + 1] = "NO_AUTOMOUNT",
>         [ilog2(0x1000) + 1] = "EMPTY_PATH",
>         [ilog2(0x0000) + 1] = "STATX_SYNC_AS_STAT",
>         [ilog2(0x2000) + 1] = "STATX_FORCE_SYNC",
>         [ilog2(0x4000) + 1] = "STATX_DONT_SYNC",
>         [ilog2(0x8000) + 1] = "RECURSIVE",
>         [ilog2(0x80000000) + 1] = "GETATTR_NOSEC",
>   };
>   $
>
> Now we need a copy of uapi/linux/fcntl.h from tools/include/ in the
> scrape only directory tools/perf/trace/beauty/include and will use that
> fs_at_flags array for other fs syscalls.
>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Ian Rogers <irogers@google.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/Makefile.perf                      |   7 +
>  tools/perf/builtin-trace.c                    |   2 +-
>  tools/perf/check-headers.sh                   |   1 +
>  tools/perf/trace/beauty/Build                 |   1 +
>  tools/perf/trace/beauty/beauty.h              |   4 +-
>  tools/perf/trace/beauty/fs_at_flags.c         |  26 ++++
>  tools/perf/trace/beauty/fs_at_flags.sh        |  21 +++
>  .../trace/beauty/include/uapi/linux/fcntl.h   | 123 ++++++++++++++++++
>  tools/perf/trace/beauty/statx.c               |  31 -----
>  9 files changed, 182 insertions(+), 34 deletions(-)
>  create mode 100644 tools/perf/trace/beauty/fs_at_flags.c
>  create mode 100755 tools/perf/trace/beauty/fs_at_flags.sh
>  create mode 100644 tools/perf/trace/beauty/include/uapi/linux/fcntl.h
>
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index ccd2dcbc64f720d2..73d5603450b0a547 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -489,6 +489,12 @@ beauty_ioctl_outdir := $(beauty_outdir)/ioctl
>  # Create output directory if not already present
>  $(shell [ -d '$(beauty_ioctl_outdir)' ] || mkdir -p '$(beauty_ioctl_outdir)')
>
> +fs_at_flags_array := $(beauty_outdir)/fs_at_flags_array.c
> +fs_at_flags_tbl := $(srctree)/tools/perf/trace/beauty/fs_at_flags.sh
> +
> +$(fs_at_flags_array): $(beauty_uapi_linux_dir)/fcntl.h $(fs_at_flags_tbl)
> +       $(Q)$(SHELL) '$(fs_at_flags_tbl)' $(beauty_uapi_linux_dir) > $@
> +

I wonder if rather than update Makefile.perf, we could push more of
the logic into tools/perf/trace/beauty/Build. It would also be nice to
add there the shellcheck logic:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/tests/Build?h=perf-tools-next#n81

>  clone_flags_array := $(beauty_outdir)/clone_flags_array.c
>  clone_flags_tbl := $(srctree)/tools/perf/trace/beauty/clone.sh
>
> @@ -772,6 +778,7 @@ build-dir   = $(or $(__build-dir),.)
>
>  prepare: $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)common-cmds.h archheaders \
>         arm64-sysreg-defs \
> +       $(fs_at_flags_array) \
>         $(clone_flags_array) \
>         $(drm_ioctl_array) \
>         $(fadvise_advice_array) \
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index 8fb032caeaf53288..8417387aafa8295d 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -1144,7 +1144,7 @@ static const struct syscall_fmt syscall_fmts[] = {
>         { .name     = "stat", .alias = "newstat", },
>         { .name     = "statx",
>           .arg = { [0] = { .scnprintf = SCA_FDAT,        /* fdat */ },
> -                  [2] = { .scnprintf = SCA_STATX_FLAGS, /* flags */ } ,
> +                  [2] = { .scnprintf = SCA_FS_AT_FLAGS, /* flags */ } ,
>                    [3] = { .scnprintf = SCA_STATX_MASK,  /* mask */ }, }, },
>         { .name     = "swapoff",
>           .arg = { [0] = { .scnprintf = SCA_FILENAME, /* specialfile */ }, }, },
> diff --git a/tools/perf/check-headers.sh b/tools/perf/check-headers.sh
> index 413c9b747216020f..d23a84fdf3efef78 100755
> --- a/tools/perf/check-headers.sh
> +++ b/tools/perf/check-headers.sh
> @@ -89,6 +89,7 @@ BEAUTY_FILES=(
>    "arch/x86/include/asm/irq_vectors.h"
>    "arch/x86/include/uapi/asm/prctl.h"
>    "include/linux/socket.h"
> +  "include/uapi/linux/fcntl.h"
>    "include/uapi/linux/fs.h"
>    "include/uapi/linux/mount.h"
>    "include/uapi/linux/prctl.h"
> diff --git a/tools/perf/trace/beauty/Build b/tools/perf/trace/beauty/Build
> index d11ce256f5114034..d8ce1b6989832134 100644
> --- a/tools/perf/trace/beauty/Build
> +++ b/tools/perf/trace/beauty/Build
> @@ -1,6 +1,7 @@
>  perf-y += clone.o
>  perf-y += fcntl.o
>  perf-y += flock.o
> +perf-y += fs_at_flags.o
>  perf-y += fsmount.o
>  perf-y += fspick.o
>  ifeq ($(SRCARCH),$(filter $(SRCARCH),x86))
> diff --git a/tools/perf/trace/beauty/beauty.h b/tools/perf/trace/beauty/beauty.h
> index 9feb794f5c6e15f4..c94ae8701bc65a2f 100644
> --- a/tools/perf/trace/beauty/beauty.h
> +++ b/tools/perf/trace/beauty/beauty.h
> @@ -234,8 +234,8 @@ size_t syscall_arg__scnprintf_socket_protocol(char *bf, size_t size, struct sysc
>  size_t syscall_arg__scnprintf_socket_level(char *bf, size_t size, struct syscall_arg *arg);
>  #define SCA_SK_LEVEL syscall_arg__scnprintf_socket_level
>
> -size_t syscall_arg__scnprintf_statx_flags(char *bf, size_t size, struct syscall_arg *arg);
> -#define SCA_STATX_FLAGS syscall_arg__scnprintf_statx_flags
> +size_t syscall_arg__scnprintf_fs_at_flags(char *bf, size_t size, struct syscall_arg *arg);
> +#define SCA_FS_AT_FLAGS syscall_arg__scnprintf_fs_at_flags
>
>  size_t syscall_arg__scnprintf_statx_mask(char *bf, size_t size, struct syscall_arg *arg);
>  #define SCA_STATX_MASK syscall_arg__scnprintf_statx_mask
> diff --git a/tools/perf/trace/beauty/fs_at_flags.c b/tools/perf/trace/beauty/fs_at_flags.c
> new file mode 100644
> index 0000000000000000..2a099953d9935782
> --- /dev/null
> +++ b/tools/perf/trace/beauty/fs_at_flags.c
> @@ -0,0 +1,26 @@
> +// SPDX-License-Identifier: LGPL-2.1
> +/*
> + * trace/beauty/fs_at_flags.c
> + *
> + *  Copyright (C) 2017, Red Hat Inc, Arnaldo Carvalho de Melo <acme@redhat.com>
> + */
> +
> +#include "trace/beauty/beauty.h"
> +#include <sys/types.h>
> +#include <linux/log2.h>
> +
> +#include "trace/beauty/generated/fs_at_flags_array.c"
> +static DEFINE_STRARRAY(fs_at_flags, "AT_");
> +
> +static size_t fs_at__scnprintf_flags(unsigned long flags, char *bf, size_t size, bool show_prefix)
> +{
> +       return strarray__scnprintf_flags(&strarray__fs_at_flags, bf, size, show_prefix, flags);
> +}
> +
> +size_t syscall_arg__scnprintf_fs_at_flags(char *bf, size_t size, struct syscall_arg *arg)
> +{
> +       bool show_prefix = arg->show_string_prefix;
> +       int flags = arg->val;
> +
> +       return fs_at__scnprintf_flags(flags, bf, size, show_prefix);
> +}
> diff --git a/tools/perf/trace/beauty/fs_at_flags.sh b/tools/perf/trace/beauty/fs_at_flags.sh
> new file mode 100755
> index 0000000000000000..456f59addf741062
> --- /dev/null
> +++ b/tools/perf/trace/beauty/fs_at_flags.sh
> @@ -0,0 +1,21 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: LGPL-2.1
> +
> +if [ $# -ne 1 ] ; then
> +       beauty_uapi_linux_dir=tools/perf/trace/beauty/include/uapi/linux/
> +else
> +       beauty_uapi_linux_dir=$1
> +fi
> +
> +linux_fcntl=${beauty_uapi_linux_dir}/fcntl.h
> +
> +printf "static const char *fs_at_flags[] = {\n"
> +regex='^[[:space:]]*#[[:space:]]*define[[:space:]]+AT_([^_]+[[:alnum:]_]+)[[:space:]]+(0x[[:xdigit:]]+)[[:space:]]*.*'
> +# AT_EACCESS is only meaningful to faccessat, so we will special case it there...
> +# AT_STATX_SYNC_TYPE is not a bit, its a mask of AT_STATX_SYNC_AS_STAT, AT_STATX_FORCE_SYNC and AT_STATX_DONT_SYNC
> +grep -E $regex ${linux_fcntl} | \
> +       grep -v AT_EACCESS | \
> +       grep -v AT_STATX_SYNC_TYPE | \
> +       sed -r "s/$regex/\2 \1/g"       | \
> +       xargs printf "\t[ilog2(%s) + 1] = \"%s\",\n"
> +printf "};\n"
> diff --git a/tools/perf/trace/beauty/include/uapi/linux/fcntl.h b/tools/perf/trace/beauty/include/uapi/linux/fcntl.h
> new file mode 100644
> index 0000000000000000..282e90aeb163c028
> --- /dev/null
> +++ b/tools/perf/trace/beauty/include/uapi/linux/fcntl.h
> @@ -0,0 +1,123 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +#ifndef _UAPI_LINUX_FCNTL_H
> +#define _UAPI_LINUX_FCNTL_H
> +
> +#include <asm/fcntl.h>
> +#include <linux/openat2.h>
> +
> +#define F_SETLEASE     (F_LINUX_SPECIFIC_BASE + 0)
> +#define F_GETLEASE     (F_LINUX_SPECIFIC_BASE + 1)
> +
> +/*
> + * Cancel a blocking posix lock; internal use only until we expose an
> + * asynchronous lock api to userspace:
> + */
> +#define F_CANCELLK     (F_LINUX_SPECIFIC_BASE + 5)
> +
> +/* Create a file descriptor with FD_CLOEXEC set. */
> +#define F_DUPFD_CLOEXEC        (F_LINUX_SPECIFIC_BASE + 6)
> +
> +/*
> + * Request nofications on a directory.
> + * See below for events that may be notified.
> + */
> +#define F_NOTIFY       (F_LINUX_SPECIFIC_BASE+2)
> +
> +/*
> + * Set and get of pipe page size array
> + */
> +#define F_SETPIPE_SZ   (F_LINUX_SPECIFIC_BASE + 7)
> +#define F_GETPIPE_SZ   (F_LINUX_SPECIFIC_BASE + 8)
> +
> +/*
> + * Set/Get seals
> + */
> +#define F_ADD_SEALS    (F_LINUX_SPECIFIC_BASE + 9)
> +#define F_GET_SEALS    (F_LINUX_SPECIFIC_BASE + 10)
> +
> +/*
> + * Types of seals
> + */
> +#define F_SEAL_SEAL    0x0001  /* prevent further seals from being set */
> +#define F_SEAL_SHRINK  0x0002  /* prevent file from shrinking */
> +#define F_SEAL_GROW    0x0004  /* prevent file from growing */
> +#define F_SEAL_WRITE   0x0008  /* prevent writes */
> +#define F_SEAL_FUTURE_WRITE    0x0010  /* prevent future writes while mapped */
> +#define F_SEAL_EXEC    0x0020  /* prevent chmod modifying exec bits */
> +/* (1U << 31) is reserved for signed error codes */
> +
> +/*
> + * Set/Get write life time hints. {GET,SET}_RW_HINT operate on the
> + * underlying inode, while {GET,SET}_FILE_RW_HINT operate only on
> + * the specific file.
> + */
> +#define F_GET_RW_HINT          (F_LINUX_SPECIFIC_BASE + 11)
> +#define F_SET_RW_HINT          (F_LINUX_SPECIFIC_BASE + 12)
> +#define F_GET_FILE_RW_HINT     (F_LINUX_SPECIFIC_BASE + 13)
> +#define F_SET_FILE_RW_HINT     (F_LINUX_SPECIFIC_BASE + 14)
> +
> +/*
> + * Valid hint values for F_{GET,SET}_RW_HINT. 0 is "not set", or can be
> + * used to clear any hints previously set.
> + */
> +#define RWH_WRITE_LIFE_NOT_SET 0
> +#define RWH_WRITE_LIFE_NONE    1
> +#define RWH_WRITE_LIFE_SHORT   2
> +#define RWH_WRITE_LIFE_MEDIUM  3
> +#define RWH_WRITE_LIFE_LONG    4
> +#define RWH_WRITE_LIFE_EXTREME 5
> +
> +/*
> + * The originally introduced spelling is remained from the first
> + * versions of the patch set that introduced the feature, see commit
> + * v4.13-rc1~212^2~51.
> + */
> +#define RWF_WRITE_LIFE_NOT_SET RWH_WRITE_LIFE_NOT_SET
> +
> +/*
> + * Types of directory notifications that may be requested.
> + */
> +#define DN_ACCESS      0x00000001      /* File accessed */
> +#define DN_MODIFY      0x00000002      /* File modified */
> +#define DN_CREATE      0x00000004      /* File created */
> +#define DN_DELETE      0x00000008      /* File removed */
> +#define DN_RENAME      0x00000010      /* File renamed */
> +#define DN_ATTRIB      0x00000020      /* File changed attibutes */
> +#define DN_MULTISHOT   0x80000000      /* Don't remove notifier */
> +
> +/*
> + * The constants AT_REMOVEDIR and AT_EACCESS have the same value.  AT_EACCESS is
> + * meaningful only to faccessat, while AT_REMOVEDIR is meaningful only to
> + * unlinkat.  The two functions do completely different things and therefore,
> + * the flags can be allowed to overlap.  For example, passing AT_REMOVEDIR to
> + * faccessat would be undefined behavior and thus treating it equivalent to
> + * AT_EACCESS is valid undefined behavior.
> + */
> +#define AT_FDCWD               -100    /* Special value used to indicate
> +                                           openat should use the current
> +                                           working directory. */
> +#define AT_SYMLINK_NOFOLLOW    0x100   /* Do not follow symbolic links.  */
> +#define AT_EACCESS             0x200   /* Test access permitted for
> +                                           effective IDs, not real IDs.  */
> +#define AT_REMOVEDIR           0x200   /* Remove directory instead of
> +                                           unlinking file.  */
> +#define AT_SYMLINK_FOLLOW      0x400   /* Follow symbolic links.  */
> +#define AT_NO_AUTOMOUNT                0x800   /* Suppress terminal automount traversal */
> +#define AT_EMPTY_PATH          0x1000  /* Allow empty relative pathname */
> +
> +#define AT_STATX_SYNC_TYPE     0x6000  /* Type of synchronisation required from statx() */
> +#define AT_STATX_SYNC_AS_STAT  0x0000  /* - Do whatever stat() does */
> +#define AT_STATX_FORCE_SYNC    0x2000  /* - Force the attributes to be sync'd with the server */
> +#define AT_STATX_DONT_SYNC     0x4000  /* - Don't sync attributes with the server */
> +
> +#define AT_RECURSIVE           0x8000  /* Apply to the entire subtree */
> +
> +/* Flags for name_to_handle_at(2). We reuse AT_ flag space to save bits... */
> +#define AT_HANDLE_FID          AT_REMOVEDIR    /* file handle is needed to
> +                                       compare object identity and may not
> +                                       be usable to open_by_handle_at(2) */
> +#if defined(__KERNEL__)
> +#define AT_GETATTR_NOSEC       0x80000000
> +#endif
> +
> +#endif /* _UAPI_LINUX_FCNTL_H */
> diff --git a/tools/perf/trace/beauty/statx.c b/tools/perf/trace/beauty/statx.c
> index 1f7e34ed4e02be86..4e0059fd02118f9c 100644
> --- a/tools/perf/trace/beauty/statx.c
> +++ b/tools/perf/trace/beauty/statx.c
> @@ -8,7 +8,6 @@
>  #include "trace/beauty/beauty.h"
>  #include <linux/kernel.h>
>  #include <sys/types.h>
> -#include <linux/fcntl.h>
>  #include <linux/stat.h>
>
>  #ifndef STATX_MNT_ID
> @@ -21,36 +20,6 @@
>  #define STATX_MNT_ID_UNIQUE    0x00004000U
>  #endif
>
> -size_t syscall_arg__scnprintf_statx_flags(char *bf, size_t size, struct syscall_arg *arg)
> -{
> -       bool show_prefix = arg->show_string_prefix;
> -       const char *prefix = "AT_";
> -       int printed = 0, flags = arg->val;
> -
> -       if (flags == 0)
> -               return scnprintf(bf, size, "%s%s", show_prefix ? "AT_STATX_" : "", "SYNC_AS_STAT");
> -#define        P_FLAG(n) \
> -       if (flags & AT_##n) { \
> -               printed += scnprintf(bf + printed, size - printed, "%s%s", printed ? "|" : "", show_prefix ? prefix : "", #n); \
> -               flags &= ~AT_##n; \
> -       }
> -
> -       P_FLAG(SYMLINK_NOFOLLOW);
> -       P_FLAG(REMOVEDIR);
> -       P_FLAG(SYMLINK_FOLLOW);
> -       P_FLAG(NO_AUTOMOUNT);
> -       P_FLAG(EMPTY_PATH);
> -       P_FLAG(STATX_FORCE_SYNC);
> -       P_FLAG(STATX_DONT_SYNC);
> -
> -#undef P_FLAG
> -
> -       if (flags)
> -               printed += scnprintf(bf + printed, size - printed, "%s%#x", printed ? "|" : "", flags);
> -
> -       return printed;
> -}
> -
>  size_t syscall_arg__scnprintf_statx_mask(char *bf, size_t size, struct syscall_arg *arg)
>  {
>         bool show_prefix = arg->show_string_prefix;
> --
> 2.44.0
>

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

* Re: [PATCH 2/5] perf beauty: Introduce scrape script for the 'statx' syscall 'mask' argument
  2024-03-20 19:31 ` [PATCH 2/5] perf beauty: Introduce scrape script for the 'statx' syscall 'mask' argument Arnaldo Carvalho de Melo
@ 2024-03-21  1:49   ` Ian Rogers
  0 siblings, 0 replies; 12+ messages in thread
From: Ian Rogers @ 2024-03-21  1:49 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Adrian Hunter, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo

On Wed, Mar 20, 2024 at 12:31 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> It was using the first variation on producing a string representation
> for a binary flag, one that used the system's stat.h and preprocessor
> tricks that had to be updated everytime a new flag was introduced.
>
> Use the more recent scrape script + strarray +
> strarray__scnprintf_flags() combo.
>
>   $ tools/perf/trace/beauty/statx_mask.sh
>   static const char *statx_mask[] = {
>         [ilog2(0x00000001) + 1] = "TYPE",
>         [ilog2(0x00000002) + 1] = "MODE",
>         [ilog2(0x00000004) + 1] = "NLINK",
>         [ilog2(0x00000008) + 1] = "UID",
>         [ilog2(0x00000010) + 1] = "GID",
>         [ilog2(0x00000020) + 1] = "ATIME",
>         [ilog2(0x00000040) + 1] = "MTIME",
>         [ilog2(0x00000080) + 1] = "CTIME",
>         [ilog2(0x00000100) + 1] = "INO",
>         [ilog2(0x00000200) + 1] = "SIZE",
>         [ilog2(0x00000400) + 1] = "BLOCKS",
>         [ilog2(0x00000800) + 1] = "BTIME",
>         [ilog2(0x00001000) + 1] = "MNT_ID",
>         [ilog2(0x00002000) + 1] = "DIOALIGN",
>         [ilog2(0x00004000) + 1] = "MNT_ID_UNIQUE",
>   };
>   $
>
> Now we need a copy of uapi/linux/stat.h from tools/include/ in the
> scrape only directory tools/perf/trace/beauty/include.
>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Ian Rogers <irogers@google.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/Makefile.perf                      |   7 +
>  tools/perf/check-headers.sh                   |   1 +
>  .../trace/beauty/include/uapi/linux/stat.h    | 195 ++++++++++++++++++
>  tools/perf/trace/beauty/statx.c               |  50 +----
>  tools/perf/trace/beauty/statx_mask.sh         |  23 +++
>  5 files changed, 235 insertions(+), 41 deletions(-)
>  create mode 100644 tools/perf/trace/beauty/include/uapi/linux/stat.h
>  create mode 100755 tools/perf/trace/beauty/statx_mask.sh
>
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index 73d5603450b0a547..0d2dbdfc44df3019 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -673,6 +673,12 @@ arch_errno_tbl := $(srctree)/tools/perf/trace/beauty/arch_errno_names.sh
>  $(arch_errno_name_array): $(arch_errno_tbl)
>         $(Q)$(SHELL) '$(arch_errno_tbl)' '$(patsubst -%,,$(CC))' $(arch_errno_hdr_dir) > $@
>
> +statx_mask_array := $(beauty_outdir)/statx_mask_array.c
> +statx_mask_tbl := $(srctree)/tools/perf/trace/beauty/statx_mask.sh
> +
> +$(statx_mask_array): $(beauty_uapi_linux_dir)/stat.h $(statx_mask_tbl)
> +       $(Q)$(SHELL) '$(statx_mask_tbl)' $(beauty_uapi_linux_dir) > $@
> +
>  sync_file_range_arrays := $(beauty_outdir)/sync_file_range_arrays.c
>  sync_file_range_tbls := $(srctree)/tools/perf/trace/beauty/sync_file_range.sh
>
> @@ -807,6 +813,7 @@ prepare: $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)common-cmds.h archheaders \
>         $(x86_arch_prctl_code_array) \
>         $(rename_flags_array) \
>         $(arch_errno_name_array) \
> +       $(statx_mask_array) \
>         $(sync_file_range_arrays) \
>         $(LIBAPI) \
>         $(LIBPERF) \
> diff --git a/tools/perf/check-headers.sh b/tools/perf/check-headers.sh
> index d23a84fdf3efef78..76726a5a7c789273 100755
> --- a/tools/perf/check-headers.sh
> +++ b/tools/perf/check-headers.sh
> @@ -94,6 +94,7 @@ BEAUTY_FILES=(
>    "include/uapi/linux/mount.h"
>    "include/uapi/linux/prctl.h"
>    "include/uapi/linux/sched.h"
> +  "include/uapi/linux/stat.h"
>    "include/uapi/linux/usbdevice_fs.h"
>    "include/uapi/sound/asound.h"
>  )
> diff --git a/tools/perf/trace/beauty/include/uapi/linux/stat.h b/tools/perf/trace/beauty/include/uapi/linux/stat.h
> new file mode 100644
> index 0000000000000000..2f2ee82d55175d05
> --- /dev/null
> +++ b/tools/perf/trace/beauty/include/uapi/linux/stat.h
> @@ -0,0 +1,195 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +#ifndef _UAPI_LINUX_STAT_H
> +#define _UAPI_LINUX_STAT_H
> +
> +#include <linux/types.h>
> +
> +#if defined(__KERNEL__) || !defined(__GLIBC__) || (__GLIBC__ < 2)
> +
> +#define S_IFMT  00170000
> +#define S_IFSOCK 0140000
> +#define S_IFLNK         0120000
> +#define S_IFREG  0100000
> +#define S_IFBLK  0060000
> +#define S_IFDIR  0040000
> +#define S_IFCHR  0020000
> +#define S_IFIFO  0010000
> +#define S_ISUID  0004000
> +#define S_ISGID  0002000
> +#define S_ISVTX  0001000
> +
> +#define S_ISLNK(m)     (((m) & S_IFMT) == S_IFLNK)
> +#define S_ISREG(m)     (((m) & S_IFMT) == S_IFREG)
> +#define S_ISDIR(m)     (((m) & S_IFMT) == S_IFDIR)
> +#define S_ISCHR(m)     (((m) & S_IFMT) == S_IFCHR)
> +#define S_ISBLK(m)     (((m) & S_IFMT) == S_IFBLK)
> +#define S_ISFIFO(m)    (((m) & S_IFMT) == S_IFIFO)
> +#define S_ISSOCK(m)    (((m) & S_IFMT) == S_IFSOCK)
> +
> +#define S_IRWXU 00700
> +#define S_IRUSR 00400
> +#define S_IWUSR 00200
> +#define S_IXUSR 00100
> +
> +#define S_IRWXG 00070
> +#define S_IRGRP 00040
> +#define S_IWGRP 00020
> +#define S_IXGRP 00010
> +
> +#define S_IRWXO 00007
> +#define S_IROTH 00004
> +#define S_IWOTH 00002
> +#define S_IXOTH 00001
> +
> +#endif
> +
> +/*
> + * Timestamp structure for the timestamps in struct statx.
> + *
> + * tv_sec holds the number of seconds before (negative) or after (positive)
> + * 00:00:00 1st January 1970 UTC.
> + *
> + * tv_nsec holds a number of nanoseconds (0..999,999,999) after the tv_sec time.
> + *
> + * __reserved is held in case we need a yet finer resolution.
> + */
> +struct statx_timestamp {
> +       __s64   tv_sec;
> +       __u32   tv_nsec;
> +       __s32   __reserved;
> +};
> +
> +/*
> + * Structures for the extended file attribute retrieval system call
> + * (statx()).
> + *
> + * The caller passes a mask of what they're specifically interested in as a
> + * parameter to statx().  What statx() actually got will be indicated in
> + * st_mask upon return.
> + *
> + * For each bit in the mask argument:
> + *
> + * - if the datum is not supported:
> + *
> + *   - the bit will be cleared, and
> + *
> + *   - the datum will be set to an appropriate fabricated value if one is
> + *     available (eg. CIFS can take a default uid and gid), otherwise
> + *
> + *   - the field will be cleared;
> + *
> + * - otherwise, if explicitly requested:
> + *
> + *   - the datum will be synchronised to the server if AT_STATX_FORCE_SYNC is
> + *     set or if the datum is considered out of date, and
> + *
> + *   - the field will be filled in and the bit will be set;
> + *
> + * - otherwise, if not requested, but available in approximate form without any
> + *   effort, it will be filled in anyway, and the bit will be set upon return
> + *   (it might not be up to date, however, and no attempt will be made to
> + *   synchronise the internal state first);
> + *
> + * - otherwise the field and the bit will be cleared before returning.
> + *
> + * Items in STATX_BASIC_STATS may be marked unavailable on return, but they
> + * will have values installed for compatibility purposes so that stat() and
> + * co. can be emulated in userspace.
> + */
> +struct statx {
> +       /* 0x00 */
> +       __u32   stx_mask;       /* What results were written [uncond] */
> +       __u32   stx_blksize;    /* Preferred general I/O size [uncond] */
> +       __u64   stx_attributes; /* Flags conveying information about the file [uncond] */
> +       /* 0x10 */
> +       __u32   stx_nlink;      /* Number of hard links */
> +       __u32   stx_uid;        /* User ID of owner */
> +       __u32   stx_gid;        /* Group ID of owner */
> +       __u16   stx_mode;       /* File mode */
> +       __u16   __spare0[1];
> +       /* 0x20 */
> +       __u64   stx_ino;        /* Inode number */
> +       __u64   stx_size;       /* File size */
> +       __u64   stx_blocks;     /* Number of 512-byte blocks allocated */
> +       __u64   stx_attributes_mask; /* Mask to show what's supported in stx_attributes */
> +       /* 0x40 */
> +       struct statx_timestamp  stx_atime;      /* Last access time */
> +       struct statx_timestamp  stx_btime;      /* File creation time */
> +       struct statx_timestamp  stx_ctime;      /* Last attribute change time */
> +       struct statx_timestamp  stx_mtime;      /* Last data modification time */
> +       /* 0x80 */
> +       __u32   stx_rdev_major; /* Device ID of special file [if bdev/cdev] */
> +       __u32   stx_rdev_minor;
> +       __u32   stx_dev_major;  /* ID of device containing file [uncond] */
> +       __u32   stx_dev_minor;
> +       /* 0x90 */
> +       __u64   stx_mnt_id;
> +       __u32   stx_dio_mem_align;      /* Memory buffer alignment for direct I/O */
> +       __u32   stx_dio_offset_align;   /* File offset alignment for direct I/O */
> +       /* 0xa0 */
> +       __u64   __spare3[12];   /* Spare space for future expansion */
> +       /* 0x100 */
> +};
> +
> +/*
> + * Flags to be stx_mask
> + *
> + * Query request/result mask for statx() and struct statx::stx_mask.
> + *
> + * These bits should be set in the mask argument of statx() to request
> + * particular items when calling statx().
> + */
> +#define STATX_TYPE             0x00000001U     /* Want/got stx_mode & S_IFMT */
> +#define STATX_MODE             0x00000002U     /* Want/got stx_mode & ~S_IFMT */
> +#define STATX_NLINK            0x00000004U     /* Want/got stx_nlink */
> +#define STATX_UID              0x00000008U     /* Want/got stx_uid */
> +#define STATX_GID              0x00000010U     /* Want/got stx_gid */
> +#define STATX_ATIME            0x00000020U     /* Want/got stx_atime */
> +#define STATX_MTIME            0x00000040U     /* Want/got stx_mtime */
> +#define STATX_CTIME            0x00000080U     /* Want/got stx_ctime */
> +#define STATX_INO              0x00000100U     /* Want/got stx_ino */
> +#define STATX_SIZE             0x00000200U     /* Want/got stx_size */
> +#define STATX_BLOCKS           0x00000400U     /* Want/got stx_blocks */
> +#define STATX_BASIC_STATS      0x000007ffU     /* The stuff in the normal stat struct */
> +#define STATX_BTIME            0x00000800U     /* Want/got stx_btime */
> +#define STATX_MNT_ID           0x00001000U     /* Got stx_mnt_id */
> +#define STATX_DIOALIGN         0x00002000U     /* Want/got direct I/O alignment info */
> +#define STATX_MNT_ID_UNIQUE    0x00004000U     /* Want/got extended stx_mount_id */
> +
> +#define STATX__RESERVED                0x80000000U     /* Reserved for future struct statx expansion */
> +
> +#ifndef __KERNEL__
> +/*
> + * This is deprecated, and shall remain the same value in the future.  To avoid
> + * confusion please use the equivalent (STATX_BASIC_STATS | STATX_BTIME)
> + * instead.
> + */
> +#define STATX_ALL              0x00000fffU
> +#endif
> +
> +/*
> + * Attributes to be found in stx_attributes and masked in stx_attributes_mask.
> + *
> + * These give information about the features or the state of a file that might
> + * be of use to ordinary userspace programs such as GUIs or ls rather than
> + * specialised tools.
> + *
> + * Note that the flags marked [I] correspond to the FS_IOC_SETFLAGS flags
> + * semantically.  Where possible, the numerical value is picked to correspond
> + * also.  Note that the DAX attribute indicates that the file is in the CPU
> + * direct access state.  It does not correspond to the per-inode flag that
> + * some filesystems support.
> + *
> + */
> +#define STATX_ATTR_COMPRESSED          0x00000004 /* [I] File is compressed by the fs */
> +#define STATX_ATTR_IMMUTABLE           0x00000010 /* [I] File is marked immutable */
> +#define STATX_ATTR_APPEND              0x00000020 /* [I] File is append-only */
> +#define STATX_ATTR_NODUMP              0x00000040 /* [I] File is not to be dumped */
> +#define STATX_ATTR_ENCRYPTED           0x00000800 /* [I] File requires key to decrypt in fs */
> +#define STATX_ATTR_AUTOMOUNT           0x00001000 /* Dir: Automount trigger */
> +#define STATX_ATTR_MOUNT_ROOT          0x00002000 /* Root of a mount */
> +#define STATX_ATTR_VERITY              0x00100000 /* [I] Verity protected file */
> +#define STATX_ATTR_DAX                 0x00200000 /* File is currently in DAX state */
> +
> +
> +#endif /* _UAPI_LINUX_STAT_H */
> diff --git a/tools/perf/trace/beauty/statx.c b/tools/perf/trace/beauty/statx.c
> index 4e0059fd02118f9c..24843e614b935f3a 100644
> --- a/tools/perf/trace/beauty/statx.c
> +++ b/tools/perf/trace/beauty/statx.c
> @@ -6,52 +6,20 @@
>   */
>
>  #include "trace/beauty/beauty.h"
> -#include <linux/kernel.h>
>  #include <sys/types.h>
> -#include <linux/stat.h>
> +#include <linux/log2.h>
>
> -#ifndef STATX_MNT_ID
> -#define STATX_MNT_ID           0x00001000U
> -#endif
> -#ifndef STATX_DIOALIGN
> -#define STATX_DIOALIGN         0x00002000U
> -#endif
> -#ifndef STATX_MNT_ID_UNIQUE
> -#define STATX_MNT_ID_UNIQUE    0x00004000U
> -#endif
> +static size_t statx__scnprintf_mask(unsigned long mask, char *bf, size_t size, bool show_prefix)
> +{
> +       #include "trace/beauty/generated/statx_mask_array.c"
> +       static DEFINE_STRARRAY(statx_mask, "STATX_");
> +       return strarray__scnprintf_flags(&strarray__statx_mask, bf, size, show_prefix, mask);
> +}
>
>  size_t syscall_arg__scnprintf_statx_mask(char *bf, size_t size, struct syscall_arg *arg)
>  {
>         bool show_prefix = arg->show_string_prefix;
> -       const char *prefix = "STATX_";
> -       int printed = 0, flags = arg->val;
> -
> -#define        P_FLAG(n) \
> -       if (flags & STATX_##n) { \
> -               printed += scnprintf(bf + printed, size - printed, "%s%s", printed ? "|" : "", show_prefix ? prefix : "", #n); \
> -               flags &= ~STATX_##n; \
> -       }
> -
> -       P_FLAG(TYPE);
> -       P_FLAG(MODE);
> -       P_FLAG(NLINK);
> -       P_FLAG(UID);
> -       P_FLAG(GID);
> -       P_FLAG(ATIME);
> -       P_FLAG(MTIME);
> -       P_FLAG(CTIME);
> -       P_FLAG(INO);
> -       P_FLAG(SIZE);
> -       P_FLAG(BLOCKS);
> -       P_FLAG(BTIME);
> -       P_FLAG(MNT_ID);
> -       P_FLAG(DIOALIGN);
> -       P_FLAG(MNT_ID_UNIQUE);
> -
> -#undef P_FLAG
> -
> -       if (flags)
> -               printed += scnprintf(bf + printed, size - printed, "%s%#x", printed ? "|" : "", flags);
> +       int mask = arg->val;
>
> -       return printed;
> +       return statx__scnprintf_mask(mask, bf, size, show_prefix);
>  }
> diff --git a/tools/perf/trace/beauty/statx_mask.sh b/tools/perf/trace/beauty/statx_mask.sh
> new file mode 100755
> index 0000000000000000..18c802ed0c71578f
> --- /dev/null
> +++ b/tools/perf/trace/beauty/statx_mask.sh
> @@ -0,0 +1,23 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: LGPL-2.1
> +
> +if [ $# -ne 1 ] ; then
> +       beauty_uapi_linux_dir=tools/perf/trace/beauty/include/uapi/linux/
> +else
> +       beauty_uapi_linux_dir=$1
> +fi
> +
> +linux_stat=${beauty_uapi_linux_dir}/stat.h
> +
> +printf "static const char *statx_mask[] = {\n"
> +regex='^[[:space:]]*#[[:space:]]*define[[:space:]]+STATX_([^_]+[[:alnum:]_]+)[[:space:]]+(0x[[:xdigit:]]+)[[:space:]]*.*'
> +# STATX_BASIC_STATS its a bitmask formed by the mask in the normal stat struct
> +# STATX_ALL is another bitmask and deprecated
> +# STATX_ATTR_*: Attributes to be found in stx_attributes and masked in stx_attributes_mask
> +grep -E $regex ${linux_stat} | \
> +       grep -v STATX_ALL | \
> +       grep -v STATX_BASIC_STATS | \
> +       grep -v '\<STATX_ATTR_' | \
> +       sed -r "s/$regex/\2 \1/g"       | \
> +       xargs printf "\t[ilog2(%s) + 1] = \"%s\",\n"
> +printf "};\n"
> --
> 2.44.0
>

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

* Re: [PATCH 3/5] perf beauty: Introduce faccessat2 flags scnprintf routine
  2024-03-20 19:31 ` [PATCH 3/5] perf beauty: Introduce faccessat2 flags scnprintf routine Arnaldo Carvalho de Melo
@ 2024-03-21  1:50   ` Ian Rogers
  0 siblings, 0 replies; 12+ messages in thread
From: Ian Rogers @ 2024-03-21  1:50 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Adrian Hunter, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo

On Wed, Mar 20, 2024 at 12:31 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> The fsaccessat and fsaccessat2 now have beautifiers for its arguments.
>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Ian Rogers <irogers@google.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/builtin-trace.c            |  9 +++++++++
>  tools/perf/trace/beauty/beauty.h      |  3 +++
>  tools/perf/trace/beauty/fs_at_flags.c | 24 ++++++++++++++++++++++++
>  3 files changed, 36 insertions(+)
>
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index 8417387aafa8295d..58546e8af9fcf481 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -947,6 +947,15 @@ static const struct syscall_fmt syscall_fmts[] = {
>           .arg = { [1] = STRARRAY(op, epoll_ctl_ops), }, },
>         { .name     = "eventfd2",
>           .arg = { [1] = { .scnprintf = SCA_EFD_FLAGS, /* flags */ }, }, },
> +       { .name     = "faccessat",
> +         .arg = { [0] = { .scnprintf = SCA_FDAT,         /* dirfd */ },
> +                  [1] = { .scnprintf = SCA_FILENAME,     /* pathname */ },
> +                  [2] = { .scnprintf = SCA_ACCMODE,      /* mode */ }, }, },
> +       { .name     = "faccessat2",
> +         .arg = { [0] = { .scnprintf = SCA_FDAT,         /* dirfd */ },
> +                  [1] = { .scnprintf = SCA_FILENAME,     /* pathname */ },
> +                  [2] = { .scnprintf = SCA_ACCMODE,      /* mode */ },
> +                  [3] = { .scnprintf = SCA_FACCESSAT2_FLAGS, /* flags */ }, }, },
>         { .name     = "fchmodat",
>           .arg = { [0] = { .scnprintf = SCA_FDAT, /* fd */ }, }, },
>         { .name     = "fchownat",
> diff --git a/tools/perf/trace/beauty/beauty.h b/tools/perf/trace/beauty/beauty.h
> index c94ae8701bc65a2f..78d10d92d351f8e2 100644
> --- a/tools/perf/trace/beauty/beauty.h
> +++ b/tools/perf/trace/beauty/beauty.h
> @@ -237,6 +237,9 @@ size_t syscall_arg__scnprintf_socket_level(char *bf, size_t size, struct syscall
>  size_t syscall_arg__scnprintf_fs_at_flags(char *bf, size_t size, struct syscall_arg *arg);
>  #define SCA_FS_AT_FLAGS syscall_arg__scnprintf_fs_at_flags
>
> +size_t syscall_arg__scnprintf_faccessat2_flags(char *bf, size_t size, struct syscall_arg *arg);
> +#define SCA_FACCESSAT2_FLAGS syscall_arg__scnprintf_faccessat2_flags
> +
>  size_t syscall_arg__scnprintf_statx_mask(char *bf, size_t size, struct syscall_arg *arg);
>  #define SCA_STATX_MASK syscall_arg__scnprintf_statx_mask
>
> diff --git a/tools/perf/trace/beauty/fs_at_flags.c b/tools/perf/trace/beauty/fs_at_flags.c
> index 2a099953d9935782..c1365e8f0b96ef43 100644
> --- a/tools/perf/trace/beauty/fs_at_flags.c
> +++ b/tools/perf/trace/beauty/fs_at_flags.c
> @@ -7,6 +7,7 @@
>
>  #include "trace/beauty/beauty.h"
>  #include <sys/types.h>
> +#include <linux/fcntl.h>
>  #include <linux/log2.h>
>
>  #include "trace/beauty/generated/fs_at_flags_array.c"
> @@ -24,3 +25,26 @@ size_t syscall_arg__scnprintf_fs_at_flags(char *bf, size_t size, struct syscall_
>
>         return fs_at__scnprintf_flags(flags, bf, size, show_prefix);
>  }
> +
> +static size_t faccessat2__scnprintf_flags(unsigned long flags, char *bf, size_t size, bool show_prefix)
> +{
> +       int printed = 0;
> +
> +       // AT_EACCESS is the same as AT_REMOVEDIR, that is in fs_at_flags_array,
> +       // special case it here.
> +       if (flags & AT_EACCESS) {
> +               flags &= ~AT_EACCESS;
> +               printed += scnprintf(bf + printed, size - printed, "%sEACCESS%s",
> +                                    show_prefix ? strarray__fs_at_flags.prefix : "", flags ? "|" : "");
> +       }
> +
> +       return strarray__scnprintf_flags(&strarray__fs_at_flags, bf + printed, size - printed, show_prefix, flags);
> +}
> +
> +size_t syscall_arg__scnprintf_faccessat2_flags(char *bf, size_t size, struct syscall_arg *arg)
> +{
> +       bool show_prefix = arg->show_string_prefix;
> +       int flags = arg->val;
> +
> +       return faccessat2__scnprintf_flags(flags, bf, size, show_prefix);
> +}
> --
> 2.44.0
>

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

* Re: [PATCH 4/5] perf trace: Beautify the 'flags' arg of unlinkat
  2024-03-20 19:31 ` [PATCH 4/5] perf trace: Beautify the 'flags' arg of unlinkat Arnaldo Carvalho de Melo
@ 2024-03-21  1:50   ` Ian Rogers
  0 siblings, 0 replies; 12+ messages in thread
From: Ian Rogers @ 2024-03-21  1:50 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Adrian Hunter, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo

On Wed, Mar 20, 2024 at 12:31 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> Reusing the fs_at_flags array done for the 'stat' syscall.
>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Ian Rogers <irogers@google.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/builtin-trace.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index 58546e8af9fcf481..ef0dfffd99fdf3cc 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -1171,7 +1171,9 @@ static const struct syscall_fmt syscall_fmts[] = {
>           .arg = { [0] = { .scnprintf = SCA_FILENAME, /* name */ }, }, },
>         { .name     = "uname", .alias = "newuname", },
>         { .name     = "unlinkat",
> -         .arg = { [0] = { .scnprintf = SCA_FDAT, /* dfd */ }, }, },
> +         .arg = { [0] = { .scnprintf = SCA_FDAT,         /* dfd */ },
> +                  [1] = { .scnprintf = SCA_FILENAME,     /* pathname */ },
> +                  [2] = { .scnprintf = SCA_FS_AT_FLAGS,  /* flags */ }, }, },
>         { .name     = "utimensat",
>           .arg = { [0] = { .scnprintf = SCA_FDAT, /* dirfd */ }, }, },
>         { .name     = "wait4",      .errpid = true,
> --
> 2.44.0
>

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

* Re: [PATCH 5/5] perf trace: Fix 'newfstatat'/'fstatat' argument pretty printing
  2024-03-20 19:31 ` [PATCH 5/5] perf trace: Fix 'newfstatat'/'fstatat' argument pretty printing Arnaldo Carvalho de Melo
@ 2024-03-21  1:50   ` Ian Rogers
  0 siblings, 0 replies; 12+ messages in thread
From: Ian Rogers @ 2024-03-21  1:50 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Adrian Hunter, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo

On Wed, Mar 20, 2024 at 12:31 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> There were needless two entries, one for 'newfstatat' and another for
> 'fstatat', keep just one and pretty print its 'flags' argument using the
> fs_at_flags scnprintf that is also used by other FS syscalls such as
> 'stat', now:
>
>   root@number:~# perf trace -e newfstatat --max-events=5
>        0.000 ( 0.010 ms): abrt-dump-jour/1400 newfstatat(dfd: 7, filename: "", statbuf: 0x7fff0d127000, flag: EMPTY_PATH) = 0
>        0.020 ( 0.003 ms): abrt-dump-jour/1400 newfstatat(dfd: 9, filename: "", statbuf: 0x55752507b0e8, flag: EMPTY_PATH) = 0
>        0.039 ( 0.004 ms): abrt-dump-jour/1400 newfstatat(dfd: 19, filename: "", statbuf: 0x557525061378, flag: EMPTY_PATH) = 0
>        0.047 ( 0.003 ms): abrt-dump-jour/1400 newfstatat(dfd: 20, filename: "", statbuf: 0x5575250b8cc8, flag: EMPTY_PATH) = 0
>        0.053 ( 0.003 ms): abrt-dump-jour/1400 newfstatat(dfd: 22, filename: "", statbuf: 0x5575250535d8, flag: EMPTY_PATH) = 0
>   root@number:~#
>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Ian Rogers <irogers@google.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/builtin-trace.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index ef0dfffd99fdf3cc..d3ec244e692a415e 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -978,7 +978,6 @@ static const struct syscall_fmt syscall_fmts[] = {
>                    [1] = { .scnprintf = SCA_FILENAME,     /* path */ },
>                    [2] = { .scnprintf = SCA_FSPICK_FLAGS, /* flags */ }, }, },
>         { .name     = "fstat", .alias = "newfstat", },
> -       { .name     = "fstatat", .alias = "newfstatat", },
>         { .name     = "futex",
>           .arg = { [1] = { .scnprintf = SCA_FUTEX_OP, /* op */ },
>                    [5] = { .scnprintf = SCA_FUTEX_VAL3, /* val3 */ }, }, },
> @@ -1060,8 +1059,10 @@ static const struct syscall_fmt syscall_fmts[] = {
>           .arg = { [0] = { .scnprintf = SCA_FDAT, /* dfd */ }, }, },
>         { .name     = "nanosleep",
>           .arg = { [0] = { .scnprintf = SCA_TIMESPEC,  /* req */ }, }, },
> -       { .name     = "newfstatat",
> -         .arg = { [0] = { .scnprintf = SCA_FDAT, /* dfd */ }, }, },
> +       { .name     = "newfstatat", .alias = "fstatat",
> +         .arg = { [0] = { .scnprintf = SCA_FDAT,         /* dirfd */ },
> +                  [1] = { .scnprintf = SCA_FILENAME,     /* pathname */ },
> +                  [3] = { .scnprintf = SCA_FS_AT_FLAGS, /* flags */ }, }, },
>         { .name     = "open",
>           .arg = { [1] = { .scnprintf = SCA_OPEN_FLAGS, /* flags */ }, }, },
>         { .name     = "open_by_handle_at",
> --
> 2.44.0
>

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

* Re: [PATCH 1/5] perf beauty: Introduce scrape script for various fs syscalls 'flags' arguments
  2024-03-21  1:47   ` Ian Rogers
@ 2024-03-21 13:35     ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-03-21 13:35 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Jiri Olsa, Namhyung Kim, Adrian Hunter, linux-kernel,
	linux-perf-users, Arnaldo Carvalho de Melo

On Wed, Mar 20, 2024 at 06:47:28PM -0700, Ian Rogers wrote:
> On Wed, Mar 20, 2024 at 12:31 PM Arnaldo Carvalho de Melo
> > +++ b/tools/perf/Makefile.perf
> > @@ -489,6 +489,12 @@ beauty_ioctl_outdir := $(beauty_outdir)/ioctl
> >  # Create output directory if not already present
> >  $(shell [ -d '$(beauty_ioctl_outdir)' ] || mkdir -p '$(beauty_ioctl_outdir)')
> >
> > +fs_at_flags_array := $(beauty_outdir)/fs_at_flags_array.c
> > +fs_at_flags_tbl := $(srctree)/tools/perf/trace/beauty/fs_at_flags.sh
> > +
> > +$(fs_at_flags_array): $(beauty_uapi_linux_dir)/fcntl.h $(fs_at_flags_tbl)
> > +       $(Q)$(SHELL) '$(fs_at_flags_tbl)' $(beauty_uapi_linux_dir) > $@
> > +
> 
> I wonder if rather than update Makefile.perf, we could push more of
> the logic into tools/perf/trace/beauty/Build. It would also be nice to
> add there the shellcheck logic:
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/tests/Build?h=perf-tools-next#n81

Sure, I thought about consolidating lots of boilerplate there and also
to move it to tools/perf/trace/beauty/Build. Will see if I can do it in
this series at some point.

Thanks for reviewing the patches,

- Arnaldo

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

end of thread, other threads:[~2024-03-21 13:35 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-20 19:31 [PATCH 0/5] More perf trace syscall pretty printing improvements Arnaldo Carvalho de Melo
2024-03-20 19:31 ` [PATCH 1/5] perf beauty: Introduce scrape script for various fs syscalls 'flags' arguments Arnaldo Carvalho de Melo
2024-03-21  1:47   ` Ian Rogers
2024-03-21 13:35     ` Arnaldo Carvalho de Melo
2024-03-20 19:31 ` [PATCH 2/5] perf beauty: Introduce scrape script for the 'statx' syscall 'mask' argument Arnaldo Carvalho de Melo
2024-03-21  1:49   ` Ian Rogers
2024-03-20 19:31 ` [PATCH 3/5] perf beauty: Introduce faccessat2 flags scnprintf routine Arnaldo Carvalho de Melo
2024-03-21  1:50   ` Ian Rogers
2024-03-20 19:31 ` [PATCH 4/5] perf trace: Beautify the 'flags' arg of unlinkat Arnaldo Carvalho de Melo
2024-03-21  1:50   ` Ian Rogers
2024-03-20 19:31 ` [PATCH 5/5] perf trace: Fix 'newfstatat'/'fstatat' argument pretty printing Arnaldo Carvalho de Melo
2024-03-21  1:50   ` Ian Rogers

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