[v2] ring-buffer: Making persistent ring buffers robust

[PATCH v2 3/4] ring-buffer: Skip invalid sub-buffers when validating persistent ring buffer

Posted by Masami Hiramatsu (Google) 1 month, 2 weeks ago

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Skip invalid sub-buffers when validating the persistent ring buffer
instead of invalidate all ring buffers.

If the cache data in memory fails to be synchronized during a reboot,
the persistent ring buffer may become partially corrupted, but other
sub-buffers may still contain readable event data, allowing usersto
recover data from the corrupted ring buffer.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 kernel/trace/ring_buffer.c |   22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index d2b69221a94c..0ae2a5ad8c3e 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -2045,17 +2045,19 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
 		if (ret < 0) {
 			pr_info("Ring buffer meta [%d] invalid buffer page\n",
 				cpu_buffer->cpu);
-			goto invalid;
-		}
-
-		/* If the buffer has content, update pages_touched */
-		if (ret)
-			local_inc(&cpu_buffer->pages_touched);
-
-		entries += ret;
-		entry_bytes += local_read(&head_page->page->commit);
-		local_set(&cpu_buffer->head_page->entries, ret);
+			/* Instead of invalidate whole ring buffer, just clear this subbuffer. */
+			local_set(&head_page->entries, 0);
+			local_set(&head_page->page->commit, 0);
+			/* TODO: commit an event to mark this is broken. */
+		} else {
+			/* If the buffer has content, update pages_touched */
+			if (ret)
+				local_inc(&cpu_buffer->pages_touched);
 
+			entries += ret;
+			entry_bytes += local_read(&head_page->page->commit);
+			local_set(&cpu_buffer->head_page->entries, ret);
+		}
 		if (head_page == cpu_buffer->commit_page)
 			break;
 	}

Re: [PATCH v2 3/4] ring-buffer: Skip invalid sub-buffers when validating persistent ring buffer

Posted by Steven Rostedt 1 month, 1 week ago

On Wed, 18 Feb 2026 19:14:35 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> Skip invalid sub-buffers when validating the persistent ring buffer
> instead of invalidate all ring buffers.

  instead of discarding the entire ring buffer.


> 
> If the cache data in memory fails to be synchronized during a reboot,
> the persistent ring buffer may become partially corrupted, but other
> sub-buffers may still contain readable event data, allowing usersto
> recover data from the corrupted ring buffer.

                  ... contain readable event data. Only discard the
                  subbuffers that are found to be corrupted.

> 
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
>  kernel/trace/ring_buffer.c |   22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index d2b69221a94c..0ae2a5ad8c3e 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -2045,17 +2045,19 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
>  		if (ret < 0) {
>  			pr_info("Ring buffer meta [%d] invalid buffer page\n",
>  				cpu_buffer->cpu);
> -			goto invalid;
> -		}
> -
> -		/* If the buffer has content, update pages_touched */
> -		if (ret)
> -			local_inc(&cpu_buffer->pages_touched);
> -
> -		entries += ret;
> -		entry_bytes += local_read(&head_page->page->commit);
> -		local_set(&cpu_buffer->head_page->entries, ret);
> +			/* Instead of invalidate whole ring buffer, just clear this subbuffer. */
> +			local_set(&head_page->entries, 0);
> +			local_set(&head_page->page->commit, 0);
> +			/* TODO: commit an event to mark this is broken. */

Here's how to fix the TODO:

			local_set(&head_page->page->commit, RB_MISSED_EVENTS);

-- Steve


> +		} else {
> +			/* If the buffer has content, update pages_touched */
> +			if (ret)
> +				local_inc(&cpu_buffer->pages_touched);
>  
> +			entries += ret;
> +			entry_bytes += local_read(&head_page->page->commit);
> +			local_set(&cpu_buffer->head_page->entries, ret);
> +		}
>  		if (head_page == cpu_buffer->commit_page)
>  			break;
>  	}

Re: [PATCH v2 3/4] ring-buffer: Skip invalid sub-buffers when validating persistent ring buffer

Posted by Masami Hiramatsu (Google) 1 month, 1 week ago

On Fri, 20 Feb 2026 14:56:56 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Wed, 18 Feb 2026 19:14:35 +0900
> "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> 
> > From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > 
> > Skip invalid sub-buffers when validating the persistent ring buffer
> > instead of invalidate all ring buffers.
> 
>   instead of discarding the entire ring buffer.
> 
> 
> > 
> > If the cache data in memory fails to be synchronized during a reboot,
> > the persistent ring buffer may become partially corrupted, but other
> > sub-buffers may still contain readable event data, allowing usersto
> > recover data from the corrupted ring buffer.
> 
>                   ... contain readable event data. Only discard the
>                   subbuffers that are found to be corrupted.
> 
> > 
> > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > ---
> >  kernel/trace/ring_buffer.c |   22 ++++++++++++----------
> >  1 file changed, 12 insertions(+), 10 deletions(-)
> > 
> > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> > index d2b69221a94c..0ae2a5ad8c3e 100644
> > --- a/kernel/trace/ring_buffer.c
> > +++ b/kernel/trace/ring_buffer.c
> > @@ -2045,17 +2045,19 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
> >  		if (ret < 0) {
> >  			pr_info("Ring buffer meta [%d] invalid buffer page\n",
> >  				cpu_buffer->cpu);
> > -			goto invalid;
> > -		}
> > -
> > -		/* If the buffer has content, update pages_touched */
> > -		if (ret)
> > -			local_inc(&cpu_buffer->pages_touched);
> > -
> > -		entries += ret;
> > -		entry_bytes += local_read(&head_page->page->commit);
> > -		local_set(&cpu_buffer->head_page->entries, ret);
> > +			/* Instead of invalidate whole ring buffer, just clear this subbuffer. */
> > +			local_set(&head_page->entries, 0);
> > +			local_set(&head_page->page->commit, 0);
> > +			/* TODO: commit an event to mark this is broken. */
> 
> Here's how to fix the TODO:
> 
> 			local_set(&head_page->page->commit, RB_MISSED_EVENTS);

Ah, that's a nice flag!

Thanks!

> 
> -- Steve
> 
> 
> > +		} else {
> > +			/* If the buffer has content, update pages_touched */
> > +			if (ret)
> > +				local_inc(&cpu_buffer->pages_touched);
> >  
> > +			entries += ret;
> > +			entry_bytes += local_read(&head_page->page->commit);
> > +			local_set(&cpu_buffer->head_page->entries, ret);
> > +		}
> >  		if (head_page == cpu_buffer->commit_page)
> >  			break;
> >  	}
> 
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>