All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Arnaldo Carvalho de Melo" <acme@kernel.org>
Cc: linux-perf-users@vger.kernel.org
Subject: Re: [PATCH 3/5] perf report: Add --no-progress option
Date: Tue, 29 Sep 2026 20:24:22 +0000	[thread overview]
Message-ID: <20260929202423.4B84F1F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260929201524.2507733-4-acme@kernel.org>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf report: --no-progress option is shadowed by auto-negation of --progress
--

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

perf report: Add --no-progress option

This patch adds a --no-progress option to perf report to disable progress
information while processing the perf.data file. This takes precedence over
--progress and also turns off the default progress displays in the TUI and
GTK browsers.

> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 963808b561e54..e334556af62f9 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c

[ ... ]

> @@ -1387,6 +1388,8 @@ int cmd_report(int argc, const char **argv)
>  			"Show or hide weight columns in annotation. Default show if non-zero."),
>  	OPT_BOOLEAN(0, "progress", &report.progress,
>  		    "Show progress while processing the perf.data file"),
> +	OPT_BOOLEAN(0, "no-progress", &report.no_progress,
> +		    "Do not show progress while processing the perf.data file"),
>  	OPT_BOOLEAN(0, "header", &report.header, "Show data header."),

[Severity: Medium]
Since the "progress" option precedes "no-progress" in cmd_report(), does the
parse-options engine intercept the command line flag --no-progress as an
auto-negation of the earlier progress option?

If so, this would shadow the explicitly defined no-progress option, leaving
report.no_progress always unset.

[ ... ]

> @@ -1798,7 +1801,9 @@ int cmd_report(int argc, const char **argv)
>  	 * processed so far for each processing phase.  --quiet asks for no
>  	 * messages at all, so it leaves the phases uncounted.
>  	 */
> -	if (report.progress && !quiet && use_browser == 0)
> +	if (report.no_progress)
> +		ui_progress__noop_init();
> +	else if (report.progress && !quiet && use_browser == 0)
>  		stdio_progress__init();

[Severity: Medium]
If report.no_progress can never be true due to the parse-options shadowing
mentioned above, does this become dead code? This would prevent
ui_progress__noop_init() from executing and leave UI progress enabled despite
the flag being passed.

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

  reply	other threads:[~2026-09-29 20:24 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-29 20:15 [PATCH v4 0/5] perf tools: Add progress diagnostics and a false-sharing workload Arnaldo Carvalho de Melo
2026-09-29 20:15 ` [PATCH 1/5] perf config: Move perf_config__set_variable() to util/config.c Arnaldo Carvalho de Melo
2026-09-29 20:27   ` sashiko-bot
2026-09-29 20:15 ` [PATCH 2/5] perf report: Add --progress option Arnaldo Carvalho de Melo
2026-09-29 20:23   ` sashiko-bot
2026-09-29 20:15 ` [PATCH 3/5] perf report: Add --no-progress option Arnaldo Carvalho de Melo
2026-09-29 20:24   ` sashiko-bot [this message]
2026-09-29 20:15 ` [PATCH 4/5] perf scripts: Add perf-stuck, to tell where a running perf is stuck Arnaldo Carvalho de Melo
2026-09-29 20:22   ` sashiko-bot
2026-09-29 20:15 ` [PATCH 5/5] perf test: Add false_sharing workload exhibiting cross-CPU false sharing Arnaldo Carvalho de Melo
2026-09-29 20:25   ` sashiko-bot
  -- strict thread matches above, loose matches on Subject: below --
2026-09-29 21:22 [PATCH v5 0/5] perf tools: Add progress diagnostics and a false-sharing workload Arnaldo Carvalho de Melo
2026-09-29 21:22 ` [PATCH 3/5] perf report: Add --no-progress option Arnaldo Carvalho de Melo
2026-09-29 21:28   ` sashiko-bot
2026-09-29 23:48   ` Namhyung Kim

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=20260929202423.4B84F1F000FF@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=acme@kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.