Re: [PATCH v12 08/11] tracing/wprobe: Add set_wprobe and clear_wprobe event triggers

[Google]

On Tue, 18 Aug 2026 17:22:27 +0800
Jinchao Wang <wangjinchao600@xxxxxxxxx> wrote:

> On 8/7/2026 11:34 PM, Masami Hiramatsu (Google) wrote:
> > From: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>
> > 
> > Add set_wprobe and clear_wprobe event triggers to dynamically attach
> > and detach hardware breakpoint address monitoring based on event field
> > contents.
> > 
> 
> This patch does not build with wprobe enabled.

Oops, thanks for testing! Let me fix it.

> 
> > Link: https://lore.kernel.org/all/59637b96946653393a7ad3c7de094094796b39c2.1785067572.git.wangjinchao600@xxxxxxxxx/
> > 
> > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>
> > ---
> > Changes in v12:
> >  - Decrement trigger data->count only when watchpoint state is actually changed.
> >  - Remove dyn_event_ops_mutex to fix lockdep circular dependency deadlock.
> >  - Add event_trigger_init() call to prevent premature freeing of trigger_data.
> >  - Use WRITE_ONCE() when modifying tw->addr to pair with READ_ONCE().
> >  - Add missing braces to else-block in wprobe_trigger_print().
> >  - Initialize tw->addr only after trace_event_enable_disable() succeeds.
> >  - Counter counts only if the trigger is actually working.
> > Changes in v11:
> >  - Soft-enable (register) wprobe event on WPROBE_DEFAULT_CLEAR_ADDRESS.
> >  - Add work_pending check before updating tw->addr.
> > ---
> >  Documentation/trace/wprobetrace.rst |   98 +++++++
> >  include/linux/trace_events.h        |    1 
> >  kernel/trace/Kconfig                |   10 +
> >  kernel/trace/trace.h                |    1 
> >  kernel/trace/trace_dynevent.h       |    1 
> >  kernel/trace/trace_events_trigger.c |    2 
> >  kernel/trace/trace_probe.c          |    2 
> >  kernel/trace/trace_probe.h          |    9 +
> >  kernel/trace/trace_wprobe.c         |  527 +++++++++++++++++++++++++++++++++++
> >  9 files changed, 648 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Documentation/trace/wprobetrace.rst b/Documentation/trace/wprobetrace.rst
> > index ad5f089b5ef5..a579347735d2 100644
> > --- a/Documentation/trace/wprobetrace.rst
> > +++ b/Documentation/trace/wprobetrace.rst
> > @@ -68,3 +68,101 @@ Here is an example to add a wprobe event on a variable `jiffies`.
> >             <idle>-0       [000] d.Z1.  717.026373: my_jiffies: (tick_do_update_jiffies64+0xbe/0x130)
> >  
> >  You can see the code which writes to `jiffies` is `tick_do_update_jiffies64()`.
> > +
> > +Combination with trigger action
> > +-------------------------------
> > +The event trigger action can extend the utilization of this wprobe.
> > +
> > +- set_wprobe:WPEVENT:FIELD[+|-ADJUST]
> > +- clear_wprobe:WPEVENT[:FIELD[+|-]ADJUST]
> 
> Both forms accept an optional [:COUNT] which is not documented here.

Ah, indeed.

> 
> > +
> > +Set these triggers to the target event, then the WPROBE event will be
> > +setup to trace the memory access at FIELD[+|-ADJUST] address.
> > +When clear_wprobe is hit, if FIELD is NOT specified, the WPEVENT is
> > +forcibly cleared. If FIELD[[+|-]ADJUST] is set, it clears WPEVENT only
> > +if its watching address is the same as the FIELD[[+|-]ADJUST] value.
> 
> Two forms of adjust:
> [+|-ADJUST]
> [+|-]ADJUST

OK. [+ADJUST|-ADJUST] is correct form.

> 
> 
> > +
> > +Notes:
> > +The set_wprobe trigger does not change the type and length, these
> > +must be set when creating a new wprobe.
> > +
> > +The WPROBE event must be disabled when setting the new trigger
> > +and it will be busy afterwards. Recommended usage is to add a new
> > +wprobe at NULL address and keep disabled.
> > +
> 
> > diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> > index d9b6fa5c35d9..5fd8ed63c516 100644
> > --- a/kernel/trace/Kconfig
> > +++ b/kernel/trace/Kconfig
> > @@ -876,6 +876,16 @@ config WPROBE_EVENTS
> >  	  Those events can be inserted wherever hardware breakpoints can be
> >  	  set, and record accessed memory address and values.
> >  
> > +config WPROBE_TRIGGERS
> > +	depends on WPROBE_EVENTS
> > +	depends on HAVE_MODIFY_LOCAL_HW_BREAKPOINT_ADDR
> > +	bool
> > +	default y
> > +	help
> > +	  This adds an event trigger which will set the wprobe on a specific
> > +	  field of an event. This allows user to trace the memory access of
> > +	  an address pointed by the event field.
> > +
> >  config BPF_EVENTS
> >  	depends on BPF_SYSCALL
> >  	depends on (KPROBE_EVENTS || UPROBE_EVENTS) && PERF_EVENTS
> > diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> > index 64851a8d021f..a789a722bc8b 100644
> > --- a/kernel/trace/trace.h
> > +++ b/kernel/trace/trace.h
> > @@ -1983,6 +1983,7 @@ trigger_data_alloc(struct event_command *cmd_ops, char *cmd, char *param,
> >  		   void *private_data);
> >  extern void trigger_data_free(struct event_trigger_data *data);
> >  extern int event_trigger_init(struct event_trigger_data *data);
> > +extern void event_trigger_free(struct event_trigger_data *data);
> >  extern int trace_event_trigger_enable_disable(struct trace_event_file *file,
> >  					      int trigger_enable);
> >  extern void update_cond_flag(struct trace_event_file *file);
> > diff --git a/kernel/trace/trace_dynevent.h b/kernel/trace/trace_dynevent.h
> > index beee3f8d7544..77c2c84dcdb5 100644
> > --- a/kernel/trace/trace_dynevent.h
> > +++ b/kernel/trace/trace_dynevent.h
> > @@ -64,6 +64,7 @@ struct dyn_event {
> >  };
> >  
> >  extern struct list_head dyn_event_list;
> > +extern struct mutex dyn_event_ops_mutex;
> 
> dyn_event_ops_mutex is not needed anymore.

