Re: [PATCH v13 10/12] selftests: ftrace: Add wprobe trigger testcase

[Masami Hiramatsu (Google) <mhiramat@kernel.org>]

On Sat, 22 Aug 2026 09:39:24 +0000
sashiko-bot@kernel.org 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) <mhiramat@kernel.org>
> 
> 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.wangjinchao600@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) <mhiramat@kernel.org>