On Thu, 13 Aug 2026 14:11:48 +0100
Vincent Donnefort <[email protected]> wrote:
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index a00ab8a9cbd0..83292d90599e 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -6976,34 +6976,52 @@ EXPORT_SYMBOL_GPL(ring_buffer_swap_cpu);
> * ring_buffer_alloc_read_page - allocate a page to read from buffer
> * @buffer: the buffer to allocate for.
> * @cpu: the cpu buffer to allocate.
> + * @prev: The previous page to be repurposed (can be NULL).
> *
> - * This function is used in conjunction with ring_buffer_read_page.
> + * This function is used in conjunction with ring_buffer_read_page().
> * When reading a full page from the ring buffer, these functions
> * can be used to speed up the process. The calling function should
> * allocate a few pages first with this function. Then when it
> * needs to get pages from the ring buffer, it passes the result
> - * of this function into ring_buffer_read_page, which will swap
> + * of this function into ring_buffer_read_page(), which will swap
> * the page that was allocated, with the read page of the buffer.
> *
> + * If @prev is provided, and it has a different order than the current
> + * subbuffer order, its payload will be freed and re-allocated. If it
> + * already matches the order, it is simply returned.
> + *
> * Returns:
> * The page allocated, or ERR_PTR
> */
> -struct buffer_data_read_page *
> -ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu)
> +struct buffer_data_read_page *ring_buffer_alloc_read_page(struct
> trace_buffer *buffer, int cpu,
> + struct
> buffer_data_read_page *prev)
I think we should do this differently. I don't like the "prev" argument.
Instead, let's pass by address.
int ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu, struct
buffer_data_read_page **rpage)
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 395238b2b715..f9399f391ac6 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -7080,8 +7080,8 @@ ssize_t tracing_buffers_read(struct file *filp, char
> __user *ubuf,
> {
> struct ftrace_buffer_info *info = filp->private_data;
> struct trace_iterator *iter = &info->iter;
> - void *trace_data;
> - int page_size;
> + void *trace_data, *prev_spare;
> + unsigned int spare_size;
> ssize_t ret = 0;
> ssize_t size;
>
> @@ -7091,36 +7091,30 @@ ssize_t tracing_buffers_read(struct file *filp, char
> __user *ubuf,
> if (iter->snapshot && tracer_uses_snapshot(iter->tr->current_trace))
> return -EBUSY;
>
> - page_size = ring_buffer_subbuf_size_get(iter->array_buffer->buffer);
> +again:
> + prev_spare = info->spare;
> + if (prev_spare) {
> + spare_size = ring_buffer_read_page_size(info->spare);
>
> - /* Make sure the spare matches the current sub buffer size */
> - if (info->spare) {
> - if (page_size != info->spare_size) {
> - ring_buffer_free_read_page(iter->array_buffer->buffer,
> - info->spare_cpu,
> info->spare);
> - info->spare = NULL;
> - }
> + /* Do we have previous read data to read? */
> + if (info->read < spare_size)
> + goto read;
> }
>
> - if (!info->spare) {
> - info->spare =
> ring_buffer_alloc_read_page(iter->array_buffer->buffer,
> - iter->cpu_file);
> - if (IS_ERR(info->spare)) {
> - ret = PTR_ERR(info->spare);
> - info->spare = NULL;
> - } else {
> - info->spare_cpu = iter->cpu_file;
> - info->spare_size = page_size;
> - }
> - }
> - if (!info->spare)
> + /* Make sure the read page order is aligned with the current buffer
> subbuf order */
> + info->spare = ring_buffer_alloc_read_page(iter->array_buffer->buffer,
> iter->cpu_file,
> + prev_spare);
> + if (IS_ERR(info->spare)) {
> + ret = PTR_ERR(info->spare);
> + info->spare = NULL;
> + ring_buffer_free_read_page(iter->array_buffer->buffer,
> info->spare_cpu, prev_spare);
> return ret;
> + }
instead of the above:
ret = ring_buffer_alloc_read_page(iter->array_buffer->buffer,
iter->cpu_file,
&info->space);
Where the above could do (under lock):
if (*rpage) {
if (*rpage)->order == buffer->subbuf_order)
return 0;
ring_buffer_free_read_page(*rpage);
*rpage = NULL;
}
*rpage = all the reader page;
That is, lets completely remove the responsibility of the user having to
keep track of the buffer order here.
>
> - /* Do we have previous read data to read? */
> - if (info->read < page_size)
> - goto read;
> + spare_size = ring_buffer_read_page_size(info->spare);
> + info->read = spare_size;
> + info->spare_cpu = iter->cpu_file;
>
> - again:
> trace_access_lock(iter->cpu_file);
> ret = ring_buffer_read_page(iter->array_buffer->buffer,
> info->spare,
And if we could make ring_buffer_read_page() return -EAGAIN if the spare is
not the proper order. And only for that case.
> @@ -7129,6 +7123,10 @@ ssize_t tracing_buffers_read(struct file *filp, char
> __user *ubuf,
> trace_access_unlock(iter->cpu_file);
>
> if (ret < 0) {
> + /* Did we race with ring_buffer_subbuf_order_set ? */
> + if (spare_size !=
> ring_buffer_subbuf_size_get(iter->array_buffer->buffer))
> + goto again;
Then here we can just have:
if (ret == -EAGAIN)
goto again;
-- Steve
> +
> if (trace_empty(iter) && !iter->closed) {
> if (update_last_data_if_empty(iter->tr))
> return 0;
> @@ -7142,12 +7140,14 @@ ssize_t tracing_buffers_read(struct file *filp, char
> __user *ubuf,
>
> goto again;
> }
> +
> return 0;
> }
>
> info->read = 0;
> +
> read:
> - size = page_size - info->read;
> + size = spare_size - info->read;
> if (size > count)
> size = count;
> trace_data = ring_buffer_read_page_data(info->spare);
