linux-trace-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
To: Randy Dunlap <rdunlap@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>,
	x86@kernel.org, Jinchao Wang <wangjinchao600@gmail.com>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	"H . Peter Anvin" <hpa@zytor.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Ian Rogers <irogers@google.com>,
	linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
	linux-doc@vger.kernel.org, linux-perf-users@vger.kernel.org
Subject: Re: [PATCH v4 1/8] tracing: wprobe: Add watchpoint probe event based on hardware breakpoint
Date: Wed, 17 Sep 2025 23:38:18 +0900	[thread overview]
Message-ID: <20250917233818.71678d0164a6fc2d11fa6e38@kernel.org> (raw)
In-Reply-To: <014136d2-8599-4a1f-ab8e-c5be4f522e5a@infradead.org>

Hi Randy,

On Sun, 14 Sep 2025 17:14:37 -0700
Randy Dunlap <rdunlap@infradead.org> wrote:
> > +  w:[GRP/][EVENT] SPEC [FETCHARGS]                       : Probe on data access
> > +
> > + GRP            : Group name for wprobe. If omitted, use "wprobes" for it.
> > + EVENT          : Event name for wprobe. If omitted, an event name is
> > +                  generated based on the address or symbol.
> > + SPEC           : Breakpoint specification.
> > +                  [r|w|rw]@<ADDRESS|SYMBOL[+|-OFFS]>[:LENGTH]
> > +
> > +   r|w|rw       : Access type, r for read, w for write, and rw for both.
> > +                  Use rw if omitted.
> 
> 		     Default is rw if omitted.

OK.

> 
> > +   ADDRESS      : Address to trace (hexadecimal).
> > +   SYMBOL       : Symbol name to trace.
> > +   LENGTH       : Length of the data to trace in bytes. (1, 2, 4, or 8)
> > +
> > + FETCHARGS      : Arguments. Each probe can have up to 128 args.
> > +  $addr         : Fetch the accessing address.
> > +  @ADDR         : Fetch memory at ADDR (ADDR should be in kernel)
> > +  @SYM[+|-offs] : Fetch memory at SYM +|- offs (SYM should be a data symbol)
> > +  +|-[u]OFFS(FETCHARG) : Fetch memory at FETCHARG +|- OFFS address.(\*1)(\*2)
> > +  \IMM          : Store an immediate value to the argument.
> > +  NAME=FETCHARG : Set NAME as the argument name of FETCHARG.
> > +  FETCHARG:TYPE : Set TYPE as the type of FETCHARG. Currently, basic types
> > +                  (u8/u16/u32/u64/s8/s16/s32/s64), hexadecimal types
> > +                  (x8/x16/x32/x64), "char", "string", "ustring", "symbol", "symstr"
> > +                  and bitfield are supported.
> > +
> > +  (\*1) this is useful for fetching a field of data structures.
> > +  (\*2) "u" means user-space dereference.
> > +
> > +For the details of TYPE, see :ref:`kprobetrace documentation <kprobetrace_types>`.
> > +
> > +Usage examples
> > +--------------
> > +Here is an example to add a wprobe event on a variable `jiffies`.
> > +::
> > +
> > +  # echo 'w:my_jiffies w@jiffies' >> dynamic_events
> > +  # cat dynamic_events
> > +  w:wprobes/my_jiffies w@jiffies
> > +  # echo 1 > events/wprobes/enable
> > +  # cat trace | head

Note, I also found this is not head, but combined with tail,
e.g. `cat trace | head -n 15 | tail -n 5`

