Re: [RFC PATCH 0/6] tracing: wprobe: Add wprobe for watchpoint

Jinchao Wang
2025-09-04T06:35:41Z

On 9/2/25 22:02, Masami Hiramatsu (Google) wrote:
> (Adding Jinchao)
>
> On Mon, 1 Sep 2025 23:44:35 +0900
> "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
>
>> Hi,
>>
>> Here is an RFC series for adding new wprobe (watch probe) which
>> provides memory access tracing event. Moreover, this can be used via
>> event trigger. Thus it can trace memory access on a dynamically
>> allocated objects too.
>
> BTW, this series is on the top of probes/for-next branch in the
> linux-trace tree.
>

Hi, Masami

Thanks for including me. I only received the cover letter, so I'm
providing my feedback here:

- trigger_data issue:
it appears that the instance being removed is not the same as
the one that was originally set.

- the function call issue:
`trace_wprobe_update_local()` is called twice, once in the trigger
callback and again in `wprobe_work_func`.

I also noticed that the Watchpoint probe and KStackWatch implementations
share very similar logic for managing hardware breakpoints/watchpoints
(`hwbp/watch`):
- `watch_init(unsigned long &place_holder)`
- `watch_on(struct perf_event_attr *attr)`
- `watch_off()` (or reset to the `place_holder` value)
- `watch_uninit()`

Their primary difference lies in their handler functions, specifically
the `perf_overflow_handler_t triggered` callback.

I believe we could work together to unify this logic. I am open to
either approach: I can refactor my watch.c, or you can introduce new
helpers. This would help us save duplicated work and review time.

> Thanks,
>
>>
>> In this version, I reuse Jinchao's arch_reinstall_hw_breakpoint()
>> patch[1].
>>
>> [1] https://lore.kernel.org/all/20250828073311.1116593-6-wangjinchao600@gmail.com/
>>
>> The basic usage of this wprobe is similar to other probes;
>>
>> w:[GRP/][EVENT] [r|w|rw]@<ADDRESS|SYMBOL[+OFFS]> [FETCHARGS]
>>
>> This defines a new wprobe event. For example, to trace jiffies update,
>> you can do;
>>
>> echo 'w:my_jiffies w@jiffies:8 value=+0($addr)' >> dynamic_events
>> echo 1 > events/wprobes/my_jiffies/enable
>>
>> Moreover, this can be combined with event trigger to trace the memory
>> accecss on slab objects. The trigger syntax is;
>>
>> set_wprobe:WPROBE_EVENT:FIELD[+ADJUST]
>> clear_wprobe:WPROBE_EVENT
>>
>> For example, trace the first 8 byte of the dentry data structure passed
>> 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
>>
>> # 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' >> 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;
>>
>> # tracer: nop
>> #
>> # entries-in-buffer/entries-written: 16/16 #P:8
>> #
>> # _-----=> irqs-off/BH-disabled
>> # / _----=> need-resched
>> # | / _---=> hardirq/softirq
>> # || / _--=> preempt-depth
>> # ||| / _-=> migrate-disable
>> # |||| / delay
>> # TASK-PID CPU# ||||| TIMESTAMP FUNCTION
>> # [ 7.026136] sh (113) used greatest stack depth: 12912 bytes left
>> | | | ||||| | |
>> sh-113 [002] ..... 7.024402: truncate: (do_truncate+0x4/0x120) dentry=0xffff8880069194b8
>> sh-113 [002] ..Zff 7.024822: watch: (lookup_fast+0xaa/0x150) address=0xffff8880069194b8 value=0x200008
>> sh-113 [002] ..Zff 7.024830: watch: (step_into+0x82/0x360) address=0xffff8880069194b8 value=0x200008
>> sh-113 [002] ..Zff 7.024834: watch: (step_into+0x9f/0x360) address=0xffff8880069194b8 value=0x200008
>> sh-113 [002] ..Zff 7.024839: watch: (path_openat+0xb3a/0xe70) address=0xffff8880069194b8 value=0x200008
>> sh-113 [002] ..Zff 7.024843: watch: (path_openat+0xb9a/0xe70) address=0xffff8880069194b8 value=0x200008
>> sh-113 [002] ..... 7.024847: truncate: (do_truncate+0x4/0x120) dentry=0xffff8880069194b8
>> sh-113 [002] ...1. 7.025364: dentry_kill: (__dentry_kill+0x0/0x220) dentry=0xffff888006919380
>> sh-113 [002] ...1. 7.025511: dentry_kill: (__dentry_kill+0x0/0x220) dentry=0xffff8880069195f0
>> rm-118 [003] ...1. 7.027543: dentry_kill: (__dentry_kill+0x0/0x220) dentry=0xffff8880069194b8
>> sh-113 [002] ...2. 7.027825: dentry_kill: (__dentry_kill+0x0/0x220) dentry=0xffff8880044429c0
>> sh-113 [002] ...2. 7.027833: dentry_kill: (__dentry_kill+0x0/0x220) dentry=0xffff888004442270
>>
>>
>> Thank you,
>>
>> ---
>>
>> Jinchao Wang (1):
>> x86/HWBP: introduce arch_reinstall_hw_breakpoint() for atomic context
>>
>> Masami Hiramatsu (Google) (5):
>> tracing: wprobe: Add watchpoint probe event based on hardware breakpoint
>> HWBP: Add modify_wide_hw_breakpoint_local() API
>> tracing: wprobe: Add wprobe event trigger
>> selftests: tracing: Add a basic testcase for wprobe
>> selftests: tracing: Add syntax testcase for wprobe
>>
>>
>> Documentation/trace/index.rst | 1
>> Documentation/trace/wprobetrace.rst | 129 ++
>> arch/Kconfig | 10
>> arch/x86/Kconfig | 1
>> arch/x86/include/asm/hw_breakpoint.h | 3
>> arch/x86/kernel/hw_breakpoint.c | 61 +
>> include/linux/hw_breakpoint.h | 6
>> include/linux/trace_events.h | 3
>> kernel/events/hw_breakpoint.c | 36 +
>> kernel/trace/Kconfig | 24
>> kernel/trace/Makefile | 1
>> kernel/trace/trace.c | 9
>> kernel/trace/trace.h | 5
>> kernel/trace/trace_probe.c | 20
>> kernel/trace/trace_probe.h | 8
>> kernel/trace/trace_wprobe.c | 1111 ++++++++++++++++++++
>> .../ftrace/test.d/dynevent/add_remove_wprobe.tc | 68 +
>> .../test.d/dynevent/wprobes_syntax_errors.tc | 20
>> 18 files changed, 1513 insertions(+), 3 deletions(-)
>> create mode 100644 Documentation/trace/wprobetrace.rst
>> create mode 100644 kernel/trace/trace_wprobe.c
>> create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/add_remove_wprobe.tc
>> create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/wprobes_syntax_errors.tc
>>
>> --
>> Masami Hiramatsu (Google) <mhiramat@kernel.org>
>
>

--
Best regards,
Jinchao