On Thu, Aug 13, 2026 at 09:31:53PM -0400, Steven Rostedt wrote:
> 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)
>
>
ack.
>
>
> > 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.
Sounds good.
--
Vincent
>
>
> > @@ -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);
