On Sat, 22 Aug 2026 09:39:24 +0000 [email protected] wrote: > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) > to consider: > - [Medium] The `trigger-wprobe.tc` test has a race condition causing > intermittent failures because the `clear_wprobe` trigger executes > asynchronously and the test does not wait for it to fire before verifying the > trace buffer. > - [Medium] The `trigger-wprobe.tc` test uses `timer=t` as a fetcharg without > enforcing a BTF requirement, which will cause false-positive test failures on > kernels without BTF support. > -- > > --- Patch [10]: [PATCH v13 10/12] selftests: ftrace: Add wprobe trigger > testcase --- > commit f8200da0c99be51814ee14df413b8fd81995a06f > Author: Masami Hiramatsu (Google) <[email protected]> > > selftests: ftrace: Add wprobe trigger testcase > > This commit adds a testcase to verify wprobe triggers. It uses set_wprobe > and clear_wprobe triggers on fprobe and static trace events to monitor > memory accesses within the trace-events-sample kernel module, and tests > wprobe trigger syntax error logging. > > Link: > https://lore.kernel.org/all/59637b96946653393a7ad3c7de094094796b39c2.1785067572.git.wangjinchao...@gmail.com/ > > > diff --git > > a/tools/testing/selftests/ftrace/test.d/trigger/trigger-wprobe.tc > > b/tools/testing/selftests/ftrace/test.d/trigger/trigger-wprobe.tc > > new file mode 100644 > > index 0000000000000..0565e2c421778 > > --- /dev/null > > +++ b/tools/testing/selftests/ftrace/test.d/trigger/trigger-wprobe.tc > [ ... ] > > +:;: "Add a wprobe event used by trigger" ;: > > +echo 'w:watch rw@0:8 address=$addr value=$value' > dynamic_events > > + > > +:;: "Add events for triggering wprobe" ;: > > +echo 'f:fprobes/testevent sample_timer_cb timer=t' >> dynamic_events > > [Severity: Medium] > Is it intentional to use a local variable t as a fetcharg here? > > This relies on BTF debug info. On kernels without > CONFIG_PROBE_EVENTS_BTF_ARGS=y or architectures lacking BTF support, > the kernel's tracing subsystem cannot resolve the argument and the > fprobe registration will fail. > > Should this test either add a requirement for BTF arguments in the requires > directive, or use a register argument like $arg1 instead? > > > + > > +:;: "Enable all events before setting triggers" ;: > > +echo 1 > tracing_on > > +echo 1 >> events/fprobes/testevent/enable > > +echo 1 >> events/sample-trace/foo_bar_with_fn/enable > [ ... ] > > +:;: "Set clear_wprobe trigger on foo_bar_with_fn" ;: > > +echo 'clear_wprobe:watch' >> events/sample-trace/foo_bar_with_fn/trigger > > +cat events/sample-trace/foo_bar_with_fn/trigger | grep ^clear_wprobe > > + > > +# Disable set_wprobe to prevent sample_timer_cb from re-arming the > > watchpoint > > +echo '!set_wprobe:watch:timer' >> events/fprobes/testevent/trigger > > + > > +# Clear trace and wait to ensure no new watchpoint events are generated > > +clear_trace > > +sleep 1 > > [Severity: Medium] > Can this sequence cause a race condition resulting in intermittent test > failures? > > If the test clears the trace buffer here without first waiting for > foo_bar_with_fn to execute and actually clear the watchpoint, there is a race > window. > > Since both foo_bar_with_fn and sample_timer_cb run periodically every > second, could sample_timer_cb execute after clear_trace but before > foo_bar_with_fn has fired? > > If so, it would access the still-armed watchpoint and log a watch event in the > newly cleared trace buffer, causing the final verification step > (! grep -q watch trace) to fail. > > > + > > +:;: "Ensure clear_wprobe trigger deactivated the watchpoint" ;: > > +! grep -q watch trace > > + > > -- > Sashiko AI review ยท > https://sashiko.dev/#/patchset/178739053919.1520941.17662338993878200834.stgit@devnote2?part=10
-- Masami Hiramatsu (Google) <[email protected]>
