Linux Trace Kernel
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Yun Zhou" <yun.zhou@windriver.com>
Cc: linux-trace-kernel@vger.kernel.org
Subject: Re: [PATCH] ftrace: add max field to function profiler stats
Date: Thu, 24 Sep 2026 03:10:16 +0000	[thread overview]
Message-ID: <20260924031016.88B601F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260924030108.2673654-1-yun.zhou@windriver.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ftrace: time_max TOCTOU data race clobbers maximum latency spikes
--

commit 9fb82451733a5ec95aac95499945b5c01e39d82b
Author: Yun Zhou <yun.zhou@windriver.com>

ftrace: add max field to function profiler stats

This commit adds a max field to the function profiler statistics to record the
maximum single-call duration. This allows users to better investigate
occasional latency spikes when reviewing function execution times.

> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 53d5db60bfa56..dfd25e3c87f71 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
[ ... ]
> @@ -920,6 +923,8 @@ static void profile_graph_return(struct ftrace_graph_ret *trace,
>  	if (rec) {
>  		rec->time += calltime;
>  		rec->time_squared += calltime * calltime;
> +		if (calltime > rec->time_max)
> +			rec->time_max = calltime;

[Severity: Medium]
Could this non-atomic check and update open a time-of-check to time-of-use
data race against concurrent nested interrupt context tracing on the same CPU?

Since profile_graph_return() is executed with local interrupts enabled, an
interrupt or NMI could fire exactly between the condition check and the
assignment. If the interrupt handler traces the identical function and
records a massive latency spike, it would update rec->time_max. When the
interrupt returns, the normal context resumes and executes the assignment,
overwriting the true maximum latency spike with its smaller value.

Because ftrace explicitly permits tracing recursion from different contexts
like hardirq or NMI, does this require an atomic compare-and-exchange loop
or similar protection to avoid dropping the genuine latency spikes this
patch intends to capture?

>  	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924030108.2673654-1-yun.zhou@windriver.com?part=1

  reply	other threads:[~2026-09-24  3:10 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-24  3:01 [PATCH] ftrace: add max field to function profiler stats Yun Zhou
2026-09-24  3:10 ` sashiko-bot [this message]
2026-09-24  5:27   ` Zhou, Yun

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=20260924031016.88B601F000FF@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=yun.zhou@windriver.com \
    /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