* [PATCH] pstore: fix ftrace dump, when ECC is enabled
From: Andrey Skvortsov @ 2026-02-15 18:51 UTC (permalink / raw)
To: Kees Cook, Tony Luck, Guilherme G. Piccoli, linux-hardening,
linux-kernel, Steven Rostedt, Masami Hiramatsu, Mark Rutland,
linux-trace-kernel
Cc: Andrey Skvortsov
total_size is sum of record->size and record->ecc_notice_size (ECC: No
errors detected). When ECC is not used, then there is no problem.
When ECC is enabled, then ftrace dump is decoded incorrectly after
restart.
First this affects starting offset calculation, that breaks
reading of all ftrace records.
CPU:66 ts:51646260179894273 3818ffff80008002 fe00ffff800080f0 0x3818ffff80008002 <- 0xfe00ffff800080f0
CPU:66 ts:56589664458375169 3818ffff80008002 ff02ffff800080f0 0x3818ffff80008002 <- 0xff02ffff800080f0
CPU:67 ts:13194139533313 afe4ffff80008002 1ffff800080f0 0xafe4ffff80008002 <- 0x1ffff800080f0
CPU:67 ts:13194139533313 b7d0ffff80008001 100ffff80008002 0xb7d0ffff80008001 <- 0x100ffff80008002
CPU:67 ts:51646260179894273 8de0ffff80008001 202ffff80008002 0x8de0ffff80008001 <- 0x202ffff80008002
Second ECC notice message is printed like ftrace record and as a
result couple of last records are completely wrong.
For example, when the starting offset is fixed:
CPU:0 ts:113 ffffffc00879bd04 ffffffc0080dc08c cpuidle_enter <- do_idle+0x20c/0x290
CPU:0 ts:114 ffffffc00879bd04 ffffffc0080dc08c cpuidle_enter <- do_idle+0x20c/0x290
CPU:100 ts:28259048229270629 6f4e203a4343450a 2073726f72726520 0x6f4e203a4343450a <- 0x2073726f72726520
Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
---
fs/pstore/inode.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
index b4e55c90f8dc2..9c2d5e5f31d65 100644
--- a/fs/pstore/inode.c
+++ b/fs/pstore/inode.c
@@ -74,9 +74,9 @@ static void *pstore_ftrace_seq_start(struct seq_file *s, loff_t *pos)
if (!data)
return NULL;
- data->off = ps->total_size % REC_SIZE;
+ data->off = ps->record->size % REC_SIZE;
data->off += *pos * REC_SIZE;
- if (data->off + REC_SIZE > ps->total_size)
+ if (data->off + REC_SIZE > ps->record->size)
return NULL;
return_ptr(data);
@@ -94,7 +94,7 @@ static void *pstore_ftrace_seq_next(struct seq_file *s, void *v, loff_t *pos)
(*pos)++;
data->off += REC_SIZE;
- if (data->off + REC_SIZE > ps->total_size)
+ if (data->off + REC_SIZE > ps->record->size)
return NULL;
return data;
--
2.51.0
^ permalink raw reply related
* [linus:master] [tracing] a46023d561: EIP:do_user_addr_fault
From: kernel test robot @ 2026-02-16 2:09 UTC (permalink / raw)
To: Steven Rostedt
Cc: oe-lkp, lkp, linux-kernel, Masami Hiramatsu, Mark Rutland,
Mathieu Desnoyers, Andrew Morton, Sebastian Andrzej Siewior,
Alexei Starovoitov, Paul E. McKenney, linux-trace-kernel,
oliver.sang
Hello,
kernel test robot noticed "EIP:do_user_addr_fault" on:
commit: a46023d5616ed3ed781e56ca93400eb9490e3646 ("tracing: Guard __DECLARE_TRACE() use of __DO_TRACE_CALL() with SRCU-fast")
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master
[test failed on linus/master ca4ee40bf13dbd3a4be3b40a00c33a1153d487e5]
[test failed on linux-next/master 635c467cc14ebdffab3f77610217c1dacaf88e8c]
in testcase: boot
config: i386-randconfig-001-20260215
compiler: clang-20
test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 32G
(please refer to attached dmesg/kmsg for entire log/backtrace)
+---------------------------------------------+------------+------------+
| | a77cb6a867 | a46023d561 |
+---------------------------------------------+------------+------------+
| EIP:do_user_addr_fault | 0 | 18 |
| EIP:do_int80_syscall_32 | 0 | 18 |
| BUG:kernel_NULL_pointer_dereference,address | 0 | 15 |
| Oops | 0 | 18 |
| Kernel_panic-not_syncing:Fatal_exception | 0 | 18 |
| BUG:unable_to_handle_page_fault_for_address | 0 | 3 |
+---------------------------------------------+------------+------------+
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202602160941.c62658c7-lkp@intel.com
[ 3.833434][ T59] ------------[ cut here ]------------
[ 3.834088][ T59] WARNING: arch/x86/mm/fault.c:1274 at do_user_addr_fault+0x387/0x480, CPU#1: modprobe/59
[ 3.835108][ T59] Modules linked in:
[ 3.835140][ T59] CPU: 1 UID: 0 PID: 59 Comm: modprobe Tainted: G T 6.19.0-rc7-00020-ga46023d5616e #1 PREEMPT(lazy)
[ 3.835140][ T59] Tainted: [T]=RANDSTRUCT
[ 3.835140][ T59] EIP: do_user_addr_fault (ld-temp.o:?)
[ 3.835140][ T59] Code: ff ff 89 f9 89 da ff 75 f0 e8 15 01 00 00 83 c4 04 e9 03 fe ff ff 89 f9 89 da ff 75 f0 e8 01 49 f6 ff 83 c4 04 e9 f7 fc ff ff <0f> 0b 89 f9 89 da ff 75 f0 e8 eb 03 00 00 83 c4 04 e9 d9 fd ff ff
All code
========
0: ff (bad)
1: ff 89 f9 89 da ff decl -0x257607(%rcx)
7: 75 f0 jne 0xfffffffffffffff9
9: e8 15 01 00 00 call 0x123
e: 83 c4 04 add $0x4,%esp
11: e9 03 fe ff ff jmp 0xfffffffffffffe19
16: 89 f9 mov %edi,%ecx
18: 89 da mov %ebx,%edx
1a: ff 75 f0 push -0x10(%rbp)
1d: e8 01 49 f6 ff call 0xfffffffffff64923
22: 83 c4 04 add $0x4,%esp
25: e9 f7 fc ff ff jmp 0xfffffffffffffd21
2a:* 0f 0b ud2 <-- trapping instruction
2c: 89 f9 mov %edi,%ecx
2e: 89 da mov %ebx,%edx
30: ff 75 f0 push -0x10(%rbp)
33: e8 eb 03 00 00 call 0x423
38: 83 c4 04 add $0x4,%esp
3b: e9 d9 fd ff ff jmp 0xfffffffffffffe19
Code starting with the faulting instruction
===========================================
0: 0f 0b ud2
2: 89 f9 mov %edi,%ecx
4: 89 da mov %ebx,%edx
6: ff 75 f0 push -0x10(%rbp)
9: e8 eb 03 00 00 call 0x3f9
e: 83 c4 04 add $0x4,%esp
11: e9 d9 fd ff ff jmp 0xfffffffffffffdef
[ 3.835140][ T59] EAX: 80000000 EBX: 00000000 ECX: 4324aef7 EDX: 431f7ada
[ 3.835140][ T59] ESI: 52cb2000 EDI: 52cb5f34 EBP: 52cb5f14 ESP: 52cb5ef0
[ 3.835140][ T59] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068 EFLAGS: 00210046
[ 3.835140][ T59] CR0: 80050033 CR2: 00000004 CR3: 1275e000 CR4: 000406b0
[ 3.835140][ T59] Call Trace:
[ 3.835140][ T59] ? debug_smp_processor_id (ld-temp.o:?)
[ 3.835140][ T59] ? trace_page_fault_kernel (ld-temp.o:?)
[ 3.835140][ T59] ? exc_page_fault (ld-temp.o:?)
[ 3.835140][ T59] ? pvclock_clocksource_read_nowd (ld-temp.o:?)
[ 3.835140][ T59] ? entry_INT80_32 (arch/x86/entry/entry_32.S:945)
[ 3.835140][ T59] ? handle_exception (arch/x86/entry/entry_32.S:1048)
[ 3.835140][ T59] ? entry_INT80_32 (arch/x86/entry/entry_32.S:945)
[ 3.835140][ T59] ? xas_find_conflict (ld-temp.o:?)
[ 3.835140][ T59] ? pvclock_clocksource_read_nowd (ld-temp.o:?)
[ 3.835140][ T59] ? do_int80_syscall_32 (ld-temp.o:?)
[ 3.835140][ T59] ? pvclock_clocksource_read_nowd (ld-temp.o:?)
[ 3.835140][ T59] ? do_int80_syscall_32 (ld-temp.o:?)
[ 3.835140][ T59] ? entry_INT80_32 (arch/x86/entry/entry_32.S:945)
[ 3.835140][ T59] irq event stamp: 5220
[ 3.835140][ T59] hardirqs last enabled at (5219): free_to_partial_list (ld-temp.o:?)
[ 3.835140][ T59] hardirqs last disabled at (5220): do_int80_syscall_32 (ld-temp.o:?)
[ 3.835140][ T59] softirqs last enabled at (2722): __do_softirq (ld-temp.o:?)
[ 3.835140][ T59] softirqs last disabled at (2713): __do_softirq (ld-temp.o:?)
[ 3.835140][ T59] ---[ end trace 0000000000000000 ]---
The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20260216/202602160941.c62658c7-lkp@intel.com
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: Enhancing Conditional Filtering for Function Graph Tracer
From: Masami Hiramatsu @ 2026-02-16 7:27 UTC (permalink / raw)
To: Donglin Peng
Cc: Steven Rostedt, linux-trace-kernel, Linux Kernel Mailing List
In-Reply-To: <CAErzpmtMrHWQVAQx=jiq_sWo=+c_JOUoQmPafo-fq+Q3qi+BBw@mail.gmail.com>
On Sat, 14 Feb 2026 18:36:24 +0800
Donglin Peng <dolinux.peng@gmail.com> wrote:
> Hi Steve and Masami,
>
> I recently had an idea while using the function graph tracer.
> Currently, it supports basic
> filtering via set_ftrace_pid(PID-based) and
> set_graph_function(function name-based).
> However, this filtering mechanism feels somewhat limited. Given that
> we already have
> features like funcgraph-args, I wonder if we could enhance filtering by allowing
> conditional tracking based on function parameters — similar to how
> trace events support
> filters/triggers.
Agreed. I think it is a good idea.
>
> To simplify implementation, I propose extending a new trigger type
> (e.g., "funcgraph").
OK, and I think the question is what do we want to filter. As you know,
the kernel function call-graph is a very context depending. I think we
may be better to start with specifying the pid filter via the tringger
instead of enabling/disabling it on all cpus.
echo "funcgraph:setpid:common_pid" >> events/fprobes/func_foo/trigger
Then, we can also specify a specific task's pid too via sched_switch
events etc. Also we can add "clearpid". Maybe we also need a special
PID (e.g. -1) for trace no process instead of using tracing_on.
Thank you,
> In ftrace_graph_ignore_func, we could look up the corresponding trace_fprobe and
> trace_event_file based on trace->func, then decide whether to trace
> the function using
> a helper like the following:
>
> static bool ftrace_graph_filter(struct trace_fprobe *tf, struct
> ftrace_regs *fregs,
> struct trace_event_file *trace_file)
> {
> struct fentry_trace_entry_head *entry;
> struct trace_event_buffer fbuffer;
> struct event_trigger_data *data;
> int dsize;
>
> dsize = __get_data_size(&tf->tp, fregs, NULL);
> entry = trace_event_buffer_reserve(&fbuffer, trace_file,
> sizeof(*entry) + tf->tp.size + dsize);
> if (!entry)
> return false;
>
> entry = ring_buffer_event_data(fbuffer.event);
> store_trace_args(&entry[1], &tf->tp, fregs, NULL, sizeof(*entry), dsize);
>
> list_for_each_entry_rcu(data, &trace_file->triggers, list) {
> if (data->cmd_ops->trigger_type == TRIGGER_TYPE_FUNCGRAPH) {
> struct event_filter *filter = rcu_dereference_sched(data->filter);
> if (filter && filter_match_preds(filter, entry))
> return true; // Allow tracing
> }
> }
> return false; // Skip tracing
> }
>
> Does this approach make sense? Any suggestions or concerns?
>
> Thanks,
> Donglin
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* [PATCH] rv: Fix multiple definition of __pcpu_unique_da_mon_this
From: Mikhail Gavrilov @ 2026-02-16 7:47 UTC (permalink / raw)
To: Gabriele Monaco, Daniel Bristot de Oliveira, Steven Rostedt
Cc: Nam Cao, linux-trace-kernel, linux-kernel, Mikhail Gavrilov
The refactoring in commit 30984ccf31b7 ("rv: Refactor da_monitor to
minimise macros") replaced per-monitor unique variable names
(da_mon_##name) with a fixed name (da_mon_this).
While this works for 'static' variables (each translation unit gets its
own copy), DEFINE_PER_CPU internally generates a non-static dummy
variable __pcpu_unique_<n> for each per-cpu definition. When multiple
per-cpu monitors (e.g. sco and sts) are built-in simultaneously, they
all produce the same __pcpu_unique_da_mon_this symbol, causing a link
error:
ld: kernel/trace/rv/monitors/sts/sts.o: multiple definition of
`__pcpu_unique_da_mon_this';
kernel/trace/rv/monitors/sco/sco.o: first defined here
Fix this by introducing a DA_MON_NAME macro that expands to a
per-monitor unique name (da_mon_<MONITOR_NAME>) via the existing
CONCATENATE helper. This restores the uniqueness that was present
before the refactoring.
Fixes: 30984ccf31b7 ("rv: Refactor da_monitor to minimise macros")
Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
---
include/rv/da_monitor.h | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
index db11d41bb438..f6da07863ed8 100644
--- a/include/rv/da_monitor.h
+++ b/include/rv/da_monitor.h
@@ -19,6 +19,7 @@
#include <linux/stringify.h>
#include <linux/bug.h>
#include <linux/sched.h>
+#define DA_MON_NAME CONCATENATE(da_mon_, MONITOR_NAME)
static struct rv_monitor rv_this;
@@ -183,14 +184,14 @@ static inline bool da_event(struct da_monitor *da_mon, struct task_struct *tsk,
/*
* global monitor (a single variable)
*/
-static struct da_monitor da_mon_this;
+static struct da_monitor DA_MON_NAME;
/*
* da_get_monitor - return the global monitor address
*/
static struct da_monitor *da_get_monitor(void)
{
- return &da_mon_this;
+ return &DA_MON_NAME;
}
/*
@@ -223,14 +224,14 @@ static inline void da_monitor_destroy(void) { }
/*
* per-cpu monitor variables
*/
-static DEFINE_PER_CPU(struct da_monitor, da_mon_this);
+static DEFINE_PER_CPU(struct da_monitor, DA_MON_NAME);
/*
* da_get_monitor - return current CPU monitor address
*/
static struct da_monitor *da_get_monitor(void)
{
- return this_cpu_ptr(&da_mon_this);
+ return this_cpu_ptr(&DA_MON_NAME);
}
/*
@@ -242,7 +243,7 @@ static void da_monitor_reset_all(void)
int cpu;
for_each_cpu(cpu, cpu_online_mask) {
- da_mon = per_cpu_ptr(&da_mon_this, cpu);
+ da_mon = per_cpu_ptr(&DA_MON_NAME, cpu);
da_monitor_reset(da_mon);
}
}
--
2.53.0
^ permalink raw reply related
* Re: [PATCH] ring-buffer: Fix possible dereference of uninitialized pointer
From: Masami Hiramatsu @ 2026-02-16 8:00 UTC (permalink / raw)
To: Daniil Dulov
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
open list:TRACING, open list:TRACING, lvc-project,
kernel test robot, Dan Carpenter
In-Reply-To: <20260213100130.2013839-1-d.dulov@aladdin.ru>
On Fri, 13 Feb 2026 13:01:30 +0300
Daniil Dulov <d.dulov@aladdin.ru> wrote:
> There is a pointer head_page in rb_meta_validate_events() which is not
> initialized at the beginning of a function. This pointer can be dereferenced
> if there is a failure during reader page validation. In this case the control
> is passed to "invalid" label where the pointer is dereferenced in a loop.
>
> To fix the issue initialize orig_head and head_page before calling
> rb_validate_buffer.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
Looks good to me.
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Thanks!
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/r/202406130130.JtTGRf7W-lkp@intel.com/
> Fixes: 5f3b6e839f3c ("ring-buffer: Validate boot range memory events")
> Signed-off-by: Daniil Dulov <d.dulov@aladdin.ru>
> ---
> kernel/trace/ring_buffer.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 630221b00838..ad08430347b0 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -1918,6 +1918,8 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
> if (!meta || !meta->head_buffer)
> return;
>
> + orig_head = head_page = cpu_buffer->head_page;
> +
> /* Do the reader page first */
> ret = rb_validate_buffer(cpu_buffer->reader_page->page, cpu_buffer->cpu);
> if (ret < 0) {
> @@ -1928,7 +1930,6 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
> entry_bytes += local_read(&cpu_buffer->reader_page->page->commit);
> local_set(&cpu_buffer->reader_page->entries, ret);
>
> - orig_head = head_page = cpu_buffer->head_page;
> ts = head_page->page->time_stamp;
>
> /*
> --
> 2.34.1
>
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH] rv: Fix multiple definition of __pcpu_unique_da_mon_this
From: Gabriele Monaco @ 2026-02-16 8:28 UTC (permalink / raw)
To: Mikhail Gavrilov
Cc: Steven Rostedt, Nam Cao, linux-trace-kernel, linux-kernel
In-Reply-To: <20260216074727.741822-1-mikhail.v.gavrilov@gmail.com>
On Mon, 2026-02-16 at 12:47 +0500, Mikhail Gavrilov wrote:
> The refactoring in commit 30984ccf31b7 ("rv: Refactor da_monitor to
> minimise macros") replaced per-monitor unique variable names
> (da_mon_##name) with a fixed name (da_mon_this).
>
> While this works for 'static' variables (each translation unit gets its
> own copy), DEFINE_PER_CPU internally generates a non-static dummy
> variable __pcpu_unique_<n> for each per-cpu definition. When multiple
> per-cpu monitors (e.g. sco and sts) are built-in simultaneously, they
> all produce the same __pcpu_unique_da_mon_this symbol, causing a link
> error:
>
> ld: kernel/trace/rv/monitors/sts/sts.o: multiple definition of
> `__pcpu_unique_da_mon_this';
> kernel/trace/rv/monitors/sco/sco.o: first defined here
>
> Fix this by introducing a DA_MON_NAME macro that expands to a
> per-monitor unique name (da_mon_<MONITOR_NAME>) via the existing
> CONCATENATE helper. This restores the uniqueness that was present
> before the refactoring.
Thanks Mikhail,
I just got a report but you were faster sending the patch!
The __pcpu_unique_ variable (and with it the requirement for per-cpu variables
to be unique although static) exists:
* for modules on specific achitectures (alpha)
* if the kernel is built with CONFIG_DEBUG_FORCE_WEAK_PER_CPU (e.g. Fedora's
debug kernel)
Could you mention this in the commit message?
Also, it's alright using a unique name also for global monitors although not
required, but please add a comment stating why this is needed at least for CPU
variables, to avoid the next "refactoring" to change it again. Something like:
Per-cpu variables requires a unique name although static in some configurations
Other than that, the change looks good.
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
Thanks,
Gabriele
>
> Fixes: 30984ccf31b7 ("rv: Refactor da_monitor to minimise macros")
> Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
> ---
> include/rv/da_monitor.h | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
> index db11d41bb438..f6da07863ed8 100644
> --- a/include/rv/da_monitor.h
> +++ b/include/rv/da_monitor.h
> @@ -19,6 +19,7 @@
> #include <linux/stringify.h>
> #include <linux/bug.h>
> #include <linux/sched.h>
> +#define DA_MON_NAME CONCATENATE(da_mon_, MONITOR_NAME)
>
> static struct rv_monitor rv_this;
>
> @@ -183,14 +184,14 @@ static inline bool da_event(struct da_monitor *da_mon,
> struct task_struct *tsk,
> /*
> * global monitor (a single variable)
> */
> -static struct da_monitor da_mon_this;
> +static struct da_monitor DA_MON_NAME;
>
> /*
> * da_get_monitor - return the global monitor address
> */
> static struct da_monitor *da_get_monitor(void)
> {
> - return &da_mon_this;
> + return &DA_MON_NAME;
> }
>
> /*
> @@ -223,14 +224,14 @@ static inline void da_monitor_destroy(void) { }
> /*
> * per-cpu monitor variables
> */
> -static DEFINE_PER_CPU(struct da_monitor, da_mon_this);
> +static DEFINE_PER_CPU(struct da_monitor, DA_MON_NAME);
>
> /*
> * da_get_monitor - return current CPU monitor address
> */
> static struct da_monitor *da_get_monitor(void)
> {
> - return this_cpu_ptr(&da_mon_this);
> + return this_cpu_ptr(&DA_MON_NAME);
> }
>
> /*
> @@ -242,7 +243,7 @@ static void da_monitor_reset_all(void)
> int cpu;
>
> for_each_cpu(cpu, cpu_online_mask) {
> - da_mon = per_cpu_ptr(&da_mon_this, cpu);
> + da_mon = per_cpu_ptr(&DA_MON_NAME, cpu);
> da_monitor_reset(da_mon);
> }
> }
^ permalink raw reply
* [PATCH v2] rv: Fix multiple definition of __pcpu_unique_da_mon_this
From: Mikhail Gavrilov @ 2026-02-16 9:01 UTC (permalink / raw)
To: Gabriele Monaco, Daniel Bristot de Oliveira, Steven Rostedt
Cc: Nam Cao, linux-trace-kernel, linux-kernel, Mikhail Gavrilov
In-Reply-To: <628790bb5ee45c8968550a54388862f020d014ff.camel@redhat.com>
The refactoring in commit 30984ccf31b7 ("rv: Refactor da_monitor to
minimise macros") replaced per-monitor unique variable names
(da_mon_##name) with a fixed name (da_mon_this).
While this works for 'static' variables (each translation unit gets its
own copy), DEFINE_PER_CPU internally generates a non-static dummy
variable __pcpu_unique_<n> for each per-cpu definition. The requirement
for this variable to be unique although static exists for modules on
specific architectures (alpha) and if the kernel is built with
CONFIG_DEBUG_FORCE_WEAK_PER_CPU (e.g. Fedora's debug kernel).
When multiple per-cpu monitors (e.g. sco and sts) are built-in
simultaneously, they all produce the same __pcpu_unique_da_mon_this
symbol, causing a link error:
ld: kernel/trace/rv/monitors/sts/sts.o: multiple definition of
`__pcpu_unique_da_mon_this';
kernel/trace/rv/monitors/sco/sco.o: first defined here
Fix this by introducing a DA_MON_NAME macro that expands to a
per-monitor unique name (da_mon_<MONITOR_NAME>) via the existing
CONCATENATE helper. This restores the uniqueness that was present
before the refactoring.
Fixes: 30984ccf31b7 ("rv: Refactor da_monitor to minimise macros")
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
---
include/rv/da_monitor.h | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
index db11d41bb438..7511f5464c48 100644
--- a/include/rv/da_monitor.h
+++ b/include/rv/da_monitor.h
@@ -20,6 +20,12 @@
#include <linux/bug.h>
#include <linux/sched.h>
+/*
+ * Per-cpu variables require a unique name although static in some
+ * configurations (e.g. CONFIG_DEBUG_FORCE_WEAK_PER_CPU or alpha modules).
+ */
+#define DA_MON_NAME CONCATENATE(da_mon_, MONITOR_NAME)
+
static struct rv_monitor rv_this;
static void react(enum states curr_state, enum events event)
@@ -183,14 +189,14 @@ static inline bool da_event(struct da_monitor *da_mon, struct task_struct *tsk,
/*
* global monitor (a single variable)
*/
-static struct da_monitor da_mon_this;
+static struct da_monitor DA_MON_NAME;
/*
* da_get_monitor - return the global monitor address
*/
static struct da_monitor *da_get_monitor(void)
{
- return &da_mon_this;
+ return &DA_MON_NAME;
}
/*
@@ -223,14 +229,14 @@ static inline void da_monitor_destroy(void) { }
/*
* per-cpu monitor variables
*/
-static DEFINE_PER_CPU(struct da_monitor, da_mon_this);
+static DEFINE_PER_CPU(struct da_monitor, DA_MON_NAME);
/*
* da_get_monitor - return current CPU monitor address
*/
static struct da_monitor *da_get_monitor(void)
{
- return this_cpu_ptr(&da_mon_this);
+ return this_cpu_ptr(&DA_MON_NAME);
}
/*
@@ -242,7 +248,7 @@ static void da_monitor_reset_all(void)
int cpu;
for_each_cpu(cpu, cpu_online_mask) {
- da_mon = per_cpu_ptr(&da_mon_this, cpu);
+ da_mon = per_cpu_ptr(&DA_MON_NAME, cpu);
da_monitor_reset(da_mon);
}
}
--
2.53.0
^ permalink raw reply related
* [PATCH] tracing: ring-buffer: Fix to check event length before using
From: Masami Hiramatsu (Google) @ 2026-02-16 9:30 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu
Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Check the event length before adding it for accessing next index in
rb_read_data_buffer(). Since this function is used for validating
possibly broken ring buffers, the length of the event could be broken.
In that case, the new event (e + len) can point a wrong address.
To avoid invalid memory access at boot, check whether the length of
each event is in the possible range before using it.
Fixes: 5f3b6e839f3c ("ring-buffer: Validate boot range memory events")
Cc: stable@vger.kernel.org
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
kernel/trace/ring_buffer.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 630221b00838..1ef17d6fd824 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -1848,6 +1848,7 @@ static int rb_read_data_buffer(struct buffer_data_page *dpage, int tail, int cpu
struct ring_buffer_event *event;
u64 ts, delta;
int events = 0;
+ int len;
int e;
*delta_ptr = 0;
@@ -1855,9 +1856,12 @@ static int rb_read_data_buffer(struct buffer_data_page *dpage, int tail, int cpu
ts = dpage->time_stamp;
- for (e = 0; e < tail; e += rb_event_length(event)) {
+ for (e = 0; e < tail; e += len) {
event = (struct ring_buffer_event *)(dpage->data + e);
+ len = rb_event_length(event);
+ if (len <= 0 || len > tail - e)
+ return -1;
switch (event->type_len) {
^ permalink raw reply related
* [PATCH v2 0/4] Clean up access to trace_event_file from a file struct
From: Petr Pavlu @ 2026-02-16 13:41 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu
Cc: Mathieu Desnoyers, Tom Zanussi, linux-kernel, linux-trace-kernel,
Petr Pavlu
This series includes several patches related to accessing
trace_event_file from a file struct. The first patch is a fix for an
edge case, the remaining patches are minor cleanups.
Changes since v1 [1]:
* Fix a compilation error when CONFIG_HIST_TRIGGERS is not set.
* Drop a patch that references the trace_event_file data in
event_file_data() and keep the simpler implementation of storing the
id in i_private.
* Inline event_file_data() into event_id_read() to enable adding
additional checks to the former.
Petr Pavlu (4):
tracing: Fix checking of freed trace_event_file for hist files
tracing: Remove unnecessary check for EVENT_FILE_FL_FREED
tracing: Clean up access to trace_event_file from a file pointer
tracing: Free up file->private_data for use by individual events
include/linux/trace_events.h | 5 +++++
kernel/trace/trace.c | 2 --
kernel/trace/trace.h | 17 +++++++++++------
kernel/trace/trace_events.c | 17 ++++++++---------
kernel/trace/trace_events_hist.c | 8 ++------
5 files changed, 26 insertions(+), 23 deletions(-)
base-commit: cee73b1e840c154f64ace682cb477c1ae2e29cc4
--
2.52.0
^ permalink raw reply
* [PATCH v2 1/4] tracing: Fix checking of freed trace_event_file for hist files
From: Petr Pavlu @ 2026-02-16 13:41 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu
Cc: Mathieu Desnoyers, Tom Zanussi, linux-kernel, linux-trace-kernel,
Petr Pavlu
In-Reply-To: <20260216134247.1311631-1-petr.pavlu@suse.com>
The event_hist_open() and event_hist_poll() functions currently retrieve
a trace_event_file pointer from a file struct by invoking
event_file_data(), which simply returns file->f_inode->i_private. The
functions then check if the pointer is NULL to determine whether the event
is still valid. This approach is flawed because i_private is assigned when
an eventfs inode is allocated and remains set throughout its lifetime.
Instead, the code should call event_file_file(), which checks for
EVENT_FILE_FL_FREED. Using the incorrect access function may result in the
code potentially opening a hist file for an event that is being removed or
becoming stuck while polling on this file.
A related issue is that although event_hist_poll() attempts to verify
whether an event file is being removed, this check may not occur or could
be unnecessarily delayed. This happens because hist_poll_wakeup() is
currently invoked only from event_hist_trigger() when a hist command is
triggered. If the event file is being removed, no associated hist command
will be triggered and a waiter will be woken up only after an unrelated
hist command is triggered.
Fix these issues by changing the access method to event_file_file() and
adding a call to hist_poll_wakeup() in remove_event_file_dir() after
setting the EVENT_FILE_FL_FREED flag. This ensures that a task polling on
a hist file is woken up and receives EPOLLERR.
Fixes: 1bd13edbbed6 ("tracing/hist: Add poll(POLLIN) support on hist file")
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
include/linux/trace_events.h | 5 +++++
kernel/trace/trace_events.c | 3 +++
kernel/trace/trace_events_hist.c | 4 ++--
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 3690221ba3d8..f925034e402d 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -683,6 +683,11 @@ static inline void hist_poll_wakeup(void)
#define hist_poll_wait(file, wait) \
poll_wait(file, &hist_poll_wq, wait)
+
+#else
+static inline void hist_poll_wakeup(void)
+{
+}
#endif
#define __TRACE_EVENT_FLAGS(name, value) \
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 137b4d9bb116..e8ed6ba155cf 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1295,6 +1295,9 @@ static void remove_event_file_dir(struct trace_event_file *file)
free_event_filter(file->filter);
file->flags |= EVENT_FILE_FL_FREED;
event_file_put(file);
+
+ /* Wake up hist poll waiters to notice the EVENT_FILE_FL_FREED flag. */
+ hist_poll_wakeup();
}
/*
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index c97bb2fda5c0..744c2aa3d668 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -5778,7 +5778,7 @@ static __poll_t event_hist_poll(struct file *file, struct poll_table_struct *wai
guard(mutex)(&event_mutex);
- event_file = event_file_data(file);
+ event_file = event_file_file(file);
if (!event_file)
return EPOLLERR;
@@ -5816,7 +5816,7 @@ static int event_hist_open(struct inode *inode, struct file *file)
guard(mutex)(&event_mutex);
- event_file = event_file_data(file);
+ event_file = event_file_file(file);
if (!event_file) {
ret = -ENODEV;
goto err;
--
2.52.0
^ permalink raw reply related
* [PATCH v2 2/4] tracing: Remove unnecessary check for EVENT_FILE_FL_FREED
From: Petr Pavlu @ 2026-02-16 13:41 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu
Cc: Mathieu Desnoyers, Tom Zanussi, linux-kernel, linux-trace-kernel,
Petr Pavlu
In-Reply-To: <20260216134247.1311631-1-petr.pavlu@suse.com>
The event_filter_write() function calls event_file_file() to retrieve
a trace_event_file associated with a given file struct. If a non-NULL
pointer is returned, the function then checks whether the trace_event_file
instance has the EVENT_FILE_FL_FREED flag set. This check is redundant
because event_file_file() already performs this validation and returns NULL
if the flag is set. The err value is also already initialized to -ENODEV.
Remove the unnecessary check for EVENT_FILE_FL_FREED in
event_filter_write().
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
---
kernel/trace/trace_events.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index e8ed6ba155cf..2ca76b048638 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -2153,12 +2153,8 @@ event_filter_write(struct file *filp, const char __user *ubuf, size_t cnt,
mutex_lock(&event_mutex);
file = event_file_file(filp);
- if (file) {
- if (file->flags & EVENT_FILE_FL_FREED)
- err = -ENODEV;
- else
- err = apply_event_filter(file, buf);
- }
+ if (file)
+ err = apply_event_filter(file, buf);
mutex_unlock(&event_mutex);
kfree(buf);
--
2.52.0
^ permalink raw reply related
* [PATCH v2 3/4] tracing: Clean up access to trace_event_file from a file pointer
From: Petr Pavlu @ 2026-02-16 13:41 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu
Cc: Mathieu Desnoyers, Tom Zanussi, linux-kernel, linux-trace-kernel,
Petr Pavlu
In-Reply-To: <20260216134247.1311631-1-petr.pavlu@suse.com>
The tracing code provides two functions event_file_file() and
event_file_data() to obtain a trace_event_file pointer from a file struct.
The primary method to use is event_file_file(), as it checks for the
EVENT_FILE_FL_FREED flag to determine whether the event is being removed.
The second function event_file_data() is an optimization for retrieving the
same data when the event_mutex is still held.
In the past, when removing an event directory in remove_event_file_dir(),
the code set i_private to NULL for all event files and readers were
expected to check for this state to recognize that the event is being
removed. In the case of event_id_read(), the value was read using
event_file_data() without acquiring the event_mutex. This required
event_file_data() to use READ_ONCE() when retrieving the i_private data.
With the introduction of eventfs, i_private is assigned when an eventfs
inode is allocated and remains set throughout its lifetime.
Remove the now unnecessary READ_ONCE() access to i_private in both
event_file_file() and event_file_data(). Inline the access to i_private in
remove_event_file_dir(), which allows event_file_data() to handle i_private
solely as a trace_event_file pointer. Add a check in event_file_data() to
ensure that the event_mutex is held and that file->flags doesn't have the
EVENT_FILE_FL_FREED flag set. Finally, move event_file_data() immediately
after event_file_code() since the latter provides a comment explaining how
both functions should be used together.
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
---
kernel/trace/trace.h | 17 +++++++++++------
kernel/trace/trace_events.c | 6 +++---
2 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 8428c437cb9d..27fb3fe6a7e0 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1739,11 +1739,6 @@ extern struct trace_event_file *find_event_file(struct trace_array *tr,
const char *system,
const char *event);
-static inline void *event_file_data(struct file *filp)
-{
- return READ_ONCE(file_inode(filp)->i_private);
-}
-
extern struct mutex event_mutex;
extern struct list_head ftrace_events;
@@ -1764,12 +1759,22 @@ static inline struct trace_event_file *event_file_file(struct file *filp)
struct trace_event_file *file;
lockdep_assert_held(&event_mutex);
- file = READ_ONCE(file_inode(filp)->i_private);
+ file = file_inode(filp)->i_private;
if (!file || file->flags & EVENT_FILE_FL_FREED)
return NULL;
return file;
}
+static inline void *event_file_data(struct file *filp)
+{
+ struct trace_event_file *file;
+
+ lockdep_assert_held(&event_mutex);
+ file = file_inode(filp)->i_private;
+ WARN_ON(!file || file->flags & EVENT_FILE_FL_FREED);
+ return file;
+}
+
extern const struct file_operations event_trigger_fops;
extern const struct file_operations event_hist_fops;
extern const struct file_operations event_hist_debug_fops;
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 2ca76b048638..d23e7a9d07e6 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -2090,12 +2090,12 @@ static int trace_format_open(struct inode *inode, struct file *file)
static ssize_t
event_id_read(struct file *filp, char __user *ubuf, size_t cnt, loff_t *ppos)
{
- int id = (long)event_file_data(filp);
+ /* id is directly in i_private and available for inode's lifetime. */
+ int id = (long)file_inode(filp)->i_private;
char buf[32];
int len;
- if (unlikely(!id))
- return -ENODEV;
+ WARN_ON(!id);
len = sprintf(buf, "%d\n", id);
--
2.52.0
^ permalink raw reply related
* [PATCH v2 4/4] tracing: Free up file->private_data for use by individual events
From: Petr Pavlu @ 2026-02-16 13:42 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu
Cc: Mathieu Desnoyers, Tom Zanussi, linux-kernel, linux-trace-kernel,
Petr Pavlu
In-Reply-To: <20260216134247.1311631-1-petr.pavlu@suse.com>
The tracing_open_file_tr() function currently copies the trace_event_file
pointer from inode->i_private to file->private_data when the file is
successfully opened. This duplication is not particularly useful, as all
event code should utilize event_file_file() or event_file_data() to
retrieve a trace_event_file pointer from a file struct and these access
functions read file->f_inode->i_private. Moreover, this setup requires the
code for opening hist files to explicitly clear file->private_data before
calling single_open(), since this function expects the private_data member
to be set to NULL and uses it to store a pointer to a seq_file.
Remove the unnecessary setting of file->private_data in
tracing_open_file_tr() and simplify the hist code.
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
---
kernel/trace/trace.c | 2 --
kernel/trace/trace_events_hist.c | 4 ----
2 files changed, 6 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index b1cb30a7b83d..01af8d88a468 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -4804,8 +4804,6 @@ int tracing_open_file_tr(struct inode *inode, struct file *filp)
event_file_get(file);
}
- filp->private_data = inode->i_private;
-
return 0;
}
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 744c2aa3d668..1ca3e14d7531 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -5831,8 +5831,6 @@ static int event_hist_open(struct inode *inode, struct file *file)
hist_file->file = file;
hist_file->last_act = get_hist_hit_count(event_file);
- /* Clear private_data to avoid warning in single_open() */
- file->private_data = NULL;
ret = single_open(file, hist_show, hist_file);
if (ret) {
kfree(hist_file);
@@ -6114,8 +6112,6 @@ static int event_hist_debug_open(struct inode *inode, struct file *file)
if (ret)
return ret;
- /* Clear private_data to avoid warning in single_open() */
- file->private_data = NULL;
ret = single_open(file, hist_debug_show, file);
if (ret)
tracing_release_file_tr(inode, file);
--
2.52.0
^ permalink raw reply related
* Re: Enhancing Conditional Filtering for Function Graph Tracer
From: Steven Rostedt @ 2026-02-16 15:09 UTC (permalink / raw)
To: Donglin Peng
Cc: Masami Hiramatsu, linux-trace-kernel, Linux Kernel Mailing List
In-Reply-To: <CAErzpmtMrHWQVAQx=jiq_sWo=+c_JOUoQmPafo-fq+Q3qi+BBw@mail.gmail.com>
On Sat, 14 Feb 2026 18:36:24 +0800
Donglin Peng <dolinux.peng@gmail.com> wrote:
> To simplify implementation, I propose extending a new trigger type
> (e.g., "funcgraph").
> In ftrace_graph_ignore_func, we could look up the corresponding trace_fprobe and
> trace_event_file based on trace->func, then decide whether to trace
> the function using
> a helper like the following:
>
> static bool ftrace_graph_filter(struct trace_fprobe *tf, struct
> ftrace_regs *fregs,
> struct trace_event_file *trace_file)
> {
> struct fentry_trace_entry_head *entry;
> struct trace_event_buffer fbuffer;
> struct event_trigger_data *data;
> int dsize;
>
> dsize = __get_data_size(&tf->tp, fregs, NULL);
> entry = trace_event_buffer_reserve(&fbuffer, trace_file,
> sizeof(*entry) + tf->tp.size + dsize);
> if (!entry)
> return false;
>
> entry = ring_buffer_event_data(fbuffer.event);
> store_trace_args(&entry[1], &tf->tp, fregs, NULL, sizeof(*entry), dsize);
>
> list_for_each_entry_rcu(data, &trace_file->triggers, list) {
> if (data->cmd_ops->trigger_type == TRIGGER_TYPE_FUNCGRAPH) {
> struct event_filter *filter = rcu_dereference_sched(data->filter);
> if (filter && filter_match_preds(filter, entry))
> return true; // Allow tracing
> }
> }
> return false; // Skip tracing
> }
>
> Does this approach make sense? Any suggestions or concerns?
My biggest concern is with performance. You want to run this against all
functions being traced?
How is this different than just using fprobes?
-- Steve
^ permalink raw reply
* Re: [PATCH v2] rv: Fix multiple definition of __pcpu_unique_da_mon_this
From: Steven Rostedt @ 2026-02-16 15:14 UTC (permalink / raw)
To: Mikhail Gavrilov
Cc: Gabriele Monaco, Daniel Bristot de Oliveira, Nam Cao,
linux-trace-kernel, linux-kernel
In-Reply-To: <20260216090141.757726-1-mikhail.v.gavrilov@gmail.com>
FYI, please send new versions of a patch as a separate thread. Do not send
it as a reply to the first version. That makes it much more difficult for
maintainers to keep track of.
You should also add what changed since the first version:
> The refactoring in commit 30984ccf31b7 ("rv: Refactor da_monitor to
> minimise macros") replaced per-monitor unique variable names
> (da_mon_##name) with a fixed name (da_mon_this).
>
> While this works for 'static' variables (each translation unit gets its
> own copy), DEFINE_PER_CPU internally generates a non-static dummy
> variable __pcpu_unique_<n> for each per-cpu definition. The requirement
> for this variable to be unique although static exists for modules on
> specific architectures (alpha) and if the kernel is built with
> CONFIG_DEBUG_FORCE_WEAK_PER_CPU (e.g. Fedora's debug kernel).
>
> When multiple per-cpu monitors (e.g. sco and sts) are built-in
> simultaneously, they all produce the same __pcpu_unique_da_mon_this
> symbol, causing a link error:
>
> ld: kernel/trace/rv/monitors/sts/sts.o: multiple definition of
> `__pcpu_unique_da_mon_this';
> kernel/trace/rv/monitors/sco/sco.o: first defined here
>
> Fix this by introducing a DA_MON_NAME macro that expands to a
> per-monitor unique name (da_mon_<MONITOR_NAME>) via the existing
> CONCATENATE helper. This restores the uniqueness that was present
> before the refactoring.
>
> Fixes: 30984ccf31b7 ("rv: Refactor da_monitor to minimise macros")
> Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
> Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
> ---
Changes since v1: https://lore.kernel.org/all/20260216074727.741822-1-mikhail.v.gavrilov@gmail.com/
- <list changes here>
> include/rv/da_monitor.h | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
> index db11d41bb438..7511f5464c48 10064
This way this patch has a link to the previous version. Then when this
patch gets committed, it the commit has a link to this patch, and that
patch has a link to a previous version and the chain of updates is archived
nicely.
-- Steve
^ permalink raw reply
* Re: [PATCH mm-unstable v14 07/16] khugepaged: introduce collapse_max_ptes_none helper function
From: Lorenzo Stoakes @ 2026-02-16 15:16 UTC (permalink / raw)
To: Nico Pache
Cc: linux-mm, linux-doc, linux-kernel, linux-trace-kernel, akpm,
david, ziy, baolin.wang, Liam.Howlett, ryan.roberts, dev.jain,
baohua, lance.yang, vbabka, rppt, surenb, mhocko, corbet, rostedt,
mhiramat, mathieu.desnoyers, matthew.brost, joshua.hahnjy,
rakie.kim, byungchul, gourry, ying.huang, apopple, jannh,
pfalcato, jackmanb, hannes, willy, peterx, wangkefeng.wang,
usamaarif642, sunnanyong, vishal.moola, thomas.hellstrom, yang,
kas, aarcange, raquini, anshuman.khandual, catalin.marinas, tiwai,
will, dave.hansen, jack, cl, jglisse, zokeefe, rientjes, rdunlap,
hughd, richard.weiyang
In-Reply-To: <CAA1CXcBQWPD=AxX9mCOdAOv85LTk+FPJQeEudQur-ymg4vbp5g@mail.gmail.com>
On Fri, Feb 06, 2026 at 10:44:03AM -0700, Nico Pache wrote:
> On Tue, Feb 3, 2026 at 5:09 AM Lorenzo Stoakes
> > > ---
> > > mm/khugepaged.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
> > > 1 file changed, 42 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> > > index 0f68902edd9a..9b7e05827749 100644
> > > --- a/mm/khugepaged.c
> > > +++ b/mm/khugepaged.c
> > > @@ -460,6 +460,44 @@ void __khugepaged_enter(struct mm_struct *mm)
> > > wake_up_interruptible(&khugepaged_wait);
> > > }
> > >
> > > +/**
> > > + * collapse_max_ptes_none - Calculate maximum allowed empty PTEs for collapse
> > > + * @order: The folio order being collapsed to
> > > + * @full_scan: Whether this is a full scan (ignore limits)
> > > + *
> > > + * For madvise-triggered collapses (full_scan=true), all limits are bypassed
> > > + * and allow up to HPAGE_PMD_NR - 1 empty PTEs.
> > > + *
> > > + * For PMD-sized collapses (order == HPAGE_PMD_ORDER), use the configured
> > > + * khugepaged_max_ptes_none value.
> > > + *
> > > + * For mTHP collapses, we currently only support khugepaged_max_pte_none values
> > > + * of 0 or (HPAGE_PMD_NR - 1). Any other value will emit a warning and no mTHP
> > > + * collapse will be attempted
> > > + *
> > > + * Return: Maximum number of empty PTEs allowed for the collapse operation
> > > + */
> > > +static unsigned int collapse_max_ptes_none(unsigned int order, bool full_scan)
> > > +{
> > > + /* ignore max_ptes_none limits */
> > > + if (full_scan)
> > > + return HPAGE_PMD_NR - 1;
> >
> > I wonder if, given we are effectively doing:
> >
> > const unsigned int nr_pages = collapse_max_ptes_none(order, /*full_scan=*/true);
> >
> > ...
> >
> > foo(nr_pages);
> >
> > In places where we ignore limits, whether we would be better off putting
> > HPAGE_PMD_NR - 1 into a define and just using that in these cases, like:
> >
> > #define COLLAPSE_MAX_PTES_LIM (HPAGE_PMD_NR - 1)
>
> Would a shorter name be appropriate? COLLAPSE_MAX_PTES_LIM(IT) is
> quite long. Can we call it MAX_PTES_LIMIT or KHUGE_MAX_PTES_LIM?
Yeah sure re: shorter/better name :) to be fair my suggestion is pretty
terrible, kinda just getting at the notion of there being _some_ define.
But MAX_PTES_LIMIT or KHUGE_MAX_PTES_LIM I think are unclear.
MAX_COLLAPSE_PTES?
Cheers, Lorenzo
^ permalink raw reply
* Re: [PATCH mm-unstable v14 08/16] khugepaged: generalize collapse_huge_page for mTHP collapse
From: Lorenzo Stoakes @ 2026-02-16 15:20 UTC (permalink / raw)
To: Nico Pache
Cc: linux-mm, linux-doc, linux-kernel, linux-trace-kernel, akpm,
david, ziy, baolin.wang, Liam.Howlett, ryan.roberts, dev.jain,
baohua, lance.yang, vbabka, rppt, surenb, mhocko, corbet, rostedt,
mhiramat, mathieu.desnoyers, matthew.brost, joshua.hahnjy,
rakie.kim, byungchul, gourry, ying.huang, apopple, jannh,
pfalcato, jackmanb, hannes, willy, peterx, wangkefeng.wang,
usamaarif642, sunnanyong, vishal.moola, thomas.hellstrom, yang,
kas, aarcange, raquini, anshuman.khandual, catalin.marinas, tiwai,
will, dave.hansen, jack, cl, jglisse, zokeefe, rientjes, rdunlap,
hughd, richard.weiyang
In-Reply-To: <CAA1CXcCCbiV9j+_SVNJrkfVRqKPXjGg+Lt3YnyNUhDHWkRjHGQ@mail.gmail.com>
On Wed, Feb 04, 2026 at 03:00:57PM -0700, Nico Pache wrote:
> On Tue, Feb 3, 2026 at 6:13 AM Lorenzo Stoakes
> <lorenzo.stoakes@oracle.com> wrote:
> >
> > On Thu, Jan 22, 2026 at 12:28:33PM -0700, Nico Pache wrote:
> > > Pass an order and offset to collapse_huge_page to support collapsing anon
> > > memory to arbitrary orders within a PMD. order indicates what mTHP size we
> > > are attempting to collapse to, and offset indicates were in the PMD to
> > > start the collapse attempt.
> > >
> > > For non-PMD collapse we must leave the anon VMA write locked until after
> > > we collapse the mTHP-- in the PMD case all the pages are isolated, but in
> > > the mTHP case this is not true, and we must keep the lock to prevent
> > > changes to the VMA from occurring.
> > >
> > > Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> > > Tested-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> > > Signed-off-by: Nico Pache <npache@redhat.com>
> > > ---
> > > mm/khugepaged.c | 111 +++++++++++++++++++++++++++++++-----------------
> > > 1 file changed, 71 insertions(+), 40 deletions(-)
> > >
> > > diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> > > index 9b7e05827749..76cb17243793 100644
> > > --- a/mm/khugepaged.c
> > > +++ b/mm/khugepaged.c
> > > @@ -1151,44 +1151,54 @@ static enum scan_result alloc_charge_folio(struct folio **foliop, struct mm_stru
> > > return SCAN_SUCCEED;
> > > }
> > >
> > > -static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long address,
> > > - int referenced, int unmapped, struct collapse_control *cc)
> > > +static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long start_addr,
> > > + int referenced, int unmapped, struct collapse_control *cc,
> > > + bool *mmap_locked, unsigned int order)
> > > {
> > > LIST_HEAD(compound_pagelist);
> > > pmd_t *pmd, _pmd;
> > > - pte_t *pte;
> > > + pte_t *pte = NULL;
> > > pgtable_t pgtable;
> > > struct folio *folio;
> > > spinlock_t *pmd_ptl, *pte_ptl;
> > > enum scan_result result = SCAN_FAIL;
> > > struct vm_area_struct *vma;
> > > struct mmu_notifier_range range;
> > > + bool anon_vma_locked = false;
> > > + const unsigned long nr_pages = 1UL << order;
> > > + const unsigned long pmd_address = start_addr & HPAGE_PMD_MASK;
> > >
> > > - VM_BUG_ON(address & ~HPAGE_PMD_MASK);
> > > + VM_WARN_ON_ONCE(pmd_address & ~HPAGE_PMD_MASK);
> > >
> > > /*
> > > * Before allocating the hugepage, release the mmap_lock read lock.
> > > * The allocation can take potentially a long time if it involves
> > > * sync compaction, and we do not need to hold the mmap_lock during
> > > * that. We will recheck the vma after taking it again in write mode.
> > > + * If collapsing mTHPs we may have already released the read_lock.
> > > */
> > > - mmap_read_unlock(mm);
> > > + if (*mmap_locked) {
> > > + mmap_read_unlock(mm);
> > > + *mmap_locked = false;
> > > + }
> > >
> > > - result = alloc_charge_folio(&folio, mm, cc, HPAGE_PMD_ORDER);
> > > + result = alloc_charge_folio(&folio, mm, cc, order);
> > > if (result != SCAN_SUCCEED)
> > > goto out_nolock;
> > >
> > > mmap_read_lock(mm);
> > > - result = hugepage_vma_revalidate(mm, address, true, &vma, cc,
> > > - HPAGE_PMD_ORDER);
> > > + *mmap_locked = true;
> > > + result = hugepage_vma_revalidate(mm, pmd_address, true, &vma, cc, order);
> >
> > Why would we use the PMD address here rather than the actual start address?
>
> The revalidation relies on the pmd_addr not the start_addr. It (only)
> uses this to make sure the VMA is still at least PMD sized, and it
> uses the order to validate that the target order is allowed. I left a
> small comment about this in the revalidate function.
Yeah having these different addresses is a bit icky, easy to make mistakes here.
Oh how we need to refactor all of these...
>
> >
> > Also please add /*expect_anon=*/ before the 'true' because it's hard to
> > understand what that references.
>
> ack
>
> >
> > > if (result != SCAN_SUCCEED) {
> > > mmap_read_unlock(mm);
> > > + *mmap_locked = false;
> > > goto out_nolock;
> > > }
> > >
> > > - result = find_pmd_or_thp_or_none(mm, address, &pmd);
> > > + result = find_pmd_or_thp_or_none(mm, pmd_address, &pmd);
> > > if (result != SCAN_SUCCEED) {
> > > mmap_read_unlock(mm);
> > > + *mmap_locked = false;
> > > goto out_nolock;
> > > }
> > >
> > > @@ -1198,13 +1208,16 @@ static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long a
> > > * released when it fails. So we jump out_nolock directly in
> > > * that case. Continuing to collapse causes inconsistency.
> > > */
> > > - result = __collapse_huge_page_swapin(mm, vma, address, pmd,
> > > - referenced, HPAGE_PMD_ORDER);
> > > - if (result != SCAN_SUCCEED)
> > > + result = __collapse_huge_page_swapin(mm, vma, start_addr, pmd,
> > > + referenced, order);
> > > + if (result != SCAN_SUCCEED) {
> > > + *mmap_locked = false;
> > > goto out_nolock;
> > > + }
> > > }
> > >
> > > mmap_read_unlock(mm);
> > > + *mmap_locked = false;
> > > /*
> > > * Prevent all access to pagetables with the exception of
> > > * gup_fast later handled by the ptep_clear_flush and the VM
> > > @@ -1214,20 +1227,20 @@ static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long a
> > > * mmap_lock.
> > > */
> > > mmap_write_lock(mm);
> > > - result = hugepage_vma_revalidate(mm, address, true, &vma, cc,
> > > - HPAGE_PMD_ORDER);
> > > + result = hugepage_vma_revalidate(mm, pmd_address, true, &vma, cc, order);
> > > if (result != SCAN_SUCCEED)
> > > goto out_up_write;
> > > /* check if the pmd is still valid */
> > > vma_start_write(vma);
> > > - result = check_pmd_still_valid(mm, address, pmd);
> > > + result = check_pmd_still_valid(mm, pmd_address, pmd);
> > > if (result != SCAN_SUCCEED)
> > > goto out_up_write;
> > >
> > > anon_vma_lock_write(vma->anon_vma);
> > > + anon_vma_locked = true;
> > >
> > > - mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, mm, address,
> > > - address + HPAGE_PMD_SIZE);
> > > + mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, mm, start_addr,
> > > + start_addr + (PAGE_SIZE << order));
> > > mmu_notifier_invalidate_range_start(&range);
> > >
> > > pmd_ptl = pmd_lock(mm, pmd); /* probably unnecessary */
> > > @@ -1239,24 +1252,21 @@ static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long a
> > > * Parallel GUP-fast is fine since GUP-fast will back off when
> > > * it detects PMD is changed.
> > > */
> > > - _pmd = pmdp_collapse_flush(vma, address, pmd);
> > > + _pmd = pmdp_collapse_flush(vma, pmd_address, pmd);
> > > spin_unlock(pmd_ptl);
> > > mmu_notifier_invalidate_range_end(&range);
> > > tlb_remove_table_sync_one();
> > >
> > > - pte = pte_offset_map_lock(mm, &_pmd, address, &pte_ptl);
> > > + pte = pte_offset_map_lock(mm, &_pmd, start_addr, &pte_ptl);
> > > if (pte) {
> > > - result = __collapse_huge_page_isolate(vma, address, pte, cc,
> > > - HPAGE_PMD_ORDER,
> > > - &compound_pagelist);
> > > + result = __collapse_huge_page_isolate(vma, start_addr, pte, cc,
> > > + order, &compound_pagelist);
> > > spin_unlock(pte_ptl);
> > > } else {
> > > result = SCAN_NO_PTE_TABLE;
> > > }
> > >
> > > if (unlikely(result != SCAN_SUCCEED)) {
> > > - if (pte)
> > > - pte_unmap(pte);
> > > spin_lock(pmd_ptl);
> > > BUG_ON(!pmd_none(*pmd));
> > > /*
> > > @@ -1266,21 +1276,21 @@ static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long a
> > > */
> > > pmd_populate(mm, pmd, pmd_pgtable(_pmd));
> > > spin_unlock(pmd_ptl);
> > > - anon_vma_unlock_write(vma->anon_vma);
> > > goto out_up_write;
> > > }
> > >
> > > /*
> > > - * All pages are isolated and locked so anon_vma rmap
> > > - * can't run anymore.
> > > + * For PMD collapse all pages are isolated and locked so anon_vma
> > > + * rmap can't run anymore. For mTHP collapse we must hold the lock
> > > */
> > > - anon_vma_unlock_write(vma->anon_vma);
> > > + if (is_pmd_order(order)) {
> > > + anon_vma_unlock_write(vma->anon_vma);
> > > + anon_vma_locked = false;
> > > + }
> > >
> > > result = __collapse_huge_page_copy(pte, folio, pmd, _pmd,
> > > - vma, address, pte_ptl,
> > > - HPAGE_PMD_ORDER,
> > > - &compound_pagelist);
> > > - pte_unmap(pte);
> > > + vma, start_addr, pte_ptl,
> > > + order, &compound_pagelist);
> > > if (unlikely(result != SCAN_SUCCEED))
> > > goto out_up_write;
> > >
> > > @@ -1290,20 +1300,42 @@ static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long a
> > > * write.
> > > */
> > > __folio_mark_uptodate(folio);
> > > - pgtable = pmd_pgtable(_pmd);
> > > + if (is_pmd_order(order)) { /* PMD collapse */
> > > + pgtable = pmd_pgtable(_pmd);
> > >
> > > - spin_lock(pmd_ptl);
> > > - BUG_ON(!pmd_none(*pmd));
> > > - pgtable_trans_huge_deposit(mm, pmd, pgtable);
> > > - map_anon_folio_pmd_nopf(folio, pmd, vma, address);
> > > + spin_lock(pmd_ptl);
> > > + WARN_ON_ONCE(!pmd_none(*pmd));
> > > + pgtable_trans_huge_deposit(mm, pmd, pgtable);
> > > + map_anon_folio_pmd_nopf(folio, pmd, vma, pmd_address);
> > > + } else { /* mTHP collapse */
> > > + pte_t mthp_pte = mk_pte(folio_page(folio, 0), vma->vm_page_prot);
> > > +
> > > + mthp_pte = maybe_mkwrite(pte_mkdirty(mthp_pte), vma);
> > > + spin_lock(pmd_ptl);
> > > + WARN_ON_ONCE(!pmd_none(*pmd));
> > > + folio_ref_add(folio, nr_pages - 1);
> > > + folio_add_new_anon_rmap(folio, vma, start_addr, RMAP_EXCLUSIVE);
> > > + folio_add_lru_vma(folio, vma);
> > > + set_ptes(vma->vm_mm, start_addr, pte, mthp_pte, nr_pages);
> > > + update_mmu_cache_range(NULL, vma, start_addr, pte, nr_pages);
> > > +
> > > + smp_wmb(); /* make PTEs visible before PMD. See pmd_install() */
> > > + pmd_populate(mm, pmd, pmd_pgtable(_pmd));
> >
> > I seriously hate this being open-coded, can we separate it out into another
> > function?
>
> Yeah I think we've discussed this before. I started to generalize
> this, and apply it to other parts of the kernel that maintain a
> similar pattern, but each potential user of the helper was slightly
> different in its approach and I was unable to find a quick solution to
> make it apply to all. I think it will require a lot more thought to
> cleanly refactor this. I figured I could leave this to the later
> cleanup work, or I could just create a static function just for
> khugepaged for now?
Yeah let's at least separate it out for this logic anyway.
Really we should have done the refactoring in advance of these changes, but that
ship has sailed :)
>
> >
> > > + }
> > > spin_unlock(pmd_ptl);
> > >
> > > folio = NULL;
> > >
> > > result = SCAN_SUCCEED;
> > > out_up_write:
> > > + if (anon_vma_locked)
> > > + anon_vma_unlock_write(vma->anon_vma);
> >
> > Thanks it's much better tracking this specifically.
> >
> > The whole damn thing needs refactoring (by this I mean - khugepaged and really
> > THP in general to be clear :) but it's not your fault.
>
> Yeah it has not been the prettiest code to try and understand/work on!
:)
>
> >
> > Could I ask though whether you might help out with some cleanups after this
> > lands :)
> >
> > I feel like we all need to do our bit to pay down some technical debt!
>
>
> Yes ofc! I had already planned on doing so. I have some in mind, and I
> believe others have already tackled some. After this land, let's
> discuss further plans (discussion thread or THP meeting).
Yeah, I'll get that TODO list discussed in meeting shared soon...
>
> Cheers,
> -- Nico
>
> >
> > > + if (pte)
> > > + pte_unmap(pte);
> > > mmap_write_unlock(mm);
> > > + *mmap_locked = false;
> > > out_nolock:
> > > + WARN_ON_ONCE(*mmap_locked);
> > > if (folio)
> > > folio_put(folio);
> > > trace_mm_collapse_huge_page(mm, result == SCAN_SUCCEED, result);
> > > @@ -1471,9 +1503,8 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
> > > pte_unmap_unlock(pte, ptl);
> > > if (result == SCAN_SUCCEED) {
> > > result = collapse_huge_page(mm, start_addr, referenced,
> > > - unmapped, cc);
> > > - /* collapse_huge_page will return with the mmap_lock released */
> > > - *mmap_locked = false;
> > > + unmapped, cc, mmap_locked,
> > > + HPAGE_PMD_ORDER);
> > > }
> > > out:
> > > trace_mm_khugepaged_scan_pmd(mm, folio, referenced,
> > > --
> > > 2.52.0
> > >
> >
> > Cheers, Lorenzo
> >
>
Cheers, Lorenzo
^ permalink raw reply
* Re: [PATCH v6 3/3] mm: vmscan: add PIDs to vmscan tracepoints
From: Usama Arif @ 2026-02-16 16:02 UTC (permalink / raw)
To: Thomas Ballasi
Cc: Usama Arif, akpm, axelrasmussen, david, hannes, linux-mm,
linux-trace-kernel, lorenzo.stoakes, mhiramat, mhocko, rostedt,
shakeel.butt, weixugc, yuanchu, zhengqi.arch
In-Reply-To: <20260213181537.54350-4-tballasi@linux.microsoft.com>
On Fri, 13 Feb 2026 10:15:37 -0800 Thomas Ballasi <tballasi@linux.microsoft.com> wrote:
> The changes aims at adding additionnal tracepoints variables to help
> debuggers attribute them to specific processes.
>
> The PID field uses in_task() to reliably detect when we're in process
> context and can safely access current->pid. When not in process
> context (such as in interrupt or in an asynchronous RCU context), the
> field is set to -1 as a sentinel value.
>
> Signed-off-by: Thomas Ballasi <tballasi@linux.microsoft.com>
> ---
> include/trace/events/vmscan.h | 35 +++++++++++++++++++++++++----------
> 1 file changed, 25 insertions(+), 10 deletions(-)
>
> diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
> index 1212f6a7c223e..a68b712ef757a 100644
> --- a/include/trace/events/vmscan.h
> +++ b/include/trace/events/vmscan.h
> @@ -122,18 +122,22 @@ DECLARE_EVENT_CLASS(mm_vmscan_direct_reclaim_begin_template,
> __field( unsigned long, gfp_flags )
> __field( u64, memcg_id )
> __field( int, order )
> + __field( int, pid )
> ),
>
> TP_fast_assign(
> __entry->gfp_flags = (__force unsigned long)gfp_flags;
> __entry->order = order;
> + __entry->pid = current->pid;
> __entry->memcg_id = mem_cgroup_id(memcg);
> ),
>
> - TP_printk("order=%d gfp_flags=%s memcg_id=%llu",
> + TP_printk("order=%d gfp_flags=%s pid=%d memcg_id=%llu %s",
> __entry->order,
> show_gfp_flags(__entry->gfp_flags),
> - __entry->memcg_id)
> + __entry->pid,
> + __entry->memcg_id,
> + __event_in_irq() ? "(in-irq)" : "")
> );
>
> DEFINE_EVENT(mm_vmscan_direct_reclaim_begin_template, mm_vmscan_direct_reclaim_begin,
> @@ -168,16 +172,20 @@ DECLARE_EVENT_CLASS(mm_vmscan_direct_reclaim_end_template,
> TP_STRUCT__entry(
> __field( unsigned long, nr_reclaimed )
> __field( u64, memcg_id )
> + __field( int, pid )
> ),
>
> TP_fast_assign(
> __entry->nr_reclaimed = nr_reclaimed;
> __entry->memcg_id = mem_cgroup_id(memcg);
> + __entry->pid = current->pid;
> ),
>
> - TP_printk("nr_reclaimed=%lu memcg_id=%llu",
> + TP_printk("nr_reclaimed=%lu pid=%d memcg_id=%llu %s",
> __entry->nr_reclaimed,
> - __entry->memcg_id)
> + __entry->pid,
> + __entry->memcg_id,
> + __event_in_irq() ? "(in-irq)" : "")
> );
>
> DEFINE_EVENT(mm_vmscan_direct_reclaim_end_template, mm_vmscan_direct_reclaim_end,
> @@ -220,9 +228,10 @@ TRACE_EVENT(mm_shrink_slab_start,
> __field(unsigned long, cache_items)
> __field(unsigned long long, delta)
> __field(unsigned long, total_scan)
> + __field(u64, memcg_id)
> __field(int, priority)
> __field(int, nid)
> - __field(u64, memcg_id)
> + __field(int, pid)
> ),
>
> TP_fast_assign(
> @@ -236,19 +245,22 @@ TRACE_EVENT(mm_shrink_slab_start,
> __entry->priority = priority;
> __entry->nid = sc->nid;
> __entry->memcg_id = mem_cgroup_id(memcg);
> + __entry->pid = current->pid;
> ),
>
> - TP_printk("%pS %p: nid: %d memcg_id: %llu objects to shrink %ld gfp_flags %s cache items %ld delta %lld total_scan %ld priority %d",
> + TP_printk("%pS %p: nid: %d pid: %d memcg_id: %llu objects to shrink %ld gfp_flags %s cache items %ld delta %lld total_scan %ld priority %d %s",
> __entry->shrink,
> __entry->shr,
> __entry->nid,
> + __entry->pid,
> __entry->memcg_id,
> __entry->nr_objects_to_shrink,
> show_gfp_flags(__entry->gfp_flags),
> __entry->cache_items,
> __entry->delta,
> __entry->total_scan,
> - __entry->priority)
> + __entry->priority,
> + __event_in_irq() ? "(in-irq)" : "")
> );
>
> TRACE_EVENT(mm_shrink_slab_end,
> @@ -266,29 +278,32 @@ TRACE_EVENT(mm_shrink_slab_end,
> __field(long, total_scan)
> __field(int, nid)
> __field(int, retval)
> + __field(int, pid)
> __field(u64, memcg_id)
> ),
>
> TP_fast_assign(
> __entry->shr = shr;
> - __entry->shrink = shr->scan_objects;
__entry->shrink is removed here, but still printed below. Was this an intended
change of this commit?
> __entry->unused_scan = unused_scan_cnt;
> __entry->new_scan = new_scan_cnt;
> __entry->total_scan = total_scan;
> __entry->nid = nid;
> __entry->retval = shrinker_retval;
> + __entry->pid = current->pid;
> __entry->memcg_id = mem_cgroup_id(memcg);
> ),
>
> - TP_printk("%pS %p: nid: %d memcg_id: %llu unused scan count %ld new scan count %ld total_scan %ld last shrinker return val %d",
> + TP_printk("%pS %p: nid: %d pid: %d memcg_id: %llu unused scan count %ld new scan count %ld total_scan %ld last shrinker return val %d %s",
> __entry->shrink,
> __entry->shr,
> __entry->nid,
> + __entry->pid,
> __entry->memcg_id,
> __entry->unused_scan,
> __entry->new_scan,
> __entry->total_scan,
> - __entry->retval)
> + __entry->retval,
> + __event_in_irq() ? "(in-irq)" : "")
> );
>
> TRACE_EVENT(mm_vmscan_lru_isolate,
> --
> 2.33.8
>
>
^ permalink raw reply
* Re: [PATCH v6 1/3] tracing: Add __event_in_*irq() helpers
From: Usama Arif @ 2026-02-16 16:13 UTC (permalink / raw)
To: Thomas Ballasi
Cc: Usama Arif, akpm, axelrasmussen, david, hannes, linux-mm,
linux-trace-kernel, lorenzo.stoakes, mhiramat, mhocko, rostedt,
shakeel.butt, weixugc, yuanchu, zhengqi.arch
In-Reply-To: <20260213181537.54350-2-tballasi@linux.microsoft.com>
On Fri, 13 Feb 2026 10:15:35 -0800 Thomas Ballasi <tballasi@linux.microsoft.com> wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
>
> Some trace events want to expose in their output if they were triggered in
> an interrupt or softirq context. Instead of recording this in the event
> structure itself, as this information is stored in the flags portion of
> the event header, add helper macros that can be used in the print format:
>
> TP_printk("val=%d %s", __entry->val, __entry_in_irq() ? "(in-irq)" : "")
Should this read __event_in_irq() and not __entry_in_irq()?
>
> This will output "(in-irq)" for the event in the trace data if the event
> was triggered in hard or soft interrupt context.
>
> Link: https://lore.kernel.org/all/20251229132942.31a2b583@gandalf.local.home/
>
^ permalink raw reply
* [PATCH v3] rv: Fix multiple definition of __pcpu_unique_da_mon_this
From: Mikhail Gavrilov @ 2026-02-16 17:27 UTC (permalink / raw)
To: Gabriele Monaco, Daniel Bristot de Oliveira, Steven Rostedt
Cc: Nam Cao, linux-trace-kernel, linux-kernel, Mikhail Gavrilov
The refactoring in commit 30984ccf31b7 ("rv: Refactor da_monitor to
minimise macros") replaced per-monitor unique variable names
(da_mon_##name) with a fixed name (da_mon_this).
While this works for 'static' variables (each translation unit gets its
own copy), DEFINE_PER_CPU internally generates a non-static dummy
variable __pcpu_unique_<n> for each per-cpu definition. The requirement
for this variable to be unique although static exists for modules on
specific architectures (alpha) and if the kernel is built with
CONFIG_DEBUG_FORCE_WEAK_PER_CPU (e.g. Fedora's debug kernel).
When multiple per-cpu monitors (e.g. sco and sts) are built-in
simultaneously, they all produce the same __pcpu_unique_da_mon_this
symbol, causing a link error:
ld: kernel/trace/rv/monitors/sts/sts.o: multiple definition of
`__pcpu_unique_da_mon_this';
kernel/trace/rv/monitors/sco/sco.o: first defined here
Fix this by introducing a DA_MON_NAME macro that expands to a
per-monitor unique name (da_mon_<MONITOR_NAME>) via the existing
CONCATENATE helper. This restores the uniqueness that was present
before the refactoring.
Fixes: 30984ccf31b7 ("rv: Refactor da_monitor to minimise macros")
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
---
Changes since v1: https://lore.kernel.org/all/20260216074727.47292-1-mikhail.v.gavrilov@gmail.com/
- Updated commit message with details on when __pcpu_unique_ uniqueness
is required (alpha modules, CONFIG_DEBUG_FORCE_WEAK_PER_CPU)
- Added code comment above DA_MON_NAME explaining the rationale
include/rv/da_monitor.h | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
index db11d41bb438..7511f5464c48 100644
--- a/include/rv/da_monitor.h
+++ b/include/rv/da_monitor.h
@@ -20,6 +20,12 @@
#include <linux/bug.h>
#include <linux/sched.h>
+/*
+ * Per-cpu variables require a unique name although static in some
+ * configurations (e.g. CONFIG_DEBUG_FORCE_WEAK_PER_CPU or alpha modules).
+ */
+#define DA_MON_NAME CONCATENATE(da_mon_, MONITOR_NAME)
+
static struct rv_monitor rv_this;
static void react(enum states curr_state, enum events event)
@@ -183,14 +189,14 @@ static inline bool da_event(struct da_monitor *da_mon, struct task_struct *tsk,
/*
* global monitor (a single variable)
*/
-static struct da_monitor da_mon_this;
+static struct da_monitor DA_MON_NAME;
/*
* da_get_monitor - return the global monitor address
*/
static struct da_monitor *da_get_monitor(void)
{
- return &da_mon_this;
+ return &DA_MON_NAME;
}
/*
@@ -223,14 +229,14 @@ static inline void da_monitor_destroy(void) { }
/*
* per-cpu monitor variables
*/
-static DEFINE_PER_CPU(struct da_monitor, da_mon_this);
+static DEFINE_PER_CPU(struct da_monitor, DA_MON_NAME);
/*
* da_get_monitor - return current CPU monitor address
*/
static struct da_monitor *da_get_monitor(void)
{
- return this_cpu_ptr(&da_mon_this);
+ return this_cpu_ptr(&DA_MON_NAME);
}
/*
@@ -242,7 +248,7 @@ static void da_monitor_reset_all(void)
int cpu;
for_each_cpu(cpu, cpu_online_mask) {
- da_mon = per_cpu_ptr(&da_mon_this, cpu);
+ da_mon = per_cpu_ptr(&DA_MON_NAME, cpu);
da_monitor_reset(da_mon);
}
}
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v2] blk-mq: use NOIO context to prevent deadlock during debugfs creation
From: Mohamed Khalfella @ 2026-02-16 18:28 UTC (permalink / raw)
To: Yu Kuai
Cc: axboe, nilay, ming.lei, hch, yi.zhang, shinichiro.kawasaki,
kbusch, rostedt, mhiramat, mathieu.desnoyers, linux-block,
linux-kernel, linux-trace-kernel
In-Reply-To: <20260214054350.2322436-1-yukuai@fnnas.com>
On Sat 2026-02-14 13:43:50 +0800, Yu Kuai wrote:
> Creating debugfs entries can trigger fs reclaim, which can enter back
> into the block layer request_queue. This can cause deadlock if the
> queue is frozen.
>
> Previously, a WARN_ON_ONCE check was used in debugfs_create_files()
> to detect this condition, but it was racy since the queue can be frozen
> from another context at any time.
>
> Introduce blk_debugfs_lock()/blk_debugfs_unlock() helpers that combine
> the debugfs_mutex with memalloc_noio_save()/restore() to prevent fs
> reclaim from triggering block I/O. Also add blk_debugfs_lock_nomemsave()
> and blk_debugfs_unlock_nomemrestore() variants for callers that don't
> need NOIO protection (e.g., debugfs removal or read-only operations).
>
> Replace all raw debugfs_mutex lock/unlock pairs with these helpers,
> using the _nomemsave/_nomemrestore variants where appropriate.
>
> Reported-by: Yi Zhang <yi.zhang@redhat.com>
> Closes: https://lore.kernel.org/all/CAHj4cs9gNKEYAPagD9JADfO5UH+OiCr4P7OO2wjpfOYeM-RV=A@mail.gmail.com/
> Reported-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> Closes: https://lore.kernel.org/all/aYWQR7CtYdk3K39g@shinmob/
> Suggested-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Yu Kuai <yukuai@fnnas.com>
Maybe added Fixes: 65d466b62984 ("blk-mq-debugfs: warn about possible deadlock")?
Reviewed-by: Mohamed Khalfella <mkhalfella@purestorage.com>
^ permalink raw reply
* Re: [PATCH v3] rv: Fix multiple definition of __pcpu_unique_da_mon_this
From: Gabriele Monaco @ 2026-02-16 19:26 UTC (permalink / raw)
To: Mikhail Gavrilov
Cc: Daniel Bristot de Oliveira, Steven Rostedt, Nam Cao,
linux-trace-kernel, linux-kernel
In-Reply-To: <20260216172707.1441516-1-mikhail.v.gavrilov@gmail.com>
2026-02-16T17:27:23Z Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>:
> The refactoring in commit 30984ccf31b7 ("rv: Refactor da_monitor to
> minimise macros") replaced per-monitor unique variable names
> (da_mon_##name) with a fixed name (da_mon_this).
>
> While this works for 'static' variables (each translation unit gets its
> own copy), DEFINE_PER_CPU internally generates a non-static dummy
> variable __pcpu_unique_<n> for each per-cpu definition. The requirement
> for this variable to be unique although static exists for modules on
> specific architectures (alpha) and if the kernel is built with
> CONFIG_DEBUG_FORCE_WEAK_PER_CPU (e.g. Fedora's debug kernel).
>
> When multiple per-cpu monitors (e.g. sco and sts) are built-in
> simultaneously, they all produce the same __pcpu_unique_da_mon_this
> symbol, causing a link error:
>
> ld: kernel/trace/rv/monitors/sts/sts.o: multiple definition of
> `__pcpu_unique_da_mon_this';
> kernel/trace/rv/monitors/sco/sco.o: first defined here
>
> Fix this by introducing a DA_MON_NAME macro that expands to a
> per-monitor unique name (da_mon_<MONITOR_NAME>) via the existing
> CONCATENATE helper. This restores the uniqueness that was present
> before the refactoring.
>
Thanks Mikhail,
there was actually no need to send a v3, I believe Steve's was simply a suggestion for future patches.
(Now actually the reader following the link would wonder if there's any difference since V2, I'm assuming there isn't).
I'm going to take this patch and send it once 7.0-rc1 is out. You don't need to do anything else now.
Cheers,
Gabriele
> Fixes: 30984ccf31b7 ("rv: Refactor da_monitor to minimise macros")
> Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
> Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
> ---
> Changes since v1: https://lore.kernel.org/all/20260216074727.47292-1-mikhail.v.gavrilov@gmail.com/
> - Updated commit message with details on when __pcpu_unique_ uniqueness
> is required (alpha modules, CONFIG_DEBUG_FORCE_WEAK_PER_CPU)
> - Added code comment above DA_MON_NAME explaining the rationale
>
> include/rv/da_monitor.h | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
> index db11d41bb438..7511f5464c48 100644
> --- a/include/rv/da_monitor.h
> +++ b/include/rv/da_monitor.h
> @@ -20,6 +20,12 @@
> #include <linux/bug.h>
> #include <linux/sched.h>
>
> +/*
> + * Per-cpu variables require a unique name although static in some
> + * configurations (e.g. CONFIG_DEBUG_FORCE_WEAK_PER_CPU or alpha modules).
> + */
> +#define DA_MON_NAME CONCATENATE(da_mon_, MONITOR_NAME)
> +
> static struct rv_monitor rv_this;
>
> static void react(enum states curr_state, enum events event)
> @@ -183,14 +189,14 @@ static inline bool da_event(struct da_monitor *da_mon, struct task_struct *tsk,
> /*
> * global monitor (a single variable)
> */
> -static struct da_monitor da_mon_this;
> +static struct da_monitor DA_MON_NAME;
>
> /*
> * da_get_monitor - return the global monitor address
> */
> static struct da_monitor *da_get_monitor(void)
> {
> - return &da_mon_this;
> + return &DA_MON_NAME;
> }
>
> /*
> @@ -223,14 +229,14 @@ static inline void da_monitor_destroy(void) { }
> /*
> * per-cpu monitor variables
> */
> -static DEFINE_PER_CPU(struct da_monitor, da_mon_this);
> +static DEFINE_PER_CPU(struct da_monitor, DA_MON_NAME);
>
> /*
> * da_get_monitor - return current CPU monitor address
> */
> static struct da_monitor *da_get_monitor(void)
> {
> - return this_cpu_ptr(&da_mon_this);
> + return this_cpu_ptr(&DA_MON_NAME);
> }
>
> /*
> @@ -242,7 +248,7 @@ static void da_monitor_reset_all(void)
> int cpu;
>
> for_each_cpu(cpu, cpu_online_mask) {
> - da_mon = per_cpu_ptr(&da_mon_this, cpu);
> + da_mon = per_cpu_ptr(&DA_MON_NAME, cpu);
> da_monitor_reset(da_mon);
> }
> }
> --
> 2.53.0
^ permalink raw reply
* Re: [PATCH v6 1/3] tracing: Add __event_in_*irq() helpers
From: Steven Rostedt @ 2026-02-17 0:54 UTC (permalink / raw)
To: Usama Arif
Cc: Thomas Ballasi, akpm, axelrasmussen, david, hannes, linux-mm,
linux-trace-kernel, lorenzo.stoakes, mhiramat, mhocko,
shakeel.butt, weixugc, yuanchu, zhengqi.arch
In-Reply-To: <20260216161323.4098568-1-usama.arif@linux.dev>
On Mon, 16 Feb 2026 08:13:21 -0800
Usama Arif <usama.arif@linux.dev> wrote:
> > TP_printk("val=%d %s", __entry->val, __entry_in_irq() ? "(in-irq)" : "")
>
> Should this read __event_in_irq() and not __entry_in_irq()?
Yeah. That's when you should have done "cut-and-paste" but instead just
typed it in by memory :-p
-- Steve
^ permalink raw reply
* Re: Enhancing Conditional Filtering for Function Graph Tracer
From: Donglin Peng @ 2026-02-17 3:09 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Steven Rostedt, linux-trace-kernel, Linux Kernel Mailing List
In-Reply-To: <20260216162716.dc8ec2f3f2fe409b6c11e491@kernel.org>
On Mon, Feb 16, 2026 at 3:27 PM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> On Sat, 14 Feb 2026 18:36:24 +0800
> Donglin Peng <dolinux.peng@gmail.com> wrote:
>
> > Hi Steve and Masami,
> >
> > I recently had an idea while using the function graph tracer.
> > Currently, it supports basic
> > filtering via set_ftrace_pid(PID-based) and
> > set_graph_function(function name-based).
> > However, this filtering mechanism feels somewhat limited. Given that
> > we already have
> > features like funcgraph-args, I wonder if we could enhance filtering by allowing
> > conditional tracking based on function parameters — similar to how
> > trace events support
> > filters/triggers.
>
> Agreed. I think it is a good idea.
>
> >
> > To simplify implementation, I propose extending a new trigger type
> > (e.g., "funcgraph").
>
> OK, and I think the question is what do we want to filter. As you know,
> the kernel function call-graph is a very context depending. I think we
> may be better to start with specifying the pid filter via the tringger
> instead of enabling/disabling it on all cpus.
>
> echo "funcgraph:setpid:common_pid" >> events/fprobes/func_foo/trigger
>
> Then, we can also specify a specific task's pid too via sched_switch
> events etc. Also we can add "clearpid". Maybe we also need a special
> PID (e.g. -1) for trace no process instead of using tracing_on.
Thanks, I'm uncertain if modifying triggers is the right approach. My initial
thought was to extend the functionality of set_graph_function for more
granular control, similar to how trace events handle filters. For example,
consider the function
vfs_write(struct file *file, const char __user *buf, size_t count, loff_t *pos):
1. Enable tracing when countmeets a threshold (e.g., ≥4KB):
echo 'vfs_write if count >= 4096' > set_graph_function
2. Filter by PID and CPU:
echo 'vfs_write if pid == 3456 && cpu == 4' > set_graph_function
This usage is somewhat similar to the filter/trigger nodes in trace events,
and it targets function entry tracepoints. I'm considering whether we could
base this on trace_fprobe, as it might align more closely with the existing
filter node functionality. I'm unsure which approach is more appropriate.
Do you have any recommendations?
>
> Thank you,
>
> > In ftrace_graph_ignore_func, we could look up the corresponding trace_fprobe and
> > trace_event_file based on trace->func, then decide whether to trace
> > the function using
> > a helper like the following:
> >
> > static bool ftrace_graph_filter(struct trace_fprobe *tf, struct
> > ftrace_regs *fregs,
> > struct trace_event_file *trace_file)
> > {
> > struct fentry_trace_entry_head *entry;
> > struct trace_event_buffer fbuffer;
> > struct event_trigger_data *data;
> > int dsize;
> >
> > dsize = __get_data_size(&tf->tp, fregs, NULL);
> > entry = trace_event_buffer_reserve(&fbuffer, trace_file,
> > sizeof(*entry) + tf->tp.size + dsize);
> > if (!entry)
> > return false;
> >
> > entry = ring_buffer_event_data(fbuffer.event);
> > store_trace_args(&entry[1], &tf->tp, fregs, NULL, sizeof(*entry), dsize);
> >
> > list_for_each_entry_rcu(data, &trace_file->triggers, list) {
> > if (data->cmd_ops->trigger_type == TRIGGER_TYPE_FUNCGRAPH) {
> > struct event_filter *filter = rcu_dereference_sched(data->filter);
> > if (filter && filter_match_preds(filter, entry))
> > return true; // Allow tracing
> > }
> > }
> > return false; // Skip tracing
> > }
> >
> > Does this approach make sense? Any suggestions or concerns?
> >
> > Thanks,
> > Donglin
>
>
> --
> Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: Enhancing Conditional Filtering for Function Graph Tracer
From: Donglin Peng @ 2026-02-17 3:09 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu
Cc: linux-trace-kernel, Linux Kernel Mailing List
In-Reply-To: <20260216100935.645ea614@gandalf.local.home>
On Mon, Feb 16, 2026 at 11:09 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Sat, 14 Feb 2026 18:36:24 +0800
> Donglin Peng <dolinux.peng@gmail.com> wrote:
>
> > To simplify implementation, I propose extending a new trigger type
> > (e.g., "funcgraph").
> > In ftrace_graph_ignore_func, we could look up the corresponding trace_fprobe and
> > trace_event_file based on trace->func, then decide whether to trace
> > the function using
> > a helper like the following:
> >
> > static bool ftrace_graph_filter(struct trace_fprobe *tf, struct
> > ftrace_regs *fregs,
> > struct trace_event_file *trace_file)
> > {
> > struct fentry_trace_entry_head *entry;
> > struct trace_event_buffer fbuffer;
> > struct event_trigger_data *data;
> > int dsize;
> >
> > dsize = __get_data_size(&tf->tp, fregs, NULL);
> > entry = trace_event_buffer_reserve(&fbuffer, trace_file,
> > sizeof(*entry) + tf->tp.size + dsize);
> > if (!entry)
> > return false;
> >
> > entry = ring_buffer_event_data(fbuffer.event);
> > store_trace_args(&entry[1], &tf->tp, fregs, NULL, sizeof(*entry), dsize);
> >
> > list_for_each_entry_rcu(data, &trace_file->triggers, list) {
> > if (data->cmd_ops->trigger_type == TRIGGER_TYPE_FUNCGRAPH) {
> > struct event_filter *filter = rcu_dereference_sched(data->filter);
> > if (filter && filter_match_preds(filter, entry))
> > return true; // Allow tracing
> > }
> > }
> > return false; // Skip tracing
> > }
> >
> > Does this approach make sense? Any suggestions or concerns?
>
> My biggest concern is with performance. You want to run this against all
> functions being traced?
>
> How is this different than just using fprobes?
Thanks, my initial thought was to extend the functionality of set_graph_function
for more granular tracing control. For instance, consider the function:
vfs_write(struct file *file, const char __user *buf, size_t count, loff_t *pos):
1. Enable tracing when count exceeds a threshold (e.g., 4KB):
echo 'vfs_write if count >= 4096' > set_graph_function
2. Filter by PID and CPU:
echo 'vfs_write if pid == 3456 && cpu == 4' > set_graph_function
To implement this, we would need data structures like trace_probe,
trace_event_call, and trace_event_file. Reusing trace_fprobe might
reduce redundant implementation, however, it's unclear whether this
approach is feasible without introducing unforeseen complexities.
To minimize performance overhead, an intermediate data structure
could be introduced when writing to set_graph_function. For example:
struct funcgraph_data {
unsigned long entry_ip; // Entry address of vfs_write
struct trace_event_file *file; // Associated trace event file
};
This structure would be added to a hash table (funcgraph_hash). At the
entry point of each instrumented function, the execution flow would
proceed as follows:
ftrace_graph_func
→ function_graph_enter_regs
→ trace_graph_entry_args
→ graph_entry
→ ftrace_graph_ignore_fun
→ ftrace_graph_filter(struct ftrace_graph_ent *trace)
Within ftrace_graph_filter, the hash table funcgraph_hash is first queried
using trace->func. If a matching funcgraph_data is found,
ftrace_graph_filter is subsequently invoked with funcgraph_data->file
to apply the filter logic.
>
> -- Steve
^ 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