Re: [PATCH perf-tools-next v4 1/5] perf ftrace: Optimise __cmd_ftrace() stack frame with strbuf streaming

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf: premature termination on EAGAIN during ftrace polling
--

commit 0da8f6d83b353c321a754b6791e275b195595b54
Author: Aaron Tomlin <atomlin@atomlin.com>

perf ftrace: Optimise __cmd_ftrace() stack frame with strbuf streaming

This commit optimizes __cmd_ftrace() by replacing a fixed stack buffer
with dynamic heap allocation and a strbuf to accumulate streamed characters
into whole lines. This reduces stack usage significantly and adds error
and EOF detection on stdout and the trace stream.

> diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
> index 6017493ff1797..efd5343dc9adc 100644
> --- a/tools/perf/builtin-ftrace.c
> +++ b/tools/perf/builtin-ftrace.c
[ ... ]
> @@ -810,20 +822,30 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace)
>  			break;
>  
>  		if (pollfd.revents & POLLIN) {
> -			int n = read(trace_fd, buf, sizeof(buf));
> -			if (n < 0)
> -				break;
> -			if (fwrite(buf, n, 1, stdout) != 1)
> +			int n = read(trace_fd, buf, TRACE_BUF_SIZE);
> +
> +			if (n <= 0)
>  				break;

[Severity: Medium]
Since trace_pipe is opened in O_NONBLOCK mode, can this lead to premature
termination of the perf ftrace tracing session?

If a spurious wakeup occurs for poll(), or if a non-fatal signal interrupts
the syscall, read() returns -1 with errno == EAGAIN or EINTR. Unconditionally
breaking out of the loop here stops tracing instead of retrying the read.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014161457.373253-1-atomlin@atomlin.com?part=1