Ah, good catch!

> 
> >  
> >  static inline
> >  int dyn_event_init(struct dyn_event *ev, struct dyn_event_operations *ops)
> > diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
> > index ad83419cb420..fa409ebd73c2 100644
> > --- a/kernel/trace/trace_events_trigger.c
> > +++ b/kernel/trace/trace_events_trigger.c
> > @@ -589,7 +589,7 @@ int event_trigger_init(struct event_trigger_data *data)
> >   * Usually used directly as the @free method in event trigger
> >   * implementations.
> >   */
> > -static void
> > +void
> >  event_trigger_free(struct event_trigger_data *data)
> >  {
> >  	if (WARN_ON_ONCE(data->ref <= 0))
> > diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> > index 9f4cad18977a..36e07275a04d 100644
> > --- a/kernel/trace/trace_probe.c
> > +++ b/kernel/trace/trace_probe.c
> > @@ -20,7 +20,7 @@
> >  #undef C
> >  #define C(a, b)		b
> >  
> > -static const char *trace_probe_err_text[] = { ERRORS };
> > +const char *trace_probe_err_text[] = { ERRORS };
> >  
> >  static const char *reserved_field_names[] = {
> >  	"common_type",
> > diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> > index 6543d4c2cda5..e08f17c99138 100644
> > --- a/kernel/trace/trace_probe.h
> > +++ b/kernel/trace/trace_probe.h
> > @@ -634,7 +634,12 @@ extern int traceprobe_define_arg_fields(struct trace_event_call *event_call,
> >  	C(TYPECAST_SYM_OFFSET,	"@SYM+/-OFFSET with typecast needs parentheses"),	\
> >  	C(USED_ARG_NAME,	"This argument name is already used"),		\
> >  	C(WPROBE_NO_MAXACT,	"Watchpoint probe does not support maxactive"),	\
> > -	C(WPROBE_NO_SIBLING,	"Watchpoint probe does not support sibling probes"),
> > +	C(WPROBE_NO_SIBLING,	"Watchpoint probe does not support sibling probes"), \
> > +	C(WPROBE_ON_KPROBE,	"Wprobe trigger is not supported on kprobe event"), \
> > +	C(WPROBE_NOT_FOUND,	"Target wprobe event is not found"),		\
> > +	C(WPROBE_BUSY,		"Target wprobe event is already enabled"),	\
> > +	C(WPROBE_NEED_FIELD,	"Wprobe trigger requires a target field"),	\
> > +	C(WPROBE_BAD_FIELD,	"Target field must be pointer size"),
> >  
> >  #undef C
> >  #define C(a, b)		TP_ERR_##a
> > @@ -658,6 +663,8 @@ void __trace_probe_log_err(int offset, int err);
> >  
> >  DEFINE_FREE(trace_probe_log_clear, const char *, if (_T) trace_probe_log_clear())
> >  
> > +extern const char *trace_probe_err_text[];
> > +
> >  #define trace_probe_log_err(offs, err)	\
> >  	__trace_probe_log_err(offs, TP_ERR_##err)
> >  
> > diff --git a/kernel/trace/trace_wprobe.c b/kernel/trace/trace_wprobe.c
> > index df592d9280a4..c64bbdc90a40 100644
> > --- a/kernel/trace/trace_wprobe.c
> > +++ b/kernel/trace/trace_wprobe.c
> > @@ -6,7 +6,9 @@
> >   */
> >  #define pr_fmt(fmt)	"trace_wprobe: " fmt
> >  
> > +#include <linux/atomic.h>
> >  #include <linux/compiler.h>
> > +#include <linux/errno.h>
> >  #include <linux/hw_breakpoint.h>
> >  #include <linux/kallsyms.h>
> >  #include <linux/list.h>
> > @@ -15,11 +17,16 @@
> >  #include <linux/perf_event.h>
> >  #include <linux/rculist.h>
> >  #include <linux/security.h>
> > +#include <linux/spinlock.h>
> >  #include <linux/tracepoint.h>
> >  #include <linux/uaccess.h>
> > +#include <linux/workqueue.h>
> > +#include <linux/irq_work.h>
> > +#include <linux/preempt.h>
> >  
> >  #include <asm/ptrace.h>
> >  
> > +#include "trace.h"
> >  #include "trace_dynevent.h"
> >  #include "trace_probe.h"
> >  #include "trace_probe_kernel.h"
> > @@ -50,6 +57,17 @@ struct trace_wprobe {
> >  	int			len;
> >  	int			type;
> >  	const char		*symbol;
> > +	raw_spinlock_t		lock;
> > +	struct irq_work		irq_work;
> > +	struct work_struct	work;
> > +	atomic_t		missed;
> 
> Is `missed`  read somewhere?
> I searched this file, only found set and inc.

No, it just count the missed. I'm considering to expose it via
new `wprobe_profile`.

Thanks,

-- 
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>