[PATCH 0/2] perf tools: Fixes for mixing per-thread and per-cpu mmaps

[PATCH 0/2] perf tools: Fixes for mixing per-thread and per-cpu mmaps

[Adrian Hunter]

Hi

Here are a couple more small fixes.


Adrian Hunter (2):
      perf record: Fix cpu mask bit setting for mixed mmaps
      libperf evlist: Fix polling of system-wide events

 tools/lib/perf/evlist.c     | 5 +++--
 tools/perf/builtin-record.c | 2 ++
 2 files changed, 5 insertions(+), 2 deletions(-)


Regards
Adrian

[PATCH 1/2] perf record: Fix cpu mask bit setting for mixed mmaps

[Adrian Hunter]

With mixed per-thread and (system-wide) per-cpu maps, the "any cpu" value
 -1 must be skipped when setting CPU mask bits.

Prior to commit cbd7bfc7fd99 ("tools/perf: Fix out of bound access to cpu
mask array") the invalid setting went unnoticed, but since then it causes
perf record to fail with an error.

Example:

 Before:

   $ perf record -e intel_pt// --per-thread uname
   Failed to initialize parallel data streaming masks

 After:

   $ perf record -e intel_pt// --per-thread uname
   Linux
   [ perf record: Woken up 1 times to write data ]
   [ perf record: Captured and wrote 0.068 MB perf.data ]

Fixes: ae4f8ae16a07 ("libperf evlist: Allow mixing per-thread and per-cpu mmaps")
Cc: stable@vger.kernel.org
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/builtin-record.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 5b808ac7a281..b76637ed2462 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -3535,6 +3535,8 @@ static int record__mmap_cpu_mask_init(struct mmap_cpu_mask *mask, struct perf_cp
 		return 0;
 
 	perf_cpu_map__for_each_cpu(cpu, idx, cpus) {
+		if (cpu.cpu == -1)
+			continue;
 		/* Return ENODEV is input cpu is greater than max cpu */
 		if ((unsigned long)cpu.cpu > mask->nbits)
 			return -ENODEV;
-- 
2.25.1

[PATCH 2/2] libperf evlist: Fix polling of system-wide events

[Adrian Hunter]

Originally, (refer commit f90d194a867a ("perf evlist: Do not poll events
that use the system_wide flag") there wasn't much reason to poll
system-wide events because:
 1. The mmaps get "merged" via set-output anyway (the per-cpu case)
 2. perf reads all mmaps when any event is woken
 3. system-wide mmaps do not fill up as fast as the mmaps for user
 selected events
But there was 1 reason not to poll which was that it prevented
correct termination due to POLLHUP on all user selected events.
That issue is now easily resolved by using fdarray_flag__nonfilterable.

With the advent of commit ae4f8ae16a07 ("libperf evlist: Allow mixing
per-thread and per-cpu mmaps"), system-wide mmaps can be used also in
the per-thread case where reason 1 does not apply.

Fix the omission of system-wide events from polling by using the
fdarray_flag__nonfilterable flag.

Example:

 Before:

    $ perf record --no-bpf-event -vvv -e intel_pt// --per-thread uname 2>err.txt
    Linux
    $ grep 'sys_perf_event_open.*=\|pollfd' err.txt
    sys_perf_event_open: pid 155076  cpu -1  group_fd -1  flags 0x8 = 5
    sys_perf_event_open: pid 155076  cpu -1  group_fd -1  flags 0x8 = 6
    sys_perf_event_open: pid -1  cpu 0  group_fd -1  flags 0x8 = 7
    sys_perf_event_open: pid -1  cpu 1  group_fd -1  flags 0x8 = 9
    sys_perf_event_open: pid -1  cpu 2  group_fd -1  flags 0x8 = 10
    sys_perf_event_open: pid -1  cpu 3  group_fd -1  flags 0x8 = 11
    sys_perf_event_open: pid -1  cpu 4  group_fd -1  flags 0x8 = 12
    sys_perf_event_open: pid -1  cpu 5  group_fd -1  flags 0x8 = 13
    sys_perf_event_open: pid -1  cpu 6  group_fd -1  flags 0x8 = 14
    sys_perf_event_open: pid -1  cpu 7  group_fd -1  flags 0x8 = 15
    thread_data[0x55fb43c29e80]: pollfd[0] <- event_fd=5
    thread_data[0x55fb43c29e80]: pollfd[1] <- event_fd=6
    thread_data[0x55fb43c29e80]: pollfd[2] <- non_perf_event fd=4

 After:

    $ perf record --no-bpf-event -vvv -e intel_pt// --per-thread uname 2>err.txt
    Linux
    $ grep 'sys_perf_event_open.*=\|pollfd' err.txt
    sys_perf_event_open: pid 156316  cpu -1  group_fd -1  flags 0x8 = 5
    sys_perf_event_open: pid 156316  cpu -1  group_fd -1  flags 0x8 = 6
    sys_perf_event_open: pid -1  cpu 0  group_fd -1  flags 0x8 = 7
    sys_perf_event_open: pid -1  cpu 1  group_fd -1  flags 0x8 = 9
    sys_perf_event_open: pid -1  cpu 2  group_fd -1  flags 0x8 = 10
    sys_perf_event_open: pid -1  cpu 3  group_fd -1  flags 0x8 = 11
    sys_perf_event_open: pid -1  cpu 4  group_fd -1  flags 0x8 = 12
    sys_perf_event_open: pid -1  cpu 5  group_fd -1  flags 0x8 = 13
    sys_perf_event_open: pid -1  cpu 6  group_fd -1  flags 0x8 = 14
    sys_perf_event_open: pid -1  cpu 7  group_fd -1  flags 0x8 = 15
    thread_data[0x55cc19e58e80]: pollfd[0] <- event_fd=5
    thread_data[0x55cc19e58e80]: pollfd[1] <- event_fd=6
    thread_data[0x55cc19e58e80]: pollfd[2] <- event_fd=7
    thread_data[0x55cc19e58e80]: pollfd[3] <- event_fd=9
    thread_data[0x55cc19e58e80]: pollfd[4] <- event_fd=10
    thread_data[0x55cc19e58e80]: pollfd[5] <- event_fd=11
    thread_data[0x55cc19e58e80]: pollfd[6] <- event_fd=12
    thread_data[0x55cc19e58e80]: pollfd[7] <- event_fd=13
    thread_data[0x55cc19e58e80]: pollfd[8] <- event_fd=14
    thread_data[0x55cc19e58e80]: pollfd[9] <- event_fd=15
    thread_data[0x55cc19e58e80]: pollfd[10] <- non_perf_event fd=4

Fixes: ae4f8ae16a07 ("libperf evlist: Allow mixing per-thread and per-cpu mmaps")
Cc: stable@vger.kernel.org
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/lib/perf/evlist.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
index 80cc810c5097..0e7347d1583d 100644
--- a/tools/lib/perf/evlist.c
+++ b/tools/lib/perf/evlist.c
@@ -441,6 +441,7 @@ mmap_per_evsel(struct perf_evlist *evlist, struct perf_evlist_mmap_ops *ops,
 
 	perf_evlist__for_each_entry(evlist, evsel) {
 		bool overwrite = evsel->attr.write_backward;
+		enum fdarray_flags flgs;
 		struct perf_mmap *map;
 		int *output, fd, cpu;
 
@@ -506,8 +507,8 @@ mmap_per_evsel(struct perf_evlist *evlist, struct perf_evlist_mmap_ops *ops,
 
 		revent = !overwrite ? POLLIN : 0;
 
-		if (!evsel->system_wide &&
-		    perf_evlist__add_pollfd(evlist, fd, map, revent, fdarray_flag__default) < 0) {
+		flgs = evsel->system_wide ? fdarray_flag__nonfilterable : fdarray_flag__default;
+		if (perf_evlist__add_pollfd(evlist, fd, map, revent, flgs) < 0) {
 			perf_mmap__put(map);
 			return -1;
 		}
-- 
2.25.1

Re: [PATCH 0/2] perf tools: Fixes for mixing per-thread and per-cpu mmaps

[Namhyung Kim]

On Thu, Sep 15, 2022 at 5:26 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> Hi
>
> Here are a couple more small fixes.
>
>
> Adrian Hunter (2):
>       perf record: Fix cpu mask bit setting for mixed mmaps
>       libperf evlist: Fix polling of system-wide events

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung


>
>  tools/lib/perf/evlist.c     | 5 +++--
>  tools/perf/builtin-record.c | 2 ++
>  2 files changed, 5 insertions(+), 2 deletions(-)
>
>
> Regards
> Adrian

Re: [PATCH 0/2] perf tools: Fixes for mixing per-thread and per-cpu mmaps

[Arnaldo Carvalho de Melo]

Em Fri, Sep 16, 2022 at 11:28:35AM -0700, Namhyung Kim escreveu:
> On Thu, Sep 15, 2022 at 5:26 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
> >
> > Hi
> >
> > Here are a couple more small fixes.
> >
> >
> > Adrian Hunter (2):
> >       perf record: Fix cpu mask bit setting for mixed mmaps
> >       libperf evlist: Fix polling of system-wide events
> 
> Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks, applied.

- Arnaldo

 
> Thanks,
> Namhyung
> 
> 
> >
> >  tools/lib/perf/evlist.c     | 5 +++--
> >  tools/perf/builtin-record.c | 2 ++
> >  2 files changed, 5 insertions(+), 2 deletions(-)
> >
> >
> > Regards
> > Adrian

-- 

- Arnaldo