* Re: [PATCH v1 06/11] rcu: Enable RCU callbacks to benefit from expedited grace periods
From: Paul E. McKenney @ 2026-07-14 18:48 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: Puranjay Mohan, rcu, linux-kernel, linux-trace-kernel,
Neeraj Upadhyay, Joel Fernandes, Josh Triplett, Boqun Feng,
Uladzislau Rezki, Steven Rostedt, Mathieu Desnoyers,
Lai Jiangshan, Zqiang, Masami Hiramatsu, Davidlohr Bueso,
Breno Leitao
In-Reply-To: <alD6o01ukyliCS37@localhost.localdomain>
On Fri, Jul 10, 2026 at 03:58:59PM +0200, Frederic Weisbecker wrote:
> Le Wed, Jun 24, 2026 at 06:23:48AM -0700, Puranjay Mohan a écrit :
> > Currently, RCU callbacks only track normal grace-period sequence
> > numbers. This means callbacks must wait for normal grace periods to
> > complete even when expedited grace periods have already elapsed.
> >
> > Use the full struct rcu_gp_seq (which tracks both the normal and
> > expedited grace-period sequences) throughout the callback
> > infrastructure.
> >
> > rcu_segcblist_advance() now checks both normal and expedited GP
> > completion via poll_state_synchronize_rcu_full(), and becomes
> > parameterless since it reads the grace-period state internally.
> > rcu_segcblist_accelerate() stores the full state (both sequences)
> > instead of just the normal one. rcu_accelerate_cbs() and
> > rcu_accelerate_cbs_unlocked() use get_state_synchronize_rcu_full() to
> > capture both sequences, and the NOCB advance checks use
> > poll_state_synchronize_rcu_full() instead of comparing only the normal
> > sequence.
> >
> > srcu_segcblist_advance() becomes a standalone implementation because it
> > compares SRCU sequences directly and cannot use
> > poll_state_synchronize_rcu_full(), which reads RCU-specific globals.
> > srcu_segcblist_accelerate() sets the ->exp field to
> > RCU_GET_STATE_NOT_TRACKED so that poll_state_synchronize_rcu_full()
> > compares only ->norm and ignores ->exp.
> >
> > Reviewed-by: Paul E. McKenney <paulmck@kernel.org>
> > Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
> > ---
> > kernel/rcu/rcu_segcblist.c | 30 +++++++++++++++++++++++-------
> > kernel/rcu/rcu_segcblist.h | 2 +-
> > kernel/rcu/tree.c | 9 +++------
> > kernel/rcu/tree_nocb.h | 33 +++++++++++++++++++++++----------
> > 4 files changed, 50 insertions(+), 24 deletions(-)
> >
> > diff --git a/kernel/rcu/rcu_segcblist.c b/kernel/rcu/rcu_segcblist.c
> > index 4e3dfe42bc097..cf8951d33e767 100644
> > --- a/kernel/rcu/rcu_segcblist.c
> > +++ b/kernel/rcu/rcu_segcblist.c
> > @@ -12,6 +12,7 @@
> > #include <linux/kernel.h>
> > #include <linux/types.h>
> >
> > +#include "rcu.h"
> > #include "rcu_segcblist.h"
> >
> > /* Initialize simple callback list. */
> > @@ -494,9 +495,9 @@ static void rcu_segcblist_advance_compact(struct rcu_segcblist *rsclp, int i)
> >
> > /*
> > * Advance the callbacks in the specified rcu_segcblist structure based
> > - * on the current value passed in for the grace-period counter.
> > + * on the current value of the grace-period counter.
> > */
> > -void rcu_segcblist_advance(struct rcu_segcblist *rsclp, struct rcu_gp_seq *gsp)
> > +void rcu_segcblist_advance(struct rcu_segcblist *rsclp)
> > {
> > int i;
> >
> > @@ -509,7 +510,7 @@ void rcu_segcblist_advance(struct rcu_segcblist *rsclp, struct rcu_gp_seq *gsp)
> > * are ready to invoke, and put them into the RCU_DONE_TAIL segment.
> > */
> > for (i = RCU_WAIT_TAIL; i < RCU_NEXT_TAIL; i++) {
> > - if (ULONG_CMP_LT(gsp->norm, rsclp->gp_seq[i].norm))
> > + if (!poll_state_synchronize_rcu_full(&rsclp->gp_seq[i]))
>
> So after more careful review, the smp_mb() at the end of a successful
> poll_state_synchronize_rcu_full() is necessary here because the current locking
> is not enough to make sure we synchronize against the end of the grace period.
>
> But what about the smp_mb() at the beginning? Paul what is the point of this one
> already? It advertizes to pair with the smp_mb() on root cleanup but what
> exactly is to be ordered here? Why does gp cleanup need to synchronize with
> failing poll_state_synchronize_rcu_full() ? The smp_mb() before rcu_seq_snap()
> in get_state_synchronize_rcu_full() should already synchronize the accesses
> before that call against the beginning of the grace period.
>
> If we keep all these barriers around and both RCU_WAIT_TAIL and RCU_NEXT_READY
> need to be advanced, that makes 4 smp_mb() calls.
Apologies for the delay, I missed this one.
You are right that if poll_state_synchronize_rcu_full() fails we don't
need ordering. Especially given that poll_state_synchronize_rcu()
doesn't have this first memory barrier.
This fits in with get_state_synchronize_full() emulating a call to
synchronize_rcu() and poll_state_synchronize_rcu_full() emulating the
return from synchronize_rcu(). So get_state_synchronize_full() has
its smp_mb() at the beginning and poll_state_synchronize_rcu_full()
at the end.
The only rationale I can give for that initial smp_mb() is that in
the comment, but it makes no sense because the code prior to the call
to poll_state_synchronize_rcu_full() cannot know whether or not this
function will return false.
Puranjay, are you going to remove this smp_mb(), or would you prefer
that I do so?
> > @@ -637,14 +638,29 @@ void rcu_segcblist_merge(struct rcu_segcblist *dst_rsclp,
> >
> > void srcu_segcblist_advance(struct rcu_segcblist *rsclp, unsigned long seq)
> > {
> > - struct rcu_gp_seq gs = { .norm = seq };
> > + int i;
> > +
> > + WARN_ON_ONCE(!rcu_segcblist_is_enabled(rsclp));
> > + if (rcu_segcblist_restempty(rsclp, RCU_DONE_TAIL))
> > + return;
> > +
> > + for (i = RCU_WAIT_TAIL; i < RCU_NEXT_TAIL; i++) {
> > + if (ULONG_CMP_LT(seq, rsclp->gp_seq[i].norm))
> > + break;
>
> Why not use the same API here and consolidate the code? ->exp is RCU_GET_STATE_NOT_TRACKED so it's
> harmless?
>
> > @@ -1164,7 +1164,7 @@ static bool rcu_accelerate_cbs(struct rcu_node *rnp, struct rcu_data *rdp)
> > * accelerating callback invocation to an earlier grace-period
> > * number.
> > */
> > - gs.norm = rcu_seq_snap(&rcu_state.gp_seq);
> > + get_state_synchronize_rcu_full(&gs);
>
> I have similar concerns about the three smp_mb() in
> get_state_synchronize_rcu_full(). It could be just two (rcu_seq_snap()
> has a barrier that could be just one). Not sure if that matters but,
> just wanted to point that.
We need the one at the beginning of get_state_synchronize_rcu_full(),
but from what I can see, not the ones in the calls to rcu_seq_snap().
I blame laziness. We could make an rcu_seq_snap_no_ordering() that
didn't have the smp_mb(), but I didn't believe that the overhead would
be visible at the system level.
Thanx, Paul
> Thanks.
>
> --
> Frederic Weisbecker
> SUSE Labs
^ permalink raw reply
* Re: [RFC PATCH v4 1/3] trace: add lock-free stackmap for stack trace deduplication
From: Steven Rostedt @ 2026-07-14 21:11 UTC (permalink / raw)
To: Li Pengfei
Cc: Masami Hiramatsu, Mathieu Desnoyers, Mark Rutland,
Jonathan Corbet, Shuah Khan, linux-kernel, linux-trace-kernel,
linux-doc, linux-kselftest, lipengfei28, zhangbo56
In-Reply-To: <20260616064119.438063-2-lipengfei28@xiaomi.com>
On Tue, 16 Jun 2026 14:41:17 +0800
Li Pengfei <ljdlns1987@gmail.com> wrote:
> --- /dev/null
> +++ b/kernel/trace/trace_stackmap.c
> @@ -0,0 +1,889 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Ftrace Stack Map - Lock-free stack trace deduplication for ftrace
> + *
> + * Modeled after tracing_map.c (used by hist triggers), this provides
> + * a lock-free hash map optimized for the ftrace hot path. The design
> + * is based on Dr. Cliff Click's non-blocking hash table algorithm.
> + *
> + * Key properties:
> + * - Lock-free insert via cmpxchg, safe in NMI/IRQ/any context
> + * - Pre-allocated element pool (zero allocation on hot path)
> + * - Linear probing with 2x over-provisioned table; probe length
> + * bounded by FTRACE_STACKMAP_MAX_PROBE to keep worst-case lookup
> + * cost constant even when the table is heavily loaded
> + * - Single global instance (initialized for the global trace array)
> + *
> + * Reset is a control-path operation, only allowed when tracing is
> + * stopped on the owning trace_array. The protocol is:
> + *
> + * - atomic_cmpxchg(&resetting, 0, 1) atomically claims reset rights
> + * and blocks new get_id() callers (they observe resetting=1 and
> + * return -EINVAL).
> + * - trace_types_lock serializes the tracer_tracing_is_on() check and
> + * the destructive ring-buffer reset against tracefs writes to
> + * tracing_on.
> + * - synchronize_rcu() drains in-flight get_id() callers from the
> + * ftrace callback path, which runs with preemption disabled.
> + *
> + * Online reset (with tracing active) is intentionally not supported
> + * to keep the design simple and the proof obligations small.
> + *
> + * The 32-bit jhash of the stack IPs is the hash table key. On hash
> + * collision, linear probing finds the next slot and full memcmp
> + * confirms the match.
> + *
> + * Concurrent userspace readers (cat stack_map / stack_map_bin) get
> + * a best-effort snapshot. They are coherent with the hot path
> + * (smp_load_acquire on entry->val); they are also serialized
> + * against reset via smap->reader_sem (readers take it in shared
> + * mode, reset in exclusive mode), so a reset cannot tear an
> + * iteration in progress -- it waits for active readers to drop
> + * the rwsem before clearing the map. The hot path is coordinated
> + * with reset separately, via acquire/release on smap->resetting.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/jhash.h>
> +#include <linux/seq_file.h>
> +#include <linux/kallsyms.h>
> +#include <linux/vmalloc.h>
> +#include <linux/atomic.h>
> +#include <linux/local_lock.h>
> +#include <linux/percpu.h>
> +#include <linux/random.h>
> +#include <linux/rcupdate.h>
> +#include <linux/log2.h>
> +#include <asm/local.h>
> +
> +#include "trace.h"
> +#include "trace_stackmap.h"
> +
> +/*
> + * Bound the linear-probe scan length. With a 2x over-provisioned table,
> + * a well-distributed hash gives very short probe chains. Capping at 64
> + * keeps worst-case lookup O(1) even when the table is heavily loaded
> + * with claimed-but-empty slots from pool exhaustion.
> + */
> +#define FTRACE_STACKMAP_MAX_PROBE 64
> +
> +/*
> + * Memory ordering of entry->val: published with smp_store_release()
> + * by the inserter; consumed with smp_load_acquire() by every reader
> + * that dereferences the elt (get_id, seq_show, bin_open). This pairs
> + * the writes to elt->{nr,ips,ref_count} (initialized BEFORE the
> + * publish) with the reads of those fields (which happen AFTER the
> + * load). seq_start / seq_next only test val for NULL and use the
> + * acquire load purely to keep memory ordering symmetric.
> + */
> +
> +/*
> + * Each pre-allocated element holds one unique stack trace.
> + * Fixed size: MAX_DEPTH entries regardless of actual depth.
> + */
> +struct stackmap_elt {
> + u32 nr; /* actual number of IPs */
> + atomic_t ref_count;
> + unsigned long ips[FTRACE_STACKMAP_MAX_DEPTH];
> +};
> +
> +/*
> + * Hash table entry: a 32-bit key (jhash of stack) + pointer to elt.
> + * key == 0 means the slot is free.
> + */
> +struct stackmap_entry {
> + u32 key; /* 0 = free, non-zero = jhash */
> + struct stackmap_elt *val; /* NULL until fully published */
> +};
> +
> +static struct stackmap_elt *stackmap_load_elt(struct stackmap_entry *entry)
> +{
> + /*
> + * Pairs with the smp_store_release() that publishes entry->val
> + * after fully initializing the element payload.
> + */
> + return smp_load_acquire(&entry->val);
> +}
> +
> +struct ftrace_stackmap {
> + struct trace_array *tr; /* owning trace_array */
> + unsigned int map_bits;
> + unsigned int map_size; /* 1 << (map_bits + 1) */
> + unsigned int max_elts; /* 1 << map_bits */
> + u32 hash_seed; /* per-instance jhash seed */
> + atomic_t next_elt; /* index into elts pool */
> + struct stackmap_entry *entries; /* hash table */
> + struct stackmap_elt *elts; /* flat element pool */
> + atomic_t resetting;
> + /*
> + * Reader/reset serialization. Held in shared mode (read lock)
> + * across seq_file iteration and binary snapshot construction;
> + * held in exclusive mode (write lock) by reset's clearing
> + * phase. The hot path (get_id) does not take this lock — it
> + * uses smp_load_acquire/smp_store_release on entry->val and
> + * the resetting flag for the lock-free protocol.
> + */
> + struct rw_semaphore reader_sem;
> + /*
> + * Per-CPU counters using local_t. local_t increments are NMI-
> + * safe on all architectures (single-instruction or interrupt-
> + * masked) and avoid the raw_spinlock_t fallback that
> + * atomic64_t uses on 32-bit GENERIC_ATOMIC64 — which would
> + * deadlock if an NMI hit while the spinlock was held.
> + */
> + local_t __percpu *successes; /* events served (hits + new inserts) */
> + local_t __percpu *drops;
> +};
> +
> +/*
> + * Cap the bits parameter to keep worst-case allocations bounded:
> + * bits=18 → 256K elts, 512K slots, ~130 MB elt pool, ~130 MB bin
> + * export.
> + * Smaller workloads should use the default (14) which gives 16K elts
> + * (~8 MB pool); bump bits via the ftrace_stackmap.bits= kernel
> + * parameter for higher unique-stack capacity.
> + */
> +#define FTRACE_STACKMAP_BITS_MIN 10
> +#define FTRACE_STACKMAP_BITS_MAX 18
> +#define FTRACE_STACKMAP_BITS_DEFAULT 14
> +
> +static unsigned int stackmap_map_bits = FTRACE_STACKMAP_BITS_DEFAULT;
> +static int __init stackmap_bits_setup(char *str)
> +{
> + unsigned long val;
> +
> + if (kstrtoul(str, 0, &val))
> + return -EINVAL;
> + val = clamp_val(val, FTRACE_STACKMAP_BITS_MIN, FTRACE_STACKMAP_BITS_MAX);
> + stackmap_map_bits = val;
> + return 0;
> +}
> +early_param("ftrace_stackmap.bits", stackmap_bits_setup);
> +
> +/* --- Element pool --- */
> +
> +static struct stackmap_elt *stackmap_get_elt(struct ftrace_stackmap *smap)
> +{
> + int idx;
> +
> + /*
> + * Fast-path early-out once the pool is fully consumed. Avoids
> + * the contended atomic RMW on next_elt for every traced event
> + * after the pool is exhausted.
> + */
> + if (atomic_read(&smap->next_elt) >= smap->max_elts)
> + return NULL;
> +
> + idx = atomic_fetch_add_unless(&smap->next_elt, 1, smap->max_elts);
> + if (idx < smap->max_elts)
> + return &smap->elts[idx];
> + return NULL;
> +}
> +
> +/* --- Create / Destroy / Reset --- */
> +
> +struct ftrace_stackmap *ftrace_stackmap_create(struct trace_array *tr)
> +{
> + struct ftrace_stackmap *smap;
> + unsigned int bits;
> +
> + smap = kzalloc_obj(*smap, GFP_KERNEL);
> + if (!smap)
> + return ERR_PTR(-ENOMEM);
> +
> + /* Defensive clamp: reject bogus bits even if early_param is bypassed. */
> + bits = clamp_val(stackmap_map_bits,
> + FTRACE_STACKMAP_BITS_MIN,
> + FTRACE_STACKMAP_BITS_MAX);
> +
> + smap->tr = tr;
> + smap->map_bits = bits;
> + smap->max_elts = 1U << bits;
> + smap->map_size = 1U << (bits + 1); /* 2x over-provision */
> +
> + smap->entries = vzalloc(sizeof(*smap->entries) * smap->map_size);
Why not:
smap->entries = vcalloc(smap->map_size, sizeof(*smap->entries));
?
> + if (!smap->entries) {
> + kfree(smap);
> + return ERR_PTR(-ENOMEM);
> + }
Make the error paths have:
if (!smap->entries)
goto fail;
> +
> + /*
> + * Single large vmalloc of the element pool, indexed flat.
> + * At bits=18 this is 256K * sizeof(struct stackmap_elt). The
> + * struct is ~520 B (8 + 4 + 4 + 64*8), so total ~135 MB.
> + */
> + smap->elts = vzalloc(sizeof(*smap->elts) * (size_t)smap->max_elts);
vcalloc()?
> + if (!smap->elts) {
goto fail;
> + vfree(smap->entries);
> + kfree(smap);
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + smap->successes = alloc_percpu(local_t);
> + if (!smap->successes) {
goto fail;
> + vfree(smap->elts);
> + vfree(smap->entries);
> + kfree(smap);
> + return ERR_PTR(-ENOMEM);
> + }
> + smap->drops = alloc_percpu(local_t);
> + if (!smap->drops) {
goto fail;
> + free_percpu(smap->successes);
> + vfree(smap->elts);
> + vfree(smap->entries);
> + kfree(smap);
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + smap->hash_seed = get_random_u32();
> + atomic_set(&smap->next_elt, 0);
> + atomic_set(&smap->resetting, 0);
> + init_rwsem(&smap->reader_sem);
> +
> + return smap;
fail:
if (smap) {
free_percpu(smap->successes);
vfree(smap->elts);
vfree(smap->entries);
kfree(smap);
// As all the above handle passing in NULL just fine.
}
return ERR_PTR(-ENOMEM);
> +}
> +
> +void ftrace_stackmap_destroy(struct ftrace_stackmap *smap)
> +{
> + if (!smap || IS_ERR(smap))
> + return;
> + free_percpu(smap->drops);
> + free_percpu(smap->successes);
> + vfree(smap->elts);
> + vfree(smap->entries);
> + kfree(smap);
> +}
> +
> +/**
> + * ftrace_stackmap_reset - clear all entries in the stackmap
> + * @smap: the stackmap to reset
> + *
> + * Returns 0 on success, -EBUSY if another reset is already in
> + * progress, or if tracing is currently active on the owning
> + * trace_array.
> + *
> + * Online reset (with tracing active) is not supported. Caller must
> + * stop tracing first (echo 0 > tracing_on).
> + *
> + * Caller is process context (typically sysfs write handler).
> + *
> + * Protocol:
> + * 1. Atomically claim reset rights via cmpxchg on @resetting.
> + * 2. Take trace_types_lock to serialize against tracefs writes to
> + * tracing_on.
> + * 3. Verify tracing is stopped on @smap->tr; if not, release the
> + * claim and return -EBUSY. The resetting flag itself blocks
> + * any subsequent get_id() callers.
> + * 4. synchronize_rcu() drains in-flight get_id() callers from the
> + * ftrace callback path (which runs preempt-disabled).
> + * 5. Reset the ring buffer(s), then memset entries, elts, and
> + * counters.
> + * 6. Release the resetting flag with release semantics so any new
> + * get_id() observes a fully cleared map.
> + */
> +int ftrace_stackmap_reset(struct ftrace_stackmap *smap)
> +{
> + struct trace_array *tr;
> + int ret = 0;
> +
> + if (!smap)
> + return 0;
> +
> + if (atomic_cmpxchg(&smap->resetting, 0, 1) != 0)
> + return -EBUSY;
> +
> + mutex_lock(&trace_types_lock);
> +
> + tr = smap->tr;
> + if (tr && tracer_tracing_is_on(tr)) {
> + ret = -EBUSY;
> + goto out_unlock;
> + }
> +
> + /*
> + * synchronize_rcu() itself is a full barrier; no extra smp_mb()
> + * is needed before it. It drains in-flight ftrace callbacks that
> + * may have already passed the resetting check with the old value.
> + */
> + synchronize_rcu();
> +
> + /*
> + * Take the reader_sem in exclusive mode. This serializes the
> + * memset against any tracefs reader (seq_file iteration or
> + * stack_map_bin snapshot) that may currently hold the rwsem
> + * for read. synchronize_rcu() already drained the hot path;
> + * this rwsem covers process-context readers that aren't
> + * preempt-disabled.
> + */
> + down_write(&smap->reader_sem);
> +
> + /*
> + * Clear the ring buffer(s) BEFORE the map, both under the write
> + * lock. The ring buffer may still hold TRACE_STACK_ID events
> + * whose stack_id points at slots we are about to free/reuse.
> + * Resetting the buffer first guarantees an external observer
> + * never sees the inconsistent "trace still has <stack_id N> but
> + * the map is already empty" window: it sees either (old buffer,
> + * old map) or (cleared buffer, old map) or (cleared buffer,
> + * cleared map) -- never (old buffer, cleared map).
> + *
> + * Use tracing_reset_all_cpus() (not _online_cpus) so per-CPU
> + * buffers belonging to currently offline CPUs are also cleared.
> + * The ring buffer is allocated per-possible-CPU; an offline CPU's
> + * buffer can still hold a TRACE_STACK_ID event written before
> + * the CPU went offline. tracing_reset_online_cpus() iterates
> + * for_each_online_buffer_cpu() and would leave that data behind
> + * to be observed once the CPU comes back online (or by the
> + * trace reader, which iterates all allocated CPU buffers),
> + * recreating the stale-stack_id window we are trying to close.
> + *
> + * Since reset requires tracing to be stopped, this makes "reset"
> + * an explicitly destructive operation on the owning trace_array,
> + * keeping ring-buffer stack_ids and the map coherent.
> + */
> + if (tr) {
> + tracing_reset_all_cpus(&tr->array_buffer);
> +#ifdef CONFIG_TRACER_SNAPSHOT
> + if (tr->allocated_snapshot)
> + tracing_reset_all_cpus(&tr->snapshot_buffer);
> +#endif
> + }
> +
> + memset(smap->entries, 0, sizeof(*smap->entries) * smap->map_size);
> + memset(smap->elts, 0, sizeof(*smap->elts) * (size_t)smap->max_elts);
> +
> + atomic_set(&smap->next_elt, 0);
> + {
Do not add anonymous blocks in functions.
> + int cpu;
Just declare cpu at the beginning of the function.
> +
> + for_each_possible_cpu(cpu) {
> + local_set(per_cpu_ptr(smap->successes, cpu), 0);
> + local_set(per_cpu_ptr(smap->drops, cpu), 0);
> + }
> + }
> +
> + up_write(&smap->reader_sem);
> +
> +out_unlock:
> + mutex_unlock(&trace_types_lock);
> +
> + /* Release resetting=0 so new get_id() observes a cleared map. */
> + atomic_set_release(&smap->resetting, 0);
> + return ret;
> +}
> +
> +/* --- Core: get_id (lock-free, NMI-safe) --- */
> +
> +int ftrace_stackmap_get_id(struct ftrace_stackmap *smap,
> + unsigned long *ips, unsigned int nr_entries)
> +{
> + u32 key_hash, idx, test_key, trace_len;
> + struct stackmap_entry *entry;
> + struct stackmap_elt *val;
> + int probes = 0;
> +
> + /*
> + * atomic_read_acquire() pairs with atomic_set_release() in the
> + * reset path. This ensures that subsequent reads of entry->key
> + * and entry->val are ordered after this check; without acquire,
> + * the CPU would only have a control dependency, which orders
> + * subsequent stores but not loads (per LKMM).
> + */
> + if (!smap || !nr_entries || atomic_read_acquire(&smap->resetting))
> + return -EINVAL;
> + /*
> + * Never truncate: a stack deeper than the map can hold must not be
> + * silently shortened, or two distinct traces sharing their first
> + * FTRACE_STACKMAP_MAX_DEPTH frames would be merged into one
> + * stack_id. The caller is expected to fall back to a full stack
> + * trace for such events. Reject defensively in case of a future
> + * caller that forgets this contract.
> + */
> + if (nr_entries > FTRACE_STACKMAP_MAX_DEPTH)
> + return -E2BIG;
> +
> + trace_len = nr_entries * sizeof(unsigned long);
> + /*
> + * jhash2() requires the length in u32 units and the data to be
> + * u32-aligned. On 64-bit kernels sizeof(unsigned long)==8, so
> + * trace_len is always a multiple of 8 (hence of 4). Use jhash2
> + * directly; the cast to u32* is safe because ips[] is naturally
> + * aligned to sizeof(unsigned long) >= 4.
> + */
> + key_hash = jhash2((const u32 *)ips, trace_len / sizeof(u32),
> + smap->hash_seed);
> + if (key_hash == 0)
> + key_hash = 1; /* 0 means free slot */
> +
> + idx = key_hash >> (32 - (smap->map_bits + 1));
> +
> + while (probes < FTRACE_STACKMAP_MAX_PROBE) {
> + idx &= (smap->map_size - 1);
> + entry = &smap->entries[idx];
> + /*
> + * READ_ONCE() to avoid LKMM data race with concurrent
> + * cmpxchg(&entry->key, 0, key_hash) on this slot.
> + */
> + test_key = READ_ONCE(entry->key);
> +
> + if (test_key == key_hash) {
> + val = stackmap_load_elt(entry);
> + /*
> + * READ_ONCE(val->nr) keeps style consistent with
> + * the seq_show / bin_open readers. nr is write-once
> + * (set before publish, never modified afterwards),
> + * so the load is data-race-free, but READ_ONCE
> + * silences any analysis tool that flags a plain
> + * read of a field that is also read under acquire
> + * elsewhere.
> + */
> + if (val && READ_ONCE(val->nr) == nr_entries &&
> + memcmp(val->ips, ips, trace_len) == 0) {
> + /*
> + * ref_count is a best-effort popularity
> + * counter. On a long (from-boot, multi-hour)
> + * trace a hot stack can be hit billions of
> + * times. atomic_add_unless() gives true
> + * saturation at INT_MAX even under concurrent
> + * hits on multiple CPUs (a plain
> + * check-then-inc could let several CPUs past
> + * the check near the cap and still wrap).
> + */
> + atomic_add_unless(&val->ref_count, 1, INT_MAX);
> + /*
> + * successes/drops are best-effort throughput
> + * counters. Saturate at LONG_MAX so they do
> + * not wrap on long runs (notably where local_t
> + * is 32-bit), matching ref_count's behaviour.
> + */
> + local_add_unless(this_cpu_ptr(smap->successes),
> + 1, LONG_MAX);
> + return (int)idx;
> + }
> + /*
> + * val == NULL: another CPU is mid-insert, or this
> + * slot is "claimed but empty" (pool exhausted).
> + * val != NULL but mismatch: 32-bit hash collision
> + * with a different stack. In both cases, advance.
> + */
> + } else if (!test_key) {
> + /*
> + * Free slot: try to claim it.
> + *
> + * If two CPUs race here with the same key_hash
> + * (same stack), one loses the cmpxchg, advances,
> + * and may insert the same stack at a later slot.
> + * This can produce a small number of duplicate
> + * entries under heavy contention. The trade-off
> + * is accepted to keep the hot path lock-free;
> + * ref_count is split across the duplicates and
> + * total memory cost is bounded by the element
> + * pool size.
> + */
> + if (cmpxchg(&entry->key, 0, key_hash) == 0) {
> + struct stackmap_elt *elt;
> +
> + elt = stackmap_get_elt(smap);
> + if (!elt) {
> + /*
> + * Pool exhausted. We claimed this
> + * slot with cmpxchg but cannot fill
> + * it. Leave key set so the slot
> + * stays "claimed but empty" — future
> + * lookups treat val==NULL as a miss
> + * and probe past it. Cannot revert
> + * key=0 without racing other CPUs.
> + */
> + local_add_unless(this_cpu_ptr(smap->drops),
> + 1, LONG_MAX);
> + return -ENOSPC;
> + }
> +
> + elt->nr = nr_entries;
> + atomic_set(&elt->ref_count, 1);
> + memcpy(elt->ips, ips, trace_len);
> +
> + /*
> + * Publish elt with release semantics so the
> + * reader's smp_load_acquire can safely
> + * dereference val->nr / val->ips.
> + */
> + smp_store_release(&entry->val, elt);
> + local_add_unless(this_cpu_ptr(smap->successes),
> + 1, LONG_MAX);
> + return (int)idx;
> + }
> + /* cmpxchg failed; another CPU claimed this slot. */
> + }
> +
> + idx++;
> + probes++;
> + }
> +
> + local_add_unless(this_cpu_ptr(smap->drops), 1, LONG_MAX);
> + return -ENOSPC;
> +}
> +
> +/* --- Text export: /sys/kernel/debug/tracing/stack_map --- */
> +
> +struct stackmap_seq_private {
> + struct ftrace_stackmap *smap;
> +};
> +
> +static void *stackmap_seq_start(struct seq_file *m, loff_t *pos)
> +{
> + struct stackmap_seq_private *priv = m->private;
> + struct ftrace_stackmap *smap = priv->smap;
> + u32 i;
> +
> + if (!smap)
> + return NULL;
> + /*
> + * Take the reader_sem to serialize against ftrace_stackmap_reset(),
> + * which holds it for write while clearing the table. Released in
> + * stackmap_seq_stop(), which seq_file calls regardless of whether
> + * start() returned an element or NULL (per Documentation/filesystems
> + * /seq_file.rst: "the iterator value returned by start() or next()
> + * is guaranteed to be passed to a subsequent next() or stop()").
> + */
> + down_read(&smap->reader_sem);
> + for (i = *pos; i < smap->map_size; i++) {
> + if (READ_ONCE(smap->entries[i].key) &&
> + stackmap_load_elt(&smap->entries[i])) {
> + *pos = i;
> + return &smap->entries[i];
> + }
> + }
> + return NULL;
> +}
> +
> +static void *stackmap_seq_next(struct seq_file *m, void *v, loff_t *pos)
> +{
> + struct stackmap_seq_private *priv = m->private;
> + struct ftrace_stackmap *smap = priv->smap;
> + u32 i;
> +
> + if (!smap)
> + return NULL;
> + for (i = *pos + 1; i < smap->map_size; i++) {
> + if (READ_ONCE(smap->entries[i].key) &&
> + stackmap_load_elt(&smap->entries[i])) {
> + *pos = i;
> + return &smap->entries[i];
> + }
> + }
> + /*
> + * Advance *pos past the end so that on the next read() the
> + * subsequent stackmap_seq_start() call returns NULL and the
> + * iteration terminates. Without this, seq_read() would loop
> + * on the last element.
> + */
> + *pos = smap->map_size;
> + return NULL;
> +}
> +
> +static void stackmap_seq_stop(struct seq_file *m, void *v)
> +{
> + struct stackmap_seq_private *priv = m->private;
> + struct ftrace_stackmap *smap = priv->smap;
> +
> + /*
> + * seq_file invokes stop() unconditionally after each iteration
> + * pass (see seq_read_iter / traverse), even when start() returned
> + * NULL. Always release here, balanced against the down_read in
> + * stackmap_seq_start().
> + */
> + if (smap)
> + up_read(&smap->reader_sem);
> +}
> +
> +static int stackmap_seq_show(struct seq_file *m, void *v)
> +{
> + struct stackmap_entry *entry = v;
> + struct stackmap_seq_private *priv = m->private;
> + struct stackmap_elt *elt;
> + u32 idx = entry - priv->smap->entries;
> + u32 i, nr;
> +
> + elt = stackmap_load_elt(entry);
> + if (!elt)
> + return 0;
> +
> + nr = READ_ONCE(elt->nr);
> + if (nr > FTRACE_STACKMAP_MAX_DEPTH)
> + nr = FTRACE_STACKMAP_MAX_DEPTH;
> +
> + seq_printf(m, "stack_id %u [ref %u, depth %u]\n",
> + idx, atomic_read(&elt->ref_count), nr);
> + for (i = 0; i < nr; i++) {
> + unsigned long ip = elt->ips[i];
> +
> + /*
> + * Mirror trace_stack_print(): __ftrace_trace_stack()
> + * may replace trampoline addresses with
> + * FTRACE_TRAMPOLINE_MARKER before the stack reaches the
> + * map, and normal addresses must go through
> + * trace_adjust_address() (KASLR / module text delta)
> + * before symbolization. Without this the export would
> + * print a bogus symbol for the marker and unadjusted
> + * addresses for everything else.
> + */
> + if (ip == FTRACE_TRAMPOLINE_MARKER) {
> + seq_printf(m, " [%u] [FTRACE TRAMPOLINE]\n", i);
> + continue;
> + }
> + seq_printf(m, " [%u] %pS\n", i,
> + (void *)trace_adjust_address(priv->smap->tr, ip));
> + }
> + seq_putc(m, '\n');
> + return 0;
> +}
> +
> +static const struct seq_operations stackmap_seq_ops = {
> + .start = stackmap_seq_start,
> + .next = stackmap_seq_next,
> + .stop = stackmap_seq_stop,
> + .show = stackmap_seq_show,
> +};
> +
> +static int stackmap_open(struct inode *inode, struct file *file)
> +{
> + struct stackmap_seq_private *priv;
> + struct seq_file *m;
> + int ret;
> +
> + ret = seq_open_private(file, &stackmap_seq_ops,
> + sizeof(struct stackmap_seq_private));
> + if (ret)
> + return ret;
> + m = file->private_data;
> + priv = m->private;
> + priv->smap = inode->i_private;
> + return 0;
> +}
> +
> +/*
> + * Accept exactly "0" or "reset" (optionally followed by a single newline).
> + */
> +static bool stackmap_write_is_reset(const char *buf, size_t n)
> +{
> + if (n > 0 && buf[n - 1] == '\n')
> + n--;
> + return (n == 1 && buf[0] == '0') ||
> + (n == 5 && memcmp(buf, "reset", 5) == 0);
> +}
> +
> +static ssize_t stackmap_write(struct file *file, const char __user *ubuf,
> + size_t count, loff_t *ppos)
> +{
> + struct seq_file *m = file->private_data;
> + struct stackmap_seq_private *priv = m->private;
> + char buf[8];
> + size_t n = min(count, sizeof(buf) - 1);
> + int ret;
> +
> + if (n == 0)
> + return -EINVAL;
> + if (copy_from_user(buf, ubuf, n))
> + return -EFAULT;
> + buf[n] = '\0';
> +
> + if (!stackmap_write_is_reset(buf, n))
> + return -EINVAL;
> +
> + /*
> + * ftrace_stackmap_reset() atomically claims reset rights via
> + * cmpxchg and returns -EBUSY if another reset is in progress
> + * or if tracing is active.
> + */
> + ret = ftrace_stackmap_reset(priv->smap);
> + if (ret)
> + return ret;
> + return count;
> +}
> +
> +const struct file_operations ftrace_stackmap_fops = {
> + .open = stackmap_open,
> + .read = seq_read,
> + .write = stackmap_write,
> + .llseek = seq_lseek,
> + .release = seq_release_private,
> +};
> +
> +/* --- Stats --- */
> +
> +static int stackmap_stat_show(struct seq_file *m, void *v)
> +{
> + struct ftrace_stackmap *smap = m->private;
> + u64 successes = 0, drops = 0;
> + u32 entries;
> + int cpu;
> +
> + if (!smap) {
> + seq_puts(m, "stackmap not initialized\n");
> + return 0;
> + }
> +
> + entries = atomic_read(&smap->next_elt);
> + for_each_possible_cpu(cpu) {
> + successes += local_read(per_cpu_ptr(smap->successes, cpu));
> + drops += local_read(per_cpu_ptr(smap->drops, cpu));
> + }
> +
> + seq_printf(m, "entries: %u / %u\n", entries, smap->max_elts);
> + seq_printf(m, "table_size: %u\n", smap->map_size);
> + seq_printf(m, "successes: %llu\n", successes);
> + seq_printf(m, "drops: %llu\n", drops);
> + if (successes + drops > 0)
> + seq_printf(m, "success_rate: %llu%%\n",
> + successes * 100 / (successes + drops));
> + return 0;
> +}
> +
> +static int stackmap_stat_open(struct inode *inode, struct file *file)
> +{
> + return single_open(file, stackmap_stat_show, inode->i_private);
> +}
> +
> +const struct file_operations ftrace_stackmap_stat_fops = {
> + .open = stackmap_stat_open,
> + .read = seq_read,
> + .llseek = seq_lseek,
> + .release = single_release,
> +};
> +
> +/* --- Binary export --- */
> +
> +struct stackmap_bin_snapshot {
> + /*
> + * Use u64 (not size_t) so data[] is 8-byte aligned on both
> + * 32-bit and 64-bit architectures. The IP array within data[]
> + * is accessed as u64*, which would alignment-fault on strict
> + * architectures (e.g. older ARM, SPARC) if data[] started at
> + * a 4-byte boundary.
> + */
> + u64 size;
> + char data[];
> +};
> +
> +static int stackmap_bin_open(struct inode *inode, struct file *file)
> +{
> + struct ftrace_stackmap *smap = inode->i_private;
> + struct stackmap_bin_snapshot *snap;
> + struct ftrace_stackmap_bin_header *hdr;
> + size_t alloc_size, off;
> + u32 nr_entries, i, nr_stacks;
> +
> + if (!smap)
> + return -ENODEV;
> +
> + /*
> + * Worst-case allocation size: every populated entry uses a
> + * full-depth stack. The (+1) gives one slack slot in case a
> + * concurrent insert lands between this snapshot and iteration.
> + * The loop below performs an explicit bounds check anyway.
> + *
> + * At bits=18 this caps at ~135 MB. The file is mode 0440
> + * (TRACE_MODE_READ), so only privileged users can open it.
> + */
> + nr_entries = atomic_read(&smap->next_elt);
> + alloc_size = sizeof(*hdr) + (nr_entries + 1) *
> + (sizeof(struct ftrace_stackmap_bin_entry) +
> + FTRACE_STACKMAP_MAX_DEPTH * sizeof(u64));
Really should have ftrace_stackmap_bin_entry have a flexible array:
(move struct ftrace_stackmap_bin_entry *e to top)
alloc_size = sizeof(*hdr) + (nr_entries + 1) *
struct_size(e, ips, FTRACE_STACKMAP_MAX_DEPTH);
> +
> + snap = vmalloc(sizeof(*snap) + alloc_size);
> + if (!snap)
> + return -ENOMEM;
> +
> + hdr = (struct ftrace_stackmap_bin_header *)snap->data;
> + hdr->magic = FTRACE_STACKMAP_BIN_MAGIC;
> + hdr->version = FTRACE_STACKMAP_BIN_VERSION;
> + hdr->reserved = 0;
> + off = sizeof(*hdr);
> + nr_stacks = 0;
> +
> + /*
> + * Take reader_sem to serialize against ftrace_stackmap_reset(),
> + * which clears the table and elt pool under the write lock.
> + */
> + down_read(&smap->reader_sem);
> +
> + for (i = 0; i < smap->map_size; i++) {
> + struct stackmap_entry *entry = &smap->entries[i];
> + struct stackmap_elt *elt;
> + struct ftrace_stackmap_bin_entry *e;
move to top of function.
> + u64 *ips_out;
> + u32 k, nr;
> +
> + if (!READ_ONCE(entry->key))
> + continue;
> + elt = stackmap_load_elt(entry);
> + if (!elt)
> + continue;
> +
> + nr = READ_ONCE(elt->nr);
> + if (nr > FTRACE_STACKMAP_MAX_DEPTH)
> + nr = FTRACE_STACKMAP_MAX_DEPTH;
> +
> + /* Bounds check: stop if we would overflow the allocation. */
> + if (off + sizeof(*e) + nr * sizeof(u64) > alloc_size)
if (off + struct_size(e, ips, nr) > alloc_size)
> + break;
> +
> + e = (struct ftrace_stackmap_bin_entry *)(snap->data + off);
> + e->stack_id = i;
> + e->nr = nr;
> + e->ref_count = atomic_read(&elt->ref_count);
> + e->reserved = 0;
> + off += sizeof(*e);
delete the above.
> +
> + ips_out = (u64 *)(snap->data + off);
ips_out = e->ips;
> + for (k = 0; k < nr; k++) {
> + unsigned long ip = elt->ips[k];
> +
> + /*
> + * Emit the trampoline marker verbatim so userspace
> + * can render it as [FTRACE TRAMPOLINE]; pass every
> + * other address through trace_adjust_address() so the
> + * binary export follows the same address-adjustment
> + * rules as the text export.
> + */
> + if (ip == FTRACE_TRAMPOLINE_MARKER)
> + ips_out[k] = (u64)FTRACE_TRAMPOLINE_MARKER;
> + else
> + ips_out[k] = (u64)trace_adjust_address(smap->tr, ip);
> + }
> + off += nr * sizeof(u64);
off += struct_size(e, ips, nr);
> + nr_stacks++;
> + }
> +
> + up_read(&smap->reader_sem);
> +
> + hdr->nr_stacks = nr_stacks;
> + snap->size = off;
> + file->private_data = snap;
> + return 0;
> +}
> +
> +static ssize_t stackmap_bin_read(struct file *file, char __user *ubuf,
> + size_t count, loff_t *ppos)
> +{
> + struct stackmap_bin_snapshot *snap = file->private_data;
> +
> + if (!snap)
> + return -EINVAL;
> + return simple_read_from_buffer(ubuf, count, ppos, snap->data, snap->size);
> +}
> +
> +static int stackmap_bin_release(struct inode *inode, struct file *file)
> +{
> + vfree(file->private_data);
> + return 0;
> +}
> +
> +const struct file_operations ftrace_stackmap_bin_fops = {
> + .open = stackmap_bin_open,
> + .read = stackmap_bin_read,
> + .llseek = default_llseek,
> + .release = stackmap_bin_release,
> +};
> diff --git a/kernel/trace/trace_stackmap.h b/kernel/trace/trace_stackmap.h
> new file mode 100644
> index 000000000000..7c2e5ab9d36d
> --- /dev/null
> +++ b/kernel/trace/trace_stackmap.h
> @@ -0,0 +1,57 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _TRACE_STACKMAP_H
> +#define _TRACE_STACKMAP_H
> +
> +#include <linux/types.h>
> +#include <linux/atomic.h>
> +
> +#define FTRACE_STACKMAP_MAX_DEPTH 64
> +
> +/* Binary export format */
> +#define FTRACE_STACKMAP_BIN_MAGIC 0x46534D42 /* 'FSMB' */
> +#define FTRACE_STACKMAP_BIN_VERSION 1
> +
> +struct ftrace_stackmap_bin_header {
> + u32 magic;
> + u32 version;
> + u32 nr_stacks;
> + u32 reserved;
> +};
> +
> +struct ftrace_stackmap_bin_entry {
> + u32 stack_id;
> + u32 nr;
> + u32 ref_count;
> + u32 reserved;
> + /* followed by u64 ips[nr] */
Why not make this a flexible array?
u64 ips[];
Then the code can be simpler as described above.
-- Steve
> +};
> +
> +struct trace_array;
> +
> +#ifdef CONFIG_FTRACE_STACKMAP
> +
> +struct ftrace_stackmap;
> +
> +struct ftrace_stackmap *ftrace_stackmap_create(struct trace_array *tr);
> +void ftrace_stackmap_destroy(struct ftrace_stackmap *smap);
> +int ftrace_stackmap_get_id(struct ftrace_stackmap *smap,
> + unsigned long *ips, unsigned int nr_entries);
> +int ftrace_stackmap_reset(struct ftrace_stackmap *smap);
> +
> +extern const struct file_operations ftrace_stackmap_fops;
> +extern const struct file_operations ftrace_stackmap_stat_fops;
> +extern const struct file_operations ftrace_stackmap_bin_fops;
> +
> +#else
> +
> +struct ftrace_stackmap;
> +static inline struct ftrace_stackmap *
> +ftrace_stackmap_create(struct trace_array *tr) { return NULL; }
> +static inline void ftrace_stackmap_destroy(struct ftrace_stackmap *s) { }
> +static inline int ftrace_stackmap_get_id(struct ftrace_stackmap *s,
> + unsigned long *ips, unsigned int n)
> +{ return -EOPNOTSUPP; }
> +static inline int ftrace_stackmap_reset(struct ftrace_stackmap *s) { return 0; }
> +
> +#endif
> +#endif /* _TRACE_STACKMAP_H */
^ permalink raw reply
* Re: [RFC PATCH 08/13] mm/kwatch: add hardware breakpoint backend
From: Steven Rostedt @ 2026-07-14 21:14 UTC (permalink / raw)
To: Jinchao Wang
Cc: Andrew Morton, Peter Zijlstra, Thomas Gleixner, Masami Hiramatsu,
Ingo Molnar, Borislav Petkov, Dave Hansen, H . Peter Anvin, x86,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Mathieu Desnoyers, David Hildenbrand, Jonathan Corbet,
Matthew Wilcox, linux-kernel, linux-mm, linux-trace-kernel,
linux-perf-users, linux-doc
In-Reply-To: <20260714183206.12688-1-wangjinchao600@gmail.com>
On Wed, 15 Jul 2026 02:32:06 +0800
Jinchao Wang <wangjinchao600@gmail.com> wrote:
> --- /dev/null
> +++ b/include/trace/events/kwatch.h
> @@ -0,0 +1,57 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM kwatch
> +
> +#if !defined(_TRACE_KWATCH_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_KWATCH_H
> +
> +#include <linux/tracepoint.h>
> +#include <linux/ptrace.h>
> +
> +#define KWATCH_STACK_DEPTH 8
> +
> +struct trace_seq;
> +const char *kwatch_trace_print_stack(struct trace_seq *p,
> + const unsigned long *stack,
> + unsigned int nr);
> +
> +TRACE_EVENT(kwatch_hit,
> + TP_PROTO(unsigned long ip, unsigned long sp, unsigned long addr,
> + u64 time_ns,
> + unsigned long *stack_entries, unsigned int stack_nr),
> + TP_ARGS(ip, sp, addr, time_ns, stack_entries, stack_nr),
> +
> + TP_STRUCT__entry(
> + __field(unsigned long, ip)
> + __field(unsigned long, sp)
> + __field(unsigned long, addr)
> + __field(u64, time_ns)
Move the time_ns to the first field, as unsigned long on 32 bit
architectures is 4 bytes, and this will make 4 byte "hole" in the event.
> + __field(unsigned int, stack_nr)
Make stack_nr the last element for the same reason.
> + __array(unsigned long, stack, KWATCH_STACK_DEPTH)
Make the above a dynamic array based on stack entries.
__dynamic_array(unsigned long, stack, min_t(unsigned int, stack_nr,
KWATCH_STACK_DEPTH);
> + ),
> +
> + TP_fast_assign(
> + unsigned int i;
unsigned long *stack = __get_dynamic_array(stack);
> +
> + __entry->ip = ip;
> + __entry->sp = sp;
> + __entry->addr = addr;
> + __entry->time_ns = time_ns;
> + __entry->stack_nr = min_t(unsigned int, stack_nr,
> + KWATCH_STACK_DEPTH);
> + for (i = 0; i < __entry->stack_nr; i++)
> + __entry->stack[i] = stack_entries[i];
stack[i] = stack_entries[i];
> + ),
> +
> + TP_printk("KWatch HIT: time=%llu.%06lu ip=%pS addr=0x%lx%s",
> + __entry->time_ns / 1000000000ULL,
> + (unsigned long)((__entry->time_ns / 1000ULL) % 1000000ULL),
> + (void *)__entry->ip, __entry->addr,
> + kwatch_trace_print_stack(p, __entry->stack,
kwatch_trace_print_stack(p, __get_dynamic_array(stack),
> + __entry->stack_nr))
> +);
> +
-- Steve
^ permalink raw reply
* Re: [RFC PATCH v4 2/3] trace: integrate stackmap into ftrace stack recording path
From: Steven Rostedt @ 2026-07-14 21:53 UTC (permalink / raw)
To: Li Pengfei
Cc: Masami Hiramatsu, Mathieu Desnoyers, Mark Rutland,
Jonathan Corbet, Shuah Khan, linux-kernel, linux-trace-kernel,
linux-doc, linux-kselftest, lipengfei28, zhangbo56
In-Reply-To: <20260616064119.438063-3-lipengfei28@xiaomi.com>
On Tue, 16 Jun 2026 14:41:18 +0800
Li Pengfei <ljdlns1987@gmail.com> wrote:
> int set_tracer_flag(struct trace_array *tr, u64 mask, int enabled)
> {
> switch (mask) {
> @@ -3993,6 +4091,33 @@ int set_tracer_flag(struct trace_array *tr, u64 mask, int enabled)
> if (!!(tr->trace_flags & mask) == !!enabled)
> return 0;
>
> +#ifdef CONFIG_FTRACE_STACKMAP
> + /*
> + * STACKMAP is intentionally global-instance-only: the dedup map,
> + * its tracefs files (stack_map / stack_map_stat / stack_map_bin)
> + * and the lifetime/reset semantics are tied to the global trace
> + * array. options/stackmap is hidden on secondary instances via
> + * TOP_LEVEL_TRACE_FLAGS, but writes still reach set_tracer_flag()
> + * through the aggregate trace_options file. Reject the enable on
> + * a secondary instance so it cannot be silently accepted and then
> + * become a no-op in the hot path (where tr->stackmap is NULL and
> + * the code falls back to a full stack trace).
> + *
> + * On the global instance, allow the enable while init is still
> + * pending (boot-time trace_options=stackmap is applied before the
> + * tracefs init work creates the map; the hot path falls back
> + * until the map is published). Only reject once init has
> + * permanently failed, so options/stackmap never reports an
> + * enabled no-op. READ_ONCE() suffices: this only inspects the
> + * init state, it does not dereference the map (the hot path uses
> + * smp_load_acquire(&tr->stackmap) for that).
> + */
> + if (mask == TRACE_ITER(STACKMAP) && enabled &&
> + (tr != &global_trace ||
> + READ_ONCE(stackmap_init_state) == STACKMAP_INIT_FAILED))
> + return -EINVAL;
> +#endif
> +
> /* Give the tracer a chance to approve the change */
> if (tr->current_trace->flag_changed)
> if (tr->current_trace->flag_changed(tr, mask, !!enabled))
> @@ -9222,6 +9347,91 @@ static __init void tracer_init_tracefs_work_func(struct work_struct *work)
> NULL, &tracing_dyn_info_fops);
> #endif
>
> +#ifdef CONFIG_FTRACE_STACKMAP
> + {
> + struct ftrace_stackmap *smap;
> + struct dentry *map_file;
> +
> + smap = ftrace_stackmap_create(&global_trace);
> + if (!IS_ERR(smap)) {
> + /*
> + * Failure-atomic init: stack_map is the single
> + * required tracefs file (it doubles as the reset
> + * interface and the human-readable resolver). If
> + * we cannot create it, the hot path must not be
> + * able to emit <stack_id N> events that no one can
> + * resolve or clear, so refuse to publish the map
> + * and tear it down.
> + *
> + * Create stack_map BEFORE smp_store_release() so an
> + * observed non-NULL global_trace.stackmap implies
> + * its resolver/reset file exists.
> + */
> + map_file = trace_create_file("stack_map",
> + TRACE_MODE_WRITE, NULL,
> + smap,
> + &ftrace_stackmap_fops);
> + if (!map_file) {
> + pr_warn("ftrace stackmap init: stack_map create failed, dedup disabled\n");
> + ftrace_stackmap_destroy(smap);
> + /*
> + * Permanent failure. Record it and clear a
> + * STACKMAP flag that a boot-time
> + * trace_options=stackmap may have set, so
> + * options/stackmap does not report an
> + * enabled no-op and later userspace enables
> + * return -EINVAL.
> + */
> + WRITE_ONCE(stackmap_init_state,
> + STACKMAP_INIT_FAILED);
> + global_trace.trace_flags &=
> + ~TRACE_ITER(STACKMAP);
80 columns is no longer a hard requirement. 100 is more the default, so the
above should be:
WRITE_ONCE(stackmap_init_state, STACKMAP_INIT_FAILED);
global_trace.trace_flags &= ~TRACE_ITER(STACKMAP);
> + } else {
> + /*
> + * smp_store_release pairs with the
> + * smp_load_acquire() in
> + * __ftrace_trace_stack(). Publishing only
> + * after the required file exists keeps
> + * "smap visible" => "resolver/reset
> + * available".
> + */
> + smp_store_release(&global_trace.stackmap,
> + smap);
> + WRITE_ONCE(stackmap_init_state,
> + STACKMAP_INIT_DONE);
Same with the above two.
> + /*
> + * stat and bin are auxiliary observability
> + * surfaces. If they fail to be created we
> + * keep dedup enabled (the kernel side still
> + * works, and stack_map alone is enough to
> + * resolve and reset); trace_create_file()
> + * already pr_warn()s on failure.
> + */
> + trace_create_file("stack_map_stat",
> + TRACE_MODE_READ, NULL,
> + smap,
> + &ftrace_stackmap_stat_fops);
> + trace_create_file("stack_map_bin",
> + TRACE_MODE_READ, NULL,
> + smap,
> + &ftrace_stackmap_bin_fops);
> + }
-- Steve
^ permalink raw reply
* Re: [PATCH] tracing: ring-buffer: allowlist clang-generated symbols
From: Steven Rostedt @ 2026-07-14 21:56 UTC (permalink / raw)
To: Vincent Donnefort
Cc: Arnd Bergmann, Masami Hiramatsu, Nathan Chancellor, Arnd Bergmann,
Mathieu Desnoyers, Nick Desaulniers, Bill Wendling, Justin Stitt,
Marc Zyngier, Thomas Weißschuh, Paolo Bonzini, linux-kernel,
linux-trace-kernel, llvm
In-Reply-To: <ajKgjNuHUlteSziZ@google.com>
On Wed, 17 Jun 2026 14:26:36 +0100
Vincent Donnefort <vdonnefort@google.com> wrote:
> On Tue, Jun 16, 2026 at 06:42:03PM +0200, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> >
> > In randconfig build testing using clang-22, I came across two
> > sets of extra symbols in the ring buffer code that may get
> > inserted by the compiler:
> >
> > Unexpected symbols in kernel/trace/simple_ring_buffer.o:
> > U memset
> >
> > Unexpected symbols in kernel/trace/simple_ring_buffer.o:
> > U llvm_gcda_emit_arcs
> > U llvm_gcda_emit_function
> > U llvm_gcda_end_file
> > U llvm_gcda_start_file
> > U llvm_gcda_summary_info
> > U llvm_gcov_init
> >
> > Add all of these to the allowlist.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> > kernel/trace/Makefile | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
> > index f934ff586bd4..aa8564fb8ff4 100644
> > --- a/kernel/trace/Makefile
> > +++ b/kernel/trace/Makefile
> > @@ -146,6 +146,7 @@ KASAN_SANITIZE_undefsyms_base.o := y
>
> Would "GCOV_PROFILE_undefsyms_base.o := y" work?
Arnd?
-- Steve
>
> >
> > UNDEFINED_ALLOWLIST = __asan __gcov __kasan __kcsan __hwasan __sancov __sanitizer __tsan __ubsan __msan \
> > __aeabi_unwind_cpp __s390_indirect_jump __x86_indirect_thunk simple_ring_buffer \
> > + memset llvm_gcda llvm_gcov \
> > $(shell $(NM) -u $(obj)/undefsyms_base.o 2>/dev/null | awk '{print $$2}')
> >
> > quiet_cmd_check_undefined = NM $<
> > --
> > 2.39.5
> >
^ permalink raw reply
* Re: [PATCH v2 3/4] tracing/remotes: Add REMOTE_EVENT_CUSTOM_PRINTK() helper
From: Steven Rostedt @ 2026-07-14 22:40 UTC (permalink / raw)
To: Vincent Donnefort
Cc: maz, oliver.upton, joey.gouly, suzuki.poulose, yuzenghui,
catalin.marinas, will, linux-arm-kernel, kvmarm, kernel-team,
qperret, tabba, linux-trace-kernel
In-Reply-To: <20260708075435.47419-4-vdonnefort@google.com>
On Wed, 8 Jul 2026 08:54:34 +0100
Vincent Donnefort <vdonnefort@google.com> wrote:
> The current REMOTE_EVENT() takes as a __printk argument a string format
> and a list of arguments, such as RE_STRUCT("foo=%d bar=%d", foo, bar).
> Add a REMOTE_EVENT_CUSTOM_PRINTK() where the __printk argument can be a
> function. This intends to support the creation of a "printk" event for
> the arm64 nVHE/pKVM hypervisor with a dynamic prototype and by extension
> a dynamic print format.
>
> Reviewed-by: Fuad Tabba <tabba@google.com>
> Tested-by: Fuad Tabba <tabba@google.com>
> Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
I'm assuming this goes in via the arm64 or KVM trees?
Acked-by: Steven Rostedt <rostedt@goodmis.org>
-- Steve
^ permalink raw reply
* [PATCH v7 00/10] tracing: wprobe: x86: Add wprobe for watchpoint
From: Masami Hiramatsu (Google) @ 2026-07-15 1:43 UTC (permalink / raw)
To: Steven Rostedt, Peter Zijlstra, Ingo Molnar, x86
Cc: Jinchao Wang, Mathieu Desnoyers, Masami Hiramatsu,
Thomas Gleixner, Borislav Petkov, Dave Hansen, H . Peter Anvin,
Alexander Shishkin, Ian Rogers, linux-kernel, linux-trace-kernel,
linux-doc, linux-perf-users
Hi,
Here is the 7th version of the 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.
The previous version is here:
https://lore.kernel.org/all/176960933881.182525.11984731584313026309.stgit@devnote2/
This version is rebased on top of probes/for-next branch, fold IS_ERR_PCPU
fix and fix some checkpatch.pl related issue (not all, some of them seems
wrong warnings).
Other updates:
- Fix a race on wprobe handler using bp->attr.bp_addr instead of tw->addr.
- Fix modify_wide_hw_breakpoint_local() to update bp->attr.bp_addr.
- Return -EOPNOTSUPP instead of -ENOSYS. (that is not a syscall function)
- Update test script to use dentry_kill instead of __dentry_kill.
To support arm64, we need to avoid major pagefault on single-stepping
issue. I think we can solve it with checking whether the address is the
kernel (which must not cause a major page fault), or not.
Usage
-----
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[+OFFSET] [if FILTER]
clear_wprobe:WPROBE_EVENT[:FIELD[+OFFSET]] [if FILTER]
set_wprobe sets WPROBE_EVENT's watch address on FIELD[+OFFSET].
clear_wprobe clears WPROBE_EVENT's watch address if it is set to
FIELD[+OFFSET]. If FIELD is omitted, forcibly clear the watch address
when trigger event is hit.
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: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;
# 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
# | | | ||||| | |
sh-113 [004] ..... 6.467444: truncate: (do_truncate+0x4/0x120) dentry=0xffff8880044f0fd8
sh-113 [004] ..Zff 6.468534: watch: (lookup_fast+0xaa/0x150) address=0xffff8880044f0fd8 value=0x200080
sh-113 [004] ..Zff 6.468542: watch: (step_into+0x82/0x360) address=0xffff8880044f0fd8 value=0x200080
sh-113 [004] ..Zff 6.468547: watch: (step_into+0x9f/0x360) address=0xffff8880044f0fd8 value=0x200080
sh-113 [004] ..Zff 6.468553: watch: (path_openat+0xb3a/0xe70) address=0xffff8880044f0fd8 value=0x200080
sh-113 [004] ..Zff 6.468557: watch: (path_openat+0xb9a/0xe70) address=0xffff8880044f0fd8 value=0x200080
sh-113 [004] ..... 6.468563: truncate: (do_truncate+0x4/0x120) dentry=0xffff8880044f0fd8
sh-113 [004] ...1. 6.469826: dentry_kill: (__dentry_kill+0x0/0x220) dentry=0xffff8880044f0ea0
sh-113 [004] ...1. 6.469859: dentry_kill: (__dentry_kill+0x0/0x220) dentry=0xffff8880044f0d68
rm-118 [001] ..Zff 6.472360: watch: (lookup_fast+0xaa/0x150) address=0xffff8880044f0fd8 value=0x200080
rm-118 [001] ..Zff 6.472366: watch: (step_into+0x82/0x360) address=0xffff8880044f0fd8 value=0x200080
rm-118 [001] ..Zff 6.472370: watch: (step_into+0x9f/0x360) address=0xffff8880044f0fd8 value=0x200080
rm-118 [001] ..Zff 6.472386: watch: (lookup_fast+0xaa/0x150) address=0xffff8880044f0fd8 value=0x200080
rm-118 [001] ..Zff 6.472390: watch: (step_into+0x82/0x360) address=0xffff8880044f0fd8 value=0x200080
rm-118 [001] ..Zff 6.472394: watch: (step_into+0x9f/0x360) address=0xffff8880044f0fd8 value=0x200080
rm-118 [001] ..Zff 6.472415: watch: (lookup_one_qstr_excl+0x2c/0x150) address=0xffff8880044f0fd8 value=0x200080
rm-118 [001] ..Zff 6.472419: watch: (lookup_one_qstr_excl+0xd5/0x150) address=0xffff8880044f0fd8 value=0x200080
rm-118 [001] ..Zff 6.472424: watch: (may_delete+0x18/0x200) address=0xffff8880044f0fd8 value=0x200080
rm-118 [001] ..Zff 6.472428: watch: (may_delete+0x194/0x200) address=0xffff8880044f0fd8 value=0x200080
rm-118 [001] ..Zff 6.472446: watch: (vfs_unlink+0x63/0x1c0) address=0xffff8880044f0fd8 value=0x200080
rm-118 [001] d.Z.. 6.472528: watch: (dont_mount+0x19/0x30) address=0xffff8880044f0fd8 value=0x200180
rm-118 [001] ..Zff 6.472533: watch: (vfs_unlink+0x11a/0x1c0) address=0xffff8880044f0fd8 value=0x200180
rm-118 [001] ..Zff 6.472538: watch: (vfs_unlink+0x12e/0x1c0) address=0xffff8880044f0fd8 value=0x200180
rm-118 [001] d.Z1. 6.472543: watch: (d_delete+0x61/0xa0) address=0xffff8880044f0fd8 value=0x200080
rm-118 [001] d.Z1. 6.472547: watch: (dentry_unlink_inode+0x14/0x110) address=0xffff8880044f0fd8 value=0x200080
rm-118 [001] d.Z1. 6.472551: watch: (dentry_unlink_inode+0x1e/0x110) address=0xffff8880044f0fd8 value=0x80
rm-118 [001] d.Z.. 6.472563: watch: (fast_dput+0x8d/0x120) address=0xffff8880044f0fd8 value=0x80
rm-118 [001] ...1. 6.472567: dentry_kill: (__dentry_kill+0x0/0x220) dentry=0xffff8880044f0fd8
sh-113 [004] ...2. 6.473049: dentry_kill: (__dentry_kill+0x0/0x220) dentry=0xffff888006e383a8
Thank you,
---
Jinchao Wang (2):
x86/hw_breakpoint: Unify breakpoint install/uninstall
x86/hw_breakpoint: Add arch_reinstall_hw_breakpoint
Masami Hiramatsu (Google) (8):
tracing: wprobe: Add watchpoint probe event based on hardware breakpoint
x86: hw_breakpoint: Add a kconfig to clarify when a breakpoint fires
selftests: tracing: Add a basic testcase for wprobe
selftests: tracing: Add syntax testcase for wprobe
tracing: wprobe: Use a new seq_print_ip_sym_offset() wrapper
HWBP: Add modify_wide_hw_breakpoint_local() API
tracing: wprobe: Add wprobe event trigger
selftests: ftrace: Add wprobe trigger testcase
Documentation/trace/index.rst | 1
Documentation/trace/wprobetrace.rst | 158 +++
arch/Kconfig | 20
arch/x86/Kconfig | 2
arch/x86/include/asm/hw_breakpoint.h | 8
arch/x86/kernel/hw_breakpoint.c | 148 ++-
include/linux/hw_breakpoint.h | 6
include/linux/trace_events.h | 3
kernel/events/hw_breakpoint.c | 39 +
kernel/trace/Kconfig | 24
kernel/trace/Makefile | 1
kernel/trace/trace.c | 9
kernel/trace/trace.h | 5
kernel/trace/trace_probe.c | 22
kernel/trace/trace_probe.h | 8
kernel/trace/trace_wprobe.c | 1108 ++++++++++++++++++++
tools/testing/selftests/ftrace/config | 2
.../ftrace/test.d/dynevent/add_remove_wprobe.tc | 68 +
.../test.d/dynevent/wprobes_syntax_errors.tc | 20
.../ftrace/test.d/trigger/trigger-wprobe.tc | 48 +
20 files changed, 1635 insertions(+), 65 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
create mode 100644 tools/testing/selftests/ftrace/test.d/trigger/trigger-wprobe.tc
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* [PATCH v7 01/10] tracing: wprobe: Add watchpoint probe event based on hardware breakpoint
From: Masami Hiramatsu (Google) @ 2026-07-15 1:44 UTC (permalink / raw)
To: Steven Rostedt, Peter Zijlstra, Ingo Molnar, x86
Cc: Jinchao Wang, Mathieu Desnoyers, Masami Hiramatsu,
Thomas Gleixner, Borislav Petkov, Dave Hansen, H . Peter Anvin,
Alexander Shishkin, Ian Rogers, linux-kernel, linux-trace-kernel,
linux-doc, linux-perf-users
In-Reply-To: <178407983818.95826.12714571928538799781.stgit@devnote2>
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Add a new probe event for the hardware breakpoint called wprobe-event.
This wprobe allows user to trace (watch) the memory access at the
specified memory address.
The new syntax is;
w[:[GROUP/]EVENT] [r|w|rw]@[ADDR|SYM][:SIZE] [FETCH_ARGs]
User also can use $addr to fetch the accessed address. But no other
variables are supported. To record updated value, use '+0($addr)'.
For example, tracing updates of the jiffies;
/sys/kernel/tracing # echo 'w:my_jiffies w@jiffies' >> dynamic_events
/sys/kernel/tracing # cat dynamic_events
w:wprobes/my_jiffies w@jiffies:4
/sys/kernel/tracing # echo 1 > events/wprobes/my_jiffies/enable
/sys/kernel/tracing # head -n 20 trace | tail -n 5
# TASK-PID CPU# ||||| TIMESTAMP FUNCTION
# | | | ||||| | |
<idle>-0 [000] d.Z1. 206.547317: my_jiffies: (tick_do_update_jiffies64+0xbe/0x130)
<idle>-0 [000] d.Z1. 206.548341: my_jiffies: (tick_do_update_jiffies64+0xbe/0x130)
<idle>-0 [000] d.Z1. 206.549346: my_jiffies: (tick_do_update_jiffies64+0xbe/0x130)
Link: https://lore.kernel.org/all/175859021100.374439.8723137923620348816.stgit@devnote2/
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Changes in v7:
- Include IS_ERR_PCPU fix.
- use seq_print_ip_sym_offset().
- fix checkpatch warning on DEFINE_FREE()
- Use bp->attr.bp_addr instead of tw->addr because it can be updated from another CPU.
---
Documentation/trace/index.rst | 1
Documentation/trace/wprobetrace.rst | 69 +++
include/linux/trace_events.h | 2
kernel/trace/Kconfig | 13 +
kernel/trace/Makefile | 1
kernel/trace/trace.c | 9
kernel/trace/trace.h | 5
kernel/trace/trace_probe.c | 22 +
kernel/trace/trace_probe.h | 8
kernel/trace/trace_wprobe.c | 692 +++++++++++++++++++++++++++++++++++
10 files changed, 819 insertions(+), 3 deletions(-)
create mode 100644 Documentation/trace/wprobetrace.rst
create mode 100644 kernel/trace/trace_wprobe.c
diff --git a/Documentation/trace/index.rst b/Documentation/trace/index.rst
index 5d9bf4694d5d..2f04f32001ed 100644
--- a/Documentation/trace/index.rst
+++ b/Documentation/trace/index.rst
@@ -36,6 +36,7 @@ the Linux kernel.
kprobes
kprobetrace
fprobetrace
+ wprobetrace
eprobetrace
fprobe
ring-buffer-design
diff --git a/Documentation/trace/wprobetrace.rst b/Documentation/trace/wprobetrace.rst
new file mode 100644
index 000000000000..025b4c39b809
--- /dev/null
+++ b/Documentation/trace/wprobetrace.rst
@@ -0,0 +1,69 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=======================================
+Watchpoint probe (wprobe) Event Tracing
+=======================================
+
+.. Author: Masami Hiramatsu <mhiramat@kernel.org>
+
+Overview
+--------
+
+Wprobe event is a dynamic event based on the hardware breakpoint, which is
+similar to other probe events, but it is for watching data access. It allows
+you to trace which code accesses a specified data.
+
+As same as other dynamic events, wprobe events are defined via
+`dynamic_events` interface file on tracefs.
+
+Synopsis of wprobe-events
+-------------------------
+::
+
+ 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.
+ Default is rw if omitted.
+ 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
+ # 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 `tick_do_update_jiffies64()`.
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 308c76b57d13..d1e5ab71d928 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -328,6 +328,7 @@ enum {
TRACE_EVENT_FL_UPROBE_BIT,
TRACE_EVENT_FL_EPROBE_BIT,
TRACE_EVENT_FL_FPROBE_BIT,
+ TRACE_EVENT_FL_WPROBE_BIT,
TRACE_EVENT_FL_CUSTOM_BIT,
TRACE_EVENT_FL_TEST_STR_BIT,
};
@@ -358,6 +359,7 @@ enum {
TRACE_EVENT_FL_UPROBE = (1 << TRACE_EVENT_FL_UPROBE_BIT),
TRACE_EVENT_FL_EPROBE = (1 << TRACE_EVENT_FL_EPROBE_BIT),
TRACE_EVENT_FL_FPROBE = (1 << TRACE_EVENT_FL_FPROBE_BIT),
+ TRACE_EVENT_FL_WPROBE = (1 << TRACE_EVENT_FL_WPROBE_BIT),
TRACE_EVENT_FL_CUSTOM = (1 << TRACE_EVENT_FL_CUSTOM_BIT),
TRACE_EVENT_FL_TEST_STR = (1 << TRACE_EVENT_FL_TEST_STR_BIT),
};
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 0ab5916575a9..b58c2565024f 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -862,6 +862,19 @@ 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
+ 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 accessed memory address and values.
+
config BPF_EVENTS
depends on BPF_SYSCALL
depends on (KPROBE_EVENTS || UPROBE_EVENTS) && PERF_EVENTS
diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index f934ff586bd4..141c8323de20 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -126,6 +126,7 @@ obj-$(CONFIG_FTRACE_RECORD_RECURSION) += trace_recursion_record.o
obj-$(CONFIG_FPROBE) += fprobe.o
obj-$(CONFIG_RETHOOK) += rethook.o
obj-$(CONFIG_FPROBE_EVENTS) += trace_fprobe.o
+obj-$(CONFIG_WPROBE_EVENTS) += trace_wprobe.o
obj-$(CONFIG_TRACEPOINT_BENCHMARK) += trace_benchmark.o
obj-$(CONFIG_RV) += rv/
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index c9e182d40059..1bc27c0ad029 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -4294,8 +4294,12 @@ static const char readme_msg[] =
" uprobe_events\t\t- Create/append/remove/show the userspace dynamic events\n"
"\t\t\t Write into this file to define/undefine new trace events.\n"
#endif
+#ifdef CONFIG_WPROBE_EVENTS
+ " wprobe_events\t\t- Create/append/remove/show the hardware breakpoint dynamic events\n"
+ "\t\t\t Write into this file to define/undefine new trace events.\n"
+#endif
#if defined(CONFIG_KPROBE_EVENTS) || defined(CONFIG_UPROBE_EVENTS) || \
- defined(CONFIG_FPROBE_EVENTS)
+ defined(CONFIG_FPROBE_EVENTS) || defined(CONFIG_WPROBE_EVENTS)
"\t accepts: event-definitions (one definition per line)\n"
#if defined(CONFIG_KPROBE_EVENTS) || defined(CONFIG_UPROBE_EVENTS)
"\t Format: p[:[<group>/][<event>]] <place> [<args>]\n"
@@ -4305,6 +4309,9 @@ static const char readme_msg[] =
"\t f[:[<group>/][<event>]] <func-name>[%return] [<args>]\n"
"\t t[:[<group>/][<event>]] <tracepoint> [<args>]\n"
#endif
+#ifdef CONFIG_WPROBE_EVENTS
+ "\t w[:[<group>/][<event>]] [r|w|rw]@<addr>[:<len>]\n"
+#endif
#ifdef CONFIG_HIST_TRIGGERS
"\t s:[synthetic/]<event> <field> [<field>]\n"
#endif
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 80fe152af1dd..2f07c5c4ffc8 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -179,6 +179,11 @@ struct fexit_trace_entry_head {
unsigned long ret_ip;
};
+struct wprobe_trace_entry_head {
+ struct trace_entry ent;
+ unsigned long ip;
+};
+
#define TRACE_BUF_SIZE 1024
struct trace_array;
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 18c212122344..2600d9699bd8 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -1344,6 +1344,24 @@ static int parse_probe_vars(char *orig_arg, const struct fetch_type *t,
return 0;
}
+ /* wprobe only support "$addr" and "$value" variable */
+ if (ctx->flags & TPARG_FL_WPROBE) {
+ if (!strcmp(arg, "addr")) {
+ code->op = FETCH_OP_BADDR;
+ return 0;
+ }
+ if (!strcmp(arg, "value")) {
+ code->op = FETCH_OP_BADDR;
+ code++;
+ code->op = FETCH_OP_DEREF;
+ code->offset = 0;
+ *pcode = code;
+ return 0;
+ }
+ err = TP_ERR_BAD_VAR;
+ goto inval;
+ }
+
if (str_has_prefix(arg, "retval")) {
if (!(ctx->flags & TPARG_FL_RETURN)) {
err = TP_ERR_RETVAL_ON_PROBE;
@@ -1491,8 +1509,9 @@ parse_probe_arg(char *arg, const struct fetch_type *type,
ret = parse_probe_vars(arg, type, pcode, end, ctx);
break;
+#ifdef CONFIG_HAVE_FUNCTION_ARG_ACCESS_API
case '%': /* named register */
- if (ctx->flags & (TPARG_FL_TEVENT | TPARG_FL_FPROBE)) {
+ if (ctx->flags & (TPARG_FL_TEVENT | TPARG_FL_FPROBE | TPARG_FL_WPROBE)) {
/* eprobe and fprobe do not handle registers */
trace_probe_log_err(ctx->offset, BAD_VAR);
break;
@@ -1505,6 +1524,7 @@ parse_probe_arg(char *arg, const struct fetch_type *type,
} else
trace_probe_log_err(ctx->offset, BAD_REG_NAME);
break;
+#endif
case '@': /* memory, file-offset or symbol */
if (isdigit(arg[1])) {
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index e6268a8dc378..64c5fe9bdfc9 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -90,6 +90,7 @@ typedef int (*print_type_func_t)(struct trace_seq *, void *, void *);
FETCH_OP(STACK, param), /* Stack: .param = index */ \
FETCH_OP(STACKP, none), /* Stack pointer */ \
FETCH_OP(RETVAL, none), /* Return value */ \
+ FETCH_OP(BADDR, none), /* Break address */ \
FETCH_OP(IMM, imm), /* Immediate: .immediate */ \
FETCH_OP(COMM, none), /* Current comm */ \
FETCH_OP(CURRENT, none), /* Current task_struct address */\
@@ -419,6 +420,7 @@ static inline int traceprobe_get_entry_data_size(struct trace_probe *tp)
#define TPARG_FL_USER BIT(4)
#define TPARG_FL_FPROBE BIT(5)
#define TPARG_FL_TPOINT BIT(6)
+#define TPARG_FL_WPROBE BIT(7)
#define TPARG_FL_LOC_MASK GENMASK(4, 0)
static inline bool tparg_is_function_entry(unsigned int flags)
@@ -600,7 +602,11 @@ extern int traceprobe_define_arg_fields(struct trace_event_call *event_call,
C(TYPECAST_SYM_OFFSET, "@SYM+/-OFFSET with typecast needs parentheses"), \
C(TYPECAST_NOT_ALIGNED, "Typecast field option is not byte-aligned"), \
C(TYPECAST_BAD_ARROW, "Typecast field option does not support -> operator"), \
- C(NOSUP_PERCPU, "Per-cpu variable access is only for kernel probes"),
+ C(NOSUP_PERCPU, "Per-cpu variable access is only for kernel probes"), \
+ C(BAD_ACCESS_FMT, "Access memory address requires @"), \
+ C(BAD_ACCESS_TYPE, "Bad memory access type"), \
+ C(BAD_ACCESS_LEN, "This memory access length is not supported"), \
+ C(BAD_ACCESS_ADDR, "Invalid access memory address"),
#undef C
#define C(a, b) TP_ERR_##a
diff --git a/kernel/trace/trace_wprobe.c b/kernel/trace/trace_wprobe.c
new file mode 100644
index 000000000000..b52f3eac719f
--- /dev/null
+++ b/kernel/trace/trace_wprobe.c
@@ -0,0 +1,692 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Hardware-breakpoint-based tracing events
+ *
+ * Copyright (C) 2023, Masami Hiramatsu <mhiramat@kernel.org>
+ */
+#define pr_fmt(fmt) "trace_wprobe: " fmt
+
+#include <linux/hw_breakpoint.h>
+#include <linux/kallsyms.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/perf_event.h>
+#include <linux/rculist.h>
+#include <linux/security.h>
+#include <linux/tracepoint.h>
+#include <linux/uaccess.h>
+
+#include <asm/ptrace.h>
+
+#include "trace_dynevent.h"
+#include "trace_probe.h"
+#include "trace_probe_kernel.h"
+#include "trace_probe_tmpl.h"
+
+#define WPROBE_EVENT_SYSTEM "wprobes"
+
+static int trace_wprobe_create(const char *raw_command);
+static int trace_wprobe_show(struct seq_file *m, struct dyn_event *ev);
+static int trace_wprobe_release(struct dyn_event *ev);
+static bool trace_wprobe_is_busy(struct dyn_event *ev);
+static bool trace_wprobe_match(const char *system, const char *event,
+ int argc, const char **argv, struct dyn_event *ev);
+
+static struct dyn_event_operations trace_wprobe_ops = {
+ .create = trace_wprobe_create,
+ .show = trace_wprobe_show,
+ .is_busy = trace_wprobe_is_busy,
+ .free = trace_wprobe_release,
+ .match = trace_wprobe_match,
+};
+
+struct trace_wprobe {
+ struct dyn_event devent;
+ struct perf_event * __percpu *bp_event;
+ unsigned long addr;
+ int len;
+ int type;
+ const char *symbol;
+ struct trace_probe tp;
+};
+
+static bool is_trace_wprobe(struct dyn_event *ev)
+{
+ return ev->ops == &trace_wprobe_ops;
+}
+
+static struct trace_wprobe *to_trace_wprobe(struct dyn_event *ev)
+{
+ return container_of(ev, struct trace_wprobe, devent);
+}
+
+#define for_each_trace_wprobe(pos, dpos) \
+ for_each_dyn_event(dpos) \
+ if (is_trace_wprobe(dpos) && (pos = to_trace_wprobe(dpos)))
+
+static bool trace_wprobe_is_busy(struct dyn_event *ev)
+{
+ struct trace_wprobe *tw = to_trace_wprobe(ev);
+
+ return trace_probe_is_enabled(&tw->tp);
+}
+
+static bool trace_wprobe_match(const char *system, const char *event,
+ int argc, const char **argv, struct dyn_event *ev)
+{
+ struct trace_wprobe *tw = to_trace_wprobe(ev);
+
+ if (event[0] != '\0' && strcmp(trace_probe_name(&tw->tp), event))
+ return false;
+
+ if (system && strcmp(trace_probe_group_name(&tw->tp), system))
+ return false;
+
+ /* TODO: match arguments */
+ return true;
+}
+
+/*
+ * Note that we don't verify the fetch_insn code, since it does not come
+ * from user space.
+ */
+static int
+process_fetch_insn(struct fetch_insn *code, void *rec, void *edata,
+ void *dest, void *base)
+{
+ void *baddr = rec;
+ unsigned long val;
+ int ret;
+
+retry:
+ /* 1st stage: get value from context */
+ switch (code->op) {
+ case FETCH_OP_BADDR:
+ val = (unsigned long)baddr;
+ break;
+ case FETCH_NOP_SYMBOL: /* Ignore a place holder */
+ code++;
+ goto retry;
+ default:
+ ret = process_common_fetch_insn(code, &val);
+ if (ret < 0)
+ return ret;
+ }
+ code++;
+
+ return process_fetch_insn_bottom(code, val, dest, base);
+}
+NOKPROBE_SYMBOL(process_fetch_insn)
+
+static void wprobe_trace_handler(struct trace_wprobe *tw,
+ unsigned long addr,
+ struct pt_regs *regs,
+ struct trace_event_file *trace_file)
+{
+ struct wprobe_trace_entry_head *entry;
+ struct trace_event_call *call = trace_probe_event_call(&tw->tp);
+ struct trace_event_buffer fbuffer;
+ int dsize;
+
+ if (WARN_ON_ONCE(call != trace_file->event_call))
+ return;
+
+ if (trace_trigger_soft_disabled(trace_file))
+ return;
+
+ if (tw->addr != addr)
+ return;
+
+ dsize = __get_data_size(&tw->tp, (void *)addr, NULL);
+
+ entry = trace_event_buffer_reserve(&fbuffer, trace_file,
+ sizeof(*entry) + tw->tp.size + dsize);
+ if (!entry)
+ return;
+
+ entry->ip = instruction_pointer(regs);
+ store_trace_args(&entry[1], &tw->tp, (void *)addr, NULL, sizeof(*entry), dsize);
+
+ fbuffer.regs = regs;
+ trace_event_buffer_commit(&fbuffer);
+}
+
+static void wprobe_perf_handler(struct perf_event *bp,
+ struct perf_sample_data *data,
+ struct pt_regs *regs)
+{
+ struct trace_wprobe *tw = bp->overflow_handler_context;
+ struct event_file_link *link;
+ unsigned long addr = bp->attr.bp_addr;
+
+ trace_probe_for_each_link_rcu(link, &tw->tp)
+ wprobe_trace_handler(tw, addr, regs, link->file);
+}
+
+static int __register_trace_wprobe(struct trace_wprobe *tw)
+{
+ struct perf_event_attr attr;
+
+ if (tw->bp_event)
+ return -EINVAL;
+
+ hw_breakpoint_init(&attr);
+ attr.bp_addr = tw->addr;
+ attr.bp_len = tw->len;
+ attr.bp_type = tw->type;
+
+ tw->bp_event = register_wide_hw_breakpoint(&attr, wprobe_perf_handler, tw);
+ if (IS_ERR_PCPU(tw->bp_event)) {
+ int ret = PTR_ERR_PCPU(tw->bp_event);
+
+ tw->bp_event = NULL;
+ return ret;
+ }
+
+ return 0;
+}
+
+static void __unregister_trace_wprobe(struct trace_wprobe *tw)
+{
+ if (tw->bp_event) {
+ unregister_wide_hw_breakpoint(tw->bp_event);
+ tw->bp_event = NULL;
+ }
+}
+
+static void free_trace_wprobe(struct trace_wprobe *tw)
+{
+ if (tw) {
+ trace_probe_cleanup(&tw->tp);
+ kfree(tw->symbol);
+ kfree(tw);
+ }
+}
+DEFINE_FREE(free_trace_wprobe, struct trace_wprobe *,
+ if (!IS_ERR_OR_NULL(_T))
+ free_trace_wprobe(_T))
+
+
+static struct trace_wprobe *alloc_trace_wprobe(const char *group,
+ const char *event,
+ const char *symbol,
+ unsigned long addr,
+ int len, int type, int nargs)
+{
+ struct trace_wprobe *tw __free(free_trace_wprobe) = NULL;
+ int ret;
+
+ tw = kzalloc(struct_size(tw, tp.args, nargs), GFP_KERNEL);
+ if (!tw)
+ return ERR_PTR(-ENOMEM);
+
+ if (symbol) {
+ tw->symbol = kstrdup(symbol, GFP_KERNEL);
+ if (!tw->symbol)
+ return ERR_PTR(-ENOMEM);
+ }
+ tw->addr = addr;
+ tw->len = len;
+ tw->type = type;
+
+ ret = trace_probe_init(&tw->tp, event, group, false, nargs);
+ if (ret < 0)
+ return ERR_PTR(ret);
+
+ dyn_event_init(&tw->devent, &trace_wprobe_ops);
+ return_ptr(tw);
+}
+
+static struct trace_wprobe *find_trace_wprobe(const char *event,
+ const char *group)
+{
+ struct dyn_event *pos;
+ struct trace_wprobe *tw;
+
+ for_each_trace_wprobe(tw, pos)
+ if (strcmp(trace_probe_name(&tw->tp), event) == 0 &&
+ strcmp(trace_probe_group_name(&tw->tp), group) == 0)
+ return tw;
+ return NULL;
+}
+
+static enum print_line_t
+print_wprobe_event(struct trace_iterator *iter, int flags,
+ struct trace_event *event)
+{
+ struct wprobe_trace_entry_head *field;
+ struct trace_seq *s = &iter->seq;
+ struct trace_probe *tp;
+
+ field = (struct wprobe_trace_entry_head *)iter->ent;
+ tp = trace_probe_primary_from_call(
+ container_of(event, struct trace_event_call, event));
+ if (WARN_ON_ONCE(!tp))
+ goto out;
+
+ trace_seq_printf(s, "%s: (", trace_probe_name(tp));
+
+ if (!seq_print_ip_sym_offset(s, field->ip, flags))
+ goto out;
+
+ trace_seq_putc(s, ')');
+
+ if (trace_probe_print_args(s, tp->args, tp->nr_args,
+ (u8 *)&field[1], field) < 0)
+ goto out;
+
+ trace_seq_putc(s, '\n');
+out:
+ return trace_handle_return(s);
+}
+
+static int wprobe_event_define_fields(struct trace_event_call *event_call)
+{
+ int ret;
+ struct wprobe_trace_entry_head field;
+ struct trace_probe *tp;
+
+ tp = trace_probe_primary_from_call(event_call);
+ if (WARN_ON_ONCE(!tp))
+ return -ENOENT;
+
+ DEFINE_FIELD(unsigned long, ip, FIELD_STRING_IP, 0);
+
+ return traceprobe_define_arg_fields(event_call, sizeof(field), tp);
+}
+
+static struct trace_event_functions wprobe_funcs = {
+ .trace = print_wprobe_event
+};
+
+static struct trace_event_fields wprobe_fields_array[] = {
+ { .type = TRACE_FUNCTION_TYPE,
+ .define_fields = wprobe_event_define_fields },
+ {}
+};
+
+static int wprobe_register(struct trace_event_call *event,
+ enum trace_reg type, void *data);
+
+static inline void init_trace_event_call(struct trace_wprobe *tw)
+{
+ struct trace_event_call *call = trace_probe_event_call(&tw->tp);
+
+ call->event.funcs = &wprobe_funcs;
+ call->class->fields_array = wprobe_fields_array;
+ call->flags = TRACE_EVENT_FL_WPROBE;
+ call->class->reg = wprobe_register;
+}
+
+static int register_wprobe_event(struct trace_wprobe *tw)
+{
+ init_trace_event_call(tw);
+ return trace_probe_register_event_call(&tw->tp);
+}
+
+static int register_trace_wprobe_event(struct trace_wprobe *tw)
+{
+ struct trace_wprobe *old_tb;
+ int ret;
+
+ guard(mutex)(&event_mutex);
+
+ old_tb = find_trace_wprobe(trace_probe_name(&tw->tp),
+ trace_probe_group_name(&tw->tp));
+ if (old_tb)
+ return -EBUSY;
+
+ ret = register_wprobe_event(tw);
+ if (ret)
+ return ret;
+
+ dyn_event_add(&tw->devent, trace_probe_event_call(&tw->tp));
+ return 0;
+}
+static int unregister_wprobe_event(struct trace_wprobe *tw)
+{
+ return trace_probe_unregister_event_call(&tw->tp);
+}
+
+static int unregister_trace_wprobe(struct trace_wprobe *tw)
+{
+ if (trace_probe_has_sibling(&tw->tp))
+ goto unreg;
+
+ if (trace_probe_is_enabled(&tw->tp))
+ return -EBUSY;
+
+ if (trace_event_dyn_busy(trace_probe_event_call(&tw->tp)))
+ return -EBUSY;
+
+ if (unregister_wprobe_event(tw))
+ return -EBUSY;
+
+unreg:
+ __unregister_trace_wprobe(tw);
+ dyn_event_remove(&tw->devent);
+ trace_probe_unlink(&tw->tp);
+
+ return 0;
+}
+
+static int enable_trace_wprobe(struct trace_event_call *call,
+ struct trace_event_file *file)
+{
+ struct trace_probe *tp;
+ struct trace_wprobe *tw;
+ bool enabled;
+ int ret = 0;
+
+ tp = trace_probe_primary_from_call(call);
+ if (WARN_ON_ONCE(!tp))
+ return -ENODEV;
+ enabled = trace_probe_is_enabled(tp);
+
+ if (file) {
+ ret = trace_probe_add_file(tp, file);
+ if (ret)
+ return ret;
+ } else {
+ trace_probe_set_flag(tp, TP_FLAG_PROFILE);
+ }
+
+ if (!enabled) {
+ list_for_each_entry(tw, trace_probe_probe_list(tp), tp.list) {
+ ret = __register_trace_wprobe(tw);
+ if (ret < 0) {
+ /* TODO: rollback */
+ return ret;
+ }
+ }
+ }
+
+ return 0;
+}
+
+static int disable_trace_wprobe(struct trace_event_call *call,
+ struct trace_event_file *file)
+{
+ struct trace_wprobe *tw;
+ struct trace_probe *tp;
+
+ tp = trace_probe_primary_from_call(call);
+ if (WARN_ON_ONCE(!tp))
+ return -ENODEV;
+
+ if (file) {
+ if (!trace_probe_get_file_link(tp, file))
+ return -ENOENT;
+ if (!trace_probe_has_single_file(tp))
+ goto out;
+ trace_probe_clear_flag(tp, TP_FLAG_TRACE);
+ } else {
+ trace_probe_clear_flag(tp, TP_FLAG_PROFILE);
+ }
+
+ if (!trace_probe_is_enabled(tp)) {
+ list_for_each_entry(tw, trace_probe_probe_list(tp), tp.list) {
+ __unregister_trace_wprobe(tw);
+ }
+ }
+
+out:
+ if (file)
+ trace_probe_remove_file(tp, file);
+
+ return 0;
+}
+
+static int wprobe_register(struct trace_event_call *event,
+ enum trace_reg type, void *data)
+{
+ struct trace_event_file *file = data;
+
+ switch (type) {
+ case TRACE_REG_REGISTER:
+ return enable_trace_wprobe(event, file);
+ case TRACE_REG_UNREGISTER:
+ return disable_trace_wprobe(event, file);
+
+#ifdef CONFIG_PERF_EVENTS
+ case TRACE_REG_PERF_REGISTER:
+ return enable_trace_wprobe(event, NULL);
+ case TRACE_REG_PERF_UNREGISTER:
+ return disable_trace_wprobe(event, NULL);
+ case TRACE_REG_PERF_OPEN:
+ case TRACE_REG_PERF_CLOSE:
+ case TRACE_REG_PERF_ADD:
+ case TRACE_REG_PERF_DEL:
+ return 0;
+#endif
+ }
+ return 0;
+}
+
+static int parse_address_spec(const char *spec, unsigned long *addr, int *type,
+ int *len, char **symbol)
+{
+ char *_spec __free(kfree) = NULL;
+ int _len = HW_BREAKPOINT_LEN_4;
+ int _type = HW_BREAKPOINT_RW;
+ unsigned long _addr = 0;
+ char *at, *col;
+
+ _spec = kstrdup(spec, GFP_KERNEL);
+ if (!_spec)
+ return -ENOMEM;
+
+ at = strchr(_spec, '@');
+ col = strchr(_spec, ':');
+
+ if (!at) {
+ trace_probe_log_err(0, BAD_ACCESS_FMT);
+ return -EINVAL;
+ }
+
+ if (at != _spec) {
+ *at = '\0';
+
+ if (strcmp(_spec, "r") == 0)
+ _type = HW_BREAKPOINT_R;
+ else if (strcmp(_spec, "w") == 0)
+ _type = HW_BREAKPOINT_W;
+ else if (strcmp(_spec, "rw") == 0)
+ _type = HW_BREAKPOINT_RW;
+ else {
+ trace_probe_log_err(0, BAD_ACCESS_TYPE);
+ return -EINVAL;
+ }
+ }
+
+ if (col) {
+ *col = '\0';
+ if (kstrtoint(col + 1, 0, &_len)) {
+ trace_probe_log_err(col + 1 - _spec, BAD_ACCESS_LEN);
+ return -EINVAL;
+ }
+
+ switch (_len) {
+ case 1:
+ _len = HW_BREAKPOINT_LEN_1;
+ break;
+ case 2:
+ _len = HW_BREAKPOINT_LEN_2;
+ break;
+ case 4:
+ _len = HW_BREAKPOINT_LEN_4;
+ break;
+ case 8:
+ _len = HW_BREAKPOINT_LEN_8;
+ break;
+ default:
+ trace_probe_log_err(col + 1 - _spec, BAD_ACCESS_LEN);
+ return -EINVAL;
+ }
+ }
+
+ if (kstrtoul(at + 1, 0, &_addr) != 0) {
+ char *off_str = strpbrk(at + 1, "+-");
+ int offset = 0;
+
+ if (off_str) {
+ if (kstrtoint(off_str, 0, &offset) != 0) {
+ trace_probe_log_err(off_str - _spec, BAD_PROBE_ADDR);
+ return -EINVAL;
+ }
+ *off_str = '\0';
+ }
+ _addr = kallsyms_lookup_name(at + 1);
+ if (!_addr) {
+ trace_probe_log_err(at + 1 - _spec, BAD_ACCESS_ADDR);
+ return -ENOENT;
+ }
+ _addr += offset;
+ *symbol = kstrdup(at + 1, GFP_KERNEL);
+ if (!*symbol)
+ return -ENOMEM;
+ }
+
+ *addr = _addr;
+ *type = _type;
+ *len = _len;
+ return 0;
+}
+
+static int __trace_wprobe_create(int argc, const char *argv[])
+{
+ /*
+ * Argument syntax:
+ * b[:[GRP/][EVENT]] SPEC
+ *
+ * SPEC:
+ * [r|w|rw]@[ADDR|SYMBOL[+OFFS]][:LEN]
+ */
+ struct traceprobe_parse_context *ctx __free(traceprobe_parse_context) = NULL;
+ struct trace_wprobe *tw __free(free_trace_wprobe) = NULL;
+ const char *event = NULL, *group = WPROBE_EVENT_SYSTEM;
+ const char *tplog __free(trace_probe_log_clear) = NULL;
+ char *symbol = NULL;
+ unsigned long addr;
+ int len, type, i;
+ int ret = 0;
+
+ if (argv[0][0] != 'w')
+ return -ECANCELED;
+
+ if (argc < 2)
+ return -EINVAL;
+
+ tplog = trace_probe_log_init("wprobe", argc, argv);
+
+ if (argv[0][1] != '\0') {
+ if (argv[0][1] != ':') {
+ trace_probe_log_set_index(0);
+ trace_probe_log_err(1, BAD_MAXACT_TYPE);
+ /* Invalid format */
+ return -EINVAL;
+ }
+ event = &argv[0][2];
+ }
+
+ trace_probe_log_set_index(1);
+ ret = parse_address_spec(argv[1], &addr, &type, &len, &symbol);
+ if (ret < 0)
+ return ret;
+
+ if (!event)
+ event = symbol ? symbol : "wprobe";
+
+ argc -= 2; argv += 2;
+ tw = alloc_trace_wprobe(group, event, symbol, addr, len, type, argc);
+ if (IS_ERR(tw))
+ return PTR_ERR(tw);
+
+ ctx = kzalloc_obj(*ctx);
+ if (!ctx)
+ return -ENOMEM;
+
+ ctx->flags = TPARG_FL_KERNEL | TPARG_FL_WPROBE;
+
+ /* parse arguments */
+ for (i = 0; i < argc; i++) {
+ trace_probe_log_set_index(i + 2);
+ ctx->offset = 0;
+ ret = traceprobe_parse_probe_arg(&tw->tp, i, argv[i], ctx);
+ if (ret)
+ return ret; /* This can be -ENOMEM */
+ }
+
+ ret = traceprobe_set_print_fmt(&tw->tp, PROBE_PRINT_NORMAL);
+ if (ret < 0)
+ return ret;
+
+ ret = register_trace_wprobe_event(tw);
+ if (!ret)
+ tw = NULL; /* To avoid free */
+
+ return ret;
+}
+
+static int trace_wprobe_create(const char *raw_command)
+{
+ return trace_probe_create(raw_command, __trace_wprobe_create);
+}
+
+static int trace_wprobe_release(struct dyn_event *ev)
+{
+ struct trace_wprobe *tw = to_trace_wprobe(ev);
+ int ret = unregister_trace_wprobe(tw);
+
+ if (!ret)
+ free_trace_wprobe(tw);
+ return ret;
+}
+
+static int trace_wprobe_show(struct seq_file *m, struct dyn_event *ev)
+{
+ struct trace_wprobe *tw = to_trace_wprobe(ev);
+ int i;
+
+ seq_printf(m, "w:%s/%s", trace_probe_group_name(&tw->tp),
+ trace_probe_name(&tw->tp));
+
+ char type_char;
+
+ if (tw->type == HW_BREAKPOINT_R)
+ type_char = 'r';
+ else if (tw->type == HW_BREAKPOINT_W)
+ type_char = 'w';
+ else
+ type_char = 'x'; /* Should be rw */
+
+ int len;
+
+ if (tw->len == HW_BREAKPOINT_LEN_1)
+ len = 1;
+ else if (tw->len == HW_BREAKPOINT_LEN_2)
+ len = 2;
+ else if (tw->len == HW_BREAKPOINT_LEN_4)
+ len = 4;
+ else
+ len = 8;
+
+ if (tw->symbol)
+ seq_printf(m, " %c@%s:%d", type_char, tw->symbol, len);
+ else
+ seq_printf(m, " %c@0x%lx:%d", type_char, tw->addr, len);
+
+ for (i = 0; i < tw->tp.nr_args; i++)
+ seq_printf(m, " %s=%s", tw->tp.args[i].name, tw->tp.args[i].comm);
+ seq_putc(m, '\n');
+
+ return 0;
+}
+
+static __init int init_wprobe_trace(void)
+{
+ return dyn_event_register(&trace_wprobe_ops);
+}
+fs_initcall(init_wprobe_trace);
+
^ permalink raw reply related
* [PATCH v7 02/10] x86: hw_breakpoint: Add a kconfig to clarify when a breakpoint fires
From: Masami Hiramatsu (Google) @ 2026-07-15 1:44 UTC (permalink / raw)
To: Steven Rostedt, Peter Zijlstra, Ingo Molnar, x86
Cc: Jinchao Wang, Mathieu Desnoyers, Masami Hiramatsu,
Thomas Gleixner, Borislav Petkov, Dave Hansen, H . Peter Anvin,
Alexander Shishkin, Ian Rogers, linux-kernel, linux-trace-kernel,
linux-doc, linux-perf-users
In-Reply-To: <178407983818.95826.12714571928538799781.stgit@devnote2>
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Add CONFIG_HAVE_POST_BREAKPOINT_HOOK which indicates the hw_breakpoint
on that architecture fires after the target memory has been modified.
This is currently x86 only behavior.
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
arch/Kconfig | 10 ++++++++++
arch/x86/Kconfig | 1 +
kernel/trace/Kconfig | 1 +
3 files changed, 12 insertions(+)
diff --git a/arch/Kconfig b/arch/Kconfig
index fa7507ac8e13..959aee9568ff 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -457,6 +457,16 @@ config HAVE_MIXED_BREAKPOINTS_REGS
Select this option if your arch implements breakpoints under the
latter fashion.
+config HAVE_POST_BREAKPOINT_HOOK
+ bool
+ depends on HAVE_HW_BREAKPOINT
+ help
+ Depending on the arch implementation of hardware breakpoints,
+ some of them provide breakpoint hook after the target memory
+ is modified.
+ Select this option if your arch implements breakpoints overflow
+ handler hooks after the target memory is modified.
+
config HAVE_USER_RETURN_NOTIFIER
bool
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index bdad90f210e4..6b7e14ef8cfb 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -246,6 +246,7 @@ config X86
select HAVE_FUNCTION_TRACER
select HAVE_GCC_PLUGINS
select HAVE_HW_BREAKPOINT
+ select HAVE_POST_BREAKPOINT_HOOK
select HAVE_IOREMAP_PROT
select HAVE_IRQ_EXIT_ON_IRQ_STACK if X86_64
select HAVE_IRQ_TIME_ACCOUNTING
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index b58c2565024f..d9b6fa5c35d9 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -866,6 +866,7 @@ config WPROBE_EVENTS
bool "Enable wprobe-based dynamic events"
depends on TRACING
depends on HAVE_HW_BREAKPOINT
+ depends on HAVE_POST_BREAKPOINT_HOOK
select PROBE_EVENTS
select DYNAMIC_EVENTS
help
^ permalink raw reply related
* [PATCH v7 03/10] selftests: tracing: Add a basic testcase for wprobe
From: Masami Hiramatsu (Google) @ 2026-07-15 1:44 UTC (permalink / raw)
To: Steven Rostedt, Peter Zijlstra, Ingo Molnar, x86
Cc: Jinchao Wang, Mathieu Desnoyers, Masami Hiramatsu,
Thomas Gleixner, Borislav Petkov, Dave Hansen, H . Peter Anvin,
Alexander Shishkin, Ian Rogers, linux-kernel, linux-trace-kernel,
linux-doc, linux-perf-users
In-Reply-To: <178407983818.95826.12714571928538799781.stgit@devnote2>
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Add 'add_remove_wprobe.tc' testcase for testing wprobe event that
tests adding and removing operations of the wprobe event.
Link: https://lore.kernel.org/all/175859026716.374439.14852239332989324292.stgit@devnote2/
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
tools/testing/selftests/ftrace/config | 1
.../ftrace/test.d/dynevent/add_remove_wprobe.tc | 68 ++++++++++++++++++++
2 files changed, 69 insertions(+)
create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/add_remove_wprobe.tc
diff --git a/tools/testing/selftests/ftrace/config b/tools/testing/selftests/ftrace/config
index 544de0db5f58..d2f503722020 100644
--- a/tools/testing/selftests/ftrace/config
+++ b/tools/testing/selftests/ftrace/config
@@ -27,3 +27,4 @@ CONFIG_STACK_TRACER=y
CONFIG_TRACER_SNAPSHOT=y
CONFIG_UPROBES=y
CONFIG_UPROBE_EVENTS=y
+CONFIG_WPROBE_EVENTS=y
diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_wprobe.tc b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_wprobe.tc
new file mode 100644
index 000000000000..20774c7f69f8
--- /dev/null
+++ b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_wprobe.tc
@@ -0,0 +1,68 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# description: Generic dynamic event - add/remove wprobe events
+# requires: dynamic_events "w[:[<group>/][<event>]] [r|w|rw]@<addr>[:<len>]":README
+
+echo 0 > events/enable
+echo > dynamic_events
+
+# Use jiffies as a variable that is frequently written to.
+TARGET=jiffies
+
+echo "w:my_wprobe w@$TARGET" >> dynamic_events
+
+grep -q my_wprobe dynamic_events
+if [ $? -ne 0 ]; then
+ echo "Failed to create wprobe event"
+ exit_fail
+fi
+
+test -d events/wprobes/my_wprobe
+if [ $? -ne 0 ]; then
+ echo "Failed to create wprobe event directory"
+ exit_fail
+fi
+
+echo 1 > events/wprobes/my_wprobe/enable
+
+# Check if the event is enabled
+cat events/wprobes/my_wprobe/enable | grep -q 1
+if [ $? -ne 0 ]; then
+ echo "Failed to enable wprobe event"
+ exit_fail
+fi
+
+# Let some time pass to trigger the breakpoint
+sleep 1
+
+# Check if we got any trace output
+if !grep -q my_wprobe trace; then
+ echo "wprobe event was not triggered"
+fi
+
+echo 0 > events/wprobes/my_wprobe/enable
+
+# Check if the event is disabled
+cat events/wprobes/my_wprobe/enable | grep -q 0
+if [ $? -ne 0 ]; then
+ echo "Failed to disable wprobe event"
+ exit_fail
+fi
+
+echo "-:my_wprobe" >> dynamic_events
+
+! grep -q my_wprobe dynamic_events
+if [ $? -ne 0 ]; then
+ echo "Failed to remove wprobe event"
+ exit_fail
+fi
+
+! test -d events/wprobes/my_wprobe
+if [ $? -ne 0 ]; then
+ echo "Failed to remove wprobe event directory"
+ exit_fail
+fi
+
+clear_trace
+
+exit 0
^ permalink raw reply related
* [PATCH v7 04/10] selftests: tracing: Add syntax testcase for wprobe
From: Masami Hiramatsu (Google) @ 2026-07-15 1:44 UTC (permalink / raw)
To: Steven Rostedt, Peter Zijlstra, Ingo Molnar, x86
Cc: Jinchao Wang, Mathieu Desnoyers, Masami Hiramatsu,
Thomas Gleixner, Borislav Petkov, Dave Hansen, H . Peter Anvin,
Alexander Shishkin, Ian Rogers, linux-kernel, linux-trace-kernel,
linux-doc, linux-perf-users
In-Reply-To: <178407983818.95826.12714571928538799781.stgit@devnote2>
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Add "wprobe_syntax_errors.tc" testcase for testing syntax errors
of the watch probe events.
Link: https://lore.kernel.org/all/175859027842.374439.6402700780945714048.stgit@devnote2/
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
.../test.d/dynevent/wprobes_syntax_errors.tc | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/wprobes_syntax_errors.tc
diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/wprobes_syntax_errors.tc b/tools/testing/selftests/ftrace/test.d/dynevent/wprobes_syntax_errors.tc
new file mode 100644
index 000000000000..56ac579d60ae
--- /dev/null
+++ b/tools/testing/selftests/ftrace/test.d/dynevent/wprobes_syntax_errors.tc
@@ -0,0 +1,20 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# description: Watch probe event parser error log check
+# requires: dynamic_events "w[:[<group>/][<event>]] [r|w|rw]@<addr>[:<len>]":README
+
+check_error() { # command-with-error-pos-by-^
+ ftrace_errlog_check 'wprobe' "$1" 'dynamic_events'
+}
+
+check_error 'w ^symbol' # BAD_ACCESS_FMT
+check_error 'w ^a@symbol' # BAD_ACCESS_TYPE
+check_error 'w w@^symbol' # BAD_ACCESS_ADDR
+check_error 'w w@jiffies^+offset' # BAD_ACCESS_ADDR
+check_error 'w w@jiffies:^100' # BAD_ACCESS_LEN
+check_error 'w w@jiffies ^$arg1' # BAD_VAR
+check_error 'w w@jiffies ^$retval' # BAD_VAR
+check_error 'w w@jiffies ^$stack' # BAD_VAR
+check_error 'w w@jiffies ^%ax' # BAD_VAR
+
+exit 0
^ permalink raw reply related
* [PATCH v7 05/10] tracing: wprobe: Use a new seq_print_ip_sym_offset() wrapper
From: Masami Hiramatsu (Google) @ 2026-07-15 1:44 UTC (permalink / raw)
To: Steven Rostedt, Peter Zijlstra, Ingo Molnar, x86
Cc: Jinchao Wang, Mathieu Desnoyers, Masami Hiramatsu,
Thomas Gleixner, Borislav Petkov, Dave Hansen, H . Peter Anvin,
Alexander Shishkin, Ian Rogers, linux-kernel, linux-trace-kernel,
linux-doc, linux-perf-users
In-Reply-To: <178407983818.95826.12714571928538799781.stgit@devnote2>
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Use a new seq_print_ip_sym_offset() wrapper function instead of
using TRACE_ITER(SYM_OFFSET) mask directly.
Link: https://lore.kernel.org/all/176226550596.59499.18020648957674458755.stgit@devnote2/
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
kernel/trace/trace_wprobe.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/trace/trace_wprobe.c b/kernel/trace/trace_wprobe.c
index b52f3eac719f..dd310a87b333 100644
--- a/kernel/trace/trace_wprobe.c
+++ b/kernel/trace/trace_wprobe.c
@@ -20,6 +20,7 @@
#include <asm/ptrace.h>
#include "trace_dynevent.h"
+#include "trace_output.h"
#include "trace_probe.h"
#include "trace_probe_kernel.h"
#include "trace_probe_tmpl.h"
^ permalink raw reply related
* [PATCH v7 06/10] x86/hw_breakpoint: Unify breakpoint install/uninstall
From: Masami Hiramatsu (Google) @ 2026-07-15 1:45 UTC (permalink / raw)
To: Steven Rostedt, Peter Zijlstra, Ingo Molnar, x86
Cc: Jinchao Wang, Mathieu Desnoyers, Masami Hiramatsu,
Thomas Gleixner, Borislav Petkov, Dave Hansen, H . Peter Anvin,
Alexander Shishkin, Ian Rogers, linux-kernel, linux-trace-kernel,
linux-doc, linux-perf-users
In-Reply-To: <178407983818.95826.12714571928538799781.stgit@devnote2>
From: Jinchao Wang <wangjinchao600@gmail.com>
Consolidate breakpoint management to reduce code duplication.
The diffstat was misleading, so the stripped code size is compared instead.
After refactoring, it is reduced from 11976 bytes to 11448 bytes on my
x86_64 system built with clang.
This also makes it easier to introduce arch_reinstall_hw_breakpoint().
In addition, including linux/types.h to fix a missing build dependency.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
arch/x86/include/asm/hw_breakpoint.h | 6 +
arch/x86/kernel/hw_breakpoint.c | 141 +++++++++++++++++++---------------
2 files changed, 84 insertions(+), 63 deletions(-)
diff --git a/arch/x86/include/asm/hw_breakpoint.h b/arch/x86/include/asm/hw_breakpoint.h
index 0bc931cd0698..aa6adac6c3a2 100644
--- a/arch/x86/include/asm/hw_breakpoint.h
+++ b/arch/x86/include/asm/hw_breakpoint.h
@@ -5,6 +5,7 @@
#include <uapi/asm/hw_breakpoint.h>
#define __ARCH_HW_BREAKPOINT_H
+#include <linux/types.h>
/*
* The name should probably be something dealt in
@@ -18,6 +19,11 @@ struct arch_hw_breakpoint {
u8 type;
};
+enum bp_slot_action {
+ BP_SLOT_ACTION_INSTALL,
+ BP_SLOT_ACTION_UNINSTALL,
+};
+
#include <linux/kdebug.h>
#include <linux/percpu.h>
#include <linux/list.h>
diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
index f846c15f21ca..877509539300 100644
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
@@ -49,7 +49,6 @@ static DEFINE_PER_CPU(unsigned long, cpu_debugreg[HBP_NUM]);
*/
static DEFINE_PER_CPU(struct perf_event *, bp_per_reg[HBP_NUM]);
-
static inline unsigned long
__encode_dr7(int drnum, unsigned int len, unsigned int type)
{
@@ -86,96 +85,112 @@ int decode_dr7(unsigned long dr7, int bpnum, unsigned *len, unsigned *type)
}
/*
- * Install a perf counter breakpoint.
- *
- * We seek a free debug address register and use it for this
- * breakpoint. Eventually we enable it in the debug control register.
- *
- * Atomic: we hold the counter->ctx->lock and we only handle variables
- * and registers local to this cpu.
+ * We seek a slot and change it or keep it based on the action.
+ * Returns slot number on success, negative error on failure.
+ * Must be called with IRQs disabled.
*/
-int arch_install_hw_breakpoint(struct perf_event *bp)
+static int manage_bp_slot(struct perf_event *bp, enum bp_slot_action action)
{
- struct arch_hw_breakpoint *info = counter_arch_bp(bp);
- unsigned long *dr7;
- int i;
-
- lockdep_assert_irqs_disabled();
+ struct perf_event *old_bp;
+ struct perf_event *new_bp;
+ int slot;
+
+ switch (action) {
+ case BP_SLOT_ACTION_INSTALL:
+ old_bp = NULL;
+ new_bp = bp;
+ break;
+ case BP_SLOT_ACTION_UNINSTALL:
+ old_bp = bp;
+ new_bp = NULL;
+ break;
+ default:
+ return -EINVAL;
+ }
- for (i = 0; i < HBP_NUM; i++) {
- struct perf_event **slot = this_cpu_ptr(&bp_per_reg[i]);
+ for (slot = 0; slot < HBP_NUM; slot++) {
+ struct perf_event **curr = this_cpu_ptr(&bp_per_reg[slot]);
- if (!*slot) {
- *slot = bp;
- break;
+ if (*curr == old_bp) {
+ *curr = new_bp;
+ return slot;
}
}
- if (WARN_ONCE(i == HBP_NUM, "Can't find any breakpoint slot"))
- return -EBUSY;
+ if (old_bp) {
+ WARN_ONCE(1, "Can't find matching breakpoint slot");
+ return -EINVAL;
+ }
+
+ WARN_ONCE(1, "No free breakpoint slots");
+ return -EBUSY;
+}
+
+static void setup_hwbp(struct arch_hw_breakpoint *info, int slot, bool enable)
+{
+ unsigned long dr7;
- set_debugreg(info->address, i);
- __this_cpu_write(cpu_debugreg[i], info->address);
+ set_debugreg(info->address, slot);
+ __this_cpu_write(cpu_debugreg[slot], info->address);
- dr7 = this_cpu_ptr(&cpu_dr7);
- *dr7 |= encode_dr7(i, info->len, info->type);
+ dr7 = this_cpu_read(cpu_dr7);
+ if (enable)
+ dr7 |= encode_dr7(slot, info->len, info->type);
+ else
+ dr7 &= ~__encode_dr7(slot, info->len, info->type);
/*
- * Ensure we first write cpu_dr7 before we set the DR7 register.
- * This ensures an NMI never see cpu_dr7 0 when DR7 is not.
+ * Enabling:
+ * Ensure we first write cpu_dr7 before we set the DR7 register.
+ * This ensures an NMI never see cpu_dr7 0 when DR7 is not.
*/
+ if (enable)
+ this_cpu_write(cpu_dr7, dr7);
+
barrier();
- set_debugreg(*dr7, 7);
+ set_debugreg(dr7, 7);
+
if (info->mask)
- amd_set_dr_addr_mask(info->mask, i);
+ amd_set_dr_addr_mask(enable ? info->mask : 0, slot);
- return 0;
+ /*
+ * Disabling:
+ * Ensure the write to cpu_dr7 is after we've set the DR7 register.
+ * This ensures an NMI never see cpu_dr7 0 when DR7 is not.
+ */
+ if (!enable)
+ this_cpu_write(cpu_dr7, dr7);
}
/*
- * Uninstall the breakpoint contained in the given counter.
- *
- * First we search the debug address register it uses and then we disable
- * it.
- *
- * Atomic: we hold the counter->ctx->lock and we only handle variables
- * and registers local to this cpu.
+ * find suitable breakpoint slot and set it up based on the action
*/
-void arch_uninstall_hw_breakpoint(struct perf_event *bp)
+static int arch_manage_bp(struct perf_event *bp, enum bp_slot_action action)
{
- struct arch_hw_breakpoint *info = counter_arch_bp(bp);
- unsigned long dr7;
- int i;
+ struct arch_hw_breakpoint *info;
+ int slot;
lockdep_assert_irqs_disabled();
- for (i = 0; i < HBP_NUM; i++) {
- struct perf_event **slot = this_cpu_ptr(&bp_per_reg[i]);
-
- if (*slot == bp) {
- *slot = NULL;
- break;
- }
- }
-
- if (WARN_ONCE(i == HBP_NUM, "Can't find any breakpoint slot"))
- return;
+ slot = manage_bp_slot(bp, action);
+ if (slot < 0)
+ return slot;
- dr7 = this_cpu_read(cpu_dr7);
- dr7 &= ~__encode_dr7(i, info->len, info->type);
+ info = counter_arch_bp(bp);
+ setup_hwbp(info, slot, action != BP_SLOT_ACTION_UNINSTALL);
- set_debugreg(dr7, 7);
- if (info->mask)
- amd_set_dr_addr_mask(0, i);
+ return 0;
+}
- /*
- * Ensure the write to cpu_dr7 is after we've set the DR7 register.
- * This ensures an NMI never see cpu_dr7 0 when DR7 is not.
- */
- barrier();
+int arch_install_hw_breakpoint(struct perf_event *bp)
+{
+ return arch_manage_bp(bp, BP_SLOT_ACTION_INSTALL);
+}
- this_cpu_write(cpu_dr7, dr7);
+void arch_uninstall_hw_breakpoint(struct perf_event *bp)
+{
+ arch_manage_bp(bp, BP_SLOT_ACTION_UNINSTALL);
}
static int arch_bp_generic_len(int x86_len)
^ permalink raw reply related
* [PATCH v7 07/10] x86/hw_breakpoint: Add arch_reinstall_hw_breakpoint
From: Masami Hiramatsu (Google) @ 2026-07-15 1:45 UTC (permalink / raw)
To: Steven Rostedt, Peter Zijlstra, Ingo Molnar, x86
Cc: Jinchao Wang, Mathieu Desnoyers, Masami Hiramatsu,
Thomas Gleixner, Borislav Petkov, Dave Hansen, H . Peter Anvin,
Alexander Shishkin, Ian Rogers, linux-kernel, linux-trace-kernel,
linux-doc, linux-perf-users
In-Reply-To: <178407983818.95826.12714571928538799781.stgit@devnote2>
From: Jinchao Wang <wangjinchao600@gmail.com>
The new arch_reinstall_hw_breakpoint() function can be used in an
atomic context, unlike the more expensive free and re-allocation path.
This allows callers to efficiently re-establish an existing breakpoint.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
arch/x86/include/asm/hw_breakpoint.h | 2 ++
arch/x86/kernel/hw_breakpoint.c | 9 +++++++++
2 files changed, 11 insertions(+)
diff --git a/arch/x86/include/asm/hw_breakpoint.h b/arch/x86/include/asm/hw_breakpoint.h
index aa6adac6c3a2..c22cc4e87fc5 100644
--- a/arch/x86/include/asm/hw_breakpoint.h
+++ b/arch/x86/include/asm/hw_breakpoint.h
@@ -21,6 +21,7 @@ struct arch_hw_breakpoint {
enum bp_slot_action {
BP_SLOT_ACTION_INSTALL,
+ BP_SLOT_ACTION_REINSTALL,
BP_SLOT_ACTION_UNINSTALL,
};
@@ -65,6 +66,7 @@ extern int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
int arch_install_hw_breakpoint(struct perf_event *bp);
+int arch_reinstall_hw_breakpoint(struct perf_event *bp);
void arch_uninstall_hw_breakpoint(struct perf_event *bp);
void hw_breakpoint_pmu_read(struct perf_event *bp);
void hw_breakpoint_pmu_unthrottle(struct perf_event *bp);
diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
index 877509539300..9af8d81075db 100644
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
@@ -100,6 +100,10 @@ static int manage_bp_slot(struct perf_event *bp, enum bp_slot_action action)
old_bp = NULL;
new_bp = bp;
break;
+ case BP_SLOT_ACTION_REINSTALL:
+ old_bp = bp;
+ new_bp = bp;
+ break;
case BP_SLOT_ACTION_UNINSTALL:
old_bp = bp;
new_bp = NULL;
@@ -188,6 +192,11 @@ int arch_install_hw_breakpoint(struct perf_event *bp)
return arch_manage_bp(bp, BP_SLOT_ACTION_INSTALL);
}
+int arch_reinstall_hw_breakpoint(struct perf_event *bp)
+{
+ return arch_manage_bp(bp, BP_SLOT_ACTION_REINSTALL);
+}
+
void arch_uninstall_hw_breakpoint(struct perf_event *bp)
{
arch_manage_bp(bp, BP_SLOT_ACTION_UNINSTALL);
^ permalink raw reply related
* [PATCH v7 08/10] HWBP: Add modify_wide_hw_breakpoint_local() API
From: Masami Hiramatsu (Google) @ 2026-07-15 1:45 UTC (permalink / raw)
To: Steven Rostedt, Peter Zijlstra, Ingo Molnar, x86
Cc: Jinchao Wang, Mathieu Desnoyers, Masami Hiramatsu,
Thomas Gleixner, Borislav Petkov, Dave Hansen, H . Peter Anvin,
Alexander Shishkin, Ian Rogers, linux-kernel, linux-trace-kernel,
linux-doc, linux-perf-users
In-Reply-To: <178407983818.95826.12714571928538799781.stgit@devnote2>
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Add modify_wide_hw_breakpoint_local() arch-wide interface which allows
hwbp users to update watch address on-line. This is available if the
arch supports CONFIG_HAVE_REINSTALL_HW_BREAKPOINT.
Note that this allows to change the type only for compatible types,
because it does not release and reserve the hwbp slot based on type.
For instance, you can not change HW_BREAKPOINT_W to HW_BREAKPOINT_X.
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Changes in v7:
- Update bp->attr.bp_attr so that we can correctly check the
address on it.
- Use -EOPNOTSUPP instead of -ENOSYS.
Changes in v4:
- Update kerneldoc comment about modify_wide_hw_breakpoint_local
according to Randy's comment.
Changes in v2:
- Check type compatibility by checking slot. (Thanks Jinchao!)
---
arch/Kconfig | 10 ++++++++++
arch/x86/Kconfig | 1 +
include/linux/hw_breakpoint.h | 6 ++++++
kernel/events/hw_breakpoint.c | 39 +++++++++++++++++++++++++++++++++++++++
4 files changed, 56 insertions(+)
diff --git a/arch/Kconfig b/arch/Kconfig
index 959aee9568ff..4a87e843bb4c 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -467,6 +467,16 @@ config HAVE_POST_BREAKPOINT_HOOK
Select this option if your arch implements breakpoints overflow
handler hooks after the target memory is modified.
+config HAVE_REINSTALL_HW_BREAKPOINT
+ bool
+ depends on HAVE_HW_BREAKPOINT
+ help
+ Depending on the arch implementation of hardware breakpoints,
+ some of them are able to update the breakpoint configuration
+ without release and reserve the hardware breakpoint register.
+ What configuration is able to update depends on hardware and
+ software implementation.
+
config HAVE_USER_RETURN_NOTIFIER
bool
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 6b7e14ef8cfb..588218da8f41 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -247,6 +247,7 @@ config X86
select HAVE_GCC_PLUGINS
select HAVE_HW_BREAKPOINT
select HAVE_POST_BREAKPOINT_HOOK
+ select HAVE_REINSTALL_HW_BREAKPOINT
select HAVE_IOREMAP_PROT
select HAVE_IRQ_EXIT_ON_IRQ_STACK if X86_64
select HAVE_IRQ_TIME_ACCOUNTING
diff --git a/include/linux/hw_breakpoint.h b/include/linux/hw_breakpoint.h
index db199d653dd1..6754ffbee9ed 100644
--- a/include/linux/hw_breakpoint.h
+++ b/include/linux/hw_breakpoint.h
@@ -81,6 +81,9 @@ register_wide_hw_breakpoint(struct perf_event_attr *attr,
perf_overflow_handler_t triggered,
void *context);
+extern int modify_wide_hw_breakpoint_local(struct perf_event *bp,
+ struct perf_event_attr *attr);
+
extern int register_perf_hw_breakpoint(struct perf_event *bp);
extern void unregister_hw_breakpoint(struct perf_event *bp);
extern void unregister_wide_hw_breakpoint(struct perf_event * __percpu *cpu_events);
@@ -124,6 +127,9 @@ register_wide_hw_breakpoint(struct perf_event_attr *attr,
perf_overflow_handler_t triggered,
void *context) { return NULL; }
static inline int
+modify_wide_hw_breakpoint_local(struct perf_event *bp,
+ struct perf_event_attr *attr) { return -EOPNOTSUPP; }
+static inline int
register_perf_hw_breakpoint(struct perf_event *bp) { return -ENOSYS; }
static inline void unregister_hw_breakpoint(struct perf_event *bp) { }
static inline void
diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
index 789add0c185a..4337688da397 100644
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -888,6 +888,45 @@ void unregister_wide_hw_breakpoint(struct perf_event * __percpu *cpu_events)
}
EXPORT_SYMBOL_GPL(unregister_wide_hw_breakpoint);
+/**
+ * modify_wide_hw_breakpoint_local - update breakpoint config for local CPU
+ * @bp: the hwbp perf event for this CPU
+ * @attr: the new attribute for @bp
+ *
+ * This does not release and reserve the slot of a HWBP; it just reuses the
+ * current slot on local CPU. So the users must update the other CPUs by
+ * themselves.
+ * Also, since this does not release/reserve the slot, this can not change the
+ * type to incompatible type of the HWBP.
+ * Return err if attr is invalid or the CPU fails to update debug register
+ * for new @attr.
+ */
+#ifdef CONFIG_HAVE_REINSTALL_HW_BREAKPOINT
+int modify_wide_hw_breakpoint_local(struct perf_event *bp,
+ struct perf_event_attr *attr)
+{
+ int ret;
+
+ if (find_slot_idx(bp->attr.bp_type) != find_slot_idx(attr->bp_type))
+ return -EINVAL;
+
+ ret = hw_breakpoint_arch_parse(bp, attr, counter_arch_bp(bp));
+ if (ret)
+ return ret;
+
+ bp->attr.bp_addr = attr->bp_addr;
+
+ return arch_reinstall_hw_breakpoint(bp);
+}
+#else
+int modify_wide_hw_breakpoint_local(struct perf_event *bp,
+ struct perf_event_attr *attr)
+{
+ return -EOPNOTSUPP;
+}
+#endif
+EXPORT_SYMBOL_GPL(modify_wide_hw_breakpoint_local);
+
/**
* hw_breakpoint_is_used - check if breakpoints are currently used
*
^ permalink raw reply related
* [PATCH v7 09/10] tracing: wprobe: Add wprobe event trigger
From: Masami Hiramatsu (Google) @ 2026-07-15 1:45 UTC (permalink / raw)
To: Steven Rostedt, Peter Zijlstra, Ingo Molnar, x86
Cc: Jinchao Wang, Mathieu Desnoyers, Masami Hiramatsu,
Thomas Gleixner, Borislav Petkov, Dave Hansen, H . Peter Anvin,
Alexander Shishkin, Ian Rogers, linux-kernel, linux-trace-kernel,
linux-doc, linux-perf-users
In-Reply-To: <178407983818.95826.12714571928538799781.stgit@devnote2>
From: Masami Hiramatsu (Google) <mhiramat@kernel.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:
- 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 does not change the type and length, these
must be set when creating a new wprobe.
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@kernel.org>
---
Changes in v7:
- Use kzalloc_obj().
- Update sample code in document.
Changes in v6:
- Update according to the latest change of trigger ops.
Changes in v5:
- Following the suggestions, the documentation was revised to suit rst.
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 | 89 ++++++++
include/linux/trace_events.h | 1
kernel/trace/Kconfig | 10 +
kernel/trace/trace_wprobe.c | 415 +++++++++++++++++++++++++++++++++++
4 files changed, 515 insertions(+)
diff --git a/Documentation/trace/wprobetrace.rst b/Documentation/trace/wprobetrace.rst
index 025b4c39b809..6f3d2afb35e3 100644
--- a/Documentation/trace/wprobetrace.rst
+++ b/Documentation/trace/wprobetrace.rst
@@ -67,3 +67,92 @@ 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]
+
+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 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.
+
+
+For example, trace the first 8 bytes 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: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::
+
+ # tracer: nop
+ #
+ # entries-in-buffer/entries-written: 32/32 #P:8
+ #
+ # _-----=> irqs-off/BH-disabled
+ # / _----=> need-resched
+ # | / _---=> hardirq/softirq
+ # || / _--=> preempt-depth
+ # ||| / _-=> migrate-disable
+ # |||| / delay
+ # TASK-PID CPU# ||||| TIMESTAMP FUNCTION
+ # | | | ||||| | |
+ sh-107 [004] ...1. 9.990418: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff888004ad6618
+ sh-107 [004] ...1. 9.990914: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff888004b3de78
+ sh-107 [004] ...1. 9.993175: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff8880049ddd40
+ sh-107 [004] ..... 9.995198: truncate: (do_truncate+0x4/0x120) dentry=0xffff8880048083a8
+ sh-107 [004] ...1. 9.995389: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff8880049db998
+ sh-107 [004] ..Zff 9.997503: watch: (lookup_fast+0xaa/0x150) address=0xffff8880048083a8 value=0x8200080
+ sh-107 [004] ..Zff 9.997509: watch: (path_openat+0x211/0xda0) address=0xffff8880048083a8 value=0x8200080
+ sh-107 [004] ..Zff 9.997514: watch: (path_openat+0xa56/0xda0) address=0xffff8880048083a8 value=0x8200080
+ sh-107 [004] ..Zff 9.997518: watch: (path_openat+0xae2/0xda0) address=0xffff8880048083a8 value=0x8200080
+ sh-107 [004] ..... 9.997521: truncate: (do_truncate+0x4/0x120) dentry=0xffff8880048083a8
+ sh-107 [004] ...1. 9.997582: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff888004808270
+ sh-107 [004] ...1. 9.999365: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff8880049db728
+ sh-107 [004] ...1. 9.999388: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff888004b1c000
+ rm-113 [005] ..Zff 10.000965: watch: (lookup_fast+0xaa/0x150) address=0xffff8880048083a8 value=0x8200080
+ rm-113 [005] ..Zff 10.000971: watch: (path_lookupat+0x97/0x1e0) address=0xffff8880048083a8 value=0x8200080
+ rm-113 [005] ..Zff 10.000984: watch: (lookup_fast+0xaa/0x150) address=0xffff8880048083a8 value=0x8200080
+ rm-113 [005] ..Zff 10.000988: watch: (path_lookupat+0x97/0x1e0) address=0xffff8880048083a8 value=0x8200080
+ rm-113 [005] ..Zff 10.001010: watch: (lookup_one_qstr_excl+0x28/0x140) address=0xffff8880048083a8 value=0x8200080
+ rm-113 [005] ..Zff 10.001014: watch: (lookup_one_qstr_excl+0xd1/0x140) address=0xffff8880048083a8 value=0x8200080
+ rm-113 [005] ..Zff 10.001018: watch: (may_delete_dentry+0x1c/0x200) address=0xffff8880048083a8 value=0x8200080
+ rm-113 [005] ..Zff 10.001021: watch: (may_delete_dentry+0x195/0x200) address=0xffff8880048083a8 value=0x8200080
+ rm-113 [005] ..Zff 10.001031: watch: (vfs_unlink+0x5e/0x260) address=0xffff8880048083a8 value=0x8200080
+ rm-113 [005] d.Z.. 10.001067: watch: (d_make_discardable+0x1b/0x40) address=0xffff8880048083a8 value=0x8200080
+ rm-113 [005] d.Z.. 10.001071: watch: (d_make_discardable+0x29/0x40) address=0xffff8880048083a8 value=0x200080
+ rm-113 [005] ...1. 10.001072: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff8880048083a8
+ rm-113 [005] ...1. 10.001218: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff8880048083a8
+ sh-107 [004] ...1. 10.001416: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff8880049db110
+ sh-107 [004] ...1. 10.001444: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff8880049db248
+ sh-107 [004] ...1. 10.001500: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff888004ad6618
+ sh-107 [004] ...1. 10.002067: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff888004b41e78
+ sh-107 [004] ...1. 10.904920: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff888004b41e78
+ sh-107 [004] ...1. 10.905129: dentry_kill: (dentry_kill+0x0/0x2c0) dentry=0xffff888004ad6618
+
+You can see the watch event is correctly configured on the dentry.
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index d1e5ab71d928..e6f3bbcbb9af 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -729,6 +729,7 @@ enum event_trigger_type {
ETT_EVENT_HIST = (1 << 4),
ETT_HIST_ENABLE = (1 << 5),
ETT_EVENT_EPROBE = (1 << 6),
+ ETT_EVENT_WPROBE = (1 << 7),
};
extern int filter_match_preds(struct event_filter *filter, void *rec);
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index d9b6fa5c35d9..af570fa2a882 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_REINSTALL_HW_BREAKPOINT
+ 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_wprobe.c b/kernel/trace/trace_wprobe.c
index dd310a87b333..1812b1e74111 100644
--- a/kernel/trace/trace_wprobe.c
+++ b/kernel/trace/trace_wprobe.c
@@ -6,6 +6,8 @@
*/
#define pr_fmt(fmt) "trace_wprobe: " fmt
+#include <linux/atomic.h>
+#include <linux/errno.h>
#include <linux/hw_breakpoint.h>
#include <linux/kallsyms.h>
#include <linux/list.h>
@@ -14,11 +16,14 @@
#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 <asm/ptrace.h>
+#include "trace.h"
#include "trace_dynevent.h"
#include "trace_output.h"
#include "trace_probe.h"
@@ -691,3 +696,413 @@ static __init int init_wprobe_trace(void)
}
fs_initcall(init_wprobe_trace);
+#ifdef CONFIG_WPROBE_TRIGGERS
+
+static int wprobe_trigger_global_enabled;
+
+#define SET_WPROBE_STR "set_wprobe"
+#define CLEAR_WPROBE_STR "clear_wprobe"
+#define WPROBE_DEFAULT_CLEAR_ADDRESS ((unsigned long)&wprobe_trigger_global_enabled)
+
+struct wprobe_trigger_data {
+ struct trace_event_file *file;
+ struct trace_wprobe *tw;
+
+ struct perf_event_attr attr;
+ raw_spinlock_t lock; /* lock protects attr */
+ struct work_struct work;// TBD: use work + IPI or use sched/raw_syscall event?
+ unsigned int offset;
+ long adjust;
+ const char *field;
+ // size must be unsigned long because it should be an address.
+ bool clear;
+};
+
+static int trace_wprobe_update_local(struct trace_wprobe *tw,
+ struct perf_event_attr *attr)
+{
+ struct perf_event *bp = *this_cpu_ptr(tw->bp_event);
+
+ return modify_wide_hw_breakpoint_local(bp, attr);
+}
+
+static void wprobe_smp_update_func(void *data)
+{
+ struct wprobe_trigger_data *trigger_data = data;
+ unsigned long flags;
+
+ raw_spin_lock_irqsave(&trigger_data->lock, flags);
+ trace_wprobe_update_local(trigger_data->tw, &trigger_data->attr);
+ raw_spin_unlock_irqrestore(&trigger_data->lock, flags);
+}
+
+static void wprobe_work_func(struct work_struct *work)
+{
+ struct wprobe_trigger_data *data = container_of(work, struct wprobe_trigger_data, work);
+
+ on_each_cpu(wprobe_smp_update_func, data, false);
+}
+
+static void wprobe_trigger(struct event_trigger_data *data,
+ struct trace_buffer *buffer, void *rec,
+ struct ring_buffer_event *event)
+{
+ struct wprobe_trigger_data *wprobe_data = data->private_data;
+ struct perf_event_attr *attr = &wprobe_data->attr;
+ struct trace_wprobe *tw = wprobe_data->tw;
+ unsigned long addr, flags;
+ int ret = -EBUSY;
+
+ addr = *(unsigned long *)(rec + wprobe_data->offset);
+ addr += wprobe_data->adjust;
+
+ raw_spin_lock_irqsave(&wprobe_data->lock, flags);
+
+ if (!wprobe_data->clear) {
+ if (tw->addr != WPROBE_DEFAULT_CLEAR_ADDRESS)
+ goto unlock;
+
+ tw->addr = attr->bp_addr = addr;
+ ret = trace_wprobe_update_local(tw, attr);
+ if (WARN_ON_ONCE(ret))
+ goto unlock;
+ clear_bit(EVENT_FILE_FL_SOFT_DISABLED_BIT, &wprobe_data->file->flags);
+ } else {
+ if (tw->addr == WPROBE_DEFAULT_CLEAR_ADDRESS)
+ goto unlock;
+ if (wprobe_data->field && tw->addr != addr)
+ goto unlock;
+
+ tw->addr = attr->bp_addr = WPROBE_DEFAULT_CLEAR_ADDRESS;
+ ret = trace_wprobe_update_local(tw, attr);
+ if (WARN_ON_ONCE(ret))
+ goto unlock;
+ set_bit(EVENT_FILE_FL_SOFT_DISABLED_BIT, &wprobe_data->file->flags);
+ }
+ schedule_work(&wprobe_data->work);
+unlock:
+ raw_spin_unlock_irqrestore(&wprobe_data->lock, flags);
+}
+
+static void free_wprobe_trigger_data(struct wprobe_trigger_data *wprobe_data)
+{
+ if (wprobe_data)
+ kfree(wprobe_data->field);
+ kfree(wprobe_data);
+}
+
+DEFINE_FREE(free_wprobe_trigger_data, struct wprobe_trigger_data *, free_wprobe_trigger_data(_T));
+
+static int wprobe_trigger_print(struct seq_file *m,
+ struct event_trigger_data *data)
+{
+ struct wprobe_trigger_data *wprobe_data = data->private_data;
+
+ if (wprobe_data->clear) {
+ seq_printf(m, "%s:%s", CLEAR_WPROBE_STR,
+ trace_event_name(wprobe_data->file->event_call));
+ if (wprobe_data->field) {
+ seq_printf(m, ":%s%+ld",
+ wprobe_data->field, wprobe_data->adjust);
+ }
+ } else
+ seq_printf(m, "%s:%s:%s%+ld", SET_WPROBE_STR,
+ trace_event_name(wprobe_data->file->event_call),
+ wprobe_data->field, wprobe_data->adjust);
+
+ if (data->filter_str)
+ seq_printf(m, " if %s\n", data->filter_str);
+ else
+ seq_putc(m, '\n');
+
+ return 0;
+}
+
+static struct wprobe_trigger_data *
+wprobe_trigger_alloc(struct trace_wprobe *tw, struct trace_event_file *file,
+ bool clear)
+{
+ struct wprobe_trigger_data *wprobe_data;
+ struct perf_event_attr *attr;
+
+ wprobe_data = kzalloc_obj(*wprobe_data);
+ if (!wprobe_data)
+ return NULL;
+
+ wprobe_data->tw = tw;
+ wprobe_data->clear = clear;
+ wprobe_data->file = file;
+
+ attr = &wprobe_data->attr;
+ hw_breakpoint_init(attr);
+ attr->bp_type = tw->type;
+ attr->bp_addr = WPROBE_DEFAULT_CLEAR_ADDRESS;
+ attr->bp_len = tw->len;
+
+ raw_spin_lock_init(&wprobe_data->lock);
+ INIT_WORK(&wprobe_data->work, wprobe_work_func);
+
+ return wprobe_data;
+}
+
+static void wprobe_trigger_free(struct event_trigger_data *data)
+{
+ struct wprobe_trigger_data *wprobe_data = data->private_data;
+
+ if (WARN_ON_ONCE(data->ref <= 0))
+ return;
+
+ data->ref--;
+ if (!data->ref) {
+ /* Remove the SOFT_MODE flag */
+ trace_event_enable_disable(wprobe_data->file, 0, 1);
+ trace_event_put_ref(wprobe_data->file->event_call);
+ trigger_data_free(data);
+ free_wprobe_trigger_data(wprobe_data);
+ }
+}
+
+static int wprobe_trigger_cmd_parse(struct event_command *cmd_ops,
+ struct trace_event_file *file,
+ char *glob, char *cmd,
+ char *param_and_filter)
+{
+ /*
+ * set_wprobe:EVENT:FIELD[+OFFS]
+ * clear_wprobe:EVENT[:FIELD[+OFFS]]
+ */
+ struct wprobe_trigger_data *wprobe_data __free(free_wprobe_trigger_data) = NULL;
+ struct event_trigger_data *trigger_data __free(kfree) = NULL;
+ struct ftrace_event_field *field = NULL;
+ struct trace_event_file *wprobe_file;
+ struct trace_array *tr = file->tr;
+ struct trace_event_call *event;
+ struct perf_event_attr *attr;
+ char *event_str, *field_str;
+ bool remove, clear = false;
+ struct trace_wprobe *tw;
+ char *param, *filter;
+ int ret;
+
+ remove = event_trigger_check_remove(glob);
+
+ if (!strcmp(cmd, CLEAR_WPROBE_STR))
+ clear = true;
+
+ if (event_trigger_empty_param(param_and_filter))
+ return -EINVAL;
+
+ ret = event_trigger_separate_filter(param_and_filter, ¶m, &filter, true);
+ if (ret)
+ return ret;
+
+ event_str = strsep(¶m, ":");
+
+ /* Find target wprobe */
+ tw = find_trace_wprobe(event_str, WPROBE_EVENT_SYSTEM);
+ if (!tw)
+ return -ENOENT;
+ /* The target wprobe must not be used (unless clear) */
+ if (!remove && !clear && trace_probe_is_enabled(&tw->tp))
+ return -EBUSY;
+
+ wprobe_file = find_event_file(tr, WPROBE_EVENT_SYSTEM, event_str);
+ if (!wprobe_file)
+ return -EINVAL;
+
+ wprobe_data = wprobe_trigger_alloc(tw, wprobe_file, clear);
+ if (!wprobe_data)
+ return -ENOMEM;
+ attr = &wprobe_data->attr;
+
+ /* Find target field, which must be equivarent to "void *" */
+ field_str = strsep(¶m, ":");
+ /* trigger removing or clear_wprobe does not need field. */
+ if (!remove && !clear && !field_str)
+ return -EINVAL;
+
+ if (field_str) {
+ char *offs;
+
+ offs = strpbrk(field_str, "+-");
+ if (offs) {
+ long val;
+
+ if (kstrtol(offs, 0, &val) < 0)
+ return -EINVAL;
+ wprobe_data->adjust = val;
+ *offs = '\0';
+ }
+
+ event = file->event_call;
+ field = trace_find_event_field(event, field_str);
+ if (!field)
+ return -ENOENT;
+
+ if (field->size != sizeof(void *))
+ return -ENOEXEC;
+ wprobe_data->offset = field->offset;
+ wprobe_data->field = kstrdup(field_str, GFP_KERNEL);
+ if (!wprobe_data->field)
+ return -ENOMEM;
+ }
+
+ trigger_data = trigger_data_alloc(cmd_ops, cmd, param, wprobe_data);
+ if (!trigger_data)
+ return -ENOMEM;
+
+ /* Up the trigger_data count to make sure nothing frees it on failure */
+ event_trigger_init(trigger_data);
+
+ if (remove) {
+ event_trigger_unregister(cmd_ops, file, glob+1, trigger_data);
+ return 0;
+ }
+
+ ret = event_trigger_parse_num(param, trigger_data);
+ if (ret)
+ return ret;
+
+ ret = event_trigger_set_filter(cmd_ops, file, filter, trigger_data);
+ if (ret < 0)
+ return ret;
+
+ /* Soft-enable (register) wprobe event on WPROBE_DEFAULT_CLEAR_ADDRESS */
+ tw->addr = attr->bp_addr = WPROBE_DEFAULT_CLEAR_ADDRESS;
+ ret = trace_event_enable_disable(wprobe_file, 1, 1);
+ if (ret < 0) {
+ event_trigger_reset_filter(cmd_ops, trigger_data);
+ return ret;
+ }
+ ret = event_trigger_register(cmd_ops, file, glob, trigger_data);
+ if (ret) {
+ event_trigger_reset_filter(cmd_ops, trigger_data);
+ trace_event_enable_disable(wprobe_file, 0, 1);
+ return ret;
+ }
+ /* Make it NULL to avoid freeing trigger_data and wprobe_data by __free() */
+ trigger_data = NULL;
+ wprobe_data = NULL;
+
+ return 0;
+}
+
+/* Return event_trigger_data if there is a trigger which points the same wprobe */
+static struct event_trigger_data *
+wprobe_trigger_find_same(struct event_trigger_data *test,
+ struct trace_event_file *file)
+{
+ struct wprobe_trigger_data *test_wprobe_data = test->private_data;
+ struct wprobe_trigger_data *wprobe_data;
+ struct event_trigger_data *iter;
+
+ list_for_each_entry(iter, &file->triggers, list) {
+ wprobe_data = iter->private_data;
+ if (!wprobe_data ||
+ iter->cmd_ops->trigger_type !=
+ test->cmd_ops->trigger_type)
+ continue;
+ if (wprobe_data->tw == test_wprobe_data->tw)
+ return iter;
+ }
+ return NULL;
+}
+
+static int wprobe_register_trigger(char *glob,
+ struct event_trigger_data *data,
+ struct trace_event_file *file)
+{
+ int ret = 0;
+
+ lockdep_assert_held(&event_mutex);
+
+ /* The same wprobe is not accept on the same file (event) */
+ if (wprobe_trigger_find_same(data, file))
+ return -EEXIST;
+
+ if (data->cmd_ops->init) {
+ ret = data->cmd_ops->init(data);
+ if (ret < 0)
+ return ret;
+ }
+
+ list_add_rcu(&data->list, &file->triggers);
+
+ update_cond_flag(file);
+ ret = trace_event_trigger_enable_disable(file, 1);
+ if (ret < 0) {
+ list_del_rcu(&data->list);
+ update_cond_flag(file);
+ }
+ return ret;
+}
+
+static void wprobe_unregister_trigger(char *glob,
+ struct event_trigger_data *test,
+ struct trace_event_file *file)
+{
+ struct event_trigger_data *data;
+
+ lockdep_assert_held(&event_mutex);
+
+ data = wprobe_trigger_find_same(test, file);
+ if (!data)
+ return;
+
+ list_del_rcu(&data->list);
+ trace_event_trigger_enable_disable(file, 0);
+ update_cond_flag(file);
+ if (data->cmd_ops->free)
+ data->cmd_ops->free(data);
+}
+
+static struct event_command trigger_wprobe_set_cmd = {
+ .name = SET_WPROBE_STR,
+ .trigger_type = ETT_EVENT_WPROBE,
+ /* This triggers after when the event is recorded. */
+ .flags = EVENT_CMD_FL_NEEDS_REC,
+ .parse = wprobe_trigger_cmd_parse,
+ .reg = wprobe_register_trigger,
+ .unreg = wprobe_unregister_trigger,
+ .set_filter = set_trigger_filter,
+ .trigger = wprobe_trigger,
+ .count_func = event_trigger_count,
+ .print = wprobe_trigger_print,
+ .init = event_trigger_init,
+ .free = wprobe_trigger_free,
+};
+
+static struct event_command trigger_wprobe_clear_cmd = {
+ .name = CLEAR_WPROBE_STR,
+ .trigger_type = ETT_EVENT_WPROBE,
+ /* This triggers after when the event is recorded. */
+ .flags = EVENT_CMD_FL_NEEDS_REC,
+ .parse = wprobe_trigger_cmd_parse,
+ .reg = wprobe_register_trigger,
+ .unreg = wprobe_unregister_trigger,
+ .set_filter = set_trigger_filter,
+ .trigger = wprobe_trigger,
+ .count_func = event_trigger_count,
+ .print = wprobe_trigger_print,
+ .init = event_trigger_init,
+ .free = wprobe_trigger_free,
+};
+
+static __init int init_trigger_wprobe_cmds(void)
+{
+ int ret;
+
+ ret = register_event_command(&trigger_wprobe_set_cmd);
+ if (WARN_ON(ret < 0))
+ return ret;
+ ret = register_event_command(&trigger_wprobe_clear_cmd);
+ if (WARN_ON(ret < 0))
+ unregister_event_command(&trigger_wprobe_set_cmd);
+
+ if (!ret)
+ wprobe_trigger_global_enabled = 1;
+
+ return ret;
+}
+fs_initcall(init_trigger_wprobe_cmds);
+#endif /* CONFIG_WPROBE_TRIGGERS */
^ permalink raw reply related
* [PATCH v7 10/10] selftests: ftrace: Add wprobe trigger testcase
From: Masami Hiramatsu (Google) @ 2026-07-15 1:45 UTC (permalink / raw)
To: Steven Rostedt, Peter Zijlstra, Ingo Molnar, x86
Cc: Jinchao Wang, Mathieu Desnoyers, Masami Hiramatsu,
Thomas Gleixner, Borislav Petkov, Dave Hansen, H . Peter Anvin,
Alexander Shishkin, Ian Rogers, linux-kernel, linux-trace-kernel,
linux-doc, linux-perf-users
In-Reply-To: <178407983818.95826.12714571928538799781.stgit@devnote2>
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Add a testcase for checking wprobe trigger. This sets set_wprobe and
clear_wprobe triggers on fprobe events to watch dentry access.
So this depends on both wprobe and fprobe.
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Changes in v7:
- Add a newline at the end of file.(style fix)
- Use dentry_kill instead of __dentry_kill.
Changes in v5:
- Enable CONFIG_WPROBE_TRIGGERS in the config for ftrace test.
Changes in v3:
- Newly added.
---
tools/testing/selftests/ftrace/config | 1
.../ftrace/test.d/trigger/trigger-wprobe.tc | 48 ++++++++++++++++++++
2 files changed, 49 insertions(+)
create mode 100644 tools/testing/selftests/ftrace/test.d/trigger/trigger-wprobe.tc
diff --git a/tools/testing/selftests/ftrace/config b/tools/testing/selftests/ftrace/config
index d2f503722020..ecdee77f360f 100644
--- a/tools/testing/selftests/ftrace/config
+++ b/tools/testing/selftests/ftrace/config
@@ -28,3 +28,4 @@ CONFIG_TRACER_SNAPSHOT=y
CONFIG_UPROBES=y
CONFIG_UPROBE_EVENTS=y
CONFIG_WPROBE_EVENTS=y
+CONFIG_WPROBE_TRIGGERS=y
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 000000000000..cc7a0532d7ff
--- /dev/null
+++ b/tools/testing/selftests/ftrace/test.d/trigger/trigger-wprobe.tc
@@ -0,0 +1,48 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# description: event trigger - test wprobe trigger
+# requires: dynamic_events "w[:[<group>/][<event>]] [r|w|rw]@<addr>[:<len>]":README events/sched/sched_process_fork/trigger
+
+echo 0 > tracing_on
+
+:;: "Add a wprobe event used by trigger" ;:
+echo 'w:watch rw@0:8 address=$addr value=+0($addr)' > dynamic_events
+
+:;: "Add events for triggering wprobe" ;:
+echo 'f:truncate do_truncate dentry=$arg2' >> dynamic_events
+echo 'f:dentry_kill dentry_kill dentry=$arg1' >> dynamic_events
+
+:;: "Add wprobe triggers" ;:
+echo 'set_wprobe:watch:dentry' >> events/fprobes/truncate/trigger
+echo 'clear_wprobe:watch:dentry' >> events/fprobes/dentry_kill/trigger
+cat events/fprobes/truncate/trigger | grep ^set_wprobe
+cat events/fprobes/dentry_kill/trigger | grep ^clear_wprobe
+
+:;: "Ensure wprobe is still disabled" ;:
+cat events/wprobes/watch/enable | grep 0
+
+:;: "Enable events for triggers" ;:
+echo 1 >> events/fprobes/truncate/enable
+echo 1 >> events/fprobes/dentry_kill/enable
+
+:;: "Start test workload" ;:
+echo 1 >> tracing_on
+
+echo aaa > /tmp/hoge
+echo bbb > /tmp/hoge
+echo ccc > /tmp/hoge
+rm /tmp/hoge
+
+:;: "Check trace results" ;:
+cat trace | grep watch
+
+:;: "Ensure wprobe becomes disabled again" ;:
+cat events/wprobes/watch/enable | grep 0
+
+:;: "Remove wprobe triggers" ;:
+echo '!set_wprobe:watch:dentry' >> events/fprobes/truncate/trigger
+echo '!clear_wprobe:watch' >> events/fprobes/dentry_kill/trigger
+! grep ^set_wprobe events/fprobes/truncate/trigger
+! grep ^clear_wprobe events/fprobes/dentry_kill/trigger
+
+exit 0
^ permalink raw reply related
* Re: [PATCH] bpf/btf: Move tracing BTF APIs to the BTF library
From: Masami Hiramatsu @ 2026-07-15 1:53 UTC (permalink / raw)
To: Steven Rostedt
Cc: Martin KaFai Lau, Alexei Starovoitov, linux-trace-kernel, bpf,
linux-kernel
In-Reply-To: <20260714103401.0aa3f9e0@gandalf.local.home>
On Tue, 14 Jul 2026 10:34:01 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
>
> Whatever happened to this patch?
IIRC, this got rejected, so we ended up having
to keep maintaining it within the trace.
(because bpf does not use these functions.)
Thanks,
>
> -- Steve
>
>
> On Tue, 10 Oct 2023 22:54:19 +0900
> "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
>
> > From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> >
> > Move the BTF APIs used in tracing to the BTF library code for sharing it
> > with others.
> > Previously, to avoid complex dependency in a series I made it on the
> > tracing tree, but now it is a good time to move it to BPF tree because
> > these functions are pure BTF functions.
> >
> > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > ---
> > include/linux/btf.h | 24 +++++++++
> > kernel/bpf/btf.c | 115 +++++++++++++++++++++++++++++++++++++++++
> > kernel/trace/Makefile | 1
> > kernel/trace/trace_btf.c | 122 --------------------------------------------
> > kernel/trace/trace_btf.h | 11 ----
> > kernel/trace/trace_probe.c | 2 -
> > 6 files changed, 140 insertions(+), 135 deletions(-)
> > delete mode 100644 kernel/trace/trace_btf.c
> > delete mode 100644 kernel/trace/trace_btf.h
> >
> > diff --git a/include/linux/btf.h b/include/linux/btf.h
> > index 928113a80a95..8372d93ea402 100644
> > --- a/include/linux/btf.h
> > +++ b/include/linux/btf.h
> > @@ -507,6 +507,14 @@ btf_get_prog_ctx_type(struct bpf_verifier_log *log, const struct btf *btf,
> > int get_kern_ctx_btf_id(struct bpf_verifier_log *log, enum bpf_prog_type prog_type);
> > bool btf_types_are_same(const struct btf *btf1, u32 id1,
> > const struct btf *btf2, u32 id2);
> > +const struct btf_type *btf_find_func_proto(const char *func_name,
> > + struct btf **btf_p);
> > +const struct btf_param *btf_get_func_param(const struct btf_type *func_proto,
> > + s32 *nr);
> > +const struct btf_member *btf_find_struct_member(struct btf *btf,
> > + const struct btf_type *type,
> > + const char *member_name,
> > + u32 *anon_offset);
> > #else
> > static inline const struct btf_type *btf_type_by_id(const struct btf *btf,
> > u32 type_id)
> > @@ -559,6 +567,22 @@ static inline bool btf_types_are_same(const struct btf *btf1, u32 id1,
> > {
> > return false;
> > }
> > +static inline const struct btf_type *btf_find_func_proto(const char *func_name,
> > + struct btf **btf_p)
> > +{
> > + return NULL;
> > +}
> > +static inline const struct btf_param *
> > +btf_get_func_param(const struct btf_type *func_proto, s32 *nr)
> > +{
> > + return NULL;
> > +}
> > +static inline const struct btf_member *
> > +btf_find_struct_member(struct btf *btf, const struct btf_type *type,
> > + const char *member_name, u32 *anon_offset)
> > +{
> > + return NULL;
> > +}
> > #endif
> >
> > static inline bool btf_type_is_struct_ptr(struct btf *btf, const struct btf_type *t)
> > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> > index 8090d7fb11ef..e5cbf3b31b78 100644
> > --- a/kernel/bpf/btf.c
> > +++ b/kernel/bpf/btf.c
> > @@ -912,6 +912,121 @@ static const struct btf_type *btf_type_skip_qualifiers(const struct btf *btf,
> > return t;
> > }
> >
> > +/*
> > + * Find a function proto type by name, and return the btf_type with its btf
> > + * in *@btf_p. Return NULL if not found.
> > + * Note that caller has to call btf_put(*@btf_p) after using the btf_type.
> > + */
> > +const struct btf_type *btf_find_func_proto(const char *func_name, struct btf **btf_p)
> > +{
> > + const struct btf_type *t;
> > + s32 id;
> > +
> > + id = bpf_find_btf_id(func_name, BTF_KIND_FUNC, btf_p);
> > + if (id < 0)
> > + return NULL;
> > +
> > + /* Get BTF_KIND_FUNC type */
> > + t = btf_type_by_id(*btf_p, id);
> > + if (!t || !btf_type_is_func(t))
> > + goto err;
> > +
> > + /* The type of BTF_KIND_FUNC is BTF_KIND_FUNC_PROTO */
> > + t = btf_type_by_id(*btf_p, t->type);
> > + if (!t || !btf_type_is_func_proto(t))
> > + goto err;
> > +
> > + return t;
> > +err:
> > + btf_put(*btf_p);
> > + return NULL;
> > +}
> > +
> > +/*
> > + * Get function parameter with the number of parameters.
> > + * This can return NULL if the function has no parameters.
> > + * It can return -EINVAL if the @func_proto is not a function proto type.
> > + */
> > +const struct btf_param *btf_get_func_param(const struct btf_type *func_proto, s32 *nr)
> > +{
> > + if (!btf_type_is_func_proto(func_proto))
> > + return ERR_PTR(-EINVAL);
> > +
> > + *nr = btf_type_vlen(func_proto);
> > + if (*nr > 0)
> > + return (const struct btf_param *)(func_proto + 1);
> > + else
> > + return NULL;
> > +}
> > +
> > +#define BTF_ANON_STACK_MAX 16
> > +
> > +struct btf_anon_stack {
> > + u32 tid;
> > + u32 offset;
> > +};
> > +
> > +/*
> > + * Find a member of data structure/union by name and return it.
> > + * Return NULL if not found, or -EINVAL if parameter is invalid.
> > + * If the member is an member of anonymous union/structure, the offset
> > + * of that anonymous union/structure is stored into @anon_offset. Caller
> > + * can calculate the correct offset from the root data structure by
> > + * adding anon_offset to the member's offset.
> > + */
> > +const struct btf_member *btf_find_struct_member(struct btf *btf,
> > + const struct btf_type *type,
> > + const char *member_name,
> > + u32 *anon_offset)
> > +{
> > + struct btf_anon_stack *anon_stack;
> > + const struct btf_member *member;
> > + u32 tid, cur_offset = 0;
> > + const char *name;
> > + int i, top = 0;
> > +
> > + anon_stack = kcalloc(BTF_ANON_STACK_MAX, sizeof(*anon_stack), GFP_KERNEL);
> > + if (!anon_stack)
> > + return ERR_PTR(-ENOMEM);
> > +
> > +retry:
> > + if (!btf_type_is_struct(type)) {
> > + member = ERR_PTR(-EINVAL);
> > + goto out;
> > + }
> > +
> > + for_each_member(i, type, member) {
> > + if (!member->name_off) {
> > + /* Anonymous union/struct: push it for later use */
> > + type = btf_type_skip_modifiers(btf, member->type, &tid);
> > + if (type && top < BTF_ANON_STACK_MAX) {
> > + anon_stack[top].tid = tid;
> > + anon_stack[top++].offset =
> > + cur_offset + member->offset;
> > + }
> > + } else {
> > + name = btf_name_by_offset(btf, member->name_off);
> > + if (name && !strcmp(member_name, name)) {
> > + if (anon_offset)
> > + *anon_offset = cur_offset;
> > + goto out;
> > + }
> > + }
> > + }
> > + if (top > 0) {
> > + /* Pop from the anonymous stack and retry */
> > + tid = anon_stack[--top].tid;
> > + cur_offset = anon_stack[top].offset;
> > + type = btf_type_by_id(btf, tid);
> > + goto retry;
> > + }
> > + member = NULL;
> > +
> > +out:
> > + kfree(anon_stack);
> > + return member;
> > +}
> > +
> > #define BTF_SHOW_MAX_ITER 10
> >
> > #define BTF_KIND_BIT(kind) (1ULL << kind)
> > diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
> > index 057cd975d014..64b61f67a403 100644
> > --- a/kernel/trace/Makefile
> > +++ b/kernel/trace/Makefile
> > @@ -99,7 +99,6 @@ obj-$(CONFIG_KGDB_KDB) += trace_kdb.o
> > endif
> > obj-$(CONFIG_DYNAMIC_EVENTS) += trace_dynevent.o
> > obj-$(CONFIG_PROBE_EVENTS) += trace_probe.o
> > -obj-$(CONFIG_PROBE_EVENTS_BTF_ARGS) += trace_btf.o
> > obj-$(CONFIG_UPROBE_EVENTS) += trace_uprobe.o
> > obj-$(CONFIG_BOOTTIME_TRACING) += trace_boot.o
> > obj-$(CONFIG_FTRACE_RECORD_RECURSION) += trace_recursion_record.o
> > diff --git a/kernel/trace/trace_btf.c b/kernel/trace/trace_btf.c
> > deleted file mode 100644
> > index ca224d53bfdc..000000000000
> > --- a/kernel/trace/trace_btf.c
> > +++ /dev/null
> > @@ -1,122 +0,0 @@
> > -// SPDX-License-Identifier: GPL-2.0
> > -#include <linux/btf.h>
> > -#include <linux/kernel.h>
> > -#include <linux/slab.h>
> > -
> > -#include "trace_btf.h"
> > -
> > -/*
> > - * Find a function proto type by name, and return the btf_type with its btf
> > - * in *@btf_p. Return NULL if not found.
> > - * Note that caller has to call btf_put(*@btf_p) after using the btf_type.
> > - */
> > -const struct btf_type *btf_find_func_proto(const char *func_name, struct btf **btf_p)
> > -{
> > - const struct btf_type *t;
> > - s32 id;
> > -
> > - id = bpf_find_btf_id(func_name, BTF_KIND_FUNC, btf_p);
> > - if (id < 0)
> > - return NULL;
> > -
> > - /* Get BTF_KIND_FUNC type */
> > - t = btf_type_by_id(*btf_p, id);
> > - if (!t || !btf_type_is_func(t))
> > - goto err;
> > -
> > - /* The type of BTF_KIND_FUNC is BTF_KIND_FUNC_PROTO */
> > - t = btf_type_by_id(*btf_p, t->type);
> > - if (!t || !btf_type_is_func_proto(t))
> > - goto err;
> > -
> > - return t;
> > -err:
> > - btf_put(*btf_p);
> > - return NULL;
> > -}
> > -
> > -/*
> > - * Get function parameter with the number of parameters.
> > - * This can return NULL if the function has no parameters.
> > - * It can return -EINVAL if the @func_proto is not a function proto type.
> > - */
> > -const struct btf_param *btf_get_func_param(const struct btf_type *func_proto, s32 *nr)
> > -{
> > - if (!btf_type_is_func_proto(func_proto))
> > - return ERR_PTR(-EINVAL);
> > -
> > - *nr = btf_type_vlen(func_proto);
> > - if (*nr > 0)
> > - return (const struct btf_param *)(func_proto + 1);
> > - else
> > - return NULL;
> > -}
> > -
> > -#define BTF_ANON_STACK_MAX 16
> > -
> > -struct btf_anon_stack {
> > - u32 tid;
> > - u32 offset;
> > -};
> > -
> > -/*
> > - * Find a member of data structure/union by name and return it.
> > - * Return NULL if not found, or -EINVAL if parameter is invalid.
> > - * If the member is an member of anonymous union/structure, the offset
> > - * of that anonymous union/structure is stored into @anon_offset. Caller
> > - * can calculate the correct offset from the root data structure by
> > - * adding anon_offset to the member's offset.
> > - */
> > -const struct btf_member *btf_find_struct_member(struct btf *btf,
> > - const struct btf_type *type,
> > - const char *member_name,
> > - u32 *anon_offset)
> > -{
> > - struct btf_anon_stack *anon_stack;
> > - const struct btf_member *member;
> > - u32 tid, cur_offset = 0;
> > - const char *name;
> > - int i, top = 0;
> > -
> > - anon_stack = kcalloc(BTF_ANON_STACK_MAX, sizeof(*anon_stack), GFP_KERNEL);
> > - if (!anon_stack)
> > - return ERR_PTR(-ENOMEM);
> > -
> > -retry:
> > - if (!btf_type_is_struct(type)) {
> > - member = ERR_PTR(-EINVAL);
> > - goto out;
> > - }
> > -
> > - for_each_member(i, type, member) {
> > - if (!member->name_off) {
> > - /* Anonymous union/struct: push it for later use */
> > - type = btf_type_skip_modifiers(btf, member->type, &tid);
> > - if (type && top < BTF_ANON_STACK_MAX) {
> > - anon_stack[top].tid = tid;
> > - anon_stack[top++].offset =
> > - cur_offset + member->offset;
> > - }
> > - } else {
> > - name = btf_name_by_offset(btf, member->name_off);
> > - if (name && !strcmp(member_name, name)) {
> > - if (anon_offset)
> > - *anon_offset = cur_offset;
> > - goto out;
> > - }
> > - }
> > - }
> > - if (top > 0) {
> > - /* Pop from the anonymous stack and retry */
> > - tid = anon_stack[--top].tid;
> > - cur_offset = anon_stack[top].offset;
> > - type = btf_type_by_id(btf, tid);
> > - goto retry;
> > - }
> > - member = NULL;
> > -
> > -out:
> > - kfree(anon_stack);
> > - return member;
> > -}
> > -
> > diff --git a/kernel/trace/trace_btf.h b/kernel/trace/trace_btf.h
> > deleted file mode 100644
> > index 4bc44bc261e6..000000000000
> > --- a/kernel/trace/trace_btf.h
> > +++ /dev/null
> > @@ -1,11 +0,0 @@
> > -/* SPDX-License-Identifier: GPL-2.0 */
> > -#include <linux/btf.h>
> > -
> > -const struct btf_type *btf_find_func_proto(const char *func_name,
> > - struct btf **btf_p);
> > -const struct btf_param *btf_get_func_param(const struct btf_type *func_proto,
> > - s32 *nr);
> > -const struct btf_member *btf_find_struct_member(struct btf *btf,
> > - const struct btf_type *type,
> > - const char *member_name,
> > - u32 *anon_offset);
> > diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> > index 4dc74d73fc1d..b33c424b8ee0 100644
> > --- a/kernel/trace/trace_probe.c
> > +++ b/kernel/trace/trace_probe.c
> > @@ -12,7 +12,7 @@
> > #define pr_fmt(fmt) "trace_probe: " fmt
> >
> > #include <linux/bpf.h>
> > -#include "trace_btf.h"
> > +#include <linux/btf.h>
> >
> > #include "trace_probe.h"
> >
>
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH] selftests/ftrace: Add test case for a symbol in a module without module name
From: Masami Hiramatsu @ 2026-07-15 1:55 UTC (permalink / raw)
To: Steven Rostedt
Cc: Shuah Khan, Andrii Nakryiko, linux-trace-kernel, bpf,
Francis Laniel, linux-kselftest
In-Reply-To: <20260714104210.28cc4c59@gandalf.local.home>
On Tue, 14 Jul 2026 10:42:10 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
>
> Is this still a think, or can we remove it from patchwork?
>
> https://patchwork.kernel.org/project/linux-trace-kernel/patch/169846405196.88147.17766692778800222203.stgit@devnote2/
Oops, thanks for reminding. Let me check.
>
> -- Steve
>
>
> On Sat, 28 Oct 2023 12:34:12 +0900
> "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
>
> > From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> >
> > Add a test case for probing on a symbol in a module without module name.
> > When probing on a symbol in a module, ftrace accepts both the syntax that
> > <MODNAME>:<SYMBOL> and <SYMBOL>. Current test case only checks the former
> > syntax. This adds a test for the latter one.
> >
> > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > ---
> > .../ftrace/test.d/kprobe/kprobe_module.tc | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc
> > index 7e74ee11edf9..4b32e1b9a8d3 100644
> > --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc
> > +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc
> > @@ -13,6 +13,12 @@ fi
> > MOD=trace_printk
> > FUNC=trace_printk_irq_work
> >
> > +:;: "Add an event on a module function without module name" ;:
> > +
> > +echo "p:event0 $FUNC" > kprobe_events
> > +test -d events/kprobes/event0 || exit_failure
> > +echo "-:kprobes/event0" >> kprobe_events
> > +
> > :;: "Add an event on a module function without specifying event name" ;:
> >
> > echo "p $MOD:$FUNC" > kprobe_events
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH] selftests/ftrace: Add test case for a symbol in a module without module name
From: Masami Hiramatsu @ 2026-07-15 1:58 UTC (permalink / raw)
To: Steven Rostedt
Cc: Shuah Khan, Andrii Nakryiko, linux-trace-kernel, bpf,
Francis Laniel, linux-kselftest
In-Reply-To: <20260714104210.28cc4c59@gandalf.local.home>
OK, this seems good to me, and tested. Let me pick it to probes/for-next.
Thanks!
On Tue, 14 Jul 2026 10:42:10 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
>
> Is this still a think, or can we remove it from patchwork?
>
> https://patchwork.kernel.org/project/linux-trace-kernel/patch/169846405196.88147.17766692778800222203.stgit@devnote2/
>
> -- Steve
>
>
> On Sat, 28 Oct 2023 12:34:12 +0900
> "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
>
> > From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> >
> > Add a test case for probing on a symbol in a module without module name.
> > When probing on a symbol in a module, ftrace accepts both the syntax that
> > <MODNAME>:<SYMBOL> and <SYMBOL>. Current test case only checks the former
> > syntax. This adds a test for the latter one.
> >
> > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > ---
> > .../ftrace/test.d/kprobe/kprobe_module.tc | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc
> > index 7e74ee11edf9..4b32e1b9a8d3 100644
> > --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc
> > +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_module.tc
> > @@ -13,6 +13,12 @@ fi
> > MOD=trace_printk
> > FUNC=trace_printk_irq_work
> >
> > +:;: "Add an event on a module function without module name" ;:
> > +
> > +echo "p:event0 $FUNC" > kprobe_events
> > +test -d events/kprobes/event0 || exit_failure
> > +echo "-:kprobes/event0" >> kprobe_events
> > +
> > :;: "Add an event on a module function without specifying event name" ;:
> >
> > echo "p $MOD:$FUNC" > kprobe_events
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* [PATCH v2] rv: Simplify task monitor slot management
From: Li Qiang @ 2026-07-15 1:58 UTC (permalink / raw)
To: linux-trace-kernel
Cc: rostedt, gmonaco, mhiramat, mathieu.desnoyers, linux-kernel
In-Reply-To: <3ea6ffdd144ce4b38d4a198550afed9516777f85.camel@redhat.com>
The slot array already tracks allocation and task_monitor_count
duplicates that state. On an invalid second release, the old code
warns but still decrements the counter, corrupting later allocations.
Use the slot array as the sole source of truth. Return after warning
about an unused slot, and return -EBUSY when no slot is free.
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
Signed-off-by: Li Qiang <liqiang01@kylinos.cn>
---
kernel/trace/rv/rv.c | 18 +++++-------------
1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/kernel/trace/rv/rv.c b/kernel/trace/rv/rv.c
index ee4e68102f17..187d87d5991c 100644
--- a/kernel/trace/rv/rv.c
+++ b/kernel/trace/rv/rv.c
@@ -164,7 +164,6 @@ struct dentry *get_monitors_root(void)
*/
LIST_HEAD(rv_monitors_list);
-static int task_monitor_count;
static bool task_monitor_slots[CONFIG_RV_PER_TASK_MONITORS];
int rv_get_task_monitor_slot(void)
@@ -173,21 +172,14 @@ int rv_get_task_monitor_slot(void)
lockdep_assert_held(&rv_interface_lock);
- if (task_monitor_count == CONFIG_RV_PER_TASK_MONITORS)
- return -EBUSY;
-
- task_monitor_count++;
-
for (i = 0; i < CONFIG_RV_PER_TASK_MONITORS; i++) {
- if (task_monitor_slots[i] == false) {
+ if (!task_monitor_slots[i]) {
task_monitor_slots[i] = true;
return i;
}
}
- WARN_ONCE(1, "RV task_monitor_count and slots are out of sync\n");
-
- return -EINVAL;
+ return -EBUSY;
}
void rv_put_task_monitor_slot(int slot)
@@ -199,10 +191,10 @@ void rv_put_task_monitor_slot(int slot)
return;
}
- WARN_ONCE(!task_monitor_slots[slot], "RV releasing unused task_monitor_slots: %d\n",
- slot);
+ if (WARN_ONCE(!task_monitor_slots[slot],
+ "RV releasing unused task monitor slot: %d\n", slot))
+ return;
- task_monitor_count--;
task_monitor_slots[slot] = false;
}
--
2.43.0
^ permalink raw reply related
* Re: [RFC PATCH v4 1/3] trace: add lock-free stackmap for stack trace deduplication
From: Li Pengfei @ 2026-07-15 3:12 UTC (permalink / raw)
To: rostedt, mhiramat
Cc: mathieu.desnoyers, mark.rutland, linux-trace-kernel, linux-kernel,
zhangbo56, lipengfei28
In-Reply-To: <20260714171144.4537f163@gandalf.local.home>
From: Pengfei Li <lipengfei28@xiaomi.com>
On Tue, 14 Jul 2026 17:11:44 -0400 Steven Rostedt wrote:
> smap->entries = vcalloc(smap->map_size, sizeof(*smap->entries));
> Make the error paths have: ... goto fail; ...
Will switch both entries and elts to vcalloc(), and collapse the
error paths into a single goto-fail ladder (freeing in reverse
alloc order, including the drops percpu).
> Do not add anonymous blocks in functions.
Will move the cpu declaration to the top of ftrace_stackmap_reset().
> Really should have ftrace_stackmap_bin_entry have a flexible
> array: u64 ips[];
Agreed - will add the u64 ips[] flexible member and use
struct_size(e, ips, nr) in stackmap_bin_open(). On-disk layout is
unchanged, so the bin format stays compatible.
I'll also fold in two issues I found locally: a missing lock on one
init failure path, and TRACE_STACK_ID not being handled in the
function_graph output (it falls through to print_graph_comment()
instead of being punted like TRACE_STACK).
One design point I'd like your steer on before respinning: reset
checks tracer_tracing_is_on() once under trace_types_lock, but
traceon triggers (ftrace_traceon / traceon_trigger) can re-enable
tracing without that lock during reset's clear phase. As far as I
can tell this is a semantic-contract issue, not a memory-safety
one: the map is protected by the resetting flag (get_id bails with
-EINVAL and synchronize_rcu() drains in-flight callers), and the
ring buffer pages aren't freed by reset, so the worst case is a
non-empty / inconsistent buffer after reset rather than corruption.
So I'm leaning toward documenting reset as best-effort ("stop
tracing, including traceon triggers, before reset") rather than
adding machinery to block the window. Does that match your view, or
is there a ring-buffer-state hazard I'm missing that would justify
blocking it explicitly?
On the element pool: your `-l '*lock*'` run is a good illustration -
the ~326K-line stack_map dump is the 16K-entry pool (bits=14)
filling up quickly under broad function tracing. I'm leaning toward
keeping eager allocation for v5 (it keeps the hot path free of an
allocation-failure path), but I'm happy to switch to lazy allocation
on the first 'echo 1 > options/stackmap' if you'd rather not pay the
~8MB resident cost when the option is never enabled. I'd keep the
stack_map_bin interface as-is.
Thanks,
Pengfei
^ permalink raw reply
* [PATCH v3 0/2] Add trace event support for GENI SE registers dump
From: Praveen Talari @ 2026-07-15 5:20 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Mark Brown
Cc: Praveen Talari, linux-kernel, linux-arm-msm, linux-trace-kernel,
linux-spi, Mukesh Kumar Savaliya, Konrad Dybcio
The GENI framework is used by multiple drivers including UART, I2C, and
SPI. When hardware-related failures occur, each driver typically relies
on local logging, which often lacks sufficient information to determine
the exact controller state.
This series introduces a common tracing mechanism for GENI Serial Engine
debug registers and demonstrates its use in the SPI driver.
Patch 1 adds a new tracepoint that captures an extensive set of GENI SE
registers, including command state, interrupt status, FIFO state, DMA
configuration, and clock-related information.
Patch 2 hooks the tracepoint into SPI error paths so that register
snapshots are automatically generated when timeouts or transfer-related
failures occur.
Usage examples:
Enable all I2C traces:
echo 1 > /sys/kernel/tracing/events/qcom_geni_se/enable
cat /sys/kernel/debug/tracing/trace_pipe
Example trace output:
114.291299: geni_se_regs: 888000.spi: m_cmd0=0x18000000
m_irq_status=0x00000080 s_cmd0=0x00000000 s_irq_status=0x08000000
geni_status=0x00000000 geni_ios=0x00000000 m_cmd_ctrl=0x00000000
m_cmd_err=0x00000000 m_fw_err=0x00000000 tx_fifo_sts=0x00000000
rx_fifo_sts=0x00000000 tx_watermark=0x00000000 rx_watermark=0x0000000d
rx_watermark_rfr=0x0000000e m_gp_length=0x00000004 s_gp_length=0x00000000
dma_tx_irq=0x00000000 dma_rx_irq=0x00000000 dma_tx_irq_en=0x0000000f
dma_rx_irq_en=0x0000001f dma_rx_len=0x00001400 dma_rx_len_in=0x00001400
dma_tx_len=0x00001400 dma_tx_len_in=0x00001400 dma_tx_ptr_l=0xffffc000
dma_tx_ptr_h=0x00000000 dma_rx_ptr_l=0xffffa000 dma_rx_ptr_h=0x00000000
dma_tx_attr=0x00000001 dma_tx_max_burst=0x00000002 dma_rx_attr=0x00000000
dma_rx_max_burst=0x00000002 dma_if_en=0x00000009 dma_if_en_ro=0x00000001
dma_general_cfg=0x0000000f dma_qsb_trans_cfg=0x00000000 dma_dbg=0x00000000
m_irq_en=0x7fc0007f s_irq_en=0x03003e3e gsi_event_en=0x00000000
se_irq_en=0x0000000f ser_m_clk_cfg=0x000000a1 ser_s_clk_cfg=0x00000000
general_cfg=0x00000048 output_ctrl=0x0000007f clk_ctrl_ro=0x00000001
fifo_if_dis=0x00000000 fw_multilock_msa=0x00000000 clk_sel=0x00000005
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
Changes in v3:
- Trace se registers before clearing tx watermark reg.
- Updated commit text.
- Link to v2: https://lore.kernel.org/all/20260711-add-tracepoints-for-se-reg-dump-v2-0-ca1e9ba62359@oss.qualcomm.com
Changes in v2:
- Updated SE_DMA_IF_EN to correct value.
- Sorted hearders.
- Link to v1: https://lore.kernel.org/all/20260706-add-tracepoints-for-se-reg-dump-v1-0-48bd08e28cf2@oss.qualcomm.com
To: Bjorn Andersson <andersson@kernel.org>
To: Konrad Dybcio <konradybcio@kernel.org>
To: Steven Rostedt <rostedt@goodmis.org>
To: Masami Hiramatsu <mhiramat@kernel.org>
To: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: Mark Brown <broonie@kernel.org>
Cc: linux-kernel@vger.kernel.org
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-trace-kernel@vger.kernel.org
Cc: linux-spi@vger.kernel.org
Cc: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
---
Praveen Talari (2):
soc: qcom: geni-se: trace: Add trace event support for GENI SE registers dump
spi: qcom-geni: add GENI SE registers trace event on error paths
drivers/soc/qcom/qcom-geni-se.c | 3 +
drivers/spi/spi-geni-qcom.c | 23 +++++-
include/linux/soc/qcom/geni-se.h | 38 +++++++++
include/trace/events/qcom_geni_se.h | 157 ++++++++++++++++++++++++++++++++++++
4 files changed, 217 insertions(+), 4 deletions(-)
---
base-commit: bee763d5f341b99cf472afeb508d4988f62a6ca1
change-id: 20260630-add-tracepoints-for-se-reg-dump-c2c8bc875658
Best regards,
--
Praveen Talari <praveen.talari@oss.qualcomm.com>
^ permalink raw reply
* [PATCH v3 1/2] soc: qcom: geni-se: trace: Add trace event support for GENI SE registers dump
From: Praveen Talari @ 2026-07-15 5:20 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Mark Brown
Cc: Praveen Talari, linux-kernel, linux-arm-msm, linux-trace-kernel,
linux-spi, Mukesh Kumar Savaliya
In-Reply-To: <20260715-add-tracepoints-for-se-reg-dump-v3-0-0f787f93badd@oss.qualcomm.com>
Diagnosing GENI SE-based driver (serial, SPI, I2C) failures currently
requires reading each hardware register individually, either through
ad hoc debug code or a debugger. This is slow, requires the state to
remain stable across the multiple reads, and cannot be run
non-intrusively during normal operation without adding printk-style
noise to each driver.
Add a new trace event header for the Qualcomm GENI Serial Engine (SE)
framework providing a geni_se_regs tracepoint. This tracepoint
captures a comprehensive snapshot of the GENI SE hardware state in a
single trace record, making it possible to correlate register values at
a precise point in time without multiple sequential reads.
The trace event records the following register groups:
- Main/secondary command and IRQ status (M_CMD0, S_CMD0, M/S_IRQ_STATUS)
- Engine status, IOS, and command control/error registers
- TX/RX FIFO status and watermark registers (including RFR watermark)
- M/S GP length registers
- DMA TX/RX IRQ, enable, length, pointer, attribute, and burst registers
- DMA interface enable, general config, QSB trans config, and debug
- M/S IRQ enable, GSI event enable, and top-level SE IRQ enable
- Serial master/slave clock config, general config, output control,
clock control RO, FIFO interface disable, and FW multilock MSA
- Clock select register
Having all these registers captured atomically in a single ftrace record
allows drivers built on top of the GENI SE framework (serial, SPI, I2C)
to invoke this tracepoint on error paths and reconstruct the full engine
state during post-mortem analysis without instrumenting each driver
separately.
Acked-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
drivers/soc/qcom/qcom-geni-se.c | 3 +
include/linux/soc/qcom/geni-se.h | 38 +++++++++
include/trace/events/qcom_geni_se.h | 157 ++++++++++++++++++++++++++++++++++++
3 files changed, 198 insertions(+)
diff --git a/drivers/soc/qcom/qcom-geni-se.c b/drivers/soc/qcom/qcom-geni-se.c
index 15636a8dc907..44bd730baec6 100644
--- a/drivers/soc/qcom/qcom-geni-se.c
+++ b/drivers/soc/qcom/qcom-geni-se.c
@@ -7,6 +7,9 @@
/* Disable MMIO tracing to prevent excessive logging of unwanted MMIO traces */
#define __DISABLE_TRACE_MMIO__
+#define CREATE_TRACE_POINTS
+#include <trace/events/qcom_geni_se.h>
+
#include <linux/acpi.h>
#include <linux/bitfield.h>
#include <linux/clk.h>
diff --git a/include/linux/soc/qcom/geni-se.h b/include/linux/soc/qcom/geni-se.h
index c5e6ab85df09..8c08c1917374 100644
--- a/include/linux/soc/qcom/geni-se.h
+++ b/include/linux/soc/qcom/geni-se.h
@@ -81,13 +81,16 @@ struct geni_se {
};
/* Common SE registers */
+#define GENI_GENERAL_CFG 0x10
#define GENI_FORCE_DEFAULT_REG 0x20
#define GENI_OUTPUT_CTRL 0x24
#define SE_GENI_STATUS 0x40
#define GENI_SER_M_CLK_CFG 0x48
#define GENI_SER_S_CLK_CFG 0x4c
+#define GENI_CLK_CTRL_RO 0x60
#define GENI_IF_DISABLE_RO 0x64
#define GENI_FW_REVISION_RO 0x68
+#define GENI_FW_MULTILOCK_MSA_RO 0x74
#define SE_GENI_CLK_SEL 0x7c
#define SE_GENI_CFG_SEQ_START 0x84
#define SE_GENI_DMA_MODE_EN 0x258
@@ -98,6 +101,8 @@ struct geni_se {
#define SE_GENI_M_IRQ_CLEAR 0x618
#define SE_GENI_M_IRQ_EN_SET 0x61c
#define SE_GENI_M_IRQ_EN_CLEAR 0x620
+#define M_CMD_ERR_STATUS 0x624
+#define M_FW_ERR_STATUS 0x628
#define SE_GENI_S_CMD0 0x630
#define SE_GENI_S_CMD_CTRL_REG 0x634
#define SE_GENI_S_IRQ_STATUS 0x640
@@ -115,15 +120,42 @@ struct geni_se {
#define SE_GENI_IOS 0x908
#define SE_GENI_M_GP_LENGTH 0x910
#define SE_GENI_S_GP_LENGTH 0x914
+/* TX DMA registers */
+#define SE_DMA_TX_PTR_L 0xc30
+#define SE_DMA_TX_PTR_H 0xc34
+#define SE_DMA_TX_ATTR 0xc38
+#define SE_DMA_TX_LEN 0xc3c
#define SE_DMA_TX_IRQ_STAT 0xc40
#define SE_DMA_TX_IRQ_CLR 0xc44
+#define SE_DMA_TX_IRQ_EN 0xc48
+#define SE_DMA_TX_IRQ_EN_SET 0xc4c
+#define SE_DMA_TX_IRQ_EN_CLR 0xc50
+#define SE_DMA_TX_LEN_IN 0xc54
#define SE_DMA_TX_FSM_RST 0xc58
+#define SE_DMA_TX_MAX_BURST 0xc5c
+/* RX DMA registers */
+#define SE_DMA_RX_PTR_L 0xd30
+#define SE_DMA_RX_PTR_H 0xd34
+#define SE_DMA_RX_ATTR 0xd38
+#define SE_DMA_RX_LEN 0xd3c
#define SE_DMA_RX_IRQ_STAT 0xd40
#define SE_DMA_RX_IRQ_CLR 0xd44
+#define SE_DMA_RX_IRQ_EN 0xd48
+#define SE_DMA_RX_IRQ_EN_SET 0xd4c
+#define SE_DMA_RX_IRQ_EN_CLR 0xd50
#define SE_DMA_RX_LEN_IN 0xd54
#define SE_DMA_RX_FSM_RST 0xd58
+#define SE_DMA_RX_MAX_BURST 0xd5c
+/* DMA general / debug registers */
+#define SE_GSI_EVENT_EN 0xe18
+#define SE_IRQ_EN 0xe1c
+#define DMA_IF_EN_RO 0xe20
#define SE_HW_PARAM_0 0xe24
#define SE_HW_PARAM_1 0xe28
+#define DMA_GENERAL_CFG 0xe30
+#define SE_DMA_QSB_TRANS_CFG 0xe38
+#define SE_DMA_DEBUG_REG0 0xe40
+#define SE_DMA_IF_EN 0x2004
/* GENI_FORCE_DEFAULT_REG fields */
#define FORCE_DEFAULT BIT(0)
@@ -269,6 +301,12 @@ struct geni_se {
#define RX_GENI_GP_IRQ_EXT GENMASK(13, 12)
#define RX_GENI_CANCEL_IRQ BIT(14)
+/* SE_DMA_DEBUG_REG0 fields */
+#define DMA_TX_ACTIVE BIT(0)
+#define DMA_RX_ACTIVE BIT(1)
+#define DMA_TX_STATE GENMASK(7, 4)
+#define DMA_RX_STATE GENMASK(11, 8)
+
/* SE_HW_PARAM_0 fields */
#define TX_FIFO_WIDTH_MSK GENMASK(29, 24)
#define TX_FIFO_WIDTH_SHFT 24
diff --git a/include/trace/events/qcom_geni_se.h b/include/trace/events/qcom_geni_se.h
new file mode 100644
index 000000000000..4a6e1ba2d147
--- /dev/null
+++ b/include/trace/events/qcom_geni_se.h
@@ -0,0 +1,157 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM qcom_geni_se
+
+#if !defined(_TRACE_QCOM_GENI_SE_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_QCOM_GENI_SE_H
+
+#include <linux/io.h>
+#include <linux/tracepoint.h>
+#include <linux/soc/qcom/geni-se.h>
+
+TRACE_EVENT(geni_se_regs,
+ TP_PROTO(struct geni_se *se),
+
+ TP_ARGS(se),
+
+ TP_STRUCT__entry(__string(geni_se_name, dev_name(se->dev))
+ __field(u32, geni_se_m_cmd0)
+ __field(u32, geni_se_m_irq_status)
+ __field(u32, geni_se_s_cmd0)
+ __field(u32, geni_se_s_irq_status)
+ __field(u32, geni_se_status)
+ __field(u32, geni_se_ios)
+ __field(u32, geni_se_m_cmd_ctrl)
+ __field(u32, geni_se_m_cmd_err)
+ __field(u32, geni_se_m_fw_err)
+ __field(u32, geni_se_tx_fifo_status)
+ __field(u32, geni_se_rx_fifo_status)
+ __field(u32, geni_se_tx_watermark)
+ __field(u32, geni_se_rx_watermark)
+ __field(u32, geni_se_rx_watermark_rfr)
+ __field(u32, geni_se_m_gp_length)
+ __field(u32, geni_se_s_gp_length)
+ __field(u32, geni_se_dma_tx_irq)
+ __field(u32, geni_se_dma_rx_irq)
+ __field(u32, geni_se_dma_tx_irq_en)
+ __field(u32, geni_se_dma_rx_irq_en)
+ __field(u32, geni_se_dma_rx_len)
+ __field(u32, geni_se_dma_rx_len_in)
+ __field(u32, geni_se_dma_tx_len)
+ __field(u32, geni_se_dma_tx_len_in)
+ __field(u32, geni_se_dma_tx_ptr_l)
+ __field(u32, geni_se_dma_tx_ptr_h)
+ __field(u32, geni_se_dma_rx_ptr_l)
+ __field(u32, geni_se_dma_rx_ptr_h)
+ __field(u32, geni_se_dma_tx_attr)
+ __field(u32, geni_se_dma_tx_max_burst)
+ __field(u32, geni_se_dma_rx_attr)
+ __field(u32, geni_se_dma_rx_max_burst)
+ __field(u32, geni_se_dma_if_en)
+ __field(u32, geni_se_dma_if_en_ro)
+ __field(u32, geni_se_dma_general_cfg)
+ __field(u32, geni_se_dma_qsb_trans_cfg)
+ __field(u32, geni_se_dma_dbg)
+ __field(u32, geni_se_m_irq_en)
+ __field(u32, geni_se_s_irq_en)
+ __field(u32, geni_se_gsi_event_en)
+ __field(u32, geni_se_irq_en)
+ __field(u32, geni_se_ser_m_clk_cfg)
+ __field(u32, geni_se_ser_s_clk_cfg)
+ __field(u32, geni_se_general_cfg)
+ __field(u32, geni_se_output_ctrl)
+ __field(u32, geni_se_clk_ctrl_ro)
+ __field(u32, geni_se_fifo_if_disable)
+ __field(u32, geni_se_fw_multilock_msa)
+ __field(u32, geni_se_clk_sel)
+ ),
+
+ TP_fast_assign(__assign_str(geni_se_name);
+ __entry->geni_se_m_cmd0 = readl(se->base + SE_GENI_M_CMD0);
+ __entry->geni_se_m_irq_status = readl(se->base + SE_GENI_M_IRQ_STATUS);
+ __entry->geni_se_s_cmd0 = readl(se->base + SE_GENI_S_CMD0);
+ __entry->geni_se_s_irq_status = readl(se->base + SE_GENI_S_IRQ_STATUS);
+ __entry->geni_se_status = readl(se->base + SE_GENI_STATUS);
+ __entry->geni_se_ios = readl(se->base + SE_GENI_IOS);
+ __entry->geni_se_m_cmd_ctrl = readl(se->base + SE_GENI_M_CMD_CTRL_REG);
+ __entry->geni_se_m_cmd_err = readl(se->base + M_CMD_ERR_STATUS);
+ __entry->geni_se_m_fw_err = readl(se->base + M_FW_ERR_STATUS);
+ __entry->geni_se_tx_fifo_status = readl(se->base + SE_GENI_TX_FIFO_STATUS);
+ __entry->geni_se_rx_fifo_status = readl(se->base + SE_GENI_RX_FIFO_STATUS);
+ __entry->geni_se_tx_watermark = readl(se->base + SE_GENI_TX_WATERMARK_REG);
+ __entry->geni_se_rx_watermark = readl(se->base + SE_GENI_RX_WATERMARK_REG);
+ __entry->geni_se_rx_watermark_rfr = readl(se->base + SE_GENI_RX_RFR_WATERMARK_REG);
+ __entry->geni_se_m_gp_length = readl(se->base + SE_GENI_M_GP_LENGTH);
+ __entry->geni_se_s_gp_length = readl(se->base + SE_GENI_S_GP_LENGTH);
+ __entry->geni_se_dma_tx_irq = readl(se->base + SE_DMA_TX_IRQ_STAT);
+ __entry->geni_se_dma_rx_irq = readl(se->base + SE_DMA_RX_IRQ_STAT);
+ __entry->geni_se_dma_tx_irq_en = readl(se->base + SE_DMA_TX_IRQ_EN);
+ __entry->geni_se_dma_rx_irq_en = readl(se->base + SE_DMA_RX_IRQ_EN);
+ __entry->geni_se_dma_rx_len = readl(se->base + SE_DMA_RX_LEN);
+ __entry->geni_se_dma_rx_len_in = readl(se->base + SE_DMA_RX_LEN_IN);
+ __entry->geni_se_dma_tx_len = readl(se->base + SE_DMA_TX_LEN);
+ __entry->geni_se_dma_tx_len_in = readl(se->base + SE_DMA_TX_LEN_IN);
+ __entry->geni_se_dma_tx_ptr_l = readl(se->base + SE_DMA_TX_PTR_L);
+ __entry->geni_se_dma_tx_ptr_h = readl(se->base + SE_DMA_TX_PTR_H);
+ __entry->geni_se_dma_rx_ptr_l = readl(se->base + SE_DMA_RX_PTR_L);
+ __entry->geni_se_dma_rx_ptr_h = readl(se->base + SE_DMA_RX_PTR_H);
+ __entry->geni_se_dma_tx_attr = readl(se->base + SE_DMA_TX_ATTR);
+ __entry->geni_se_dma_tx_max_burst = readl(se->base + SE_DMA_TX_MAX_BURST);
+ __entry->geni_se_dma_rx_attr = readl(se->base + SE_DMA_RX_ATTR);
+ __entry->geni_se_dma_rx_max_burst = readl(se->base + SE_DMA_RX_MAX_BURST);
+ __entry->geni_se_dma_if_en = readl(se->base + SE_DMA_IF_EN);
+ __entry->geni_se_dma_if_en_ro = readl(se->base + DMA_IF_EN_RO);
+ __entry->geni_se_dma_general_cfg = readl(se->base + DMA_GENERAL_CFG);
+ __entry->geni_se_dma_qsb_trans_cfg = readl(se->base + SE_DMA_QSB_TRANS_CFG);
+ __entry->geni_se_dma_dbg = readl(se->base + SE_DMA_DEBUG_REG0);
+ __entry->geni_se_m_irq_en = readl(se->base + SE_GENI_M_IRQ_EN);
+ __entry->geni_se_s_irq_en = readl(se->base + SE_GENI_S_IRQ_EN);
+ __entry->geni_se_gsi_event_en = readl(se->base + SE_GSI_EVENT_EN);
+ __entry->geni_se_irq_en = readl(se->base + SE_IRQ_EN);
+ __entry->geni_se_ser_m_clk_cfg = readl(se->base + GENI_SER_M_CLK_CFG);
+ __entry->geni_se_ser_s_clk_cfg = readl(se->base + GENI_SER_S_CLK_CFG);
+ __entry->geni_se_general_cfg = readl(se->base + GENI_GENERAL_CFG);
+ __entry->geni_se_output_ctrl = readl(se->base + GENI_OUTPUT_CTRL);
+ __entry->geni_se_clk_ctrl_ro = readl(se->base + GENI_CLK_CTRL_RO);
+ __entry->geni_se_fifo_if_disable = readl(se->base + GENI_IF_DISABLE_RO);
+ __entry->geni_se_fw_multilock_msa = readl(se->base + GENI_FW_MULTILOCK_MSA_RO);
+ __entry->geni_se_clk_sel = readl(se->base + SE_GENI_CLK_SEL);
+ ),
+
+ TP_printk("%s: m_cmd0=0x%08x m_irq_status=0x%08x s_cmd0=0x%08x s_irq_status=0x%08x geni_status=0x%08x geni_ios=0x%08x m_cmd_ctrl=0x%08x m_cmd_err=0x%08x m_fw_err=0x%08x tx_fifo_sts=0x%08x rx_fifo_sts=0x%08x tx_watermark=0x%08x rx_watermark=0x%08x rx_watermark_rfr=0x%08x m_gp_length=0x%08x s_gp_length=0x%08x dma_tx_irq=0x%08x dma_rx_irq=0x%08x dma_tx_irq_en=0x%08x dma_rx_irq_en=0x%08x dma_rx_len=0x%08x dma_rx_len_in=0x%08x dma_tx_len=0x%08x dma_tx_len_in=0x%08x dma_tx_ptr_l=0x%08x dma_tx_ptr_h=0x%08x dma_rx_ptr_l=0x%08x dma_rx_ptr_h=0x%08x dma_tx_attr=0x%08x dma_tx_max_burst=0x%08x dma_rx_attr=0x%08x dma_rx_max_burst=0x%08x dma_if_en=0x%08x dma_if_en_ro=0x%08x dma_general_cfg=0x%08x dma_qsb_trans_cfg=0x%08x dma_dbg=0x%08x m_irq_en=0x%08x s_irq_en=0x%08x gsi_event_en=0x%08x se_irq_en=0x%08x ser_m_clk_cfg=0x%08x ser_s_clk_cfg=0x%08x general_cfg=0x%08x output_ctrl=0x%08x clk_ctrl_ro=0x%08x fifo_if_dis=0x%08x fw_multilock_msa=0x%08x clk_sel=0x%08x",
+ __get_str(geni_se_name),
+ __entry->geni_se_m_cmd0, __entry->geni_se_m_irq_status,
+ __entry->geni_se_s_cmd0, __entry->geni_se_s_irq_status,
+ __entry->geni_se_status, __entry->geni_se_ios,
+ __entry->geni_se_m_cmd_ctrl,
+ __entry->geni_se_m_cmd_err, __entry->geni_se_m_fw_err,
+ __entry->geni_se_tx_fifo_status, __entry->geni_se_rx_fifo_status,
+ __entry->geni_se_tx_watermark, __entry->geni_se_rx_watermark,
+ __entry->geni_se_rx_watermark_rfr,
+ __entry->geni_se_m_gp_length, __entry->geni_se_s_gp_length,
+ __entry->geni_se_dma_tx_irq, __entry->geni_se_dma_rx_irq,
+ __entry->geni_se_dma_tx_irq_en, __entry->geni_se_dma_rx_irq_en,
+ __entry->geni_se_dma_rx_len, __entry->geni_se_dma_rx_len_in,
+ __entry->geni_se_dma_tx_len, __entry->geni_se_dma_tx_len_in,
+ __entry->geni_se_dma_tx_ptr_l, __entry->geni_se_dma_tx_ptr_h,
+ __entry->geni_se_dma_rx_ptr_l, __entry->geni_se_dma_rx_ptr_h,
+ __entry->geni_se_dma_tx_attr, __entry->geni_se_dma_tx_max_burst,
+ __entry->geni_se_dma_rx_attr, __entry->geni_se_dma_rx_max_burst,
+ __entry->geni_se_dma_if_en, __entry->geni_se_dma_if_en_ro,
+ __entry->geni_se_dma_general_cfg, __entry->geni_se_dma_qsb_trans_cfg,
+ __entry->geni_se_dma_dbg,
+ __entry->geni_se_m_irq_en, __entry->geni_se_s_irq_en,
+ __entry->geni_se_gsi_event_en, __entry->geni_se_irq_en,
+ __entry->geni_se_ser_m_clk_cfg, __entry->geni_se_ser_s_clk_cfg,
+ __entry->geni_se_general_cfg, __entry->geni_se_output_ctrl,
+ __entry->geni_se_clk_ctrl_ro, __entry->geni_se_fifo_if_disable,
+ __entry->geni_se_fw_multilock_msa, __entry->geni_se_clk_sel)
+);
+
+#endif /* _TRACE_QCOM_GENI_SE_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
--
2.34.1
^ permalink raw reply related
* [PATCH v3 2/2] spi: qcom-geni: add GENI SE registers trace event on error paths
From: Praveen Talari @ 2026-07-15 5:20 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Mark Brown
Cc: Praveen Talari, linux-kernel, linux-arm-msm, linux-trace-kernel,
linux-spi, Mukesh Kumar Savaliya, Konrad Dybcio
In-Reply-To: <20260715-add-tracepoints-for-se-reg-dump-v3-0-0f787f93badd@oss.qualcomm.com>
The GENI SPI driver reports various transfer failures such as command
timeouts, DMA reset timeouts, DMA transaction errors, and unexpected
interrupt conditions. However, diagnosing the root cause of these
failures is difficult as the hardware state is not captured when the
error occurs.
Add trace_geni_se_regs() calls at critical SPI error handling paths to
automatically capture GENI serial engine debug registers when failures
are detected. This includes:
- M_CMD abort/cancel timeout
- DMA TX/RX FSM reset timeout
- DMA transaction failures and pending residue conditions
- Unexpected interrupt error status
- Premature transfer completion with pending TX/RX data
Dumping the SE debug registers at the time of failure provides
additional hardware context and significantly improves post-mortem
analysis of SPI transfer issues without affecting normal operation.
Acked-by: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
drivers/spi/spi-geni-qcom.c | 23 +++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index 2914d781dbf5..8528f9b80f03 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -1,6 +1,8 @@
// SPDX-License-Identifier: GPL-2.0
// Copyright (c) 2017-2018, The Linux foundation. All rights reserved.
+#include <trace/events/qcom_geni_se.h>
+
#define CREATE_TRACE_POINTS
#include <trace/events/qcom_geni_spi.h>
@@ -192,6 +194,7 @@ static void handle_se_timeout(struct spi_controller *spi)
time_left = wait_for_completion_timeout(&mas->abort_done, HZ);
if (!time_left) {
dev_err(mas->dev, "Failed to cancel/abort m_cmd\n");
+ trace_geni_se_regs(se);
/*
* No need for a lock since SPI core has a lock and we never
@@ -209,8 +212,10 @@ static void handle_se_timeout(struct spi_controller *spi)
writel(1, se->base + SE_DMA_TX_FSM_RST);
spin_unlock_irq(&mas->lock);
time_left = wait_for_completion_timeout(&mas->tx_reset_done, HZ);
- if (!time_left)
+ if (!time_left) {
dev_err(mas->dev, "DMA TX RESET failed\n");
+ trace_geni_se_regs(se);
+ }
}
if (xfer->rx_buf) {
spin_lock_irq(&mas->lock);
@@ -218,8 +223,10 @@ static void handle_se_timeout(struct spi_controller *spi)
writel(1, se->base + SE_DMA_RX_FSM_RST);
spin_unlock_irq(&mas->lock);
time_left = wait_for_completion_timeout(&mas->rx_reset_done, HZ);
- if (!time_left)
+ if (!time_left) {
dev_err(mas->dev, "DMA RX RESET failed\n");
+ trace_geni_se_regs(se);
+ }
}
} else {
/*
@@ -391,10 +398,12 @@ static void
spi_gsi_callback_result(void *cb, const struct dmaengine_result *result)
{
struct spi_controller *spi = cb;
+ struct spi_geni_master *mas = spi_controller_get_devdata(spi);
spi->cur_msg->status = -EIO;
if (result->result != DMA_TRANS_NOERROR) {
dev_err(&spi->dev, "DMA txn failed: %d\n", result->result);
+ trace_geni_se_regs(&mas->se);
spi_finalize_current_transfer(spi);
return;
}
@@ -404,6 +413,7 @@ spi_gsi_callback_result(void *cb, const struct dmaengine_result *result)
dev_dbg(&spi->dev, "DMA txn completed\n");
} else {
dev_err(&spi->dev, "DMA xfer has pending: %d\n", result->residue);
+ trace_geni_se_regs(&mas->se);
}
spi_finalize_current_transfer(spi);
@@ -953,8 +963,10 @@ static irqreturn_t geni_spi_isr(int irq, void *data)
if (m_irq & (M_CMD_OVERRUN_EN | M_ILLEGAL_CMD_EN | M_CMD_FAILURE_EN |
M_RX_FIFO_RD_ERR_EN | M_RX_FIFO_WR_ERR_EN |
- M_TX_FIFO_RD_ERR_EN | M_TX_FIFO_WR_ERR_EN))
+ M_TX_FIFO_RD_ERR_EN | M_TX_FIFO_WR_ERR_EN)) {
dev_warn(mas->dev, "Unexpected IRQ err status %#010x\n", m_irq);
+ trace_geni_se_regs(se);
+ }
spin_lock(&mas->lock);
@@ -983,13 +995,16 @@ static irqreturn_t geni_spi_isr(int irq, void *data)
* weren't written correctly.
*/
if (mas->tx_rem_bytes) {
+ trace_geni_se_regs(se);
writel(0, se->base + SE_GENI_TX_WATERMARK_REG);
dev_err(mas->dev, "Premature done. tx_rem = %d bpw%d\n",
mas->tx_rem_bytes, mas->cur_bits_per_word);
}
- if (mas->rx_rem_bytes)
+ if (mas->rx_rem_bytes) {
dev_err(mas->dev, "Premature done. rx_rem = %d bpw%d\n",
mas->rx_rem_bytes, mas->cur_bits_per_word);
+ trace_geni_se_regs(se);
+ }
} else {
complete(&mas->cs_done);
}
--
2.34.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox