Linux Perf Users
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Leo Yan" <leo.yan@arm.com>
Cc: linux-perf-users@vger.kernel.org
Subject: Re: [PATCH v2 12/14] perf tests: Check CoreSight syscall entry and exit
Date: Wed, 23 Sep 2026 16:46:08 +0000	[thread overview]
Message-ID: <20260923164608.D9D311F00893@smtp.kernel.org> (raw)
In-Reply-To: <20260923-perf_cs_etm_fix_non_taken-v2-12-6ab8c07a5455@arm.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf/tests: spurious test failure via masked skip code
--

commit 9f0a06a503d95652f0f88ac67977ecbfd36fc578
Author: Leo Yan <leo.yan@arm.com>

perf tests: Check CoreSight syscall entry and exit

Record the existing callchain workload, which issues gettid() through
SVC. The test requires syscall/sysret pairs to advance the PC by four
bytes.

> diff --git a/tools/perf/tests/shell/coresight/syscall_entry_exit.sh b/tools/perf/tests/shell/coresight/syscall_entry_exit.sh
> new file mode 100755
> index 0000000000000..bf61b1fdf6f15
> --- /dev/null
> +++ b/tools/perf/tests/shell/coresight/syscall_entry_exit.sh
> @@ -0,0 +1,21 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# CoreSight syscall entry and exit (exclusive)
> +
> +# shellcheck source=../lib/coresight_exception.sh
> +. "$(dirname "$0")/../lib/coresight_exception.sh"
> +
> +test_syscall_entry_exit()
> +{
> +	# callchain() invokes gettid() through SVC, which returns to PC + 4.
> +	if ! record_trace callchain; then
> +		cat "$tmpdir/record.log"
> +		echo "Failed to record the callchain workload"
> +		return 1
> +	fi
> +	decode_trace || return 1
> +	check_exception_pairs "syscall" "sysret" 4 || return 1

[Severity: Medium]
When check_exception_pairs() returns 2 (SKIP) due to trace gaps eliminating all
complete entry/exit pairs, does this statement incorrectly translate that return
code into a hard failure?

Since the test workload recognizes this as a non-fatal condition, using
|| return 1 here triggers the short-circuit fallback because 2 is non-zero.
This forces a return status of 1 (FAIL), causing legitimate trace gaps under
load to produce test failures instead of graceful skips.

Additionally, returning FAIL here causes the cleanup() trap to leak temporary
directory artifacts into /tmp.

> +}
> +
> +setup_exception_test || exit $?
> +test_syscall_entry_exit

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923-perf_cs_etm_fix_non_taken-v2-0-6ab8c07a5455@arm.com?part=12

  reply	other threads:[~2026-09-23 16:46 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-23 15:21 [PATCH v2 00/14] perf cs-etm: Fix bogus branch samples on exceptions Leo Yan
2026-09-23 15:21 ` [PATCH v2 01/14] perf sample: Allow decoders to supply branch return addresses Leo Yan
2026-09-23 15:29   ` sashiko-bot
2026-09-24 13:00   ` Adrian Hunter
2026-09-23 15:21 ` [PATCH v2 02/14] perf intel-pt: Preserve return addresses for asynchronous branches Leo Yan
2026-09-23 15:43   ` sashiko-bot
2026-09-24 13:00   ` Adrian Hunter
2026-09-23 15:21 ` [PATCH v2 03/14] perf cs-etm: Break branch history when instruction memory is unavailable Leo Yan
2026-09-23 15:46   ` sashiko-bot
2026-09-23 15:21 ` [PATCH v2 04/14] perf cs-etm: Centralize packet ISA initialization Leo Yan
2026-09-23 15:50   ` sashiko-bot
2026-09-23 15:21 ` [PATCH v2 05/14] perf cs-etm: Use the recorded instruction size for A32 and A64 Leo Yan
2026-09-23 16:01   ` sashiko-bot
2026-09-29 13:55   ` James Clark
2026-09-23 15:21 ` [PATCH v2 06/14] perf cs-etm: Mark branches that were not taken Leo Yan
2026-09-23 16:09   ` sashiko-bot
2026-09-23 15:21 ` [PATCH v2 07/14] perf cs-etm: Factor out final instruction sample synthesis Leo Yan
2026-09-23 16:13   ` sashiko-bot
2026-09-23 15:21 ` [PATCH v2 08/14] perf cs-etm: Centralize branch sample synthesis checks Leo Yan
2026-09-23 16:18   ` sashiko-bot
2026-09-23 15:21 ` [PATCH v2 09/14] perf cs-etm: Classify exception calls using the exception packet Leo Yan
2026-09-23 16:24   ` sashiko-bot
2026-09-29 14:08   ` James Clark
2026-09-23 15:21 ` [PATCH v2 10/14] perf cs-etm: Synthesize exception entries separately from branches Leo Yan
2026-09-23 16:36   ` sashiko-bot
2026-09-23 15:21 ` [PATCH v2 11/14] perf tests: Check CoreSight IRQ entry and exit Leo Yan
2026-09-23 16:40   ` sashiko-bot
2026-09-29 12:22   ` James Clark
2026-09-23 15:21 ` [PATCH v2 12/14] perf tests: Check CoreSight syscall " Leo Yan
2026-09-23 16:46   ` sashiko-bot [this message]
2026-09-23 15:21 ` [PATCH v2 13/14] perf tests: Check CoreSight abort " Leo Yan
2026-09-23 16:49   ` sashiko-bot
2026-09-23 15:21 ` [PATCH v2 14/14] perf tests: Check CoreSight emulated instruction " Leo Yan
2026-09-23 16:56   ` sashiko-bot
2026-09-29 14:41 ` [PATCH v2 00/14] perf cs-etm: Fix bogus branch samples on exceptions 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=20260923164608.D9D311F00893@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=leo.yan@arm.com \
    --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