mbox series

[v9,0/8] function_graph: Support recording and printing the return value of function

Message ID cover.1680176068.git.pengdonglin@sangfor.com.cn (mailing list archive)
Headers show
Series function_graph: Support recording and printing the return value of function | expand

Message

pengdonglin March 30, 2023, 11:39 a.m. UTC
When using the function_graph tracer to analyze system call failures,
it can be time-consuming to analyze the trace logs and locate the kernel
function that first returns an error. This change aims to simplify the
process by recording the function return value to the 'retval' member of
'ftrace_graph_ent' and printing it when outputing the trace log.

Note that even if a function's return type is void, a return value will
still be printed, so it should be ignored. If you care about this, the
BTF file can be used to obtain the details of function return type. We
can implement a tool to process the trace log and display the return
value based on its actual type.

Here is an example:

...

 1)               |  cgroup_attach_task() {
 1)               |    cgroup_migrate_add_src() {
 1)   1.403 us    |      cset_cgroup_from_root(); /* = 0xffff93fc86f58010 */
 1)   2.154 us    |    } /* cgroup_migrate_add_src = 0xffffb286c1297d00 */
 1) ! 386.538 us  |    cgroup_migrate_prepare_dst(); /* = 0x0 */
 1)               |    cgroup_migrate() {
 1)   0.651 us    |      cgroup_migrate_add_task(); /* = 0xffff93fcfd346c00 */
 1)               |      cgroup_migrate_execute() {
 1)               |        cpu_cgroup_can_attach() {
 1)               |          cgroup_taskset_first() {
 1)   0.732 us    |            cgroup_taskset_next(); /* = 0xffff93fc8fb20000 */
 1)   1.232 us    |          } /* cgroup_taskset_first = 0xffff93fc8fb20000 */
 1)   0.380 us    |          sched_rt_can_attach(); /* = 0x0 */
 1)   2.335 us    |        } /* cpu_cgroup_can_attach = -22 */
 1)   4.369 us    |      } /* cgroup_migrate_execute = -22 */
 1)   7.143 us    |    } /* cgroup_migrate = -22 */
 1)               |    cgroup_migrate_finish() {
 1)   0.411 us    |      put_css_set_locked(); /* = 0x8 */
 1) + 62.397 us   |      put_css_set_locked(); /* = 0x80000001 */
 1) + 64.742 us   |    } /* cgroup_migrate_finish = 0x80000000 */
 1) ! 465.605 us  |  } /* cgroup_attach_task = -22 */

...

After processing the above trace logs using BTF information:

...

 1)               |  cgroup_attach_task() {
 1)               |    cgroup_migrate_add_src() {
 1)   1.403 us    |      cset_cgroup_from_root(); /* = 0xffff93fc86f58010 */
 1)   2.154 us    |    } /* cgroup_migrate_add_src */
 1) ! 386.538 us  |    cgroup_migrate_prepare_dst(); /* = 0 */
 1)               |    cgroup_migrate() {
 1)   0.651 us    |      cgroup_migrate_add_task();
 1)               |      cgroup_migrate_execute() {
 1)               |        cpu_cgroup_can_attach() {
 1)               |          cgroup_taskset_first() {
 1)   0.732 us    |            cgroup_taskset_next(); /* = 0xffff93fc8fb20000 */
 1)   1.232 us    |          } /* cgroup_taskset_first = 0xffff93fc8fb20000 */
 1)   0.380 us    |          sched_rt_can_attach(); /* = 0 */
 1)   2.335 us    |        } /* cpu_cgroup_can_attach = -22 */
 1)   4.369 us    |      } /* cgroup_migrate_execute = -22 */
 1)   7.143 us    |    } /* cgroup_migrate = -22 */
 1)               |    cgroup_migrate_finish() {
 1)   0.411 us    |      put_css_set_locked();
 1) + 62.397 us   |      put_css_set_locked();
 1) + 64.742 us   |    } /* cgroup_migrate_finish */
 1) ! 465.605 us  |  } /* cgroup_attach_task = -22 */

...

---
v9:
 - Fix align issues in ARM asm code
 - Fix align issues in LoongArch asm code
 - Update commit messages
 - Update comments for ftrace_return_to_handler

v8:
 - Fix issues in ARM64 asm code
 - Fix issues in selftest
 - Add some comments on CONFIG_HAVE_FUNCTION_GRAPH_RETVAL
 - Make CONFIG_FUNCTION_GRAPH_RETVAL switable
 - Modify the control range of CONFIG_HAVE_FUNCTION_GRAPH_RETVAL

v7:
 - Rename trace option 'graph_retval_hex' to 'funcgraph-retval-hex'
 - Introduce a new structure fgraph_ret_regs for each architecture to
   hold return registers
 - Separate each architecture modification info individual patches
 - Add a test case for funcgraph-retval
 - Update documentation description
 - Support LoongArch

v6:
 - Remove the conversion code for short and char types, because these
   two types are rarely used to store an error code.
 - Modify the limitations for funcgraph-retval
 - Optimize the English expression

v5:
 - Pass both the return values to ftrace_return_to_handler
 - Modify the parameter sequence of ftrace_return_to_handler to
   decrease the modification of assembly code, thanks to Russell King
 - Wrap __ftrace_return_to_handler with ftrace_return_to_handler
   for compatible
 - Describe the limitations of funcgraph-retval

v4:
 - Modify commit message
 - Introduce new option graph_retval_hex to control display format
 - Introduce macro CONFIG_FUNCTION_GRAPH_RETVAL and
   CONFIG_HAVE_FUNCTION_GRAPH_RETVAL
 - Add related arch maintainers to review

