* [PATCH v8 5/9] x86/hw_breakpoint: Unify breakpoint install/uninstall
From: Masami Hiramatsu (Google) @ 2026-07-16 2:53 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: <178417033089.209165.16717079876036408877.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.
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Changes in v8:
- Add missing barrier() on the disable path of setup_hwbp() to prevent
the compiler from reordering this_cpu_write(cpu_dr7, ...) before
set_debugreg(dr7, 7).
- Clear existing slot control and enable bits in cpu_dr7 inside
setup_hwbp() using __encode_dr7() before setting new ones to prevent
register state corruption on update/reinstall.
---
arch/x86/include/asm/hw_breakpoint.h | 6 +
arch/x86/kernel/hw_breakpoint.c | 144 +++++++++++++++++++---------------
2 files changed, 86 insertions(+), 64 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..c323c2aab2af 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,113 @@ 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;
+ }
- set_debugreg(info->address, i);
- __this_cpu_write(cpu_debugreg[i], info->address);
+ WARN_ONCE(1, "No free breakpoint slots");
+ return -EBUSY;
+}
- dr7 = this_cpu_ptr(&cpu_dr7);
- *dr7 |= encode_dr7(i, info->len, info->type);
+static void setup_hwbp(struct arch_hw_breakpoint *info, int slot, bool enable)
+{
+ unsigned long dr7;
+
+ set_debugreg(info->address, slot);
+ __this_cpu_write(cpu_debugreg[slot], info->address);
+
+ dr7 = this_cpu_read(cpu_dr7);
+ dr7 &= ~(__encode_dr7(slot, 0xc, 0x3) |
+ (DR_LOCAL_ENABLE << (slot * DR_ENABLE_SIZE)));
+ if (enable)
+ 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);
- if (info->mask)
- amd_set_dr_addr_mask(info->mask, i);
+ set_debugreg(dr7, 7);
+
+ amd_set_dr_addr_mask(enable ? info->mask : 0, slot);
- return 0;
+ barrier();
+
+ /*
+ * 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;
- }
- }
+ slot = manage_bp_slot(bp, action);
+ if (slot < 0)
+ return slot;
- if (WARN_ONCE(i == HBP_NUM, "Can't find any breakpoint slot"))
- return;
+ info = counter_arch_bp(bp);
+ setup_hwbp(info, slot, action != BP_SLOT_ACTION_UNINSTALL);
- dr7 = this_cpu_read(cpu_dr7);
- dr7 &= ~__encode_dr7(i, info->len, info->type);
-
- 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 v8 4/9] selftests: tracing: Add syntax testcase for wprobe
From: Masami Hiramatsu (Google) @ 2026-07-16 2:52 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: <178417033089.209165.16717079876036408877.stgit@devnote2>
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Add "wprobe_syntax_errors.tc" testcase for testing syntax errors
of the watch probe events.
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 v8 3/9] selftests: tracing: Add a basic testcase for wprobe
From: Masami Hiramatsu (Google) @ 2026-07-16 2:52 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: <178417033089.209165.16717079876036408877.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.
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Changes in v8:
- Fixed silently test failure path.
---
tools/testing/selftests/ftrace/config | 1
.../ftrace/test.d/dynevent/add_remove_wprobe.tc | 69 ++++++++++++++++++++
2 files changed, 70 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..5ddf28ddd2c4
--- /dev/null
+++ b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_wprobe.tc
@@ -0,0 +1,69 @@
+#!/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"
+ exit_fail
+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 v8 2/9] x86: hw_breakpoint: Add a kconfig to clarify when a breakpoint fires
From: Masami Hiramatsu (Google) @ 2026-07-16 2:52 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: <178417033089.209165.16717079876036408877.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 v8 1/9] tracing: wprobe: Add watchpoint probe event based on hardware breakpoint
From: Masami Hiramatsu (Google) @ 2026-07-16 2:52 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: <178417033089.209165.16717079876036408877.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 and $value to fetch
the accessed memory value (shorthand for '+0($addr)'). No other variables
are supported.
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)
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Changes in v8:
- Include required header files.
- Use READ_ONCE(tw->addr) in trace handler to safely check dynamically
updated addresses.
- Prohibit unsafe perf support by returning -EOPNOTSUPP in
wprobe_register().
- Add rollback logic to unregister already-enabled sibling probes if
registration fails mid-loop.
- Resolve symbol offsets dynamically in trace_wprobe_show() using
kallsyms_lookup_name().
- Fix memory leak of parse_address_spec()'s symbol output in
__trace_wprobe_create().
- Print "rw" instead of "x" for read-write type breakpoints in
trace_wprobe_show().
- Document the $value fetcharg in wprobetrace.rst.
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 | 70 +++
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 | 709 +++++++++++++++++++++++++++++++++++
10 files changed, 837 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..eb4f10607530
--- /dev/null
+++ b/Documentation/trace/wprobetrace.rst
@@ -0,0 +1,70 @@
+.. 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.
+ $value : Fetch the memory value at the accessing address (same as +0($addr)).
+ @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..942ad2810d36
--- /dev/null
+++ b/kernel/trace/trace_wprobe.c
@@ -0,0 +1,709 @@
+// 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/compiler.h>
+#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"
+#include "trace_output.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 (READ_ONCE(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) {
+ struct trace_wprobe *tmp;
+
+ list_for_each_entry(tmp, trace_probe_probe_list(tp), tp.list) {
+ if (tmp == tw)
+ break;
+ __unregister_trace_wprobe(tmp);
+ }
+ if (file)
+ trace_probe_remove_file(tp, file);
+ else
+ trace_probe_clear_flag(tp, TP_FLAG_PROFILE);
+ 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:
+ case TRACE_REG_PERF_UNREGISTER:
+ case TRACE_REG_PERF_OPEN:
+ case TRACE_REG_PERF_CLOSE:
+ case TRACE_REG_PERF_ADD:
+ case TRACE_REG_PERF_DEL:
+ return -EOPNOTSUPP;
+#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 __free(kfree) = 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));
+
+ const char *type_str;
+
+ if (tw->type == HW_BREAKPOINT_R)
+ type_str = "r";
+ else if (tw->type == HW_BREAKPOINT_W)
+ type_str = "w";
+ else
+ type_str = "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) {
+ unsigned long sym_addr = kallsyms_lookup_name(tw->symbol);
+ long offset = sym_addr ? (long)(tw->addr - sym_addr) : 0;
+
+ if (offset)
+ seq_printf(m, " %s@%s%+ld:%d", type_str, tw->symbol, offset, len);
+ else
+ seq_printf(m, " %s@%s:%d", type_str, tw->symbol, len);
+ } else {
+ seq_printf(m, " %s@0x%lx:%d", type_str, 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 v8 0/9] tracing: wprobe: x86: Add wprobe for watchpoint
From: Masami Hiramatsu (Google) @ 2026-07-16 2:52 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 8th 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/178407983818.95826.12714571928538799781.stgit@devnote2/
This version fixes many issues commented by Sashiko. Let me list it up.
[PATCH 1/9]
- Include required header files.
- Use READ_ONCE(tw->addr) in trace handler to safely check dynamically
updated addresses.
- Prohibit unsafe perf support by returning -EOPNOTSUPP in
wprobe_register().
- Add rollback logic to unregister already-enabled sibling probes if
registration fails mid-loop.
- Resolve symbol offsets dynamically in trace_wprobe_show() using
kallsyms_lookup_name().
- Fix memory leak of parse_address_spec()'s symbol output in
__trace_wprobe_create().
- Print "rw" instead of "x" for read-write type breakpoints in
trace_wprobe_show().
- Document the $value fetcharg in wprobetrace.rst.
[PATCH 3/9]
- Fixed silently test failure path.
[PATCH 5/9]
- Add missing barrier() on the disable path of setup_hwbp() to prevent
the compiler from reordering this_cpu_write(cpu_dr7, ...) before
set_debugreg(dr7, 7).
- Clear existing slot control and enable bits in cpu_dr7 inside
setup_hwbp() using __encode_dr7() before setting new ones to prevent
register state corruption on update/reinstall.
[PATCH 7/9]
- Parse new attributes into a temporary struct arch_hw_breakpoint
copy to prevent in-place mutation and corruption on parse error paths.
- Synchronize logical perf_event attributes by updating
bp->attr.bp_type and bp->attr.bp_len in modify_wide_hw_breakpoint_local().
[PATCH 8/9]
- Redesign wprobe_trigger() to be safe in NMI/hardirq contexts by
deferring register updates to a workqueue via irq_work.
- Skip trigger execution and increment an atomic missed count
(tw->missed) if the tracepoint runs in NMI context to prevent
recursive spinlock deadlocks. (this is currently hidden counter)
- Prohibit attaching wprobe triggers to kprobe_events by checking
TRACE_EVENT_FL_KPROBE in wprobe_trigger_cmd_parse().
- Use call_rcu() in trigger deactivation path and add
synchronize_rcu() in parse failure path to ensure safe RCU
lifetime cleanup.
- Acquire the target tracepoint's module reference via
trace_event_try_get_ref() to prevent module refcount underflows.
- Fix event_trigger_data memory leak by properly freeing the
initial refcount in wprobe_trigger_cmd_parse() on success.
- Call on_each_cpu() with wait=true in wprobe_work_func() to
prevent use-after-free during trigger unregistration.
- Synchronize concurrent wprobe triggers on the same event by
using a shared raw spinlock (tw->lock).
- Drop the support of kprobe events (that should be done later).
[PATCH 9/9]
- Use TMPDIR for making a test file.
- Ensure the fprobe and target functions exists, if not, exit
with UNRESOLVED (== skip).
To support kprobe events, we need to check the previous NMI context
or need to use an atomic refcount etc. But that should be another
patch for review.
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: 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
Thank you,
---
Jinchao Wang (2):
x86/hw_breakpoint: Unify breakpoint install/uninstall
x86/hw_breakpoint: Add arch_reinstall_hw_breakpoint
Masami Hiramatsu (Google) (7):
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
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 | 163 +++
arch/Kconfig | 20
arch/x86/Kconfig | 2
arch/x86/include/asm/hw_breakpoint.h | 8
arch/x86/kernel/hw_breakpoint.c | 151 ++-
include/linux/hw_breakpoint.h | 6
include/linux/trace_events.h | 3
kernel/events/hw_breakpoint.c | 43 +
kernel/trace/Kconfig | 24
kernel/trace/Makefile | 1
kernel/trace/trace.c | 9
kernel/trace/trace.h | 6
kernel/trace/trace_probe.c | 22
kernel/trace/trace_probe.h | 8
kernel/trace/trace_wprobe.c | 1146 ++++++++++++++++++++
tools/testing/selftests/ftrace/config | 2
.../ftrace/test.d/dynevent/add_remove_wprobe.tc | 69 +
.../test.d/dynevent/wprobes_syntax_errors.tc | 20
.../ftrace/test.d/trigger/trigger-wprobe.tc | 65 +
20 files changed, 1703 insertions(+), 66 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
* Re: [PATCH v7 10/10] selftests: ftrace: Add wprobe trigger testcase
From: Masami Hiramatsu @ 2026-07-16 2:37 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: sashiko-reviews, sashiko-bot, linux-perf-users,
Linux Trace Kernel, Steven Rostedt
In-Reply-To: <20260716105913.e04dd8dfa271a263f959d847@kernel.org>
Oops, sorry for confusion, this is for [9/10].
Thanks,
On Thu, 16 Jul 2026 10:59:13 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> On Wed, 15 Jul 2026 02:05:55 +0000
> sashiko-bot@kernel.org wrote:
>
> > Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
> > - [Critical] Deadlocks due to NMI-unsafe operations (schedule_work and raw_spin_lock_irqsave) in tracepoint context.
> > - [High] Missing cancel_work_sync() causes a Use-After-Free of wprobe_trigger_data via the workqueue.
> > - [High] Synchronous free of wprobe_data violates RCU removal ordering, risking concurrent use-after-free.
> > - [High] Missing trace_event_try_get_ref() and imbalanced trace_event_put_ref() cause module refcount underflow.
> > - [High] on_each_cpu is called with wait=false for an IPI that references wprobe_data.
> > - [High] Immediate kfree() of RCU-protected trigger_data and wprobe_data on the error path.
> > - [Medium] Permanent memory leak of wprobe trigger objects on successful registration.
> > - [Medium] tw->addr is read and written concurrently by different triggers without a shared lock.
> > --
> >
>
> Thanks for the review comments.
> BTW, why this only to linux-perf-users? (Maybe I need to modify Sashiko setting)
>
> > --- Patch [9]: [PATCH v7 09/10] tracing: wprobe: Add wprobe event trigger ---
> > The format of this report is altered due to recitation restrictions.
> > I am providing a free-form summary of the findings rather than inline
> > quoting the patch.
> >
> > Commit: 4087513c82b11b98d27cfc4452df9a1a8e41ddd5
> > Author: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > Subject: tracing: wprobe: Add wprobe event trigger
> >
> > This patch adds a wprobe event trigger to set and clear watch events
> > dynamically, allowing watchpoints on local variables and slab objects.
> >
> > [Severity: Critical]
> > Is it safe to call raw_spin_lock_irqsave() and schedule_work() directly
> > from wprobe_trigger()? Tracepoints can execute in arbitrary contexts,
> > including NMI or with scheduler locks held. Calling schedule_work() is
> > unsafe in these contexts. Additionally, acquiring wprobe_data->lock here
> > can cause a hard deadlock if the tracepoint interrupts wprobe_smp_update_func()
> > while it is holding the exact same lock in IPI context.
>
> Hmm, good catch!
> OK, use a pair of irq_work and work to call on_each_cpu() for some
> interrupt context.
> For NMI context, I think wprobe_trigger should check the original state
> is in_nmi(). This can work for normal trace events or fprobe, but not
> for kprobes, because kprobe handler itself is working inside NMI context.
> Hmm, let me check how can I fix this. Until solving this problem, this
> wprobe trigger may not work with kprobes.
> Also, I found the lock should protect the wprobe itself, not data.
> Let me move the irq_work and lock to trace_wprobe.
>
> Hmm, we may need to pass to original context (calculated by preempt_count)
> to trigger event so that it can evaluate the original context.
> (BTW, )
>
> > [Severity: High]
> > Does free_wprobe_trigger_data() safely handle the embedded work struct?
> > When a trigger is unregistered, this function frees wprobe_data via kfree().
> > If wprobe_trigger() recently scheduled wprobe_data->work, the workqueue
> > could execute wprobe_work_func() on the freed memory since there is no
> > cancel_work_sync() before freeing.
>
> OK, so move the worker to wprobe because anyway the worker will update
> the wprobe itself.
>
> >
> > [Severity: High]
> > Is an RCU grace period needed in wprobe_trigger_free()?
> > wprobe_unregister_trigger() unlinks the trigger and calls
> > wprobe_trigger_free(), which synchronously frees wprobe_data. Since
> > tracepoints execute under RCU, could a concurrent tracepoint execution
> > still be accessing wprobe_data when it gets freed?
>
> Indeed, let's use call_rcu().
>
> >
> > [Severity: High]
> > Can the error path in wprobe_trigger_cmd_parse() cause a use-after-free?
> > If event_trigger_register() successfully exposes the trigger to RCU
> > readers but a subsequent step fails, returning an error causes the
> > __free(kfree) cleanup to immediately free trigger_data and wprobe_data.
> > This bypasses any RCU grace period for objects that were just briefly
> > exposed to concurrent tracepoint readers.
>
> Ah, good catch. OK, let me call synchronize_rcu() before returning.
>
> >
> > [Severity: High]
> > Is it intentional that on_each_cpu() is called with wait=false in
> > wprobe_work_func()? Because it doesn't wait, the work function completes
> > immediately while IPIs are still pending on other CPUs. If the trigger
> > is unregistered and freed right after, wprobe_smp_update_func() will
> > access freed wprobe_data.
>
> Ah, indeed. I will use a combination of irq_worker and worker and use
> on_each_cpu() with wait=true for safety.
>
> >
> > [Severity: High]
> > Does wprobe_trigger_cmd_parse() acquire the target event's module
> > reference? The code calls trace_event_enable_disable() directly without
> > first calling trace_event_try_get_ref(). However, wprobe_trigger_free()
> > unconditionally calls trace_event_put_ref(), which could lead to a module
> > refcount underflow.
>
> OK, let me fix it.
>
> >
> > [Severity: Medium]
> > Is there a memory leak on the success path of wprobe_trigger_cmd_parse()?
> > The function increments the refcount via event_trigger_init(), and then
> > event_trigger_register() increments it again. On success, trigger_data
> > is set to NULL to bypass the cleanup macro, but the initial refcount is
> > never dropped. When the trigger is later unregistered, the refcount only
> > drops to 1, permanently leaking the objects.
>
> OK, call trace_event_put_ref directly in wprobe_trigger_cmd_parse().
>
>
> >
> > [Severity: Medium]
> > How is concurrent access to tw->addr synchronized between different
> > triggers? If a user attaches multiple triggers (e.g., set_wprobe and
> > clear_wprobe) targeting the same wprobe event, they allocate separate
> > wprobe_data instances with independent locks. When wprobe_trigger()
> > executes, they will read and write the shared tw->addr simultaneously
> > without a common lock, potentially corrupting the hardware breakpoint state.
>
> I decided to use a lock in trace_wprobe side and use READ_ONCE/WRITE_ONCE()
> which may be enough.
>
> Thank you,
>
> --
> Masami Hiramatsu (Google) <mhiramat@kernel.org>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH v7 10/10] selftests: ftrace: Add wprobe trigger testcase
From: Masami Hiramatsu @ 2026-07-16 1:59 UTC (permalink / raw)
To: sashiko-reviews
Cc: sashiko-bot, linux-perf-users, Linux Trace Kernel, Steven Rostedt
In-Reply-To: <20260715021641.B3D691F000E9@smtp.kernel.org>
On Wed, 15 Jul 2026 02:05:55 +0000
sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
> - [Critical] Deadlocks due to NMI-unsafe operations (schedule_work and raw_spin_lock_irqsave) in tracepoint context.
> - [High] Missing cancel_work_sync() causes a Use-After-Free of wprobe_trigger_data via the workqueue.
> - [High] Synchronous free of wprobe_data violates RCU removal ordering, risking concurrent use-after-free.
> - [High] Missing trace_event_try_get_ref() and imbalanced trace_event_put_ref() cause module refcount underflow.
> - [High] on_each_cpu is called with wait=false for an IPI that references wprobe_data.
> - [High] Immediate kfree() of RCU-protected trigger_data and wprobe_data on the error path.
> - [Medium] Permanent memory leak of wprobe trigger objects on successful registration.
> - [Medium] tw->addr is read and written concurrently by different triggers without a shared lock.
> --
>
Thanks for the review comments.
BTW, why this only to linux-perf-users? (Maybe I need to modify Sashiko setting)
> --- Patch [9]: [PATCH v7 09/10] tracing: wprobe: Add wprobe event trigger ---
> The format of this report is altered due to recitation restrictions.
> I am providing a free-form summary of the findings rather than inline
> quoting the patch.
>
> Commit: 4087513c82b11b98d27cfc4452df9a1a8e41ddd5
> Author: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> Subject: tracing: wprobe: Add wprobe event trigger
>
> This patch adds a wprobe event trigger to set and clear watch events
> dynamically, allowing watchpoints on local variables and slab objects.
>
> [Severity: Critical]
> Is it safe to call raw_spin_lock_irqsave() and schedule_work() directly
> from wprobe_trigger()? Tracepoints can execute in arbitrary contexts,
> including NMI or with scheduler locks held. Calling schedule_work() is
> unsafe in these contexts. Additionally, acquiring wprobe_data->lock here
> can cause a hard deadlock if the tracepoint interrupts wprobe_smp_update_func()
> while it is holding the exact same lock in IPI context.
Hmm, good catch!
OK, use a pair of irq_work and work to call on_each_cpu() for some
interrupt context.
For NMI context, I think wprobe_trigger should check the original state
is in_nmi(). This can work for normal trace events or fprobe, but not
for kprobes, because kprobe handler itself is working inside NMI context.
Hmm, let me check how can I fix this. Until solving this problem, this
wprobe trigger may not work with kprobes.
Also, I found the lock should protect the wprobe itself, not data.
Let me move the irq_work and lock to trace_wprobe.
Hmm, we may need to pass to original context (calculated by preempt_count)
to trigger event so that it can evaluate the original context.
(BTW, )
> [Severity: High]
> Does free_wprobe_trigger_data() safely handle the embedded work struct?
> When a trigger is unregistered, this function frees wprobe_data via kfree().
> If wprobe_trigger() recently scheduled wprobe_data->work, the workqueue
> could execute wprobe_work_func() on the freed memory since there is no
> cancel_work_sync() before freeing.
OK, so move the worker to wprobe because anyway the worker will update
the wprobe itself.
>
> [Severity: High]
> Is an RCU grace period needed in wprobe_trigger_free()?
> wprobe_unregister_trigger() unlinks the trigger and calls
> wprobe_trigger_free(), which synchronously frees wprobe_data. Since
> tracepoints execute under RCU, could a concurrent tracepoint execution
> still be accessing wprobe_data when it gets freed?
Indeed, let's use call_rcu().
>
> [Severity: High]
> Can the error path in wprobe_trigger_cmd_parse() cause a use-after-free?
> If event_trigger_register() successfully exposes the trigger to RCU
> readers but a subsequent step fails, returning an error causes the
> __free(kfree) cleanup to immediately free trigger_data and wprobe_data.
> This bypasses any RCU grace period for objects that were just briefly
> exposed to concurrent tracepoint readers.
Ah, good catch. OK, let me call synchronize_rcu() before returning.
>
> [Severity: High]
> Is it intentional that on_each_cpu() is called with wait=false in
> wprobe_work_func()? Because it doesn't wait, the work function completes
> immediately while IPIs are still pending on other CPUs. If the trigger
> is unregistered and freed right after, wprobe_smp_update_func() will
> access freed wprobe_data.
Ah, indeed. I will use a combination of irq_worker and worker and use
on_each_cpu() with wait=true for safety.
>
> [Severity: High]
> Does wprobe_trigger_cmd_parse() acquire the target event's module
> reference? The code calls trace_event_enable_disable() directly without
> first calling trace_event_try_get_ref(). However, wprobe_trigger_free()
> unconditionally calls trace_event_put_ref(), which could lead to a module
> refcount underflow.
OK, let me fix it.
>
> [Severity: Medium]
> Is there a memory leak on the success path of wprobe_trigger_cmd_parse()?
> The function increments the refcount via event_trigger_init(), and then
> event_trigger_register() increments it again. On success, trigger_data
> is set to NULL to bypass the cleanup macro, but the initial refcount is
> never dropped. When the trigger is later unregistered, the refcount only
> drops to 1, permanently leaking the objects.
OK, call trace_event_put_ref directly in wprobe_trigger_cmd_parse().
>
> [Severity: Medium]
> How is concurrent access to tw->addr synchronized between different
> triggers? If a user attaches multiple triggers (e.g., set_wprobe and
> clear_wprobe) targeting the same wprobe event, they allocate separate
> wprobe_data instances with independent locks. When wprobe_trigger()
> executes, they will read and write the shared tw->addr simultaneously
> without a common lock, potentially corrupting the hardware breakpoint state.
I decided to use a lock in trace_wprobe side and use READ_ONCE/WRITE_ONCE()
which may be enough.
Thank you,
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH v7 05/10] tracing: wprobe: Use a new seq_print_ip_sym_offset() wrapper
From: Masami Hiramatsu @ 2026-07-16 0:24 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Steven Rostedt, Peter Zijlstra, Ingo Molnar, x86, Jinchao Wang,
Mathieu Desnoyers, 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: <178407989708.95826.17630423823637072384.stgit@devnote2>
Oops, I missed to fold this in the 1st patch...
(the seq_print_ip_sym_offset() part is already folded)
Thanks,
On Wed, 15 Jul 2026 10:44:57 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> 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"
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH] tracing: ring-buffer: allowlist clang-generated symbols
From: Nathan Chancellor @ 2026-07-15 23:40 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Steven Rostedt, Vincent Donnefort, Arnd Bergmann,
Masami Hiramatsu, Mathieu Desnoyers, Nick Desaulniers,
Bill Wendling, Justin Stitt, Marc Zyngier, Thomas Weißschuh,
Paolo Bonzini, linux-kernel, linux-trace-kernel, llvm
In-Reply-To: <cc1232f7-fa15-4e1b-ba24-3d77dcb813c9@app.fastmail.com>
On Wed, Jul 15, 2026 at 09:38:21AM +0200, Arnd Bergmann wrote:
> On Tue, Jul 14, 2026, at 23:56, Steven Rostedt wrote:
> > 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?
>
> Yes, turning off gcov for this file should avoid the llvm_gcda and
> llvm_gcov symbols, but not the memset() symbol.
This would actually turn on GCOV for this translation unit and cause the
llvm_gcda and llvm_gcov symbols to be included in UNDEFINED_ALLOWLIST
via the $(NM) call.
> All the other features that leave annotations are handled by
> listing the symbol names (__asan __gcov __kasan __kcsan __hwasan
> __sancov __sanitizer __tsan __ubsan __msan), so I think for
> consistency it makes sense to treat llvm gcov the same way
> we handle gcc gcov and the rest.
I do tend to agree with this though, as opposed to using
GCOV_PROFILE_... or ..._SANTIZE variables.
--
Cheers,
Nathan
^ permalink raw reply
* Re: [PATCH v2 03/33] tools/testing/vma: use vma_start_pgoff() in merge tests
From: Liam R. Howlett @ 2026-07-15 19:53 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Andrew Morton, David Hildenbrand, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Rik van Riel, Harry Yoo,
Jann Horn, Lance Yang, Pedro Falcato, Russell King, Dinh Nguyen,
Simon Schuster, James E.J. Bottomley, Helge Deller,
Alexander Viro, Christian Brauner, Jan Kara, Dan Williams,
Matthew Wilcox, Muchun Song, Oscar Salvador, Masami Hiramatsu,
Oleg Nesterov, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Miaohe Lin, Naoya Horiguchi, Xu Xin,
Chengming Zhou, SJ Park, Matthew Brost, Joshua Hahn, Rakie Kim,
Byungchul Park, Gregory Price, Ying Huang, Alistair Popple,
Hugh Dickins, Peter Xu, Kees Cook, Marek Szyprowski, Robin Murphy,
Andrey Konovalov, Alexander Potapenko, Dmitry Vyukov,
Steven Rostedt, Mathieu Desnoyers, Jarkko Sakkinen, Dave Hansen,
Thomas Gleixner, Borislav Petkov, x86, H. Peter Anvin, Ian Abbott,
H Hartley Sweeten, Lucas Stach, Christian Gmeiner, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Matthew Auld, Jason Gunthorpe,
Yishai Hadas, Shameer Kolothum, Kevin Tian, Ankit Agrawal,
Alex Williamson, Paolo Bonzini, Shakeel Butt, Usama Arif,
linux-mm, linux-kernel, linux-arm-kernel, linux-parisc,
linux-fsdevel, nvdimm, linux-perf-users, linux-trace-kernel,
damon, iommu, kasan-dev, linux-sgx, etnaviv, dri-devel,
linux-arm-msm, freedreno, linux-tegra, kvm, Russell King
In-Reply-To: <20260710-b4-pre-scalable-cow-v2-3-2a5aa403d977@kernel.org>
On 26/07/10 09:16PM, Lorenzo Stoakes wrote:
> Now we have the vma_start_pgoff() helper, update the merge tests to make
> use of it for consistency.
>
> No functional change intended.
>
> Reviewed-by: Pedro Falcato <pfalcato@suse.de>
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
Reviewed-by: Liam R. Howlett (Oracle) <liam@infradead.org>
> ---
> tools/testing/vma/tests/merge.c | 38 +++++++++++++++++++-------------------
> 1 file changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/tools/testing/vma/tests/merge.c b/tools/testing/vma/tests/merge.c
> index 03b6f9820e0a..f8666a755749 100644
> --- a/tools/testing/vma/tests/merge.c
> +++ b/tools/testing/vma/tests/merge.c
> @@ -118,7 +118,7 @@ static bool test_simple_merge(void)
>
> ASSERT_EQ(vma->vm_start, 0);
> ASSERT_EQ(vma->vm_end, 0x3000);
> - ASSERT_EQ(vma->vm_pgoff, 0);
> + ASSERT_EQ(vma_start_pgoff(vma), 0);
> ASSERT_FLAGS_SAME_MASK(&vma->flags, vma_flags);
>
> detach_free_vma(vma);
> @@ -150,7 +150,7 @@ static bool test_simple_modify(void)
>
> ASSERT_EQ(vma->vm_start, 0x1000);
> ASSERT_EQ(vma->vm_end, 0x2000);
> - ASSERT_EQ(vma->vm_pgoff, 1);
> + ASSERT_EQ(vma_start_pgoff(vma), 1);
>
> /*
> * Now walk through the three split VMAs and make sure they are as
> @@ -162,7 +162,7 @@ static bool test_simple_modify(void)
>
> ASSERT_EQ(vma->vm_start, 0);
> ASSERT_EQ(vma->vm_end, 0x1000);
> - ASSERT_EQ(vma->vm_pgoff, 0);
> + ASSERT_EQ(vma_start_pgoff(vma), 0);
>
> detach_free_vma(vma);
> vma_iter_clear(&vmi);
> @@ -171,7 +171,7 @@ static bool test_simple_modify(void)
>
> ASSERT_EQ(vma->vm_start, 0x1000);
> ASSERT_EQ(vma->vm_end, 0x2000);
> - ASSERT_EQ(vma->vm_pgoff, 1);
> + ASSERT_EQ(vma_start_pgoff(vma), 1);
>
> detach_free_vma(vma);
> vma_iter_clear(&vmi);
> @@ -180,7 +180,7 @@ static bool test_simple_modify(void)
>
> ASSERT_EQ(vma->vm_start, 0x2000);
> ASSERT_EQ(vma->vm_end, 0x3000);
> - ASSERT_EQ(vma->vm_pgoff, 2);
> + ASSERT_EQ(vma_start_pgoff(vma), 2);
>
> detach_free_vma(vma);
> mtree_destroy(&mm.mm_mt);
> @@ -209,7 +209,7 @@ static bool test_simple_expand(void)
>
> ASSERT_EQ(vma->vm_start, 0);
> ASSERT_EQ(vma->vm_end, 0x3000);
> - ASSERT_EQ(vma->vm_pgoff, 0);
> + ASSERT_EQ(vma_start_pgoff(vma), 0);
>
> detach_free_vma(vma);
> mtree_destroy(&mm.mm_mt);
> @@ -231,7 +231,7 @@ static bool test_simple_shrink(void)
>
> ASSERT_EQ(vma->vm_start, 0);
> ASSERT_EQ(vma->vm_end, 0x1000);
> - ASSERT_EQ(vma->vm_pgoff, 0);
> + ASSERT_EQ(vma_start_pgoff(vma), 0);
>
> detach_free_vma(vma);
> mtree_destroy(&mm.mm_mt);
> @@ -324,7 +324,7 @@ static bool __test_merge_new(bool is_sticky, bool a_is_sticky, bool b_is_sticky,
> ASSERT_TRUE(merged);
> ASSERT_EQ(vma->vm_start, 0);
> ASSERT_EQ(vma->vm_end, 0x4000);
> - ASSERT_EQ(vma->vm_pgoff, 0);
> + ASSERT_EQ(vma_start_pgoff(vma), 0);
> ASSERT_EQ(vma->anon_vma, &dummy_anon_vma);
> ASSERT_TRUE(vma_write_started(vma));
> ASSERT_EQ(mm.map_count, 3);
> @@ -343,7 +343,7 @@ static bool __test_merge_new(bool is_sticky, bool a_is_sticky, bool b_is_sticky,
> ASSERT_TRUE(merged);
> ASSERT_EQ(vma->vm_start, 0);
> ASSERT_EQ(vma->vm_end, 0x5000);
> - ASSERT_EQ(vma->vm_pgoff, 0);
> + ASSERT_EQ(vma_start_pgoff(vma), 0);
> ASSERT_EQ(vma->anon_vma, &dummy_anon_vma);
> ASSERT_TRUE(vma_write_started(vma));
> ASSERT_EQ(mm.map_count, 3);
> @@ -364,7 +364,7 @@ static bool __test_merge_new(bool is_sticky, bool a_is_sticky, bool b_is_sticky,
> ASSERT_TRUE(merged);
> ASSERT_EQ(vma->vm_start, 0x6000);
> ASSERT_EQ(vma->vm_end, 0x9000);
> - ASSERT_EQ(vma->vm_pgoff, 6);
> + ASSERT_EQ(vma_start_pgoff(vma), 6);
> ASSERT_EQ(vma->anon_vma, &dummy_anon_vma);
> ASSERT_TRUE(vma_write_started(vma));
> ASSERT_EQ(mm.map_count, 3);
> @@ -384,7 +384,7 @@ static bool __test_merge_new(bool is_sticky, bool a_is_sticky, bool b_is_sticky,
> ASSERT_TRUE(merged);
> ASSERT_EQ(vma->vm_start, 0);
> ASSERT_EQ(vma->vm_end, 0x9000);
> - ASSERT_EQ(vma->vm_pgoff, 0);
> + ASSERT_EQ(vma_start_pgoff(vma), 0);
> ASSERT_EQ(vma->anon_vma, &dummy_anon_vma);
> ASSERT_TRUE(vma_write_started(vma));
> ASSERT_EQ(mm.map_count, 2);
> @@ -404,7 +404,7 @@ static bool __test_merge_new(bool is_sticky, bool a_is_sticky, bool b_is_sticky,
> ASSERT_TRUE(merged);
> ASSERT_EQ(vma->vm_start, 0xa000);
> ASSERT_EQ(vma->vm_end, 0xc000);
> - ASSERT_EQ(vma->vm_pgoff, 0xa);
> + ASSERT_EQ(vma_start_pgoff(vma), 0xa);
> ASSERT_EQ(vma->anon_vma, &dummy_anon_vma);
> ASSERT_TRUE(vma_write_started(vma));
> ASSERT_EQ(mm.map_count, 2);
> @@ -423,7 +423,7 @@ static bool __test_merge_new(bool is_sticky, bool a_is_sticky, bool b_is_sticky,
> ASSERT_TRUE(merged);
> ASSERT_EQ(vma->vm_start, 0);
> ASSERT_EQ(vma->vm_end, 0xc000);
> - ASSERT_EQ(vma->vm_pgoff, 0);
> + ASSERT_EQ(vma_start_pgoff(vma), 0);
> ASSERT_EQ(vma->anon_vma, &dummy_anon_vma);
> ASSERT_TRUE(vma_write_started(vma));
> ASSERT_EQ(mm.map_count, 1);
> @@ -443,7 +443,7 @@ static bool __test_merge_new(bool is_sticky, bool a_is_sticky, bool b_is_sticky,
> ASSERT_NE(vma, NULL);
> ASSERT_EQ(vma->vm_start, 0);
> ASSERT_EQ(vma->vm_end, 0xc000);
> - ASSERT_EQ(vma->vm_pgoff, 0);
> + ASSERT_EQ(vma_start_pgoff(vma), 0);
> ASSERT_EQ(vma->anon_vma, &dummy_anon_vma);
>
> detach_free_vma(vma);
> @@ -805,7 +805,7 @@ static bool test_vma_merge_new_with_close(void)
> ASSERT_EQ(vmg.state, VMA_MERGE_SUCCESS);
> ASSERT_EQ(vma->vm_start, 0);
> ASSERT_EQ(vma->vm_end, 0x5000);
> - ASSERT_EQ(vma->vm_pgoff, 0);
> + ASSERT_EQ(vma_start_pgoff(vma), 0);
> ASSERT_EQ(vma->vm_ops, &vm_ops);
> ASSERT_TRUE(vma_write_started(vma));
> ASSERT_EQ(mm.map_count, 2);
> @@ -865,7 +865,7 @@ static bool __test_merge_existing(bool prev_is_sticky, bool middle_is_sticky, bo
> ASSERT_EQ(vma_next->anon_vma, &dummy_anon_vma);
> ASSERT_EQ(vma->vm_start, 0x2000);
> ASSERT_EQ(vma->vm_end, 0x3000);
> - ASSERT_EQ(vma->vm_pgoff, 2);
> + ASSERT_EQ(vma_start_pgoff(vma), 2);
> ASSERT_TRUE(vma_write_started(vma));
> ASSERT_TRUE(vma_write_started(vma_next));
> ASSERT_EQ(mm.map_count, 2);
> @@ -931,7 +931,7 @@ static bool __test_merge_existing(bool prev_is_sticky, bool middle_is_sticky, bo
> ASSERT_EQ(vma_prev->anon_vma, &dummy_anon_vma);
> ASSERT_EQ(vma->vm_start, 0x6000);
> ASSERT_EQ(vma->vm_end, 0x7000);
> - ASSERT_EQ(vma->vm_pgoff, 6);
> + ASSERT_EQ(vma_start_pgoff(vma), 6);
> ASSERT_TRUE(vma_write_started(vma_prev));
> ASSERT_TRUE(vma_write_started(vma));
> ASSERT_EQ(mm.map_count, 2);
> @@ -1416,7 +1416,7 @@ static bool test_merge_extend(void)
> ASSERT_EQ(vma_merge_extend(&vmi, vma, 0x2000), vma);
> ASSERT_EQ(vma->vm_start, 0);
> ASSERT_EQ(vma->vm_end, 0x4000);
> - ASSERT_EQ(vma->vm_pgoff, 0);
> + ASSERT_EQ(vma_start_pgoff(vma), 0);
> ASSERT_TRUE(vma_write_started(vma));
> ASSERT_EQ(mm.map_count, 1);
>
> @@ -1456,7 +1456,7 @@ static bool test_expand_only_mode(void)
> ASSERT_EQ(vmg.state, VMA_MERGE_SUCCESS);
> ASSERT_EQ(vma->vm_start, 0x3000);
> ASSERT_EQ(vma->vm_end, 0x9000);
> - ASSERT_EQ(vma->vm_pgoff, 3);
> + ASSERT_EQ(vma_start_pgoff(vma), 3);
> ASSERT_TRUE(vma_write_started(vma));
> ASSERT_EQ(vma_iter_addr(&vmi), 0x3000);
> vma_assert_attached(vma);
>
> --
> 2.55.0
>
^ permalink raw reply
* Re: [PATCH v2 02/33] mm: add kdoc comments for vma_start/last_pgoff()
From: Liam R. Howlett @ 2026-07-15 19:51 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Andrew Morton, David Hildenbrand, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Rik van Riel, Harry Yoo,
Jann Horn, Lance Yang, Pedro Falcato, Russell King, Dinh Nguyen,
Simon Schuster, James E.J. Bottomley, Helge Deller,
Alexander Viro, Christian Brauner, Jan Kara, Dan Williams,
Matthew Wilcox, Muchun Song, Oscar Salvador, Masami Hiramatsu,
Oleg Nesterov, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Miaohe Lin, Naoya Horiguchi, Xu Xin,
Chengming Zhou, SJ Park, Matthew Brost, Joshua Hahn, Rakie Kim,
Byungchul Park, Gregory Price, Ying Huang, Alistair Popple,
Hugh Dickins, Peter Xu, Kees Cook, Marek Szyprowski, Robin Murphy,
Andrey Konovalov, Alexander Potapenko, Dmitry Vyukov,
Steven Rostedt, Mathieu Desnoyers, Jarkko Sakkinen, Dave Hansen,
Thomas Gleixner, Borislav Petkov, x86, H. Peter Anvin, Ian Abbott,
H Hartley Sweeten, Lucas Stach, Christian Gmeiner, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Matthew Auld, Jason Gunthorpe,
Yishai Hadas, Shameer Kolothum, Kevin Tian, Ankit Agrawal,
Alex Williamson, Paolo Bonzini, Shakeel Butt, Usama Arif,
linux-mm, linux-kernel, linux-arm-kernel, linux-parisc,
linux-fsdevel, nvdimm, linux-perf-users, linux-trace-kernel,
damon, iommu, kasan-dev, linux-sgx, etnaviv, dri-devel,
linux-arm-msm, freedreno, linux-tegra, kvm, Russell King
In-Reply-To: <20260710-b4-pre-scalable-cow-v2-2-2a5aa403d977@kernel.org>
On 26/07/10 09:16PM, Lorenzo Stoakes wrote:
> Describe what vma_start_pgoff() and vma_last_pgoff() actually provide in
> detail.
>
> This is in order that we can differentiate this between functions that will
> be added in a subsequent patch which provide a different page offset.
>
> We go to lengths to describe the edge cases that can be run into here.
>
> No functional change intended.
>
> Reviewed-by: Pedro Falcato <pfalcato@suse.de>
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
Reviewed-by: Liam R. Howlett (Oracle) <liam@infradead.org>
> ---
> include/linux/mm.h | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 09b06d8fea74..abac72785277 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -4307,11 +4307,41 @@ static inline unsigned long vma_pages(const struct vm_area_struct *vma)
> return (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
> }
>
> +/**
> + * vma_start_pgoff() - Get the page offset of the start of @vma
> + * @vma: The VMA whose page offset is required.
> + *
> + * If the VMA is file-backed, this is the page offset into the file.
> + *
> + * If @vma is anonymous, this is the virtual page offset of the start of the
> + * VMA - if unfaulted, then vma->vm_start >> PAGE_SHIFT, if faulted then the
> + * virtual page offset at the time of first fault.
> + *
> + * If @vma is a MAP_PRIVATE file-backed mapping, then this returns the
> + * page offset within the file.
> + *
> + * Edge cases: nommu does not abide by these, MAP_PRIVATE-/dev/zero satisfies
> + * vma_is_anonymous() but has file-backed page offset, and MAP_PRIVATE-pfnmap
> + * regions have their page offset set to the first PFN in the range.
> + *
> + * Returns: The page offset of the start of @vma.
> + */
> static inline pgoff_t vma_start_pgoff(const struct vm_area_struct *vma)
> {
> return vma->vm_pgoff;
> }
>
> +/**
> + * vma_last_pgoff() - Get the page offset of the last page in @vma
> + * @vma: The VMA whose last page offset is required.
> + *
> + * This returns the last page offset contained within @vma.
> + *
> + * See the description of vma_start_pgoff() for a description of VMA page
> + * offsets.
> + *
> + * Returns: The last page offset of @vma.
> + */
> static inline pgoff_t vma_last_pgoff(const struct vm_area_struct *vma)
> {
> return vma_start_pgoff(vma) + vma_pages(vma) - 1;
>
> --
> 2.55.0
>
^ permalink raw reply
* Re: [RFC] tracing: Try user copies with page faults disabled first
From: Steven Rostedt @ 2026-07-15 19:11 UTC (permalink / raw)
To: Usama Arif
Cc: linux-kernel, linux-trace-kernel, mathieu.desnoyers, mhiramat,
leitao
In-Reply-To: <20260715155454.4127988-1-usama.arif@linux.dev>
On Wed, 15 Jul 2026 08:54:54 -0700
Usama Arif <usama.arif@linux.dev> wrote:
> trace_user_fault_read() is called with preemption disabled to copy user
> memory into a per-cpu scratch buffer. The existing implementation enables
> preemption around the copy because faulting user memory can sleep. That
> opens a window where another task can run on the same CPU and clobber the
> per-cpu buffer, so the copy is wrapped in a retry loop: sample
> nr_context_switches_cpu(), do the preempt-enabled copy, and retry if the
> counter changed. If this fails to complete 100 times, the function gives up
> with a warning.
>
> nr_context_switches_cpu() reads rq->nr_switches. That counter increments
> for every context switch on the CPU, not only for switches to tasks that
> use this tracing scratch buffer. On a heavily loaded system, unrelated
> scheduler activity can move the counter during every preempt-enabled copy
> attempt, exhaust the retry guard, and trigger the warning.
>
> This is showing up across the Meta fleet around 100 times a day since the
> kernel began upgrading to 7.1, mostly on arm servers:
>
> Error: Too many tries to read user space
> WARNING: kernel/trace/trace.c:6244 at trace_user_fault_read+0x284/0x2c8, CPU#28: Collection-18/677527
> CPU: 28 UID: 0 PID: 677527 Comm: Collection-18 Kdump: loaded Not tainted 7.1.0-.... #1 PREEMPTLAZY
> Hardware name: Quanta Java Island MP 29F0EMA08CH/Java Island, BIOS F0EJ3A16 03/12/2026
> Call trace:
> trace_user_fault_read+0x284/0x2c8 (P)
> syscall_get_data+0x144/0x2c0
> perf_syscall_enter+0xc0/0x2d8
> syscall_trace_enter+0x1a0/0x270
> do_el0_svc+0x54/0xb8
> el0_svc+0x44/0x268
> el0t_64_sync_handler+0x7c/0x120
> el0t_64_sync+0x17c/0x180
> ---[ end trace 0000000000000000 ]---
>
> The retry loop is only needed when preemption must be enabled for the user
> copy. If the user pages are already resident, the copy can complete without
> fault handling that sleeps, and preemption can stay disabled throughout.
>
> Add a fast path that first tries the copy with page faults disabled. For
> the plain copy_from_user case, use __copy_from_user_inatomic(). If the
> probe faults, the architecture exception-table fixup returns a non-zero
> not-copied count and trace_user_fault_read() falls back to the existing
> preempt-enabled slow path.
>
> Custom copy callbacks need the same behavior. Update the syscall argument
> copy callbacks to report a non-zero return only when the pagefault-disabled
> probe faults. With page faults enabled, keep their previous behavior:
> record the syscall event and omit only the individual user argument that
> still cannot be copied.
>
> The slow path remains in place for nonresident pages and permanent copy
> failures. nr_context_switches_cpu() still overcounts, but the retry loop is
> now avoided for the common resident-page case that does not need fault
> handling.
This was reported also under memory load.
I have a patch that will only do a retry if another *user* task schedules
in, and will not be bothered by kernel tasks (which may be scheduled in due
to the copy from user to begin with).
Can you see if that works too?
https://lore.kernel.org/all/20260710083357.49e05ff6@gandalf.local.home/
I'm also thinking it may be backwards to try with page_fault disabled
first. Because if the text is not present in memory (which is the case for
a lot of syscalls), it will fail there and then go into the "slow path" to
try with enabling preemption. If you bug hits then, it could cause the
warning to show up.
If you first try with preemption disabled, it will likely pull the page
into memory, so if it does schedule, doing it with pagefault_disabled() a
second time is more likely to succeed (the page has been pulled in via the
first attempt). That is, the first attempt pulls in the page but fails due
to being scheduled out, the second attempt with pagefault_disabled() may be
more likely to succeed. Thus, on option is to simply alternate between
re-enabling preemption, and calling with page_fault() disabled.
-- Steve
>
> Reported-by: Breno Leitao <leitao@debian.org>
> Signed-off-by: Usama Arif <usama.arif@linux.dev>
> ---
> This warning is very likely occuring when the fleetwide profiler runs
> and something else seems to load the server. (As we have perf_syscall_enter()
> in the stack and I see the profiler process active when the warning prints).
> It can occur several days after boot, so its a bit difficult to verify
> if the warning will go away with this patch deployed.
> If the patch looks good, we can deploy it in the fleet and report back.
> ---
^ permalink raw reply
* Re: [PATCH] tracing: Propagate errors from remote event bulk updates
From: Steven Rostedt @ 2026-07-15 18:19 UTC (permalink / raw)
To: Vincent Donnefort
Cc: Jackie Liu, mhiramat, mathieu.desnoyers, linux-trace-kernel
In-Reply-To: <alc11bkkDvpaqYrh@google.com>
On Wed, 15 Jul 2026 08:25:09 +0100
Vincent Donnefort <vdonnefort@google.com> wrote:
> > I checked the v2 series. Patch 02/18 handles registration failure cleanup,
> > and patch 03/18 changes the boolean parsing in
> > remote_events_dir_enable_write(), but the loop still ignores errors from
> > trace_remote_enable_event().
> >
> > Would you prefer this fix to be folded into your series, or should I resend
> > a v2 rebased on top of it?
>
> Ha yes my bad. Then no, no need to send it separately.
So should I take this patch and send it to Linus now?
-- Steve
^ permalink raw reply
* Re: [PATCH v2 01/33] mm: move vma_start_pgoff() into mm.h and clean up
From: Liam R. Howlett @ 2026-07-15 18:05 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Andrew Morton, David Hildenbrand, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Rik van Riel, Harry Yoo,
Jann Horn, Lance Yang, Pedro Falcato, Russell King, Dinh Nguyen,
Simon Schuster, James E.J. Bottomley, Helge Deller,
Alexander Viro, Christian Brauner, Jan Kara, Dan Williams,
Matthew Wilcox, Muchun Song, Oscar Salvador, Masami Hiramatsu,
Oleg Nesterov, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Miaohe Lin, Naoya Horiguchi, Xu Xin,
Chengming Zhou, SJ Park, Matthew Brost, Joshua Hahn, Rakie Kim,
Byungchul Park, Gregory Price, Ying Huang, Alistair Popple,
Hugh Dickins, Peter Xu, Kees Cook, Marek Szyprowski, Robin Murphy,
Andrey Konovalov, Alexander Potapenko, Dmitry Vyukov,
Steven Rostedt, Mathieu Desnoyers, Jarkko Sakkinen, Dave Hansen,
Thomas Gleixner, Borislav Petkov, x86, H. Peter Anvin, Ian Abbott,
H Hartley Sweeten, Lucas Stach, Christian Gmeiner, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Matthew Auld, Jason Gunthorpe,
Yishai Hadas, Shameer Kolothum, Kevin Tian, Ankit Agrawal,
Alex Williamson, Paolo Bonzini, Shakeel Butt, Usama Arif,
linux-mm, linux-kernel, linux-arm-kernel, linux-parisc,
linux-fsdevel, nvdimm, linux-perf-users, linux-trace-kernel,
damon, iommu, kasan-dev, linux-sgx, etnaviv, dri-devel,
linux-arm-msm, freedreno, linux-tegra, kvm, Russell King
In-Reply-To: <20260710-b4-pre-scalable-cow-v2-1-2a5aa403d977@kernel.org>
On 26/07/10 09:16PM, Lorenzo Stoakes wrote:
> vma_last_pgoff() already lives there, so it's a bit odd to keep
> vma_start_pgoff() in mm/interval_tree.c. Move them together.
>
> These each return unsigned long, which pgoff_t is typedef'd to. Make this
> consistent and have these functions return pgoff_t instead.
>
> Additionally, express vma_last_pgoff() in terms of vma_start_pgoff(), since
> we wrap the vma->vm_pgoff access, we may as well use it here.
>
> Also while we're here, const-ify the VMA and cleanup a bit.
>
> Also update the VMA userland tests to reflect the change.
This patch could have been 01/35? Disappointing, but I will try to get
over it.
>
> No functional change intended.
>
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Acked-by: David Hildenbrand (Arm) <david@kernel.org>
> Reviewed-by: Pedro Falcato <pfalcato@suse.de>
> Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
Reviewed-by: Liam R. Howlett (Oracle) <liam@infradead.org>
> ---
> include/linux/mm.h | 9 +++++++--
> mm/interval_tree.c | 5 -----
> tools/testing/vma/include/dup.h | 5 +++++
> 3 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 25e669632d2c..09b06d8fea74 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -4307,9 +4307,14 @@ static inline unsigned long vma_pages(const struct vm_area_struct *vma)
> return (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
> }
>
> -static inline unsigned long vma_last_pgoff(struct vm_area_struct *vma)
> +static inline pgoff_t vma_start_pgoff(const struct vm_area_struct *vma)
> {
> - return vma->vm_pgoff + vma_pages(vma) - 1;
> + return vma->vm_pgoff;
> +}
> +
> +static inline pgoff_t vma_last_pgoff(const struct vm_area_struct *vma)
> +{
> + return vma_start_pgoff(vma) + vma_pages(vma) - 1;
> }
>
> static inline unsigned long vma_desc_size(const struct vm_area_desc *desc)
> diff --git a/mm/interval_tree.c b/mm/interval_tree.c
> index 32bcfbfcf15f..344d1f5946c7 100644
> --- a/mm/interval_tree.c
> +++ b/mm/interval_tree.c
> @@ -10,11 +10,6 @@
> #include <linux/rmap.h>
> #include <linux/interval_tree_generic.h>
>
> -static inline unsigned long vma_start_pgoff(struct vm_area_struct *v)
> -{
> - return v->vm_pgoff;
> -}
> -
> INTERVAL_TREE_DEFINE(struct vm_area_struct, shared.rb,
> unsigned long, shared.rb_subtree_last,
> vma_start_pgoff, vma_last_pgoff, /* empty */, vma_interval_tree)
> diff --git a/tools/testing/vma/include/dup.h b/tools/testing/vma/include/dup.h
> index bf26b3f48d3a..668650067c7c 100644
> --- a/tools/testing/vma/include/dup.h
> +++ b/tools/testing/vma/include/dup.h
> @@ -1301,6 +1301,11 @@ static inline unsigned long vma_pages(const struct vm_area_struct *vma)
> return (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
> }
>
> +static inline pgoff_t vma_start_pgoff(const struct vm_area_struct *vma)
> +{
> + return vma->vm_pgoff;
> +}
> +
> static inline int vfs_mmap_prepare(struct file *file, struct vm_area_desc *desc)
> {
> return file->f_op->mmap_prepare(desc);
>
> --
> 2.55.0
>
^ permalink raw reply
* Re: [PATCH v2 25/33] mm/vma: make vma_set_range() static, drop insert_vm_struct() decl
From: Vlastimil Babka (SUSE) @ 2026-07-15 17:45 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton, David Hildenbrand,
Liam R. Howlett, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Rik van Riel, Harry Yoo, Jann Horn, Lance Yang, Pedro Falcato,
Russell King, Dinh Nguyen, Simon Schuster, James E.J. Bottomley,
Helge Deller, Alexander Viro, Christian Brauner, Jan Kara,
Dan Williams, Matthew Wilcox, Muchun Song, Oscar Salvador,
Masami Hiramatsu, Oleg Nesterov, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Miaohe Lin, Naoya Horiguchi, Xu Xin,
Chengming Zhou, SJ Park, Matthew Brost, Joshua Hahn, Rakie Kim,
Byungchul Park, Gregory Price, Ying Huang, Alistair Popple,
Hugh Dickins, Peter Xu, Kees Cook, Marek Szyprowski, Robin Murphy,
Andrey Konovalov, Alexander Potapenko, Dmitry Vyukov,
Steven Rostedt, Mathieu Desnoyers, Jarkko Sakkinen, Dave Hansen,
Thomas Gleixner, Borislav Petkov, x86, H. Peter Anvin, Ian Abbott,
H Hartley Sweeten, Lucas Stach, Christian Gmeiner, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Matthew Auld, Jason Gunthorpe,
Yishai Hadas, Shameer Kolothum, Kevin Tian, Ankit Agrawal,
Alex Williamson, Paolo Bonzini, Shakeel Butt, Usama Arif
Cc: linux-mm, linux-kernel, linux-arm-kernel, linux-parisc,
linux-fsdevel, nvdimm, linux-perf-users, linux-trace-kernel,
damon, iommu, kasan-dev, linux-sgx, etnaviv, dri-devel,
linux-arm-msm, freedreno, linux-tegra, kvm, Russell King
In-Reply-To: <20260710-b4-pre-scalable-cow-v2-25-2a5aa403d977@kernel.org>
On 7/10/26 22:17, Lorenzo Stoakes wrote:
> With __install_special_mapping() moved to vma.c, vma_set_range() can be
> made into a static function there and is now completely isolated from the
> rest of mm.
Pedantic nit: it was already "static", but in a .h file
Yeah it was also "inline" but that's not the important part of this change.
So I'd say it was rather moved to vma.c, but you can ignore this nit if you
want.
> While we're here, we can also remove the insert_vm_struct() declaration
> from mm.h - the function is implemented in vma.c and already declared in
> vma.h, and has no users outside of mm.
>
> Also update the VMA userland tests to reflect this change.
>
> No functional change intended.
>
> Reviewed-by: Pedro Falcato <pfalcato@suse.de>
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
> ---
> include/linux/mm.h | 1 -
> mm/internal.h | 9 ---------
> mm/vma.c | 8 ++++++++
> tools/testing/vma/shared.c | 9 ---------
> tools/testing/vma/shared.h | 5 -----
> 5 files changed, 8 insertions(+), 24 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 3d69b597b9b1..762313b47301 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -4130,7 +4130,6 @@ void anon_rmap_tree_verify(struct anon_vma_chain *avc);
>
> /* mmap.c */
> extern int __vm_enough_memory(const struct mm_struct *mm, long pages, int cap_sys_admin);
> -extern int insert_vm_struct(struct mm_struct *, struct vm_area_struct *);
> extern void exit_mmap(struct mm_struct *);
> bool mmap_read_lock_maybe_expand(struct mm_struct *mm, struct vm_area_struct *vma,
> unsigned long addr, bool write);
> diff --git a/mm/internal.h b/mm/internal.h
> index 01a762bcc2b2..2c650d280e90 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -1540,15 +1540,6 @@ extern bool mirrored_kernelcore;
> bool memblock_has_mirror(void);
> void memblock_free_all(void);
>
> -static __always_inline void vma_set_range(struct vm_area_struct *vma,
> - unsigned long start, unsigned long end,
> - pgoff_t pgoff)
> -{
> - vma->vm_start = start;
> - vma->vm_end = end;
> - vma->vm_pgoff = pgoff;
> -}
> -
> static inline bool vma_soft_dirty_enabled(struct vm_area_struct *vma)
> {
> /*
> diff --git a/mm/vma.c b/mm/vma.c
> index 5308aa5a8c91..cc0e449ad0be 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -70,6 +70,14 @@ struct mmap_state {
> .state = VMA_MERGE_START, \
> }
>
> +static void vma_set_range(struct vm_area_struct *vma, unsigned long start,
> + unsigned long end, pgoff_t pgoff)
> +{
> + vma->vm_start = start;
> + vma->vm_end = end;
> + vma->vm_pgoff = pgoff;
> +}
> +
> /* Was this VMA ever forked from a parent, i.e. maybe contains CoW mappings? */
> static bool vma_is_fork_child(struct vm_area_struct *vma)
> {
> diff --git a/tools/testing/vma/shared.c b/tools/testing/vma/shared.c
> index 2565a5aecb80..bea9ea6db02a 100644
> --- a/tools/testing/vma/shared.c
> +++ b/tools/testing/vma/shared.c
> @@ -120,12 +120,3 @@ unsigned long rlimit(unsigned int limit)
> {
> return (unsigned long)-1;
> }
> -
> -void vma_set_range(struct vm_area_struct *vma,
> - unsigned long start, unsigned long end,
> - pgoff_t pgoff)
> -{
> - vma->vm_start = start;
> - vma->vm_end = end;
> - vma->vm_pgoff = pgoff;
> -}
> diff --git a/tools/testing/vma/shared.h b/tools/testing/vma/shared.h
> index 8b9e3b11c3cb..ca4f1238f1c7 100644
> --- a/tools/testing/vma/shared.h
> +++ b/tools/testing/vma/shared.h
> @@ -125,8 +125,3 @@ void __vma_set_dummy_anon_vma(struct vm_area_struct *vma,
> /* Provide a simple dummy VMA/anon_vma dummy setup for testing. */
> void vma_set_dummy_anon_vma(struct vm_area_struct *vma,
> struct anon_vma_chain *avc);
> -
> -/* Helper function to specify a VMA's range. */
> -void vma_set_range(struct vm_area_struct *vma,
> - unsigned long start, unsigned long end,
> - pgoff_t pgoff);
>
^ permalink raw reply
* Re: [PATCH v2 24/33] mm/vma: move __install_special_mapping() to vma.c
From: Vlastimil Babka (SUSE) @ 2026-07-15 17:39 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton, David Hildenbrand,
Liam R. Howlett, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Rik van Riel, Harry Yoo, Jann Horn, Lance Yang, Pedro Falcato,
Russell King, Dinh Nguyen, Simon Schuster, James E.J. Bottomley,
Helge Deller, Alexander Viro, Christian Brauner, Jan Kara,
Dan Williams, Matthew Wilcox, Muchun Song, Oscar Salvador,
Masami Hiramatsu, Oleg Nesterov, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Miaohe Lin, Naoya Horiguchi, Xu Xin,
Chengming Zhou, SJ Park, Matthew Brost, Joshua Hahn, Rakie Kim,
Byungchul Park, Gregory Price, Ying Huang, Alistair Popple,
Hugh Dickins, Peter Xu, Kees Cook, Marek Szyprowski, Robin Murphy,
Andrey Konovalov, Alexander Potapenko, Dmitry Vyukov,
Steven Rostedt, Mathieu Desnoyers, Jarkko Sakkinen, Dave Hansen,
Thomas Gleixner, Borislav Petkov, x86, H. Peter Anvin, Ian Abbott,
H Hartley Sweeten, Lucas Stach, Christian Gmeiner, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Matthew Auld, Jason Gunthorpe,
Yishai Hadas, Shameer Kolothum, Kevin Tian, Ankit Agrawal,
Alex Williamson, Paolo Bonzini, Shakeel Butt, Usama Arif
Cc: linux-mm, linux-kernel, linux-arm-kernel, linux-parisc,
linux-fsdevel, nvdimm, linux-perf-users, linux-trace-kernel,
damon, iommu, kasan-dev, linux-sgx, etnaviv, dri-devel,
linux-arm-msm, freedreno, linux-tegra, kvm, Russell King
In-Reply-To: <20260710-b4-pre-scalable-cow-v2-24-2a5aa403d977@kernel.org>
On 7/10/26 22:17, Lorenzo Stoakes wrote:
> This function is operating on VMAs and rightly belongs in vma.c, where it
> can be subject to VMA userland testing and allows us to isolate it from the
> rest of mm.
>
> The _install_special_mapping() function will remain in mmap.c as a wrapper,
> since this is used by architecture-specific code.
>
> Doing so allows us to isolate more functions in vma.c for the same reasons.
>
> This forms part of work to allow for tracking MAP_PRIVATE file-backed
> mappings by their anonymous virtual page offset, as doing so allows us to
> isolate and keep code that interacts with this together.
>
> No functional change intended.
>
> Reviewed-by: Pedro Falcato <pfalcato@suse.de>
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 23/33] mm/vma: add and use vma_[add/sub]_pgoff()
From: Vlastimil Babka (SUSE) @ 2026-07-15 17:38 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton, David Hildenbrand,
Liam R. Howlett, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Rik van Riel, Harry Yoo, Jann Horn, Lance Yang, Pedro Falcato,
Russell King, Dinh Nguyen, Simon Schuster, James E.J. Bottomley,
Helge Deller, Alexander Viro, Christian Brauner, Jan Kara,
Dan Williams, Matthew Wilcox, Muchun Song, Oscar Salvador,
Masami Hiramatsu, Oleg Nesterov, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Miaohe Lin, Naoya Horiguchi, Xu Xin,
Chengming Zhou, SJ Park, Matthew Brost, Joshua Hahn, Rakie Kim,
Byungchul Park, Gregory Price, Ying Huang, Alistair Popple,
Hugh Dickins, Peter Xu, Kees Cook, Marek Szyprowski, Robin Murphy,
Andrey Konovalov, Alexander Potapenko, Dmitry Vyukov,
Steven Rostedt, Mathieu Desnoyers, Jarkko Sakkinen, Dave Hansen,
Thomas Gleixner, Borislav Petkov, x86, H. Peter Anvin, Ian Abbott,
H Hartley Sweeten, Lucas Stach, Christian Gmeiner, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Matthew Auld, Jason Gunthorpe,
Yishai Hadas, Shameer Kolothum, Kevin Tian, Ankit Agrawal,
Alex Williamson, Paolo Bonzini, Shakeel Butt, Usama Arif
Cc: linux-mm, linux-kernel, linux-arm-kernel, linux-parisc,
linux-fsdevel, nvdimm, linux-perf-users, linux-trace-kernel,
damon, iommu, kasan-dev, linux-sgx, etnaviv, dri-devel,
linux-arm-msm, freedreno, linux-tegra, kvm, Russell King
In-Reply-To: <20260710-b4-pre-scalable-cow-v2-23-2a5aa403d977@kernel.org>
On 7/10/26 22:17, Lorenzo Stoakes wrote:
> Add helpers for adding or subtracting to a VMA's page offset, exposed
> internally for VMA users within mm in mm/vma.h.
>
> This is to lay the foundations for tracking anonymous page offset for
> MAP_PRIVATE file-backed mappings, where adding and subtracting from this
> value must be reflected in both the file and anonymous offsets.
>
> These are used on VMA split and downward stack expansion.
>
> No functional change intended.
>
> Reviewed-by: Pedro Falcato <pfalcato@suse.de>
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
Nits:
> ---
> mm/nommu.c | 6 ++++--
> mm/vma.c | 6 +++---
> mm/vma.h | 12 ++++++++++++
> 3 files changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/mm/nommu.c b/mm/nommu.c
> index c0a0869cd0d6..2a0136f6081d 100644
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -41,6 +41,7 @@
> #include <asm/tlbflush.h>
> #include <asm/mmu_context.h>
> #include "internal.h"
> +#include "vma.h"
>
> unsigned long highest_memmap_pfn;
> int heap_stack_gap = 0;
> @@ -1361,7 +1362,8 @@ static int split_vma(struct vma_iterator *vmi, struct vm_area_struct *vma,
> region->vm_top = region->vm_end = new->vm_end = addr;
> } else {
> region->vm_start = new->vm_start = addr;
> - region->vm_pgoff = new->vm_pgoff += npages;
Ah that takes care of the other case of ugliness I noticed earlier, good.
> + vma_add_pgoff(new, npages);
> + region->vm_pgoff = vma_start_pgoff(new);
> }
>
> vma_iter_config(vmi, new->vm_start, new->vm_end);
> @@ -1378,7 +1380,7 @@ static int split_vma(struct vma_iterator *vmi, struct vm_area_struct *vma,
> delete_nommu_region(vma->vm_region);
> if (new_below) {
> vma->vm_region->vm_start = vma->vm_start = addr;
> - vma->vm_pgoff += npages;
> + vma_add_pgoff(vma, npages);
> vma->vm_region->vm_pgoff = vma_start_pgoff(vma);
> } else {
> vma->vm_region->vm_end = vma->vm_end = addr;
> diff --git a/mm/vma.c b/mm/vma.c
> index 7aa0149f076c..bdd99ba56b4d 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -517,7 +517,7 @@ __split_vma(struct vma_iterator *vmi, struct vm_area_struct *vma,
> new->vm_end = addr;
> } else {
> new->vm_start = addr;
> - new->vm_pgoff += linear_page_delta(vma, addr);
> + vma_add_pgoff(new, linear_page_delta(vma, addr));
> }
>
> err = -ENOMEM;
> @@ -556,7 +556,7 @@ __split_vma(struct vma_iterator *vmi, struct vm_area_struct *vma,
>
> if (new_below) {
> vma->vm_start = addr;
> - vma->vm_pgoff += (addr - new->vm_start) >> PAGE_SHIFT;
> + vma_add_pgoff(vma, (addr - new->vm_start) >> PAGE_SHIFT);
Hm isn't this also a case for using linear_page_delta(addr, new)?
(I guess in patch 21/33)
Would be like the hunk above.
> } else {
> vma->vm_end = addr;
> }
> @@ -3305,7 +3305,7 @@ int expand_downwards(struct vm_area_struct *vma, unsigned long address)
> vm_stat_account(mm, vma->vm_flags, grow);
> anon_rmap_tree_pre_update_vma(vma);
> vma->vm_start = address;
> - vma->vm_pgoff -= grow;
> + vma_sub_pgoff(vma, grow);
> /* Overwrite old entry in mtree. */
> vma_iter_store_overwrite(&vmi, vma);
> anon_rmap_tree_post_update_vma(vma);
> diff --git a/mm/vma.h b/mm/vma.h
> index 2342516ce00e..47fe35e5307e 100644
> --- a/mm/vma.h
> +++ b/mm/vma.h
> @@ -247,6 +247,18 @@ static inline pgoff_t vmg_end_pgoff(const struct vma_merge_struct *vmg)
> return vmg_start_pgoff(vmg) + vmg_pages(vmg);
> }
>
> +static inline void vma_add_pgoff(struct vm_area_struct *vma, pgoff_t delta)
> +{
> + vma_assert_can_modify(vma);
> + vma->vm_pgoff += delta;
> +}
> +
> +static inline void vma_sub_pgoff(struct vm_area_struct *vma, pgoff_t delta)
> +{
> + vma_assert_can_modify(vma);
> + vma->vm_pgoff -= delta;
> +}
> +
> #define VMG_STATE(name, mm_, vmi_, start_, end_, vma_flags_, pgoff_) \
> struct vma_merge_struct name = { \
> .mm = mm_, \
>
^ permalink raw reply
* Re: [PATCH v2 22/33] mm/vma: introduce vma_assert_can_modify()
From: Vlastimil Babka (SUSE) @ 2026-07-15 17:33 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton, David Hildenbrand,
Liam R. Howlett, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Rik van Riel, Harry Yoo, Jann Horn, Lance Yang, Pedro Falcato,
Russell King, Dinh Nguyen, Simon Schuster, James E.J. Bottomley,
Helge Deller, Alexander Viro, Christian Brauner, Jan Kara,
Dan Williams, Matthew Wilcox, Muchun Song, Oscar Salvador,
Masami Hiramatsu, Oleg Nesterov, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Miaohe Lin, Naoya Horiguchi, Xu Xin,
Chengming Zhou, SJ Park, Matthew Brost, Joshua Hahn, Rakie Kim,
Byungchul Park, Gregory Price, Ying Huang, Alistair Popple,
Hugh Dickins, Peter Xu, Kees Cook, Marek Szyprowski, Robin Murphy,
Andrey Konovalov, Alexander Potapenko, Dmitry Vyukov,
Steven Rostedt, Mathieu Desnoyers, Jarkko Sakkinen, Dave Hansen,
Thomas Gleixner, Borislav Petkov, x86, H. Peter Anvin, Ian Abbott,
H Hartley Sweeten, Lucas Stach, Christian Gmeiner, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Matthew Auld, Jason Gunthorpe,
Yishai Hadas, Shameer Kolothum, Kevin Tian, Ankit Agrawal,
Alex Williamson, Paolo Bonzini, Shakeel Butt, Usama Arif
Cc: linux-mm, linux-kernel, linux-arm-kernel, linux-parisc,
linux-fsdevel, nvdimm, linux-perf-users, linux-trace-kernel,
damon, iommu, kasan-dev, linux-sgx, etnaviv, dri-devel,
linux-arm-msm, freedreno, linux-tegra, kvm, Russell King
In-Reply-To: <20260710-b4-pre-scalable-cow-v2-22-2a5aa403d977@kernel.org>
On 7/10/26 22:17, Lorenzo Stoakes wrote:
> vma_assert_write_locked() and vma_assert_attached() are useful for their
> own purposes, however VMA code absolutely does allow the modification of
> non-write locked VMAs if they are at that point detached (i.e. unreachable
> from anywhere).
>
> It's therefore useful to be able to assert that a VMA is either
> detached (modification doesn't matter) or write locked (you're explicitly
> locked for modification).
>
> Therefore introduce vma_assert_can_modify() for this purpose.
>
> While we're here, make vma_is_attached() available generally - if
> !CONFIG_PER_VMA_LOCK, then there's no sense in which a VMA is
> detached (vma_mark_detached() is a noop), so have this default to true in
> this case.
>
> Also update VMA userland tests to reflect this change, correcting the
> previously open-coded vma_assert_[attached,detached]() there.
>
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 21/33] mm: use linear_page_[index, delta]() consistently
From: Vlastimil Babka (SUSE) @ 2026-07-15 17:30 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton, David Hildenbrand,
Liam R. Howlett, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Rik van Riel, Harry Yoo, Jann Horn, Lance Yang, Pedro Falcato,
Russell King, Dinh Nguyen, Simon Schuster, James E.J. Bottomley,
Helge Deller, Alexander Viro, Christian Brauner, Jan Kara,
Dan Williams, Matthew Wilcox, Muchun Song, Oscar Salvador,
Masami Hiramatsu, Oleg Nesterov, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Miaohe Lin, Naoya Horiguchi, Xu Xin,
Chengming Zhou, SJ Park, Matthew Brost, Joshua Hahn, Rakie Kim,
Byungchul Park, Gregory Price, Ying Huang, Alistair Popple,
Hugh Dickins, Peter Xu, Kees Cook, Marek Szyprowski, Robin Murphy,
Andrey Konovalov, Alexander Potapenko, Dmitry Vyukov,
Steven Rostedt, Mathieu Desnoyers, Jarkko Sakkinen, Dave Hansen,
Thomas Gleixner, Borislav Petkov, x86, H. Peter Anvin, Ian Abbott,
H Hartley Sweeten, Lucas Stach, Christian Gmeiner, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Matthew Auld, Jason Gunthorpe,
Yishai Hadas, Shameer Kolothum, Kevin Tian, Ankit Agrawal,
Alex Williamson, Paolo Bonzini, Shakeel Butt, Usama Arif
Cc: linux-mm, linux-kernel, linux-arm-kernel, linux-parisc,
linux-fsdevel, nvdimm, linux-perf-users, linux-trace-kernel,
damon, iommu, kasan-dev, linux-sgx, etnaviv, dri-devel,
linux-arm-msm, freedreno, linux-tegra, kvm, Russell King,
Kai Huang, Ackerley Tng
In-Reply-To: <20260710-b4-pre-scalable-cow-v2-21-2a5aa403d977@kernel.org>
On 7/10/26 22:17, Lorenzo Stoakes wrote:
> There are a number of places where we open code what linear_page_index()
> and linear_page_delta() calculate.
>
> Replace this code with the appropriate functions for consistency.
>
> No functional change intended.
>
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> # for DRM
> Acked-by: Kai Huang <kai.huang@intel.com> # for sgx
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Reviewed-by: Pedro Falcato <pfalcato@suse.de> # for mm
> Reviewed-by: Ackerley Tng <ackerleytng@google.com> # for guest_memfd
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 20/33] mm/vma: remove duplicative vma_pgoff_offset() helper
From: Vlastimil Babka (SUSE) @ 2026-07-15 17:28 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton, David Hildenbrand,
Liam R. Howlett, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Rik van Riel, Harry Yoo, Jann Horn, Lance Yang, Pedro Falcato,
Russell King, Dinh Nguyen, Simon Schuster, James E.J. Bottomley,
Helge Deller, Alexander Viro, Christian Brauner, Jan Kara,
Dan Williams, Matthew Wilcox, Muchun Song, Oscar Salvador,
Masami Hiramatsu, Oleg Nesterov, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Miaohe Lin, Naoya Horiguchi, Xu Xin,
Chengming Zhou, SJ Park, Matthew Brost, Joshua Hahn, Rakie Kim,
Byungchul Park, Gregory Price, Ying Huang, Alistair Popple,
Hugh Dickins, Peter Xu, Kees Cook, Marek Szyprowski, Robin Murphy,
Andrey Konovalov, Alexander Potapenko, Dmitry Vyukov,
Steven Rostedt, Mathieu Desnoyers, Jarkko Sakkinen, Dave Hansen,
Thomas Gleixner, Borislav Petkov, x86, H. Peter Anvin, Ian Abbott,
H Hartley Sweeten, Lucas Stach, Christian Gmeiner, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Matthew Auld, Jason Gunthorpe,
Yishai Hadas, Shameer Kolothum, Kevin Tian, Ankit Agrawal,
Alex Williamson, Paolo Bonzini, Shakeel Butt, Usama Arif
Cc: linux-mm, linux-kernel, linux-arm-kernel, linux-parisc,
linux-fsdevel, nvdimm, linux-perf-users, linux-trace-kernel,
damon, iommu, kasan-dev, linux-sgx, etnaviv, dri-devel,
linux-arm-msm, freedreno, linux-tegra, kvm, Russell King
In-Reply-To: <20260710-b4-pre-scalable-cow-v2-20-2a5aa403d977@kernel.org>
On 7/10/26 22:17, Lorenzo Stoakes wrote:
> This is doing what linear_page_index() does, so eliminate it and replace it
> with linear_page_index().
>
> Update the VMA userland tests to reflect this change.
>
> No functional change intended.
>
> Reviewed-by: Pedro Falcato <pfalcato@suse.de>
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 19/33] mm: prefer vma_[start,end]_pgoff() to vma->vm_pgoff in kernel/
From: Vlastimil Babka (SUSE) @ 2026-07-15 16:44 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton, David Hildenbrand,
Liam R. Howlett, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Rik van Riel, Harry Yoo, Jann Horn, Lance Yang, Pedro Falcato,
Russell King, Dinh Nguyen, Simon Schuster, James E.J. Bottomley,
Helge Deller, Alexander Viro, Christian Brauner, Jan Kara,
Dan Williams, Matthew Wilcox, Muchun Song, Oscar Salvador,
Masami Hiramatsu, Oleg Nesterov, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Miaohe Lin, Naoya Horiguchi, Xu Xin,
Chengming Zhou, SJ Park, Matthew Brost, Joshua Hahn, Rakie Kim,
Byungchul Park, Gregory Price, Ying Huang, Alistair Popple,
Hugh Dickins, Peter Xu, Kees Cook, Marek Szyprowski, Robin Murphy,
Andrey Konovalov, Alexander Potapenko, Dmitry Vyukov,
Steven Rostedt, Mathieu Desnoyers, Jarkko Sakkinen, Dave Hansen,
Thomas Gleixner, Borislav Petkov, x86, H. Peter Anvin, Ian Abbott,
H Hartley Sweeten, Lucas Stach, Christian Gmeiner, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Matthew Auld, Jason Gunthorpe,
Yishai Hadas, Shameer Kolothum, Kevin Tian, Ankit Agrawal,
Alex Williamson, Paolo Bonzini, Shakeel Butt, Usama Arif
Cc: linux-mm, linux-kernel, linux-arm-kernel, linux-parisc,
linux-fsdevel, nvdimm, linux-perf-users, linux-trace-kernel,
damon, iommu, kasan-dev, linux-sgx, etnaviv, dri-devel,
linux-arm-msm, freedreno, linux-tegra, kvm, Russell King
In-Reply-To: <20260710-b4-pre-scalable-cow-v2-19-2a5aa403d977@kernel.org>
On 7/10/26 22:17, Lorenzo Stoakes wrote:
> Be consistent in using vma_start_pgoff() and vma_end_pgoff(), which clearly
> indicates which part of the VMA the page offset refers to and aids
> greppability.
>
> This is part of a broader series laying the ground to provide a virtual
> page offset for MAP_PRIVATE-file backed anon folios.
>
> No functional change intended.
>
> Acked-by: Marek Szyprowski <m.szyprowski@samsung.com> # for kernel/dma
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Acked-by: Pedro Falcato <pfalcato@suse.de>
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
^ permalink raw reply
* Re: [PATCH v3 04/11] arm64/mm: Add set_memory_device() and set_memory_normal()
From: Thierry Reding @ 2026-07-15 16:01 UTC (permalink / raw)
To: Will Deacon
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Hunter,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Sowjanya Komatineni, Luca Ceresoli,
Mikko Perttunen, Yury Norov, Rasmus Villemoes, Russell King,
Alexander Gordeev, Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, Andrew Morton,
David Hildenbrand, Lorenzo Stoakes, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Marek Szyprowski, Robin Murphy, Sumit Semwal, Benjamin Gaignard,
Brian Starkey, John Stultz, T.J. Mercier, Christian König,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Catalin Marinas, Thierry Reding, devicetree, linux-tegra,
linux-kernel, dri-devel, linux-media, linux-arm-kernel,
linux-s390, linux-mm, iommu, linaro-mm-sig, linux-trace-kernel,
Thierry Reding, Chun Ng
In-Reply-To: <ak_8Po3NxgAU7k0T@orome>
[-- Attachment #1: Type: text/plain, Size: 6356 bytes --]
On Thu, Jul 09, 2026 at 09:58:01PM +0200, Thierry Reding wrote:
> On Tue, Jul 07, 2026 at 12:27:13PM +0100, Will Deacon wrote:
> > On Mon, Jul 06, 2026 at 03:49:24PM +0200, Thierry Reding wrote:
> > > On Fri, Jul 03, 2026 at 06:13:31PM +0100, Will Deacon wrote:
> > > > On Thu, Jul 02, 2026 at 06:41:23PM +0200, Thierry Reding wrote:
> > > > > On Thu, Jul 02, 2026 at 03:46:44PM +0200, Thierry Reding wrote:
> > > > > > On Thu, Jul 02, 2026 at 10:18:47AM +0100, Will Deacon wrote:
> > > > > > > On Wed, Jul 01, 2026 at 06:08:15PM +0200, Thierry Reding wrote:
> > > > > > > > From: Chun Ng <chunn@nvidia.com>
> > > > > > > >
> > > > > > > > Add helpers to swap PROT_NORMAL and PROT_DEVICE_nGnRnE protection bits
> > > > > > > > on a kernel-linear-map range.
> > > > > > >
> > > > > > > That sounds like a really terrible idea. Why is this necessary and how
> > > > > > > does it interact with things like load_unaligned_zeropad()?
> > > > > >
> > > > > > This is necessary because once the memory controller has walled off the
> > > > > > new memory region the CPU must not access it under any circumstances or
> > > > > > it'll cause the CPU to lock up (I think technically it'll hit an SError
> > > > > > but in practice that just means it'll freeze, as far as I can tell).
> > > > > >
> > > > > > Probably doesn't interact well at all with load_unaligned_zeropad().
> > > > > >
> > > > > > > I think you should unmap the memory from the linear map and memremap()
> > > > > > > it instead.
> > > > > >
> > > > > > Given that the memory can never be accessed by the CPU after the memory
> > > > > > controller locks it down, I don't think we'll even need memremap(). The
> > > > > > only thing we really need is the sg_table we hand out via the DMA BUFs
> > > > > > so that they can be used by device drivers to program their DMA engines
> > > > > > internally.
> > > > > >
> > > > > > Looking through some of the architecture code around this, shouldn't we
> > > > > > simply be using set_memory_encrypted() and set_memory_decrypted() for
> > > > > > this? While they might've been created for slightly other use-cases,
> > > > > > they seem to be doing exactly what we want (i.e. remove the page range
> > > > > > from the linear mapping and flushing it, or restoring the valid bit and
> > > > > > standard permissions, respectively).
> > > > >
> > > > > Ah... I guess we can't do it because we're not in a realm world and so
> > > > > the early checks in __set_memory_enc_dec() would return early and turn
> > > > > it into a no-op.
> > > > >
> > > > > How about if I extract a common helper and provide set_memory_p() and
> > > > > set_memory_np() in terms of those. Those are available on x86 and
> > > > > PowerPC as well, so fairly standard. I suppose at that point we're
> > > > > closer to set_memory_valid().
> > > >
> > > > Why not just call set_direct_map_invalid_noflush() +
> > > > flush_tlb_kernel_range() for each page? We already have APIs for this.
> > >
> > > Having a "standard" helper with a fixed and documented purposed seemed
> > > like a preferable approach for this particular case. We also may want to
> > > make the driver that uses this buildable as a module, in which case we'd
> > > need to export these rather low-level APIs. And then there's also the
> > > fact that we typically call this on a rather large region of memory
> > > (usually something like 512 MiB), so doing it page-by-page is rather
> > > suboptimal.
> > >
> > > > The big challenge I see with any linear map manipulation, however, is
> > > > that it will rely on can_set_direct_map() which likely means you need to
> > > > give up some performance and/or security to make this work. Does memory
> > > > become inaccesible dynamically at runtime? If not, the best bet would
> > > > be to describe it as a carveout in the DT and mark it as "no-map" so
> > > > we avoid mapping it in the first place.
> > >
> > > VPR exists in two modes: static and resizable. For static VPR we do
> > > exactly that: describe it as carveout in DT with no-map and deal with it
> > > accordingly in the driver. Resizable VPR is for device that have small
> > > amounts of RAM. Content-protected video playback will in the worst case
> > > consume around 1.8 GiB of RAM, so we want to be able to reuse for other
> > > purposes when VPR is unused on those devices. In that case, the memory
> > > is also described as a reserved-memory region in DT, but it is marked as
> > > reusable so that it can be managed by CMA.
> > >
> > > The resize operation is fairly slow to begin with because we need to
> > > stall the GPU and put it into reset before the operation, then take it
> > > out of reset and resume it afterwards.
> > >
> > > What kind of performance impact do you expect?
> >
> > You'll need to measure it, but we've seen reports of double-digit
> > percentage regressions in performance and power. As I said, the problem
> > is that you need to split the linear map to 4k page at runtime to unmap
> > the dynamic carveout, but that isn't something that can be done on most
> > CPUs. Therefore you end up having to use page-granular mappings for the
> > entire thing, similarly to how 'rodata_full' drives can_set_direct_map()
> > and the perf/power hit affects everything.
>
> The VPR has fairly large alignment restrictions (1 MiB) and we do unmap
> in fairly large chunks (512 MiB currently, but we can change that if it
> is helpful) because we really want to avoid resizing operations, so the
> tradeoff is between frequency of resize vs. potential memory wasted.
>
> Does that change anything with regards to performance?
Turns out that the system we need this for is very likely going to end
up using 4 KiB pages anyway because it doesn't have a whole lot of RAM
(which is the whole reason we want the VPR to be resizable in the first
place). So it sounds like set_direct_map_*() is a good way forward.
Still, wouldn't it potentially be much faster to unmap entire 2 MiB
blocks at a time if we know the driver guarantees the alignment? Maybe
the changes to add num_pages as an argument to set_direct_map_*() that
Mike mentioned would already be an improvement because it avoids those
gratuitous calls to can_set_direct_map().
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [RFC] tracing: Try user copies with page faults disabled first
From: Usama Arif @ 2026-07-15 15:54 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel, mathieu.desnoyers, mhiramat,
rostedt, leitao
Cc: Usama Arif
trace_user_fault_read() is called with preemption disabled to copy user
memory into a per-cpu scratch buffer. The existing implementation enables
preemption around the copy because faulting user memory can sleep. That
opens a window where another task can run on the same CPU and clobber the
per-cpu buffer, so the copy is wrapped in a retry loop: sample
nr_context_switches_cpu(), do the preempt-enabled copy, and retry if the
counter changed. If this fails to complete 100 times, the function gives up
with a warning.
nr_context_switches_cpu() reads rq->nr_switches. That counter increments
for every context switch on the CPU, not only for switches to tasks that
use this tracing scratch buffer. On a heavily loaded system, unrelated
scheduler activity can move the counter during every preempt-enabled copy
attempt, exhaust the retry guard, and trigger the warning.
This is showing up across the Meta fleet around 100 times a day since the
kernel began upgrading to 7.1, mostly on arm servers:
Error: Too many tries to read user space
WARNING: kernel/trace/trace.c:6244 at trace_user_fault_read+0x284/0x2c8, CPU#28: Collection-18/677527
CPU: 28 UID: 0 PID: 677527 Comm: Collection-18 Kdump: loaded Not tainted 7.1.0-.... #1 PREEMPTLAZY
Hardware name: Quanta Java Island MP 29F0EMA08CH/Java Island, BIOS F0EJ3A16 03/12/2026
Call trace:
trace_user_fault_read+0x284/0x2c8 (P)
syscall_get_data+0x144/0x2c0
perf_syscall_enter+0xc0/0x2d8
syscall_trace_enter+0x1a0/0x270
do_el0_svc+0x54/0xb8
el0_svc+0x44/0x268
el0t_64_sync_handler+0x7c/0x120
el0t_64_sync+0x17c/0x180
---[ end trace 0000000000000000 ]---
The retry loop is only needed when preemption must be enabled for the user
copy. If the user pages are already resident, the copy can complete without
fault handling that sleeps, and preemption can stay disabled throughout.
Add a fast path that first tries the copy with page faults disabled. For
the plain copy_from_user case, use __copy_from_user_inatomic(). If the
probe faults, the architecture exception-table fixup returns a non-zero
not-copied count and trace_user_fault_read() falls back to the existing
preempt-enabled slow path.
Custom copy callbacks need the same behavior. Update the syscall argument
copy callbacks to report a non-zero return only when the pagefault-disabled
probe faults. With page faults enabled, keep their previous behavior:
record the syscall event and omit only the individual user argument that
still cannot be copied.
The slow path remains in place for nonresident pages and permanent copy
failures. nr_context_switches_cpu() still overcounts, but the retry loop is
now avoided for the common resident-page case that does not need fault
handling.
Reported-by: Breno Leitao <leitao@debian.org>
Signed-off-by: Usama Arif <usama.arif@linux.dev>
---
This warning is very likely occuring when the fleetwide profiler runs
and something else seems to load the server. (As we have perf_syscall_enter()
in the stack and I see the profiler process active when the warning prints).
It can occur several days after boot, so its a bit difficult to verify
if the warning will go away with this patch deployed.
If the patch looks good, we can deploy it in the fleet and report back.
---
kernel/trace/trace.c | 22 +++++++++++++++--
kernel/trace/trace_syscalls.c | 45 ++++++++++++++++++++++++++++++++---
2 files changed, 62 insertions(+), 5 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 1146b83b711a..fe1637afac84 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -6173,8 +6173,10 @@ int trace_user_fault_put(struct trace_user_buf_info *tinfo)
* size: The @size of the ptr to read
* data: The @data parameter
*
- * It is expected that @copy_func will return 0 on success and non zero
- * if there was a fault.
+ * @copy_func may be called with page faults disabled first. It is
+ * expected that @copy_func will return 0 on success and non zero if the
+ * copy needs to be retried with page faults enabled or if there was a
+ * fault.
*
* Returns a pointer to the buffer with the content read from @ptr.
* Preemption must remain disabled while the caller accesses the
@@ -6201,6 +6203,22 @@ char *trace_user_fault_read(struct trace_user_buf_info *tinfo,
if (size > tinfo->size)
return NULL;
+ /*
+ * Fastpath: try the copy with page faults disabled. Preemption is
+ * already disabled by the caller, so no other task can run on this
+ * CPU to corrupt the per-CPU buffer, and the seqcount-style retry
+ * loop below is unnecessary. This succeeds whenever the user pages
+ * are already present.
+ */
+ pagefault_disable();
+ if (copy_func)
+ ret = copy_func(buffer, ptr, size, data);
+ else
+ ret = __copy_from_user_inatomic(buffer, ptr, size);
+ pagefault_enable();
+ if (!ret)
+ return buffer;
+
/*
* This acts similar to a seqcount. The per CPU context switches are
* recorded, migration is disabled and preemption is enabled. The
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index e98ee7e1e66f..bc9d0134f7c1 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -8,6 +8,7 @@
#include <linux/module.h> /* for MODULE_NAME_LEN via KSYM_SYMBOL_LEN */
#include <linux/ftrace.h>
#include <linux/perf_event.h>
+#include <linux/uaccess.h>
#include <linux/xarray.h>
#include <asm/syscall.h>
@@ -660,32 +661,70 @@ struct syscall_args {
int uargs;
};
+/*
+ * The callback return value is consumed by trace_user_fault_read() as a
+ * whole-buffer status: 0 means the per-CPU scratch buffer can be used,
+ * non-zero means retry with page faults enabled or fail the read.
+ *
+ * Syscall tracing also needs per-argument status, because one bad user
+ * pointer should omit only that appended argument, not the syscall event
+ * itself. Store that per-argument result in args->read[].
+ *
+ * When called by the pagefault-disabled fast path, a fault may just mean the
+ * user page needs to be faulted in, so report it to trace_user_fault_read().
+ * When called by the pagefault-enabled slow path, record the per-argument
+ * failure in args->read[] and return 0 so the syscall event can still be emitted.
+ */
static int syscall_copy_user(char *buf, const char __user *ptr,
size_t size, void *data)
{
struct syscall_args *args = data;
+ bool inatomic = pagefault_disabled();
+ bool faulted = false;
int ret;
for (int i = 0; i < args->uargs; i++, buf += SYSCALL_FAULT_ARG_SZ) {
ptr = (char __user *)args->ptr_array[i];
ret = strncpy_from_user(buf, ptr, size);
args->read[i] = ret;
+ if (inatomic && ret < 0)
+ faulted = true;
}
- return 0;
+
+ return faulted ? -EFAULT : 0;
}
+/*
+ * Copy explicitly sized user arguments into the per-CPU scratch buffer.
+ * The callback return value is whole-buffer status for trace_user_fault_read():
+ * return non-zero from the pagefault-disabled fast path so it can retry with
+ * faults enabled. Per-argument status is stored in args->read[].
+ *
+ * The __copy_from_user*() helpers return bytes not copied. Store @size for a
+ * fully copied argument and -1 for a copy fault. When the pagefault-enabled
+ * slow path still faults, return 0 so the syscall event can be emitted and
+ * only that appended user argument is omitted.
+ */
static int syscall_copy_user_array(char *buf, const char __user *ptr,
size_t size, void *data)
{
struct syscall_args *args = data;
+ bool inatomic = pagefault_disabled();
+ bool faulted = false;
int ret;
for (int i = 0; i < args->uargs; i++, buf += SYSCALL_FAULT_ARG_SZ) {
ptr = (char __user *)args->ptr_array[i];
- ret = __copy_from_user(buf, ptr, size);
+ if (inatomic)
+ ret = __copy_from_user_inatomic(buf, ptr, size);
+ else
+ ret = __copy_from_user(buf, ptr, size);
args->read[i] = ret ? -1 : size;
+ if (inatomic && ret)
+ faulted = true;
}
- return 0;
+
+ return faulted ? -EFAULT : 0;
}
static char *sys_fault_user(unsigned int buf_size,
--
2.53.0-Meta
^ permalink raw reply related
* [PATCH v3 6/6] rtla/osnoise: Trace IPI events when recording a trace file
From: Valentin Schneider @ 2026-07-15 15:45 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Tomas Glozar,
Costa Shulyupin, Crystal Wood, John Kacur, Ivan Pravdin,
Jonathan Corbet
In-Reply-To: <20260715154553.2020891-1-vschneid@redhat.com>
IPIs can now be monitored and accounted by osnoise top. When that is
the case, also record them when saving a trace file.
To match what is being recorded by the tool for its own analysis, event
filters are applied to the events recorded to the trace output.
Signed-off-by: Valentin Schneider <vschneid@redhat.com>
---
tools/tracing/rtla/src/common.c | 2 +-
tools/tracing/rtla/src/common.h | 2 +-
tools/tracing/rtla/src/osnoise.c | 72 +++++++++++++++++++++++++++-
tools/tracing/rtla/src/osnoise.h | 4 ++
tools/tracing/rtla/src/osnoise_top.c | 36 ++------------
5 files changed, 80 insertions(+), 36 deletions(-)
diff --git a/tools/tracing/rtla/src/common.c b/tools/tracing/rtla/src/common.c
index d0a8a6edbf0cb..dd302427557ca 100644
--- a/tools/tracing/rtla/src/common.c
+++ b/tools/tracing/rtla/src/common.c
@@ -204,7 +204,7 @@ int run_tool(struct tool_ops *ops, int argc, char *argv[])
if (params->threshold_actions.present[ACTION_TRACE_OUTPUT] ||
params->end_actions.present[ACTION_TRACE_OUTPUT]) {
- tool->record = osnoise_init_trace_tool(ops->tracer);
+ tool->record = osnoise_init_trace_tool(params, ops->tracer);
if (!tool->record) {
err_msg("Failed to enable the trace instance\n");
goto out_free;
diff --git a/tools/tracing/rtla/src/common.h b/tools/tracing/rtla/src/common.h
index 045253230fcf2..421e06e10f3f1 100644
--- a/tools/tracing/rtla/src/common.h
+++ b/tools/tracing/rtla/src/common.h
@@ -178,7 +178,7 @@ int osnoise_set_workload(struct osnoise_context *context, bool onoff);
void osnoise_destroy_tool(struct osnoise_tool *top);
struct osnoise_tool *osnoise_init_tool(char *tool_name);
-struct osnoise_tool *osnoise_init_trace_tool(const char *tracer);
+struct osnoise_tool *osnoise_init_trace_tool(struct common_params *params, const char *tracer);
bool osnoise_trace_is_off(struct osnoise_tool *tool, struct osnoise_tool *record);
int osnoise_set_stop_us(struct osnoise_context *context, long long stop_us);
int osnoise_set_stop_total_us(struct osnoise_context *context,
diff --git a/tools/tracing/rtla/src/osnoise.c b/tools/tracing/rtla/src/osnoise.c
index 4ff5dad013b10..ae6e5f03e828f 100644
--- a/tools/tracing/rtla/src/osnoise.c
+++ b/tools/tracing/rtla/src/osnoise.c
@@ -1178,10 +1178,56 @@ struct osnoise_tool *osnoise_init_tool(char *tool_name)
return top;
}
+/*
+ * osnoise_init_ipi_filters - Initialize event filtering for IPI events
+ */
+int osnoise_init_ipi_filters(struct osnoise_tool *tool,
+ struct common_params *params,
+ bool *filters_enabled)
+{
+ char filter[MAX_PATH];
+ int retval;
+ /*
+ * If tracing on a subset of possible CPUs, leverage the kernel filtering
+ * infrastructure to only generate events on traced CPUs.
+ * Older kernels (pre v6.6) may have the IPI events but not the ability
+ * to filter them, so allow that to fail gracefully.
+ */
+
+ snprintf(filter, ARRAY_SIZE(filter), "cpu & CPUS{%s}\n", params->cpus);
+ retval = tracefs_event_file_write(tool->trace.inst,
+ "ipi", "ipi_send_cpu", "filter",
+ filter);
+ if (retval < 0) {
+ debug_msg("Could not set ipi_send_cpu CPU filter\n");
+ *filters_enabled = false;
+ return 0;
+ }
+
+
+ snprintf(filter, ARRAY_SIZE(filter), "cpumask & CPUS{%s}\n", params->cpus);
+ retval = tracefs_event_file_write(tool->trace.inst,
+ "ipi", "ipi_send_cpumask", "filter",
+ filter);
+ if (retval < 0) {
+ /*
+ * If we managed to set up the previous filter but not
+ * this one, something's really wrong
+ */
+ err_msg("Could not set ipi_send_cpumask CPU filter\n");
+ *filters_enabled = false;
+ return -1;
+ }
+
+ *filters_enabled = true;
+ return 0;
+}
+
/*
* osnoise_init_trace_tool - init a tracer instance to trace osnoise events
*/
-struct osnoise_tool *osnoise_init_trace_tool(const char *tracer)
+struct osnoise_tool *osnoise_init_trace_tool(struct common_params *params,
+ const char *tracer)
{
struct osnoise_tool *trace;
int retval;
@@ -1196,6 +1242,30 @@ struct osnoise_tool *osnoise_init_trace_tool(const char *tracer)
goto out_err;
}
+ if (!params->ipi)
+ goto done;
+
+ retval = tracefs_event_enable(trace->trace.inst, "ipi", "ipi_send_cpu");
+ if (retval < 0 && !errno) {
+ err_msg("Could not find ipi_send_cpu event\n");
+ goto out_err;
+ }
+
+ retval = tracefs_event_enable(trace->trace.inst, "ipi", "ipi_send_cpumask");
+ if (retval < 0 && !errno) {
+ err_msg("Could not find ipi_send_cpumask event\n");
+ goto out_err;
+ }
+
+ if (params->cpus) {
+ bool unused;
+
+ retval = osnoise_init_ipi_filters(trace, params, &unused);
+ if (retval < 0)
+ goto out_err;
+ }
+
+done:
retval = enable_tracer_by_name(trace->trace.inst, tracer);
if (retval) {
err_msg("Could not enable %s tracer for tracing\n", tracer);
diff --git a/tools/tracing/rtla/src/osnoise.h b/tools/tracing/rtla/src/osnoise.h
index 340ff5a64e6e4..81a704c361ec0 100644
--- a/tools/tracing/rtla/src/osnoise.h
+++ b/tools/tracing/rtla/src/osnoise.h
@@ -63,6 +63,10 @@ int osnoise_enable(struct osnoise_tool *tool);
int osnoise_main(int argc, char **argv);
int hwnoise_main(int argc, char **argv);
+int osnoise_init_ipi_filters(struct osnoise_tool *tool,
+ struct common_params *params,
+ bool *filters_enabled);
+
extern struct tool_ops timerlat_top_ops, timerlat_hist_ops;
extern struct tool_ops osnoise_top_ops, osnoise_hist_ops;
diff --git a/tools/tracing/rtla/src/osnoise_top.c b/tools/tracing/rtla/src/osnoise_top.c
index afab2f341a1e9..87d28865515b5 100644
--- a/tools/tracing/rtla/src/osnoise_top.c
+++ b/tools/tracing/rtla/src/osnoise_top.c
@@ -392,7 +392,7 @@ osnoise_ipi_cpumask_handler(struct trace_seq *s, struct tep_record *record,
*/
struct osnoise_tool *osnoise_init_top(struct common_params *params)
{
- bool ipi_filters_enabled = false;
+ bool ipi_filters_enabled;
struct osnoise_tool *tool;
int retval;
@@ -424,41 +424,11 @@ struct osnoise_tool *osnoise_init_top(struct common_params *params)
goto out_err;
}
- /*
- * If tracing on a subset of possible CPUs, leverage the kernel filtering
- * infrastructure to only generate events on traced CPUs.
- * Older kernels (pre v6.6) may have the IPI events but not the ability
- * to filter them, so allow that to fail gracefully.
- */
if (params->cpus) {
- char filter[MAX_PATH];
-
- snprintf(filter, ARRAY_SIZE(filter), "cpu & CPUS{%s}\n", params->cpus);
- retval = tracefs_event_file_write(tool->trace.inst,
- "ipi", "ipi_send_cpu", "filter",
- filter);
- if (retval < 0) {
- debug_msg("Could not set ipi_send_cpu CPU filter\n");
- goto no_filter;
- }
-
-
- snprintf(filter, ARRAY_SIZE(filter), "cpumask & CPUS{%s}\n", params->cpus);
- retval = tracefs_event_file_write(tool->trace.inst,
- "ipi", "ipi_send_cpumask", "filter",
- filter);
- if (retval < 0) {
- /*
- * If we managed to set up the previous filter but not
- * this one, something's really wrong
- */
- err_msg("Could not set ipi_send_cpumask CPU filter\n");
+ retval = osnoise_init_ipi_filters(tool, params, &ipi_filters_enabled);
+ if (retval < 0)
goto out_err;
- }
-
- ipi_filters_enabled = true;
}
-no_filter:
/*
* If no filtering is available and we're tracing all CPUs, we can still
--
2.55.0
^ 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