Re: [PATCH perf-tools-next v12 01/10] perf ftrace: Handle signals, non-blocking reads and pager exits in __cmd_ftrace()

[Namhyung Kim <namhyung@kernel.org>]

On Sun, Oct 25, 2026 at 10:57:26PM -0400, Aaron Tomlin wrote:
> In __cmd_ftrace(), trace_fd is configured for non-blocking I/O via
> fcntl(trace_fd, F_SETFL, O_NONBLOCK) and polled using poll(). However,
> the polling loop previously treated any negative return code from poll()
> or read() as an unrecoverable failure.
> 
> If poll() or read() is interrupted by a non-fatal signal (i.e. SIGWINCH
> upon terminal resizing), errno is set to EINTR. Furthermore, read() on
> trace_pipe can return EAGAIN/EWOULDBLOCK even after poll() indicates
> POLLIN (for instance, under concurrent readers, spurious wakeups, or
> block-mode trace iterator options). In all such cases, breaking from the
> loop causes perf ftrace to terminate tracing prematurely.
> 
> Additionally, output errors on stdout (i.e. when a pager or consumer
> process exits, closing the downstream pipe) were ignored by fflush(),
> leaving __cmd_ftrace() polling indefinitely until the workload
> completes.
> 
> Resolve these concerns by retrying poll() upon receiving EINTR, retrying
> read() on EAGAIN, EWOULDBLOCK, and EINTR, and breaking on EOF (n == 0).
> Furthermore, detect stdout errors and broken pipes via fflush() and
> ferror() to exit immediately when the downstream output sink closes, and
> handle EINTR and ferror(stdout) cleanly in the subsequent buffer
> draining loop.
> 
> Fixes: d01f4e8db22c ("perf ftrace: Introduce new 'ftrace' tool")
> Cc: stable@vger.kernel.org
> Tested-by: Ian Rogers <irogers@google.com>
> Reviewed-by: Ian Rogers <irogers@google.com>
> Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung

> ---
>  tools/perf/builtin-ftrace.c | 27 ++++++++++++++++++++++-----
>  1 file changed, 22 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
> index 6017493ff179..7cdc9ae2fbbb 100644
> --- a/tools/perf/builtin-ftrace.c
> +++ b/tools/perf/builtin-ftrace.c
> @@ -14,6 +14,7 @@
>  #include <poll.h>
>  #include <semaphore.h>
>  #include <signal.h>
> +#include <stdio.h>
>  #include <stdlib.h>
>  #include <unistd.h>
>  #include <linux/capability.h>
> @@ -806,17 +807,27 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace)
>  	}
>  
>  	while (!done) {
> -		if (poll(&pollfd, 1, -1) < 0)
> +		if (poll(&pollfd, 1, -1) < 0) {
> +			if (errno == EINTR)
> +				continue;
>  			break;
> +		}
>  
>  		if (pollfd.revents & POLLIN) {
>  			int n = read(trace_fd, buf, sizeof(buf));
> -			if (n < 0)
> +
> +			if (n < 0) {
> +				if (errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR)
> +					continue;
> +				break;
> +			}
> +			if (n == 0)
>  				break;
>  			if (fwrite(buf, n, 1, stdout) != 1)
>  				break;
>  			/* flush output since stdout is in full buffering mode due to pager */
> -			fflush(stdout);
> +			if (fflush(stdout) < 0 || ferror(stdout))
> +				break;
>  		}
>  	}
>  
> @@ -831,9 +842,15 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace)
>  	}
>  
>  	/* read remaining buffer contents */
> -	while (true) {
> +	while (!ferror(stdout)) {
>  		int n = read(trace_fd, buf, sizeof(buf));
> -		if (n <= 0)
> +
> +		if (n < 0) {
> +			if (errno == EINTR)
> +				continue;
> +			break;
> +		}
> +		if (n == 0)
>  			break;
>  		if (fwrite(buf, n, 1, stdout) != 1)
>  			break;
> -- 
> 2.55.0
>