[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZzOpvzN-OTLZPyFh@x1>
Date: Tue, 12 Nov 2024 16:17:19 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Howard Chu <howardchu95@...il.com>
Cc: peterz@...radead.org, namhyung@...nel.org, irogers@...gle.com,
mingo@...hat.com, mark.rutland@....com, james.clark@...aro.org,
alexander.shishkin@...ux.intel.com, jolsa@...nel.org,
adrian.hunter@...el.com, kan.liang@...ux.intel.com,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7 00/10] perf record --off-cpu: Dump off-cpu samples
directly
On Tue, Nov 12, 2024 at 03:39:25PM -0300, Arnaldo Carvalho de Melo wrote:
> ⬢ [acme@...lbox 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@...lbox 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@...lbox 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@...lbox perf-tools-next]$
>
> Then try to apply just the 5th patch:
>
> ⬢ [acme@...lbox perf-tools-next]$ b4 am -P5 -ctsl --cc-trailers 20241108204137.2444151-2-howardchu95@...il.com
> ⬢ [acme@...lbox 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@...lbox 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@...lbox 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);
Powered by blists - more mailing lists