mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Namhyung Kim <namhyung@kernel.org>
To: Aaron Tomlin <atomlin@atomlin.com>
Cc: peterz@infradead.org, mingo@redhat.com, acme@kernel.org,
	mark.rutland@arm.com, alexander.shishkin@linux.intel.com,
	jolsa@kernel.org, irogers@google.com, adrian.hunter@intel.com,
	chjohnst@mail.com, neelx@suse.com, sean@ashe.io,
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH perf-tools-next v12 06/10] perf ftrace: Support display of inlined functions in function graph tracer
Date: Tue, 3 Nov 2026 23:46:49 -0800	[thread overview]
Message-ID: <auri6Q6oisHLi9Y6@z2> (raw)
In-Reply-To: <zri4x5jfbpykc7jxdicixkrjdw4f7tui72t7is3mos6cnzbhj5@2wct4vkl7sno>

On Mon, Nov 02, 2026 at 02:20:47PM -0500, Aaron Tomlin wrote:
[SNIP]
> > > +		cs->stack[cs->depth].inlined = true;
> > > +		cs->stack[cs->depth].indent = entry_indent;
> > > +		cs->stack[cs->depth].raw_indent = -1;
> > 
> > I'm curious if it's possible to get rid of the indent and raw_indent.
> > I think you can get the indent from the depth easily.  The raw indent
> > would be from any non-inlined parent if needed.  Also having inlined
> > depth in the cs/ts seems unnecessary.  What do you think?
> 
> We still need to record the trace line's indentation rather than deriving
> it purely from ts->depth * 2 because tracing can start mid-callchain (or
> after ring-buffer drops), where ts->depth == 0 has a non-zero kernel
> indentation (e.g. 6 or 8 spaces), and bare "}" exits must be matched
> against that kernel indentation.

Right, but it may start from non-zero depth then.  Probably save the
starting depth in the ts struct.

> 
> However, we do not need both indent and raw_indent on struct
> ftrace_stack_frame, nor do we need inlined_depth in struct
> ftrace_task_state:
> 
>     1.  In struct ftrace_stack_frame, indent was only used for inlined
>         frames and raw_indent was only used for non-inlined frames, so they
>         can be consolidated into a single indent field.
> 
>     2.  In struct ftrace_task_state, inlined_depth can be dropped
>         altogether and computed from the active inlined frames on ts->stack
>         when needed.
> 
> I will simplify both structures accordingly.

Thanks!

> 
> > > +#define FTRACE_MAX_INLINES 32
> > 
> > Isn't 32 too big?  I guess 8 would be enough..
> 
> No, 8 is not enough in practice. I will prefer to keep 32.
> For instance, after a -O2 kernel build:
> 
> ❯ addr2line -f -i -e vmlinux 0xffffffff816916e9
>       __do_trace_rss_stat
>       include/trace/events/kmem.h:427 (discriminator 20)
>       trace_rss_stat
>       include/trace/events/kmem.h:427 (discriminator 20)
>       mm_trace_rss_stat
>       mm/memory.c:169 (discriminator 20)
>       add_mm_counter
>       include/linux/mm.h:3469 (discriminator 20)
>       add_mm_rss_vec
>       mm/memory.c:492 (discriminator 20)
>       zap_pte_range
>       mm/memory.c:2087 (discriminator 20)
>       zap_pmd_range
>       mm/memory.c:2151 (discriminator 20)
>       zap_pud_range
>       mm/memory.c:2179 (discriminator 20)
>       zap_p4d_range
>       mm/memory.c:2200 (discriminator 20)
>       __zap_vma_range
>       mm/memory.c:2240 (discriminator 20)
> 
> Another example:
> 
> ❯ addr2line -f -i -e vmlinux 0xffffffff821c34af
>       constant_test_bit
>       arch/x86/include/asm/bitops.h:202
>       arch_test_bit
>       arch/x86/include/asm/bitops.h:232
>       _test_bit
>       include/asm-generic/bitops/instrumented-non-atomic.h:142
>       test_ti_thread_flag
>       include/linux/thread_info.h:133
>       test_tsk_thread_flag
>       include/linux/sched.h:2102
>       task_sigpending
>       include/linux/sched/signal.h:384
>       fatal_signal_pending
>       include/linux/sched/signal.h:406
>       ptrace_report_syscall
>       include/linux/ptrace.h:427
>       ptrace_report_syscall_exit
>       include/linux/ptrace.h:477
>       arch_ptrace_report_syscall_exit
>       include/linux/entry-common.h:233
>       syscall_exit_work
>       include/linux/entry-common.h:266
>       syscall_exit_to_user_mode_work
>       include/linux/entry-common.h:297
>       syscall_exit_to_user_mode
>       include/linux/entry-common.h:334
>       do_syscall_64
>       arch/x86/entry/syscall_64.c:89

