Re: [PATCH v2 5/8] perf report: Add --progress option

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Stack misalignment in stdio progress tracking when STDIO_PROGRESS__MAX_DEPTH is exceeded.
--

commit 79112b3dd0e87e5c48cc7db61f548c4ae162bb0b
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf report: Add --progress option

This commit adds a --progress option to perf report that introduces a stdio
backend for ui_progress updates. It allows for progress tracking during event
processing, merging, and sorting phases by keeping track of nested phases.

> diff --git a/tools/perf/ui/stdio/progress.c b/tools/perf/ui/stdio/progress.c
> new file mode 100644
> index 0000000000000..1d1eddd192777
> --- /dev/null
> +++ b/tools/perf/ui/stdio/progress.c

[ ... ]

> +static void stdio_progress__finish(void);
> +
> +static void __stdio_progress__init(struct ui_progress *p)
> +{
> +	/*
> +	 * The default step (total / 16) is meant for the TUI progress
> +	 * bar, for stdio, where a percentage is printed, use 1% steps.
> +	 */
> +	p->next = p->step = p->total / 100 ?: 1;
> +
> +	if (stdio_progress__depth == STDIO_PROGRESS__MAX_DEPTH) {
> +		pr_warning("progress phases nested deeper than %d, completing %s\n",
> +			   STDIO_PROGRESS__MAX_DEPTH,
> +			   stdio_progress__stack[stdio_progress__depth - 1].p->title);
> +		stdio_progress__finish();
> +	}

[Severity: Medium]
Does this forceful drop cause stack misalignment for the unwinding callers?

When STDIO_PROGRESS__MAX_DEPTH is exceeded, this forcefully calls
stdio_progress__finish(). However, because the caller isn't informed that its
progress phase was dropped, its subsequent call to ui_progress__finish() will
inadvertently pop the wrong phase (the one just below it on the stack).

Could this state desynchronization lead to incorrect progress reporting and
premature completion of parent phases?

> +
> +	/* Start a nested phase in a line of its own. */
> +	if (stdio_progress__depth && stdio_progress__is_tty)
> +		fputc('\n', stderr);
> +
> +	stdio_progress__stack[stdio_progress__depth++] =
> +		(struct stdio_progress_phase) {
> +			.p = p,
> +			.last_printed = 0,
> +			.last_len = 0,
> +		};
> +
> +	stdio_progress__print_phase(&stdio_progress__stack[stdio_progress__depth - 1],
> +				    p->curr);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260913032632.116277-1-acme@kernel.org?part=5