* [PATCH] x86/stacktrace: Mark arch_stack_walk() and unwinder functions notrace
From: Yuanhe Shu @ 2026-07-06 9:54 UTC (permalink / raw)
To: Josh Poimboeuf, Peter Zijlstra, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86
Cc: H . Peter Anvin, Steven Rostedt, Masami Hiramatsu, linux-kernel,
linux-trace-kernel, stable, Yuanhe Shu
When the function tracer's func_stack_trace option and the function graph
profiler (function_profile_enabled) are both active, a recursive ftrace
reentrance can occur, leading to a hard lockup. This was observed during
ftrace selftest (ftracetest-ktap) execution:
watchdog: Watchdog detected hard LOCKUP on cpu 204
RIP: profile_graph_entry+0xa0/0x160
Call Trace:
function_graph_enter+0xc9/0x120
arch_ftrace_ops_list_func+0x112/0x230
ftrace_call+0x5/0x44
unwind_next_frame+0x5/0x870 <-- traced by ftrace
arch_stack_walk+0x88/0xf0
stack_trace_save+0x4b/0x70
__ftrace_trace_stack+0x12e/0x170
function_stack_trace_call+0x7c/0xa0
arch_ftrace_ops_list_func+0x112/0x230
ftrace_call+0x5/0x44
irqtime_account_irq+0x5/0xb0
__irq_exit_rcu+0x12/0xc0
...
The root cause is a recursive ftrace reentrance:
function_stack_trace_call() invokes __trace_stack() ->
arch_stack_walk() -> unwind_next_frame() to capture a backtrace.
Since the unwinder functions (__unwind_start(),
unwind_next_frame(), unwind_get_return_address(),
unwind_get_return_address_ptr()) are not marked notrace, the
function graph tracer instruments them, re-entering the ftrace
infrastructure from within an ftrace callback. This results in a
hard lockup with interrupts disabled, detected by the watchdog NMI.
On arm64 and riscv, arch_stack_walk() has already been marked
noinstr to prevent this class of bugs. See
commit 0fbcd8abf337 ("arm64: Prohibit instrumentation on arch_stack_walk()")
and commit 23b2188920a2 ("riscv: stacktrace: convert arch_stack_walk() to noinstr").
However, x86 was not fixed because:
1) x86's return_address() uses the generic
__builtin_return_address() instead of arch_stack_walk(), so the
lockdep recursion path that triggered the arm64 fix does not
exist on x86.
2) On arm64, all unwinder helpers are __always_inline within
arch_stack_walk(), so a single noinstr annotation suffices.
On riscv, the helper walk_stackframe() was already marked
notrace. On x86 however, the ORC unwinder implements
__unwind_start(), unwind_next_frame(), and
unwind_get_return_address() as separate non-inline exported
functions without any instrumentation protection, so marking
only arch_stack_walk() is insufficient.
Fix this by marking arch_stack_walk() and the non-inline unwinder
functions it calls (__unwind_start(), unwind_next_frame(),
unwind_get_return_address(), unwind_get_return_address_ptr())
as notrace, preventing ftrace from instrumenting the entire stack
unwinding path.
Fixes: 3599fe12a125 ("x86/stacktrace: Use common infrastructure")
Cc: stable@vger.kernel.org
Signed-off-by: Yuanhe Shu <xiangzao@linux.alibaba.com>
---
arch/x86/include/asm/unwind.h | 10 +++++-----
arch/x86/kernel/stacktrace.c | 12 ++++++++++--
arch/x86/kernel/unwind_frame.c | 10 +++++-----
arch/x86/kernel/unwind_guess.c | 10 +++++-----
arch/x86/kernel/unwind_orc.c | 10 +++++-----
5 files changed, 30 insertions(+), 22 deletions(-)
diff --git a/arch/x86/include/asm/unwind.h b/arch/x86/include/asm/unwind.h
index 7cede4dc21f0..15b699f2edc0 100644
--- a/arch/x86/include/asm/unwind.h
+++ b/arch/x86/include/asm/unwind.h
@@ -39,11 +39,11 @@ struct unwind_state {
#endif
};
-void __unwind_start(struct unwind_state *state, struct task_struct *task,
- struct pt_regs *regs, unsigned long *first_frame);
-bool unwind_next_frame(struct unwind_state *state);
-unsigned long unwind_get_return_address(struct unwind_state *state);
-unsigned long *unwind_get_return_address_ptr(struct unwind_state *state);
+void notrace __unwind_start(struct unwind_state *state, struct task_struct *task,
+ struct pt_regs *regs, unsigned long *first_frame);
+bool notrace unwind_next_frame(struct unwind_state *state);
+unsigned long notrace unwind_get_return_address(struct unwind_state *state);
+unsigned long *notrace unwind_get_return_address_ptr(struct unwind_state *state);
static inline bool unwind_done(struct unwind_state *state)
{
diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c
index ee117fcf46ed..1e5a06439adb 100644
--- a/arch/x86/kernel/stacktrace.c
+++ b/arch/x86/kernel/stacktrace.c
@@ -12,8 +12,16 @@
#include <asm/stacktrace.h>
#include <asm/unwind.h>
-void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie,
- struct task_struct *task, struct pt_regs *regs)
+/*
+ * arch_stack_walk() and the functions it calls (__unwind_start(),
+ * unwind_next_frame(), unwind_get_return_address(),
+ * unwind_get_return_address_ptr()) must not be instrumented by ftrace,
+ * as they are invoked from within ftrace callbacks (e.g.,
+ * function_stack_trace_call). Tracing these functions would cause
+ * recursive ftrace reentrance, leading to a hard lockup.
+ */
+void notrace arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie,
+ struct task_struct *task, struct pt_regs *regs)
{
struct unwind_state state;
unsigned long addr;
diff --git a/arch/x86/kernel/unwind_frame.c b/arch/x86/kernel/unwind_frame.c
index d8ba93778ae3..07d1b9f0208f 100644
--- a/arch/x86/kernel/unwind_frame.c
+++ b/arch/x86/kernel/unwind_frame.c
@@ -11,7 +11,7 @@
#define FRAME_HEADER_SIZE (sizeof(long) * 2)
-unsigned long unwind_get_return_address(struct unwind_state *state)
+unsigned long notrace unwind_get_return_address(struct unwind_state *state)
{
if (unwind_done(state))
return 0;
@@ -20,7 +20,7 @@ unsigned long unwind_get_return_address(struct unwind_state *state)
}
EXPORT_SYMBOL_GPL(unwind_get_return_address);
-unsigned long *unwind_get_return_address_ptr(struct unwind_state *state)
+unsigned long *notrace unwind_get_return_address_ptr(struct unwind_state *state)
{
if (unwind_done(state))
return NULL;
@@ -261,7 +261,7 @@ static bool update_stack_state(struct unwind_state *state,
}
__no_kmsan_checks
-bool unwind_next_frame(struct unwind_state *state)
+bool notrace unwind_next_frame(struct unwind_state *state)
{
struct pt_regs *regs;
unsigned long *next_bp;
@@ -370,8 +370,8 @@ bool unwind_next_frame(struct unwind_state *state)
}
EXPORT_SYMBOL_GPL(unwind_next_frame);
-void __unwind_start(struct unwind_state *state, struct task_struct *task,
- struct pt_regs *regs, unsigned long *first_frame)
+void notrace __unwind_start(struct unwind_state *state, struct task_struct *task,
+ struct pt_regs *regs, unsigned long *first_frame)
{
unsigned long *bp;
diff --git a/arch/x86/kernel/unwind_guess.c b/arch/x86/kernel/unwind_guess.c
index 884d68a6e714..22d12e79984b 100644
--- a/arch/x86/kernel/unwind_guess.c
+++ b/arch/x86/kernel/unwind_guess.c
@@ -6,7 +6,7 @@
#include <asm/stacktrace.h>
#include <asm/unwind.h>
-unsigned long unwind_get_return_address(struct unwind_state *state)
+unsigned long notrace unwind_get_return_address(struct unwind_state *state)
{
unsigned long addr;
@@ -19,12 +19,12 @@ unsigned long unwind_get_return_address(struct unwind_state *state)
}
EXPORT_SYMBOL_GPL(unwind_get_return_address);
-unsigned long *unwind_get_return_address_ptr(struct unwind_state *state)
+unsigned long *notrace unwind_get_return_address_ptr(struct unwind_state *state)
{
return NULL;
}
-bool unwind_next_frame(struct unwind_state *state)
+bool notrace unwind_next_frame(struct unwind_state *state)
{
struct stack_info *info = &state->stack_info;
@@ -48,8 +48,8 @@ bool unwind_next_frame(struct unwind_state *state)
}
EXPORT_SYMBOL_GPL(unwind_next_frame);
-void __unwind_start(struct unwind_state *state, struct task_struct *task,
- struct pt_regs *regs, unsigned long *first_frame)
+void notrace __unwind_start(struct unwind_state *state, struct task_struct *task,
+ struct pt_regs *regs, unsigned long *first_frame)
{
memset(state, 0, sizeof(*state));
diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
index 6407bc9256bf..f2a450ee66e6 100644
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -377,7 +377,7 @@ void __init unwind_init(void)
orc_init = true;
}
-unsigned long unwind_get_return_address(struct unwind_state *state)
+unsigned long notrace unwind_get_return_address(struct unwind_state *state)
{
if (unwind_done(state))
return 0;
@@ -386,7 +386,7 @@ unsigned long unwind_get_return_address(struct unwind_state *state)
}
EXPORT_SYMBOL_GPL(unwind_get_return_address);
-unsigned long *unwind_get_return_address_ptr(struct unwind_state *state)
+unsigned long *notrace unwind_get_return_address_ptr(struct unwind_state *state)
{
if (unwind_done(state))
return NULL;
@@ -481,7 +481,7 @@ static bool get_reg(struct unwind_state *state, unsigned int reg_off,
return false;
}
-bool unwind_next_frame(struct unwind_state *state)
+bool notrace unwind_next_frame(struct unwind_state *state)
{
unsigned long ip_p, sp, tmp, orig_ip = state->ip, prev_sp = state->sp;
enum stack_type prev_type = state->stack_info.type;
@@ -709,8 +709,8 @@ bool unwind_next_frame(struct unwind_state *state)
}
EXPORT_SYMBOL_GPL(unwind_next_frame);
-void __unwind_start(struct unwind_state *state, struct task_struct *task,
- struct pt_regs *regs, unsigned long *first_frame)
+void notrace __unwind_start(struct unwind_state *state, struct task_struct *task,
+ struct pt_regs *regs, unsigned long *first_frame)
{
memset(state, 0, sizeof(*state));
state->task = task;
--
2.39.3
^ permalink raw reply related
* Re: [PATCH 2/4] refcount: add ref_trace_final_put tracepoint
From: Usama Arif @ 2026-07-06 10:07 UTC (permalink / raw)
To: Eugene Mavick via B4 Relay
Cc: Usama Arif, Will Deacon, Peter Zijlstra, Boqun Feng, Mark Rutland,
Gary Guo, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Andrew Morton, Dennis Zhou, Tejun Heo, Christoph Lameter,
linux-kernel, linux-trace-kernel, linux-mm, Eugene Mavick
In-Reply-To: <20260705-refcount-final-put-trace-v1-2-0ae936edb750@mavick.dev>
On Sun, 05 Jul 2026 09:20:52 +0800 Eugene Mavick via B4 Relay <devnull+m.mavick.dev@kernel.org> wrote:
> From: Eugene Mavick <m@mavick.dev>
>
> Add the ref_trace_final_put tracepoint to __refcount_sub_and_test().
> This fires when a refcount_t reaches zero, capturing the caller
> address, the function name, and the refcount_t address.
>
> Signed-off-by: Eugene Mavick <m@mavick.dev>
> ---
> include/linux/refcount.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/include/linux/refcount.h b/include/linux/refcount.h
> index ba7657ced281..7b1fbf326a21 100644
> --- a/include/linux/refcount.h
> +++ b/include/linux/refcount.h
> @@ -107,6 +107,7 @@
> #include <linux/limits.h>
> #include <linux/refcount_types.h>
> #include <linux/spinlock_types.h>
> +#include <linux/ref_trace.h>
>
> struct mutex;
>
> @@ -393,6 +394,7 @@ bool __refcount_sub_and_test(int i, refcount_t *r, int *oldp)
>
> if (old > 0 && old == i) {
> smp_acquire__after_ctrl_dep();
> + trace_ref_final_put(r);
Should refcount_dec_if_one() also fire the tracepoint?
> return true;
> }
>
>
> --
> 2.51.2
>
>
>
^ permalink raw reply
* [PATCH 0/2] Add trace event support for GENI SE registers dump
From: Praveen Talari @ 2026-07-06 11:08 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Mark Brown
Cc: linux-kernel, linux-arm-msm, linux-trace-kernel, linux-spi,
Mukesh Kumar Savaliya, aniket.randive, chandana.chiluveru,
Praveen Talari
The GENI framework is used by multiple drivers including UART, I2C, and
SPI. When hardware-related failures occur, each driver typically relies
on local logging, which often lacks sufficient information to determine
the exact controller state.
This series introduces a common tracing mechanism for GENI Serial Engine
debug registers and demonstrates its use in the SPI driver.
Patch 1 adds a new tracepoint that captures an extensive set of GENI SE
registers, including command state, interrupt status, FIFO state, DMA
configuration, and clock-related information.
Patch 2 hooks the tracepoint into SPI error paths so that register
snapshots are automatically generated when timeouts or transfer-related
failures occur.
Usage examples:
Enable all I2C traces:
echo 1 > /sys/kernel/tracing/events/qcom_geni_se/enable
cat /sys/kernel/debug/tracing/trace_pipe
Example trace output:
114.291299: geni_se_regs: 888000.spi: m_cmd0=0x18000000
m_irq_status=0x00000080 s_cmd0=0x00000000 s_irq_status=0x08000000
geni_status=0x00000000 geni_ios=0x00000000 m_cmd_ctrl=0x00000000
m_cmd_err=0x00000000 m_fw_err=0x00000000 tx_fifo_sts=0x00000000
rx_fifo_sts=0x00000000 tx_watermark=0x00000000 rx_watermark=0x0000000d
rx_watermark_rfr=0x0000000e m_gp_length=0x00000004 s_gp_length=0x00000000
dma_tx_irq=0x00000000 dma_rx_irq=0x00000000 dma_tx_irq_en=0x0000000f
dma_rx_irq_en=0x0000001f dma_rx_len=0x00001400 dma_rx_len_in=0x00001400
dma_tx_len=0x00001400 dma_tx_len_in=0x00001400 dma_tx_ptr_l=0xffffc000
dma_tx_ptr_h=0x00000000 dma_rx_ptr_l=0xffffa000 dma_rx_ptr_h=0x00000000
dma_tx_attr=0x00000001 dma_tx_max_burst=0x00000002 dma_rx_attr=0x00000000
dma_rx_max_burst=0x00000002 dma_if_en=0x00000009 dma_if_en_ro=0x00000001
dma_general_cfg=0x0000000f dma_qsb_trans_cfg=0x00000000 dma_dbg=0x00000000
m_irq_en=0x7fc0007f s_irq_en=0x03003e3e gsi_event_en=0x00000000
se_irq_en=0x0000000f ser_m_clk_cfg=0x000000a1 ser_s_clk_cfg=0x00000000
general_cfg=0x00000048 output_ctrl=0x0000007f clk_ctrl_ro=0x00000001
fifo_if_dis=0x00000000 fw_multilock_msa=0x00000000 clk_sel=0x00000005
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
Praveen Talari (2):
soc: qcom: geni-se: trace: Add trace event support for GENI SE registers dump
spi: qcom-geni: add GENI SE registers trace event on error paths
drivers/spi/spi-geni-qcom.c | 22 ++++-
include/linux/soc/qcom/geni-se.h | 38 +++++++++
include/trace/events/qcom_geni_se.h | 157 ++++++++++++++++++++++++++++++++++++
3 files changed, 213 insertions(+), 4 deletions(-)
---
base-commit: 7de6ae9e12207ec146f2f3f1e58d1a99317e88bc
change-id: 20260630-add-tracepoints-for-se-reg-dump-c2c8bc875658
Best regards,
--
Praveen Talari <praveen.talari@oss.qualcomm.com>
^ permalink raw reply
* [PATCH 1/2] soc: qcom: geni-se: trace: Add trace event support for GENI SE registers dump
From: Praveen Talari @ 2026-07-06 11:08 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Mark Brown
Cc: linux-kernel, linux-arm-msm, linux-trace-kernel, linux-spi,
Mukesh Kumar Savaliya, aniket.randive, chandana.chiluveru,
Praveen Talari
In-Reply-To: <20260706-add-tracepoints-for-se-reg-dump-v1-0-48bd08e28cf2@oss.qualcomm.com>
Add a new trace event header for the Qualcomm GENI Serial Engine (SE)
framework providing a geni_se_regs tracepoint. This tracepoint
captures a comprehensive snapshot of the GENI SE hardware state in a
single trace record, making it possible to correlate register values at
a precise point in time without multiple sequential reads.
The trace event records the following register groups:
- Main/secondary command and IRQ status (M_CMD0, S_CMD0, M/S_IRQ_STATUS)
- Engine status, IOS, and command control/error registers
- TX/RX FIFO status and watermark registers (including RFR watermark)
- M/S GP length registers
- DMA TX/RX IRQ, enable, length, pointer, attribute, and burst registers
- DMA interface enable, general config, QSB trans config, and debug
- M/S IRQ enable, GSI event enable, and top-level SE IRQ enable
- Serial master/slave clock config, general config, output control,
clock control RO, FIFO interface disable, and FW multilock MSA
- Clock select register
Having all these registers captured atomically in a single ftrace record
allows drivers built on top of the GENI SE framework (serial, SPI, I2C)
to invoke this tracepoint on error paths and reconstruct the full engine
state during post-mortem analysis without instrumenting each driver
separately.
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
include/linux/soc/qcom/geni-se.h | 38 +++++++++
include/trace/events/qcom_geni_se.h | 157 ++++++++++++++++++++++++++++++++++++
2 files changed, 195 insertions(+)
diff --git a/include/linux/soc/qcom/geni-se.h b/include/linux/soc/qcom/geni-se.h
index c5e6ab85df09..58c84b5fb3c2 100644
--- a/include/linux/soc/qcom/geni-se.h
+++ b/include/linux/soc/qcom/geni-se.h
@@ -81,13 +81,17 @@ struct geni_se {
};
/* Common SE registers */
+#define SE_DMA_IF_EN 0x4
+#define GENI_GENERAL_CFG 0x10
#define GENI_FORCE_DEFAULT_REG 0x20
#define GENI_OUTPUT_CTRL 0x24
#define SE_GENI_STATUS 0x40
#define GENI_SER_M_CLK_CFG 0x48
#define GENI_SER_S_CLK_CFG 0x4c
+#define GENI_CLK_CTRL_RO 0x60
#define GENI_IF_DISABLE_RO 0x64
#define GENI_FW_REVISION_RO 0x68
+#define GENI_FW_MULTILOCK_MSA_RO 0x74
#define SE_GENI_CLK_SEL 0x7c
#define SE_GENI_CFG_SEQ_START 0x84
#define SE_GENI_DMA_MODE_EN 0x258
@@ -98,6 +102,8 @@ struct geni_se {
#define SE_GENI_M_IRQ_CLEAR 0x618
#define SE_GENI_M_IRQ_EN_SET 0x61c
#define SE_GENI_M_IRQ_EN_CLEAR 0x620
+#define M_CMD_ERR_STATUS 0x624
+#define M_FW_ERR_STATUS 0x628
#define SE_GENI_S_CMD0 0x630
#define SE_GENI_S_CMD_CTRL_REG 0x634
#define SE_GENI_S_IRQ_STATUS 0x640
@@ -115,15 +121,41 @@ struct geni_se {
#define SE_GENI_IOS 0x908
#define SE_GENI_M_GP_LENGTH 0x910
#define SE_GENI_S_GP_LENGTH 0x914
+/* TX DMA registers */
+#define SE_DMA_TX_PTR_L 0xc30
+#define SE_DMA_TX_PTR_H 0xc34
+#define SE_DMA_TX_ATTR 0xc38
+#define SE_DMA_TX_LEN 0xc3c
#define SE_DMA_TX_IRQ_STAT 0xc40
#define SE_DMA_TX_IRQ_CLR 0xc44
+#define SE_DMA_TX_IRQ_EN 0xc48
+#define SE_DMA_TX_IRQ_EN_SET 0xc4c
+#define SE_DMA_TX_IRQ_EN_CLR 0xc50
+#define SE_DMA_TX_LEN_IN 0xc54
#define SE_DMA_TX_FSM_RST 0xc58
+#define SE_DMA_TX_MAX_BURST 0xc5c
+/* RX DMA registers */
+#define SE_DMA_RX_PTR_L 0xd30
+#define SE_DMA_RX_PTR_H 0xd34
+#define SE_DMA_RX_ATTR 0xd38
+#define SE_DMA_RX_LEN 0xd3c
#define SE_DMA_RX_IRQ_STAT 0xd40
#define SE_DMA_RX_IRQ_CLR 0xd44
+#define SE_DMA_RX_IRQ_EN 0xd48
+#define SE_DMA_RX_IRQ_EN_SET 0xd4c
+#define SE_DMA_RX_IRQ_EN_CLR 0xd50
#define SE_DMA_RX_LEN_IN 0xd54
#define SE_DMA_RX_FSM_RST 0xd58
+#define SE_DMA_RX_MAX_BURST 0xd5c
+/* DMA general / debug registers */
+#define SE_GSI_EVENT_EN 0xe18
+#define SE_IRQ_EN 0xe1c
+#define DMA_IF_EN_RO 0xe20
#define SE_HW_PARAM_0 0xe24
#define SE_HW_PARAM_1 0xe28
+#define DMA_GENERAL_CFG 0xe30
+#define SE_DMA_QSB_TRANS_CFG 0xe38
+#define SE_DMA_DEBUG_REG0 0xe40
/* GENI_FORCE_DEFAULT_REG fields */
#define FORCE_DEFAULT BIT(0)
@@ -269,6 +301,12 @@ struct geni_se {
#define RX_GENI_GP_IRQ_EXT GENMASK(13, 12)
#define RX_GENI_CANCEL_IRQ BIT(14)
+/* SE_DMA_DEBUG_REG0 fields */
+#define DMA_TX_ACTIVE BIT(0)
+#define DMA_RX_ACTIVE BIT(1)
+#define DMA_TX_STATE GENMASK(7, 4)
+#define DMA_RX_STATE GENMASK(11, 8)
+
/* SE_HW_PARAM_0 fields */
#define TX_FIFO_WIDTH_MSK GENMASK(29, 24)
#define TX_FIFO_WIDTH_SHFT 24
diff --git a/include/trace/events/qcom_geni_se.h b/include/trace/events/qcom_geni_se.h
new file mode 100644
index 000000000000..4a6e1ba2d147
--- /dev/null
+++ b/include/trace/events/qcom_geni_se.h
@@ -0,0 +1,157 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM qcom_geni_se
+
+#if !defined(_TRACE_QCOM_GENI_SE_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_QCOM_GENI_SE_H
+
+#include <linux/io.h>
+#include <linux/tracepoint.h>
+#include <linux/soc/qcom/geni-se.h>
+
+TRACE_EVENT(geni_se_regs,
+ TP_PROTO(struct geni_se *se),
+
+ TP_ARGS(se),
+
+ TP_STRUCT__entry(__string(geni_se_name, dev_name(se->dev))
+ __field(u32, geni_se_m_cmd0)
+ __field(u32, geni_se_m_irq_status)
+ __field(u32, geni_se_s_cmd0)
+ __field(u32, geni_se_s_irq_status)
+ __field(u32, geni_se_status)
+ __field(u32, geni_se_ios)
+ __field(u32, geni_se_m_cmd_ctrl)
+ __field(u32, geni_se_m_cmd_err)
+ __field(u32, geni_se_m_fw_err)
+ __field(u32, geni_se_tx_fifo_status)
+ __field(u32, geni_se_rx_fifo_status)
+ __field(u32, geni_se_tx_watermark)
+ __field(u32, geni_se_rx_watermark)
+ __field(u32, geni_se_rx_watermark_rfr)
+ __field(u32, geni_se_m_gp_length)
+ __field(u32, geni_se_s_gp_length)
+ __field(u32, geni_se_dma_tx_irq)
+ __field(u32, geni_se_dma_rx_irq)
+ __field(u32, geni_se_dma_tx_irq_en)
+ __field(u32, geni_se_dma_rx_irq_en)
+ __field(u32, geni_se_dma_rx_len)
+ __field(u32, geni_se_dma_rx_len_in)
+ __field(u32, geni_se_dma_tx_len)
+ __field(u32, geni_se_dma_tx_len_in)
+ __field(u32, geni_se_dma_tx_ptr_l)
+ __field(u32, geni_se_dma_tx_ptr_h)
+ __field(u32, geni_se_dma_rx_ptr_l)
+ __field(u32, geni_se_dma_rx_ptr_h)
+ __field(u32, geni_se_dma_tx_attr)
+ __field(u32, geni_se_dma_tx_max_burst)
+ __field(u32, geni_se_dma_rx_attr)
+ __field(u32, geni_se_dma_rx_max_burst)
+ __field(u32, geni_se_dma_if_en)
+ __field(u32, geni_se_dma_if_en_ro)
+ __field(u32, geni_se_dma_general_cfg)
+ __field(u32, geni_se_dma_qsb_trans_cfg)
+ __field(u32, geni_se_dma_dbg)
+ __field(u32, geni_se_m_irq_en)
+ __field(u32, geni_se_s_irq_en)
+ __field(u32, geni_se_gsi_event_en)
+ __field(u32, geni_se_irq_en)
+ __field(u32, geni_se_ser_m_clk_cfg)
+ __field(u32, geni_se_ser_s_clk_cfg)
+ __field(u32, geni_se_general_cfg)
+ __field(u32, geni_se_output_ctrl)
+ __field(u32, geni_se_clk_ctrl_ro)
+ __field(u32, geni_se_fifo_if_disable)
+ __field(u32, geni_se_fw_multilock_msa)
+ __field(u32, geni_se_clk_sel)
+ ),
+
+ TP_fast_assign(__assign_str(geni_se_name);
+ __entry->geni_se_m_cmd0 = readl(se->base + SE_GENI_M_CMD0);
+ __entry->geni_se_m_irq_status = readl(se->base + SE_GENI_M_IRQ_STATUS);
+ __entry->geni_se_s_cmd0 = readl(se->base + SE_GENI_S_CMD0);
+ __entry->geni_se_s_irq_status = readl(se->base + SE_GENI_S_IRQ_STATUS);
+ __entry->geni_se_status = readl(se->base + SE_GENI_STATUS);
+ __entry->geni_se_ios = readl(se->base + SE_GENI_IOS);
+ __entry->geni_se_m_cmd_ctrl = readl(se->base + SE_GENI_M_CMD_CTRL_REG);
+ __entry->geni_se_m_cmd_err = readl(se->base + M_CMD_ERR_STATUS);
+ __entry->geni_se_m_fw_err = readl(se->base + M_FW_ERR_STATUS);
+ __entry->geni_se_tx_fifo_status = readl(se->base + SE_GENI_TX_FIFO_STATUS);
+ __entry->geni_se_rx_fifo_status = readl(se->base + SE_GENI_RX_FIFO_STATUS);
+ __entry->geni_se_tx_watermark = readl(se->base + SE_GENI_TX_WATERMARK_REG);
+ __entry->geni_se_rx_watermark = readl(se->base + SE_GENI_RX_WATERMARK_REG);
+ __entry->geni_se_rx_watermark_rfr = readl(se->base + SE_GENI_RX_RFR_WATERMARK_REG);
+ __entry->geni_se_m_gp_length = readl(se->base + SE_GENI_M_GP_LENGTH);
+ __entry->geni_se_s_gp_length = readl(se->base + SE_GENI_S_GP_LENGTH);
+ __entry->geni_se_dma_tx_irq = readl(se->base + SE_DMA_TX_IRQ_STAT);
+ __entry->geni_se_dma_rx_irq = readl(se->base + SE_DMA_RX_IRQ_STAT);
+ __entry->geni_se_dma_tx_irq_en = readl(se->base + SE_DMA_TX_IRQ_EN);
+ __entry->geni_se_dma_rx_irq_en = readl(se->base + SE_DMA_RX_IRQ_EN);
+ __entry->geni_se_dma_rx_len = readl(se->base + SE_DMA_RX_LEN);
+ __entry->geni_se_dma_rx_len_in = readl(se->base + SE_DMA_RX_LEN_IN);
+ __entry->geni_se_dma_tx_len = readl(se->base + SE_DMA_TX_LEN);
+ __entry->geni_se_dma_tx_len_in = readl(se->base + SE_DMA_TX_LEN_IN);
+ __entry->geni_se_dma_tx_ptr_l = readl(se->base + SE_DMA_TX_PTR_L);
+ __entry->geni_se_dma_tx_ptr_h = readl(se->base + SE_DMA_TX_PTR_H);
+ __entry->geni_se_dma_rx_ptr_l = readl(se->base + SE_DMA_RX_PTR_L);
+ __entry->geni_se_dma_rx_ptr_h = readl(se->base + SE_DMA_RX_PTR_H);
+ __entry->geni_se_dma_tx_attr = readl(se->base + SE_DMA_TX_ATTR);
+ __entry->geni_se_dma_tx_max_burst = readl(se->base + SE_DMA_TX_MAX_BURST);
+ __entry->geni_se_dma_rx_attr = readl(se->base + SE_DMA_RX_ATTR);
+ __entry->geni_se_dma_rx_max_burst = readl(se->base + SE_DMA_RX_MAX_BURST);
+ __entry->geni_se_dma_if_en = readl(se->base + SE_DMA_IF_EN);
+ __entry->geni_se_dma_if_en_ro = readl(se->base + DMA_IF_EN_RO);
+ __entry->geni_se_dma_general_cfg = readl(se->base + DMA_GENERAL_CFG);
+ __entry->geni_se_dma_qsb_trans_cfg = readl(se->base + SE_DMA_QSB_TRANS_CFG);
+ __entry->geni_se_dma_dbg = readl(se->base + SE_DMA_DEBUG_REG0);
+ __entry->geni_se_m_irq_en = readl(se->base + SE_GENI_M_IRQ_EN);
+ __entry->geni_se_s_irq_en = readl(se->base + SE_GENI_S_IRQ_EN);
+ __entry->geni_se_gsi_event_en = readl(se->base + SE_GSI_EVENT_EN);
+ __entry->geni_se_irq_en = readl(se->base + SE_IRQ_EN);
+ __entry->geni_se_ser_m_clk_cfg = readl(se->base + GENI_SER_M_CLK_CFG);
+ __entry->geni_se_ser_s_clk_cfg = readl(se->base + GENI_SER_S_CLK_CFG);
+ __entry->geni_se_general_cfg = readl(se->base + GENI_GENERAL_CFG);
+ __entry->geni_se_output_ctrl = readl(se->base + GENI_OUTPUT_CTRL);
+ __entry->geni_se_clk_ctrl_ro = readl(se->base + GENI_CLK_CTRL_RO);
+ __entry->geni_se_fifo_if_disable = readl(se->base + GENI_IF_DISABLE_RO);
+ __entry->geni_se_fw_multilock_msa = readl(se->base + GENI_FW_MULTILOCK_MSA_RO);
+ __entry->geni_se_clk_sel = readl(se->base + SE_GENI_CLK_SEL);
+ ),
+
+ TP_printk("%s: m_cmd0=0x%08x m_irq_status=0x%08x s_cmd0=0x%08x s_irq_status=0x%08x geni_status=0x%08x geni_ios=0x%08x m_cmd_ctrl=0x%08x m_cmd_err=0x%08x m_fw_err=0x%08x tx_fifo_sts=0x%08x rx_fifo_sts=0x%08x tx_watermark=0x%08x rx_watermark=0x%08x rx_watermark_rfr=0x%08x m_gp_length=0x%08x s_gp_length=0x%08x dma_tx_irq=0x%08x dma_rx_irq=0x%08x dma_tx_irq_en=0x%08x dma_rx_irq_en=0x%08x dma_rx_len=0x%08x dma_rx_len_in=0x%08x dma_tx_len=0x%08x dma_tx_len_in=0x%08x dma_tx_ptr_l=0x%08x dma_tx_ptr_h=0x%08x dma_rx_ptr_l=0x%08x dma_rx_ptr_h=0x%08x dma_tx_attr=0x%08x dma_tx_max_burst=0x%08x dma_rx_attr=0x%08x dma_rx_max_burst=0x%08x dma_if_en=0x%08x dma_if_en_ro=0x%08x dma_general_cfg=0x%08x dma_qsb_trans_cfg=0x%08x dma_dbg=0x%08x m_irq_en=0x%08x s_irq_en=0x%08x gsi_event_en=0x%08x se_irq_en=0x%08x ser_m_clk_cfg=0x%08x ser_s_clk_cfg=0x%08x general_cfg=0x%08x output_ctrl=0x%08x clk_ctrl_ro=0x%08x fifo_if_dis=0x%08x fw_multilock_msa=0x%08x clk_sel=0x%08x",
+ __get_str(geni_se_name),
+ __entry->geni_se_m_cmd0, __entry->geni_se_m_irq_status,
+ __entry->geni_se_s_cmd0, __entry->geni_se_s_irq_status,
+ __entry->geni_se_status, __entry->geni_se_ios,
+ __entry->geni_se_m_cmd_ctrl,
+ __entry->geni_se_m_cmd_err, __entry->geni_se_m_fw_err,
+ __entry->geni_se_tx_fifo_status, __entry->geni_se_rx_fifo_status,
+ __entry->geni_se_tx_watermark, __entry->geni_se_rx_watermark,
+ __entry->geni_se_rx_watermark_rfr,
+ __entry->geni_se_m_gp_length, __entry->geni_se_s_gp_length,
+ __entry->geni_se_dma_tx_irq, __entry->geni_se_dma_rx_irq,
+ __entry->geni_se_dma_tx_irq_en, __entry->geni_se_dma_rx_irq_en,
+ __entry->geni_se_dma_rx_len, __entry->geni_se_dma_rx_len_in,
+ __entry->geni_se_dma_tx_len, __entry->geni_se_dma_tx_len_in,
+ __entry->geni_se_dma_tx_ptr_l, __entry->geni_se_dma_tx_ptr_h,
+ __entry->geni_se_dma_rx_ptr_l, __entry->geni_se_dma_rx_ptr_h,
+ __entry->geni_se_dma_tx_attr, __entry->geni_se_dma_tx_max_burst,
+ __entry->geni_se_dma_rx_attr, __entry->geni_se_dma_rx_max_burst,
+ __entry->geni_se_dma_if_en, __entry->geni_se_dma_if_en_ro,
+ __entry->geni_se_dma_general_cfg, __entry->geni_se_dma_qsb_trans_cfg,
+ __entry->geni_se_dma_dbg,
+ __entry->geni_se_m_irq_en, __entry->geni_se_s_irq_en,
+ __entry->geni_se_gsi_event_en, __entry->geni_se_irq_en,
+ __entry->geni_se_ser_m_clk_cfg, __entry->geni_se_ser_s_clk_cfg,
+ __entry->geni_se_general_cfg, __entry->geni_se_output_ctrl,
+ __entry->geni_se_clk_ctrl_ro, __entry->geni_se_fifo_if_disable,
+ __entry->geni_se_fw_multilock_msa, __entry->geni_se_clk_sel)
+);
+
+#endif /* _TRACE_QCOM_GENI_SE_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
--
2.34.1
^ permalink raw reply related
* [PATCH 2/2] spi: qcom-geni: add GENI SE registers trace event on error paths
From: Praveen Talari @ 2026-07-06 11:08 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Mark Brown
Cc: linux-kernel, linux-arm-msm, linux-trace-kernel, linux-spi,
Mukesh Kumar Savaliya, aniket.randive, chandana.chiluveru,
Praveen Talari
In-Reply-To: <20260706-add-tracepoints-for-se-reg-dump-v1-0-48bd08e28cf2@oss.qualcomm.com>
The GENI SPI driver reports various transfer failures such as command
timeouts, DMA reset timeouts, DMA transaction errors, and unexpected
interrupt conditions. However, diagnosing the root cause of these
failures is difficult as the hardware state is not captured when the
error occurs.
Add trace_geni_se_regs() calls at critical SPI error handling paths to
automatically capture GENI serial engine debug registers when failures
are detected. This includes:
- M_CMD abort/cancel timeout
- DMA TX/RX FSM reset timeout
- DMA transaction failures and pending residue conditions
- Unexpected interrupt error status
- Premature transfer completion with pending TX/RX data
Dumping the SE debug registers at the time of failure provides
additional hardware context and significantly improves post-mortem
analysis of SPI transfer issues without affecting normal operation.
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
drivers/spi/spi-geni-qcom.c | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index 26e723cfea61..7db0836308c2 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -3,6 +3,7 @@
#define CREATE_TRACE_POINTS
#include <trace/events/qcom_geni_spi.h>
+#include <trace/events/qcom_geni_se.h>
#include <linux/clk.h>
#include <linux/dmaengine.h>
@@ -184,6 +185,7 @@ static void handle_se_timeout(struct spi_controller *spi)
time_left = wait_for_completion_timeout(&mas->abort_done, HZ);
if (!time_left) {
dev_err(mas->dev, "Failed to cancel/abort m_cmd\n");
+ trace_geni_se_regs(se);
/*
* No need for a lock since SPI core has a lock and we never
@@ -201,8 +203,10 @@ static void handle_se_timeout(struct spi_controller *spi)
writel(1, se->base + SE_DMA_TX_FSM_RST);
spin_unlock_irq(&mas->lock);
time_left = wait_for_completion_timeout(&mas->tx_reset_done, HZ);
- if (!time_left)
+ if (!time_left) {
dev_err(mas->dev, "DMA TX RESET failed\n");
+ trace_geni_se_regs(se);
+ }
}
if (xfer->rx_buf) {
spin_lock_irq(&mas->lock);
@@ -210,8 +214,10 @@ static void handle_se_timeout(struct spi_controller *spi)
writel(1, se->base + SE_DMA_RX_FSM_RST);
spin_unlock_irq(&mas->lock);
time_left = wait_for_completion_timeout(&mas->rx_reset_done, HZ);
- if (!time_left)
+ if (!time_left) {
dev_err(mas->dev, "DMA RX RESET failed\n");
+ trace_geni_se_regs(se);
+ }
}
} else {
/*
@@ -382,10 +388,12 @@ static void
spi_gsi_callback_result(void *cb, const struct dmaengine_result *result)
{
struct spi_controller *spi = cb;
+ struct spi_geni_master *mas = spi_controller_get_devdata(spi);
spi->cur_msg->status = -EIO;
if (result->result != DMA_TRANS_NOERROR) {
dev_err(&spi->dev, "DMA txn failed: %d\n", result->result);
+ trace_geni_se_regs(&mas->se);
spi_finalize_current_transfer(spi);
return;
}
@@ -395,6 +403,7 @@ spi_gsi_callback_result(void *cb, const struct dmaengine_result *result)
dev_dbg(&spi->dev, "DMA txn completed\n");
} else {
dev_err(&spi->dev, "DMA xfer has pending: %d\n", result->residue);
+ trace_geni_se_regs(&mas->se);
}
spi_finalize_current_transfer(spi);
@@ -941,8 +950,10 @@ static irqreturn_t geni_spi_isr(int irq, void *data)
if (m_irq & (M_CMD_OVERRUN_EN | M_ILLEGAL_CMD_EN | M_CMD_FAILURE_EN |
M_RX_FIFO_RD_ERR_EN | M_RX_FIFO_WR_ERR_EN |
- M_TX_FIFO_RD_ERR_EN | M_TX_FIFO_WR_ERR_EN))
+ M_TX_FIFO_RD_ERR_EN | M_TX_FIFO_WR_ERR_EN)) {
dev_warn(mas->dev, "Unexpected IRQ err status %#010x\n", m_irq);
+ trace_geni_se_regs(se);
+ }
spin_lock(&mas->lock);
@@ -974,10 +985,13 @@ static irqreturn_t geni_spi_isr(int irq, void *data)
writel(0, se->base + SE_GENI_TX_WATERMARK_REG);
dev_err(mas->dev, "Premature done. tx_rem = %d bpw%d\n",
mas->tx_rem_bytes, mas->cur_bits_per_word);
+ trace_geni_se_regs(se);
}
- if (mas->rx_rem_bytes)
+ if (mas->rx_rem_bytes) {
dev_err(mas->dev, "Premature done. rx_rem = %d bpw%d\n",
mas->rx_rem_bytes, mas->cur_bits_per_word);
+ trace_geni_se_regs(se);
+ }
} else {
complete(&mas->cs_done);
}
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v10 2/6] mm/memory-failure: surface unhandlable kernel pages as -ENOTRECOVERABLE
From: Miaohe Lin @ 2026-07-06 12:22 UTC (permalink / raw)
To: Breno Leitao
Cc: linux-mm, linux-kernel, linux-doc, linux-kselftest,
linux-trace-kernel, kernel-team, Andrew Morton, David Hildenbrand,
Lorenzo Stoakes, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Shuah Khan, Naoya Horiguchi,
Jonathan Corbet, Shuah Khan, Liam R. Howlett, lance.yang,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers
In-Reply-To: <20260630-ecc_panic-v10-2-c6ed5b62eea2@debian.org>
On 2026/6/30 20:46, Breno Leitao wrote:
> get_any_page() collapses every HWPoisonHandlable() rejection into a
> single -EIO via the __get_hwpoison_page() -> -EBUSY -> shake_page()
> -> retry path. That is correct for the transient case (a userspace
> folio briefly off LRU during migration or compaction, which a later
> shake can drag back), but wrong for stable kernel-owned pages: slab,
> page-table, large-kmalloc and PG_reserved pages will never become
> HWPoisonHandlable(), so the retry loop is wasted work and the final
> -EIO loses the "this is structurally unrecoverable" information.
> memory_failure() then maps -EIO into MF_MSG_GET_HWPOISON, which the
> panic-on-unrecoverable sysctl deliberately does not act on.
>
> Introduce is_kernel_owned_page(), a small predicate that positively
> identifies pages the hwpoison handler cannot recover from:
>
> is_kernel_owned_page(p) :=
> PageReserved(p) ||
> PageSlab(head) || PageTable(head) || PageLargeKmalloc(head)
>
> where head = compound_head(p).
>
> PG_reserved is a per-page flag (PF_NO_COMPOUND) and is tested on the
> page directly. The slab, page-table and large-kmalloc page-type bits
> are only stored on the head page, so those tests resolve the compound
> head first, then re-read compound_head(page) afterwards: a concurrent
> split or compound free that moves head invalidates the just-read flags
> and the loop retries. The lookup still takes no refcount, mirroring
> the rest of get_any_page(); the recheck closes the common split race,
> and a residual free->alloc->free in the same window can only mis-tag
> a genuinely poisoned page, never reclassify a handlable one.
>
> No MF_SOFT_OFFLINE / page_has_movable_ops() opt-out is needed: a
> movable_ops page is always PageOffline or PageZsmalloc, whose
> page_type is mutually exclusive with slab, page-table and
> large-kmalloc, and it never carries PG_reserved, so it can never
> match any of the checks above.
>
> The list is intentionally not exhaustive. vmalloc and kernel-stack
> pages, for example, do not carry a page_type bit and would need a
> different oracle; they keep going through the existing retry path
> unchanged. This is the smallest set we can identify with certainty
> by page type.
>
> Wire the helper into the top of get_any_page() to short-circuit
> those pages before the retry loop runs. On a hit, drop the caller's
> MF_COUNT_INCREASED reference (if any) and return -ENOTRECOVERABLE
> straight away. Pages outside the helper's positive list still take
> the existing retry path and return -EIO, leaving operator-visible
> behaviour for those cases unchanged.
>
> Extend the unhandlable-page pr_err() to fire for either errno and
> update the get_hwpoison_page() kerneldoc to document the new return.
>
> memory_failure() still folds every negative return into
> MF_MSG_GET_HWPOISON via its existing "else if (res < 0)" branch, so
> this patch on its own only changes the errno that soft_offline_page()
> can propagate to its callers. A follow-up wires -ENOTRECOVERABLE
> through memory_failure() and reports MF_MSG_KERNEL for the
> unrecoverable cases, which is what the
> panic_on_unrecoverable_memory_failure sysctl observes.
>
> Suggested-by: David Hildenbrand <david@kernel.org>
> Suggested-by: Lance Yang <lance.yang@linux.dev>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
> mm/memory-failure.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 50 insertions(+), 2 deletions(-)
>
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index f4d3e6e20e13f..087658484e242 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -1325,6 +1325,38 @@ static inline bool HWPoisonHandlable(struct page *page, unsigned long flags)
> return PageLRU(page) || is_free_buddy_page(page);
> }
>
> +/*
> + * Positive identification of pages the hwpoison handler cannot recover:
> + * pages owned by kernel internals with no userspace mapping to unmap, no
> + * file mapping to invalidate, and no migration target.
> + */
> +static inline bool is_kernel_owned_page(struct page *page)
> +{
> + struct page *head;
> + bool kernel_owned;
> +
> + /* PG_reserved is a per-page flag, never set on a compound page. */
> + if (PageReserved(page))
> + return true;
> +
> + /*
> + * Page-type bits live only on the head page, so resolve any tail
> + * first. The check takes no refcount; recheck the head afterwards
> + * so a concurrent split or compound free cannot leave us trusting
> + * a stale view. A residual free->alloc->free cannot be closed here
> + * (frozen slab and large-kmalloc pages cannot be pinned), but is
> + * harmless: where a wrong verdict could panic, memory_failure() has
> + * already set PageHWPoison, which bars the page from the allocator.
> + */
> +retry:
> + head = compound_head(page);
It's irrelevant to this issue but should we use folio?
Anyway, this patch looks good to me.
Acked-by: Miaohe Lin <linmiaohe@huawei.com>
Thanks.
.
^ permalink raw reply
* Re: [PATCH v2 1/2] signal: avoid shared siginfo namespace rewrites
From: Christian Brauner @ 2026-07-06 12:39 UTC (permalink / raw)
To: Bradley Morgan
Cc: Oleg Nesterov, Christian Brauner, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Andrew Morton,
Peter Zijlstra, Marco Elver, Aleksandr Nogikh, Thomas Gleixner,
Adrian Huang, Kexin Sun, linux-kernel, linux-trace-kernel, stable
In-Reply-To: <86a8857d58d43ee26a8b365b837fd24830343494.1782159692.git.include@grrlz.net>
> send_signal_locked() rewrites sender ids for the target namespace.
> Group sends reuse the same siginfo, so one recipient can affect the
> next.
>
> Copy the siginfo before changing it.
>
> Fixes: 7a0cf094944e ("signal: Correct namespace fixups of si_pid and si_uid")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bradley Morgan <include@grrlz.net>
Reviewed-by: Christian Brauner (Amutable) <brauner@kernel.org> Reviewed-by: Christian Brauner (Amutable) <brauner@kernel.org>
--
Christian Brauner <brauner@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 2/2] signal: make send_signal_locked() take const siginfo
From: Christian Brauner @ 2026-07-06 12:39 UTC (permalink / raw)
To: Bradley Morgan
Cc: Oleg Nesterov, Christian Brauner, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Andrew Morton,
Peter Zijlstra, Marco Elver, Aleksandr Nogikh, Thomas Gleixner,
Adrian Huang, Kexin Sun, linux-kernel, linux-trace-kernel
In-Reply-To: <f754c4e5c82b45bcbb770aa8bb1f4ab1d87a0b0e.1782159692.git.include@grrlz.net>
> send_signal_locked() should not change the caller's siginfo. Make that
> part of the type and keep the local rewrite on its copy.
>
> Suggested-by: Oleg Nesterov <oleg@redhat.com>
> Signed-off-by: Bradley Morgan <include@grrlz.net>
Reviewed-by: Christian Brauner (Amutable) <brauner@kernel.org>
--
Christian Brauner <brauner@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 2/2] signal: make send_signal_locked() take const siginfo
From: Oleg Nesterov @ 2026-07-06 13:33 UTC (permalink / raw)
To: Christian Brauner
Cc: Bradley Morgan, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Andrew Morton, Peter Zijlstra, Marco Elver,
Aleksandr Nogikh, Thomas Gleixner, Adrian Huang, Kexin Sun,
linux-kernel, linux-trace-kernel
In-Reply-To: <20260706-karpfen-beruhen-ausschalten-9f2b0a55b8de@brauner>
Just in case, I can't read the code until Wednesday, but...
On 07/06, Christian Brauner wrote:
>
> > send_signal_locked() should not change the caller's siginfo. Make that
> > part of the type and keep the local rewrite on its copy.
> >
> > Suggested-by: Oleg Nesterov <oleg@redhat.com>
> > Signed-off-by: Bradley Morgan <include@grrlz.net>
>
> Reviewed-by: Christian Brauner (Amutable) <brauner@kernel.org>
Agreed, but IIRC this change should be rebased on top of -mm tree (I sent
the patch which changes send_signal_locked(), and thanks for your review btw!)
And, again iirc, with that patch "make siginfo const" become really trivial,
we only need to add "const" to every "kernel_siginfo *info" in the
send_signal_locked()'s callchain.
Oleg.
^ permalink raw reply
* Re: [PATCH 1/4] tracing: add ref_trace_final_put tracepoint
From: Steven Rostedt @ 2026-07-06 13:33 UTC (permalink / raw)
To: Eugene Mavick
Cc: Will Deacon, Peter Zijlstra, Boqun Feng, Mark Rutland, Gary Guo,
Masami Hiramatsu, Mathieu Desnoyers, Andrew Morton, Dennis Zhou,
Tejun Heo, Christoph Lameter, linux-kernel, linux-trace-kernel,
linux-mm
In-Reply-To: <20260705-refcount-final-put-trace-v1-1-cdd0014626a9@mavick.dev>
On Sun, 05 Jul 2026 07:19:20 +0800
Eugene Mavick <m@mavick.dev> wrote:
> +++ b/lib/ref_trace.c
> @@ -0,0 +1,12 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#define CREATE_TRACE_POINTS
> +#include <trace/events/ref_trace.h>
> +
> +//Wrapper function for functions defined entirely in header files
> +void do_ref_trace_final_put(unsigned long caller, const char *fn, const void *obj)
> +{
> + trace_ref_trace_final_put(caller, fn, obj);
> +}
> +EXPORT_SYMBOL_GPL(do_ref_trace_final_put);
Since this is only called when ref_trace_final_put tracepoint is enabled,
you can use the direct call that doesn't use the static_branch(). The above
should be:
//Wrapper function for functions defined entirely in header files
void do_ref_trace_final_put(unsigned long caller, const char *fn, const void *obj)
{
trace_call__ref_trace_final_put(caller, fn, obj);
}
-- Steve
^ permalink raw reply
* Re: [PATCH 1/4] tracing: add ref_trace_final_put tracepoint
From: Steven Rostedt @ 2026-07-06 13:36 UTC (permalink / raw)
To: Eugene Mavick
Cc: Will Deacon, Peter Zijlstra, Boqun Feng, Mark Rutland, Gary Guo,
Masami Hiramatsu, Mathieu Desnoyers, Andrew Morton, Dennis Zhou,
Tejun Heo, Christoph Lameter, linux-kernel, linux-trace-kernel,
linux-mm
In-Reply-To: <20260705-refcount-final-put-trace-v1-1-cdd0014626a9@mavick.dev>
On Sun, 05 Jul 2026 07:19:20 +0800
Eugene Mavick <m@mavick.dev> wrote:
> +#ifdef CONFIG_TRACEPOINTS
> +/* Wrapper function implemented in lib/ref_trace.c */
> +extern void do_ref_trace_final_put(unsigned long caller, const char *fn, const void *obj);
> +
> +#define trace_ref_final_put(obj) \
> + do { \
> + if (tracepoint_enabled(ref_trace_final_put)) \
> + do_ref_trace_final_put(_RET_IP_, __func__, obj); \
> + } while (0)
> +
Also, you may want to make the macro called do_trace_ref_final_put(), to
show that it is not a tracepoint (which all start with "trace_").
-- Steve
^ permalink raw reply
* Re: [PATCH v2 2/2] signal: make send_signal_locked() take const siginfo
From: Bradley Morgan @ 2026-07-06 13:38 UTC (permalink / raw)
To: Oleg Nesterov, Christian Brauner
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Andrew Morton, Peter Zijlstra, Marco Elver, Aleksandr Nogikh,
Thomas Gleixner, Adrian Huang, Kexin Sun, linux-kernel,
linux-trace-kernel
In-Reply-To: <akuusnRXP_tyuiel@redhat.com>
On July 6, 2026 2:33:38 PM GMT+01:00, Oleg Nesterov <oleg@redhat.com>
wrote:
>Just in case, I can't read the code until Wednesday, but...
>
>On 07/06, Christian Brauner wrote:
>>
>> > send_signal_locked() should not change the caller's siginfo. Make that
>> > part of the type and keep the local rewrite on its copy.
>> >
>> > Suggested-by: Oleg Nesterov <oleg@redhat.com>
>> > Signed-off-by: Bradley Morgan <include@grrlz.net>
>>
>> Reviewed-by: Christian Brauner (Amutable) <brauner@kernel.org>
>
>Agreed, but IIRC this change should be rebased on top of -mm tree (I sent
>the patch which changes send_signal_locked(), and thanks for your review
>btw!)
>
>And, again iirc, with that patch "make siginfo const" become really
>trivial,
>we only need to add "const" to every "kernel_siginfo *info" in the
>send_signal_locked()'s callchain.
>
>Oleg.
>
>
Makes sense, thanks a lot.
Thanks!
^ permalink raw reply
* Re: [PATCH v3 04/11] arm64/mm: Add set_memory_device() and set_memory_normal()
From: Thierry Reding @ 2026-07-06 13:49 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: <akftuw9NyRy36fXA@willie-the-truck>
[-- Attachment #1: Type: text/plain, Size: 4204 bytes --]
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?
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 1/3] rv/reactors: fix lockdep "Invalid wait context" in rv_react()
From: Gabriele Monaco @ 2026-07-06 14:02 UTC (permalink / raw)
To: Thomas Weißschuh, wen.yang; +Cc: Nam Cao, linux-trace-kernel, linux-kernel
In-Reply-To: <20260623112942-4943fc6a-c1d5-4eed-a91f-66b3a034caa7@linutronix.de>
On Tue, 2026-06-23 at 11:38 +0200, Thomas Weißschuh wrote:
> This now allows reactors to take (raw) spinlocks. The original idea was
> to not allow that as a reactor can be called from LD_WAIT_FREE context.
> So I am not sure this is the right fix. Not that I have a better one
> available right now.
As far as I understand it, LD_WAIT_FREE is fairly impossible to apply on
preemptible code (here we see it hit by an interrupt).
Since we kind of have to allow raw spinlock to avoid this (even if we don't take
them explicitly), why wouldn't a reactor ever be allowed to take raw spinlocks?
Technically it wouldn't be wrong to take locks from RV monitors, although most
monitors don't do it explicitly.
If we ever happen to take the wrong lock explicitly from a reactor triggered by
a nasty event (e.g. sched_switch), I believe lockdep would still be complaining
down that path, so we probably don't need to be too strict in rv_react().
Obviously we don't want to disable interrutps for LD_WAIT_FREE to hold either.
Am I missing something?
Thanks,
Gabriele
^ permalink raw reply
* Re: [PATCH] ufs: core: tracing: Do not dereference pointers in TP_printk()
From: Bart Van Assche @ 2026-07-06 14:22 UTC (permalink / raw)
To: Steven Rostedt, LKML, Linux Trace Kernel, linux-scsi
Cc: Masami Hiramatsu, Mathieu Desnoyers, Alim Akhtar, Avri Altman,
James Bottomley, Martin K. Petersen, Peter Wang
In-Reply-To: <20260630185412.283c26c5@gandalf.local.home>
On 6/30/26 3:54 PM, Steven Rostedt wrote:
> The trace events in drivers/ufs/core/ufs_trace.h were converted to take a
> pointer to the hba structure as an argument for the tracepoint and then in
> TP_printk() the printing of the dev_name from the ring buffer was
> converted to using the dev dereferenced pointer from the hba saved
> pointer.
>
> This is not allowed as the TP_printk() is executed at the time the trace
> event is read from /sys/kernel/tracing/trace file. That can happen
> literally, seconds, minutes, hours, weeks, days, or even months later!
> There is no guarantee that the hba pointer will still exist by the time it
> is dereferenced when the "trace" file is read.
>
> Instead, save the device name from the hba pointer at the time the
> tracepoint is called and place it into the ring buffer event. Then the
> TP_printk() can read the name directly from the ring buffer and remove the
> possibility that it will read a freed pointer and crash the kernel.
>
> This was detected when testing the trace event code that looks for
> TP_printk() parameters doing illegal derferences[1]
>
> [1] https://lore.kernel.org/all/20260630184836.74d477b6@gandalf.local.home/
Thanks Steven!
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
^ permalink raw reply
* Re: [PATCH 2/3] rv/reactors: add KUnit tests for reactor_printk
From: Gabriele Monaco @ 2026-07-06 14:41 UTC (permalink / raw)
To: wen.yang; +Cc: Nam Cao, linux-trace-kernel, linux-kernel
In-Reply-To: <690593305ad075539a804e0bf94493335354e6b9.1781541556.git.wen.yang@linux.dev>
On Tue, 2026-06-16 at 00:44 +0800, wen.yang@linux.dev wrote:
> From: Wen Yang <wen.yang@linux.dev>
>
> Add KUnit tests for the printk reactor covering:
> - Reactor registration and unregistration lifecycle
> - React callback invocation via rv_react()
> - Double registration rejection
> - Multiple register/unregister cycles
>
> The mock callback calls vprintk_deferred() — the same path as the real
> reactor — then busy-waits to simulate I/O back-pressure, exercising the
> LD_WAIT_FREE constraint of rv_react() under load.
>
> Signed-off-by: Wen Yang <wen.yang@linux.dev>
> ---
> kernel/trace/rv/Kconfig | 10 ++
> kernel/trace/rv/Makefile | 1 +
> kernel/trace/rv/reactor_printk_kunit.c | 123 +++++++++++++++++++++++++
> 3 files changed, 134 insertions(+)
> create mode 100644 kernel/trace/rv/reactor_printk_kunit.c
>
> diff --git a/kernel/trace/rv/Kconfig b/kernel/trace/rv/Kconfig
> index 3884b14df375..ff47895c897f 100644
> --- a/kernel/trace/rv/Kconfig
> +++ b/kernel/trace/rv/Kconfig
> @@ -104,6 +104,16 @@ config RV_REACT_PRINTK
> Enables the printk reactor. The printk reactor emits a printk()
> message if an exception is found.
>
> +config RV_REACT_PRINTK_KUNIT
> + bool "KUnit tests for reactor_printk" if !KUNIT_ALL_TESTS
> + depends on RV_REACT_PRINTK && KUNIT
> + default KUNIT_ALL_TESTS
> + help
> + This builds KUnit tests for the printk reactor. These are only
> + for development and testing, not for regular kernel use cases.
> +
> + If unsure, say N.
> +
> config RV_REACT_PANIC
> bool "Panic reactor"
> depends on RV_REACTORS
> diff --git a/kernel/trace/rv/Makefile b/kernel/trace/rv/Makefile
> index 94498da35b37..ef0a2dcb927c 100644
> --- a/kernel/trace/rv/Makefile
> +++ b/kernel/trace/rv/Makefile
> @@ -23,4 +23,5 @@ obj-$(CONFIG_RV_MON_NOMISS) += monitors/nomiss/nomiss.o
> # Add new monitors here
> obj-$(CONFIG_RV_REACTORS) += rv_reactors.o
> obj-$(CONFIG_RV_REACT_PRINTK) += reactor_printk.o
> +obj-$(CONFIG_RV_REACT_PRINTK_KUNIT) += reactor_printk_kunit.o
> obj-$(CONFIG_RV_REACT_PANIC) += reactor_panic.o
> diff --git a/kernel/trace/rv/reactor_printk_kunit.c
> b/kernel/trace/rv/reactor_printk_kunit.c
> new file mode 100644
> index 000000000000..933aa5602226
> --- /dev/null
> +++ b/kernel/trace/rv/reactor_printk_kunit.c
> @@ -0,0 +1,123 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit tests for reactor_printk
> + *
> + */
> +
> +#include <kunit/test.h>
> +#include <linux/rv.h>
> +#include <linux/printk.h>
> +#include <linux/sched/clock.h>
> +#include <linux/processor.h>
> +
> +/*
> + * Simulated execution time for mock_printk_react (sched_clock units,
> + * nanoseconds). Models the time a real printk reactor callback may consume
> + * under I/O pressure, exercising the LD_WAIT_FREE constraint of rv_react().
> + */
> +#define MOCK_REACT_DURATION_NS 5000000ULL
> +
> +/*
> + * Mock react callback mirroring rv_printk_reaction().
> + *
> + * Calls vprintk_deferred() — the same path as the real reactor — then holds
> + * the CPU for MOCK_REACT_DURATION_NS via a sched_clock() timed busy-loop,
> + * simulating a callback that is slow due to I/O back-pressure.
> + * sched_clock() is notrace and lock-free; no sleep or lock acquisition is
> + * performed, satisfying the LD_WAIT_FREE constraint of rv_react().
> + */
> +__printf(1, 0) static void mock_printk_react(const char *msg, va_list args)
> +{
> + u64 start = sched_clock();
> +
I'm fine testing something that looks like the printk reactor rather than the
real thing, but here sched_clock() is playing with preemption and could trigger
one, this isn't accurate.
I'm not sure all implementations are free from this problem, but why don't you
just use mdelay() here?
> + vprintk_deferred(msg, args);
> +
> + while (sched_clock() - start < MOCK_REACT_DURATION_NS)
> + cpu_relax();
> +}
> +
> +static struct rv_reactor mock_printk_reactor = {
> + .name = "test_printk",
> + .description = "test printk reactor",
> + .react = mock_printk_react,
> +};
> +
> +/* Test 1: register and unregister reactor */
> +static void test_printk_register_unregister(struct kunit *test)
> +{
> + int ret;
> +
> + ret = rv_register_reactor(&mock_printk_reactor);
> + KUNIT_EXPECT_EQ(test, ret, 0);
Yeah checking the return value is the maximum we can do without exporting
internal static functions. I'd say we don't need to be too pedantic on that.
> + KUNIT_EXPECT_STREQ(test, mock_printk_reactor.name, "test_printk");
> +
> + rv_unregister_reactor(&mock_printk_reactor);
> +}
> +
> +/* Test 2: react callback is invoked via rv_react() */
> +static void test_printk_react_called(struct kunit *test)
> +{
> + struct rv_reactor reactor = {
> + .name = "printk_cb_test",
> + .react = mock_printk_react,
> + };
> + struct rv_monitor monitor = {
> + .name = "test_monitor",
> + .reactor = &reactor,
> + .react = mock_printk_react,
> + };
> +
> + rv_react(&monitor, "printk violation message");
> +}
> +
> +/* Test 3: double registration should fail */
> +static void test_printk_double_register(struct kunit *test)
> +{
> + int ret;
> +
> + ret = rv_register_reactor(&mock_printk_reactor);
> + KUNIT_ASSERT_EQ(test, ret, 0);
> +
> + ret = rv_register_reactor(&mock_printk_reactor);
> + KUNIT_EXPECT_NE(test, ret, 0);
> +
> + rv_unregister_reactor(&mock_printk_reactor);
> +}
> +
> +/* Test 4: register/unregister cycle */
> +static void test_printk_register_cycle(struct kunit *test)
> +{
> + int ret, i;
> +
> + for (i = 0; i < 5; i++) {
> + ret = rv_register_reactor(&mock_printk_reactor);
> + KUNIT_EXPECT_EQ(test, ret, 0);
> +
> + rv_unregister_reactor(&mock_printk_reactor);
> + }
> +}
> +
> +/* Test 5: react callback is not NULL (printk reactors must provide react) */
> +static void test_printk_react_not_null(struct kunit *test)
> +{
> + KUNIT_EXPECT_NOT_NULL(test, mock_printk_reactor.react);
As others pointed out, those assertions on a variable you control aren't needed.
Thanks,
Gabriele
> +}
> +
> +static struct kunit_case reactor_printk_kunit_cases[] = {
> + KUNIT_CASE(test_printk_register_unregister),
> + KUNIT_CASE(test_printk_react_called),
> + KUNIT_CASE(test_printk_double_register),
> + KUNIT_CASE(test_printk_register_cycle),
> + KUNIT_CASE(test_printk_react_not_null),
> + {}
> +};
> +
> +static struct kunit_suite reactor_printk_kunit_suite = {
> + .name = "rv_reactor_printk",
> + .test_cases = reactor_printk_kunit_cases,
> +};
> +
> +kunit_test_suite(reactor_printk_kunit_suite);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("KUnit tests for reactor_printk");
^ permalink raw reply
* Re: [PATCH 3/3] rv/reactors: add KUnit tests for reactor_panic
From: Gabriele Monaco @ 2026-07-06 14:48 UTC (permalink / raw)
To: wen.yang; +Cc: Nam Cao, linux-trace-kernel, linux-kernel
In-Reply-To: <810f587d63c84b64067f990299be02b88f5d8106.1781541556.git.wen.yang@linux.dev>
On Tue, 2026-06-16 at 00:44 +0800, wen.yang@linux.dev wrote:
> From: Wen Yang <wen.yang@linux.dev>
>
> Add KUnit tests for the panic reactor covering:
> - Reactor registration and unregistration lifecycle
> - Panic notifier chain reachability
>
> The real rv_panic_reaction() calls vpanic(), which is __noreturn and
> halts the system. KUnit cannot test across that boundary. Instead, the
> test drives atomic_notifier_call_chain(&panic_notifier_list, ...) directly
> with a high-priority mock notifier that returns NOTIFY_STOP, verifying
> that the panic notification propagates without triggering real handlers
> (kdump, watchdog, reboot).
>
> The mock notifier busy-waits to simulate real handler execution time
> (e.g., crash_save_vmcoreinfo, emergency_restart preamble) under the
> panic context constraints.
>
> Signed-off-by: Wen Yang <wen.yang@linux.dev>
> ---
> kernel/trace/rv/Kconfig | 10 +++
> kernel/trace/rv/Makefile | 1 +
> kernel/trace/rv/reactor_panic_kunit.c | 106 ++++++++++++++++++++++++++
> 3 files changed, 117 insertions(+)
> create mode 100644 kernel/trace/rv/reactor_panic_kunit.c
>
> diff --git a/kernel/trace/rv/Kconfig b/kernel/trace/rv/Kconfig
> index ff47895c897f..6c6c43c5f86c 100644
> --- a/kernel/trace/rv/Kconfig
> +++ b/kernel/trace/rv/Kconfig
> @@ -121,3 +121,13 @@ config RV_REACT_PANIC
> help
> Enables the panic reactor. The panic reactor emits a printk()
> message if an exception is found and panic()s the system.
> +
> +config RV_REACT_PANIC_KUNIT
> + bool "KUnit tests for reactor_panic" if !KUNIT_ALL_TESTS
> + depends on RV_REACT_PANIC && KUNIT
> + default KUNIT_ALL_TESTS
> + help
> + This builds KUnit tests for the panic reactor. These are only
> + for development and testing, not for regular kernel use cases.
> +
> + If unsure, say N.
> diff --git a/kernel/trace/rv/Makefile b/kernel/trace/rv/Makefile
> index ef0a2dcb927c..2ebfe5e5068c 100644
> --- a/kernel/trace/rv/Makefile
> +++ b/kernel/trace/rv/Makefile
> @@ -25,3 +25,4 @@ obj-$(CONFIG_RV_REACTORS) += rv_reactors.o
> obj-$(CONFIG_RV_REACT_PRINTK) += reactor_printk.o
> obj-$(CONFIG_RV_REACT_PRINTK_KUNIT) += reactor_printk_kunit.o
> obj-$(CONFIG_RV_REACT_PANIC) += reactor_panic.o
> +obj-$(CONFIG_RV_REACT_PANIC_KUNIT) += reactor_panic_kunit.o
> diff --git a/kernel/trace/rv/reactor_panic_kunit.c
> b/kernel/trace/rv/reactor_panic_kunit.c
> new file mode 100644
> index 000000000000..f9a09ae7aaad
> --- /dev/null
> +++ b/kernel/trace/rv/reactor_panic_kunit.c
> @@ -0,0 +1,106 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit tests for reactor_panic
> + *
> + */
> +
> +#include <kunit/test.h>
> +#include <linux/rv.h>
> +#include <linux/panic_notifier.h>
> +#include <linux/notifier.h>
> +#include <linux/limits.h>
> +#include <linux/sched/clock.h>
> +#include <linux/processor.h>
> +
> +/* Simulated execution time for mock panic notifier (nanoseconds). */
> +#define RV_PANIC_NOTIFIER_EXEC_NS 2000000ULL
> +
> +/* Test state */
> +static struct {
> + bool notifier_called;
> +} panic_test_state;
A struct isn't really needed I guess, but good to have a way to test there was a
call.
> +
> +/*
> + * Mock panic notifier callback.
> + *
> + * Runs at INT_MAX priority and returns NOTIFY_STOP to prevent real panic
> + * handlers (kdump, watchdog) from executing during the test. Busy-waits
> + * RV_PANIC_NOTIFIER_EXEC_NS to simulate a real handler's execution time.
> + */
> +static int mock_panic_notifier_fn(struct notifier_block *nb,
> + unsigned long action, void *data)
> +{
> + char *msg = data;
> + u64 start = sched_clock();
> +
> + panic_test_state.notifier_called = true;
> + pr_emerg("KUnit: reactor_panic test intercepted panic notifier:
> %s\n",
> + msg ? msg : "(no message)");
Since we cannot panic(), do we really need to test yet another mock reactor?
What if you put the notifier_called in the other test and drop this test
altogether?
We can simulate what panic() does right now, but it isn't too representative if
this ever changes.
Also it's good to make this reactor test buildable as a module. Currently some
functions aren't exported, feel free to just EXPORT_SYMBOL_GPL() them. There is
no need not to have modular reactors, just nobody did it before.
Thanks,
Gabriele
> +
> + while (sched_clock() - start < RV_PANIC_NOTIFIER_EXEC_NS)
> + cpu_relax();
> +
> + return NOTIFY_STOP;
> +}
> +
> +static struct notifier_block mock_panic_nb = {
> + .notifier_call = mock_panic_notifier_fn,
> + .priority = INT_MAX,
> +};
> +
> +static struct rv_reactor mock_panic_reactor = {
> + .name = "test_panic",
> + .description = "test panic reactor",
> +};
> +
> +static int reactor_panic_kunit_init(struct kunit *test)
> +{
> + panic_test_state.notifier_called = false;
> + return 0;
> +}
> +
> +/* Test 1: register and unregister reactor */
> +static void test_panic_register_unregister(struct kunit *test)
> +{
> + int ret;
> +
> + ret = rv_register_reactor(&mock_panic_reactor);
> + KUNIT_EXPECT_EQ(test, ret, 0);
> + KUNIT_EXPECT_STREQ(test, mock_panic_reactor.name, "test_panic");
> +
> + rv_unregister_reactor(&mock_panic_reactor);
> +}
> +
> +/*
> + * Test 2: panic notifier chain is reachable.
> + *
> + * vpanic() calls atomic_notifier_call_chain(&panic_notifier_list, ...).
> + * Drive the chain directly to verify panic notifiers receive the
> notification —
> + * the observable side-effect of reactor_panic without halting the system.
> + */
> +static void test_panic_notifier_called(struct kunit *test)
> +{
> + atomic_notifier_chain_register(&panic_notifier_list, &mock_panic_nb);
> + atomic_notifier_call_chain(&panic_notifier_list, 0,
> + "panic violation message");
> + atomic_notifier_chain_unregister(&panic_notifier_list,
> &mock_panic_nb);
> +
> + KUNIT_EXPECT_TRUE(test, panic_test_state.notifier_called);
> +}
> +
> +static struct kunit_case reactor_panic_kunit_cases[] = {
> + KUNIT_CASE(test_panic_register_unregister),
> + KUNIT_CASE(test_panic_notifier_called),
> + {}
> +};
> +
> +static struct kunit_suite reactor_panic_kunit_suite = {
> + .name = "rv_reactor_panic",
> + .init = reactor_panic_kunit_init,
> + .test_cases = reactor_panic_kunit_cases,
> +};
> +
> +kunit_test_suite(reactor_panic_kunit_suite);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("KUnit tests for reactor_panic");
^ permalink raw reply
* Re: [PATCH] x86/stacktrace: Mark arch_stack_walk() and unwinder functions notrace
From: Steven Rostedt @ 2026-07-06 14:57 UTC (permalink / raw)
To: Yuanhe Shu
Cc: Josh Poimboeuf, Peter Zijlstra, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H . Peter Anvin,
Masami Hiramatsu, linux-kernel, linux-trace-kernel, stable
In-Reply-To: <20260706095445.1683434-1-xiangzao@linux.alibaba.com>
On Mon, 6 Jul 2026 17:54:45 +0800
Yuanhe Shu <xiangzao@linux.alibaba.com> wrote:
> When the function tracer's func_stack_trace option and the function graph
> profiler (function_profile_enabled) are both active, a recursive ftrace
> reentrance can occur, leading to a hard lockup. This was observed during
> ftrace selftest (ftracetest-ktap) execution:
>
> watchdog: Watchdog detected hard LOCKUP on cpu 204
> RIP: profile_graph_entry+0xa0/0x160
> Call Trace:
> function_graph_enter+0xc9/0x120
> arch_ftrace_ops_list_func+0x112/0x230
> ftrace_call+0x5/0x44
> unwind_next_frame+0x5/0x870 <-- traced by ftrace
> arch_stack_walk+0x88/0xf0
> stack_trace_save+0x4b/0x70
> __ftrace_trace_stack+0x12e/0x170
> function_stack_trace_call+0x7c/0xa0
> arch_ftrace_ops_list_func+0x112/0x230
> ftrace_call+0x5/0x44
> irqtime_account_irq+0x5/0xb0
> __irq_exit_rcu+0x12/0xc0
> ...
>
> The root cause is a recursive ftrace reentrance:
> function_stack_trace_call() invokes __trace_stack() ->
> arch_stack_walk() -> unwind_next_frame() to capture a backtrace.
> Since the unwinder functions (__unwind_start(),
> unwind_next_frame(), unwind_get_return_address(),
> unwind_get_return_address_ptr()) are not marked notrace, the
> function graph tracer instruments them, re-entering the ftrace
> infrastructure from within an ftrace callback. This results in a
> hard lockup with interrupts disabled, detected by the watchdog NMI.
I'm fine with this change, but I'm wondering why the recursion protection
didn't catch this. There may be a missing check somewhere. I'll ack this
change, but I also want to add the check that would have prevented this
lockup.
Acked-by: Steven Rostedt <rostedt@goodmis.org>
-- Steve
^ permalink raw reply
* Re: [PATCH 1/2] soc: qcom: geni-se: trace: Add trace event support for GENI SE registers dump
From: Steven Rostedt @ 2026-07-06 14:59 UTC (permalink / raw)
To: Praveen Talari
Cc: Bjorn Andersson, Konrad Dybcio, Masami Hiramatsu,
Mathieu Desnoyers, Mark Brown, linux-kernel, linux-arm-msm,
linux-trace-kernel, linux-spi, Mukesh Kumar Savaliya,
aniket.randive, chandana.chiluveru
In-Reply-To: <20260706-add-tracepoints-for-se-reg-dump-v1-1-48bd08e28cf2@oss.qualcomm.com>
On Mon, 06 Jul 2026 16:38:12 +0530
Praveen Talari <praveen.talari@oss.qualcomm.com> wrote:
> +TRACE_EVENT(geni_se_regs,
> + TP_PROTO(struct geni_se *se),
> +
> + TP_ARGS(se),
> +
> + TP_STRUCT__entry(__string(geni_se_name, dev_name(se->dev))
> + __field(u32, geni_se_m_cmd0)
> + __field(u32, geni_se_m_irq_status)
> + __field(u32, geni_se_s_cmd0)
> + __field(u32, geni_se_s_irq_status)
> + __field(u32, geni_se_status)
> + __field(u32, geni_se_ios)
> + __field(u32, geni_se_m_cmd_ctrl)
> + __field(u32, geni_se_m_cmd_err)
> + __field(u32, geni_se_m_fw_err)
> + __field(u32, geni_se_tx_fifo_status)
> + __field(u32, geni_se_rx_fifo_status)
> + __field(u32, geni_se_tx_watermark)
> + __field(u32, geni_se_rx_watermark)
> + __field(u32, geni_se_rx_watermark_rfr)
> + __field(u32, geni_se_m_gp_length)
> + __field(u32, geni_se_s_gp_length)
> + __field(u32, geni_se_dma_tx_irq)
> + __field(u32, geni_se_dma_rx_irq)
> + __field(u32, geni_se_dma_tx_irq_en)
> + __field(u32, geni_se_dma_rx_irq_en)
> + __field(u32, geni_se_dma_rx_len)
> + __field(u32, geni_se_dma_rx_len_in)
> + __field(u32, geni_se_dma_tx_len)
> + __field(u32, geni_se_dma_tx_len_in)
> + __field(u32, geni_se_dma_tx_ptr_l)
> + __field(u32, geni_se_dma_tx_ptr_h)
> + __field(u32, geni_se_dma_rx_ptr_l)
> + __field(u32, geni_se_dma_rx_ptr_h)
> + __field(u32, geni_se_dma_tx_attr)
> + __field(u32, geni_se_dma_tx_max_burst)
> + __field(u32, geni_se_dma_rx_attr)
> + __field(u32, geni_se_dma_rx_max_burst)
> + __field(u32, geni_se_dma_if_en)
> + __field(u32, geni_se_dma_if_en_ro)
> + __field(u32, geni_se_dma_general_cfg)
> + __field(u32, geni_se_dma_qsb_trans_cfg)
> + __field(u32, geni_se_dma_dbg)
> + __field(u32, geni_se_m_irq_en)
> + __field(u32, geni_se_s_irq_en)
> + __field(u32, geni_se_gsi_event_en)
> + __field(u32, geni_se_irq_en)
> + __field(u32, geni_se_ser_m_clk_cfg)
> + __field(u32, geni_se_ser_s_clk_cfg)
> + __field(u32, geni_se_general_cfg)
> + __field(u32, geni_se_output_ctrl)
> + __field(u32, geni_se_clk_ctrl_ro)
> + __field(u32, geni_se_fifo_if_disable)
> + __field(u32, geni_se_fw_multilock_msa)
> + __field(u32, geni_se_clk_sel)
> + ),
Wow, a pretty big trace event! But it still fits in the ring buffer.
Acked-by: Steven Rostedt <rostedt@goodmis.org>
-- Steve
^ permalink raw reply
* Re: [PATCH 3/3] rv/reactors: add KUnit tests for reactor_panic
From: Gabriele Monaco @ 2026-07-06 15:00 UTC (permalink / raw)
To: wen.yang; +Cc: Nam Cao, linux-trace-kernel, linux-kernel
In-Reply-To: <810f587d63c84b64067f990299be02b88f5d8106.1781541556.git.wen.yang@linux.dev>
On Tue, 2026-06-16 at 00:44 +0800, wen.yang@linux.dev wrote:
> +/*
> + * Test 2: panic notifier chain is reachable.
> + *
> + * vpanic() calls atomic_notifier_call_chain(&panic_notifier_list, ...).
> + * Drive the chain directly to verify panic notifiers receive the
> notification —
> + * the observable side-effect of reactor_panic without halting the system.
> + */
> +static void test_panic_notifier_called(struct kunit *test)
> +{
> + atomic_notifier_chain_register(&panic_notifier_list, &mock_panic_nb);
> + atomic_notifier_call_chain(&panic_notifier_list, 0,
> + "panic violation message");
> + atomic_notifier_chain_unregister(&panic_notifier_list,
> &mock_panic_nb);
I just realised this isn't even testing the reactor, it's testing the
notifier_chain thing, I don't think we really need this here or am I missing
something?
Thanks,
Gabriele
> +
> + KUNIT_EXPECT_TRUE(test, panic_test_state.notifier_called);
> +}
> +
> +static struct kunit_case reactor_panic_kunit_cases[] = {
> + KUNIT_CASE(test_panic_register_unregister),
> + KUNIT_CASE(test_panic_notifier_called),
> + {}
> +};
> +
> +static struct kunit_suite reactor_panic_kunit_suite = {
> + .name = "rv_reactor_panic",
> + .init = reactor_panic_kunit_init,
> + .test_cases = reactor_panic_kunit_cases,
> +};
> +
> +kunit_test_suite(reactor_panic_kunit_suite);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("KUnit tests for reactor_panic");
^ permalink raw reply
* Re: [PATCH 2/2] spi: qcom-geni: add GENI SE registers trace event on error paths
From: Konrad Dybcio @ 2026-07-06 15:35 UTC (permalink / raw)
To: Praveen Talari, Bjorn Andersson, Konrad Dybcio, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Mark Brown
Cc: linux-kernel, linux-arm-msm, linux-trace-kernel, linux-spi,
Mukesh Kumar Savaliya, aniket.randive, chandana.chiluveru
In-Reply-To: <20260706-add-tracepoints-for-se-reg-dump-v1-2-48bd08e28cf2@oss.qualcomm.com>
On 7/6/26 1:08 PM, Praveen Talari wrote:
> The GENI SPI driver reports various transfer failures such as command
> timeouts, DMA reset timeouts, DMA transaction errors, and unexpected
> interrupt conditions. However, diagnosing the root cause of these
> failures is difficult as the hardware state is not captured when the
> error occurs.
>
> Add trace_geni_se_regs() calls at critical SPI error handling paths to
> automatically capture GENI serial engine debug registers when failures
> are detected. This includes:
>
> - M_CMD abort/cancel timeout
> - DMA TX/RX FSM reset timeout
> - DMA transaction failures and pending residue conditions
> - Unexpected interrupt error status
> - Premature transfer completion with pending TX/RX data
>
> Dumping the SE debug registers at the time of failure provides
> additional hardware context and significantly improves post-mortem
> analysis of SPI transfer issues without affecting normal operation.
>
> Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
> ---
> drivers/spi/spi-geni-qcom.c | 22 ++++++++++++++++++----
> 1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
> index 26e723cfea61..7db0836308c2 100644
> --- a/drivers/spi/spi-geni-qcom.c
> +++ b/drivers/spi/spi-geni-qcom.c
> @@ -3,6 +3,7 @@
>
> #define CREATE_TRACE_POINTS
> #include <trace/events/qcom_geni_spi.h>
> +#include <trace/events/qcom_geni_se.h>
nit: this could be sorted alphabetically (Mark, could you fix
it up while applying?)
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v10 6/6] selftests/mm: add hwpoison-panic destructive test
From: Breno Leitao @ 2026-07-06 16:14 UTC (permalink / raw)
To: Mike Rapoport
Cc: Miaohe Lin, Andrew Morton, David Hildenbrand, Lorenzo Stoakes,
Vlastimil Babka, Suren Baghdasaryan, Michal Hocko, Shuah Khan,
Naoya Horiguchi, Jonathan Corbet, Shuah Khan, Liam R. Howlett,
lance.yang, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
linux-mm, linux-kernel, linux-doc, linux-kselftest,
linux-trace-kernel, kernel-team
In-Reply-To: <akjS7kiGjVwWaWEz@kernel.org>
Hello Mike,
On Sat, Jul 04, 2026 at 12:31:26PM +0300, Mike Rapoport wrote:
> On Tue, Jun 30, 2026 at 05:46:09AM -0700, Breno Leitao wrote:
> I'm looking at these awk scripts and od encodings and I wonder if wasn't it
> simpler to write the test in C.
>
> We have a bunch of helpers in tools/testing/selftests/mm/vm_utils.h for
> accessing /proc files and there is already /proc/iomem parser in
> tools/testing/selftests/mm/pfnmap.c that also could be lifter to vm_util
I'm fine with either approach.
My main concern is keeping the selftest decoupled from the feature
itself, so that review of the test doesn't block the feature from
landing. Would that work for you?
Thanks
--breno
^ permalink raw reply
* Re: [PATCH v10 6/6] selftests/mm: add hwpoison-panic destructive test
From: Mike Rapoport @ 2026-07-06 16:23 UTC (permalink / raw)
To: Breno Leitao
Cc: Miaohe Lin, Andrew Morton, David Hildenbrand, Lorenzo Stoakes,
Vlastimil Babka, Suren Baghdasaryan, Michal Hocko, Shuah Khan,
Naoya Horiguchi, Jonathan Corbet, Shuah Khan, Liam R. Howlett,
lance.yang, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
linux-mm, linux-kernel, linux-doc, linux-kselftest,
linux-trace-kernel, kernel-team
In-Reply-To: <akvSzb0mRSS5aCg_@gmail.com>
On Mon, Jul 06, 2026 at 09:14:22AM -0700, Breno Leitao wrote:
> Hello Mike,
>
> On Sat, Jul 04, 2026 at 12:31:26PM +0300, Mike Rapoport wrote:
> > On Tue, Jun 30, 2026 at 05:46:09AM -0700, Breno Leitao wrote:
>
> > I'm looking at these awk scripts and od encodings and I wonder if wasn't it
> > simpler to write the test in C.
> >
> > We have a bunch of helpers in tools/testing/selftests/mm/vm_utils.h for
> > accessing /proc files and there is already /proc/iomem parser in
> > tools/testing/selftests/mm/pfnmap.c that also could be lifter to vm_util
>
> I'm fine with either approach.
>
> My main concern is keeping the selftest decoupled from the feature
> itself, so that review of the test doesn't block the feature from
> landing. Would that work for you?
Sure.
> Thanks
> --breno
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [PATCH 2/2] tracing/synthetic: Free type string on error path
From: Steven Rostedt @ 2026-07-06 17:15 UTC (permalink / raw)
To: Yu Peng
Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-trace-kernel,
linux-kernel
In-Reply-To: <20260603062533.1096320-2-pengyu@kylinos.cn>
On Wed, 3 Jun 2026 14:25:33 +0800
Yu Peng <pengyu@kylinos.cn> wrote:
> parse_synth_field() builds a "__data_loc ..." type string before
> assigning it to field->type. If the seq_buf check fails, the common
> cleanup cannot free the temporary string. Free it before leaving.
>
> Signed-off-by: Yu Peng <pengyu@kylinos.cn>
> ---
> kernel/trace/trace_events_synth.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace_events_synth.c b/kernel/trace/trace_events_synth.c
> index cdd5b93328358..dc15658a887cb 100644
> --- a/kernel/trace/trace_events_synth.c
> +++ b/kernel/trace/trace_events_synth.c
> @@ -839,8 +839,10 @@ static struct synth_field *parse_synth_field(int argc, char **argv,
> seq_buf_puts(&s, "__data_loc ");
> seq_buf_puts(&s, field->type);
>
> - if (WARN_ON_ONCE(!seq_buf_buffer_left(&s)))
> + if (WARN_ON_ONCE(!seq_buf_buffer_left(&s))) {
> + kfree(type);
> goto free;
> + }
> s.buffer[s.len] = '\0';
>
> kfree(field->type);
Can you do this instead?
diff --git a/kernel/trace/trace_events_synth.c b/kernel/trace/trace_events_synth.c
index cdd5b9332835..7ff0f00edbdd 100644
--- a/kernel/trace/trace_events_synth.c
+++ b/kernel/trace/trace_events_synth.c
@@ -828,7 +828,7 @@ static struct synth_field *parse_synth_field(int argc, char **argv,
} else if (size == 0) {
if (synth_field_is_string(field->type) ||
synth_field_is_stack(field->type)) {
- char *type;
+ char *type __free(kfree) = NULL;
len = sizeof("__data_loc ") + strlen(field->type) + 1;
type = kzalloc(len, GFP_KERNEL);
@@ -844,7 +844,7 @@ static struct synth_field *parse_synth_field(int argc, char **argv,
s.buffer[s.len] = '\0';
kfree(field->type);
- field->type = type;
+ field->type = no_free_ptr(type);
field->is_dynamic = true;
size = sizeof(u64);
-- Steve
^ permalink raw reply related
* [PATCH bpf-next 0/2] uprobes: Switch uretprobes_srcu to SRCU-fast-updown
From: Puranjay Mohan @ 2026-07-06 17:27 UTC (permalink / raw)
To: Lai Jiangshan, Paul E. McKenney, Josh Triplett, Masami Hiramatsu,
Oleg Nesterov, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Alexei Starovoitov,
Andrii Nakryiko
Cc: Puranjay Mohan, Steven Rostedt, Mathieu Desnoyers, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, rcu, linux-kernel, linux-trace-kernel,
linux-perf-users, bpf
uretprobes_srcu currently uses normal SRCU. Normal SRCU issues an
smp_mb() on both srcu_read_lock() and srcu_read_unlock(), i.e. two full
memory barriers per read-side critical section. For uretprobes this cost
is paid on every uretprobe invocation: prepare_uretprobe() takes the
read lock that is later dropped on the normal return path
(hprobe_finalize()) or from ri_timer()/dup_utask() (hprobe_expire()).
Switch uretprobes_srcu to the SRCU-fast-updown flavor. SRCU-fast moves
the read-side ordering off the reader and onto the (rare) grace-period
side: synchronize_srcu() rides on synchronize_rcu() instead of relying
on reader-side smp_mb(). This is a good trade for uretprobes, where
reader-side hits vastly outnumber grace periods (uprobe unregistration).
The updown variant (rather than plain SRCU-fast) is required because the
read lock is acquired in prepare_uretprobe() on the way out to user
space and is released only once the return instance is finalized -- from
a different context than it was taken: the normal return path
(uprobe_handle_trampoline() -> hprobe_finalize()), the timer callback,
or the fork path (ri_timer()/dup_utask() -> hprobe_expire()).
srcu_down_read_fast()/srcu_up_read_fast() are designed for this
semaphore-like, cross-context pattern and, unlike the same-context
srcu_read_lock_fast() variant, do not carry lockdep read-side tracking
that would warn on it -- which is why the old code had to use the raw
__srcu_read_lock() here. For the short, same-context sections in
ri_timer() and dup_utask(), guard(srcu_fast_updown) is used instead,
giving proper lockdep coverage.
Patch 1 adds the guard(srcu_fast_updown) definition, following the
existing guard(srcu)/guard(srcu_fast) pattern.
Patch 2 does the uretprobes_srcu conversion.
Note
----
Only uretprobes_srcu is converted; the main uprobe readers (RB-tree
lookup and consumer-list iteration) are deliberately left on RCU Tasks
Trace. RCU Tasks Trace is already implemented on top of
srcu_read_lock_fast(), so the reader-side cost is identical, and it has
a nesting fast path that the uprobe -> sleepable-BPF-program call chain
relies on (the BPF trampoline takes rcu_read_lock_trace() while uprobes
already holds it; the nested acquire is just a counter bump). Converting
those readers to a separate srcu_struct would turn one real + one nested
lock into two real locks and lose that optimization for no reader-side
gain. uretprobes_srcu is different: it uses normal SRCU (not Tasks
Trace), its readers are long-lived and cross-context, and it genuinely
benefits from dropping the per-reader barriers.
Puranjay Mohan (2):
srcu: Add lock guard for srcu_fast_updown flavor
uprobes: Switch uretprobes_srcu to SRCU-fast-updown
include/linux/srcu.h | 7 +++++++
include/linux/uprobes.h | 5 +++--
kernel/events/uprobes.c | 29 +++++++++++++++++------------
3 files changed, 27 insertions(+), 14 deletions(-)
base-commit: 87bfe634b1193db90e5170e1ddbad04a63ef4501
--
2.53.0-Meta
^ permalink raw reply
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