Oh, indeed.  Thanks for providing this!

Namhyung


  reply	other threads:[~2026-11-04  7:46 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-26  2:57 [PATCH perf-tools-next v12 00/10] perf ftrace: Support inlined functions and display enhancements " Aaron Tomlin
2026-10-26  2:57 ` [PATCH perf-tools-next v12 01/10] perf ftrace: Handle signals, non-blocking reads and pager exits in __cmd_ftrace() Aaron Tomlin
2026-10-31  7:12   ` Namhyung Kim
2026-10-31 22:29     ` Aaron Tomlin
2026-10-26  2:57 ` [PATCH perf-tools-next v12 02/10] perf ftrace: Optimise __cmd_ftrace() stack frame with strbuf streaming Aaron Tomlin
2026-10-26  2:57 ` [PATCH perf-tools-next v12 03/10] perf ftrace: Support filtering return address comments Aaron Tomlin
2026-10-26  2:57 ` [PATCH perf-tools-next v12 04/10] perf ftrace: Add basic task call stack management Aaron Tomlin
2026-10-31  7:05   ` Namhyung Kim
2026-10-31 22:27     ` Aaron Tomlin
2026-11-04  7:35       ` Namhyung Kim
2026-10-26  2:57 ` [PATCH perf-tools-next v12 05/10] perf ftrace: Support context switch tracking in function graph tracer Aaron Tomlin
2026-10-31 22:01   ` Namhyung Kim
2026-11-02  0:04     ` Aaron Tomlin
2026-11-04  7:42       ` Namhyung Kim
2026-10-26  2:57 ` [PATCH perf-tools-next v12 06/10] perf ftrace: Support display of inlined functions " Aaron Tomlin
2026-10-31  7:11   ` Namhyung Kim
2026-11-02 15:22     ` Aaron Tomlin
2026-10-31 23:33   ` Namhyung Kim
2026-11-02 19:20     ` Aaron Tomlin
2026-11-04  7:46       ` Namhyung Kim [this message]
2026-10-26  2:57 ` [PATCH perf-tools-next v12 07/10] perf symbol: Introduce vmlinux_path__find() helper Aaron Tomlin
2026-10-26  2:57 ` [PATCH perf-tools-next v12 08/10] perf test: Add test for ftrace inline and task stack tracking Aaron Tomlin
2026-10-26  2:57 ` [PATCH perf-tools-next v12 09/10] perf ftrace: Support omitting execution duration in function graph tracer Aaron Tomlin
2026-10-26  2:57 ` [PATCH perf-tools-next v12 10/10] perf ftrace: Filter extraneous functions " Aaron Tomlin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=auri6Q6oisHLi9Y6@z2 \
    --to=namhyung@kernel.org \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=atomlin@atomlin.com \
    --cc=chjohnst@mail.com \
    --cc=irogers@google.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=neelx@suse.com \
    --cc=peterz@infradead.org \
    --cc=sean@ashe.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®