Message-Id: <20250917231446.670b2c9cac245c81b268b5b2@kernel.org>
Date: Wed, 17 Sep 2025 23:14:46 +0900
From: Masami Hiramatsu (Google) <mhiramat@...nel.org>
To: Randy Dunlap <rdunlap@...radead.org>
Cc: Steven Rostedt <rostedt@...dmis.org>, Peter Zijlstra
 <peterz@...radead.org>, Ingo Molnar <mingo@...nel.org>, x86@...nel.org,
 Jinchao Wang <wangjinchao600@...il.com>, Mathieu Desnoyers
 <mathieu.desnoyers@...icios.com>, Thomas Gleixner <tglx@...utronix.de>,
 Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>,
 "H . Peter Anvin" <hpa@...or.com>, Alexander Shishkin
 <alexander.shishkin@...ux.intel.com>, Ian Rogers <irogers@...gle.com>,
 linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org,
 linux-doc@...r.kernel.org, linux-perf-users@...r.kernel.org
Subject: Re: [PATCH v4 5/8] tracing: wprobe: Add wprobe event trigger

Hi Randy,

On Sun, 14 Sep 2025 17:25:19 -0700
Randy Dunlap <rdunlap@...radead.org> wrote:

> 
> 
> On 9/14/25 7:10 AM, Masami Hiramatsu (Google) wrote:
> > From: Masami Hiramatsu (Google) <mhiramat@...nel.org>
> > 
> > Add wprobe event trigger to set and clear the watch event dynamically.
> > This allows us to set an watchpoint on a given local variables and
> > a slab object instead of static objects.
> > 
> > The trigger syntax is below;
> 
>                         below:
> (just a nit :)

OK.

> 
> > 
> >   - set_wprobe:WPROBE:FIELD[+OFFSET] [if FILTER]
> >   - clear_wprobe:WPROBE[:FIELD[+OFFSET]] [if FILTER]
> > 
> > set_wprobe sets the address pointed by FIELD[+offset] to the WPROBE
> > event. The FIELD is the field name of trigger event.
> > clear_wprobe clears the watch address of WPROBE event. If the FIELD
> > option is specified, it clears only if the current watch address is
> > same as the given FIELD[+OFFSET] value.
> > 
> > The set_wprobe trigger do not change type and length. That should be
> 
>                          does                             should be done
> ?

OK.

> 
> > set when a new wprobe is created.
> > 
> > Also, 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.
> > 
> > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@...nel.org>
> > ---
> >  Changes in v3:
> >   - Add FIELD option support for clear_wprobe and update document.
> >   - Fix to unregister/free event_trigger_data on file correctly.
> >   - Fix syntax comments.
> >  Changes in v2:
> >   - Getting local cpu perf_event from trace_wprobe directly.
> >   - Remove trace_wprobe_local_perf() because it is conditionally unused.
> >   - Make CONFIG_WPROBE_TRIGGERS a hidden config.
> > ---
> >  Documentation/trace/wprobetrace.rst |   88 +++++++
> >  include/linux/trace_events.h        |    1 
> >  kernel/trace/Kconfig                |   10 +
> >  kernel/trace/trace_wprobe.c         |  430 +++++++++++++++++++++++++++++++++++
> >  4 files changed, 529 insertions(+)
> > 
> > diff --git a/Documentation/trace/wprobetrace.rst b/Documentation/trace/wprobetrace.rst
> > index 9774f57e2947..a1812a8ac491 100644
> > --- a/Documentation/trace/wprobetrace.rst
> > +++ b/Documentation/trace/wprobetrace.rst
> > @@ -67,3 +67,91 @@ 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 `do_timer()`.
> > +
> > +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]
> > +
> > +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.
> > +
> > +Notes:
> > +The set_wprobe trigger do not change type and length. That should be
>                           does not                          should be done

OK.

> 
> 
> > +set when a new wprobe is created.
> > +
> > +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.
> > +
> > +
> > +For example, trace the first 8 byte of the dentry data structure passed
> 
>                                   bytes

OK

> 
> > +to do_truncate() until it is deleted by __dentry_kill().
> > +(Note: all tracefs setup uses '>>' so that it does not kick do_truncate())
> > +
> > +  # echo 'w:watch rw@0:8 address=$addr value=+0($addr)' > dynamic_events
> 
> Just using '>' here is OK?

Yes, until this, no probe hooks do_truncate, so it should not kick any
events. But maybe it is better to use '>>' to avoid confusing.

> 
> > +
> > +  # echo 'f:truncate do_truncate dentry=$arg2' >> dynamic_events
> > +  # echo 'set_wprobe:watch:dentry' >> events/fprobes/truncate/trigger
> > +
> > +  # echo 'f:dentry_kill __dentry_kill dentry=$arg1' >> dynamic_events
> > +  # echo 'clear_wprobe:watch:dentry' >> events/fprobes/dentry_kill/trigger
> > +
> > +  # echo 1 >> events/fprobes/truncate/enable
> > +  # echo 1 >> events/fprobes/dentry_kill/enable
> > +
> > +  # echo aaa > /tmp/hoge
> > +  # echo bbb > /tmp/hoge
> > +  # echo ccc > /tmp/hoge
> > +  # rm /tmp/hoge
> > +
> > +Then, the trace data will show;
> 
> Usually that should be:      show:
> 
> but in .rst it might need to be    show::

Ah,, yes, that's right. I missed that.

Thank you!

> 
> I don't know. Haven't tested it.
> 
> 
> -- 
> ~Randy
> 


-- 
Masami Hiramatsu (Google) <mhiramat@...nel.org>

Powered by blists - more mailing lists