Linux Perf Users
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "James Clark" <james.clark@linaro.org>
Cc: linux-perf-users@vger.kernel.org
Subject: Re: [PATCH v4 02/19] perf test: Add workload-ctl option
Date: Tue, 09 Jun 2026 14:40:17 +0000	[thread overview]
Message-ID: <20260609144018.2D77E1F00893@smtp.kernel.org> (raw)
In-Reply-To: <20260609-james-cs-context-tracking-fix-v4-2-44f9fb9e5c42@linaro.org>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit message claims to add a `--workload-ctl` option, but the implementation actually adds a `--record-ctl` option.
--

commit be5e10b3228f286fe50c54ca1ba231a4a933afc1
Author: James Clark <james.clark@linaro.org>

perf test: Add workload-ctl option

This commit adds an option to 'perf test' to open a named FIFO and send
enable and disable commands before and after invoking a builtin workload.
This allows syncing with a 'perf record' session to limit the recording
scope.

> perf test: Add workload-ctl option
> 
> Add a --workload-ctl=fifo:ctl-fifo[,ack-fifo] option for 'perf test
> -w'.

[Severity: Medium]
This isn't a bug in the code, but there is a mismatch between the commit
message and the implemented option. The commit title and message specify
'--workload-ctl', but the documentation and code implement '--record-ctl'.

Could the commit message be updated to match the implementation, or should
the option be renamed to '--workload-ctl' in the code?

> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
[ ... ]
> @@ -1486,6 +1664,8 @@ int cmd_test(int argc, const char **argv)
>  	OPT_UINTEGER('r', "runs-per-test", &runs_per_test,
>  		     "Run each test the given number of times, default 1"),
>  	OPT_STRING('w', "workload", &workload, "work", "workload to run for testing, use '--list-workloads' to list the available ones."),
> +	OPT_STRING(0, "record-ctl", &workload_control, "fifo:ctl-fifo[,ack-fifo]",
> +		   "Write enable to the fifo just before running the workload and disable after, with optional ack from ack-fifo"),

[Severity: Medium]
This is where the option is defined as "record-ctl" in cmd_test() instead
of the promised "workload-ctl". Does this option name need to be updated?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609-james-cs-context-tracking-fix-v4-0-44f9fb9e5c42@linaro.org?part=2

  reply	other threads:[~2026-06-09 14:40 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-09 14:30 [PATCH v4 00/19] perf cs-etm: Queue context packets for frontend James Clark
2026-06-09 14:30 ` [PATCH v4 01/19] " James Clark
2026-06-09 14:46   ` sashiko-bot
2026-06-09 14:30 ` [PATCH v4 02/19] perf test: Add workload-ctl option James Clark
2026-06-09 14:40   ` sashiko-bot [this message]
2026-06-09 14:30 ` [PATCH v4 03/19] perf test: Add a workload that forces context switches James Clark
2026-06-09 14:30 ` [PATCH v4 04/19] perf test cs-etm: Test process attribution James Clark
2026-06-09 14:41   ` sashiko-bot
2026-06-09 14:30 ` [PATCH v4 05/19] perf test: Add deterministic workload James Clark
2026-06-09 14:30 ` [PATCH v4 06/19] perf test cs-etm: Replace unroll loop thread with deterministic decode test James Clark
2026-06-09 14:52   ` sashiko-bot
2026-06-09 14:31 ` [PATCH v4 07/19] perf test cs-etm: Remove asm_pure_loop test James Clark
2026-06-09 14:53   ` sashiko-bot
2026-06-09 14:31 ` [PATCH v4 08/19] perf test cs-etm: Replace memcpy test with raw dump stress test James Clark
2026-06-09 14:31 ` [PATCH v4 09/19] perf test: Add named_threads workload James Clark
2026-06-09 14:50   ` sashiko-bot
2026-06-09 14:31 ` [PATCH v4 10/19] perf test cs-etm: Test decoding for concurrent threads test James Clark
2026-06-09 14:31 ` [PATCH v4 11/19] perf test cs-etm: Remove duplicate branch tests James Clark
2026-06-09 14:31 ` [PATCH v4 12/19] perf test cs-etm: Skip if not root James Clark
2026-06-09 14:53   ` sashiko-bot
2026-06-09 14:31 ` [PATCH v4 13/19] perf test cs-etm: Reduce snapshot size James Clark
2026-06-09 14:31 ` [PATCH v4 14/19] perf test cs-etm: Speed up basic test James Clark
2026-06-09 14:31 ` [PATCH v4 15/19] perf test cs-etm: Remove unused Coresight workloads James Clark
2026-06-09 14:31 ` [PATCH v4 16/19] perf test cs-etm: Make disassembly test use kcore James Clark
2026-06-09 14:31 ` [PATCH v4 17/19] perf test cs-etm: Add all branch instructions to test James Clark
2026-06-09 14:31 ` [PATCH v4 18/19] perf test cs-etm: Speed up disassembly test James Clark
2026-06-09 15:06   ` sashiko-bot
2026-06-09 14:31 ` [PATCH v4 19/19] perf test cs-etm: Move existing tests to coresight folder James Clark

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=20260609144018.2D77E1F00893@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=james.clark@linaro.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox