Commit 60b1f578 authored Oct 08, 2024 by Jeff Xie Committed by Steven Rostedt (Google) Nov 18, 2024

ftrace: Get the true parent ip for function tracer

When using both function tracer and function graph simultaneously,
it is found that function tracer sometimes captures a fake parent ip
(return_to_handler) instead of the true parent ip.

This issue is easy to reproduce. Below are my reproduction steps:

jeff-labs:~/bin # ./trace-net.sh

jeff-labs:~/bin # cat /sys/kernel/debug/tracing/instances/foo/trace | grep return_to_handler
    trace-net.sh-405     [001] ...2.    31.859501: avc_has_perm+0x4/0x190 <-return_to_handler+0x0/0x40
    trace-net.sh-405     [001] ...2.    31.859503: simple_setattr+0x4/0x70 <-return_to_handler+0x0/0x40
    trace-net.sh-405     [001] ...2.    31.859503: truncate_pagecache+0x4/0x60 <-return_to_handler+0x0/0x40
    trace-net.sh-405     [001] ...2.    31.859505: unmap_mapping_range+0x4/0x140 <-return_to_handler+0x0/0x40
    trace-net.sh-405     [001] ...3.    31.859508: _raw_spin_unlock+0x4/0x30 <-return_to_handler+0x0/0x40
    [...]

The following is my simple trace script:

<snip>
jeff-labs:~/bin # cat ./trace-net.sh
TRACE_PATH="/sys/kernel/tracing"

set_events() {
        echo 1 > $1/events/net/enable
        echo 1 > $1/events/tcp/enable
        echo 1 > $1/events/sock/enable
        echo 1 > $1/events/napi/enable
        echo 1 > $1/events/fib/enable
        echo 1 > $1/events/neigh/enable
}

set_events ${TRACE_PATH}
echo 1 > ${TRACE_PATH}/options/sym-offset
echo 1 > ${TRACE_PATH}/options/funcgraph-tail
echo 1 > ${TRACE_PATH}/options/funcgraph-proc
echo 1 > ${TRACE_PATH}/options/funcgraph-abstime

echo 'tcp_orphan*' > ${TRACE_PATH}/set_ftrace_notrace
echo function_graph > ${TRACE_PATH}/current_tracer

INSTANCE_FOO=${TRACE_PATH}/instances/foo
if [ ! -e $INSTANCE_FOO ]; then
        mkdir ${INSTANCE_FOO}
fi
set_events ${INSTANCE_FOO}
echo 1 > ${INSTANCE_FOO}/options/sym-offset
echo 'tcp_orphan*' > ${INSTANCE_FOO}/set_ftrace_notrace
echo function > ${INSTANCE_FOO}/current_tracer

echo 1 > ${TRACE_PATH}/tracing_on
echo 1 > ${INSTANCE_FOO}/tracing_on

echo > ${TRACE_PATH}/trace
echo > ${INSTANCE_FOO}/trace
</snip>

Link: https://lore.kernel.org/20241008033159.22459-1-jeff.xie@linux.dev


Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Jeff Xie <jeff.xie@linux.dev>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>

parent 6371b4bc

kernel/trace/trace_functions.c

+26 −0

Original line number	Diff line number	Diff line
		@@ -176,6 +176,27 @@ static void function_trace_start(struct trace_array *tr)
		tracing_reset_online_cpus(&tr->array_buffer);
		}

		#ifdef CONFIG_FUNCTION_GRAPH_TRACER
		static __always_inline unsigned long
		function_get_true_parent_ip(unsigned long parent_ip, struct ftrace_regs *fregs)
		{
		unsigned long true_parent_ip;
		int idx = 0;

		true_parent_ip = parent_ip;
		if (unlikely(parent_ip == (unsigned long)&return_to_handler) && fregs)
		true_parent_ip = ftrace_graph_ret_addr(current, &idx, parent_ip,
		(unsigned long *)ftrace_regs_get_stack_pointer(fregs));
		return true_parent_ip;
		}
		#else
		static __always_inline unsigned long
		function_get_true_parent_ip(unsigned long parent_ip, struct ftrace_regs *fregs)
		{
		return parent_ip;
		}
		#endif

		static void
		function_trace_call(unsigned long ip, unsigned long parent_ip,
		struct ftrace_ops op, struct ftrace_regs fregs)
		@@ -192,6 +213,8 @@ function_trace_call(unsigned long ip, unsigned long parent_ip,
		if (bit < 0)
		return;

		parent_ip = function_get_true_parent_ip(parent_ip, fregs);

		trace_ctx = tracing_gen_ctx();

		data = this_cpu_ptr(tr->array_buffer.data);
		@@ -239,6 +262,7 @@ function_stack_trace_call(unsigned long ip, unsigned long parent_ip,
		* recursive protection is performed.
		*/
		local_irq_save(flags);
		parent_ip = function_get_true_parent_ip(parent_ip, fregs);
		cpu = raw_smp_processor_id();
		data = per_cpu_ptr(tr->array_buffer.data, cpu);
		disabled = atomic_inc_return(&data->disabled);
		@@ -306,6 +330,7 @@ function_no_repeats_trace_call(unsigned long ip, unsigned long parent_ip,
		if (bit < 0)
		return;

		parent_ip = function_get_true_parent_ip(parent_ip, fregs);
		data = this_cpu_ptr(tr->array_buffer.data);
		if (atomic_read(&data->disabled))
		goto out;
		@@ -352,6 +377,7 @@ function_stack_no_repeats_trace_call(unsigned long ip, unsigned long parent_ip,
		* recursive protection is performed.
		*/
		local_irq_save(flags);
		parent_ip = function_get_true_parent_ip(parent_ip, fregs);
		cpu = raw_smp_processor_id();
		data = per_cpu_ptr(tr->array_buffer.data, cpu);
		disabled = atomic_inc_return(&data->disabled);