> > +  #           TASK-PID     CPU#  |||||  TIMESTAMP  FUNCTION
> > +  #              | |         |   |||||     |         |
> > +           <idle>-0       [000] d.Z1.  717.026259: my_jiffies: (tick_do_update_jiffies64+0xbe/0x130)
> > +           <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()`.
> 
> I'm having trouble getting from tick_do_update_jiffies64+0xbe/0x130,
> which I expect is
> 	jiffies_64 += ticks;
> in that function, over to do_timer(), which also updates jiffies_64,
> but is not called by tick_do_update_jiffies64(). AFAICT, there are
> no calls to do_timer() in the file (kernel/time/tick-sched.c).
> 
> Can you explain, please?

Hmm, in my code base

static void tick_do_update_jiffies64(ktime_t now)
{
	...
	} else {
		last_jiffies_update = ktime_add_ns(last_jiffies_update,
						   TICK_NSEC);
	}

	/* Advance jiffies to complete the 'jiffies_seq' protected job */
	jiffies_64 += ticks;

	...

So this function seems correctly update the jiffies_64.
If you ask about where it comes from, I can also enable stacktrace on
that event. (echo 1 >> options/stacktrace)

             cat-124     [005] d.Z1.   537.689753: my_jiffies: (tick_do_update_jiffies64+0xbe/0x130)
             cat-124     [005] d.Z1.   537.689762: <stack trace>
 => tick_do_update_jiffies64
 => tick_nohz_handler
 => __hrtimer_run_queues
 => hrtimer_interrupt
 => __sysvec_apic_timer_interrupt
 => sysvec_apic_timer_interrupt
 => asm_sysvec_apic_timer_interrupt

So it came from hrtimer_interrupt().

> 
> 
> 
> > diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> > index d2c79da81e4f..dd8919386425 100644
> > --- a/kernel/trace/Kconfig
> > +++ b/kernel/trace/Kconfig
> > @@ -807,6 +807,20 @@ config EPROBE_EVENTS
> >  	  convert the type of an event field. For example, turn an
> >  	  address into a string.
> >  
> > +config WPROBE_EVENTS
> > +	bool "Enable wprobe-based dynamic events"
> > +	depends on TRACING
> > +	depends on HAVE_HW_BREAKPOINT
> > +	select PROBE_EVENTS
> > +	select DYNAMIC_EVENTS
> > +	default y
> 
> Wny default y?

No big reason. This is just a dynamic event and unless the super user
adds this event this does not work on the system. I can make it N so
developer can enable it when builds their kernel.

Thank you,

> 
> > +	help
> > +	  This allows the user to add watchpoint tracing events based on
> > +	  hardware breakpoints on the fly via the ftrace interface.
> > +
> > +	  Those events can be inserted wherever hardware breakpoints can be
> > +	  set, and record various register and memory values.
> > +
> >  config BPF_EVENTS
> >  	depends on BPF_SYSCALL
> >  	depends on (KPROBE_EVENTS || UPROBE_EVENTS) && PERF_EVENTS
> 
> 
> thanks.
> -- 
> ~Randy
> 


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

  reply	other threads:[~2025-09-17 14:38 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-14 14:09 [PATCH v4 0/8] tracing: wprobe: Add wprobe for watchpoint Masami Hiramatsu (Google)
2025-09-14 14:09 ` [PATCH v4 1/8] tracing: wprobe: Add watchpoint probe event based on hardware breakpoint Masami Hiramatsu (Google)
2025-09-15  0:14   ` Randy Dunlap
2025-09-17 14:38     ` Masami Hiramatsu [this message]
2025-09-17 17:13       ` Randy Dunlap
2025-09-17 22:42         ` Masami Hiramatsu
2025-09-14 14:09 ` [PATCH v4 2/8] x86/hw_breakpoint: Unify breakpoint install/uninstall Masami Hiramatsu (Google)
2025-09-14 14:10 ` [PATCH v4 3/8] x86/hw_breakpoint: Add arch_reinstall_hw_breakpoint Masami Hiramatsu (Google)
2025-09-14 14:10 ` [PATCH v4 4/8] HWBP: Add modify_wide_hw_breakpoint_local() API Masami Hiramatsu (Google)
2025-09-14 14:10 ` [PATCH v4 5/8] tracing: wprobe: Add wprobe event trigger Masami Hiramatsu (Google)
2025-09-15  0:25   ` Randy Dunlap
2025-09-17 14:14     ` Masami Hiramatsu
2025-09-14 14:10 ` [PATCH v4 6/8] selftests: tracing: Add a basic testcase for wprobe Masami Hiramatsu (Google)
2025-09-14 14:10 ` [PATCH v4 7/8] selftests: tracing: Add syntax " Masami Hiramatsu (Google)
2025-09-14 14:11 ` [PATCH v4 8/8] selftests: ftrace: Add wprobe trigger testcase Masami Hiramatsu (Google)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250917233818.71678d0164a6fc2d11fa6e38@kernel.org \
    --to=mhiramat@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=irogers@google.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rdunlap@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=wangjinchao600@gmail.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).