[PATCH v6 1/2] perf: Reveal PMU type in fdinfo

[PATCH v6 1/2] perf: Reveal PMU type in fdinfo

[Chun-Tse Shao]

It gives useful info on knowing which PMUs are reserved by this process.
Also add config which would be useful.
Testing cycles:

  $ ./perf stat -e cycles &
  $ cat /proc/`pidof perf`/fdinfo/3
  pos:    0
  flags:  02000002
  mnt_id: 16
  ino:    3081
  perf_event_attr.type:   0
  perf_event_attr.config: 0x0
  perf_event_attr.config1:        0x0
  perf_event_attr.config2:        0x0
  perf_event_attr.config3:        0x0

Testing L1-dcache-load-misses:

  $ ./perf stat -e L1-dcache-load-misses &
  $ cat /proc/`pidof perf`/fdinfo/3
  pos:    0
  flags:  02000002
  mnt_id: 16
  ino:    1072
  perf_event_attr.type:   3
  perf_event_attr.config: 0x10000
  perf_event_attr.config1:        0x0
  perf_event_attr.config2:        0x0
  perf_event_attr.config3:        0x0

Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: Chun-Tse Shao <ctshao@google.com>
---
v6:
  Rebase.

v5: lore.kernel.org/20251121230951.3240124-1-ctshao@google.com/
  Added config1, config2 and config3 and print output in hex.

v4:
  lore.kernel.org/20250603181634.1362626-1-ctshao@google.com/
  Removed the first patch in v3 since it is merged.
  Retested to make sure the patch still work.

v3: lore.kernel.org/20241106003007.2112584-3-ctshao@google.com/

 kernel/events/core.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 6d1f8bad7e1c..d88947e9f774 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -58,6 +58,7 @@
 #include <linux/percpu-rwsem.h>
 #include <linux/unwind_deferred.h>
 #include <linux/kvm_types.h>
+#include <linux/seq_file.h>

 #include "internal.h"

@@ -7506,6 +7507,20 @@ static int perf_fasync(int fd, struct file *filp, int on)
 	return 0;
 }

+static void perf_show_fdinfo(struct seq_file *m, struct file *f)
+{
+	struct perf_event *event = f->private_data;
+
+	seq_printf(m, "perf_event_attr.type:\t%u\n", event->orig_type);
+	seq_printf(m, "perf_event_attr.config:\t0x%llx\n", (unsigned long long)event->attr.config);
+	seq_printf(m, "perf_event_attr.config1:\t0x%llx\n",
+		   (unsigned long long)event->attr.config1);
+	seq_printf(m, "perf_event_attr.config2:\t0x%llx\n",
+		   (unsigned long long)event->attr.config2);
+	seq_printf(m, "perf_event_attr.config3:\t0x%llx\n",
+		   (unsigned long long)event->attr.config3);
+}
+
 static const struct file_operations perf_fops = {
 	.release		= perf_release,
 	.read			= perf_read,
@@ -7514,6 +7529,7 @@ static const struct file_operations perf_fops = {
 	.compat_ioctl		= perf_compat_ioctl,
 	.mmap			= perf_mmap,
 	.fasync			= perf_fasync,
+	.show_fdinfo		= perf_show_fdinfo,
 };

 /*
--
2.54.0.746.g67dd491aae-goog

[PATCH v6 2/2] perf evsel: Find process with busy PMUs for EBUSY

[Chun-Tse Shao]

It parses fdinfo with PMU type, comparing with the event which failed to
open, and report the processes causing EBUSY error.

Testing cycles and intel_pt//

  $ ./perf stat -e cycles &
  [1] 55569
  $ ./perf stat -e intel_pt// &
  [2] 55683
  $ ./perf stat -e intel_pt//
  Error:
  The PMU intel_pt counters are busy and in use by another process.
  Possible processes:
  55683 ./perf stat -e intel_pt//

Only perf with intel_pt was reported.

Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: Chun-Tse Shao <ctshao@google.com>
---
 tools/perf/util/evsel.c | 79 +++++++++++++++++++++++++++++------------
 1 file changed, 57 insertions(+), 22 deletions(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 2ee87fd84d3e..f6282099320e 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -3928,7 +3928,8 @@ static bool find_process(const char *name)
 	return ret ? false : true;
 }
 
-static int dump_perf_event_processes(char *msg, size_t size)
+static int dump_perf_event_processes(const struct perf_event_attr *failed_attr,
+				     char *msg, size_t size)
 {
 	DIR *proc_dir;
 	struct dirent *proc_entry;
@@ -3969,29 +3970,61 @@ static int dump_perf_event_processes(char *msg, size_t size)
 				continue;
 			/* Take care as readlink doesn't null terminate the string. */
 			if (!strncmp(buf, "anon_inode:[perf_event]", link_size)) {
-				int cmdline_fd;
-				ssize_t cmdline_size;
-
-				scnprintf(buf, sizeof(buf), "%s/cmdline", proc_entry->d_name);
-				cmdline_fd = openat(dirfd(proc_dir), buf, O_RDONLY);
-				if (cmdline_fd == -1)
-					continue;
-				cmdline_size = read(cmdline_fd, buf, sizeof(buf) - 1);
-				close(cmdline_fd);
-				if (cmdline_size < 0)
+				int fdinfo_fd;
+				ssize_t fdinfo_size;
+				char *line;
+				u32 perf_event_type = UINT32_MAX;
+
+				/* Let's check the PMU type reserved by this process */
+				scnprintf(buf, sizeof(buf), "%s/fdinfo/%s",
+					  proc_entry->d_name, fd_entry->d_name);
+				fdinfo_fd = openat(dirfd(proc_dir), buf, O_RDONLY);
+				fdinfo_size = read(fdinfo_fd, buf, sizeof(buf) - 1);
+				if (fdinfo_size < 0)
 					continue;
-				buf[cmdline_size] = '\0';
-				for (ssize_t i = 0; i < cmdline_size; i++) {
-					if (buf[i] == '\0')
-						buf[i] = ' ';
+				buf[fdinfo_size] = '\0';
+
+				line = strtok(buf, "\n");
+				while (line != NULL) {
+					if (sscanf(line,
+						   "perf_event_attr.type:\t%u",
+						   &perf_event_type) == 1)
+						break;
+					line = strtok(NULL, "\n");
 				}
 
-				if (printed == 0)
-					printed += scnprintf(msg, size, "Possible processes:\n");
-
-				printed += scnprintf(msg + printed, size - printed,
-						"%s %s\n", proc_entry->d_name, buf);
-				break;
+				/* Report the process which reserves the conflicted PMU. */
+				/* If fdinfo does not contain PMU type, report it too. */
+				if (perf_event_type == failed_attr->type ||
+				    perf_event_type == UINT32_MAX) {
+					int cmdline_fd;
+					ssize_t cmdline_size;
+
+					scnprintf(buf, sizeof(buf),
+						  "%s/cmdline",
+						  proc_entry->d_name);
+					cmdline_fd = openat(dirfd(proc_dir), buf, O_RDONLY);
+					if (cmdline_fd == -1)
+						continue;
+					cmdline_size = read(cmdline_fd, buf, sizeof(buf) - 1);
+					close(cmdline_fd);
+					if (cmdline_size < 0)
+						continue;
+					buf[cmdline_size] = '\0';
+					for (ssize_t i = 0; i < cmdline_size; i++) {
+						if (buf[i] == '\0')
+							buf[i] = ' ';
+					}
+
+					if (printed == 0)
+						printed += scnprintf(
+							msg, size,
+							"Possible processes:\n");
+
+					printed += scnprintf(msg + printed, size - printed,
+							"%s %s\n", proc_entry->d_name, buf);
+					break;
+				}
 			}
 		}
 		closedir(fd_dir);
@@ -4109,7 +4142,9 @@ int evsel__open_strerror(struct evsel *evsel, struct target *target,
 			msg, size,
 			"The PMU %s counters are busy and in use by another process.\n",
 			evsel->pmu ? evsel->pmu->name : "");
-		return printed + dump_perf_event_processes(msg + printed, size - printed);
+		return printed + dump_perf_event_processes(&evsel->core.attr,
+							   msg + printed,
+							   size - printed);
 		break;
 	case EINVAL:
 		if (evsel->core.attr.sample_type & PERF_SAMPLE_CODE_PAGE_SIZE && perf_missing_features.code_page_size)
-- 
2.54.0.746.g67dd491aae-goog

Re: [PATCH v6 2/2] perf evsel: Find process with busy PMUs for EBUSY

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The file descriptor `fdinfo_fd` is permanently leaked on all successful open paths.
--

commit 89c8fa0a46a0a7739ea59073f27c6161911eb75b
Author: Chun-Tse Shao <ctshao@google.com>

perf evsel: Find process with busy PMUs for EBUSY

This commit parses the fdinfo of perf events to extract the PMU type.
It then compares this type with the failed event to accurately report
the specific processes that are causing an EBUSY error.

> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 2ee87fd84d3ea..f6282099320ee 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
[ ... ]
> @@ -3969,29 +3970,61 @@ static int dump_perf_event_processes(char *msg, size_t size)
>  				continue;
>  			/* Take care as readlink doesn't null terminate the string. */
>  			if (!strncmp(buf, "anon_inode:[perf_event]", link_size)) {
> -				int cmdline_fd;
> -				ssize_t cmdline_size;
> -
> -				scnprintf(buf, sizeof(buf), "%s/cmdline", proc_entry->d_name);
> -				cmdline_fd = openat(dirfd(proc_dir), buf, O_RDONLY);
> -				if (cmdline_fd == -1)
> -					continue;
> -				cmdline_size = read(cmdline_fd, buf, sizeof(buf) - 1);
> -				close(cmdline_fd);
> -				if (cmdline_size < 0)
> +				int fdinfo_fd;
> +				ssize_t fdinfo_size;
> +				char *line;
> +				u32 perf_event_type = UINT32_MAX;
> +
> +				/* Let's check the PMU type reserved by this process */
> +				scnprintf(buf, sizeof(buf), "%s/fdinfo/%s",
> +					  proc_entry->d_name, fd_entry->d_name);
> +				fdinfo_fd = openat(dirfd(proc_dir), buf, O_RDONLY);
> +				fdinfo_size = read(fdinfo_fd, buf, sizeof(buf) - 1);
> +				if (fdinfo_size < 0)
>  					continue;

Is fdinfo_fd leaked here and on subsequent execution paths in this loop?

Since this loop scans potentially thousands of file descriptors across all
active processes in /proc, could leaking a file descriptor for every
discovered perf event rapidly exhaust the process's file descriptor limit
and lead to EMFILE?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518174159.1177549-1-ctshao@google.com?part=2

Re: [PATCH v6 1/2] perf: Reveal PMU type in fdinfo

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `perf_show_fdinfo` function omits the `config4` field from the `perf_event_attr` struct.
- [Medium] Lockless read of `event->attr` fields can lead to data tearing and inconsistent output.
--

commit ec3f500bd8b814c602df062a02b83af7397b6b5d
Author: Chun-Tse Shao <ctshao@google.com>

perf: Reveal PMU type in fdinfo

This commit adds fdinfo output to reveal the PMU configuration for a process.
It exposes perf_event_attr fields such as type, config, config1, config2,
and config3 through the /proc/*/fdinfo/ interface.

> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 6d1f8bad7e1c5..d88947e9f7740 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
[ ... ]
> @@ -7506,6 +7507,20 @@ static int perf_fasync(int fd, struct file *filp, int on)
>  	return 0;
>  }
>  
> +static void perf_show_fdinfo(struct seq_file *m, struct file *f)
> +{
> +	struct perf_event *event = f->private_data;
> +
> +	seq_printf(m, "perf_event_attr.type:\t%u\n", event->orig_type);
> +	seq_printf(m, "perf_event_attr.config:\t0x%llx\n", (unsigned long long)event->attr.config);
> +	seq_printf(m, "perf_event_attr.config1:\t0x%llx\n",
> +		   (unsigned long long)event->attr.config1);
> +	seq_printf(m, "perf_event_attr.config2:\t0x%llx\n",
> +		   (unsigned long long)event->attr.config2);

Can these fields be read safely without locking or READ_ONCE()?

Since event->attr.config1 (which aliases bp_addr) and event->attr.config2
(which aliases bp_len) can be concurrently modified via the
PERF_EVENT_IOC_MODIFY_ATTRIBUTES ioctl, there could be data tearing on
32-bit architectures if they are updated locklessly with respect to VFS
readers.

> +	seq_printf(m, "perf_event_attr.config3:\t0x%llx\n",
> +		   (unsigned long long)event->attr.config3);

Does this code need to print config4 as well?

The perf_event_attr struct has been extended to include config4.

By omitting it from the fdinfo output, we might be missing information for
users relying on this interface for events and PMUs that utilize this newer
attribute field.

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518174159.1177549-1-ctshao@google.com?part=1