Re: [RFC PATCH v7 08/10] selftests/ftrace: add a stackmap basic functionality test
From: sashiko-bot@kernel.org
Date: 2026-09-12 09:02:14
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The commit message claims a check is performed to verify that post-reset successes are below pre-reset successes, but this check is explicitly omitted in the script because it is flaky. -- commit 874f10b935c1daaa7ebb380b794d32d569fa81af Author: Pengfei Li [off-list ref] selftests/ftrace: add a stackmap basic functionality test This commit adds a basic selftest for ftrace stackmap deduplication. It exercises stackmap through sched_switch event stack capture, resets the map while tracing, and validates counter logic and record reuse.
Reset the map five times while tracing and the filtered writer remain
active. Before each reset, require at least eight successes. Immediately
after reset, send SIGSTOP and wait until /proc reports the worker in a
stopped state before disabling tracing and sampling counters. Require the
post-reset success count to be below the pre-reset count, proving that the
sample belongs to a new generation. Reset again while the writer is
stopped and require exactly zero entries before resuming and refilling.[Severity: Medium] Does the commit message need to be updated to match the code? The commit message claims a check is performed to verify the post-reset success count is below the pre-reset count. However, the test code itself explicitly documents that no such counter is compared, because it would make the test flaky:
quoted hunk ↗ jump to hunk
diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/stackmap-basic.tc b/tools/testing/selftests/ftrace/test.d/ftrace/stackmap-basic.tc --- /dev/null +++ b/tools/testing/selftests/ftrace/test.d/ftrace/stackmap-basic.tc
[ ... ]
+# Reset repeatedly while the owned writer is active, then again once it is +# quiesced. Only deterministic properties are asserted: +# +# - While the writer runs, the reset must be accepted. No counter is +# compared here: the writer resumes claiming records as soon as reset() +# returns, so any snapshot taken afterwards is a moving target and would +# make this test flaky rather than prove anything.
Should the commit message be revised to reflect the actual test logic that intentionally skips this comparison? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260912083753.3426176-1-lipengfei28@xiaomi.com?part=8