[PATCH v7 00/10] perf record --off-cpu: Dump off-cpu samples directly

Howard Chu posted 10 patches 2 weeks ago
There is a newer version of this series
tools/perf/builtin-record.c             |  26 ++++++
tools/perf/builtin-script.c             |   4 +-
tools/perf/tests/builtin-test.c         |   1 +
tools/perf/tests/shell/record_offcpu.sh |  31 ++++++-
tools/perf/tests/tests.h                |   1 +
tools/perf/tests/workloads/Build        |   1 +
tools/perf/tests/workloads/offcpu.c     |  16 ++++
tools/perf/util/bpf_off_cpu.c           | 115 ++++++++++++++----------
tools/perf/util/bpf_skel/off_cpu.bpf.c  |  85 ++++++++++++++++--
tools/perf/util/evsel.c                 |  34 ++++++-
tools/perf/util/evsel.h                 |   2 +
tools/perf/util/off_cpu.h               |   3 +-
tools/perf/util/record.h                |   1 +
13 files changed, 264 insertions(+), 56 deletions(-)
create mode 100644 tools/perf/tests/workloads/offcpu.c
[PATCH v7 00/10] perf record --off-cpu: Dump off-cpu samples directly
Posted by Howard Chu 2 weeks ago
Changes in v7:
 - Make off-cpu event system-wide
 - Use strtoull instead of strtoul
 - Delete unused variable such as sample_id, and sample_type
 - Use i as index to update BPF perf_event map
 - MAX_OFFCPU_LEN 128 is too big, make it smaller.
 - Delete some bound check as it's always guaranteed
 - Do not set ip_pos in BPF
 - Add a new field for storing stack traces in the tstamp map
 - Dump the off-cpu sample directly or save it in the off_cpu map, not both
 - Delete the sample_type_off_cpu check
 - Use __set_off_cpu_sample() to parse samples instead of a two-pass parsing

Changes in v6:
 - Make patches bisectable

Changes in v5:
 - Delete unnecessary copy in BPF program
 - Remove sample_embed from perf header, hard code off-cpu stuff instead
 - Move evsel__is_offcpu_event() to evsel.h
 - Minor changes to the test
 - Edit some comments

Changes in v4:
 - Minimize the size of data output by perf_event_output()
 - Keep only one off-cpu event
 - Change off-cpu threshold's unit to microseconds
 - Set a default off-cpu threshold
 - Print the correct error message for the field 'embed' in perf data header

Changes in v3:
 - Add off-cpu-thresh argument
 - Process direct off-cpu samples in post

Changes in v2:
 - Remove unnecessary comments.
 - Rename function off_cpu_change_type to off_cpu_prepare_parse

v1:

As mentioned in: https://bugzilla.kernel.org/show_bug.cgi?id=207323

Currently, off-cpu samples are dumped when perf record is exiting. This
results in off-cpu samples being after the regular samples. This patch
series makes possible dumping off-cpu samples on-the-fly, directly into
perf ring buffer. And it dispatches those samples to the correct format
for perf.data consumers.

Before:
```
     migration/0      21 [000] 27981.041319: 2944637851    cycles:P:  ffffffff90d2e8aa record_times+0xa ([kernel.kallsyms])
            perf  770116 [001] 27981.041375:          1    cycles:P:  ffffffff90ee4960 event_function+0xf0 ([kernel.kallsyms])
            perf  770116 [001] 27981.041377:          1    cycles:P:  ffffffff90c184b1 intel_bts_enable_local+0x31 ([kernel.kallsyms])
            perf  770116 [001] 27981.041379:      51611    cycles:P:  ffffffff91a160b0 native_sched_clock+0x30 ([kernel.kallsyms])
     migration/1      26 [001] 27981.041400: 4227682775    cycles:P:  ffffffff90d06a74 wakeup_preempt+0x44 ([kernel.kallsyms])
     migration/2      32 [002] 27981.041477: 4159401534    cycles:P:  ffffffff90d11993 update_load_avg+0x63 ([kernel.kallsyms])

sshd  708098 [000] 18446744069.414584:     286392 offcpu-time: 
	    79a864f1c8bb ppoll+0x4b (/usr/lib/libc.so.6)
	    585690935cca [unknown] (/usr/bin/sshd)
```

After:
```
            perf  774767 [003] 28178.033444:        497           cycles:P:  ffffffff91a160c3 native_sched_clock+0x43 ([kernel.kallsyms])
            perf  774767 [003] 28178.033445:     399440           cycles:P:  ffffffff91c01f8d nmi_restore+0x25 ([kernel.kallsyms])
         swapper       0 [001] 28178.036639:  376650973           cycles:P:  ffffffff91a1ae99 intel_idle+0x59 ([kernel.kallsyms])
         swapper       0 [003] 28178.182921:  348779378           cycles:P:  ffffffff91a1ae99 intel_idle+0x59 ([kernel.kallsyms])
    blueman-tray    1355 [000] 28178.627906:  100184571 offcpu-time: 
	    7528eef1c39d __poll+0x4d (/usr/lib/libc.so.6)
	    7528edf7d8fd [unknown] (/usr/lib/libglib-2.0.so.0.8000.2)
	    7528edf1af95 g_main_context_iteration+0x35 (/usr/lib/libglib-2.0.so.0.8000.2)
	    7528eda4ab86 g_application_run+0x1f6 (/usr/lib/libgio-2.0.so.0.8000.2)
	    7528ee6aa596 [unknown] (/usr/lib/libffi.so.8.1.4)
	    7fff24e862d8 [unknown] ([unknown])


    blueman-tray    1355 [000] 28178.728137:  100187539 offcpu-time: 
	    7528eef1c39d __poll+0x4d (/usr/lib/libc.so.6)
	    7528edf7d8fd [unknown] (/usr/lib/libglib-2.0.so.0.8000.2)
	    7528edf1af95 g_main_context_iteration+0x35 (/usr/lib/libglib-2.0.so.0.8000.2)
	    7528eda4ab86 g_application_run+0x1f6 (/usr/lib/libgio-2.0.so.0.8000.2)
	    7528ee6aa596 [unknown] (/usr/lib/libffi.so.8.1.4)
	    7fff24e862d8 [unknown] ([unknown])


         swapper       0 [000] 28178.463253:  195945410           cycles:P:  ffffffff91a1ae99 intel_idle+0x59 ([kernel.kallsyms])
     dbus-broker     412 [002] 28178.464855:  376737008           cycles:P:  ffffffff91c000a0 entry_SYSCALL_64+0x20 ([kernel.kallsyms])
```

Howard Chu (10):
  perf record --off-cpu: Add --off-cpu-thresh option
  perf evsel: Expose evsel__is_offcpu_event() for future use
  perf record --off-cpu: Parse off-cpu event
  perf record --off-cpu: Preparation of off-cpu BPF program
  perf record --off-cpu: Dump off-cpu samples in BPF
  perf evsel: Assemble offcpu samples
  perf record --off-cpu: Disable perf_event's callchain collection
  perf script: Display off-cpu samples correctly
  perf record --off-cpu: Dump the remaining samples in BPF's stack trace
    map
  perf test: Add direct off-cpu test

 tools/perf/builtin-record.c             |  26 ++++++
 tools/perf/builtin-script.c             |   4 +-
 tools/perf/tests/builtin-test.c         |   1 +
 tools/perf/tests/shell/record_offcpu.sh |  31 ++++++-
 tools/perf/tests/tests.h                |   1 +
 tools/perf/tests/workloads/Build        |   1 +
 tools/perf/tests/workloads/offcpu.c     |  16 ++++
 tools/perf/util/bpf_off_cpu.c           | 115 ++++++++++++++----------
 tools/perf/util/bpf_skel/off_cpu.bpf.c  |  85 ++++++++++++++++--
 tools/perf/util/evsel.c                 |  34 ++++++-
 tools/perf/util/evsel.h                 |   2 +
 tools/perf/util/off_cpu.h               |   3 +-
 tools/perf/util/record.h                |   1 +
 13 files changed, 264 insertions(+), 56 deletions(-)
 create mode 100644 tools/perf/tests/workloads/offcpu.c

