* [PATCH v10 00/30] Tracefs support for pKVM
From: Vincent Donnefort @ 2026-01-26 10:43 UTC (permalink / raw)
To: rostedt, mhiramat, mathieu.desnoyers, linux-trace-kernel, maz,
oliver.upton, joey.gouly, suzuki.poulose, yuzenghui
Cc: kvmarm, linux-arm-kernel, jstultz, qperret, will, aneesh.kumar,
kernel-team, linux-kernel, Vincent Donnefort
The growing set of features supported by the hypervisor in protected
mode necessitates debugging and profiling tools. Tracefs is the
ideal candidate for this task:
* It is simple to use and to script.
* It is supported by various tools, from the trace-cmd CLI to the
Android web-based perfetto.
* The ring-buffer, where are stored trace events consists of linked
pages, making it an ideal structure for sharing between kernel and
hypervisor.
This series first introduces a new generic way of creating remote events and
remote buffers. Then it adds support to the pKVM hypervisor.
1. ring-buffer
--------------
To setup the per-cpu ring-buffers, a new interface is created:
ring_buffer_remote: Describes what the kernel needs to know about the
remote writer, that is, the set of pages forming the
ring-buffer and a callback for the reader/head
swapping (enables consuming read)
ring_buffer_remote(): Creates a read-only ring-buffer from a
ring_buffer_remote.
To keep the internals of `struct ring_buffer` in sync with the remote,
the meta-page is used. It was originally introduced to enable user-space
mapping of the ring-buffer [1]. In this case, the kernel is not the
producer anymore but the reader. The function to read that meta-page is:
ring_buffer_poll_remote():
Update `struct ring_buffer` based on the remote
meta-page. Wake-up readers if necessary.
The kernel has to poll the meta-page to be notified of newly written
events.
2. Tracefs
----------
This series introduce a new trace_remote that does the link between
tracefs and the remote ring-buffer.
The interface is found in the remotes/ directory at the root of the
tracefs mount point. Each remote is like an instance and you'll find
there a subset of the regular Tracefs user-space interface:
remotes/test
|-- buffer_size_kb
|-- events
| |-- enable
| |-- header_event
| |-- header_page
| `-- test
| `-- selftest
| |-- enable
| |-- format
| `-- id
|-- per_cpu
| `-- cpu0
| |-- trace
| `-- trace_pipe
|-- trace
|-- trace_pipe
|-- tracing_on
Behind the scenes, kernel/trace/trace_remote.c creates this tracefs
hierarchy without relying on kernel/trace/trace.c. This is due to
fundamental differences:
* Remote tracing doesn't support trace_array's system-specific
features (snapshots, tracers, etc.).
* Logged event formats differ (e.g., no PID for remote events).
* Buffer operations require specific remote interactions.
3. Simple Ring-Buffer
---------------------
As the current ring-buffer.c implementation has too many dependencies to
be used directly by the pKVM hypervisor. A new simple implementation is
created and can be found in kernel/trace/simple-ring-buffer.c.
This implementation is write-only and is used by both the pKVM
hypervisor and a trace_remote test module.
4. Events
---------
A new REMOTE_EVENT() macro is added to simplify the creation of events
on the kernel side. As remote tracing buffer are read only, only the
event structure and a way of printing must be declared. The prototype of
the macro is very similar to the well-known TRACE_EVENT()
REMOTE_EVENT(my_event, id,
RE_STRUCT(
re_field(u64, foobar)
),
RE_PRINTK("foobar=%lld", __entry->foobar)
)
)
5. pKVM
-------
The pKVM support simply creates a "hypervisor" trace_remote on the
kernel side and inherits from simple-ring-buffer.c on the hypervisor
side.
A new event macro is created HYP_EVENT() that is under the hood re-using
REMOTE_EVENT() (defined in the previous paragaph) as well as generate
hypervisor specific struct and trace_<event>() functions.
5. Limitations:
---------------
Non-consuming reading of the buffer isn't supported (i.e. cat trace ->
-EPERM) due to current the lack of support in the ring-buffer meta-page.
[1] https://tracingsummit.org/ts/2022/hypervisortracing/
[2] https://lore.kernel.org/all/20240510140435.3550353-1-vdonnefort@google.com/
changes since v9
- Add vCPU PID to hyp_enter/hyp_exit (Marc)
- Remove useless X1 setting for tracing HVCs (Marc)
- Fix REMOTE_PRINTK_COUNT_ARGS()
- Rebase on 6.19-rc7
Changes since v8
- Do not enable tracing if unstable cnvct (Marc)
- Add support for nVHE (Marc)
- Add PKVM_DISABLE_STAGE2_ON_PANIC (Marc)
- NVHE_EL2_TRACING depends on NVHE_EL2_DEBUG (Marc)
- Add a reason for hyp_enter/hyp_exit events (Marc)
- Remove PKVM_SELFTESTS in favour of NVHE_EL2_DEBUG
- Add wrapper for arm_smccc_1_2, now used in nvhe/ffa.c
Changes since v7
- Add missing EXPORT_SYMBOL_GPL for remote_test.ko
- Rebase on 6.18-rc4
Changes since v6
- Add requires field to the selftest (Masami)
- Use guard() for ring_buffer_poll_remote (Steven)
- Rename ring_buffer_remote() to ring_buffer_alloc_remote() (Steven)
- kerneldoc for trace_buffer_remote and simple_ring_buffer (Steven)
- Validate trace_buffer_desc size in trace_remote_alloc_buffer
(Steven)
- Add non-consuming ring-buffer read (Steven)
- Add spinning failsafe in simple_ring_buffer (Steven)
- Range check for hyp_trace_desc::bpages_backing_* in hyp_trace_desc_validate()
- unsigned int cpu in hyp_trace_desc_validate()
- Fix event/format file
- Add tests with an offline CPU
- Add tests for non-consuming read
- Add documentation
- Rebase on 6.17
Changes since v5 (https://lore.kernel.org/all/20250516134031.661124-1-vdonnefort@google.com/)
- Add tishift lib to the hyp (Aneesh)
- Rebase on 6.17-rc2
Changes since v4 (https://lore.kernel.org/all/20250506164820.515876-1-vdonnefort@google.com/)
- Extend meta-page with pages_touched and pages_lost
- Create ring_buffer_types.h
- Fix simple_ring_buffer build for 32-bits arch and x86
- Try unload buffer on reset (+ test)
- Minor renaming and comments
Changes since v3 (https://lore.kernel.org/all/20250224121353.98697-1-vdonnefort@google.com/)
- Move tracefs support from kvm/hyp_trace.c into a generic trace_remote.c.
- Move ring-buffer implementation from nvhe/trace.c into a generic
simple-ring-buffer.c
- Rebase on 6.15-rc4.
Changes since v2 (https://lore.kernel.org/all/20250108114536.627715-1-vdonnefort@google.com/)
- Fix ring-buffer remote reset
- Fix fast-forward in rb_page_desc()
- Refactor nvhe/trace.c
- struct hyp_buffer_page more compact
- Add a struct_len to trace_page_desc
- Extend reset testing
- Rebase on 6.14-rc3
Changes since v1 (https://lore.kernel.org/all/20240911093029.3279154-1-vdonnefort@google.com/)
- Add 128-bits mult fallback in the unlikely event of an overflow. (John)
- Fix ELF section sort.
- __always_inline trace_* event macros.
- Fix events/<event>/enable permissions.
- Rename ring-buffer "writer" to "remote".
- Rename CONFIG_PROTECTED_NVHE_TESTING to PKVM_SELFTEST to align with
Quentin's upcoming selftest
- Rebase on 6.13-rc3.
Changes since RFC (https://lore.kernel.org/all/20240805173234.3542917-1-vdonnefort@google.com/)
- hypervisor trace clock:
- mult/shift computed in hyp_trace.c. (John)
- Update clock when it deviates from kernel boot clock. (John)
- Add trace_clock file.
- Separate patch for better readability.
- Add a proper reset interface which does not need to teardown the
tracing buffers. (Steven)
- Return -EPERM on trace access. (Steven)
- Add per-cpu trace file.
- Automatically teardown and free the tracing buffer when it is empty,
without readers and not currently tracing.
- Show in buffer_size_kb if the buffer is loaded in the hypervisor or
not.
- Extend tests to cover reset and unload.
- CC timekeeping folks on relevant patches (Marc)
Vincent Donnefort (30):
ring-buffer: Add page statistics to the meta-page
ring-buffer: Store bpage pointers into subbuf_ids
ring-buffer: Introduce ring-buffer remotes
ring-buffer: Add non-consuming read for ring-buffer remotes
tracing: Introduce trace remotes
tracing: Add reset to trace remotes
tracing: Add non-consuming read to trace remotes
tracing: Add init callback to trace remotes
tracing: Add events to trace remotes
tracing: Add events/ root files to trace remotes
tracing: Add helpers to create trace remote events
ring-buffer: Export buffer_data_page and macros
tracing: Introduce simple_ring_buffer
tracing: Add a trace remote module for testing
tracing: selftests: Add trace remote tests
Documentation: tracing: Add tracing remotes
tracing: load/unload page callbacks for simple_ring_buffer
tracing: Check for undefined symbols in simple_ring_buffer
KVM: arm64: Add PKVM_DISABLE_STAGE2_ON_PANIC
KVM: arm64: Add clock support to nVHE/pKVM hyp
KVM: arm64: Initialise hyp_nr_cpus for nVHE hyp
KVM: arm64: Support unaligned fixmap in the pKVM hyp
KVM: arm64: Add tracing capability for the nVHE/pKVM hyp
KVM: arm64: Add trace remote for the nVHE/pKVM hyp
KVM: arm64: Sync boot clock with the nVHE/pKVM hyp
KVM: arm64: Add trace reset to the nVHE/pKVM hyp
KVM: arm64: Add event support to the nVHE/pKVM hyp and trace remote
KVM: arm64: Add hyp_enter/hyp_exit events to nVHE/pKVM hyp
KVM: arm64: Add selftest event support to nVHE/pKVM hyp
tracing: selftests: Add hypervisor trace remote tests
Documentation/trace/index.rst | 11 +
Documentation/trace/remotes.rst | 59 +
arch/arm64/include/asm/kvm_asm.h | 8 +
arch/arm64/include/asm/kvm_define_hypevents.h | 16 +
arch/arm64/include/asm/kvm_host.h | 3 +
arch/arm64/include/asm/kvm_hyp.h | 4 +-
arch/arm64/include/asm/kvm_hypevents.h | 60 +
arch/arm64/include/asm/kvm_hyptrace.h | 26 +
arch/arm64/kernel/image-vars.h | 4 +
arch/arm64/kernel/vmlinux.lds.S | 18 +
arch/arm64/kvm/Kconfig | 64 +-
arch/arm64/kvm/Makefile | 2 +
arch/arm64/kvm/arm.c | 12 +-
arch/arm64/kvm/handle_exit.c | 2 +-
arch/arm64/kvm/hyp/include/nvhe/arm-smccc.h | 23 +
arch/arm64/kvm/hyp/include/nvhe/clock.h | 16 +
.../kvm/hyp/include/nvhe/define_events.h | 14 +
arch/arm64/kvm/hyp/include/nvhe/mem_protect.h | 2 -
arch/arm64/kvm/hyp/include/nvhe/trace.h | 70 +
arch/arm64/kvm/hyp/nvhe/Makefile | 6 +-
arch/arm64/kvm/hyp/nvhe/clock.c | 65 +
arch/arm64/kvm/hyp/nvhe/events.c | 25 +
arch/arm64/kvm/hyp/nvhe/ffa.c | 28 +-
arch/arm64/kvm/hyp/nvhe/host.S | 2 +-
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 87 +-
arch/arm64/kvm/hyp/nvhe/hyp.lds.S | 6 +
arch/arm64/kvm/hyp/nvhe/mm.c | 4 +-
arch/arm64/kvm/hyp/nvhe/psci-relay.c | 7 +-
arch/arm64/kvm/hyp/nvhe/setup.c | 4 +-
arch/arm64/kvm/hyp/nvhe/stacktrace.c | 6 +-
arch/arm64/kvm/hyp/nvhe/switch.c | 5 +-
arch/arm64/kvm/hyp/nvhe/trace.c | 306 ++++
arch/arm64/kvm/hyp_trace.c | 443 ++++++
arch/arm64/kvm/hyp_trace.h | 11 +
arch/arm64/kvm/stacktrace.c | 8 +-
fs/tracefs/inode.c | 1 +
include/linux/ring_buffer.h | 58 +
include/linux/ring_buffer_types.h | 41 +
include/linux/simple_ring_buffer.h | 120 ++
include/linux/trace_remote.h | 92 ++
include/linux/trace_remote_event.h | 33 +
include/trace/define_remote_events.h | 73 +
include/uapi/linux/trace_mmap.h | 8 +-
kernel/trace/Kconfig | 14 +
kernel/trace/Makefile | 20 +
kernel/trace/remote_test.c | 259 ++++
kernel/trace/remote_test_events.h | 10 +
kernel/trace/ring_buffer.c | 356 ++++-
kernel/trace/simple_ring_buffer.c | 468 ++++++
kernel/trace/trace.c | 2 +-
kernel/trace/trace.h | 6 +
kernel/trace/trace_remote.c | 1318 +++++++++++++++++
.../ftrace/test.d/remotes/buffer_size.tc | 25 +
.../selftests/ftrace/test.d/remotes/functions | 88 ++
.../test.d/remotes/hypervisor/buffer_size.tc | 11 +
.../ftrace/test.d/remotes/hypervisor/reset.tc | 11 +
.../ftrace/test.d/remotes/hypervisor/trace.tc | 11 +
.../test.d/remotes/hypervisor/trace_pipe.tc | 11 +
.../test.d/remotes/hypervisor/unloading.tc | 11 +
.../selftests/ftrace/test.d/remotes/reset.tc | 90 ++
.../selftests/ftrace/test.d/remotes/trace.tc | 127 ++
.../ftrace/test.d/remotes/trace_pipe.tc | 127 ++
.../ftrace/test.d/remotes/unloading.tc | 41 +
63 files changed, 4740 insertions(+), 119 deletions(-)
create mode 100644 Documentation/trace/remotes.rst
create mode 100644 arch/arm64/include/asm/kvm_define_hypevents.h
create mode 100644 arch/arm64/include/asm/kvm_hypevents.h
create mode 100644 arch/arm64/include/asm/kvm_hyptrace.h
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/arm-smccc.h
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/clock.h
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/define_events.h
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/trace.h
create mode 100644 arch/arm64/kvm/hyp/nvhe/clock.c
create mode 100644 arch/arm64/kvm/hyp/nvhe/events.c
create mode 100644 arch/arm64/kvm/hyp/nvhe/trace.c
create mode 100644 arch/arm64/kvm/hyp_trace.c
create mode 100644 arch/arm64/kvm/hyp_trace.h
create mode 100644 include/linux/ring_buffer_types.h
create mode 100644 include/linux/simple_ring_buffer.h
create mode 100644 include/linux/trace_remote.h
create mode 100644 include/linux/trace_remote_event.h
create mode 100644 include/trace/define_remote_events.h
create mode 100644 kernel/trace/remote_test.c
create mode 100644 kernel/trace/remote_test_events.h
create mode 100644 kernel/trace/simple_ring_buffer.c
create mode 100644 kernel/trace/trace_remote.c
create mode 100644 tools/testing/selftests/ftrace/test.d/remotes/buffer_size.tc
create mode 100644 tools/testing/selftests/ftrace/test.d/remotes/functions
create mode 100644 tools/testing/selftests/ftrace/test.d/remotes/hypervisor/buffer_size.tc
create mode 100644 tools/testing/selftests/ftrace/test.d/remotes/hypervisor/reset.tc
create mode 100644 tools/testing/selftests/ftrace/test.d/remotes/hypervisor/trace.tc
create mode 100644 tools/testing/selftests/ftrace/test.d/remotes/hypervisor/trace_pipe.tc
create mode 100644 tools/testing/selftests/ftrace/test.d/remotes/hypervisor/unloading.tc
create mode 100644 tools/testing/selftests/ftrace/test.d/remotes/reset.tc
create mode 100644 tools/testing/selftests/ftrace/test.d/remotes/trace.tc
create mode 100644 tools/testing/selftests/ftrace/test.d/remotes/trace_pipe.tc
create mode 100644 tools/testing/selftests/ftrace/test.d/remotes/unloading.tc
base-commit: 63804fed149a6750ffd28610c5c1c98cce6bd377
--
2.52.0.457.g6b5491de43-goog
^ permalink raw reply
* Re: [PATCHv6 bpf-next 0/9] ftrace,bpf: Use single direct ops for bpf trampolines
From: Jiri Olsa @ 2026-01-26 9:48 UTC (permalink / raw)
To: Steven Rostedt
Cc: Florent Revest, Andrii Nakryiko, Mark Rutland, bpf, linux-kernel,
linux-trace-kernel, linux-arm-kernel, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Menglong Dong, Song Liu
In-Reply-To: <CAEf4BzaRU0DoM9hHtW5Bm7njodfg06JzTOe=ABAqZ6iMjAt4iQ@mail.gmail.com>
hi,
gentle ping, thanks
jirka
On Thu, Jan 15, 2026 at 10:54:09AM -0800, Andrii Nakryiko wrote:
> On Tue, Dec 30, 2025 at 6:50 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > hi,
> > while poking the multi-tracing interface I ended up with just one ftrace_ops
> > object to attach all trampolines.
> >
> > This change allows to use less direct API calls during the attachment changes
> > in the future code, so in effect speeding up the attachment.
> >
> > In current code we get a speed up from using just a single ftrace_ops object.
> >
> > - with current code:
> >
> > Performance counter stats for 'bpftrace -e fentry:vmlinux:ksys_* {} -c true':
> >
> > 6,364,157,902 cycles:k
> > 828,728,902 cycles:u
> > 1,064,803,824 instructions:u # 1.28 insn per cycle
> > 23,797,500,067 instructions:k # 3.74 insn per cycle
> >
> > 4.416004987 seconds time elapsed
> >
> > 0.164121000 seconds user
> > 1.289550000 seconds sys
> >
> >
> > - with the fix:
> >
> > Performance counter stats for 'bpftrace -e fentry:vmlinux:ksys_* {} -c true':
> >
> > 6,535,857,905 cycles:k
> > 810,809,429 cycles:u
> > 1,064,594,027 instructions:u # 1.31 insn per cycle
> > 23,962,552,894 instructions:k # 3.67 insn per cycle
> >
> > 1.666961239 seconds time elapsed
> >
> > 0.157412000 seconds user
> > 1.283396000 seconds sys
> >
> >
> >
> > The speedup seems to be related to the fact that with single ftrace_ops object
> > we don't call ftrace_shutdown anymore (we use ftrace_update_ops instead) and
> > we skip the synchronize rcu calls (each ~100ms) at the end of that function.
> >
> > rfc: https://lore.kernel.org/bpf/20250729102813.1531457-1-jolsa@kernel.org/
> > v1: https://lore.kernel.org/bpf/20250923215147.1571952-1-jolsa@kernel.org/
> > v2: https://lore.kernel.org/bpf/20251113123750.2507435-1-jolsa@kernel.org/
> > v3: https://lore.kernel.org/bpf/20251120212402.466524-1-jolsa@kernel.org/
> > v4: https://lore.kernel.org/bpf/20251203082402.78816-1-jolsa@kernel.org/
> > v5: https://lore.kernel.org/bpf/20251215211402.353056-10-jolsa@kernel.org/
> >
> > v6 changes:
> > - rename add_hash_entry_direct to add_ftrace_hash_entry_direct [Steven]
> > - factor hash_add/hash_sub [Steven]
> > - add kerneldoc header for update_ftrace_direct_* functions [Steven]
> > - few assorted smaller fixes [Steven]
> > - added missing direct_ops wrappers for !CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> > case [Steven]
> >
>
> So this looks good from BPF side, I think. Steven, if you don't mind
> giving this patch set another look and if everything is to your liking
> giving your ack, we can then apply it to bpf-next. Thanks!
>
> > v5 changes:
> > - do not export ftrace_hash object [Steven]
> > - fix update_ftrace_direct_add new_filter_hash leak [ci]
> >
> > v4 changes:
> > - rebased on top of bpf-next/master (with jmp attach changes)
> > added patch 1 to deal with that
> > - added extra checks for update_ftrace_direct_del/mod to address
> > the ci bot review
> >
> > v3 changes:
> > - rebased on top of bpf-next/master
> > - fixed update_ftrace_direct_del cleanup path
> > - added missing inline to update_ftrace_direct_* stubs
> >
> > v2 changes:
> > - rebased on top fo bpf-next/master plus Song's livepatch fixes [1]
> > - renamed the API functions [2] [Steven]
> > - do not export the new api [Steven]
> > - kept the original direct interface:
> >
> > I'm not sure if we want to melt both *_ftrace_direct and the new interface
> > into single one. It's bit different in semantic (hence the name change as
> > Steven suggested [2]) and I don't think the changes are not that big so
> > we could easily keep both APIs.
> >
> > v1 changes:
> > - make the change x86 specific, after discussing with Mark options for
> > arm64 [Mark]
> >
> > thanks,
> > jirka
> >
> >
> > [1] https://lore.kernel.org/bpf/20251027175023.1521602-1-song@kernel.org/
> > [2] https://lore.kernel.org/bpf/20250924050415.4aefcb91@batman.local.home/
> > ---
> > Jiri Olsa (9):
> > ftrace,bpf: Remove FTRACE_OPS_FL_JMP ftrace_ops flag
> > ftrace: Make alloc_and_copy_ftrace_hash direct friendly
> > ftrace: Export some of hash related functions
> > ftrace: Add update_ftrace_direct_add function
> > ftrace: Add update_ftrace_direct_del function
> > ftrace: Add update_ftrace_direct_mod function
> > bpf: Add trampoline ip hash table
> > ftrace: Factor ftrace_ops ops_func interface
> > bpf,x86: Use single ftrace_ops for direct calls
> >
> > arch/x86/Kconfig | 1 +
> > include/linux/bpf.h | 7 ++-
> > include/linux/ftrace.h | 31 +++++++++-
> > kernel/bpf/trampoline.c | 259 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------
> > kernel/trace/Kconfig | 3 +
> > kernel/trace/ftrace.c | 406 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------
> > 6 files changed, 632 insertions(+), 75 deletions(-)
> >
^ permalink raw reply
* Re: [PATCH] tracing: kprobe-event: Return directly when dyn_event_list is empty
From: Steven Rostedt @ 2026-01-26 4:21 UTC (permalink / raw)
To: sunliming
Cc: mhiramat, mathieu.desnoyers, linux-kernel, linux-trace-kernel,
sunliming
In-Reply-To: <01b1d5a3-8be0-4d9a-45d4-4da7c3edf746@linux.dev>
On Mon, 26 Jan 2026 10:56:59 +0800
sunliming <sunliming@linux.dev> wrote:
> During kernel boot, the setup_boot_kprobe_events() function causes
> significant delays, increasing overall startup time.
>
> The root cause is a lock contention chain: its child function
>
> enable_boot_kprobe_events() requires the event_mutex, which is already
>
> held by early_event_add_tracer(). early_event_add_tracer() itself is
> blocked
Ah, early_event_add_tracer() is called by the work function that sets
up tracers (and the event directories within them).
>
> waiting for the trace_event_sem read-write lock, which is held for an
> extended
>
> period by trace_event_update_all(). trace_event_update_all() is
> runing in
>
> eval_map_work_func() in eval_map_wq.
>
> On my ARM64 platform machine, the latency can reach around 200ms：
>
> [ 0.268848] trace_kprobe: try_get_event_mutex
> kernel/trace/tracekprobe.c,1902,enable_boot_kprobe_events
> [ 0.268849] try down_write
> kernel/trace/traceevents.c,4114,early_event_add_tracer
> [ 0.482382] done up_write
> kernel/trace/trace_events.c,3074,trace_event_eval_update
> [ 0.482386] done down_write
> kernel/trace/trace_events.c,4116,early_event_add_tracer
> [ 0.482868] done up_write
> kernel/trace/trace_events.c,4119,early_event_add_tracer
> [ 0.482877] trace_kprobe: enable_boot_kprobe_events geted eventmutex
> [ 0.482879] trace_kprobe: enable_bootk_probe_events release eventmutex
I take it that you are aware of this work, as it comes from someone
with the same email domain as you have.
https://lore.kernel.org/all/20260122032051.386331-1-tianyaxiong@kylinos.cn/
That is moving the kprobe setup into the same work function.
-- Steve
^ permalink raw reply
* Re: [PATCH 1/3] tracing: Export eval_map_wq for asynchronous use by other modules
From: Yaxiong Tian @ 2026-01-26 2:45 UTC (permalink / raw)
To: Steven Rostedt
Cc: axboe, mhiramat, mathieu.desnoyers, linux-block, linux-kernel,
linux-trace-kernel
In-Reply-To: <20260123153247.004688f6@gandalf.local.home>
在 2026/1/24 04:32, Steven Rostedt 写道:
> On Thu, 22 Jan 2026 11:24:15 +0800
> Yaxiong Tian <tianyaxiong@kylinos.cn> wrote:
>
>> The eval_map_work_func() function, though queued in eval_map_wq,
>> holds the trace_event_sem read-write lock for a long time during
>> kernel boot. This causes blocking issues for other functions.
>>
>> Making eval_map_wq extern allows other modules to schedule their
>> work asynchronously on this queue, preventing it from blocking
>> the main boot thread.
> If you are going to make this wq generic for other parts of the tracing
> code, then let's rename it. As "eval_map" is specific to the enum mappings
> being updated (which I hope someday I can get that update working at build
> time. And when I do, the name will be totally meaningless).
>
> Let's rename it to "trace_init_wq" instead.
>
> -- Steve
Thank you for your suggestion. I have renamed it to trace_init_wq and
released the V2 version.
^ permalink raw reply
* [PATCH v2 3/3] blktrace: Make init_blk_tracer() asynchronous
From: Yaxiong Tian @ 2026-01-26 2:43 UTC (permalink / raw)
To: rostedt, axboe, mhiramat, mathieu.desnoyers
Cc: linux-block, linux-kernel, linux-trace-kernel, Yaxiong Tian
In-Reply-To: <20260126024033.290136-1-tianyaxiong@kylinos.cn>
The init_blk_tracer() function causes significant boot delay as it
waits for the trace_event_sem lock held by trace_event_update_all().
Specifically, its child function register_trace_event() requires
this lock, which is occupied for an extended period during boot.
To mitigate this contention, we have moved init_blk_tracer() to the
trace_init_wq workqueue, allowing it to execute asynchronously and
prevent blocking the main boot thread.
Signed-off-by: Yaxiong Tian <tianyaxiong@kylinos.cn>
---
kernel/trace/blktrace.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index d031c8d80be4..d611cd1f02ef 100644
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -1832,7 +1832,9 @@ static struct trace_event trace_blk_event = {
.funcs = &trace_blk_event_funcs,
};
-static int __init init_blk_tracer(void)
+static struct work_struct blktrace_works __initdata;
+
+static int __init __init_blk_tracer(void)
{
if (!register_trace_event(&trace_blk_event)) {
pr_warn("Warning: could not register block events\n");
@@ -1852,6 +1854,25 @@ static int __init init_blk_tracer(void)
return 0;
}
+static void __init blktrace_works_func(struct work_struct *work)
+{
+ __init_blk_tracer();
+}
+
+static int __init init_blk_tracer(void)
+{
+ int ret = 0;
+
+ if (trace_init_wq) {
+ INIT_WORK(&blktrace_works, blktrace_works_func);
+ queue_work(trace_init_wq, &blktrace_works);
+ } else {
+ ret = __init_blk_tracer();
+ }
+
+ return ret;
+}
+
device_initcall(init_blk_tracer);
static int blk_trace_remove_queue(struct request_queue *q)
--
2.25.1
^ permalink raw reply related
* [PATCH v2 2/3] tracing/kprobes: Make setup_boot_kprobe_events() asynchronous
From: Yaxiong Tian @ 2026-01-26 2:43 UTC (permalink / raw)
To: rostedt, axboe, mhiramat, mathieu.desnoyers
Cc: linux-block, linux-kernel, linux-trace-kernel, Yaxiong Tian
In-Reply-To: <20260126024033.290136-1-tianyaxiong@kylinos.cn>
During kernel boot, the setup_boot_kprobe_events() function causes
significant delays, increasing overall startup time.
The root cause is a lock contention chain: its child function
enable_boot_kprobe_events() requires the event_mutex, which is
already held by early_event_add_tracer(). early_event_add_tracer()
itself is blocked waiting for the trace_event_sem read-write lock,
which is held for an extended period by trace_event_update_all().
To resolve this, we have moved the execution of
setup_boot_kprobe_events() to the trace_init_wq workqueue, allowing
it to run asynchronously.
Signed-off-by: Yaxiong Tian <tianyaxiong@kylinos.cn>
---
kernel/trace/trace_kprobe.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 9953506370a5..4c6621f02696 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -2031,6 +2031,13 @@ static __init int init_kprobe_trace_early(void)
}
core_initcall(init_kprobe_trace_early);
+static struct work_struct kprobe_trace_work __initdata;
+
+static void __init kprobe_trace_works_func(struct work_struct *work)
+{
+ setup_boot_kprobe_events();
+}
+
/* Make a tracefs interface for controlling probe points */
static __init int init_kprobe_trace(void)
{
@@ -2048,7 +2055,12 @@ static __init int init_kprobe_trace(void)
trace_create_file("kprobe_profile", TRACE_MODE_READ,
NULL, NULL, &kprobe_profile_ops);
- setup_boot_kprobe_events();
+ if (trace_init_wq) {
+ INIT_WORK(&kprobe_trace_work, kprobe_trace_works_func);
+ queue_work(trace_init_wq, &kprobe_trace_work);
+ } else {
+ setup_boot_kprobe_events();
+ }
return 0;
}
--
2.25.1
^ permalink raw reply related
* [PATCH v2 1/3] tracing: Rename `eval_map_wq` and export it for asynchronous use by other modules
From: Yaxiong Tian @ 2026-01-26 2:43 UTC (permalink / raw)
To: rostedt, axboe, mhiramat, mathieu.desnoyers
Cc: linux-block, linux-kernel, linux-trace-kernel, Yaxiong Tian
In-Reply-To: <20260126024033.290136-1-tianyaxiong@kylinos.cn>
The eval_map_work_func() function, though queued in eval_map_wq,
holds the trace_event_sem read-write lock for a long time during
kernel boot. This causes blocking issues for other functions.
Rename eval_map_wq to trace_init_wq and export it, thereby allowing
other modules to schedule work on this queue asynchronously and
avoiding blockage of the main boot thread.
Signed-off-by: Yaxiong Tian <tianyaxiong@kylinos.cn>
---
kernel/trace/trace.c | 18 +++++++++---------
kernel/trace/trace.h | 2 ++
2 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index e18005807395..c61e30cb7339 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -10774,7 +10774,7 @@ int tracing_init_dentry(void)
extern struct trace_eval_map *__start_ftrace_eval_maps[];
extern struct trace_eval_map *__stop_ftrace_eval_maps[];
-static struct workqueue_struct *eval_map_wq __initdata;
+struct workqueue_struct *trace_init_wq __initdata;
static struct work_struct eval_map_work __initdata;
static struct work_struct tracerfs_init_work __initdata;
@@ -10790,15 +10790,15 @@ static int __init trace_eval_init(void)
{
INIT_WORK(&eval_map_work, eval_map_work_func);
- eval_map_wq = alloc_workqueue("eval_map_wq", WQ_UNBOUND, 0);
- if (!eval_map_wq) {
- pr_err("Unable to allocate eval_map_wq\n");
+ trace_init_wq = alloc_workqueue("trace_init_wq", WQ_UNBOUND, 0);
+ if (!trace_init_wq) {
+ pr_err("Unable to allocate trace_init_wq\n");
/* Do work here */
eval_map_work_func(&eval_map_work);
return -ENOMEM;
}
- queue_work(eval_map_wq, &eval_map_work);
+ queue_work(trace_init_wq, &eval_map_work);
return 0;
}
@@ -10807,8 +10807,8 @@ subsys_initcall(trace_eval_init);
static int __init trace_eval_sync(void)
{
/* Make sure the eval map updates are finished */
- if (eval_map_wq)
- destroy_workqueue(eval_map_wq);
+ if (trace_init_wq)
+ destroy_workqueue(trace_init_wq);
return 0;
}
@@ -10969,9 +10969,9 @@ static __init int tracer_init_tracefs(void)
if (ret)
return 0;
- if (eval_map_wq) {
+ if (trace_init_wq) {
INIT_WORK(&tracerfs_init_work, tracer_init_tracefs_work_func);
- queue_work(eval_map_wq, &tracerfs_init_work);
+ queue_work(trace_init_wq, &tracerfs_init_work);
} else {
tracer_init_tracefs_work_func(NULL);
}
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index de4e6713b84e..e52f259f8945 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -770,6 +770,8 @@ extern unsigned long nsecs_to_usecs(unsigned long nsecs);
extern unsigned long tracing_thresh;
+extern struct workqueue_struct *trace_init_wq __initdata;
+
/* PID filtering */
bool trace_find_filtered_pid(struct trace_pid_list *filtered_pids,
--
2.25.1
^ permalink raw reply related
* [PATCH v2 0/3] Tracing: Accelerate Kernel Boot by Asynchronizing
From: Yaxiong Tian @ 2026-01-26 2:40 UTC (permalink / raw)
To: rostedt, axboe, mhiramat, mathieu.desnoyers
Cc: linux-block, linux-kernel, linux-trace-kernel, Yaxiong Tian
On my ARM64 platform, I observed that certain tracing module
initializations run for up to 200ms—for example, init_kprobe_trace().
Analysis reveals the root cause: the execution flow eval_map_work_func()
→trace_event_update_with_eval_map()→trace_event_update_all()
is highly time-consuming. Although this flow is placed in eval_map_wq
for asynchronous execution, it holds the trace_event_sem lock, causing
other modules to be blocked either directly or indirectly.
To resolve this issue, I rename `eval_map_wq` and export it and moved
other initialization functions under the tracing subsystem that are
related to this lock to run asynchronously on this workqueue. After this
optimization, boot time is reduced by approximately 200ms.
---
Changes in v2:
- Rename eval_map_wq to trace_init_wq.
Yaxiong Tian (3):
tracing: Rename `eval_map_wq` and export it for asynchronous use by
other modules
tracing/kprobes: Make setup_boot_kprobe_events() asynchronous
blktrace: Make init_blk_tracer() asynchronous
kernel/trace/blktrace.c | 23 ++++++++++++++++++++++-
kernel/trace/trace.c | 18 +++++++++---------
kernel/trace/trace.h | 2 ++
kernel/trace/trace_kprobe.c | 14 +++++++++++++-
4 files changed, 46 insertions(+), 11 deletions(-)
--
2.25.1
^ permalink raw reply
* Re: [PATCH v5 v5 1/3] docs: tracing/fprobe: Document list filters and :entry/:exit
From: Seokwoo Chung @ 2026-01-25 20:23 UTC (permalink / raw)
To: Steven Rostedt
Cc: mhiramat, corbet, shuah, mathieu.desnoyers, linux-kernel,
linux-trace-kernel, linux-doc, linux-kselftest
In-Reply-To: <20260120155340.2dac7fab@gandalf.local.home>
On Tue, Jan 20, 2026 at 03:53:40PM -0500, Steven Rostedt wrote:
> On Sat, 17 Jan 2026 20:18:13 -0500
> "Seokwoo Chung (Ryan)" <seokwoo.chung130@gmail.com> wrote:
>
> > Update fprobe event documentation to describe comma-separated symbol lists,
> > exclusions, and explicit suffixes.
>
> Usually, the documentation updates comes *after* the changes.
>
> -- Steve
>
Thanks for the comment. As you noticed, I sent another patch since I forgot to
put log. That said, please let me know if you want me to continue on this
series. My apologies on the ordering. I noted after Masami mentioned but I
misordered when sending. Thank you.
Best regards,
Ryan Chung
> >
> > Signed-off-by: Seokwoo Chung (Ryan) <seokwoo.chung130@gmail.com>
> > ---
> > Documentation/trace/fprobetrace.rst | 17 ++++++++++++++---
> > 1 file changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/trace/fprobetrace.rst b/Documentation/trace/fprobetrace.rst
> > index b4c2ca3d02c1..bbcfd57f0005 100644
> > --- a/Documentation/trace/fprobetrace.rst
> > +++ b/Documentation/trace/fprobetrace.rst
> > @@ -25,14 +25,18 @@ Synopsis of fprobe-events
> > -------------------------
> > ::
> >
> > - f[:[GRP1/][EVENT1]] SYM [FETCHARGS] : Probe on function entry
> > - f[MAXACTIVE][:[GRP1/][EVENT1]] SYM%return [FETCHARGS] : Probe on function exit
> > + f[:[GRP1/][EVENT1]] SYM[%return] [FETCHARGS] : Single function
> > + f[:[GRP1/][EVENT1]] SYM[,[!]SYM[,...]][:entry|:exit] [FETCHARGS] :Multiple
> > + function
> > t[:[GRP2/][EVENT2]] TRACEPOINT [FETCHARGS] : Probe on tracepoint
> >
> > GRP1 : Group name for fprobe. If omitted, use "fprobes" for it.
> > GRP2 : Group name for tprobe. If omitted, use "tracepoints" for it.
> > EVENT1 : Event name for fprobe. If omitted, the event name is
> > - "SYM__entry" or "SYM__exit".
> > + - For a single literal symbol, the event name is
> > + "SYM__entry" or "SYM__exit".
> > + - For a *list or any wildcard*, an explicit [GRP1/][EVENT1] is
> > + required; otherwise the parser rejects it.
> > EVENT2 : Event name for tprobe. If omitted, the event name is
> > the same as "TRACEPOINT", but if the "TRACEPOINT" starts
> > with a digit character, "_TRACEPOINT" is used.
> > @@ -40,6 +44,13 @@ Synopsis of fprobe-events
> > can be probed simultaneously, or 0 for the default value
> > as defined in Documentation/trace/fprobe.rst
> >
> > + SYM : Function name or comma-separated list of symbols.
> > + - SYM prefixed with "!" are exclusions.
> > + - ":entry" suffix means it probes entry of given symbols
> > + (default)
> > + - ":exit" suffix means it probes exit of given symbols.
> > + - "%return" suffix means it probes exit of SYM (single
> > + symbol).
> > FETCHARGS : Arguments. Each probe can have up to 128 args.
> > ARG : Fetch "ARG" function argument using BTF (only for function
> > entry or tracepoint.) (\*1)
>
^ permalink raw reply
* Re: [PATCH] tracing: Remove notrace from trace_event_raw_event_synth()
From: Tom Zanussi @ 2026-01-25 19:24 UTC (permalink / raw)
To: Steven Rostedt, LKML, Linux Trace Kernel
Cc: Masami Hiramatsu, Mathieu Desnoyers
In-Reply-To: <20260122204526.068a98c9@gandalf.local.home>
Hi Steve,
On Thu, 2026-01-22 at 20:45 -0500, Steven Rostedt wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
>
> When debugging the synthetic events, being able to function trace its
> functions is very useful (now that CONFIG_FUNCTION_SELF_TRACING is
> available). For some reason trace_event_raw_event_synth() was marked as
> "notrace", which was totally unnecessary as all of the tracing directory
> had function tracing disabled until the recent FUNCTION_SELF_TRACING was
> added.
>
> Remove the notrace annotation from trace_event_raw_event_synth() as
> there's no reason to not trace it when tracing synthetic event functions.
>
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Acked-by: Tom Zanussi <zanussi@kernel.org>
Thanks,
Tom
> ---
> kernel/trace/trace_events_synth.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/trace/trace_events_synth.c b/kernel/trace/trace_events_synth.c
> index 4554c458b78c..79a506fc2856 100644
> --- a/kernel/trace/trace_events_synth.c
> +++ b/kernel/trace/trace_events_synth.c
> @@ -493,9 +493,9 @@ static unsigned int trace_stack(struct synth_trace_event *entry,
> return len;
> }
>
> -static notrace void trace_event_raw_event_synth(void *__data,
> - u64 *var_ref_vals,
> - unsigned int *var_ref_idx)
> +static void trace_event_raw_event_synth(void *__data,
> + u64 *var_ref_vals,
> + unsigned int *var_ref_idx)
> {
> unsigned int i, n_u64, val_idx, len, data_size = 0;
> struct trace_event_file *trace_file = __data;
^ permalink raw reply
* Re: [PATCH] tracing: Have hist_debug show what function a field uses
From: Tom Zanussi @ 2026-01-25 19:21 UTC (permalink / raw)
To: Steven Rostedt, LKML, Linux Trace Kernel
Cc: Masami Hiramatsu, Mathieu Desnoyers
In-Reply-To: <20260122203822.58df4d80@gandalf.local.home>
Hi Steve,
On Thu, 2026-01-22 at 20:38 -0500, Steven Rostedt wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
>
> When CONFIG_HIST_TRIGGERS_DEBUG is enabled, each trace event has a
> "hist_debug" file that explains the histogram internal data. This is very
> useful for debugging histograms.
>
> One bit of data that was missing from this file was what function a
> histogram field uses to process its data. The hist_field structure now has
> a fn_num that is used by a switch statement in hist_fn_call() to call a
> function directly (to avoid spectre mitigations).
>
> Instead of displaying that number, create a string array that maps to the
> histogram function enums so that the function for a field may be
> displayed:
>
> ~# cat /sys/kernel/tracing/events/sched/sched_switch/hist_debug
> [..]
> hist_data: 0000000043d62762
>
> n_vals: 2
> n_keys: 1
> n_fields: 3
>
> val fields:
>
> hist_data->fields[0]:
> flags:
> VAL: HIST_FIELD_FL_HITCOUNT
> type: u64
> size: 8
> is_signed: 0
> function: hist_field_counter()
>
> hist_data->fields[1]:
> flags:
> HIST_FIELD_FL_VAR
> var.name: __arg_3921_2
> var.idx (into tracing_map_elt.vars[]): 0
> type: unsigned long[]
> size: 128
> is_signed: 0
> function: hist_field_nop()
>
> key fields:
>
> hist_data->fields[2]:
> flags:
> HIST_FIELD_FL_KEY
> ftrace_event_field name: prev_pid
> type: pid_t
> size: 8
> is_signed: 1
> function: hist_field_s32()
>
> The "function:" field above is added.
>
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Nice and useful addition, thanks!
Reviewed-by: Tom Zanussi <zanussi@kernel.org>
Tested-by: Tom Zanussi <zanussi@kernel.org>
Tom
> ---
> kernel/trace/trace_events_hist.c | 75 +++++++++++++++++++-------------
> 1 file changed, 44 insertions(+), 31 deletions(-)
>
> diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
> index 4a7f945e368d..24441b30a1a4 100644
> --- a/kernel/trace/trace_events_hist.c
> +++ b/kernel/trace/trace_events_hist.c
> @@ -105,38 +105,44 @@ enum field_op_id {
> FIELD_OP_MULT,
> };
>
> +#define FIELD_FUNCS \
> + C(NOP, "nop"), \
> + C(VAR_REF, "var_ref"), \
> + C(COUNTER, "counter"), \
> + C(CONST, "const"), \
> + C(LOG2, "log2"), \
> + C(BUCKET, "bucket"), \
> + C(TIMESTAMP, "timestamp"), \
> + C(CPU, "cpu"), \
> + C(COMM, "comm"), \
> + C(STRING, "string"), \
> + C(DYNSTRING, "dynstring"), \
> + C(RELDYNSTRING, "reldynstring"), \
> + C(PSTRING, "pstring"), \
> + C(S64, "s64"), \
> + C(U64, "u64"), \
> + C(S32, "s32"), \
> + C(U32, "u32"), \
> + C(S16, "s16"), \
> + C(U16, "u16"), \
> + C(S8, "s8"), \
> + C(U8, "u8"), \
> + C(UMINUS, "uminus"), \
> + C(MINUS, "minus"), \
> + C(PLUS, "plus"), \
> + C(DIV, "div"), \
> + C(MULT, "mult"), \
> + C(DIV_POWER2, "div_power2"), \
> + C(DIV_NOT_POWER2, "div_not_power2"), \
> + C(DIV_MULT_SHIFT, "div_mult_shift"), \
> + C(EXECNAME, "execname"), \
> + C(STACK, "stack"),
> +
> +#undef C
> +#define C(a, b) HIST_FIELD_FN_##a
> +
> enum hist_field_fn {
> - HIST_FIELD_FN_NOP,
> - HIST_FIELD_FN_VAR_REF,
> - HIST_FIELD_FN_COUNTER,
> - HIST_FIELD_FN_CONST,
> - HIST_FIELD_FN_LOG2,
> - HIST_FIELD_FN_BUCKET,
> - HIST_FIELD_FN_TIMESTAMP,
> - HIST_FIELD_FN_CPU,
> - HIST_FIELD_FN_COMM,
> - HIST_FIELD_FN_STRING,
> - HIST_FIELD_FN_DYNSTRING,
> - HIST_FIELD_FN_RELDYNSTRING,
> - HIST_FIELD_FN_PSTRING,
> - HIST_FIELD_FN_S64,
> - HIST_FIELD_FN_U64,
> - HIST_FIELD_FN_S32,
> - HIST_FIELD_FN_U32,
> - HIST_FIELD_FN_S16,
> - HIST_FIELD_FN_U16,
> - HIST_FIELD_FN_S8,
> - HIST_FIELD_FN_U8,
> - HIST_FIELD_FN_UMINUS,
> - HIST_FIELD_FN_MINUS,
> - HIST_FIELD_FN_PLUS,
> - HIST_FIELD_FN_DIV,
> - HIST_FIELD_FN_MULT,
> - HIST_FIELD_FN_DIV_POWER2,
> - HIST_FIELD_FN_DIV_NOT_POWER2,
> - HIST_FIELD_FN_DIV_MULT_SHIFT,
> - HIST_FIELD_FN_EXECNAME,
> - HIST_FIELD_FN_STACK,
> + FIELD_FUNCS
> };
>
> /*
> @@ -5845,6 +5851,12 @@ const struct file_operations event_hist_fops = {
> };
>
> #ifdef CONFIG_HIST_TRIGGERS_DEBUG
> +
> +#undef C
> +#define C(a, b) b
> +
> +static const char * const field_funcs[] = { FIELD_FUNCS };
> +
> static void hist_field_debug_show_flags(struct seq_file *m,
> unsigned long flags)
> {
> @@ -5909,6 +5921,7 @@ static int hist_field_debug_show(struct seq_file *m,
> seq_printf(m, " type: %s\n", field->type);
> seq_printf(m, " size: %u\n", field->size);
> seq_printf(m, " is_signed: %u\n", field->is_signed);
> + seq_printf(m, " function: hist_field_%s()\n", field_funcs[field->fn_num]);
>
> return 0;
> }
^ permalink raw reply
* Re: [PATCH] tracing: Fix crash on synthetic stacktrace field usage
From: Tom Zanussi @ 2026-01-25 19:20 UTC (permalink / raw)
To: Steven Rostedt, LKML, Linux Trace Kernel
Cc: Masami Hiramatsu, Mathieu Desnoyers
In-Reply-To: <20260122194824.6905a38e@gandalf.local.home>
Hi Steve,
On Thu, 2026-01-22 at 19:48 -0500, Steven Rostedt wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
>
> When creating a synthetic event based on an existing synthetic event that
> had a stacktrace field and the new synthetic event used that field a
> kernel crash occurred:
>
> ~# cd /sys/kernel/tracing
> ~# echo 's:stack unsigned long stack[];' > dynamic_events
> ~# echo 'hist:keys=prev_pid:s0=common_stacktrace if prev_state & 3' >> events/sched/sched_switch/trigger
> ~# echo 'hist:keys=next_pid:s1=$s0:onmatch(sched.sched_switch).trace(stack,$s1)' >> events/sched/sched_switch/trigger
>
> The above creates a synthetic event that takes a stacktrace when a task
> schedules out in a non-running state and passes that stacktrace to the
> sched_switch event when that task schedules back in. It triggers the
> "stack" synthetic event that has a stacktrace as its field (called "stack").
>
> ~# echo 's:syscall_stack s64 id; unsigned long stack[];' >> dynamic_events
> ~# echo 'hist:keys=common_pid:s2=stack' >> events/synthetic/stack/trigger
> ~# echo 'hist:keys=common_pid:s3=$s2,i0=id:onmatch(synthetic.stack).trace(syscall_stack,$i0,$s3)' >> events/raw_syscalls/sys_exit/trigger
>
> The above makes another synthetic event called "syscall_stack" that
> attaches the first synthetic event (stack) to the sys_exit trace event and
> records the stacktrace from the stack event with the id of the system call
> that is exiting.
>
> When enabling this event (or using it in a historgram):
>
> ~# echo 1 > events/synthetic/syscall_stack/enable
>
> Produces a kernel crash!
>
> BUG: unable to handle page fault for address: 0000000000400010
> #PF: supervisor read access in kernel mode
> #PF: error_code(0x0000) - not-present page
> PGD 0 P4D 0
> Oops: Oops: 0000 [#1] SMP PTI
> CPU: 6 UID: 0 PID: 1257 Comm: bash Not tainted 6.16.3+deb14-amd64 #1 PREEMPT(lazy) Debian 6.16.3-1
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.17.0-debian-1.17.0-1 04/01/2014
> RIP: 0010:trace_event_raw_event_synth+0x90/0x380
> Code: c5 00 00 00 00 85 d2 0f 84 e1 00 00 00 31 db eb 34 0f 1f 00 66 66 2e 0f 1f 84 00 00 00 00 00 66 66 2e 0f 1f 84 00 00 00 00 00 <49> 8b 04 24 48 83 c3 01 8d 0c c5 08 00 00 00 01 cd 41 3b 5d 40 0f
> RSP: 0018:ffffd2670388f958 EFLAGS: 00010202
> RAX: ffff8ba1065cc100 RBX: 0000000000000000 RCX: 0000000000000000
> RDX: 0000000000000001 RSI: fffff266ffda7b90 RDI: ffffd2670388f9b0
> RBP: 0000000000000010 R08: ffff8ba104e76000 R09: ffffd2670388fa50
> R10: ffff8ba102dd42e0 R11: ffffffff9a908970 R12: 0000000000400010
> R13: ffff8ba10a246400 R14: ffff8ba10a710220 R15: fffff266ffda7b90
> FS: 00007fa3bc63f740(0000) GS:ffff8ba2e0f48000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000400010 CR3: 0000000107f9e003 CR4: 0000000000172ef0
> Call Trace:
> <TASK>
> ? __tracing_map_insert+0x208/0x3a0
> action_trace+0x67/0x70
> event_hist_trigger+0x633/0x6d0
> event_triggers_call+0x82/0x130
> trace_event_buffer_commit+0x19d/0x250
> trace_event_raw_event_sys_exit+0x62/0xb0
> syscall_exit_work+0x9d/0x140
> do_syscall_64+0x20a/0x2f0
> ? trace_event_raw_event_sched_switch+0x12b/0x170
> ? save_fpregs_to_fpstate+0x3e/0x90
> ? _raw_spin_unlock+0xe/0x30
> ? finish_task_switch.isra.0+0x97/0x2c0
> ? __rseq_handle_notify_resume+0xad/0x4c0
> ? __schedule+0x4b8/0xd00
> ? restore_fpregs_from_fpstate+0x3c/0x90
> ? switch_fpu_return+0x5b/0xe0
> ? do_syscall_64+0x1ef/0x2f0
> ? do_fault+0x2e9/0x540
> ? __handle_mm_fault+0x7d1/0xf70
> ? count_memcg_events+0x167/0x1d0
> ? handle_mm_fault+0x1d7/0x2e0
> ? do_user_addr_fault+0x2c3/0x7f0
> entry_SYSCALL_64_after_hwframe+0x76/0x7e
>
> The reason is that the stacktrace field is not labeled as such, and is
> treated as a normal field and not as a dynamic event that it is.
>
> In trace_event_raw_event_synth() the event is field is still treated as a
> dynamic array, but the retrieval of the data is considered a normal field,
> and the reference is just the meta data:
>
> // Meta data is retrieved instead of a dynamic array
> str_val = (char *)(long)var_ref_vals[val_idx];
>
> // Then when it tries to process it:
> len = *((unsigned long *)str_val) + 1;
>
> It triggers a kernel page fault.
>
> To fix this, first when defining the fields of the first synthetic event,
> set the filter type to FILTER_STACKTRACE. This is used later by the second
> synthetic event to know that this field is a stacktrace. When creating
> the field of the new synthetic event, have it use this FILTER_STACKTRACE
> to know to create a stacktrace field to copy the stacktrace into.
>
> Cc: stable@vger.kernel.org
> Fixes: 00cf3d672a9d ("tracing: Allow synthetic events to pass around stacktraces")
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Looks good to me.
Reviewed-by: Tom Zanussi <zanussi@kernel.org>
Tested-by: Tom Zanussi <zanussi@kernel.org>
Thanks,
Tom
> ---
> kernel/trace/trace_events_hist.c | 9 +++++++++
> kernel/trace/trace_events_synth.c | 8 +++++++-
> 2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
> index 5e6e70540eef..c97bb2fda5c0 100644
> --- a/kernel/trace/trace_events_hist.c
> +++ b/kernel/trace/trace_events_hist.c
> @@ -2057,6 +2057,15 @@ static struct hist_field *create_hist_field(struct hist_trigger_data *hist_data,
> hist_field->fn_num = HIST_FIELD_FN_RELDYNSTRING;
> else
> hist_field->fn_num = HIST_FIELD_FN_PSTRING;
> + } else if (field->filter_type == FILTER_STACKTRACE) {
> + flags |= HIST_FIELD_FL_STACKTRACE;
> +
> + hist_field->size = MAX_FILTER_STR_VAL;
> + hist_field->type = kstrdup_const(field->type, GFP_KERNEL);
> + if (!hist_field->type)
> + goto free;
> +
> + hist_field->fn_num = HIST_FIELD_FN_STACK;
> } else {
> hist_field->size = field->size;
> hist_field->is_signed = field->is_signed;
> diff --git a/kernel/trace/trace_events_synth.c b/kernel/trace/trace_events_synth.c
> index 4554c458b78c..45c187e77e21 100644
> --- a/kernel/trace/trace_events_synth.c
> +++ b/kernel/trace/trace_events_synth.c
> @@ -130,7 +130,9 @@ static int synth_event_define_fields(struct trace_event_call *call)
> struct synth_event *event = call->data;
> unsigned int i, size, n_u64;
> char *name, *type;
> + int filter_type;
> bool is_signed;
> + bool is_stack;
> int ret = 0;
>
> for (i = 0, n_u64 = 0; i < event->n_fields; i++) {
> @@ -138,8 +140,12 @@ static int synth_event_define_fields(struct trace_event_call *call)
> is_signed = event->fields[i]->is_signed;
> type = event->fields[i]->type;
> name = event->fields[i]->name;
> + is_stack = event->fields[i]->is_stack;
> +
> + filter_type = is_stack ? FILTER_STACKTRACE : FILTER_OTHER;
> +
> ret = trace_define_field(call, type, name, offset, size,
> - is_signed, FILTER_OTHER);
> + is_signed, filter_type);
> if (ret)
> break;
>
^ permalink raw reply
* Re: [RFC PATCH 0/3] rtla: Synchronize sample collection methods
From: Steven Rostedt @ 2026-01-24 18:50 UTC (permalink / raw)
To: Tomas Glozar
Cc: Masami Hiramatsu, Mathieu Desnoyers, Crystal Wood, John Kacur,
Luis Goncalves, LKML, Linux Trace Kernel
In-Reply-To: <20260123152534.1036533-1-tglozar@redhat.com>
On Fri, 23 Jan 2026 16:25:31 +0100
Tomas Glozar <tglozar@redhat.com> wrote:
> To enable RTLA to analyze samples consistently, the first patch adds two fields
> to the osnoise:timerlat_sample tracepoint: instances_registered and
> instances_on. During the recording of a timerlat sample, timerlat counts
> how many instances are registered and how many are on, and attaches
> the information to the osnoise:timerlat_sample trace event, which is moved
> to occur after the samples are recorded.
Can't RTLA simply write into trace_marker or trace_marker_raw an event
that states "tracing is now active" and ignore anything before that event.
Heck, it will include a timestamp, so you only need to write once and
ignore any event that occurred before that timestamp.
-- Steve
^ permalink raw reply
* Re: [PATCH] tracing: kprobe-event: Return directly when dyn_event_list is empty
From: Steven Rostedt @ 2026-01-24 17:02 UTC (permalink / raw)
To: sunliming
Cc: mhiramat, mathieu.desnoyers, linux-kernel, linux-trace-kernel,
sunliming
In-Reply-To: <20260123013641.23066-1-sunliming@linux.dev>
On Fri, 23 Jan 2026 09:36:41 +0800
sunliming@linux.dev wrote:
> From: sunliming <sunliming@kylinos.cn>
>
> In enable_boot_kprobe_events(), it returns directly when dyn_event_list is
> empty, thereby reducing the function's execution time. This function may
> otherwise wait for the event_mutex lock for tens of milliseconds on certain
> machines, which is unnecessary when dyn_event_list is empty.
Have you measured this?
I'm curious as to what may be holding the event_mutex at this time.
This is called by the initcall functions which are currently all
serialized. Any conflict would have to be caused by a worker or kthread
and not another initcall callback.
I'm not against the patch, I just want to understand more about it.
>
> Signed-off-by: sunliming <sunliming@kylinos.cn>
> ---
> kernel/trace/trace_kprobe.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 9953506370a5..d89a403c99d4 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -1982,6 +1982,9 @@ static __init void enable_boot_kprobe_events(void)
> struct trace_kprobe *tk;
> struct dyn_event *pos;
>
> + if (list_empty(&dyn_event_list))
> + return;
The above should definitely have a comment or we should wrap that with
another macro because the above is assuming that
for_each_trace_kprobe() uses dyn_event_list. If that ever changes in
the future, this will be broken.
Perhaps we should add a:
if (trace_kprobe_list_empty())
return;
?
-- Steve
> +
> guard(mutex)(&event_mutex);
> for_each_trace_kprobe(tk, pos) {
> list_for_each_entry(file, &tr->events, list)
^ permalink raw reply
* Re: [PATCH] rv: Fix dead link to monitor_synthesis.rst
From: Gabriele Monaco @ 2026-01-24 7:07 UTC (permalink / raw)
To: Soham Metha
Cc: linux-kernel-mentees, shuah, skhan, linux-kernel, Steven Rostedt,
linux-trace-kernel
In-Reply-To: <c79d4fbd-e9a4-4d73-a650-b01f437a4c0b@gmail.com>
2026-01-23T20:06:35Z Soham Metha <sohammetha01@gmail.com>:
> On 04/12/25 01:52, Soham Metha wrote:
>> --- a/include/rv/da_monitor.h
>> +++ b/include/rv/da_monitor.h
>> @@ -8,7 +8,7 @@
>> * The dot2k tool is available at tools/verification/dot2k/
>> *
>> * For further information, see:
>> - * Documentation/trace/rv/da_monitor_synthesis.rst
>> + * Documentation/trace/rv/monitor_synthesis.rst
>> */
> Hi,
>
> Just a gentle ping on this documentation patch in case it was missed.
> Please let me know if any changes are needed.
Hi Soham,
the same change was included in [1], which is already on linux-next.
I think I missed your patch since it apparently arrived first.
Thanks anyway,
Gabriele
[1] - https://lore.kernel.org/lkml/20251230075337.11993-1-slopixelz@gmail.com
^ permalink raw reply
* Re: [PATCH mm-unstable v14 03/16] introduce collapse_single_pmd to unify khugepaged and madvise_collapse
From: Lance Yang @ 2026-01-24 4:41 UTC (permalink / raw)
To: Nico Pache, Garg, Shivank
Cc: akpm, david, lorenzo.stoakes, ziy, baolin.wang, Liam.Howlett,
ryan.roberts, dev.jain, baohua, vbabka, rppt, surenb, mhocko,
linux-trace-kernel, linux-doc, corbet, rostedt, mhiramat,
mathieu.desnoyers, linux-kernel, 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, David Hildenbrand, linux-mm
In-Reply-To: <CAA1CXcDm75=hM_g0x7ox05nGrGykE8ry_+NbryYu=T+TY114MQ@mail.gmail.com>
On 2026/1/24 07:26, Nico Pache wrote:
> On Thu, Jan 22, 2026 at 10:08 PM Lance Yang <lance.yang@linux.dev> wrote:
>>
>>
>>
>> On 2026/1/23 03:28, Nico Pache wrote:
>>> The khugepaged daemon and madvise_collapse have two different
>>> implementations that do almost the same thing.
>>>
>>> Create collapse_single_pmd to increase code reuse and create an entry
>>> point to these two users.
>>>
>>> Refactor madvise_collapse and collapse_scan_mm_slot to use the new
>>> collapse_single_pmd function. This introduces a minor behavioral change
>>> that is most likely an undiscovered bug. The current implementation of
>>> khugepaged tests collapse_test_exit_or_disable before calling
>>> collapse_pte_mapped_thp, but we weren't doing it in the madvise_collapse
>>> case. By unifying these two callers madvise_collapse now also performs
>>> this check. We also modify the return value to be SCAN_ANY_PROCESS which
>>> properly indicates that this process is no longer valid to operate on.
>>>
>>> We also guard the khugepaged_pages_collapsed variable to ensure its only
>>> incremented for khugepaged.
>>>
>>> Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
>>> Reviewed-by: Lance Yang <lance.yang@linux.dev>
>>> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>>> Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
>>> Reviewed-by: Zi Yan <ziy@nvidia.com>
>>> Acked-by: David Hildenbrand <david@redhat.com>
>>> Signed-off-by: Nico Pache <npache@redhat.com>
>>> ---
>>
>> I think this patch introduces some functional changes compared to previous
>> version[1] ...
>>
>> Maybe we should drop the r-b tags and let folks take another look?
>>
>> There might be an issue with the vma access in madvise_collapse(). See
>> below:
>>
>> [1]
>> https://lore.kernel.org/linux-mm/20251201174627.23295-3-npache@redhat.com/
>>
>>> mm/khugepaged.c | 106 +++++++++++++++++++++++++++---------------------
>>> 1 file changed, 60 insertions(+), 46 deletions(-)
>>>
>>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>>> index fefcbdca4510..59e5a5588d85 100644
>>> --- a/mm/khugepaged.c
>>> +++ b/mm/khugepaged.c
>>> @@ -2394,6 +2394,54 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm, unsigned long a
>>> return result;
>>> }
>>>
>>> +/*
>>> + * Try to collapse a single PMD starting at a PMD aligned addr, and return
>>> + * the results.
>>> + */
>>> +static enum scan_result collapse_single_pmd(unsigned long addr,
>>> + struct vm_area_struct *vma, bool *mmap_locked,
>>> + struct collapse_control *cc)
>>> +{
>>> + struct mm_struct *mm = vma->vm_mm;
>>> + enum scan_result result;
>>> + struct file *file;
>>> + pgoff_t pgoff;
>>> +
>>> + if (vma_is_anonymous(vma)) {
>>> + result = collapse_scan_pmd(mm, vma, addr, mmap_locked, cc);
>>> + goto end;
>>> + }
>>> +
>>> + file = get_file(vma->vm_file);
>>> + pgoff = linear_page_index(vma, addr);
>>> +
>>> + mmap_read_unlock(mm);
>>> + *mmap_locked = false;
>>> + result = collapse_scan_file(mm, addr, file, pgoff, cc);
>>> + fput(file);
>>> +
>>> + if (result != SCAN_PTE_MAPPED_HUGEPAGE)
>>> + goto end;
>>> +
>>> + mmap_read_lock(mm);
>>> + *mmap_locked = true;
>>> + if (collapse_test_exit_or_disable(mm)) {
>>> + mmap_read_unlock(mm);
>>> + *mmap_locked = false;
>>> + return SCAN_ANY_PROCESS;
>>> + }
>>> + result = try_collapse_pte_mapped_thp(mm, addr, !cc->is_khugepaged);
>>> + if (result == SCAN_PMD_MAPPED)
>>> + result = SCAN_SUCCEED;
>>> + mmap_read_unlock(mm);
>>> + *mmap_locked = false;
>>> +
>>> +end:
>>> + if (cc->is_khugepaged && result == SCAN_SUCCEED)
>>> + ++khugepaged_pages_collapsed;
>>> + return result;
>>> +}
>>> +
>>> static unsigned int collapse_scan_mm_slot(unsigned int pages, enum scan_result *result,
>>> struct collapse_control *cc)
>>> __releases(&khugepaged_mm_lock)
>>> @@ -2466,34 +2514,9 @@ static unsigned int collapse_scan_mm_slot(unsigned int pages, enum scan_result *
>>> VM_BUG_ON(khugepaged_scan.address < hstart ||
>>> khugepaged_scan.address + HPAGE_PMD_SIZE >
>>> hend);
>>> - if (!vma_is_anonymous(vma)) {
>>> - struct file *file = get_file(vma->vm_file);
>>> - pgoff_t pgoff = linear_page_index(vma,
>>> - khugepaged_scan.address);
>>> -
>>> - mmap_read_unlock(mm);
>>> - mmap_locked = false;
>>> - *result = collapse_scan_file(mm,
>>> - khugepaged_scan.address, file, pgoff, cc);
>>> - fput(file);
>>> - if (*result == SCAN_PTE_MAPPED_HUGEPAGE) {
>>> - mmap_read_lock(mm);
>>> - if (collapse_test_exit_or_disable(mm))
>>> - goto breakouterloop;
>>> - *result = try_collapse_pte_mapped_thp(mm,
>>> - khugepaged_scan.address, false);
>>> - if (*result == SCAN_PMD_MAPPED)
>>> - *result = SCAN_SUCCEED;
>>> - mmap_read_unlock(mm);
>>> - }
>>> - } else {
>>> - *result = collapse_scan_pmd(mm, vma,
>>> - khugepaged_scan.address, &mmap_locked, cc);
>>> - }
>>> -
>>> - if (*result == SCAN_SUCCEED)
>>> - ++khugepaged_pages_collapsed;
>>>
>>> + *result = collapse_single_pmd(khugepaged_scan.address,
>>> + vma, &mmap_locked, cc);
>>> /* move to next address */
>>> khugepaged_scan.address += HPAGE_PMD_SIZE;
>>> progress += HPAGE_PMD_NR;
>>> @@ -2799,6 +2822,7 @@ int madvise_collapse(struct vm_area_struct *vma, unsigned long start,
>>> cond_resched();
>>> mmap_read_lock(mm);
>>> mmap_locked = true;
>>> + *lock_dropped = true;
>>> result = hugepage_vma_revalidate(mm, addr, false, &vma,
>>> cc);
>>> if (result != SCAN_SUCCEED) {
>>> @@ -2809,17 +2833,17 @@ int madvise_collapse(struct vm_area_struct *vma, unsigned long start,
>>> hend = min(hend, vma->vm_end & HPAGE_PMD_MASK);
>>> }
>>> mmap_assert_locked(mm);
>>> - if (!vma_is_anonymous(vma)) {
>>> - struct file *file = get_file(vma->vm_file);
>>> - pgoff_t pgoff = linear_page_index(vma, addr);
>>>
>>> - mmap_read_unlock(mm);
>>> - mmap_locked = false;
>>> + result = collapse_single_pmd(addr, vma, &mmap_locked, cc);
>>> +
>>> + if (!mmap_locked)
>>> *lock_dropped = true;
>>> - result = collapse_scan_file(mm, addr, file, pgoff, cc);
>>>
>>> - if (result == SCAN_PAGE_DIRTY_OR_WRITEBACK && !triggered_wb &&
>>> - mapping_can_writeback(file->f_mapping)) {
>>> + if (result == SCAN_PAGE_DIRTY_OR_WRITEBACK && !triggered_wb) {
>>> + struct file *file = get_file(vma->vm_file);
>>> + pgoff_t pgoff = linear_page_index(vma, addr);
>>
>>
>> After collapse_single_pmd() returns, mmap_lock might have been released.
>> Between
>> that unlock and here, another thread could unmap/remap the VMA, making
>> the vma
>> pointer stale when we access vma->vm_file?
>
> + Shivank, I thought they were on the CC list.
>
> Hey! I thought of this case, but then figured it was no different than
> what is currently implemented for the writeback-retry logic, since the
> mmap lock is dropped and not revalidated. BUT I failed to consider
> that the file reference is held throughout that time.
>
> I thought of moving the functionality into collapse_single_pmd(), but
> figured I'd keep it in madvise_collapse() as it's the sole user of
> that functionality. Given the potential file ref issue, that may be
> the best solution, and I dont think it should be too difficult. I'll
> queue that up, and also drop the r-b tags as you suggested.
>
> Ok, here's my solution, does this look like the right approach?:
Hey! Thanks for the quick fix!
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 59e5a5588d85..dda9fdc35767 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -2418,6 +2418,14 @@ static enum scan_result
> collapse_single_pmd(unsigned long addr,
> mmap_read_unlock(mm);
> *mmap_locked = false;
> result = collapse_scan_file(mm, addr, file, pgoff, cc);
> +
> + if (!cc->is_khugepaged && result == SCAN_PAGE_DIRTY_OR_WRITEBACK &&
> + mapping_can_writeback(file->f_mapping)) {
> + loff_t lstart = (loff_t)pgoff << PAGE_SHIFT;
> + loff_t lend = lstart + HPAGE_PMD_SIZE - 1;
> +
> + filemap_write_and_wait_range(file->f_mapping, lstart, lend);
> + }
> fput(file);
>
> if (result != SCAN_PTE_MAPPED_HUGEPAGE)
> @@ -2840,19 +2848,8 @@ int madvise_collapse(struct vm_area_struct
> *vma, unsigned long start,
> *lock_dropped = true;
>
> if (result == SCAN_PAGE_DIRTY_OR_WRITEBACK && !triggered_wb) {
> - struct file *file = get_file(vma->vm_file);
> - pgoff_t pgoff = linear_page_index(vma, addr);
> -
> - if (mapping_can_writeback(file->f_mapping)) {
> - loff_t lstart = (loff_t)pgoff << PAGE_SHIFT;
> - loff_t lend = lstart + HPAGE_PMD_SIZE - 1;
> -
> -
> filemap_write_and_wait_range(file->f_mapping, lstart, lend);
> - triggered_wb = true;
> - fput(file);
> - goto retry;
> - }
> - fput(file);
> + triggered_wb = true;
> + goto retry;
> }
>
> switch (result) {
>
>
>
> -- Nico
From a quick glimpse, that looks good to me ;)
Only madvise needs writeback and then retry once, and khugepaged just
skips dirty pages and moves on.
Now, we grab the file reference before dropping mmap_lock, then only
use the file pointer during writeback - no vma access after unlock.
So even if the VMA gets unmapped, we're safe, IIUC.
[...]
^ permalink raw reply
* Re: [PATCH mm-unstable v14 03/16] introduce collapse_single_pmd to unify khugepaged and madvise_collapse
From: Nico Pache @ 2026-01-23 23:26 UTC (permalink / raw)
To: Lance Yang, Garg, Shivank
Cc: akpm, david, lorenzo.stoakes, ziy, baolin.wang, Liam.Howlett,
ryan.roberts, dev.jain, baohua, vbabka, rppt, surenb, mhocko,
linux-trace-kernel, linux-doc, corbet, rostedt, mhiramat,
mathieu.desnoyers, linux-kernel, 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, David Hildenbrand, linux-mm
In-Reply-To: <65dcf7ab-1299-411f-9cbc-438ae72ff757@linux.dev>
On Thu, Jan 22, 2026 at 10:08 PM Lance Yang <lance.yang@linux.dev> wrote:
>
>
>
> On 2026/1/23 03:28, Nico Pache wrote:
> > The khugepaged daemon and madvise_collapse have two different
> > implementations that do almost the same thing.
> >
> > Create collapse_single_pmd to increase code reuse and create an entry
> > point to these two users.
> >
> > Refactor madvise_collapse and collapse_scan_mm_slot to use the new
> > collapse_single_pmd function. This introduces a minor behavioral change
> > that is most likely an undiscovered bug. The current implementation of
> > khugepaged tests collapse_test_exit_or_disable before calling
> > collapse_pte_mapped_thp, but we weren't doing it in the madvise_collapse
> > case. By unifying these two callers madvise_collapse now also performs
> > this check. We also modify the return value to be SCAN_ANY_PROCESS which
> > properly indicates that this process is no longer valid to operate on.
> >
> > We also guard the khugepaged_pages_collapsed variable to ensure its only
> > incremented for khugepaged.
> >
> > Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
> > Reviewed-by: Lance Yang <lance.yang@linux.dev>
> > Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> > Reviewed-by: Zi Yan <ziy@nvidia.com>
> > Acked-by: David Hildenbrand <david@redhat.com>
> > Signed-off-by: Nico Pache <npache@redhat.com>
> > ---
>
> I think this patch introduces some functional changes compared to previous
> version[1] ...
>
> Maybe we should drop the r-b tags and let folks take another look?
>
> There might be an issue with the vma access in madvise_collapse(). See
> below:
>
> [1]
> https://lore.kernel.org/linux-mm/20251201174627.23295-3-npache@redhat.com/
>
> > mm/khugepaged.c | 106 +++++++++++++++++++++++++++---------------------
> > 1 file changed, 60 insertions(+), 46 deletions(-)
> >
> > diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> > index fefcbdca4510..59e5a5588d85 100644
> > --- a/mm/khugepaged.c
> > +++ b/mm/khugepaged.c
> > @@ -2394,6 +2394,54 @@ static enum scan_result collapse_scan_file(struct mm_struct *mm, unsigned long a
> > return result;
> > }
> >
> > +/*
> > + * Try to collapse a single PMD starting at a PMD aligned addr, and return
> > + * the results.
> > + */
> > +static enum scan_result collapse_single_pmd(unsigned long addr,
> > + struct vm_area_struct *vma, bool *mmap_locked,
> > + struct collapse_control *cc)
> > +{
> > + struct mm_struct *mm = vma->vm_mm;
> > + enum scan_result result;
> > + struct file *file;
> > + pgoff_t pgoff;
> > +
> > + if (vma_is_anonymous(vma)) {
> > + result = collapse_scan_pmd(mm, vma, addr, mmap_locked, cc);
> > + goto end;
> > + }
> > +
> > + file = get_file(vma->vm_file);
> > + pgoff = linear_page_index(vma, addr);
> > +
> > + mmap_read_unlock(mm);
> > + *mmap_locked = false;
> > + result = collapse_scan_file(mm, addr, file, pgoff, cc);
> > + fput(file);
> > +
> > + if (result != SCAN_PTE_MAPPED_HUGEPAGE)
> > + goto end;
> > +
> > + mmap_read_lock(mm);
> > + *mmap_locked = true;
> > + if (collapse_test_exit_or_disable(mm)) {
> > + mmap_read_unlock(mm);
> > + *mmap_locked = false;
> > + return SCAN_ANY_PROCESS;
> > + }
> > + result = try_collapse_pte_mapped_thp(mm, addr, !cc->is_khugepaged);
> > + if (result == SCAN_PMD_MAPPED)
> > + result = SCAN_SUCCEED;
> > + mmap_read_unlock(mm);
> > + *mmap_locked = false;
> > +
> > +end:
> > + if (cc->is_khugepaged && result == SCAN_SUCCEED)
> > + ++khugepaged_pages_collapsed;
> > + return result;
> > +}
> > +
> > static unsigned int collapse_scan_mm_slot(unsigned int pages, enum scan_result *result,
> > struct collapse_control *cc)
> > __releases(&khugepaged_mm_lock)
> > @@ -2466,34 +2514,9 @@ static unsigned int collapse_scan_mm_slot(unsigned int pages, enum scan_result *
> > VM_BUG_ON(khugepaged_scan.address < hstart ||
> > khugepaged_scan.address + HPAGE_PMD_SIZE >
> > hend);
> > - if (!vma_is_anonymous(vma)) {
> > - struct file *file = get_file(vma->vm_file);
> > - pgoff_t pgoff = linear_page_index(vma,
> > - khugepaged_scan.address);
> > -
> > - mmap_read_unlock(mm);
> > - mmap_locked = false;
> > - *result = collapse_scan_file(mm,
> > - khugepaged_scan.address, file, pgoff, cc);
> > - fput(file);
> > - if (*result == SCAN_PTE_MAPPED_HUGEPAGE) {
> > - mmap_read_lock(mm);
> > - if (collapse_test_exit_or_disable(mm))
> > - goto breakouterloop;
> > - *result = try_collapse_pte_mapped_thp(mm,
> > - khugepaged_scan.address, false);
> > - if (*result == SCAN_PMD_MAPPED)
> > - *result = SCAN_SUCCEED;
> > - mmap_read_unlock(mm);
> > - }
> > - } else {
> > - *result = collapse_scan_pmd(mm, vma,
> > - khugepaged_scan.address, &mmap_locked, cc);
> > - }
> > -
> > - if (*result == SCAN_SUCCEED)
> > - ++khugepaged_pages_collapsed;
> >
> > + *result = collapse_single_pmd(khugepaged_scan.address,
> > + vma, &mmap_locked, cc);
> > /* move to next address */
> > khugepaged_scan.address += HPAGE_PMD_SIZE;
> > progress += HPAGE_PMD_NR;
> > @@ -2799,6 +2822,7 @@ int madvise_collapse(struct vm_area_struct *vma, unsigned long start,
> > cond_resched();
> > mmap_read_lock(mm);
> > mmap_locked = true;
> > + *lock_dropped = true;
> > result = hugepage_vma_revalidate(mm, addr, false, &vma,
> > cc);
> > if (result != SCAN_SUCCEED) {
> > @@ -2809,17 +2833,17 @@ int madvise_collapse(struct vm_area_struct *vma, unsigned long start,
> > hend = min(hend, vma->vm_end & HPAGE_PMD_MASK);
> > }
> > mmap_assert_locked(mm);
> > - if (!vma_is_anonymous(vma)) {
> > - struct file *file = get_file(vma->vm_file);
> > - pgoff_t pgoff = linear_page_index(vma, addr);
> >
> > - mmap_read_unlock(mm);
> > - mmap_locked = false;
> > + result = collapse_single_pmd(addr, vma, &mmap_locked, cc);
> > +
> > + if (!mmap_locked)
> > *lock_dropped = true;
> > - result = collapse_scan_file(mm, addr, file, pgoff, cc);
> >
> > - if (result == SCAN_PAGE_DIRTY_OR_WRITEBACK && !triggered_wb &&
> > - mapping_can_writeback(file->f_mapping)) {
> > + if (result == SCAN_PAGE_DIRTY_OR_WRITEBACK && !triggered_wb) {
> > + struct file *file = get_file(vma->vm_file);
> > + pgoff_t pgoff = linear_page_index(vma, addr);
>
>
> After collapse_single_pmd() returns, mmap_lock might have been released.
> Between
> that unlock and here, another thread could unmap/remap the VMA, making
> the vma
> pointer stale when we access vma->vm_file?
+ Shivank, I thought they were on the CC list.
Hey! I thought of this case, but then figured it was no different than
what is currently implemented for the writeback-retry logic, since the
mmap lock is dropped and not revalidated. BUT I failed to consider
that the file reference is held throughout that time.
I thought of moving the functionality into collapse_single_pmd(), but
figured I'd keep it in madvise_collapse() as it's the sole user of
that functionality. Given the potential file ref issue, that may be
the best solution, and I dont think it should be too difficult. I'll
queue that up, and also drop the r-b tags as you suggested.
Ok, here's my solution, does this look like the right approach?:
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 59e5a5588d85..dda9fdc35767 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -2418,6 +2418,14 @@ static enum scan_result
collapse_single_pmd(unsigned long addr,
mmap_read_unlock(mm);
*mmap_locked = false;
result = collapse_scan_file(mm, addr, file, pgoff, cc);
+
+ if (!cc->is_khugepaged && result == SCAN_PAGE_DIRTY_OR_WRITEBACK &&
+ mapping_can_writeback(file->f_mapping)) {
+ loff_t lstart = (loff_t)pgoff << PAGE_SHIFT;
+ loff_t lend = lstart + HPAGE_PMD_SIZE - 1;
+
+ filemap_write_and_wait_range(file->f_mapping, lstart, lend);
+ }
fput(file);
if (result != SCAN_PTE_MAPPED_HUGEPAGE)
@@ -2840,19 +2848,8 @@ int madvise_collapse(struct vm_area_struct
*vma, unsigned long start,
*lock_dropped = true;
if (result == SCAN_PAGE_DIRTY_OR_WRITEBACK && !triggered_wb) {
- struct file *file = get_file(vma->vm_file);
- pgoff_t pgoff = linear_page_index(vma, addr);
-
- if (mapping_can_writeback(file->f_mapping)) {
- loff_t lstart = (loff_t)pgoff << PAGE_SHIFT;
- loff_t lend = lstart + HPAGE_PMD_SIZE - 1;
-
-
filemap_write_and_wait_range(file->f_mapping, lstart, lend);
- triggered_wb = true;
- fput(file);
- goto retry;
- }
- fput(file);
+ triggered_wb = true;
+ goto retry;
}
switch (result) {
-- Nico
>
> Would it be safer to get the file reference before calling
> collapse_single_pmd()?
> Or we need to revalidate the VMA after getting the lock back?
>
>
> Thanks,
> Lance
>
> > +
> > + if (mapping_can_writeback(file->f_mapping)) {
> > loff_t lstart = (loff_t)pgoff << PAGE_SHIFT;
> > loff_t lend = lstart + HPAGE_PMD_SIZE - 1;
> >
> > @@ -2829,26 +2853,16 @@ int madvise_collapse(struct vm_area_struct *vma, unsigned long start,
> > goto retry;
> > }
> > fput(file);
> > - } else {
> > - result = collapse_scan_pmd(mm, vma, addr, &mmap_locked, cc);
> > }
> > - if (!mmap_locked)
> > - *lock_dropped = true;
> >
> > -handle_result:
> > switch (result) {
> > case SCAN_SUCCEED:
> > case SCAN_PMD_MAPPED:
> > ++thps;
> > break;
> > - case SCAN_PTE_MAPPED_HUGEPAGE:
> > - BUG_ON(mmap_locked);
> > - mmap_read_lock(mm);
> > - result = try_collapse_pte_mapped_thp(mm, addr, true);
> > - mmap_read_unlock(mm);
> > - goto handle_result;
> > /* Whitelisted set of results where continuing OK */
> > case SCAN_NO_PTE_TABLE:
> > + case SCAN_PTE_MAPPED_HUGEPAGE:
> > case SCAN_PTE_NON_PRESENT:
> > case SCAN_PTE_UFFD_WP:
> > case SCAN_LACK_REFERENCED_PAGE:
>
^ permalink raw reply related
* Re: [PATCH 1/3] tracing: Export eval_map_wq for asynchronous use by other modules
From: Steven Rostedt @ 2026-01-23 20:32 UTC (permalink / raw)
To: Yaxiong Tian
Cc: axboe, mhiramat, mathieu.desnoyers, linux-block, linux-kernel,
linux-trace-kernel
In-Reply-To: <20260122032415.395548-1-tianyaxiong@kylinos.cn>
On Thu, 22 Jan 2026 11:24:15 +0800
Yaxiong Tian <tianyaxiong@kylinos.cn> wrote:
> The eval_map_work_func() function, though queued in eval_map_wq,
> holds the trace_event_sem read-write lock for a long time during
> kernel boot. This causes blocking issues for other functions.
>
> Making eval_map_wq extern allows other modules to schedule their
> work asynchronously on this queue, preventing it from blocking
> the main boot thread.
If you are going to make this wq generic for other parts of the tracing
code, then let's rename it. As "eval_map" is specific to the enum mappings
being updated (which I hope someday I can get that update working at build
time. And when I do, the name will be totally meaningless).
Let's rename it to "trace_init_wq" instead.
-- Steve
^ permalink raw reply
* Re: [PATCH] rv: Fix dead link to monitor_synthesis.rst
From: Soham Metha @ 2026-01-23 20:06 UTC (permalink / raw)
To: linux-kernel-mentees
Cc: shuah, skhan, linux-kernel, Steven Rostedt, Gabriele Monaco,
linux-trace-kernel
In-Reply-To: <20251203202259.69116-1-sohammetha01@gmail.com>
On 04/12/25 01:52, Soham Metha wrote:
> The file 'da_monitor_synthesis.rst' was renamed to 'monitor_synthesis.rst' in
> commit f40a7c060207
> ("Documentation/rv: Prepare monitor synthesis document for LTL inclusion").
>
> Update the reference to point to the new filename.
>
> Signed-off-by: Soham Metha <sohammetha01@gmail.com>
> ---
> No functional changes.
>
> include/rv/da_monitor.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
> index 17fa4f6e5ea6..c2cea209f230 100644
> --- a/include/rv/da_monitor.h
> +++ b/include/rv/da_monitor.h
> @@ -8,7 +8,7 @@
> * The dot2k tool is available at tools/verification/dot2k/
> *
> * For further information, see:
> - * Documentation/trace/rv/da_monitor_synthesis.rst
> + * Documentation/trace/rv/monitor_synthesis.rst
> */
>
> #include <rv/automata.h>
Hi,
Just a gentle ping on this documentation patch in case it was missed.
Please let me know if any changes are needed.
Thanks,
Soham
^ permalink raw reply
* Re: [RFC v4 6/7] ext4: fast commit: add lock_updates tracepoint
From: Steven Rostedt @ 2026-01-23 17:57 UTC (permalink / raw)
To: Li Chen
Cc: Zhang Yi, Theodore Ts'o, Andreas Dilger, Masami Hiramatsu,
Mathieu Desnoyers, linux-ext4, linux-kernel, linux-trace-kernel
In-Reply-To: <20260120112538.132774-7-me@linux.beauty>
On Tue, 20 Jan 2026 19:25:35 +0800
Li Chen <me@linux.beauty> wrote:
> Commit-time fast commit snapshots run under jbd2_journal_lock_updates(),
> so it is useful to quantify the time spent with updates locked and to
> understand why snapshotting can fail.
>
> Add a new tracepoint, ext4_fc_lock_updates, reporting the time spent in
> the updates-locked window along with the number of snapshotted inodes
> and ranges. Record the first snapshot failure reason in a stable snap_err
> field for tooling.
>
> Signed-off-by: Li Chen <me@linux.beauty>
> ---
> fs/ext4/fast_commit.c | 86 ++++++++++++++++++++++++++++++-------
> include/trace/events/ext4.h | 33 ++++++++++++++
> 2 files changed, 104 insertions(+), 15 deletions(-)
>
> diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
> index d1eefee60912..d266eb2a4219 100644
> --- a/fs/ext4/fast_commit.c
> +++ b/fs/ext4/fast_commit.c
> @@ -193,6 +193,27 @@ static struct kmem_cache *ext4_fc_range_cachep;
> #define EXT4_FC_SNAPSHOT_MAX_INODES 1024
> #define EXT4_FC_SNAPSHOT_MAX_RANGES 2048
>
> +/*
> + * Snapshot failure reasons for ext4_fc_lock_updates tracepoint.
> + * Keep these stable for tooling.
> + */
> +enum ext4_fc_snap_err {
> + EXT4_FC_SNAP_ERR_NONE = 0,
> + EXT4_FC_SNAP_ERR_ES_MISS,
> + EXT4_FC_SNAP_ERR_ES_DELAYED,
> + EXT4_FC_SNAP_ERR_ES_OTHER,
> + EXT4_FC_SNAP_ERR_INODES_CAP,
> + EXT4_FC_SNAP_ERR_RANGES_CAP,
> + EXT4_FC_SNAP_ERR_NOMEM,
> + EXT4_FC_SNAP_ERR_INODE_LOC,
> +};
> +
> +static inline void ext4_fc_set_snap_err(int *snap_err, int err)
> +{
> + if (snap_err && *snap_err == EXT4_FC_SNAP_ERR_NONE)
> + *snap_err = err;
> +}
> +
> diff --git a/include/trace/events/ext4.h b/include/trace/events/ext4.h
> index fd76d14c2776..a1493971821d 100644
> --- a/include/trace/events/ext4.h
> +++ b/include/trace/events/ext4.h
> @@ -2812,6 +2812,39 @@ TRACE_EVENT(ext4_fc_commit_stop,
> __entry->num_fc_ineligible, __entry->nblks_agg, __entry->tid)
> );
>
Why not make the snap_err into a human readable format?
#define TRACE_SNAP_ERR \
EM(NONE) \
EM(ES_MISS) \
EM(ES_DELAYED) \
EM(ES_OTHER) \
EM(INODES_CAP) \
EM(RANGES_CAP) \
EM(NOMEM) \
EMe(INODE_LOC) \
#undef EM
#undef EMe
#define EM(a) TRACE_DEFINE_ENUM(EXT4_FC_SNAP_ERR_##a);
#define EMe(a) TRACE_DEFINE_ENUM(EXT4_FC_SNAP_ERR_##a);
TRACE_SNAP_ERR
#undef EM
#undef EMe
#define EM(a) { EXT4_FC_SNAP_ERR_##a, #a },
#define EM(a) { EXT4_FC_SNAP_ERR_##a, #a }
> +TRACE_EVENT(ext4_fc_lock_updates,
> + TP_PROTO(struct super_block *sb, tid_t commit_tid, u64 locked_ns,
> + unsigned int nr_inodes, unsigned int nr_ranges, int err,
> + int snap_err),
> +
> + TP_ARGS(sb, commit_tid, locked_ns, nr_inodes, nr_ranges, err, snap_err),
> +
> + TP_STRUCT__entry(/* entry */
> + __field(dev_t, dev)
> + __field(tid_t, tid)
> + __field(u64, locked_ns)
> + __field(unsigned int, nr_inodes)
> + __field(unsigned int, nr_ranges)
> + __field(int, err)
> + __field(int, snap_err)
> + ),
> +
> + TP_fast_assign(/* assign */
> + __entry->dev = sb->s_dev;
> + __entry->tid = commit_tid;
> + __entry->locked_ns = locked_ns;
> + __entry->nr_inodes = nr_inodes;
> + __entry->nr_ranges = nr_ranges;
> + __entry->err = err;
> + __entry->snap_err = snap_err;
> + ),
> +
> + TP_printk("dev %d,%d tid %u locked_ns %llu nr_inodes %u nr_ranges %u err %d snap_err %d",
> + MAJOR(__entry->dev), MINOR(__entry->dev), __entry->tid,
> + __entry->locked_ns, __entry->nr_inodes, __entry->nr_ranges,
> + __entry->err, __entry->snap_err)
And instead of having the raw value of __entry->snap_err, use:
__entry->err, __print_symbolic(__entry->snap_err, TRACE_SNAP_ERR))
-- Steve
> +);
> +
> #define FC_REASON_NAME_STAT(reason) \
> show_fc_reason(reason), \
> __entry->fc_ineligible_rc[reason]
1
^ permalink raw reply
* Re: [PATCH v4 2/2] mm/vmscan: add tracepoint and reason for kswapd_failures reset
From: Steven Rostedt @ 2026-01-23 17:45 UTC (permalink / raw)
To: Jiayuan Chen
Cc: linux-mm, Jiayuan Chen, Shakeel Butt, Johannes Weiner,
Andrew Morton, David Hildenbrand, Lorenzo Stoakes,
Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Axel Rasmussen, Yuanchu Xie,
Wei Xu, Masami Hiramatsu, Mathieu Desnoyers, Brendan Jackman,
Zi Yan, Qi Zheng, linux-kernel, linux-trace-kernel
In-Reply-To: <20260120024402.387576-3-jiayuan.chen@linux.dev>
On Tue, 20 Jan 2026 10:43:49 +0800
Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
> Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
> Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
> Signed-off-by: Jiayuan Chen <jiayuan.chen@shopee.com>
> Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
> ---
> include/linux/mmzone.h | 19 ++++++++++---
> include/trace/events/vmscan.h | 51 +++++++++++++++++++++++++++++++++++
> mm/memory-tiers.c | 2 +-
> mm/page_alloc.c | 4 +--
> mm/show_mem.c | 3 +--
> mm/vmscan.c | 29 +++++++++++++-------
> mm/vmstat.c | 2 +-
> 7 files changed, 91 insertions(+), 19 deletions(-)
From a tracing POV:
Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>
-- Steve
^ permalink raw reply
* Re: [PATCH v2] scripts/tracepoint-update: fix memory leak in add_string() on failure
From: Steven Rostedt @ 2026-01-23 17:42 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Weigang He, Mathieu Desnoyers, linux-kernel, linux-trace-kernel
In-Reply-To: <20260121112253.3e02bd1ebb50ad6d1651bb1f@kernel.org>
On Wed, 21 Jan 2026 11:22:53 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> >
> > Fix this by freeing *vals and setting it to NULL when realloc() fails.
> > This makes the error handling self-contained in add_string() so callers
> > don't need to handle cleanup on failure.
>
> This looks not enough. If the memory allocation is failed, it should NOT
> continue anything.
>
> I think we need to make the command itself failure when it fails to
> allocate memory, as below:
That's a separate bug. I'm taking this current patch as-is as a fix for the
leak. Not stopping immediately is a separate issue.
-- Steve
^ permalink raw reply
* Re: [PATCH v2] function_graph: Fix args pointer mismatch in print_graph_retval()
From: Steven Rostedt @ 2026-01-23 17:18 UTC (permalink / raw)
To: Donglin Peng; +Cc: mhiramat, linux-trace-kernel, linux-kernel, Donglin Peng
In-Reply-To: <20260112021601.1300479-1-dolinux.peng@gmail.com>
On Mon, 12 Jan 2026 10:16:01 +0800
Donglin Peng <dolinux.peng@gmail.com> wrote:
> From: Donglin Peng <pengdonglin@xiaomi.com>
>
> When funcgraph-args and funcgraph-retaddr are both enabled, many kernel
> functions display invalid parameters in trace logs[1].
>
> The issue occurs because print_graph_retval() passes a mismatched args
> pointer to print_function_args(). Fix this by retrieving the correct
> args pointer using the FGRAPH_ENTRY_ARGS() macro.
>
> [1] https://gitee.com/pengdonglin137/funcgraph_visualization/raw/master/samples/error_args.png
Please do not provide links to images. This stays in the git logs for the
lifetime of the Linux kernel. Images like this will likely not live as long
and the link becomes wasted space in the git history.
Either do a cut and paste of the bad output, or don't include it at all.
-- Steve
> Fixes: f83ac7544fbf ("function_graph: Enable funcgraph-args and funcgraph-retaddr to work simultaneously")
> Signed-off-by: Donglin Peng <pengdonglin@xiaomi.com>
^ permalink raw reply
* Re: [PATCH] tracing: Up the hist stacktrace size from 16 to 31
From: Steven Rostedt @ 2026-01-23 16:27 UTC (permalink / raw)
To: LKML, Linux Trace Kernel; +Cc: Masami Hiramatsu, Mathieu Desnoyers, Tom Zanussi
In-Reply-To: <20260123105415.2be26bf4@gandalf.local.home>
On Fri, 23 Jan 2026 10:54:15 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> Also change the BUILD_BUG_ON() to allow the size of the stacktrace storage
> to be equal to the max size. It already accounts for the empty slot via
> a "+ 1":
Correction. One slot is used to hold the number of elements in the stack.
-- Steve
^ permalink raw reply
* [PATCH] tracing: Up the hist stacktrace size from 16 to 31
From: Steven Rostedt @ 2026-01-23 15:54 UTC (permalink / raw)
To: LKML, Linux Trace Kernel; +Cc: Masami Hiramatsu, Mathieu Desnoyers, Tom Zanussi
From: Steven Rostedt <rostedt@goodmis.org>
Recording stacktraces is very useful, but the size of 16 deep is very
restrictive. For example, in seeing where tasks schedule out in a non
running state, the following can be used:
~# cd /sys/kernel/tracing
~# echo 'hist:keys=common_stacktrace:vals=hitcount if prev_state & 3' > events/sched/sched_switch/trigger
~# cat events/sched/sched_switch/hist
[..]
{ common_stacktrace:
__schedule+0xdc0/0x1860
schedule+0x27/0xd0
schedule_timeout+0xb5/0x100
wait_for_completion+0x8a/0x140
xfs_buf_iowait+0x20/0xd0 [xfs]
xfs_buf_read_map+0x103/0x250 [xfs]
xfs_trans_read_buf_map+0x161/0x310 [xfs]
xfs_btree_read_buf_block+0xa0/0x120 [xfs]
xfs_btree_lookup_get_block+0xa3/0x1e0 [xfs]
xfs_btree_lookup+0xea/0x530 [xfs]
xfs_alloc_fixup_trees+0x72/0x570 [xfs]
xfs_alloc_ag_vextent_size+0x67f/0x800 [xfs]
xfs_alloc_vextent_iterate_ags.constprop.0+0x52/0x230 [xfs]
xfs_alloc_vextent_start_ag+0x9d/0x1b0 [xfs]
xfs_bmap_btalloc+0x2af/0x680 [xfs]
xfs_bmapi_allocate+0xdb/0x2c0 [xfs]
} hitcount: 1
[..]
The above stops at 16 functions where knowing more would be useful. As the
allocated storage for stacks is the same for strings, and that size is 256
bytes, there is a lot of space not being used for stacktraces.
16 * 8 = 128
Up the size to 31 (it requires the last slot to be zero, so it can't be 32).
Also change the BUILD_BUG_ON() to allow the size of the stacktrace storage
to be equal to the max size. It already accounts for the empty slot via
a "+ 1":
BUILD_BUG_ON((HIST_STACKTRACE_DEPTH + 1) * sizeof(long) >= STR_VAR_LEN_MAX);
Change that from ">=" to just ">", as now they are equal.
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/trace.h | 2 +-
kernel/trace/trace_events_hist.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 8888fc9335b6..69e7defba6c6 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -128,7 +128,7 @@ enum trace_type {
#define FAULT_STRING "(fault)"
-#define HIST_STACKTRACE_DEPTH 16
+#define HIST_STACKTRACE_DEPTH 31
#define HIST_STACKTRACE_SIZE (HIST_STACKTRACE_DEPTH * sizeof(unsigned long))
#define HIST_STACKTRACE_SKIP 5
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 24441b30a1a4..dfb7c4754bf8 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -3154,7 +3154,7 @@ static inline void __update_field_vars(struct tracing_map_elt *elt,
u64 var_val;
/* Make sure stacktrace can fit in the string variable length */
- BUILD_BUG_ON((HIST_STACKTRACE_DEPTH + 1) * sizeof(long) >= STR_VAR_LEN_MAX);
+ BUILD_BUG_ON((HIST_STACKTRACE_DEPTH + 1) * sizeof(long) > STR_VAR_LEN_MAX);
for (i = 0, j = field_var_str_start; i < n_field_vars; i++) {
struct field_var *field_var = field_vars[i];
--
2.51.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox