[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAH0uvogRFE_gXhSQTbXJiu-Yj2b+H1U4K40bDOgwTFyTOtHFDA@mail.gmail.com>
Date: Mon, 29 Jul 2024 23:24:47 +0800
From: Howard Chu <howardchu95@...il.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: irogers@...gle.com, acme@...nel.org, adrian.hunter@...el.com,
jolsa@...nel.org, kan.liang@...ux.intel.com, linux-perf-users@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 0/5] Dump off-cpu samples directly
Hello Namhyung,
Thanks for reviewing this patch.
On Mon, Jul 29, 2024 at 9:21 AM Namhyung Kim <namhyung@...nel.org> wrote:
>
> Hello Howard,
>
> On Fri, Jul 26, 2024 at 06:28:21PM +0800, Howard Chu wrote:
> > 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.
>
> Thanks for your work!
>
> But I'm not sure we need a separate event for offcpu-time-direct. If we
> fix the format for the direct event, we can adjust the format of offcpu-
> time when it dumps at the end.
Thank you and Ian for this advice, I'll do that.
>
> Anyway, as far as I can see you don't need to fill the sample info in
> the offcpu-time-direct manually in your BPF program. Because the
> bpf_perf_event_output() will call perf_event_output() which fills all
> the sample information according to the sample_type flags.
>
> Well.. it'll set IP to the schedule function, but it should be ok.
> (updating IP using CALLCHAIN like in off_cpu_write() is a kinda hack and
> not absoluately necessary, probably I can get rid of it.. Let's go with
> simple for now.)
>
> So I think what you need is to ensure it has the uncessary flags. And
> the only info it needs to fill is the time between the previous schedule
> and this can be added to the raw data.
Sure thing, thank you. Other than the off-cpu duration, do you think
we should collect the callchain as well?
This idea is great because it minimizes the data we need to transfer
from kernel space to user space. But with this, the whole
"full-fledged sample in BPF output" is not the case anymore, because
we only take a couple of things(such as the time between the
schedules) from the raw data. This idea is more like an extension of
the sample data, I think it's interesting, will try to implement it.
Thanks,
Howard
>
> Thanks,
> Namhyung
>
> >
> > 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
> >
> > 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-direct:
> > 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-direct:
> > 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 (5):
> > perf record off-cpu: Add direct off-cpu event
> > perf record off-cpu: Dumping samples in BPF
> > perf record off-cpu: processing of embedded sample
> > perf record off-cpu: save embedded sample type
> > perf record off-cpu: Add direct off-cpu test
> >
> > tools/perf/builtin-record.c | 2 +
> > tools/perf/builtin-script.c | 2 +-
> > tools/perf/tests/builtin-test.c | 1 +
> > tools/perf/tests/shell/record_offcpu.sh | 29 +++++
> > 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 | 53 ++++++++-
> > tools/perf/util/bpf_skel/off_cpu.bpf.c | 143 ++++++++++++++++++++++++
> > tools/perf/util/evsel.c | 16 ++-
> > tools/perf/util/evsel.h | 13 +++
> > tools/perf/util/header.c | 12 ++
> > tools/perf/util/off_cpu.h | 1 +
> > tools/perf/util/record.h | 1 +
> > tools/perf/util/session.c | 23 +++-
> > 15 files changed, 309 insertions(+), 5 deletions(-)
> > create mode 100644 tools/perf/tests/workloads/offcpu.c
> >
> > --
> > 2.45.2
> >
Powered by blists - more mailing lists