-- 
2.43.0
Re: [PATCH v7 00/10] perf record --off-cpu: Dump off-cpu samples directly
Posted by Arnaldo Carvalho de Melo 1 week, 4 days ago
⬢ [acme@toolbox perf-tools-next]$        git am ./v7_20241108_howardchu95_perf_record_off_cpu_dump_off_cpu_samples_directly.mbx
Applying: perf record --off-cpu: Add --off-cpu-thresh option
Applying: perf evsel: Expose evsel__is_offcpu_event() for future use
Applying: perf record --off-cpu: Parse off-cpu event
Applying: perf record --off-cpu: Preparation of off-cpu BPF program
Applying: perf record --off-cpu: Dump off-cpu samples in BPF
error: corrupt patch at line 107
Patch failed at 0005 perf record --off-cpu: Dump off-cpu samples in BPF
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
⬢ [acme@toolbox perf-tools-next]$

This is on top of:

https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git tmp.perf-tools-next

The message:

error: corrupt patch at line 107

Doesn't look like a clash with something that changed after you prepared
this patch set.

If we apply the first 4:

⬢ [acme@toolbox perf-tools-next]$        git am ./v7_20241108_howardchu95_perf_record_off_cpu_dump_off_cpu_samples_directly.mbx
Applying: perf record --off-cpu: Add --off-cpu-thresh option
Applying: perf evsel: Expose evsel__is_offcpu_event() for future use
Applying: perf record --off-cpu: Parse off-cpu event
Applying: perf record --off-cpu: Preparation of off-cpu BPF program
⬢ [acme@toolbox perf-tools-next]$ 

Then try to apply just the 5th patch:

⬢ [acme@toolbox perf-tools-next]$ b4 am -P5 -ctsl --cc-trailers 20241108204137.2444151-2-howardchu95@gmail.com
⬢ [acme@toolbox perf-tools-next]$ patch -p1 < ./v7_20241108_howardchu95_perf_record_off_cpu_dump_off_cpu_samples_directly.mbx
patching file tools/perf/util/bpf_skel/off_cpu.bpf.c
patch: **** malformed patch at line 138:  

⬢ [acme@toolbox perf-tools-next]$

You have:

@@ -209,6 +268,12 @@ static int off_cpu_stat(u64 *ctx, struct task_struct *prev,
        pelem->state = state;
        pelem->stack_id = stack_id;

+       /*
+        * If stacks are successfully collected by bpf_get_stackid(), collect them once more
+        * in task_storage for direct off-cpu sample dumping
+        */
+       if (stack_id > 0 && bpf_get_stack(ctx, &pelem->stack, MAX_STACKS * sizeof(u64), BPF_F_USER_STACK)) {
+       }
+
 next:
        pelem = bpf_task_storage_get(&tstamp, next, NULL, 0);

@@ -223,11 +288,19 @@ static int off_cpu_stat(u64 *ctx, struct task_struct *prev,

see the -209,6 +268,12? Did you edit it manually? It should be -209,6 +268,13

And also what is the point of that empty if block?

- Arnaldo

On Fri, Nov 08, 2024 at 12:41:27PM -0800, Howard Chu wrote:
> Changes in v7:
>  - Make off-cpu event system-wide
>  - Use strtoull instead of strtoul
>  - Delete unused variable such as sample_id, and sample_type
>  - Use i as index to update BPF perf_event map
>  - MAX_OFFCPU_LEN 128 is too big, make it smaller.
>  - Delete some bound check as it's always guaranteed
>  - Do not set ip_pos in BPF
>  - Add a new field for storing stack traces in the tstamp map
>  - Dump the off-cpu sample directly or save it in the off_cpu map, not both
>  - Delete the sample_type_off_cpu check
>  - Use __set_off_cpu_sample() to parse samples instead of a two-pass parsing
> 
> Changes in v6:
>  - Make patches bisectable
> 
> Changes in v5:
>  - Delete unnecessary copy in BPF program
>  - Remove sample_embed from perf header, hard code off-cpu stuff instead
>  - Move evsel__is_offcpu_event() to evsel.h
>  - Minor changes to the test
>  - Edit some comments
> 
> Changes in v4:
>  - Minimize the size of data output by perf_event_output()
>  - Keep only one off-cpu event
>  - Change off-cpu threshold's unit to microseconds
>  - Set a default off-cpu threshold
>  - Print the correct error message for the field 'embed' in perf data header
> 
> Changes in v3:
>  - Add off-cpu-thresh argument
>  - Process direct off-cpu samples in post
> 
> Changes in v2:
>  - Remove unnecessary comments.
>  - Rename function off_cpu_change_type to off_cpu_prepare_parse
> 
> v1:
> 
> As mentioned in: https://bugzilla.kernel.org/show_bug.cgi?id=207323
> 
> Currently, off-cpu samples are dumped when perf record is exiting. This
> results in off-cpu samples being after the regular samples. This patch
> series makes possible dumping off-cpu samples on-the-fly, directly into
> perf ring buffer. And it dispatches those samples to the correct format
> for perf.data consumers.
> 
> Before:
> ```
>      migration/0      21 [000] 27981.041319: 2944637851    cycles:P:  ffffffff90d2e8aa record_times+0xa ([kernel.kallsyms])
>             perf  770116 [001] 27981.041375:          1    cycles:P:  ffffffff90ee4960 event_function+0xf0 ([kernel.kallsyms])
>             perf  770116 [001] 27981.041377:          1    cycles:P:  ffffffff90c184b1 intel_bts_enable_local+0x31 ([kernel.kallsyms])
>             perf  770116 [001] 27981.041379:      51611    cycles:P:  ffffffff91a160b0 native_sched_clock+0x30 ([kernel.kallsyms])
>      migration/1      26 [001] 27981.041400: 4227682775    cycles:P:  ffffffff90d06a74 wakeup_preempt+0x44 ([kernel.kallsyms])
>      migration/2      32 [002] 27981.041477: 4159401534    cycles:P:  ffffffff90d11993 update_load_avg+0x63 ([kernel.kallsyms])
> 
> sshd  708098 [000] 18446744069.414584:     286392 offcpu-time: 
> 	    79a864f1c8bb ppoll+0x4b (/usr/lib/libc.so.6)
> 	    585690935cca [unknown] (/usr/bin/sshd)
> ```
> 
> After:
> ```
>             perf  774767 [003] 28178.033444:        497           cycles:P:  ffffffff91a160c3 native_sched_clock+0x43 ([kernel.kallsyms])
>             perf  774767 [003] 28178.033445:     399440           cycles:P:  ffffffff91c01f8d nmi_restore+0x25 ([kernel.kallsyms])
>          swapper       0 [001] 28178.036639:  376650973           cycles:P:  ffffffff91a1ae99 intel_idle+0x59 ([kernel.kallsyms])
>          swapper       0 [003] 28178.182921:  348779378           cycles:P:  ffffffff91a1ae99 intel_idle+0x59 ([kernel.kallsyms])
>     blueman-tray    1355 [000] 28178.627906:  100184571 offcpu-time: 
> 	    7528eef1c39d __poll+0x4d (/usr/lib/libc.so.6)
> 	    7528edf7d8fd [unknown] (/usr/lib/libglib-2.0.so.0.8000.2)
> 	    7528edf1af95 g_main_context_iteration+0x35 (/usr/lib/libglib-2.0.so.0.8000.2)
> 	    7528eda4ab86 g_application_run+0x1f6 (/usr/lib/libgio-2.0.so.0.8000.2)
> 	    7528ee6aa596 [unknown] (/usr/lib/libffi.so.8.1.4)
> 	    7fff24e862d8 [unknown] ([unknown])
> 
> 
>     blueman-tray    1355 [000] 28178.728137:  100187539 offcpu-time: 
> 	    7528eef1c39d __poll+0x4d (/usr/lib/libc.so.6)
> 	    7528edf7d8fd [unknown] (/usr/lib/libglib-2.0.so.0.8000.2)
> 	    7528edf1af95 g_main_context_iteration+0x35 (/usr/lib/libglib-2.0.so.0.8000.2)
> 	    7528eda4ab86 g_application_run+0x1f6 (/usr/lib/libgio-2.0.so.0.8000.2)
> 	    7528ee6aa596 [unknown] (/usr/lib/libffi.so.8.1.4)
> 	    7fff24e862d8 [unknown] ([unknown])
> 
> 
>          swapper       0 [000] 28178.463253:  195945410           cycles:P:  ffffffff91a1ae99 intel_idle+0x59 ([kernel.kallsyms])
>      dbus-broker     412 [002] 28178.464855:  376737008           cycles:P:  ffffffff91c000a0 entry_SYSCALL_64+0x20 ([kernel.kallsyms])
> ```
> 
> Howard Chu (10):
>   perf record --off-cpu: Add --off-cpu-thresh option
>   perf evsel: Expose evsel__is_offcpu_event() for future use
>   perf record --off-cpu: Parse off-cpu event
>   perf record --off-cpu: Preparation of off-cpu BPF program
>   perf record --off-cpu: Dump off-cpu samples in BPF
>   perf evsel: Assemble offcpu samples
>   perf record --off-cpu: Disable perf_event's callchain collection
>   perf script: Display off-cpu samples correctly
>   perf record --off-cpu: Dump the remaining samples in BPF's stack trace
>     map
>   perf test: Add direct off-cpu test
> 
>  tools/perf/builtin-record.c             |  26 ++++++
>  tools/perf/builtin-script.c             |   4 +-
>  tools/perf/tests/builtin-test.c         |   1 +
>  tools/perf/tests/shell/record_offcpu.sh |  31 ++++++-
>  tools/perf/tests/tests.h                |   1 +
>  tools/perf/tests/workloads/Build        |   1 +
>  tools/perf/tests/workloads/offcpu.c     |  16 ++++
>  tools/perf/util/bpf_off_cpu.c           | 115 ++++++++++++++----------
>  tools/perf/util/bpf_skel/off_cpu.bpf.c  |  85 ++++++++++++++++--
>  tools/perf/util/evsel.c                 |  34 ++++++-
>  tools/perf/util/evsel.h                 |   2 +
>  tools/perf/util/off_cpu.h               |   3 +-
>  tools/perf/util/record.h                |   1 +
>  13 files changed, 264 insertions(+), 56 deletions(-)
>  create mode 100644 tools/perf/tests/workloads/offcpu.c
> 
> -- 
> 2.43.0
Re: [PATCH v7 00/10] perf record --off-cpu: Dump off-cpu samples directly
Posted by Arnaldo Carvalho de Melo 1 week, 4 days ago
On Tue, Nov 12, 2024 at 03:39:25PM -0300, Arnaldo Carvalho de Melo wrote:
> ⬢ [acme@toolbox perf-tools-next]$        git am ./v7_20241108_howardchu95_perf_record_off_cpu_dump_off_cpu_samples_directly.mbx
> Applying: perf record --off-cpu: Add --off-cpu-thresh option
> Applying: perf evsel: Expose evsel__is_offcpu_event() for future use
> Applying: perf record --off-cpu: Parse off-cpu event
> Applying: perf record --off-cpu: Preparation of off-cpu BPF program
> Applying: perf record --off-cpu: Dump off-cpu samples in BPF
> error: corrupt patch at line 107
> Patch failed at 0005 perf record --off-cpu: Dump off-cpu samples in BPF
> hint: Use 'git am --show-current-patch=diff' to see the failed patch
> hint: When you have resolved this problem, run "git am --continue".
> hint: If you prefer to skip this patch, run "git am --skip" instead.
> hint: To restore the original branch and stop patching, run "git am --abort".
> hint: Disable this message with "git config advice.mergeConflict false"
> ⬢ [acme@toolbox perf-tools-next]$
> 
> This is on top of:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git tmp.perf-tools-next
> 
> The message:
> 
> error: corrupt patch at line 107
> 
> Doesn't look like a clash with something that changed after you prepared
> this patch set.
> 
> If we apply the first 4:
> 
> ⬢ [acme@toolbox perf-tools-next]$        git am ./v7_20241108_howardchu95_perf_record_off_cpu_dump_off_cpu_samples_directly.mbx
> Applying: perf record --off-cpu: Add --off-cpu-thresh option
> Applying: perf evsel: Expose evsel__is_offcpu_event() for future use
> Applying: perf record --off-cpu: Parse off-cpu event
> Applying: perf record --off-cpu: Preparation of off-cpu BPF program
> ⬢ [acme@toolbox perf-tools-next]$ 
> 
> Then try to apply just the 5th patch:
> 
> ⬢ [acme@toolbox perf-tools-next]$ b4 am -P5 -ctsl --cc-trailers 20241108204137.2444151-2-howardchu95@gmail.com
> ⬢ [acme@toolbox perf-tools-next]$ patch -p1 < ./v7_20241108_howardchu95_perf_record_off_cpu_dump_off_cpu_samples_directly.mbx
> patching file tools/perf/util/bpf_skel/off_cpu.bpf.c
> patch: **** malformed patch at line 138:  
> 
> ⬢ [acme@toolbox perf-tools-next]$
> 
> You have:
> 
> @@ -209,6 +268,12 @@ static int off_cpu_stat(u64 *ctx, struct task_struct *prev,
>         pelem->state = state;
>         pelem->stack_id = stack_id;
> 
> +       /*
> +        * If stacks are successfully collected by bpf_get_stackid(), collect them once more
> +        * in task_storage for direct off-cpu sample dumping
> +        */
> +       if (stack_id > 0 && bpf_get_stack(ctx, &pelem->stack, MAX_STACKS * sizeof(u64), BPF_F_USER_STACK)) {
> +       }
> +
>  next:
>         pelem = bpf_task_storage_get(&tstamp, next, NULL, 0);
> 
> @@ -223,11 +288,19 @@ static int off_cpu_stat(u64 *ctx, struct task_struct *prev,
> 
> see the -209,6 +268,12? Did you edit it manually? It should be -209,6 +268,13

So, I fixed up that patch hunk, applied the patch and tried to build
perf:

  CC      /tmp/build/perf-tools-next/util/bpf_off_cpu.o
  CC      /tmp/build/perf-tools-next/util/bpf-filter.o
  CC      /tmp/build/perf-tools-next/util/bpf_lock_contention.o
  CC      /tmp/build/perf-tools-next/util/bpf_kwork.o
  CC      /tmp/build/perf-tools-next/util/bpf_kwork_top.o
  CC      /tmp/build/perf-tools-next/util/annotate-data.o
  CC      /tmp/build/perf-tools-next/util/data-convert-bt.o
  CC      /tmp/build/perf-tools-next/util/data-convert-json.o
util/bpf_off_cpu.c: In function ‘off_cpu_start’:
util/bpf_off_cpu.c:78:9: error: ‘evsel’ undeclared (first use in this function)
   78 |         evsel = evlist__find_evsel_by_str(evlist, OFFCPU_EVENT);
      |         ^~~~~
util/bpf_off_cpu.c:78:9: note: each undeclared identifier is reported only once for each function it appears in
In file included from /tmp/build/perf-tools-next/libperf/include/internal/cpumap.h:6,
                 from /tmp/build/perf-tools-next/libperf/include/internal/evsel.h:9,
                 from /home/acme/git/perf-tools-next/tools/perf/util/evsel.h:10,
                 from util/bpf_off_cpu.c:4:
util/bpf_off_cpu.c:84:42: error: ‘i’ undeclared (first use in this function)
   84 |         perf_cpu_map__for_each_cpu(pcpu, i, evsel->core.cpus) {
      |                                          ^
/tmp/build/perf-tools-next/libperf/include/perf/cpumap.h:90:15: note: in definition of macro ‘perf_cpu_map__for_each_cpu’
   90 |         for ((idx) = 0, (cpu) = perf_cpu_map__cpu(cpus, idx);   \
      |               ^~~
util/bpf_off_cpu.c:84:36: error: ‘pcpu’ undeclared (first use in this function)
   84 |         perf_cpu_map__for_each_cpu(pcpu, i, evsel->core.cpus) {
      |                                    ^~~~
/tmp/build/perf-tools-next/libperf/include/perf/cpumap.h:90:26: note: in definition of macro ‘perf_cpu_map__for_each_cpu’
   90 |         for ((idx) = 0, (cpu) = perf_cpu_map__cpu(cpus, idx);   \
      |                          ^~~
/tmp/build/perf-tools-next/libperf/include/perf/cpumap.h:90:23: error: left-hand operand of comma expression has no effect [-Werror=unused-value]
   90 |         for ((idx) = 0, (cpu) = perf_cpu_map__cpu(cpus, idx);   \
      |                       ^
util/bpf_off_cpu.c:84:9: note: in expansion of macro ‘perf_cpu_map__for_each_cpu’
   84 |         perf_cpu_map__for_each_cpu(pcpu, i, evsel->core.cpus) {
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/build/perf-tools-next/libperf/include/perf/cpumap.h:92:21: error: left-hand operand of comma expression has no effect [-Werror=unused-value]
   92 |              (idx)++, (cpu) = perf_cpu_map__cpu(cpus, idx))
      |                     ^
util/bpf_off_cpu.c:84:9: note: in expansion of macro ‘perf_cpu_map__for_each_cpu’
   84 |         perf_cpu_map__for_each_cpu(pcpu, i, evsel->core.cpus) {
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
util/bpf_off_cpu.c:85:17: error: ‘err’ undeclared (first use in this function)
   85 |                 err = bpf_map__update_elem(skel->maps.offcpu_output, &pcpu.cpu, sizeof(__u32),
      |                 ^~~
cc1: all warnings being treated as errors
  CC      /tmp/build/perf-tools-next/util/jitdump.o
  CC      /tmp/build/perf-tools-next/util/bpf-event.o
make[4]: *** [/home/acme/git/perf-tools-next/tools/build/Makefile.build:105: /tmp/build/perf-tools-next/util/bpf_off_cpu.o] Error 1
make[4]: *** Waiting for unfinished jobs....
make[3]: *** [/home/acme/git/perf-tools-next/tools/build/Makefile.build:158: util] Error 2
make[2]: *** [Makefile.perf:789: /tmp/build/perf-tools-next/perf-util-in.o] Error 2
make[1]: *** [Makefile.perf:292: sub-make] Error 2
make: *** [Makefile:119: install-bin] Error 2
make: Leaving directory '/home/acme/git/perf-tools-next/tools/perf'
⬢ [acme@toolbox perf-tools-next]$

I squashed the patch below and I'm trying to apply the other patches to do some
minimal testing on the feature itself, but the organization of the
patches needs some work.

- Arnaldo

diff --git a/tools/perf/util/bpf_off_cpu.c b/tools/perf/util/bpf_off_cpu.c
index cfe5b17393e9ed3a..531465b07952f3b7 100644
--- a/tools/perf/util/bpf_off_cpu.c
+++ b/tools/perf/util/bpf_off_cpu.c
@@ -61,6 +61,9 @@ static int off_cpu_config(struct evlist *evlist)
 static void off_cpu_start(void *arg)
 {
 	struct evlist *evlist = arg;
+	struct evsel *evsel;
+	struct perf_cpu pcpu;
+	int i;
 
 	/* update task filter for the given workload */
 	if (skel->rodata->has_task && skel->rodata->uses_tgid &&
@@ -82,6 +85,8 @@ static void off_cpu_start(void *arg)
 	}
 
 	perf_cpu_map__for_each_cpu(pcpu, i, evsel->core.cpus) {
+		int err;
+
 		err = bpf_map__update_elem(skel->maps.offcpu_output, &pcpu.cpu, sizeof(__u32),
 					   xyarray__entry(evsel->core.fd, i, 0),
 					   sizeof(__u32), BPF_ANY);
Re: [PATCH v7 00/10] perf record --off-cpu: Dump off-cpu samples directly
Posted by Arnaldo Carvalho de Melo 1 week, 4 days ago
On Tue, Nov 12, 2024 at 04:17:19PM -0300, Arnaldo Carvalho de Melo wrote:
> I squashed the patch below and I'm trying to apply the other patches to do some
> minimal testing on the feature itself, but the organization of the
> patches needs some work.
 
> +++ b/tools/perf/util/bpf_off_cpu.c
> @@ -61,6 +61,9 @@ static int off_cpu_config(struct evlist *evlist)
>  static void off_cpu_start(void *arg)
>  {
>  	struct evlist *evlist = arg;
> +	struct evsel *evsel;
> +	struct perf_cpu pcpu;
> +	int i;
>  
>  	/* update task filter for the given workload */
>  	if (skel->rodata->has_task && skel->rodata->uses_tgid &&
> @@ -82,6 +85,8 @@ static void off_cpu_start(void *arg)
>  	}
>  
>  	perf_cpu_map__for_each_cpu(pcpu, i, evsel->core.cpus) {
> +		int err;
> +
>  		err = bpf_map__update_elem(skel->maps.offcpu_output, &pcpu.cpu, sizeof(__u32),
>  					   xyarray__entry(evsel->core.fd, i, 0),
>  					   sizeof(__u32), BPF_ANY);

This is not enough, as it in the end tries to use that
skel->maps.offcpu_output that is only introduced at a later patch, it
seems, not checked yet, but explains the error below:

  LD      /tmp/build/perf-tools-next/perf-test-in.o
  AR      /tmp/build/perf-tools-next/libperf-test.a
  CC      /tmp/build/perf-tools-next/util/parse-events.o
util/bpf_off_cpu.c: In function ‘off_cpu_start’:
util/bpf_off_cpu.c:90:54: error: ‘struct <anonymous>’ has no member named ‘offcpu_output’
   90 |                 err = bpf_map__update_elem(skel->maps.offcpu_output, &pcpu.cpu, sizeof(__u32),
      |                                                      ^
make[4]: *** [/home/acme/git/perf-tools-next/tools/build/Makefile.build:106: /tmp/build/perf-tools-next/util/bpf_off_cpu.o] Error 1
make[4]: *** Waiting for unfinished jobs....
make[3]: *** [/home/acme/git/perf-tools-next/tools/build/Makefile.build:158: util] Error 2
make[2]: *** [Makefile.perf:789: /tmp/build/perf-tools-next/perf-util-in.o] Error 2
make[2]: *** Waiting for unfinished jobs....
  CC      /tmp/build/perf-tools-next/pmu-events/pmu-events.o
  LD      /tmp/build/perf-tools-next/pmu-events/pmu-events-in.o
make[1]: *** [Makefile.perf:292: sub-make] Error 2
make: *** [Makefile:119: install-bin] Error 2
make: Leaving directory '/home/acme/git/perf-tools-next/tools/perf'
⬢ [acme@toolbox perf-tools-next]$ 


Ok, at the end of the series it builds, and the 'perf test' entry
introduced in this series passes:

root@x1:~# perf test off
121: perf record offcpu profiling tests                              : Ok
root@x1:~# perf test -v off
121: perf record offcpu profiling tests                              : Ok
root@x1:~# perf test -vv off
121: perf record offcpu profiling tests:
--- start ---
test child forked, pid 1303134
Checking off-cpu privilege
Basic off-cpu test
Basic off-cpu test [Success]
Child task off-cpu test
Child task off-cpu test [Success]
Direct off-cpu test
Direct off-cpu test [Success]
---- end(0) ----
121: perf record offcpu profiling tests                              : Ok
root@x1:~#

But the only examples I could find so far for this feature were on the
'perf test' at the end of this series.

I think we need to have some examples in the 'perf-record' man page
showing how to use it, explaining the whole process, etc.

I'll continue testing it and trying to move things around so that it
gets bisectable and testable step by step, documenting the whole
process as I go, probably tomorrow.

The series with my fixes is at:

https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git perf-off-cpu77918
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/log/?h=perf-off-cpu


- Arnaldo
Re: [PATCH v7 00/10] perf record --off-cpu: Dump off-cpu samples directly
Posted by Howard Chu 1 week ago
Hello Arnaldo,

On Tue, Nov 12, 2024 at 11:56 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> On Tue, Nov 12, 2024 at 04:17:19PM -0300, Arnaldo Carvalho de Melo wrote:
> > I squashed the patch below and I'm trying to apply the other patches to do some
> > minimal testing on the feature itself, but the organization of the
> > patches needs some work.
>
> > +++ b/tools/perf/util/bpf_off_cpu.c
> > @@ -61,6 +61,9 @@ static int off_cpu_config(struct evlist *evlist)
> >  static void off_cpu_start(void *arg)
> >  {
> >       struct evlist *evlist = arg;
> > +     struct evsel *evsel;
> > +     struct perf_cpu pcpu;
> > +     int i;
> >
> >       /* update task filter for the given workload */
> >       if (skel->rodata->has_task && skel->rodata->uses_tgid &&
> > @@ -82,6 +85,8 @@ static void off_cpu_start(void *arg)
> >       }
> >
> >       perf_cpu_map__for_each_cpu(pcpu, i, evsel->core.cpus) {
> > +             int err;
> > +
> >               err = bpf_map__update_elem(skel->maps.offcpu_output, &pcpu.cpu, sizeof(__u32),
> >                                          xyarray__entry(evsel->core.fd, i, 0),
> >                                          sizeof(__u32), BPF_ANY);
>
> This is not enough, as it in the end tries to use that
> skel->maps.offcpu_output that is only introduced at a later patch, it
> seems, not checked yet, but explains the error below:
>
>   LD      /tmp/build/perf-tools-next/perf-test-in.o
>   AR      /tmp/build/perf-tools-next/libperf-test.a
>   CC      /tmp/build/perf-tools-next/util/parse-events.o
> util/bpf_off_cpu.c: In function ‘off_cpu_start’:
> util/bpf_off_cpu.c:90:54: error: ‘struct <anonymous>’ has no member named ‘offcpu_output’
>    90 |                 err = bpf_map__update_elem(skel->maps.offcpu_output, &pcpu.cpu, sizeof(__u32),
>       |                                                      ^
> make[4]: *** [/home/acme/git/perf-tools-next/tools/build/Makefile.build:106: /tmp/build/perf-tools-next/util/bpf_off_cpu.o] Error 1
> make[4]: *** Waiting for unfinished jobs....
> make[3]: *** [/home/acme/git/perf-tools-next/tools/build/Makefile.build:158: util] Error 2
> make[2]: *** [Makefile.perf:789: /tmp/build/perf-tools-next/perf-util-in.o] Error 2
> make[2]: *** Waiting for unfinished jobs....
>   CC      /tmp/build/perf-tools-next/pmu-events/pmu-events.o
>   LD      /tmp/build/perf-tools-next/pmu-events/pmu-events-in.o
> make[1]: *** [Makefile.perf:292: sub-make] Error 2
> make: *** [Makefile:119: install-bin] Error 2
> make: Leaving directory '/home/acme/git/perf-tools-next/tools/perf'
> ⬢ [acme@toolbox perf-tools-next]$
>
>
> Ok, at the end of the series it builds, and the 'perf test' entry
> introduced in this series passes:
>
> root@x1:~# perf test off
> 121: perf record offcpu profiling tests                              : Ok
> root@x1:~# perf test -v off
> 121: perf record offcpu profiling tests                              : Ok
> root@x1:~# perf test -vv off
> 121: perf record offcpu profiling tests:
> --- start ---
> test child forked, pid 1303134
> Checking off-cpu privilege
> Basic off-cpu test
> Basic off-cpu test [Success]
> Child task off-cpu test
> Child task off-cpu test [Success]
> Direct off-cpu test
> Direct off-cpu test [Success]
> ---- end(0) ----
> 121: perf record offcpu profiling tests                              : Ok
> root@x1:~#
>
> But the only examples I could find so far for this feature were on the
> 'perf test' at the end of this series.
>
> I think we need to have some examples in the 'perf-record' man page
> showing how to use it, explaining the whole process, etc.

Sure, I'll send patches to do that. :)

>
> I'll continue testing it and trying to move things around so that it
> gets bisectable and testable step by step, documenting the whole
> process as I go, probably tomorrow.
>
> The series with my fixes is at:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git perf-off-cpu77918
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/log/?h=perf-off-cpu
>
>
> - Arnaldo

Thanks,
Howard
Re: [PATCH v7 00/10] perf record --off-cpu: Dump off-cpu samples directly
Posted by Howard Chu 1 week, 4 days ago
Hello,

On Tue, Nov 12, 2024 at 11:56 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> On Tue, Nov 12, 2024 at 04:17:19PM -0300, Arnaldo Carvalho de Melo wrote:
> > I squashed the patch below and I'm trying to apply the other patches to do some
> > minimal testing on the feature itself, but the organization of the
> > patches needs some work.
>
> > +++ b/tools/perf/util/bpf_off_cpu.c
> > @@ -61,6 +61,9 @@ static int off_cpu_config(struct evlist *evlist)
> >  static void off_cpu_start(void *arg)
> >  {
> >       struct evlist *evlist = arg;
> > +     struct evsel *evsel;
> > +     struct perf_cpu pcpu;
> > +     int i;
> >
> >       /* update task filter for the given workload */
> >       if (skel->rodata->has_task && skel->rodata->uses_tgid &&
> > @@ -82,6 +85,8 @@ static void off_cpu_start(void *arg)
> >       }
> >
> >       perf_cpu_map__for_each_cpu(pcpu, i, evsel->core.cpus) {
> > +             int err;
> > +
> >               err = bpf_map__update_elem(skel->maps.offcpu_output, &pcpu.cpu, sizeof(__u32),
> >                                          xyarray__entry(evsel->core.fd, i, 0),
> >                                          sizeof(__u32), BPF_ANY);
>
> This is not enough, as it in the end tries to use that
> skel->maps.offcpu_output that is only introduced at a later patch, it
> seems, not checked yet, but explains the error below:
>
>   LD      /tmp/build/perf-tools-next/perf-test-in.o
>   AR      /tmp/build/perf-tools-next/libperf-test.a
>   CC      /tmp/build/perf-tools-next/util/parse-events.o
> util/bpf_off_cpu.c: In function ‘off_cpu_start’:
> util/bpf_off_cpu.c:90:54: error: ‘struct <anonymous>’ has no member named ‘offcpu_output’
>    90 |                 err = bpf_map__update_elem(skel->maps.offcpu_output, &pcpu.cpu, sizeof(__u32),
>       |                                                      ^
> make[4]: *** [/home/acme/git/perf-tools-next/tools/build/Makefile.build:106: /tmp/build/perf-tools-next/util/bpf_off_cpu.o] Error 1
> make[4]: *** Waiting for unfinished jobs....
> make[3]: *** [/home/acme/git/perf-tools-next/tools/build/Makefile.build:158: util] Error 2
> make[2]: *** [Makefile.perf:789: /tmp/build/perf-tools-next/perf-util-in.o] Error 2
> make[2]: *** Waiting for unfinished jobs....
>   CC      /tmp/build/perf-tools-next/pmu-events/pmu-events.o
>   LD      /tmp/build/perf-tools-next/pmu-events/pmu-events-in.o
> make[1]: *** [Makefile.perf:292: sub-make] Error 2
> make: *** [Makefile:119: install-bin] Error 2
> make: Leaving directory '/home/acme/git/perf-tools-next/tools/perf'
> ⬢ [acme@toolbox perf-tools-next]$
>
>
> Ok, at the end of the series it builds, and the 'perf test' entry
> introduced in this series passes:
>
> root@x1:~# perf test off
> 121: perf record offcpu profiling tests                              : Ok
> root@x1:~# perf test -v off
> 121: perf record offcpu profiling tests                              : Ok
> root@x1:~# perf test -vv off
> 121: perf record offcpu profiling tests:
> --- start ---
> test child forked, pid 1303134
> Checking off-cpu privilege
> Basic off-cpu test
> Basic off-cpu test [Success]
> Child task off-cpu test
> Child task off-cpu test [Success]
> Direct off-cpu test
> Direct off-cpu test [Success]
> ---- end(0) ----
> 121: perf record offcpu profiling tests                              : Ok
> root@x1:~#
>
> But the only examples I could find so far for this feature were on the
> 'perf test' at the end of this series.
>
> I think we need to have some examples in the 'perf-record' man page
> showing how to use it, explaining the whole process, etc.
>
> I'll continue testing it and trying to move things around so that it
> gets bisectable and testable step by step, documenting the whole
> process as I go, probably tomorrow.
>
> The series with my fixes is at:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git perf-off-cpu77918
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/log/?h=perf-off-cpu

Thank you, I'll use that as the base of v8 :), adding Ian's suggestions.

>
>
> - Arnaldo

Thanks,
Howard
Re: [PATCH v7 00/10] perf record --off-cpu: Dump off-cpu samples directly
Posted by Arnaldo Carvalho de Melo 1 week, 4 days ago
On Tue, Nov 12, 2024 at 04:17:24PM -0300, Arnaldo Carvalho de Melo wrote:
> On Tue, Nov 12, 2024 at 03:39:25PM -0300, Arnaldo Carvalho de Melo wrote:
> make: *** [Makefile:119: install-bin] Error 2
> make: Leaving directory '/home/acme/git/perf-tools-next/tools/perf'
> ⬢ [acme@toolbox perf-tools-next]$
> 
> I squashed the patch below and I'm trying to apply the other patches to do some
> minimal testing on the feature itself, but the organization of the
> patches needs some work.

Fails a few patches later, trying to fix it.

- Arnaldo
Re: [PATCH v7 00/10] perf record --off-cpu: Dump off-cpu samples directly
Posted by Arnaldo Carvalho de Melo 1 week, 4 days ago
On Tue, Nov 12, 2024 at 04:18:27PM -0300, Arnaldo Carvalho de Melo wrote:
> On Tue, Nov 12, 2024 at 04:17:24PM -0300, Arnaldo Carvalho de Melo wrote:
> > On Tue, Nov 12, 2024 at 03:39:25PM -0300, Arnaldo Carvalho de Melo wrote:
> > make: *** [Makefile:119: install-bin] Error 2
> > make: Leaving directory '/home/acme/git/perf-tools-next/tools/perf'
> > ⬢ [acme@toolbox perf-tools-next]$
> > 
> > I squashed the patch below and I'm trying to apply the other patches to do some
> > minimal testing on the feature itself, but the organization of the
> > patches needs some work.
> 
> Fails a few patches later, trying to fix it.

Sorry, forgot to add it:

⬢ [acme@toolbox perf-tools-next]$        git am ./v7_20241108_howardchu95_perf_record_off_cpu_dump_off_cpu_samples_directly.mbx
Applying: perf evsel: Assemble offcpu samples
Applying: perf record --off-cpu: Disable perf_event's callchain collection
Applying: perf script: Display off-cpu samples correctly
Applying: perf record --off-cpu: Dump the remaining samples in BPF's stack trace map
error: patch failed: tools/perf/util/bpf_off_cpu.c:61
error: tools/perf/util/bpf_off_cpu.c: patch does not apply
Patch failed at 0004 perf record --off-cpu: Dump the remaining samples in BPF's stack trace map
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
⬢ [acme@toolbox perf-tools-next]$
Re: [PATCH v7 00/10] perf record --off-cpu: Dump off-cpu samples directly
Posted by Arnaldo Carvalho de Melo 1 week, 4 days ago
On Tue, Nov 12, 2024 at 04:18:45PM -0300, Arnaldo Carvalho de Melo wrote:
> On Tue, Nov 12, 2024 at 04:18:27PM -0300, Arnaldo Carvalho de Melo wrote:
> > On Tue, Nov 12, 2024 at 04:17:24PM -0300, Arnaldo Carvalho de Melo wrote:
> > > On Tue, Nov 12, 2024 at 03:39:25PM -0300, Arnaldo Carvalho de Melo wrote:
> > > make: *** [Makefile:119: install-bin] Error 2
> > > make: Leaving directory '/home/acme/git/perf-tools-next/tools/perf'
> > > ⬢ [acme@toolbox perf-tools-next]$
> > > 
> > > I squashed the patch below and I'm trying to apply the other patches to do some
> > > minimal testing on the feature itself, but the organization of the
> > > patches needs some work.
> > 
> > Fails a few patches later, trying to fix it.
> 
> Sorry, forgot to add it:
> 
> ⬢ [acme@toolbox perf-tools-next]$        git am ./v7_20241108_howardchu95_perf_record_off_cpu_dump_off_cpu_samples_directly.mbx
> Applying: perf evsel: Assemble offcpu samples
> Applying: perf record --off-cpu: Disable perf_event's callchain collection
> Applying: perf script: Display off-cpu samples correctly
> Applying: perf record --off-cpu: Dump the remaining samples in BPF's stack trace map
> error: patch failed: tools/perf/util/bpf_off_cpu.c:61
> error: tools/perf/util/bpf_off_cpu.c: patch does not apply
> Patch failed at 0004 perf record --off-cpu: Dump the remaining samples in BPF's stack trace map
> hint: Use 'git am --show-current-patch=diff' to see the failed patch
> hint: When you have resolved this problem, run "git am --continue".
> hint: If you prefer to skip this patch, run "git am --skip" instead.
> hint: To restore the original branch and stop patching, run "git am --abort".
> hint: Disable this message with "git config advice.mergeConflict false"
> ⬢ [acme@toolbox perf-tools-next]$

So now its just a fallout from the fix on a previous patch, I'm patching
it up:

⬢ [acme@toolbox perf-tools-next]$ vim ./v7_20241108_howardchu95_perf_record_off_cpu_dump_off_cpu_samples_directly.mbx
⬢ [acme@toolbox perf-tools-next]$ patch -p1 < ./v7_20241108_howardchu95_perf_record_off_cpu_dump_off_cpu_samples_directly.mbx
patching file tools/perf/util/bpf_off_cpu.c
Hunk #2 FAILED at 63.
Hunk #3 succeeded at 311 (offset 5 lines).
Hunk #4 succeeded at 351 (offset 5 lines).
1 out of 4 hunks FAILED -- saving rejects to file tools/perf/util/bpf_off_cpu.c.rej
⬢ [acme@toolbox perf-tools-next]$ 
⬢ [acme@toolbox perf-tools-next]$ vim tools/perf/util/bpf_off_cpu.c.rej
⬢ [acme@toolbox perf-tools-next]$ cat tools/perf/util/bpf_off_cpu.c.rej
--- tools/perf/util/bpf_off_cpu.c
+++ tools/perf/util/bpf_off_cpu.c
@@ -63,6 +65,9 @@ static int off_cpu_config(struct evlist *evlist)
 static void off_cpu_start(void *arg)
 {
 	struct evlist *evlist = arg;
+	struct evsel *evsel;
+	struct perf_cpu pcpu;
+	int i, err;
 
 	/* update task filter for the given workload */
 	if (skel->rodata->has_task && skel->rodata->uses_tgid &&
⬢ [acme@toolbox perf-tools-next]$
Re: [PATCH v7 00/10] perf record --off-cpu: Dump off-cpu samples directly
Posted by Ian Rogers 1 week, 4 days ago
On Tue, Nov 12, 2024 at 10:39 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> ⬢ [acme@toolbox perf-tools-next]$        git am ./v7_20241108_howardchu95_perf_record_off_cpu_dump_off_cpu_samples_directly.mbx
> Applying: perf record --off-cpu: Add --off-cpu-thresh option
> Applying: perf evsel: Expose evsel__is_offcpu_event() for future use
> Applying: perf record --off-cpu: Parse off-cpu event
> Applying: perf record --off-cpu: Preparation of off-cpu BPF program
> Applying: perf record --off-cpu: Dump off-cpu samples in BPF
> error: corrupt patch at line 107
> Patch failed at 0005 perf record --off-cpu: Dump off-cpu samples in BPF
> hint: Use 'git am --show-current-patch=diff' to see the failed patch
> hint: When you have resolved this problem, run "git am --continue".
> hint: If you prefer to skip this patch, run "git am --skip" instead.
> hint: To restore the original branch and stop patching, run "git am --abort".
> hint: Disable this message with "git config advice.mergeConflict false"
> ⬢ [acme@toolbox perf-tools-next]$
>
> This is on top of:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git tmp.perf-tools-next
>
> The message:
>
> error: corrupt patch at line 107
>
> Doesn't look like a clash with something that changed after you prepared
> this patch set.
>
> If we apply the first 4:
>
> ⬢ [acme@toolbox perf-tools-next]$        git am ./v7_20241108_howardchu95_perf_record_off_cpu_dump_off_cpu_samples_directly.mbx
> Applying: perf record --off-cpu: Add --off-cpu-thresh option
> Applying: perf evsel: Expose evsel__is_offcpu_event() for future use
> Applying: perf record --off-cpu: Parse off-cpu event
> Applying: perf record --off-cpu: Preparation of off-cpu BPF program
> ⬢ [acme@toolbox perf-tools-next]$
>
> Then try to apply just the 5th patch:
>
> ⬢ [acme@toolbox perf-tools-next]$ b4 am -P5 -ctsl --cc-trailers 20241108204137.2444151-2-howardchu95@gmail.com
> ⬢ [acme@toolbox perf-tools-next]$ patch -p1 < ./v7_20241108_howardchu95_perf_record_off_cpu_dump_off_cpu_samples_directly.mbx
> patching file tools/perf/util/bpf_skel/off_cpu.bpf.c
> patch: **** malformed patch at line 138:
>
> ⬢ [acme@toolbox perf-tools-next]$
>
> You have:
>
> @@ -209,6 +268,12 @@ static int off_cpu_stat(u64 *ctx, struct task_struct *prev,
>         pelem->state = state;
>         pelem->stack_id = stack_id;
>
> +       /*
> +        * If stacks are successfully collected by bpf_get_stackid(), collect them once more
> +        * in task_storage for direct off-cpu sample dumping
> +        */
> +       if (stack_id > 0 && bpf_get_stack(ctx, &pelem->stack, MAX_STACKS * sizeof(u64), BPF_F_USER_STACK)) {
> +       }
> +
>  next:
>         pelem = bpf_task_storage_get(&tstamp, next, NULL, 0);
>
> @@ -223,11 +288,19 @@ static int off_cpu_stat(u64 *ctx, struct task_struct *prev,
>
> see the -209,6 +268,12? Did you edit it manually? It should be -209,6 +268,13
>
> And also what is the point of that empty if block?

There was some discussion of this already:
https://lore.kernel.org/lkml/CAP-5=fVXgQjAh1OFDN7DMp=xw5cAHRaO8j8UQfe4CZHrZc8uFg@mail.gmail.com/
TL;DR, it avoids an unused return value warning.

Thanks,
Ian

> - Arnaldo
>
> On Fri, Nov 08, 2024 at 12:41:27PM -0800, Howard Chu wrote:
> > Changes in v7:
> >  - Make off-cpu event system-wide
> >  - Use strtoull instead of strtoul
> >  - Delete unused variable such as sample_id, and sample_type
> >  - Use i as index to update BPF perf_event map
> >  - MAX_OFFCPU_LEN 128 is too big, make it smaller.
> >  - Delete some bound check as it's always guaranteed
> >  - Do not set ip_pos in BPF
> >  - Add a new field for storing stack traces in the tstamp map
> >  - Dump the off-cpu sample directly or save it in the off_cpu map, not both
> >  - Delete the sample_type_off_cpu check
> >  - Use __set_off_cpu_sample() to parse samples instead of a two-pass parsing
> >
> > Changes in v6:
> >  - Make patches bisectable
> >
> > Changes in v5:
> >  - Delete unnecessary copy in BPF program
> >  - Remove sample_embed from perf header, hard code off-cpu stuff instead
> >  - Move evsel__is_offcpu_event() to evsel.h
> >  - Minor changes to the test
> >  - Edit some comments
> >
> > Changes in v4:
> >  - Minimize the size of data output by perf_event_output()
> >  - Keep only one off-cpu event
> >  - Change off-cpu threshold's unit to microseconds
> >  - Set a default off-cpu threshold
> >  - Print the correct error message for the field 'embed' in perf data header
> >
> > Changes in v3:
> >  - Add off-cpu-thresh argument
> >  - Process direct off-cpu samples in post
> >
> > Changes in v2:
> >  - Remove unnecessary comments.
> >  - Rename function off_cpu_change_type to off_cpu_prepare_parse
> >
> > v1:
> >
> > As mentioned in: https://bugzilla.kernel.org/show_bug.cgi?id=207323
> >
> > Currently, off-cpu samples are dumped when perf record is exiting. This
> > results in off-cpu samples being after the regular samples. This patch
> > series makes possible dumping off-cpu samples on-the-fly, directly into
> > perf ring buffer. And it dispatches those samples to the correct format
> > for perf.data consumers.
> >
> > Before:
> > ```
> >      migration/0      21 [000] 27981.041319: 2944637851    cycles:P:  ffffffff90d2e8aa record_times+0xa ([kernel.kallsyms])
> >             perf  770116 [001] 27981.041375:          1    cycles:P:  ffffffff90ee4960 event_function+0xf0 ([kernel.kallsyms])
> >             perf  770116 [001] 27981.041377:          1    cycles:P:  ffffffff90c184b1 intel_bts_enable_local+0x31 ([kernel.kallsyms])
> >             perf  770116 [001] 27981.041379:      51611    cycles:P:  ffffffff91a160b0 native_sched_clock+0x30 ([kernel.kallsyms])
> >      migration/1      26 [001] 27981.041400: 4227682775    cycles:P:  ffffffff90d06a74 wakeup_preempt+0x44 ([kernel.kallsyms])
> >      migration/2      32 [002] 27981.041477: 4159401534    cycles:P:  ffffffff90d11993 update_load_avg+0x63 ([kernel.kallsyms])
> >
> > sshd  708098 [000] 18446744069.414584:     286392 offcpu-time:
> >           79a864f1c8bb ppoll+0x4b (/usr/lib/libc.so.6)
> >           585690935cca [unknown] (/usr/bin/sshd)
> > ```
> >
> > After:
> > ```
> >             perf  774767 [003] 28178.033444:        497           cycles:P:  ffffffff91a160c3 native_sched_clock+0x43 ([kernel.kallsyms])
> >             perf  774767 [003] 28178.033445:     399440           cycles:P:  ffffffff91c01f8d nmi_restore+0x25 ([kernel.kallsyms])
> >          swapper       0 [001] 28178.036639:  376650973           cycles:P:  ffffffff91a1ae99 intel_idle+0x59 ([kernel.kallsyms])
> >          swapper       0 [003] 28178.182921:  348779378           cycles:P:  ffffffff91a1ae99 intel_idle+0x59 ([kernel.kallsyms])
> >     blueman-tray    1355 [000] 28178.627906:  100184571 offcpu-time:
> >           7528eef1c39d __poll+0x4d (/usr/lib/libc.so.6)
> >           7528edf7d8fd [unknown] (/usr/lib/libglib-2.0.so.0.8000.2)
> >           7528edf1af95 g_main_context_iteration+0x35 (/usr/lib/libglib-2.0.so.0.8000.2)
> >           7528eda4ab86 g_application_run+0x1f6 (/usr/lib/libgio-2.0.so.0.8000.2)
> >           7528ee6aa596 [unknown] (/usr/lib/libffi.so.8.1.4)
> >           7fff24e862d8 [unknown] ([unknown])
> >
> >
> >     blueman-tray    1355 [000] 28178.728137:  100187539 offcpu-time:
> >           7528eef1c39d __poll+0x4d (/usr/lib/libc.so.6)
> >           7528edf7d8fd [unknown] (/usr/lib/libglib-2.0.so.0.8000.2)
> >           7528edf1af95 g_main_context_iteration+0x35 (/usr/lib/libglib-2.0.so.0.8000.2)
> >           7528eda4ab86 g_application_run+0x1f6 (/usr/lib/libgio-2.0.so.0.8000.2)
> >           7528ee6aa596 [unknown] (/usr/lib/libffi.so.8.1.4)
> >           7fff24e862d8 [unknown] ([unknown])
> >
> >
> >          swapper       0 [000] 28178.463253:  195945410           cycles:P:  ffffffff91a1ae99 intel_idle+0x59 ([kernel.kallsyms])
> >      dbus-broker     412 [002] 28178.464855:  376737008           cycles:P:  ffffffff91c000a0 entry_SYSCALL_64+0x20 ([kernel.kallsyms])
> > ```
> >
> > Howard Chu (10):
> >   perf record --off-cpu: Add --off-cpu-thresh option
> >   perf evsel: Expose evsel__is_offcpu_event() for future use
> >   perf record --off-cpu: Parse off-cpu event
> >   perf record --off-cpu: Preparation of off-cpu BPF program
> >   perf record --off-cpu: Dump off-cpu samples in BPF
> >   perf evsel: Assemble offcpu samples
> >   perf record --off-cpu: Disable perf_event's callchain collection
> >   perf script: Display off-cpu samples correctly
> >   perf record --off-cpu: Dump the remaining samples in BPF's stack trace
> >     map
> >   perf test: Add direct off-cpu test
> >
> >  tools/perf/builtin-record.c             |  26 ++++++
> >  tools/perf/builtin-script.c             |   4 +-
> >  tools/perf/tests/builtin-test.c         |   1 +
> >  tools/perf/tests/shell/record_offcpu.sh |  31 ++++++-
> >  tools/perf/tests/tests.h                |   1 +
> >  tools/perf/tests/workloads/Build        |   1 +
> >  tools/perf/tests/workloads/offcpu.c     |  16 ++++
> >  tools/perf/util/bpf_off_cpu.c           | 115 ++++++++++++++----------
> >  tools/perf/util/bpf_skel/off_cpu.bpf.c  |  85 ++++++++++++++++--
> >  tools/perf/util/evsel.c                 |  34 ++++++-
> >  tools/perf/util/evsel.h                 |   2 +
> >  tools/perf/util/off_cpu.h               |   3 +-
> >  tools/perf/util/record.h                |   1 +
> >  13 files changed, 264 insertions(+), 56 deletions(-)
> >  create mode 100644 tools/perf/tests/workloads/offcpu.c
> >
> > --
> > 2.43.0