On Tue, 28 Apr 2026 15:55:08 -0400 Steven Rostedt <[email protected]> wrote:
> On Fri, 24 Apr 2026 15:52:27 +0900 > "Masami Hiramatsu (Google)" <[email protected]> wrote: > > > @@ -5648,11 +5668,12 @@ __rb_get_reader_page(struct ring_buffer_per_cpu > > *cpu_buffer) > > again: > > /* > > * This should normally only loop twice. But because the > > - * start of the reader inserts an empty page, it causes > > - * a case where we will loop three times. There should be no > > - * reason to loop four times (that I know of). > > + * start of the reader inserts an empty page, it causes a > > + * case where we will loop three times. There should be no > > + * reason to loop four times unless the ring buffer is a > > + * recovered persistent ring buffer. > > Can you explain more to why this is allowed for persistent ring buffer? Ah, that was introduced in v15. Changes in v15: - Skip reader_page loop check on persistent ring buffer because there can be contiguous empty(invalidated) pages. So, finding next reader_page, we need to skip empty pages, which is normally not contiguous. However, if we see more than 3 invalid pages on recovering persistent ring buffer, it will be reset and become empty. > > Note, I do not like any loops that can go into an infinite loop and lock up > the machine. If something goes wrong with a persistent ring buffer, then > this could possibly go into an infinite loop. Yeah, so I think we should not use goto here. OK, let me update it to an actual loop. > > I want to understand why this is allowed, and possibly add a check that > prevents this from never ending. It should not be a never ending loop (there are other exit conditions), but I agreed. What about limiting with nr_subbufs? diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c index 5326924615a4..aa89ec96e964 100644 --- a/kernel/trace/ring_buffer.c +++ b/kernel/trace/ring_buffer.c @@ -5630,7 +5630,8 @@ __rb_get_reader_page(struct ring_buffer_per_cpu *cpu_buffer) * a case where we will loop three times. There should be no * reason to loop four times (that I know of). */ - if (RB_WARN_ON(cpu_buffer, ++nr_loops > 3)) { + if (RB_WARN_ON(cpu_buffer, + ++nr_loops > (cpu_buffer->ring_meta ? cpu_buffer->nr_subbufs : 3))) { reader = NULL; goto out; } > > -- Steve > > > > */ > > - if (RB_WARN_ON(cpu_buffer, ++nr_loops > 3)) { > > + if (RB_WARN_ON(cpu_buffer, ++nr_loops > 3 && !cpu_buffer->ring_meta)) { > > reader = NULL; > > goto out; > > } > -- Masami Hiramatsu (Google) <[email protected]>