v3:
 - Modify the commit message: add trace logs processed with the btf tool

v2:
 - Modify the commit message: use BTF to get the return type of function

Donglin Peng (8):
  function_graph: Support recording and printing the return value of
    function
  tracing: Add documentation for funcgraph-retval and
    funcgraph-retval-hex
  ARM: ftrace: Enable HAVE_FUNCTION_GRAPH_RETVAL
  arm64: ftrace: Enable HAVE_FUNCTION_GRAPH_RETVAL
  riscv: ftrace: Enable HAVE_FUNCTION_GRAPH_RETVAL
  x86/ftrace: Enable HAVE_FUNCTION_GRAPH_RETVAL
  LoongArch: ftrace: Enable HAVE_FUNCTION_GRAPH_RETVAL
  selftests/ftrace: Add funcgraph-retval test case

 Documentation/trace/ftrace.rst                | 74 +++++++++++++++
 arch/arm/Kconfig                              |  1 +
 arch/arm/include/asm/ftrace.h                 | 20 ++++
 arch/arm/kernel/asm-offsets.c                 |  6 +-
 arch/arm/kernel/entry-ftrace.S                | 10 +-
 arch/arm64/Kconfig                            |  1 +
 arch/arm64/include/asm/ftrace.h               | 20 ++++
 arch/arm64/kernel/asm-offsets.c               | 12 +++
 arch/arm64/kernel/entry-ftrace.S              | 27 +++---
 arch/loongarch/Kconfig                        |  1 +
 arch/loongarch/include/asm/ftrace.h           | 20 ++++
 arch/loongarch/kernel/asm-offsets.c           | 12 +++
 arch/loongarch/kernel/mcount.S                | 14 +--
 arch/loongarch/kernel/mcount_dyn.S            | 15 +--
 arch/riscv/Kconfig                            |  1 +
 arch/riscv/include/asm/ftrace.h               | 19 ++++
 arch/riscv/kernel/mcount.S                    |  7 +-
 arch/x86/Kconfig                              |  1 +
 arch/x86/include/asm/ftrace.h                 | 18 ++++
 arch/x86/kernel/ftrace_32.S                   |  8 +-
 arch/x86/kernel/ftrace_64.S                   |  7 +-
 include/linux/ftrace.h                        |  3 +
 kernel/trace/Kconfig                          | 15 +++
 kernel/trace/fgraph.c                         | 23 ++++-
 kernel/trace/trace.h                          |  2 +
 kernel/trace/trace_entries.h                  | 26 ++++++
 kernel/trace/trace_functions_graph.c          | 93 +++++++++++++++++--
 .../ftrace/test.d/ftrace/fgraph-retval.tc     | 39 ++++++++
 28 files changed, 441 insertions(+), 54 deletions(-)
 create mode 100644 tools/testing/selftests/ftrace/test.d/ftrace/fgraph-retval.tc

Comments

Borislav Petkov March 30, 2023, 1:59 p.m. UTC | #1
On Thu, Mar 30, 2023 at 04:39:07AM -0700, Donglin Peng wrote:
> v9:
>  - Fix align issues in ARM asm code
>  - Fix align issues in LoongArch asm code
>  - Update commit messages
>  - Update comments for ftrace_return_to_handler
> 
> v8:
>  - Fix issues in ARM64 asm code
>  - Fix issues in selftest
>  - Add some comments on CONFIG_HAVE_FUNCTION_GRAPH_RETVAL
>  - Make CONFIG_FUNCTION_GRAPH_RETVAL switable
>  - Modify the control range of CONFIG_HAVE_FUNCTION_GRAPH_RETVAL

Please control your spamming:

From: Documentation/process/submitting-patches.rst

Don't get discouraged - or impatient
------------------------------------

After you have submitted your change, be patient and wait.  Reviewers are
busy people and may not get to your patch right away.

Once upon a time, patches used to disappear into the void without comment,
but the development process works more smoothly than that now.  You should
receive comments within a week or so; if that does not happen, make sure
that you have sent your patches to the right place.  Wait for a minimum of
						     ^^^^^^^^^^^^^^^^^^^^^

one week before resubmitting or pinging reviewers - possibly longer during
^^^^^^^^^^^^^^^^^^^^^^^^^^^^

busy times like merge windows.
Steven Rostedt March 30, 2023, 2:15 p.m. UTC | #2
On Thu, 30 Mar 2023 15:59:48 +0200
Borislav Petkov <bp@alien8.de> wrote:

> Please control your spamming:
> 
> From: Documentation/process/submitting-patches.rst
> 
> Don't get discouraged - or impatient
> ------------------------------------
> 
> After you have submitted your change, be patient and wait.  Reviewers are
> busy people and may not get to your patch right away.
> 
> Once upon a time, patches used to disappear into the void without comment,
> but the development process works more smoothly than that now.  You should
> receive comments within a week or so; if that does not happen, make sure
> that you have sent your patches to the right place.  Wait for a minimum of
> 						     ^^^^^^^^^^^^^^^^^^^^^
> 
> one week before resubmitting or pinging reviewers - possibly longer during
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> busy times like merge windows.

To be fair, this isn't a ping or a resend. It's addressing comments that
were given in v8 that was sent out on Tuesday.

This isn't v1, where I would agree with waiting a week for comments. v1 was
sent back in 3/6. I do not expect (nor want) someone to wait a week on v8
for all feedback to come in, unless it was a redesign of the code. But it's
now starting to settle, and that "week waiting" period isn't necessary for
fixing up minor suggestions by the reviewers.

I was hoping this would be good to go by the end of the week, so I could
get it into linux-next by next week.

-- Steve