* Re: [PATCH v10 04/30] ring-buffer: Add non-consuming read for ring-buffer remotes
From: Steven Rostedt @ 2026-01-29 0:26 UTC (permalink / raw)
To: Vincent Donnefort
Cc: mhiramat, mathieu.desnoyers, linux-trace-kernel, maz,
oliver.upton, joey.gouly, suzuki.poulose, yuzenghui, kvmarm,
linux-arm-kernel, jstultz, qperret, will, aneesh.kumar,
kernel-team, linux-kernel
In-Reply-To: <20260126104419.1649811-5-vdonnefort@google.com>
On Mon, 26 Jan 2026 10:43:53 +0000
Vincent Donnefort <vdonnefort@google.com> wrote:
> Hopefully, the remote will only swap pages on the kernel instruction (via
> the swap_reader_page() callback). This means we know at what point the
> ring-buffer geometry has changed. It is therefore possible to rearrange
> the kernel view of that ring-buffer to allow non-consuming read.
>
> Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>
-- Steve
^ permalink raw reply
* Re: [PATCH v10 03/30] ring-buffer: Introduce ring-buffer remotes
From: Steven Rostedt @ 2026-01-29 0:26 UTC (permalink / raw)
To: Vincent Donnefort
Cc: mhiramat, mathieu.desnoyers, linux-trace-kernel, maz,
oliver.upton, joey.gouly, suzuki.poulose, yuzenghui, kvmarm,
linux-arm-kernel, jstultz, qperret, will, aneesh.kumar,
kernel-team, linux-kernel
In-Reply-To: <20260126104419.1649811-4-vdonnefort@google.com>
On Mon, 26 Jan 2026 10:43:52 +0000
Vincent Donnefort <vdonnefort@google.com> wrote:
> A ring-buffer remote is an entity outside of the kernel (most likely a
> firmware or a hypervisor) capable of writing events in a ring-buffer
> following the same format as the tracefs ring-buffer.
>
> To setup the ring-buffer on the kernel side, a description of the pages
> forming the ring-buffer (struct trace_buffer_desc) must be given.
> Callbacks (swap_reader_page and reset) must also be provided.
>
> It is expected from the remote to keep the meta-page updated.
>
> Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>
-- Steve
^ permalink raw reply
* Re: [PATCH v10 02/30] ring-buffer: Store bpage pointers into subbuf_ids
From: Steven Rostedt @ 2026-01-29 0:26 UTC (permalink / raw)
To: Vincent Donnefort
Cc: mhiramat, mathieu.desnoyers, linux-trace-kernel, maz,
oliver.upton, joey.gouly, suzuki.poulose, yuzenghui, kvmarm,
linux-arm-kernel, jstultz, qperret, will, aneesh.kumar,
kernel-team, linux-kernel
In-Reply-To: <20260126104419.1649811-3-vdonnefort@google.com>
On Mon, 26 Jan 2026 10:43:51 +0000
Vincent Donnefort <vdonnefort@google.com> wrote:
> The subbuf_ids field allows to point to a specific page from the
> ring-buffer based on its ID. As a preparation or the upcoming
> ring-buffer remote support, point this array to the buffer_page instead
> of the buffer_data_page.
>
> Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>
-- Steve
^ permalink raw reply
* Re: [PATCH v10 01/30] ring-buffer: Add page statistics to the meta-page
From: Steven Rostedt @ 2026-01-29 0:25 UTC (permalink / raw)
To: Vincent Donnefort
Cc: mhiramat, mathieu.desnoyers, linux-trace-kernel, maz,
oliver.upton, joey.gouly, suzuki.poulose, yuzenghui, kvmarm,
linux-arm-kernel, jstultz, qperret, will, aneesh.kumar,
kernel-team, linux-kernel
In-Reply-To: <20260126104419.1649811-2-vdonnefort@google.com>
On Mon, 26 Jan 2026 10:43:50 +0000
Vincent Donnefort <vdonnefort@google.com> wrote:
> Add two fields pages_touched and pages_lost to the ring-buffer
> meta-page. Those fields are useful to get the number of used pages in
> the ring-buffer.
>
> Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>
-- Steve
^ permalink raw reply
* Re: [PATCH 2/2] bpf: Implement kretprobe fallback for kprobe multi link
From: Masami Hiramatsu @ 2026-01-29 0:13 UTC (permalink / raw)
To: bot+bpf-ci
Cc: liujing.root, ast, daniel, andrii, martin.lau, eddyz87, song,
yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, bpf,
linux-kernel, linux-trace-kernel, liujing40, martin.lau, clm,
ihor.solodrai
In-Reply-To: <4ee5b08b6cebfc87b552a0fde1f09fd5a2c190813760aa7e68f1e6d060f15ccd@mail.kernel.org>
On Thu, 18 Dec 2025 13:33:31 +0000 (UTC)
bot+bpf-ci@kernel.org wrote:
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index 1fd07c10378f..426a1c627508 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
>
> [ ... ]
>
> > +int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
> > +{
> > + struct bpf_kprobe_multi_link *link = NULL;
> > + struct bpf_link_primer link_primer;
> > + struct bpf_kprobe *kprobes = NULL;
> > + u32 flags, cnt;
> > + u64 __user *ucookies;
> > + unsigned long __user *uaddrs;
> > + unsigned long __user *usyms;
> > + int err, i;
>
> [ ... ]
>
> > + for (i = 0; i < cnt; i++) {
> > + unsigned long addr;
> > +
> > + if (uaddrs) {
> > + if (__get_user(addr, uaddrs + i)) {
> > + err = -EFAULT;
> > + goto error;
> > + }
> > + } else {
> > + unsigned long __user usymbol;
> > + char buf[KSYM_NAME_LEN];
> > +
> > + if (__get_user(usymbol, usyms + i)) {
> > + err = -EFAULT;
> > + goto error;
> > + }
> > + err = strncpy_from_user(buf, (const char __user *) usymbol, KSYM_NAME_LEN);
> > + if (err == KSYM_NAME_LEN)
> > + err = -E2BIG;
> > + if (err < 0)
> > + goto error;
> > +
> > + addr = kallsyms_lookup_name(buf);
> > + if (!addr)
> > + goto error;
>
> When kallsyms_lookup_name() returns 0 (symbol not found), is err being set
> to an error code before jumping to the error label? It looks like err will
> still hold the positive return value from strncpy_from_user() (the string
> length), which would be returned to the caller as a success indication.
Good catch. It should set err = -ENOENT.
Thanks,
>
> The error label at the end of the function does:
>
> error:
> kvfree(kprobes);
> kfree(link);
> return err;
>
> This could cause the syscall to return a positive value to userspace instead
> of an error, and also leak a prog reference since link_create() only calls
> bpf_prog_put() when ret < 0.
>
> > + }
> > + if (prog->kprobe_override && !within_error_injection_list(addr)) {
> > + err = -EINVAL;
> > + goto error;
> > + }
>
> [ ... ]
>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/20338242683
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH v2] ring-buffer: Add helper functions for allocations
From: Masami Hiramatsu @ 2026-01-28 23:47 UTC (permalink / raw)
To: Steven Rostedt
Cc: LKML, Linux Trace Kernel, Masami Hiramatsu, Mathieu Desnoyers,
Linus Torvalds
In-Reply-To: <20251125121153.35c07461@gandalf.local.home>
On Tue, 25 Nov 2025 12:11:53 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
>
> The allocation of the per CPU buffer descriptor, the buffer page
> descriptors and the buffer page data itself can be pretty ugly:
>
> kzalloc_node(ALIGN(sizeof(struct buffer_page), cache_line_size()),
> GFP_KERNEL, cpu_to_node(cpu));
>
> And the data pages:
>
> page = alloc_pages_node(cpu_to_node(cpu),
> GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_COMP | __GFP_ZERO, order);
> if (!page)
> return NULL;
> bpage->page = page_address(page);
> rb_init_page(bpage->page);
>
> Add helper functions to make the code easier to read.
>
> This does make all allocations of the data page (bpage->page) allocated
> with the __GFP_RETRY_MAYFAIL flag (and not just the bulk allocator). Which
> is actually better, as allocating the data page for the ring buffer tracing
> should try hard but not trigger the OOM killer.
>
> Link: https://lore.kernel.org/all/CAHk-=wjMMSAaqTjBSfYenfuzE1bMjLj+2DLtLWJuGt07UGCH_Q@mail.gmail.com/
>
> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Looks good to me.
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Thanks,
> ---
> Changes since v1: https://patch.msgid.link/20251124140906.71a2abf6@gandalf.local.home
>
> - Removed set but unused variables (kernel test robot)
>
> kernel/trace/ring_buffer.c | 97 +++++++++++++++++++++-----------------
> 1 file changed, 53 insertions(+), 44 deletions(-)
>
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 1244d2c5c384..6295443b0944 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -401,6 +401,41 @@ static void free_buffer_page(struct buffer_page *bpage)
> kfree(bpage);
> }
>
> +/*
> + * For best performance, allocate cpu buffer data cache line sized
> + * and per CPU.
> + */
> +#define alloc_cpu_buffer(cpu) (struct ring_buffer_per_cpu *) \
> + kzalloc_node(ALIGN(sizeof(struct ring_buffer_per_cpu), \
> + cache_line_size()), GFP_KERNEL, cpu_to_node(cpu));
> +
> +#define alloc_cpu_page(cpu) (struct buffer_page *) \
> + kzalloc_node(ALIGN(sizeof(struct buffer_page), \
> + cache_line_size()), GFP_KERNEL, cpu_to_node(cpu));
> +
> +static struct buffer_data_page *alloc_cpu_data(int cpu, int order)
> +{
> + struct buffer_data_page *dpage;
> + struct page *page;
> + gfp_t mflags;
> +
> + /*
> + * __GFP_RETRY_MAYFAIL flag makes sure that the allocation fails
> + * gracefully without invoking oom-killer and the system is not
> + * destabilized.
> + */
> + mflags = GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_COMP | __GFP_ZERO;
> +
> + page = alloc_pages_node(cpu_to_node(cpu), mflags, order);
> + if (!page)
> + return NULL;
> +
> + dpage = page_address(page);
> + rb_init_page(dpage);
> +
> + return dpage;
> +}
> +
> /*
> * We need to fit the time_stamp delta into 27 bits.
> */
> @@ -2204,7 +2239,6 @@ static int __rb_allocate_pages(struct ring_buffer_per_cpu *cpu_buffer,
> struct ring_buffer_cpu_meta *meta = NULL;
> struct buffer_page *bpage, *tmp;
> bool user_thread = current->mm != NULL;
> - gfp_t mflags;
> long i;
>
> /*
> @@ -2218,13 +2252,6 @@ static int __rb_allocate_pages(struct ring_buffer_per_cpu *cpu_buffer,
> if (i < nr_pages)
> return -ENOMEM;
>
> - /*
> - * __GFP_RETRY_MAYFAIL flag makes sure that the allocation fails
> - * gracefully without invoking oom-killer and the system is not
> - * destabilized.
> - */
> - mflags = GFP_KERNEL | __GFP_RETRY_MAYFAIL;
> -
> /*
> * If a user thread allocates too much, and si_mem_available()
> * reports there's enough memory, even though there is not.
> @@ -2241,10 +2268,8 @@ static int __rb_allocate_pages(struct ring_buffer_per_cpu *cpu_buffer,
> meta = rb_range_meta(buffer, nr_pages, cpu_buffer->cpu);
>
> for (i = 0; i < nr_pages; i++) {
> - struct page *page;
>
> - bpage = kzalloc_node(ALIGN(sizeof(*bpage), cache_line_size()),
> - mflags, cpu_to_node(cpu_buffer->cpu));
> + bpage = alloc_cpu_page(cpu_buffer->cpu);
> if (!bpage)
> goto free_pages;
>
> @@ -2267,13 +2292,10 @@ static int __rb_allocate_pages(struct ring_buffer_per_cpu *cpu_buffer,
> bpage->range = 1;
> bpage->id = i + 1;
> } else {
> - page = alloc_pages_node(cpu_to_node(cpu_buffer->cpu),
> - mflags | __GFP_COMP | __GFP_ZERO,
> - cpu_buffer->buffer->subbuf_order);
> - if (!page)
> + int order = cpu_buffer->buffer->subbuf_order;
> + bpage->page = alloc_cpu_data(cpu_buffer->cpu, order);
> + if (!bpage->page)
> goto free_pages;
> - bpage->page = page_address(page);
> - rb_init_page(bpage->page);
> }
> bpage->order = cpu_buffer->buffer->subbuf_order;
>
> @@ -2324,14 +2346,12 @@ static int rb_allocate_pages(struct ring_buffer_per_cpu *cpu_buffer,
> static struct ring_buffer_per_cpu *
> rb_allocate_cpu_buffer(struct trace_buffer *buffer, long nr_pages, int cpu)
> {
> - struct ring_buffer_per_cpu *cpu_buffer __free(kfree) = NULL;
> + struct ring_buffer_per_cpu *cpu_buffer __free(kfree) =
> + alloc_cpu_buffer(cpu);
> struct ring_buffer_cpu_meta *meta;
> struct buffer_page *bpage;
> - struct page *page;
> int ret;
>
> - cpu_buffer = kzalloc_node(ALIGN(sizeof(*cpu_buffer), cache_line_size()),
> - GFP_KERNEL, cpu_to_node(cpu));
> if (!cpu_buffer)
> return NULL;
>
> @@ -2347,8 +2367,7 @@ rb_allocate_cpu_buffer(struct trace_buffer *buffer, long nr_pages, int cpu)
> init_waitqueue_head(&cpu_buffer->irq_work.full_waiters);
> mutex_init(&cpu_buffer->mapping_lock);
>
> - bpage = kzalloc_node(ALIGN(sizeof(*bpage), cache_line_size()),
> - GFP_KERNEL, cpu_to_node(cpu));
> + bpage = alloc_cpu_page(cpu);
> if (!bpage)
> return NULL;
>
> @@ -2370,13 +2389,10 @@ rb_allocate_cpu_buffer(struct trace_buffer *buffer, long nr_pages, int cpu)
> rb_meta_buffer_update(cpu_buffer, bpage);
> bpage->range = 1;
> } else {
> - page = alloc_pages_node(cpu_to_node(cpu),
> - GFP_KERNEL | __GFP_COMP | __GFP_ZERO,
> - cpu_buffer->buffer->subbuf_order);
> - if (!page)
> + int order = cpu_buffer->buffer->subbuf_order;
> + bpage->page = alloc_cpu_data(cpu, order);
> + if (!bpage->page)
> goto fail_free_reader;
> - bpage->page = page_address(page);
> - rb_init_page(bpage->page);
> }
>
> INIT_LIST_HEAD(&cpu_buffer->reader_page->list);
> @@ -6464,7 +6480,6 @@ ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu)
> struct ring_buffer_per_cpu *cpu_buffer;
> struct buffer_data_read_page *bpage = NULL;
> unsigned long flags;
> - struct page *page;
>
> if (!cpumask_test_cpu(cpu, buffer->cpumask))
> return ERR_PTR(-ENODEV);
> @@ -6486,22 +6501,16 @@ ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu)
> arch_spin_unlock(&cpu_buffer->lock);
> local_irq_restore(flags);
>
> - if (bpage->data)
> - goto out;
> -
> - page = alloc_pages_node(cpu_to_node(cpu),
> - GFP_KERNEL | __GFP_NORETRY | __GFP_COMP | __GFP_ZERO,
> - cpu_buffer->buffer->subbuf_order);
> - if (!page) {
> - kfree(bpage);
> - return ERR_PTR(-ENOMEM);
> + if (bpage->data) {
> + rb_init_page(bpage->data);
> + } else {
> + bpage->data = alloc_cpu_data(cpu, cpu_buffer->buffer->subbuf_order);
> + if (!bpage->data) {
> + kfree(bpage);
> + return ERR_PTR(-ENOMEM);
> + }
> }
>
> - bpage->data = page_address(page);
> -
> - out:
> - rb_init_page(bpage->data);
> -
> return bpage;
> }
> EXPORT_SYMBOL_GPL(ring_buffer_alloc_read_page);
> --
> 2.51.0
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH v4 0/5] Tracing: Accelerate Kernel Boot by Asynchronizing
From: Masami Hiramatsu @ 2026-01-28 23:38 UTC (permalink / raw)
To: Yaxiong Tian
Cc: rostedt, axboe, mathieu.desnoyers, corbet, skhan,
linux-trace-kernel, linux-block, linux-kernel, linux-doc
In-Reply-To: <20260128125117.1704853-1-tianyaxiong@kylinos.cn>
On Wed, 28 Jan 2026 20:51:17 +0800
Yaxiong Tian <tianyaxiong@kylinos.cn> wrote:
> 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. Also in
> init_blk_tracer(), this functions require trace_event_sem device_initcall.
>
> To resolve this issue, I rename `eval_map_wq` and make it global 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.
>
> Given that asynchronous initialization makes it indeterminate when tracing
> will begin, we introduce the trace_async_init kernel parameter.Asynchronous
> behavior is enabled only when this parameter is explicitly provided.
>
> Based on my analysis and testing, I've identified that only these two
> locations significantly impact timing. Other initcall_* functions do not
> exhibit relevant lock contention.
>
> A brief summary of the test results is as follows:
> Before this PATCHS:
> [ 0.224933] calling init_kprobe_trace+0x0/0xe0 @ 1
> [ 0.455016] initcall init_kprobe_trace+0x0/0xe0 returned 0 after 230080 usecs
>
> Only opt setup_boot_kprobe_events() can see:
> [ 0.258609] calling init_blk_tracer+0x0/0x68 @ 1
> [ 0.454991] initcall init_blk_tracer+0x0/0x68 returned 0 after 196377 usecs
>
> After this PATCHS:
> [ 0.224940] calling init_kprobe_trace+0x0/0xe0 @ 1
> [ 0.224946] initcall init_kprobe_trace+0x0/0xe0 returned 0 after 3 usecs
> skip --------
> [ 0.264835] calling init_blk_tracer+0x0/0x68 @ 1
> [ 0.264841] initcall init_blk_tracer+0x0/0x68 returned 0 after 2 usecs
OK, this series looks good to me.
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
for this series.
Thank you,
>
> ---
> Changes in v2:
> - Rename eval_map_wq to trace_init_wq.
> Changes in v3:
> - Opt PATCH 1/3 commit
> Changes in v4:
> - add trace_async_init boot parameter in patch2
> - add init_kprobe_trace's skip logic in patch3
> - add Suggested-by tag
> - Other synchronous optimizations related to trace_async_init
>
> Yaxiong Tian (5):
> tracing: Rename `eval_map_wq` and allow other parts of tracing use it
> tracing: add trace_async_init boot parameter
> tracing/kprobes: Skip setup_boot_kprobe_events() when no cmdline event
> tracing/kprobes: Make setup_boot_kprobe_events() asynchronous when
> trace_async_init set
> blktrace: Make init_blk_tracer() asynchronous when trace_async_init
> set
>
> .../admin-guide/kernel-parameters.txt | 8 ++++++
> kernel/trace/blktrace.c | 23 +++++++++++++++-
> kernel/trace/trace.c | 27 ++++++++++++-------
> kernel/trace/trace.h | 2 ++
> kernel/trace/trace_kprobe.c | 18 ++++++++++++-
> 5 files changed, 67 insertions(+), 11 deletions(-)
>
> --
> 2.25.1
>
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH v10 13/30] tracing: Introduce simple_ring_buffer
From: Steven Rostedt @ 2026-01-28 23:18 UTC (permalink / raw)
To: Vincent Donnefort
Cc: mhiramat, mathieu.desnoyers, linux-trace-kernel, maz,
oliver.upton, joey.gouly, suzuki.poulose, yuzenghui, kvmarm,
linux-arm-kernel, jstultz, qperret, will, aneesh.kumar,
kernel-team, linux-kernel
In-Reply-To: <20260126104419.1649811-14-vdonnefort@google.com>
On Mon, 26 Jan 2026 10:44:02 +0000
Vincent Donnefort <vdonnefort@google.com> wrote:
> +
> +/**
> + * simple_ring_buffer_init - Init @cpu_buffer based on @desc
> + *
> + * @cpu_buffer: A simple_rb_per_cpu buffer to init, allocated by the caller.
> + * @bpages: Array of simple_buffer_pages, with as many elements as @desc->nr_page_va
> + * @desc: A ring_buffer_desc
> + *
> + * Returns: 0 on success or -EINVAL if the content of @desc is invalid
> + */
Again, kerneldoc goes with the function and not the prototype.
Especially helps with review, as I like to see the kerneldoc what reading
the function.
-- Steve
> +int simple_ring_buffer_init(struct simple_rb_per_cpu *cpu_buffer, struct simple_buffer_page *bpages,
> + const struct ring_buffer_desc *desc);
> +
> +/**
> + * simple_ring_buffer_unload - Prepare @cpu_buffer for deletion
> + *
> + * @cpu_buffer: A simple_rb_per_cpu that will be deleted.
> + */
> +void simple_ring_buffer_unload(struct simple_rb_per_cpu *cpu_buffer);
> +
> +/**
> + * simple_ring_buffer_reserve - Reserve an entry in @cpu_buffer
> + *
> + * @cpu_buffer: A simple_rb_per_cpu
> + * @length: Size of the entry in bytes
> + * @timestamp: Timestamp of the entry
> + *
> + * Returns the address of the entry where to write data or NULL
> + */
> +void *simple_ring_buffer_reserve(struct simple_rb_per_cpu *cpu_buffer, unsigned long length,
> + u64 timestamp);
> +
> +/**
> + * simple_ring_buffer_commit - Commit the entry reserved with simple_ring_buffer_reserve()
> + *
> + * @cpu_buffer: The simple_rb_per_cpu where the entry has been reserved
> + */
> +void simple_ring_buffer_commit(struct simple_rb_per_cpu *cpu_buffer);
> +
> +/**
> + * simple_ring_buffer_enable_tracing - Enable or disable writing to @cpu_buffer
> + *
> + * @cpu_buffer: A simple_rb_per_cpu
> + * @enable: True to enable tracing, False to disable it
> + *
> + * Returns 0 on success or -ENODEV if @cpu_buffer was unloaded
> + */
> +int simple_ring_buffer_enable_tracing(struct simple_rb_per_cpu *cpu_buffer, bool enable);
> +
> +/**
> + * simple_ring_buffer_reset - Reset @cpu_buffer
> + *
> + * @cpu_buffer: A simple_rb_per_cpu
> + *
> + * This will not clear the content of the data, only reset counters and pointers
> + *
> + * Returns 0 on success or -ENODEV if @cpu_buffer was unloaded.
> + */
> +int simple_ring_buffer_reset(struct simple_rb_per_cpu *cpu_buffer);
> +
> +/**
> + * simple_ring_buffer_swap_reader_page - Swap ring-buffer head with the reader
> + *
> + * This function enables consuming reading. It ensures the current head page will not be overwritten
> + * and can be safely read.
> + *
> + * @cpu_buffer: A simple_rb_per_cpu
> + *
> + * Returns 0 on success, -ENODEV if @cpu_buffer was unloaded or -EBUSY if we failed to catch the
> + * head page.
> + */
> +int simple_ring_buffer_swap_reader_page(struct simple_rb_per_cpu *cpu_buffer);
> +
> +#endif
^ permalink raw reply
* Re: [PATCH v10 09/30] tracing: Add events to trace remotes
From: Steven Rostedt @ 2026-01-28 23:06 UTC (permalink / raw)
To: Vincent Donnefort
Cc: mhiramat, mathieu.desnoyers, linux-trace-kernel, maz,
oliver.upton, joey.gouly, suzuki.poulose, yuzenghui, kvmarm,
linux-arm-kernel, jstultz, qperret, will, aneesh.kumar,
kernel-team, linux-kernel
In-Reply-To: <20260126104419.1649811-10-vdonnefort@google.com>
On Mon, 26 Jan 2026 10:43:58 +0000
Vincent Donnefort <vdonnefort@google.com> wrote:
> +
> +static int __cmp_events(const void *id, const void *evt)
> +{
> + return (long)id - ((struct remote_event *)evt)->id;
Hmm, just to be safe, we should probably make everything int here, to not
have any overflow issues:
static int __cmp_events(const void *key, const void *data)
{
const struct remote_event *evt = data;
int id = (int)((long)key);
return id - (int)evt->id;
}
-- Steve
> +}
> +
> +static struct remote_event *trace_remote_find_event(struct trace_remote *remote, unsigned short id)
> +{
> + return bsearch((const void *)(unsigned long)id, remote->events, remote->nr_events,
> + sizeof(*remote->events), __cmp_events);
> +}
^ permalink raw reply
* Re: [PATCH v10 07/30] tracing: Add non-consuming read to trace remotes
From: Steven Rostedt @ 2026-01-28 22:52 UTC (permalink / raw)
To: Vincent Donnefort
Cc: mhiramat, mathieu.desnoyers, linux-trace-kernel, maz,
oliver.upton, joey.gouly, suzuki.poulose, yuzenghui, kvmarm,
linux-arm-kernel, jstultz, qperret, will, aneesh.kumar,
kernel-team, linux-kernel
In-Reply-To: <20260126104419.1649811-8-vdonnefort@google.com>
On Mon, 26 Jan 2026 10:43:56 +0000
Vincent Donnefort <vdonnefort@google.com> wrote:
> Allow reading the trace file for trace remotes. This performs a
> non-consuming read of the trace buffer.
>
> Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
>
> diff --git a/kernel/trace/trace_remote.c b/kernel/trace/trace_remote.c
> index 49c4ae127533..a744bbf48e88 100644
> --- a/kernel/trace/trace_remote.c
> +++ b/kernel/trace/trace_remote.c
> @@ -18,14 +18,25 @@
> #define TRACEFS_MODE_WRITE 0640
> #define TRACEFS_MODE_READ 0440
>
> +enum tri_type {
> + TRI_CONSUMING,
> + TRI_NONCONSUMING,
> +};
> +
> struct trace_remote_iterator {
> struct trace_remote *remote;
> struct trace_seq seq;
> struct delayed_work poll_work;
> unsigned long lost_events;
> u64 ts;
> + union {
> + struct ring_buffer_iter **rb_iters;
> + struct ring_buffer_iter *rb_iter;
I don't care for the union, it can be error prone and for what? 8 bytes?
It's not a fast path and the memory is temporary.
Just make two entries, where one is NULL. That way if there's a mistake and
the wrong one is used it will be pretty obvious that it gets a NULL pointer
dereference than some random error.
> + };
> int cpu;
> int evt_cpu;
> + loff_t pos;
> + enum tri_type type;
> };
>
> struct trace_remote {
> @@ -36,6 +47,8 @@ struct trace_remote {
> unsigned long trace_buffer_size;
> struct ring_buffer_remote rb_remote;
> struct mutex lock;
> + struct rw_semaphore reader_lock;
> + struct rw_semaphore *pcpu_reader_locks;
> unsigned int nr_readers;
> unsigned int poll_ms;
> bool tracing_on;
> @@ -225,6 +238,20 @@ static int trace_remote_get(struct trace_remote *remote, int cpu)
> if (ret)
> return ret;
>
> + if (cpu != RING_BUFFER_ALL_CPUS && !remote->pcpu_reader_locks) {
> + int lock_cpu;
> +
> + remote->pcpu_reader_locks = kcalloc(nr_cpu_ids, sizeof(*remote->pcpu_reader_locks),
> + GFP_KERNEL);
> + if (!remote->pcpu_reader_locks) {
> + trace_remote_try_unload(remote);
> + return -ENOMEM;
> + }
> +
> + for_each_possible_cpu(lock_cpu)
> + init_rwsem(&remote->pcpu_reader_locks[lock_cpu]);
> + }
> +
> remote->nr_readers++;
>
> return 0;
> @@ -239,6 +266,9 @@ static void trace_remote_put(struct trace_remote *remote)
> if (remote->nr_readers)
> return;
>
> + kfree(remote->pcpu_reader_locks);
> + remote->pcpu_reader_locks = NULL;
> +
> trace_remote_try_unload(remote);
> }
>
> @@ -253,6 +283,48 @@ static void __poll_remote(struct work_struct *work)
> msecs_to_jiffies(iter->remote->poll_ms));
> }
>
> +static int __alloc_ring_buffer_iter(struct trace_remote_iterator *iter, int cpu)
> +{
> + bool once = false;
> +
> + if (cpu != RING_BUFFER_ALL_CPUS) {
> + iter->rb_iter = ring_buffer_read_start(iter->remote->trace_buffer, cpu, GFP_KERNEL);
> +
> + return iter->rb_iter ? 0 : -ENOMEM;
> + }
> +
> + iter->rb_iters = kcalloc(nr_cpu_ids, sizeof(*iter->rb_iters), GFP_KERNEL);
> + if (!iter->rb_iters)
> + return -ENOMEM;
> +
> + for_each_possible_cpu(cpu) {
> + iter->rb_iters[cpu] = ring_buffer_read_start(iter->remote->trace_buffer, cpu,
> + GFP_KERNEL);
> + if (iter->rb_iters[cpu])
> + once = true;
Do we really want to succeed if only one cpu passes?
> + }
> +
> + return once ? 0 : -ENOMEM;
> +}
> +
> +static void __free_ring_buffer_iter(struct trace_remote_iterator *iter, int cpu)
> +{
> + if (!iter->rb_iter)
> + return;
> +
> + if (cpu != RING_BUFFER_ALL_CPUS) {
> + ring_buffer_read_finish(iter->rb_iter);
> + return;
> + }
> +
> + for_each_possible_cpu(cpu) {
> + if (iter->rb_iters[cpu])
> + ring_buffer_read_finish(iter->rb_iters[cpu]);
> + }
> +
> + kfree(iter->rb_iters);
> +}
> +
> static struct trace_remote_iterator
> *trace_remote_iter(struct trace_remote *remote, int cpu, enum tri_type type)
> {
> @@ -261,6 +333,8 @@ static struct trace_remote_iterator
>
> lockdep_assert_held(&remote->lock);
>
> + if (type == TRI_NONCONSUMING && !trace_remote_loaded(remote))
> + return NULL;
>
> ret = trace_remote_get(remote, cpu);
> if (ret)
> @@ -275,9 +349,21 @@ static struct trace_remote_iterator
> if (iter) {
> iter->remote = remote;
> iter->cpu = cpu;
> + iter->type = type;
> trace_seq_init(&iter->seq);
> - INIT_DELAYED_WORK(&iter->poll_work, __poll_remote);
> - schedule_delayed_work(&iter->poll_work, msecs_to_jiffies(remote->poll_ms));
> +
> + switch (type) {
> + case TRI_CONSUMING:
> + INIT_DELAYED_WORK(&iter->poll_work, __poll_remote);
> + schedule_delayed_work(&iter->poll_work, msecs_to_jiffies(remote->poll_ms));
> + break;
> + case TRI_NONCONSUMING:
> + ret = __alloc_ring_buffer_iter(iter, cpu);
> + break;
> + }
> +
> + if (ret)
> + goto err;
>
> return iter;
> }
> @@ -301,10 +387,100 @@ static void trace_remote_iter_free(struct trace_remote_iterator *iter)
>
> lockdep_assert_held(&remote->lock);
>
> + switch (iter->type) {
> + case TRI_CONSUMING:
> + cancel_delayed_work_sync(&iter->poll_work);
> + break;
> + case TRI_NONCONSUMING:
> + __free_ring_buffer_iter(iter, iter->cpu);
> + break;
> + }
> +
> kfree(iter);
> trace_remote_put(remote);
> }
>
> +static void trace_remote_iter_read_start(struct trace_remote_iterator *iter)
> +{
> + struct trace_remote *remote = iter->remote;
> + int cpu = iter->cpu;
> +
> + /* Acquire global reader lock */
> + if (cpu == RING_BUFFER_ALL_CPUS && iter->type == TRI_CONSUMING)
> + down_write(&remote->reader_lock);
> + else
> + down_read(&remote->reader_lock);
> +
> + if (cpu == RING_BUFFER_ALL_CPUS)
> + return;
> +
> + /*
> + * No need for the remote lock here, iter holds a reference on
> + * remote->nr_readers
> + */
> +
> + /* Get the per-CPU one */
> + if (WARN_ON_ONCE(!remote->pcpu_reader_locks))
> + return;
> +
> + if (iter->type == TRI_CONSUMING)
> + down_write(&remote->pcpu_reader_locks[cpu]);
> + else
> + down_read(&remote->pcpu_reader_locks[cpu]);
> +}
> +
> +static void trace_remote_iter_read_finished(struct trace_remote_iterator *iter)
> +{
> + struct trace_remote *remote = iter->remote;
> + int cpu = iter->cpu;
> +
> + /* Release per-CPU reader lock */
> + if (cpu != RING_BUFFER_ALL_CPUS) {
> + /*
> + * No need for the remote lock here, iter holds a reference on
> + * remote->nr_readers
> + */
> + if (iter->type == TRI_CONSUMING)
> + up_write(&remote->pcpu_reader_locks[cpu]);
> + else
> + up_read(&remote->pcpu_reader_locks[cpu]);
> + }
> +
> + /* Release global reader lock */
> + if (cpu == RING_BUFFER_ALL_CPUS && iter->type == TRI_CONSUMING)
> + up_write(&remote->reader_lock);
> + else
> + up_read(&remote->reader_lock);
> +}
> +
> +static struct ring_buffer_iter *__get_rb_iter(struct trace_remote_iterator *iter, int cpu)
> +{
> + return iter->cpu != RING_BUFFER_ALL_CPUS ? iter->rb_iter : iter->rb_iters[cpu];
> +}
> +
> +static struct ring_buffer_event *
> +__peek_event(struct trace_remote_iterator *iter, int cpu, u64 *ts, unsigned long *lost_events)
> +{
> + struct ring_buffer_event *rb_evt;
> + struct ring_buffer_iter *rb_iter;
> +
> + switch (iter->type) {
> + case TRI_CONSUMING:
> + return ring_buffer_peek(iter->remote->trace_buffer, cpu, ts, lost_events);
> + case TRI_NONCONSUMING:
> + rb_iter = __get_rb_iter(iter, cpu);
> + rb_evt = ring_buffer_iter_peek(rb_iter, ts);
> + if (!rb_evt)
> + return NULL;
> +
> + *lost_events = ring_buffer_iter_dropped(rb_iter);
> +
> + return rb_evt;
> + }
> +
> + return NULL;
> +}
> +
> static bool trace_remote_iter_read_event(struct trace_remote_iterator *iter)
> {
> struct trace_buffer *trace_buffer = iter->remote->trace_buffer;
> @@ -314,7 +490,7 @@ static bool trace_remote_iter_read_event(struct trace_remote_iterator *iter)
> if (ring_buffer_empty_cpu(trace_buffer, cpu))
> return false;
>
> - if (!ring_buffer_peek(trace_buffer, cpu, &iter->ts, &iter->lost_events))
> + if (!__peek_event(iter, cpu, &iter->ts, &iter->lost_events))
> return false;
>
> iter->evt_cpu = cpu;
> @@ -329,7 +505,7 @@ static bool trace_remote_iter_read_event(struct trace_remote_iterator *iter)
> if (ring_buffer_empty_cpu(trace_buffer, cpu))
> continue;
>
> - if (!ring_buffer_peek(trace_buffer, cpu, &ts, &lost_events))
> + if (!__peek_event(iter, cpu, &ts, &lost_events))
> continue;
>
> if (ts >= iter->ts)
> @@ -343,7 +519,21 @@ static bool trace_remote_iter_read_event(struct trace_remote_iterator *iter)
> return iter->ts != U64_MAX;
> }
>
> -static int trace_remote_iter_print(struct trace_remote_iterator *iter)
> +static void trace_remote_iter_move(struct trace_remote_iterator *iter)
> +{
> + struct trace_buffer *trace_buffer = iter->remote->trace_buffer;
> +
> + switch (iter->type) {
> + case TRI_CONSUMING:
> + ring_buffer_consume(trace_buffer, iter->evt_cpu, NULL, NULL);
> + break;
> + case TRI_NONCONSUMING:
> + ring_buffer_iter_advance(__get_rb_iter(iter, iter->evt_cpu));
> + break;
> + }
> +}
> +
> +static int trace_remote_iter_print_event(struct trace_remote_iterator *iter)
> {
> unsigned long usecs_rem;
> u64 ts = iter->ts;
> @@ -371,7 +561,11 @@ static int trace_pipe_open(struct inode *inode, struct file *filp)
> cpu = (long)inode->i_cdev - 1;
>
> guard(mutex)(&remote->lock);
> - iter = trace_remote_iter(remote, cpu);
> +
> + iter = trace_remote_iter(remote, cpu, TRI_CONSUMING);
> + if (IS_ERR(iter))
> + return PTR_ERR(iter);
> +
> filp->private_data = iter;
>
> return IS_ERR(iter) ? PTR_ERR(iter) : 0;
> @@ -406,6 +600,8 @@ static ssize_t trace_pipe_read(struct file *filp, char __user *ubuf, size_t cnt,
> if (ret < 0)
> return ret;
>
> + trace_remote_iter_read_start(iter);
> +
> while (trace_remote_iter_read_event(iter)) {
> int prev_len = iter->seq.seq.len;
>
> @@ -414,9 +610,11 @@ static ssize_t trace_pipe_read(struct file *filp, char __user *ubuf, size_t cnt,
> break;
> }
>
> - ring_buffer_consume(trace_buffer, iter->evt_cpu, NULL, NULL);
> + trace_remote_iter_move(iter);
> }
>
> + trace_remote_iter_read_finished(iter);
> +
> goto copy_to_user;
> }
>
> @@ -426,6 +624,119 @@ static const struct file_operations trace_pipe_fops = {
> .release = trace_pipe_release,
> };
>
> +static void *trace_seq_start(struct seq_file *m, loff_t *pos)
Don't call these "trace_seq_*", as it's confusing as functions that start
with "trace_seq_*" are to be used as API for struct trace_seq instances.
They're static functions, call them s_start() or whatever ;-)
> +{
> + struct trace_remote_iterator *iter = m->private;
> + loff_t i = *pos;
> +
> + if (!iter)
> + return NULL;
> +
> + if (iter->pos <= *pos) {
> + do {
> + if (!trace_remote_iter_read_event(iter))
> + return NULL;
> +
> + trace_remote_iter_move(iter);
> + iter->pos++;
> + } while (i--);
> + }
> +
> + return iter;
> +}
> +
> +static void *trace_seq_next(struct seq_file *m, void *v, loff_t *pos)
> +{
> + struct trace_remote_iterator *iter = m->private;
> +
> + ++*pos;
> +
> + if (!iter || !trace_remote_iter_read_event(iter))
> + return NULL;
> +
> + trace_remote_iter_move(iter);
> + iter->pos++;
> +
> + return iter;
> +}
BTW, I usually use the next function to increment the start function so
there's not duplicate code.
static void *tri_start(struct seq_file *m, loff_t *pos)
{
struct trace_remote_iterator *iter = m->private;
loff_t i = *pos;
if (!iter)
return NULL;
if (iter->pos <= *pos) {
do {
iter = tri_next(m, v, pos);
if (!iter)
return NULL;
} while (i--);
}
return iter;
}
> +
> +static int trace_seq_show(struct seq_file *m, void *v)
> +{
> + struct trace_remote_iterator *iter = v;
> +
> + trace_seq_init(&iter->seq);
> +
> + if (trace_remote_iter_print_event(iter)) {
> + seq_printf(m, "[EVENT %d PRINT TOO BIG]\n", iter->evt->id);
> + return 0;
> + }
> +
> + return trace_print_seq(m, &iter->seq);
> +}
> +
> +static void trace_seq_stop(struct seq_file *s, void *v) { }
> +
> +static const struct seq_operations trace_seq_ops = {
> + .start = trace_seq_start,
> + .next = trace_seq_next,
> + .show = trace_seq_show,
> + .stop = trace_seq_stop,
> +};
> +
> +static int trace_open(struct inode *inode, struct file *filp)
> +{
> + struct trace_remote *remote = inode->i_private;
> + struct trace_remote_iterator *iter = NULL;
> + int cpu = RING_BUFFER_ALL_CPUS;
> + int ret;
> +
> + if (!(filp->f_mode & FMODE_READ))
> + return 0;
> +
> + if (inode->i_cdev)
> + cpu = (long)inode->i_cdev - 1;
Hmm, we probably should use the helper function here. That is make
tracing_get_cpu() non-static and use that. When inode->i_cdev is zero it
returns RING_BUFFER_ALL_CPUS so you don't need to initialize cpu.
It should be used in the other locations too.
-- Steve
> +
> + guard(mutex)(&remote->lock);
> +
> + iter = trace_remote_iter(remote, cpu, TRI_NONCONSUMING);
> + if (IS_ERR(iter))
> + return PTR_ERR(iter);
> +
> + ret = seq_open(filp, &trace_seq_ops);
> + if (ret) {
> + trace_remote_iter_free(iter);
> + return ret;
> + }
> +
> + if (iter)
> + trace_remote_iter_read_start(iter);
> +
> + ((struct seq_file *)filp->private_data)->private = (void *)iter;
> +
> + return 0;
> +}
> +
> +static int trace_release(struct inode *inode, struct file *filp)
> +{
> + struct trace_remote_iterator *iter;
> +
> + if (!(filp->f_mode & FMODE_READ))
> + return 0;
> +
> + iter = ((struct seq_file *)filp->private_data)->private;
> + seq_release(inode, filp);
> +
> + if (!iter)
> + return 0;
> +
> + guard(mutex)(&iter->remote->lock);
> +
> + trace_remote_iter_read_finished(iter);
> + trace_remote_iter_free(iter);
> +
> + return 0;
> +}
> +
> static ssize_t trace_write(struct file *filp, const char __user *ubuf, size_t cnt, loff_t *ppos)
> {
> struct inode *inode = file_inode(filp);
> @@ -443,7 +754,11 @@ static ssize_t trace_write(struct file *filp, const char __user *ubuf, size_t cn
> }
>
> static const struct file_operations trace_fops = {
> + .open = trace_open,
> .write = trace_write,
> + .read = seq_read,
> + .read_iter = seq_read_iter,
> + .release = trace_release,
> };
>
> static int trace_remote_init_tracefs(const char *name, struct trace_remote *remote)
> @@ -532,6 +847,7 @@ int trace_remote_register(const char *name, struct trace_remote_callbacks *cbs,
> remote->trace_buffer_size = 7 << 10;
> remote->poll_ms = 100;
> mutex_init(&remote->lock);
> + init_rwsem(&remote->reader_lock);
>
> if (trace_remote_init_tracefs(name, remote)) {
> kfree(remote);
^ permalink raw reply
* [PATCH AUTOSEL 6.18] tracing: Avoid possible signed 64-bit truncation
From: Sasha Levin @ 2026-01-28 22:33 UTC (permalink / raw)
To: patches, stable
Cc: Ian Rogers, Mathieu Desnoyers, Masami Hiramatsu (Google),
Steven Rostedt (Google), Sasha Levin, linux-kernel,
linux-trace-kernel
In-Reply-To: <20260128223332.2806589-1-sashal@kernel.org>
From: Ian Rogers <irogers@google.com>
[ Upstream commit 00f13e28a9c3acd40f0551cde7e9d2d1a41585bf ]
64-bit truncation to 32-bit can result in the sign of the truncated
value changing. The cmp_mod_entry is used in bsearch and so the
truncation could result in an invalid search order. This would only
happen were the addresses more than 2GB apart and so unlikely, but
let's fix the potentially broken compare anyway.
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Link: https://patch.msgid.link/20260108002625.333331-1-irogers@google.com
Signed-off-by: Ian Rogers <irogers@google.com>
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The original buggy code was introduced in v6.15 and later. It's only
present in 6.15+ kernels.
### 8. SUMMARY OF ANALYSIS
**The Bug:**
- The `cmp_mod_entry()` function uses subtraction of two `unsigned long`
values and returns the result as `int`
- On 64-bit systems, if addresses differ by more than 2^31 (~2GB), the
truncation from 64-bit to 32-bit can flip the sign
- This would cause `bsearch()` to make wrong decisions about search
direction
- Result: potentially incorrect module address lookups in trace data
**The Fix:**
- Replaces arithmetic subtraction with simple comparisons
- Returns -1, 0, or 1 directly based on comparisons
- No overflow or truncation possible with the new code
- Logic is more readable and provably correct
**Stable Criteria Evaluation:**
1. ✅ **Obviously correct and tested**: Simple logic, reviewed by
maintainers
2. ✅ **Fixes a real bug**: Yes, a potential comparator correctness bug
3. ⚠️ **Important issue**: Moderate - unlikely to trigger (requires >2GB
address separation) but could cause incorrect trace output
4. ✅ **Small and contained**: Only changes one function body (~6 lines)
5. ✅ **No new features**: Pure bug fix
6. ✅ **Applies cleanly**: Should apply to 6.15+ kernels where this code
exists
**Risk Assessment:**
- Very low risk - the change is small and the new logic is simpler
- The original code has a provable bug (integer overflow on truncation)
- The new code has no such issues
**Concerns:**
- The code only exists in 6.15+ kernels (introduced March 2025)
- The bug is "unlikely" per the author (requires addresses >2GB apart)
- No known real-world reports of this actually causing issues
### DECISION
This is a valid bug fix that:
- Fixes a real (though unlikely to trigger) bug in the comparator
function
- Is very small and self-contained
- Has been reviewed and acked by the tracing maintainers
- Has near-zero regression risk
- Applies to 6.15+ kernels only
The fix is surgical, obviously correct, and addresses a potential
correctness issue. While the bug is unlikely to trigger in practice
(addresses must be >2GB apart), it could cause silent data corruption in
trace output when it does trigger. The fix is trivial and risk-free.
**YES**
kernel/trace/trace.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 142e3b737f0bc..907923d5f8bbb 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -6061,10 +6061,10 @@ static int cmp_mod_entry(const void *key, const void *pivot)
unsigned long addr = (unsigned long)key;
const struct trace_mod_entry *ent = pivot;
- if (addr >= ent[0].mod_addr && addr < ent[1].mod_addr)
- return 0;
- else
- return addr - ent->mod_addr;
+ if (addr < ent[0].mod_addr)
+ return -1;
+
+ return addr >= ent[1].mod_addr;
}
/**
--
2.51.0
^ permalink raw reply related
* Re: [PATCH v2] uprobes: fix incorrect lockdep condition in filter_chain()
From: Masami Hiramatsu @ 2026-01-28 22:29 UTC (permalink / raw)
To: Breno Leitao
Cc: Oleg Nesterov, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Andrii Nakryiko, linux-kernel, linux-trace-kernel,
linux-perf-users, kernel-team, stable
In-Reply-To: <20260128-uprobe_rcu-v2-1-994ea6d32730@debian.org>
On Wed, 28 Jan 2026 10:16:11 -0800
Breno Leitao <leitao@debian.org> wrote:
> The list_for_each_entry_rcu() in filter_chain() uses
> rcu_read_lock_trace_held() as the lockdep condition, but the function
> holds consumer_rwsem, not the RCU trace lock.
>
> This gives me the following output when running with some locking debug
> option enabled:
>
> kernel/events/uprobes.c:1141 RCU-list traversed in non-reader section!!
> filter_chain
> register_for_each_vma
> uprobe_unregister_nosync
> __probe_event_disable
>
> Remove the incorrect lockdep condition since the rwsem provides
> sufficient protection for the list traversal.
>
Looks good to me.
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Thanks,
> Cc: stable@vger.kernel.org
> Fixes: cc01bd044e6a ("uprobes: travers uprobe's consumer list locklessly under SRCU protection")
> Acked-by: Oleg Nesterov <oleg@redhat.com>
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
> Changes in v2:
> - updated the "fixes" tag (Oleg)
> - Link to v1: https://patch.msgid.link/20260128-uprobe_rcu-v1-1-d41316763799@debian.org
> ---
> kernel/events/uprobes.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index d546d32390a81..726d13b375f3d 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -1138,7 +1138,7 @@ static bool filter_chain(struct uprobe *uprobe, struct mm_struct *mm)
> bool ret = false;
>
> down_read(&uprobe->consumer_rwsem);
> - list_for_each_entry_rcu(uc, &uprobe->consumers, cons_node, rcu_read_lock_trace_held()) {
> + list_for_each_entry(uc, &uprobe->consumers, cons_node) {
> ret = consumer_filter(uc, mm);
> if (ret)
> break;
>
> ---
> base-commit: 1f97d9dcf53649c41c33227b345a36902cbb08ad
> change-id: 20260128-uprobe_rcu-e21867ab4c1b
>
> Best regards,
> --
> Breno Leitao <leitao@debian.org>
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [RFC PATCH v1 05/37] KVM: guest_memfd: Wire up kvm_get_memory_attributes() to per-gmem attributes
From: Ackerley Tng @ 2026-01-28 21:47 UTC (permalink / raw)
To: Alexey Kardashevskiy, cgroups, kvm, linux-doc, linux-fsdevel,
linux-kernel, linux-kselftest, linux-mm, linux-trace-kernel, x86
Cc: akpm, binbin.wu, bp, brauner, chao.p.peng, chenhuacai, corbet,
dave.hansen, dave.hansen, david, dmatlack, erdemaktas, fan.du,
fvdl, haibo1.xu, hannes, hch, hpa, hughd, ira.weiny,
isaku.yamahata, jack, james.morse, jarkko, jgg, jgowans, jhubbard,
jroedel, jthoughton, jun.miao, kai.huang, keirf, kent.overstreet,
liam.merwick, maciej.wieczor-retman, mail, maobibo,
mathieu.desnoyers, maz, mhiramat, mhocko, mic, michael.roth,
mingo, mlevitsk, mpe, muchun.song, nikunj, nsaenz, oliver.upton,
palmer, pankaj.gupta, paul.walmsley, pbonzini, peterx, pgonda,
prsampat, pvorel, qperret, richard.weiyang, rick.p.edgecombe,
rientjes, rostedt, roypat, rppt, seanjc, shakeel.butt, shuah,
steven.price, steven.sistare, suzuki.poulose, tabba, tglx,
thomas.lendacky, vannapurve, vbabka, viro, vkuznets, wei.w.wang,
will, willy, wyihan, xiaoyao.li, yan.y.zhao, yilun.xu, yuzenghui,
zhiquan1.li
In-Reply-To: <07836b1d-d0d8-40f2-8f7b-7805beca31d0@amd.com>
Alexey Kardashevskiy <aik@amd.com> writes:
>
> [...snip...]
>
>
Thanks for bringing this up!
> I am trying to make it work with TEE-IO where fd of VFIO MMIO is a dmabuf fd while the rest (guest RAM) is gmemfd. The above suggests that if there is gmemfd - then the memory attributes are handled by gmemfd which is... expected?
>
I think this is not expected.
IIUC MMIO guest physical addresses don't have an associated memslot, but
if you managed to get to that line in kvm_gmem_get_memory_attributes(),
then there is an associated memslot (slot != NULL)?
Either way, guest_memfd shouldn't store attributes for guest physical
addresses that don't belong to some guest_memfd memslot.
I think we need a broader discussion for this on where to store memory
attributes for MMIO addresses.
I think we should at least have line of sight to storing memory
attributes for MMIO addresses, in case we want to design something else,
since we're putting vm_memory_attributes on a deprecation path with this
series.
Sean, what do you think?
Alexey, shall we discuss this at either the upcoming PUCK or guest_memfd
biweekly session?
> The problem at hand is that kvm_mmu_faultin_pfn() fails at "if (fault->is_private != kvm_mem_is_private(kvm, fault->gfn))" and marking MMIO as private using kvm_vm_ioctl_set_mem_attributes() does not work as kvm_gmem_get_memory_attributes() fails on dmabuf fds.
>
> I worked around this like below but wonder what is the proper way? Thanks,
>
>
> @@ -768,13 +768,13 @@ unsigned long kvm_gmem_get_memory_attributes(struct kvm *kvm, gfn_t gfn)
> */
> if (!slot)
> return 0;
>
> CLASS(gmem_get_file, file)(slot);
> if (!file)
> - return false;
> + return kvm_get_vm_memory_attributes(kvm, gfn);
>
> /*
> * Don't take the filemap invalidation lock, as temporarily acquiring
> * that lock wouldn't provide any meaningful protection. The caller
> * _must_ protect consumption of private vs. shared by checking
> * mmu_invalidate_retry_gfn() under mmu_lock.
>
>
>
> --
> Alexey
^ permalink raw reply
* Re: [PATCH] tracing: trace_mmap.h: fix a kernel-doc warning
From: Steven Rostedt @ 2026-01-28 21:43 UTC (permalink / raw)
To: Randy Dunlap
Cc: linux-kernel, Masami Hiramatsu, Mathieu Desnoyers,
linux-trace-kernel
In-Reply-To: <20260128062522.403456-1-rdunlap@infradead.org>
On Tue, 27 Jan 2026 22:25:22 -0800
Randy Dunlap <rdunlap@infradead.org> wrote:
> Add a description of struct reader to resolve a kernel-doc warning:
>
> Warning: include/uapi/linux/trace_mmap.h:43 struct member 'reader' not described in 'trace_buffer_meta'
>
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> ---
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: linux-trace-kernel@vger.kernel.org
>
> include/uapi/linux/trace_mmap.h | 1 +
> 1 file changed, 1 insertion(+)
>
> --- linux-next-20260126.orig/include/uapi/linux/trace_mmap.h
> +++ linux-next-20260126/include/uapi/linux/trace_mmap.h
> @@ -13,6 +13,7 @@
> * @reader.lost_events: Number of events lost at the time of the reader swap.
> * @reader.id: subbuf ID of the current reader. ID range [0 : @nr_subbufs - 1]
> * @reader.read: Number of bytes read on the reader subbuf.
> + * @reader: The reader composite info structure
Wouldn't it look better if @reader came before its field descriptions?
-- Steve
> * @flags: Placeholder for now, 0 until new features are supported.
> * @entries: Number of entries in the ring-buffer.
> * @overrun: Number of entries lost in the ring-buffer.
^ permalink raw reply
* Re: [PATCH v10 05/30] tracing: Introduce trace remotes
From: Steven Rostedt @ 2026-01-28 20:52 UTC (permalink / raw)
To: Vincent Donnefort
Cc: mhiramat, mathieu.desnoyers, linux-trace-kernel, maz,
oliver.upton, joey.gouly, suzuki.poulose, yuzenghui, kvmarm,
linux-arm-kernel, jstultz, qperret, will, aneesh.kumar,
kernel-team, linux-kernel
In-Reply-To: <20260126104419.1649811-6-vdonnefort@google.com>
On Mon, 26 Jan 2026 10:43:54 +0000
Vincent Donnefort <vdonnefort@google.com> wrote:
> + if (!trace_create_file("tracing_on", TRACEFS_MODE_WRITE, remote_d, remote,
> + &tracing_on_fops) ||
> + !trace_create_file("buffer_size_kb", TRACEFS_MODE_WRITE, remote_d, remote,
> + &buffer_size_kb_fops) ||
> + !trace_create_file("trace_pipe", TRACEFS_MODE_READ, remote_d, remote,
> + &trace_pipe_fops))
> + goto err;
> +
Oh, and don't do this in an if statement. Make them each individually as:
ret = trace_create_file(...);
if (!ret)
goto err;
Linus hates complex if statements and I can see him having a conniption if
he were to see this.
-- Steve
^ permalink raw reply
* Re: [PATCH v10 05/30] tracing: Introduce trace remotes
From: Steven Rostedt @ 2026-01-28 20:37 UTC (permalink / raw)
To: Vincent Donnefort
Cc: mhiramat, mathieu.desnoyers, linux-trace-kernel, maz,
oliver.upton, joey.gouly, suzuki.poulose, yuzenghui, kvmarm,
linux-arm-kernel, jstultz, qperret, will, aneesh.kumar,
kernel-team, linux-kernel
In-Reply-To: <20260126104419.1649811-6-vdonnefort@google.com>
On Mon, 26 Jan 2026 10:43:54 +0000
Vincent Donnefort <vdonnefort@google.com> wrote:
> diff --git a/include/linux/trace_remote.h b/include/linux/trace_remote.h
> new file mode 100644
> index 000000000000..feb3433c2128
> --- /dev/null
> +++ b/include/linux/trace_remote.h
> @@ -0,0 +1,80 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef _LINUX_TRACE_REMOTE_H
> +#define _LINUX_TRACE_REMOTE_H
> +
> +#include <linux/ring_buffer.h>
> +
> +/**
> + * struct trace_remote_callbacks - Callbacks used by Tracefs to control the remote
> + *
> + * @load_trace_buffer: Called before Tracefs accesses the trace buffer for the first
> + * time. Must return a &trace_buffer_desc
> + * (most likely filled with trace_remote_alloc_buffer())
> + * @unload_trace_buffer:
> + * Called once Tracefs has no use for the trace buffer
> + * (most likely call trace_remote_free_buffer())
> + * @enable_tracing: Called on Tracefs tracing_on. It is expected from the
> + * remote to allow writing.
> + * @swap_reader_page: Called when Tracefs consumes a new page from a
> + * ring-buffer. It is expected from the remote to isolate a
> + * new reader-page from the @cpu ring-buffer.
> + */
> +struct trace_remote_callbacks {
> + struct trace_buffer_desc *(*load_trace_buffer)(unsigned long size, void *priv);
> + void (*unload_trace_buffer)(struct trace_buffer_desc *desc, void *priv);
> + int (*enable_tracing)(bool enable, void *priv);
> + int (*swap_reader_page)(unsigned int cpu, void *priv);
> +};
> +
> +/**
> + * trace_remote_register() - Register a Tracefs remote
> + *
> + * A trace remote is an entity, outside of the kernel (most likely firmware or
> + * hypervisor) capable of writing events into a Tracefs compatible ring-buffer.
> + * The kernel would then act as a reader.
> + *
> + * The registered remote will be found under the Tracefs directory
> + * remotes/<name>.
> + *
> + * @name: Name of the remote, used for the Tracefs remotes/ directory.
> + * @cbs: Set of callbacks used to control the remote.
> + * @priv: Private data, passed to each callback from @cbs.
> + * @events: Array of events. &remote_event.name and &remote_event.id must be
> + * filled by the caller.
> + * @nr_events: Number of events in the @events array.
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +int trace_remote_register(const char *name, struct trace_remote_callbacks *cbs, void *priv);
Nit, but kerneldoc goes before the functions and not before their prototypes.
> +
> +/**
> + * trace_remote_alloc_buffer() - Dynamically allocate a trace buffer
> + *
> + * Helper to dynamically allocate a set of pages (enough to cover @buffer_size)
> + * for each CPU from @cpumask and fill @desc. Most likely called from
> + * &trace_remote_callbacks.load_trace_buffer.
> + *
> + * @desc: Uninitialized trace_buffer_desc
> + * @desc_size: Size of the trace_buffer_desc. Must be at least equal to
> + * trace_buffer_desc_size()
> + * @buffer_size: Size in bytes of each per-CPU ring-buffer
> + * @cpumask: CPUs to allocate a ring-buffer for
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +int trace_remote_alloc_buffer(struct trace_buffer_desc *desc, size_t desc_size, size_t buffer_size,
> + const struct cpumask *cpumask);
> +
> +/**
> + * trace_remote_free_buffer() - Free trace buffer allocated with
> + * trace_remote_alloc_buffer()
> + *
> + * Most likely called from &trace_remote_callbacks.unload_trace_buffer.
> + *
> + * @desc: Descriptor of the per-CPU ring-buffers, originally filled by
> + * trace_remote_alloc_buffer()
> + */
> +void trace_remote_free_buffer(struct trace_buffer_desc *desc);
> +
> +#endif
> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> index bfa2ec46e075..135edf143073 100644
> --- a/kernel/trace/Kconfig
> +++ b/kernel/trace/Kconfig
> @@ -1278,4 +1278,7 @@ config HIST_TRIGGERS_DEBUG
>
> source "kernel/trace/rv/Kconfig"
>
> +config TRACE_REMOTE
> + bool
> +
> endif # FTRACE
> diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
> index fc5dcc888e13..6e75d710fc25 100644
> --- a/kernel/trace/Makefile
> +++ b/kernel/trace/Makefile
> @@ -127,4 +127,5 @@ obj-$(CONFIG_FPROBE_EVENTS) += trace_fprobe.o
> obj-$(CONFIG_TRACEPOINT_BENCHMARK) += trace_benchmark.o
> obj-$(CONFIG_RV) += rv/
>
> +obj-$(CONFIG_TRACE_REMOTE) += trace_remote.o
> libftrace-y := ftrace.o
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 8bd4ec08fb36..1fcbdea992d2 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -9356,7 +9356,7 @@ static struct dentry *tracing_dentry_percpu(struct trace_array *tr, int cpu)
> return tr->percpu_dir;
> }
>
> -static struct dentry *
> +struct dentry *
> trace_create_cpu_file(const char *name, umode_t mode, struct dentry *parent,
> void *data, long cpu, const struct file_operations *fops)
> {
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index b6d42fe06115..fdbcd068ba38 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -680,6 +680,12 @@ struct dentry *trace_create_file(const char *name,
> struct dentry *parent,
> void *data,
> const struct file_operations *fops);
> +struct dentry *trace_create_cpu_file(const char *name,
> + umode_t mode,
> + struct dentry *parent,
> + void *data,
> + long cpu,
> + const struct file_operations *fops);
>
>
> /**
> diff --git a/kernel/trace/trace_remote.c b/kernel/trace/trace_remote.c
> new file mode 100644
> index 000000000000..3d80ff98fd90
> --- /dev/null
> +++ b/kernel/trace/trace_remote.c
> @@ -0,0 +1,570 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2025 - Google LLC
> + * Author: Vincent Donnefort <vdonnefort@google.com>
> + */
> +
> +#include <linux/kstrtox.h>
> +#include <linux/lockdep.h>
> +#include <linux/mutex.h>
> +#include <linux/tracefs.h>
> +#include <linux/trace_remote.h>
> +#include <linux/trace_seq.h>
> +#include <linux/types.h>
> +
> +#include "trace.h"
> +
> +#define TRACEFS_DIR "remotes"
> +#define TRACEFS_MODE_WRITE 0640
> +#define TRACEFS_MODE_READ 0440
> +
> +struct trace_remote_iterator {
> + struct trace_remote *remote;
> + struct trace_seq seq;
> + struct delayed_work poll_work;
> + unsigned long lost_events;
> + u64 ts;
> + int cpu;
> + int evt_cpu;
> +};
> +
> +struct trace_remote {
> + struct trace_remote_callbacks *cbs;
> + void *priv;
> + struct trace_buffer *trace_buffer;
> + struct trace_buffer_desc *trace_buffer_desc;
> + unsigned long trace_buffer_size;
> + struct ring_buffer_remote rb_remote;
> + struct mutex lock;
> + unsigned int nr_readers;
> + unsigned int poll_ms;
> + bool tracing_on;
> +};
> +
> +static bool trace_remote_loaded(struct trace_remote *remote)
> +{
> + return remote->trace_buffer;
Nit, but since you are returning a bool, make it obvious that is what you
want:
return !!remote->trace_buffer;
> +}
> +
> +static int trace_remote_load(struct trace_remote *remote)
> +{
> + struct ring_buffer_remote *rb_remote = &remote->rb_remote;
> +
> + lockdep_assert_held(&remote->lock);
> +
> + if (trace_remote_loaded(remote))
> + return 0;
> +
> + remote->trace_buffer_desc = remote->cbs->load_trace_buffer(remote->trace_buffer_size,
> + remote->priv);
> + if (IS_ERR(remote->trace_buffer_desc))
> + return PTR_ERR(remote->trace_buffer_desc);
Hmm, on error, should we reset remote->trace_buffer_desc to NULL?
Just to be safe. Could just do:
struct trace_buffer_desc *desc;
desc = remote->cbs->load_trace_buffer(remote->trace_buffer_size, remote->priv);
if (IS_ERR(desc))
return PTR_ERR(desc);
rb_remote->desc = desc;
> +
> + rb_remote->desc = remote->trace_buffer_desc;
> + rb_remote->swap_reader_page = remote->cbs->swap_reader_page;
> + rb_remote->priv = remote->priv;
> + remote->trace_buffer = ring_buffer_alloc_remote(rb_remote);
> + if (!remote->trace_buffer) {
> + remote->cbs->unload_trace_buffer(remote->trace_buffer_desc, remote->priv);
remote->cbs->unload_trace_buffer(desc, remote->priv);
> + return -ENOMEM;
> + }
remote->trace_buffer_desc = desc;
Only set it on success.
> +
> + return 0;
> +}
> +
> +static void trace_remote_try_unload(struct trace_remote *remote)
> +{
> + lockdep_assert_held(&remote->lock);
> +
> + if (!trace_remote_loaded(remote))
> + return;
> +
> + /* The buffer is being read or writable */
> + if (remote->nr_readers || remote->tracing_on)
> + return;
> +
> + /* The buffer has readable data */
> + if (!ring_buffer_empty(remote->trace_buffer))
> + return;
> +
> + ring_buffer_free(remote->trace_buffer);
> + remote->trace_buffer = NULL;
> + remote->cbs->unload_trace_buffer(remote->trace_buffer_desc, remote->priv);
> +}
> +
> +static int trace_remote_enable_tracing(struct trace_remote *remote)
> +{
> + int ret;
> +
> + lockdep_assert_held(&remote->lock);
> +
> + if (remote->tracing_on)
> + return 0;
> +
> + ret = trace_remote_load(remote);
> + if (ret)
> + return ret;
> +
> + ret = remote->cbs->enable_tracing(true, remote->priv);
> + if (ret) {
> + trace_remote_try_unload(remote);
> + return ret;
> + }
> +
> + remote->tracing_on = true;
> +
> + return 0;
> +}
> +
> +static int trace_remote_disable_tracing(struct trace_remote *remote)
> +{
> + int ret;
> +
> + lockdep_assert_held(&remote->lock);
> +
> + if (!remote->tracing_on)
> + return 0;
> +
> + ret = remote->cbs->enable_tracing(false, remote->priv);
> + if (ret)
> + return ret;
> +
> + ring_buffer_poll_remote(remote->trace_buffer, RING_BUFFER_ALL_CPUS);
> + remote->tracing_on = false;
> + trace_remote_try_unload(remote);
> +
> + return 0;
> +}
> +
> +static ssize_t
> +tracing_on_write(struct file *filp, const char __user *ubuf, size_t cnt, loff_t *ppos)
> +{
> + struct trace_remote *remote = filp->private_data;
> + unsigned long val;
> + int ret;
> +
> + ret = kstrtoul_from_user(ubuf, cnt, 10, &val);
> + if (ret)
> + return ret;
> +
> + guard(mutex)(&remote->lock);
> +
> + ret = val ? trace_remote_enable_tracing(remote) : trace_remote_disable_tracing(remote);
> + if (ret)
> + return ret;
> +
> + return cnt;
> +}
> +static int tracing_on_show(struct seq_file *s, void *unused)
> +{
> + struct trace_remote *remote = s->private;
> +
> + seq_printf(s, "%d\n", remote->tracing_on);
> +
> + return 0;
> +}
> +DEFINE_SHOW_STORE_ATTRIBUTE(tracing_on);
> +
> +static ssize_t buffer_size_kb_write(struct file *filp, const char __user *ubuf, size_t cnt,
> + loff_t *ppos)
> +{
> + struct trace_remote *remote = filp->private_data;
> + unsigned long val;
> + int ret;
> +
> + ret = kstrtoul_from_user(ubuf, cnt, 10, &val);
> + if (ret)
> + return ret;
> +
> + /* KiB to Bytes */
> + if (!val || check_shl_overflow(val, 10, &val))
> + return -EINVAL;
> +
> + guard(mutex)(&remote->lock);
> +
> + remote->trace_buffer_size = val;
Should this be allowed to change when it is already loaded?
-- Steve
> +
> + return cnt;
> +}
> +
> +static int buffer_size_kb_show(struct seq_file *s, void *unused)
> +{
> + struct trace_remote *remote = s->private;
> +
> + seq_printf(s, "%lu (%s)\n", remote->trace_buffer_size >> 10,
> + trace_remote_loaded(remote) ? "loaded" : "unloaded");
> +
> + return 0;
> +}
> +DEFINE_SHOW_STORE_ATTRIBUTE(buffer_size_kb);
> +
^ permalink raw reply
* Re: [PATCHv6 bpf-next 0/9] ftrace,bpf: Use single direct ops for bpf trampolines
From: patchwork-bot+netdevbpf @ 2026-01-28 20:00 UTC (permalink / raw)
To: Jiri Olsa
Cc: rostedt, revest, mark.rutland, bpf, linux-kernel,
linux-trace-kernel, linux-arm-kernel, ast, daniel, andrii,
menglong8.dong, song
In-Reply-To: <20251230145010.103439-1-jolsa@kernel.org>
Hello:
This series was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:
On Tue, 30 Dec 2025 15:50:01 +0100 you 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.
>
> [...]
Here is the summary with links:
- [PATCHv6,bpf-next,1/9] ftrace,bpf: Remove FTRACE_OPS_FL_JMP ftrace_ops flag
https://git.kernel.org/bpf/bpf-next/c/4be42c922201
- [PATCHv6,bpf-next,2/9] ftrace: Make alloc_and_copy_ftrace_hash direct friendly
https://git.kernel.org/bpf/bpf-next/c/676bfeae7bd5
- [PATCHv6,bpf-next,3/9] ftrace: Export some of hash related functions
https://git.kernel.org/bpf/bpf-next/c/0e860d07c29d
- [PATCHv6,bpf-next,4/9] ftrace: Add update_ftrace_direct_add function
https://git.kernel.org/bpf/bpf-next/c/05dc5e9c1fe1
- [PATCHv6,bpf-next,5/9] ftrace: Add update_ftrace_direct_del function
https://git.kernel.org/bpf/bpf-next/c/8d2c1233f371
- [PATCHv6,bpf-next,6/9] ftrace: Add update_ftrace_direct_mod function
https://git.kernel.org/bpf/bpf-next/c/e93672f770d7
- [PATCHv6,bpf-next,7/9] bpf: Add trampoline ip hash table
https://git.kernel.org/bpf/bpf-next/c/7d0452497c29
- [PATCHv6,bpf-next,8/9] ftrace: Factor ftrace_ops ops_func interface
https://git.kernel.org/bpf/bpf-next/c/956747efd82a
- [PATCHv6,bpf-next,9/9] bpf,x86: Use single ftrace_ops for direct calls
https://git.kernel.org/bpf/bpf-next/c/424f6a361096
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* [PATCH v2] uprobes: fix incorrect lockdep condition in filter_chain()
From: Breno Leitao @ 2026-01-28 18:16 UTC (permalink / raw)
To: Masami Hiramatsu, Oleg Nesterov, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Andrii Nakryiko
Cc: linux-kernel, linux-trace-kernel, linux-perf-users, kernel-team,
stable, Breno Leitao
The list_for_each_entry_rcu() in filter_chain() uses
rcu_read_lock_trace_held() as the lockdep condition, but the function
holds consumer_rwsem, not the RCU trace lock.
This gives me the following output when running with some locking debug
option enabled:
kernel/events/uprobes.c:1141 RCU-list traversed in non-reader section!!
filter_chain
register_for_each_vma
uprobe_unregister_nosync
__probe_event_disable
Remove the incorrect lockdep condition since the rwsem provides
sufficient protection for the list traversal.
Cc: stable@vger.kernel.org
Fixes: cc01bd044e6a ("uprobes: travers uprobe's consumer list locklessly under SRCU protection")
Acked-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Breno Leitao <leitao@debian.org>
---
Changes in v2:
- updated the "fixes" tag (Oleg)
- Link to v1: https://patch.msgid.link/20260128-uprobe_rcu-v1-1-d41316763799@debian.org
---
kernel/events/uprobes.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index d546d32390a81..726d13b375f3d 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -1138,7 +1138,7 @@ static bool filter_chain(struct uprobe *uprobe, struct mm_struct *mm)
bool ret = false;
down_read(&uprobe->consumer_rwsem);
- list_for_each_entry_rcu(uc, &uprobe->consumers, cons_node, rcu_read_lock_trace_held()) {
+ list_for_each_entry(uc, &uprobe->consumers, cons_node) {
ret = consumer_filter(uc, mm);
if (ret)
break;
---
base-commit: 1f97d9dcf53649c41c33227b345a36902cbb08ad
change-id: 20260128-uprobe_rcu-e21867ab4c1b
Best regards,
--
Breno Leitao <leitao@debian.org>
^ permalink raw reply related
* Re: [RFC 0/5] ext4: mark more ops fast-commit ineligible
From: Theodore Ts'o @ 2026-01-28 18:05 UTC (permalink / raw)
To: Andreas Dilger, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, linux-ext4, linux-kernel, linux-trace-kernel,
Li Chen
Cc: Theodore Ts'o
In-Reply-To: <20251211115146.897420-1-me@linux.beauty>
On Thu, 11 Dec 2025 19:51:37 +0800, Li Chen wrote:
> ext4 fast commit only logs operations with replay support. This series
> marks a few more operations as fast-commit ineligible and accounts
> them via fc_info so behaviour under fast commit is easier to reason
> about.
>
> Testing was done in a QEMU guest on loopback ext4 filesystems created
> with -O fast_commit[/,verity] by exercising each operation and checking
> /proc/fs/ext4/*/fc_info for the corresponding ineligible reason and
> ineligible commit counters. Detailed steps are in each commit's message.
>
> [...]
Applied, thanks!
[1/5] ext4: mark inode format migration fast-commit ineligible
commit: 87e79fa122bc9a6576f1690ee264fcbd77d3ab58
[2/5] ext4: mark fs-verity enable fast-commit ineligible
commit: 16d43b9748c655b36a675cc55789f40fd827e9b1
[3/5] ext4: mark move extents fast-commit ineligible
commit: 690558921d9f9388c6bc83610451d8cb393e4d88
[4/5] ext4: mark group add fast-commit ineligible
commit: 89b4336fd5ec78f51f9d3a1d100f3ffa3228e604
[5/5] ext4: mark group extend fast-commit ineligible
commit: 1f8dd813a1c771b13c303f73d876164bc9b327cc
Best regards,
--
Theodore Ts'o <tytso@mit.edu>
^ permalink raw reply
* Re: [PATCH] uprobes: fix incorrect lockdep condition in filter_chain()
From: Breno Leitao @ 2026-01-28 17:51 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Oleg Nesterov, Masami Hiramatsu, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Andrii Nakryiko, linux-kernel, linux-trace-kernel,
linux-perf-users, kernel-team, stable
In-Reply-To: <CAEf4BzYJJiUdQTjDgr_uVSQ+uBhYWKki0vjS5VffTzbST1uS2g@mail.gmail.com>
On Wed, Jan 28, 2026 at 09:23:45AM -0800, Andrii Nakryiko wrote:
> On Wed, Jan 28, 2026 at 5:51 AM Oleg Nesterov <oleg@redhat.com> wrote:
> >
> > On 01/28, Breno Leitao wrote:
> > >
> > > The list_for_each_entry_rcu() in filter_chain() uses
> > > rcu_read_lock_trace_held() as the lockdep condition, but the function
> > > holds consumer_rwsem, not the RCU trace lock.
> > >
> > > This gives me the following output when running with some locking debug
> > > option enabled:
> > >
> > > kernel/events/uprobes.c:1141 RCU-list traversed in non-reader section!!
> > > filter_chain
> > > register_for_each_vma
> > > uprobe_unregister_nosync
> > > __probe_event_disable
> > >
> > > Remove the incorrect lockdep condition since the rwsem provides
> > > sufficient protection for the list traversal.
> >
> > I hope Andrii will recheck, but looks obviously correct to me.
>
> yeah, I did, and it also looks obviously correct to me, I didn't need
> to use rcu flavor there in the first place, I think.
>
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
>
> >
> > > Fixes: 87195a1ee332a ("uprobes: switch to RCU Tasks Trace flavor for better performance")
> >
> > This commit just change the __list_check_rcu() condition...
> >
> > Perhaps
> > Fixes: cc01bd044e6a ("uprobes: travers uprobe's consumer list locklessly under SRCU protection")
> >
>
> yep, this one is the earliest change adding unnecessary rcu flavor of
> list_for_each_entry
Ack. I will respin with the correct "fixes" tag.
--breno
^ permalink raw reply
* Re: [RFC PATCH v1 01/37] KVM: guest_memfd: Introduce per-gmem attributes, use to guard user mappings
From: Ackerley Tng @ 2026-01-28 17:50 UTC (permalink / raw)
To: Yan Zhao
Cc: cgroups, kvm, linux-doc, linux-fsdevel, linux-kernel,
linux-kselftest, linux-mm, linux-trace-kernel, x86, akpm,
binbin.wu, bp, brauner, chao.p.peng, chenhuacai, corbet,
dave.hansen, dave.hansen, david, dmatlack, erdemaktas, fan.du,
fvdl, haibo1.xu, hannes, hch, hpa, hughd, ira.weiny,
isaku.yamahata, jack, james.morse, jarkko, jgg, jgowans, jhubbard,
jroedel, jthoughton, jun.miao, kai.huang, keirf, kent.overstreet,
liam.merwick, maciej.wieczor-retman, mail, maobibo,
mathieu.desnoyers, maz, mhiramat, mhocko, mic, michael.roth,
mingo, mlevitsk, mpe, muchun.song, nikunj, nsaenz, oliver.upton,
palmer, pankaj.gupta, paul.walmsley, pbonzini, peterx, pgonda,
prsampat, pvorel, qperret, richard.weiyang, rick.p.edgecombe,
rientjes, rostedt, roypat, rppt, seanjc, shakeel.butt, shuah,
steven.price, steven.sistare, suzuki.poulose, tabba, tglx,
thomas.lendacky, vannapurve, vbabka, viro, vkuznets, wei.w.wang,
will, willy, wyihan, xiaoyao.li, yilun.xu, yuzenghui, zhiquan1.li
In-Reply-To: <aW3kOgKL7TjpW4AT@yzhao56-desk.sh.intel.com>
Yan Zhao <yan.y.zhao@intel.com> writes:
>
> [...snip...]
>
>
> So, it's possible for kvm_mem_is_private() to access invalid mtree data and hit
> the WARN_ON_ONCE() in kvm_gmem_get_attributes().
>
> I reported a similar error in [*].
>
> [*] https://lore.kernel.org/all/aIwD5kGbMibV7ksk@yzhao56-desk.sh.intel.com
>
Will add locking in the next revision. Thanks!
^ permalink raw reply
* Re: [PATCH] uprobes: fix incorrect lockdep condition in filter_chain()
From: Andrii Nakryiko @ 2026-01-28 17:23 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Breno Leitao, Masami Hiramatsu, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Andrii Nakryiko, linux-kernel, linux-trace-kernel,
linux-perf-users, kernel-team, stable
In-Reply-To: <aXoUOEhDfncEkC-f@redhat.com>
On Wed, Jan 28, 2026 at 5:51 AM Oleg Nesterov <oleg@redhat.com> wrote:
>
> On 01/28, Breno Leitao wrote:
> >
> > The list_for_each_entry_rcu() in filter_chain() uses
> > rcu_read_lock_trace_held() as the lockdep condition, but the function
> > holds consumer_rwsem, not the RCU trace lock.
> >
> > This gives me the following output when running with some locking debug
> > option enabled:
> >
> > kernel/events/uprobes.c:1141 RCU-list traversed in non-reader section!!
> > filter_chain
> > register_for_each_vma
> > uprobe_unregister_nosync
> > __probe_event_disable
> >
> > Remove the incorrect lockdep condition since the rwsem provides
> > sufficient protection for the list traversal.
>
> I hope Andrii will recheck, but looks obviously correct to me.
yeah, I did, and it also looks obviously correct to me, I didn't need
to use rcu flavor there in the first place, I think.
Acked-by: Andrii Nakryiko <andrii@kernel.org>
>
> > Fixes: 87195a1ee332a ("uprobes: switch to RCU Tasks Trace flavor for better performance")
>
> This commit just change the __list_check_rcu() condition...
>
> Perhaps
> Fixes: cc01bd044e6a ("uprobes: travers uprobe's consumer list locklessly under SRCU protection")
>
yep, this one is the earliest change adding unnecessary rcu flavor of
list_for_each_entry
> makes more sense?
>
> Acked-by: Oleg Nesterov <oleg@redhat.com>
>
^ permalink raw reply
* Re: [RFC PATCH v1 01/37] KVM: guest_memfd: Introduce per-gmem attributes, use to guard user mappings
From: Ackerley Tng @ 2026-01-28 17:07 UTC (permalink / raw)
To: Binbin Wu
Cc: cgroups, kvm, linux-doc, linux-fsdevel, linux-kernel,
linux-kselftest, linux-mm, linux-trace-kernel, x86, akpm, bp,
brauner, chao.p.peng, chenhuacai, corbet, dave.hansen,
dave.hansen, david, dmatlack, erdemaktas, fan.du, fvdl, haibo1.xu,
hannes, hch, hpa, hughd, ira.weiny, isaku.yamahata, jack,
james.morse, jarkko, jgg, jgowans, jhubbard, jroedel, jthoughton,
jun.miao, kai.huang, keirf, kent.overstreet, liam.merwick,
maciej.wieczor-retman, mail, maobibo, mathieu.desnoyers, maz,
mhiramat, mhocko, mic, michael.roth, mingo, mlevitsk, mpe,
muchun.song, nikunj, nsaenz, oliver.upton, palmer, pankaj.gupta,
paul.walmsley, pbonzini, peterx, pgonda, prsampat, pvorel,
qperret, richard.weiyang, rick.p.edgecombe, rientjes, rostedt,
roypat, rppt, seanjc, shakeel.butt, shuah, steven.price,
steven.sistare, suzuki.poulose, tabba, tglx, thomas.lendacky,
vannapurve, vbabka, viro, vkuznets, wei.w.wang, will, willy,
wyihan, xiaoyao.li, yan.y.zhao, yilun.xu, yuzenghui, zhiquan1.li
In-Reply-To: <ab3f297e-44d5-4f42-aa17-f2e7c135580e@linux.intel.com>
Binbin Wu <binbin.wu@linux.intel.com> writes:
> On 10/18/2025 4:11 AM, Ackerley Tng wrote:
> [...]
>>
>> +static int kvm_gmem_init_inode(struct inode *inode, loff_t size, u64 flags)
>> +{
>> + struct gmem_inode *gi = GMEM_I(inode);
>> + MA_STATE(mas, &gi->attributes, 0, (size >> PAGE_SHIFT) - 1);
>> + u64 attrs;
>> + int r;
>> +
>> + inode->i_op = &kvm_gmem_iops;
>> + inode->i_mapping->a_ops = &kvm_gmem_aops;
>> + inode->i_mode |= S_IFREG;
>> + inode->i_size = size;
>> + mapping_set_gfp_mask(inode->i_mapping, GFP_HIGHUSER);
>> + mapping_set_inaccessible(inode->i_mapping);
>> + /* Unmovable mappings are supposed to be marked unevictable as well. */
> AS_UNMOVABLE has been removed and got merged into AS_INACCESSIBLE, not sure if
> it's better to use "Inaccessible" instead of "Unmovable"
>
Thanks, will update comment as follows:
/*
* guest_memfd memory is not migratable or swappable - set
* inaccessible to gate off both.
*/
mapping_set_inaccessible(inode->i_mapping);
WARN_ON_ONCE(!mapping_unevictable(inode->i_mapping));
>> + WARN_ON_ONCE(!mapping_unevictable(inode->i_mapping));
>> +
>> + gi->flags = flags;
>> +
>> + mt_set_external_lock(&gi->attributes,
>> + &inode->i_mapping->invalidate_lock);
>> +
>> + /*
>> + * Store default attributes for the entire gmem instance. Ensuring every
>> + * index is represented in the maple tree at all times simplifies the
>> + * conversion and merging logic.
>> + */
>> + attrs = gi->flags & GUEST_MEMFD_FLAG_INIT_SHARED ? 0 : KVM_MEMORY_ATTRIBUTE_PRIVATE;
>> +
>> + /*
>> + * Acquire the invalidation lock purely to make lockdep happy. There
>> + * should be no races at this time since the inode hasn't yet been fully
>> + * created.
>> + */
>> + filemap_invalidate_lock(inode->i_mapping);
>> + r = mas_store_gfp(&mas, xa_mk_value(attrs), GFP_KERNEL);
>> + filemap_invalidate_unlock(inode->i_mapping);
>> +
>> + return r;
>> +}
>> +
> [...]
>> @@ -925,13 +986,39 @@ static struct inode *kvm_gmem_alloc_inode(struct super_block *sb)
>>
>> mpol_shared_policy_init(&gi->policy, NULL);
>>
>> + /*
>> + * Memory attributes are protected the filemap invalidation lock, but
> ^
> protected by
Thanks!
>> + * the lock structure isn't available at this time. Immediately mark
>> + * maple tree as using external locking so that accessing the tree
>> + * before its fully initialized results in NULL pointer dereferences
>> + * and not more subtle bugs.
>> + */
>> + mt_init_flags(&gi->attributes, MT_FLAGS_LOCK_EXTERN);
>> +
>> gi->flags = 0;
>> return &gi->vfs_inode;
>> }
>>
>> static void kvm_gmem_destroy_inode(struct inode *inode)
>> {
>> - mpol_free_shared_policy(&GMEM_I(inode)->policy);
>> + struct gmem_inode *gi = GMEM_I(inode);
>> +
>> + mpol_free_shared_policy(&gi->policy);
>> +
>> + /*
>> + * Note! Checking for an empty tree is functionally necessary to avoid
>> + * explosions if the tree hasn't been initialized, i.e. if the inode is
>
> It makes sense to skip __mt_destroy() when mtree is empty.
> But what explosions it could trigger if mtree is empty?
> It seems __mt_destroy() can handle the case if the external lock is not set.
>
>
Hope this updated comment clarify the explosion:
/*
* Note! Checking for an empty tree is functionally necessary
* to avoid explosions if the tree hasn't been fully
* initialized, i.e. if the inode is being destroyed before
* guest_memfd can set the external lock, lockdep would find
* that the tree's internal ma_lock was not held.
*/
>> + * being destroyed before guest_memfd can set the external lock.
>> + */
>> + if (!mtree_empty(&gi->attributes)) {
>> + /*
>> + * Acquire the invalidation lock purely to make lockdep happy,
>> + * the inode is unreachable at this point.
>> + */
>> + filemap_invalidate_lock(inode->i_mapping);
>> + __mt_destroy(&gi->attributes);
>> + filemap_invalidate_unlock(inode->i_mapping);
>> + }
>> }
>>
>> static void kvm_gmem_free_inode(struct inode *inode)
>> --
>> 2.51.0.858.gf9c4a03a3a-goog
^ 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-28 16:38 UTC (permalink / raw)
To: linux-mm, linux-doc, linux-kernel, linux-trace-kernel, akpm
Cc: david, lorenzo.stoakes, 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,
David Hildenbrand, shivankg
In-Reply-To: <20260122192841.128719-4-npache@redhat.com>
Hi Andrew,
could you please apply the following fixup to avoid potentially using a stale
VMA in the new writeback-retry logic for madvise collapse.
Thank you!
-- Nico
----8<----
commit a9ac3b1bfa926dd707ac3a785583f8d7a0579578
Author: Nico Pache <npache@redhat.com>
Date: Fri Jan 23 16:32:42 2026 -0700
madvise writeback retry logic fix
Signed-off-by: Nico Pache <npache@redhat.com>
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 59e5a5588d85..2b054f7d9753 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)) {
+ const loff_t lstart = (loff_t)pgoff << PAGE_SHIFT;
+ const 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) {
--
2.52.0
On 1/22/26 12:28 PM, 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>
> ---
> 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);
> +
> + 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
* [PATCH v2] iommu: Fix NULL pointer deref when io_page_fault tracepoint fires
From: Daniel Thompson @ 2026-01-28 15:48 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers
Cc: linux-kernel, linux-trace-kernel, Will Deacon, Robin Murphy,
linux-arm-kernel, Daniel Thompson
The arm-smmu driver is unable to allocate the blame for a page fault to
a specific device so it calls report_iommu_fault() with the dev argument
set to NULL. Normally this doesn't cause anything catastrophic but on a
system with the io_page_fault tracepoint enabled this results in a NULL
pointer deref (resulting in a fairly spectacular crash on the hardware
I'm currently working on).
Fix this by adding logic to the tracepoint to safely propagate NULL.
Fixes: f8f934c180f6 ("iommu/arm-smmu: Add support for driver IOMMU fault handlers")
Signed-off-by: Daniel Thompson <daniel@riscstar.com>
---
Changes in v2:
- Add a Fixes:. It points to the earliest point I can find where it becomes
possible for the tracepoint to be triggered with dev set to NULL.
- Link to v1: https://lore.kernel.org/r/20260116-iommu-io_page_fault_null_fix-v1-1-6c20c2e62987@riscstar.com
---
include/trace/events/iommu.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/trace/events/iommu.h b/include/trace/events/iommu.h
index 373007e567cb827458a729b8200bbcc1b7d76912..1315193f13b8812ad4e29e6b0c0c66ca806ce08d 100644
--- a/include/trace/events/iommu.h
+++ b/include/trace/events/iommu.h
@@ -131,8 +131,8 @@ DECLARE_EVENT_CLASS(iommu_error,
TP_ARGS(dev, iova, flags),
TP_STRUCT__entry(
- __string(device, dev_name(dev))
- __string(driver, dev_driver_string(dev))
+ __string(device, dev ? dev_name(dev) : NULL)
+ __string(driver, dev ? dev_driver_string(dev) : NULL)
__field(u64, iova)
__field(int, flags)
),
---
base-commit: 0f61b1860cc3f52aef9036d7235ed1f017632193
change-id: 20260116-iommu-io_page_fault_null_fix-f81b4e8b5423
Best regards,
--
Daniel Thompson <daniel@riscstar.com>
^ 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