Re: [PATCH 1/6] ring-buffer: Fix subbuf resize concurrency
From: Steven Rostedt <rostedt@goodmis.org>
Date: 2026-08-10 22:26:57
Also in:
lkml
On Mon, 10 Aug 2026 13:56:28 +0100 Vincent Donnefort [off-list ref] wrote:
+static __always_inline unsigned int rb_subbuf_size(struct trace_buffer *buffer)
+{
+ return PAGE_SIZE << buffer->subbuf_order;
+}quoted hunk ↗ jump to hunk
@@ -3513,7 +3524,7 @@ rb_event_index(struct ring_buffer_per_cpu *cpu_buffer, struct ring_buffer_event { unsigned long addr = (unsigned long)event; - addr &= (PAGE_SIZE << cpu_buffer->buffer->subbuf_order) - 1; + addr &= (unsigned long)rb_subbuf_size(cpu_buffer->buffer) - 1;
This one is fine because it already sits in a helper function.
return addr - BUF_PAGE_HDR_SIZE; }
quoted hunk ↗ jump to hunk
@@ -4102,7 +4113,7 @@ rb_try_to_discard(struct ring_buffer_per_cpu *cpu_buffer, new_index = rb_event_index(cpu_buffer, event); old_index = new_index + rb_event_ts_length(event); addr = (unsigned long)event; - addr &= ~((PAGE_SIZE << cpu_buffer->buffer->subbuf_order) - 1); + addr &= ~((unsigned long)rb_subbuf_size(cpu_buffer->buffer) - 1); bpage = READ_ONCE(cpu_buffer->tail_page);
quoted hunk ↗ jump to hunk
@@ -5012,7 +5023,7 @@ rb_decrement_entry(struct ring_buffer_per_cpu *cpu_buffer, struct buffer_page *bpage = cpu_buffer->commit_page; struct buffer_page *start; - addr &= ~((PAGE_SIZE << cpu_buffer->buffer->subbuf_order) - 1); + addr &= ~((unsigned long)rb_subbuf_size(cpu_buffer->buffer) - 1); /* Do the likely case first */ if (likely(bpage->page == (void *)addr)) {
I really hate the above open coded typecasting to get the address correct.
Seems very fragile to me. As it is getting the address of the sub buffer,
let's add another helper function:
/**
* rb_subbuf_addr - Return the address of the start of a subbuffer
* @cpu_buffer: The cpu buffer that @addr is on
* @addr: An address of an event on a subbuffer
*
* Returns: The start of the subbuffer for where @addr sits
*/
static __always_inline unsigned long rb_subbuf_addr(struct ring_buffer_per_cpu *cpu_buffer,
unsigned long addr)
{
return addr & ~((unsigned long)(rb_subbuf_size(cpu_buffer->buffer) - 1));
}
And use that for these locatons.
-- Steve