-
Notifications
You must be signed in to change notification settings - Fork 6.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
subsys: samples: doc: arch: Make perf subsystem #72890
base: main
Are you sure you want to change the base?
subsys: samples: doc: arch: Make perf subsystem #72890
Conversation
Hello @KushnerovMikhail, and thank you very much for your first pull request to the Zephyr project! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✨.Beautiful ✨.
doc/services/debugging/perf.rst
Outdated
|
||
.. code-block:: bash | ||
./FlameGraph/flamegraph.pl perf_buf.foolded > graph.svg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
./FlameGraph/flamegraph.pl perf_buf.foolded > graph.svg | |
./FlameGraph/flamegraph.pl perf_buf.folded > graph.svg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the comment. Corrected it.
doc/services/debugging/perf.rst
Outdated
|
||
.. code-block:: bash | ||
python zephyr/scripts/perf/stackcollapse.py perf_buf <build_dir>/zephyr/zephyr.elf > perf_buf.foolded |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
python zephyr/scripts/perf/stackcollapse.py perf_buf <build_dir>/zephyr/zephyr.elf > perf_buf.foolded | |
python zephyr/scripts/perf/stackcollapse.py perf_buf <build_dir>/zephyr/zephyr.elf > perf_buf.folded |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the comment. Corrected it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please split the commit into multiple commits, i.e. 1 commit per arch, docs, adding the subsystem code and finally the sample.
Also, perf itself does not qualify as standalone subsystem. I suggest putting this under subsys/profiling/
830c289
to
20d23f2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ARM64 supports stack unwinding as well. The docs can use some review from @kartben
subsys/profiling/Kconfig
Outdated
@@ -0,0 +1,14 @@ | |||
# cOPYRIGHT (C) 2023 kns gROUP llc (yadro) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This and other similar ones, be consistent
# cOPYRIGHT (C) 2023 kns gROUP llc (yadro) | |
# Copyright (c) 2023 KNS Group LLC (YADRO) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know how I let this happen. Thanks for the comment. Corrected it.
subsys/profiling/Kconfig
Outdated
menuconfig PROFILING | ||
bool "Profiling tools" | ||
help | ||
Enable profiling tools, such as perf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This and other Kconfig files, fix indent
Enable profiling tools, such as perf | |
Enable profiling tools, such as perf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the comment. Corrected it.
subsys/profiling/perf/Kconfig
Outdated
|
||
config PROFILING_PERF | ||
bool "Perf support" | ||
default n |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
n is already default
default n |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrected it.
subsys/profiling/perf/Kconfig
Outdated
depends on !OMIT_FRAME_POINTER | ||
depends on OVERRIDE_FRAME_POINTER_DEFAULT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can use CONFIG_FRAME_POINTER
after #72646 is merged
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Сhanged code to use a CONFIG_FRAME_POINTER
.
subsys/profiling/perf/perf.c
Outdated
perf_data.idx += trace_length; | ||
} else { | ||
--perf_data.idx; | ||
printf("perf buf override!\n"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should print to the shell, have a look at k_timer_user_data_set
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to create an additional handler thread because the shell_print does not work in interrupts.
I hope I understood correctly how you wanted me to use k_timer_user_data_set
.
subsys/profiling/perf/perf.c
Outdated
|
||
k_timer_start(&perf_data.timer, K_NO_WAIT, K_NSEC(period)); | ||
|
||
printf("Enabled perf\n"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should print to the shell
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrected it. It prints to shell now.
subsys/profiling/perf/perf.c
Outdated
|
||
int cmd_perf_print(const struct shell *sh, size_t argc, char **argv) | ||
{ | ||
printf("Perf buf length %zu\n", perf_data.idx); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please print to shell
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrected it. It prints to shell now.
subsys/profiling/perf/perf.c
Outdated
} | ||
|
||
SHELL_STATIC_SUBCMD_SET_CREATE(m_sub_perf, | ||
SHELL_CMD_ARG(record, NULL, "Start recording for X ms on Y Hz", cmd_perf_record, 3, 0), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe something like
#define CMD_HELP_RECORD \
"Start recording for <duration> ms on <frequency> Hz\n" \
"Usage: record <duration> <frequency>\n"
SHELL_CMD_ARG(record, NULL, "Start recording for X ms on Y Hz", cmd_perf_record, 3, 0), | |
SHELL_CMD_ARG(record, NULL, CMD_HELP_RECORD, cmd_perf_record, 3, 0), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your help string is really better then mine. Thanks for help
arch/riscv/core/perf.c
Outdated
} | ||
|
||
/* | ||
* This function use frame poiters to unwind stack and get trace of return addresses. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* This function use frame poiters to unwind stack and get trace of return addresses. | |
* This function use frame pointers to unwind stack and get trace of return addresses. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrected it.
* Return addresses are translated in corresponding function's names using .elf file. | ||
* So we get function call trace | ||
*/ | ||
size_t arch_perf_current_stack_trace(uintptr_t *buf, size_t size) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any chance that this and https://github.com/zephyrproject-rtos/zephyr/blob/main/arch/riscv/core/stacktrace.c can share some of the unwinding logics?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #73587
Add profiling subsystem. Add perf util based on periodic stack unwinding. Perf from Linux was taken as a reference. The operation of module is based on frame pointer usage and saving registers during interruption handling. The unwinding function stay in timer as expiry functioin so is called during interruption handling. Thus the function have access to saved registers (program counter and frame pointer in particular) of the current thread and use it to unwind the thread stack. Timer starting and results printing function are made as shell commands for conveniency. Originally-by: Yonatan Goldschmidt <yon.goldschmidt@gmail.com> Signed-off-by: Mikhail Kushnerov <m.kushnerov@yadro.com>
Implement stack trace function for riscv arch, that get required thread register values and unwind stack with it. Signed-off-by: Mikhail Kushnerov <m.kushnerov@yadro.com>
Implement stack trace function for x86_64 arch, that get required thread register values and unwind stack with it. Originally-by: Yonatan Goldschmidt <yon.goldschmidt@gmail.com> Signed-off-by: Mikhail Kushnerov <m.kushnerov@yadro.com>
Samples, that were obtained by profiling perf tool, can be be translated into flamegraph using stackcollapse.py script. Originally-by: Yonatan Goldschmidt <yon.goldschmidt@gmail.com> Signed-off-by: Mikhail Kushnerov <m.kushnerov@yadro.com>
Add doc for profiling perf tool Signed-off-by: Mikhail Kushnerov <m.kushnerov@yadro.com>
The operation of perf tool can be checked using this sample. Signed-off-by: Mikhail Kushnerov <m.kushnerov@yadro.com>
20d23f2
to
4fe8184
Compare
Add profiling util based on periodic stack unwinding. Perf from Linux was taken as a reference.
Code is based on Sampling profiler (#29304) by @Jongy.
The operation of module is based on frame pointer usage and saving registers during interruption handling by zephyr.
General work description:
The unwinding function stay in timer as expiry function so is called during interruption handling. Thus the function have access to saved registers (program counter and frame pointer in particular) of the current thread and use it to unwind the thread stack.
Timer starting and results printing function are made as shell commands for conveniency.
Obtained samples can be translated into flamegraph using stackcollapse.py script.
Flame graph example, generated from
echo_server
sample:Kconfig optioins:
CONFIG_PERF
enables perf subsystem in compilation.CONFIG_PERF_BUFFER_SIZE
specifies size of buffer, where samples are stored.Kconfig requirements:
CONFIG_THREAD_STACK_INFO=y
providesstart
andsize
thread stack values.CONFIG_SMP=n
SMP support is not implemented yet.CONFIG_OMIT_FRAME_POINTER=n
&&CONFIG_OVERRIDE_FRAME_POINTER_DEFAULT=y
provide frame pointer.CONFIG_SHELL=y
needs because subsystem provides shell commands.CONFIG_RISCV=y
||CONFIG_X86_64=y
- subsystem implemented only for x86_64 and riscv yet.