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 <[email protected]> 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: > 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/[email protected]?part=8
