[PATCH v4 0/3] ring-buffer: Making persistent ring buffers robust

[PATCH v4 0/3] ring-buffer: Making persistent ring buffers robust

[Masami Hiramatsu (Google)]

Hi,

Here is the 4th version of improvement patches for making persistent
ring buffers robust to failures.
The previous version is here:

https://lore.kernel.org/linux-trace-kernel/177186335195.133407.907308822749006594.stgit@devnote2/

In this version, I fixed to move rb_commit_index() after the
definition of ring_buffer_per_cpu[2/3].

Thank you,

---

Masami Hiramatsu (Google) (3):
      ring-buffer: Flush and stop persistent ring buffer on panic
      ring-buffer: Handle RB_MISSED_* flags on commit field correctly
      ring-buffer: Skip invalid sub-buffers when validating persistent ring buffer


 kernel/trace/ring_buffer.c |   72 +++++++++++++++++++++++++++++---------------
 1 file changed, 47 insertions(+), 25 deletions(-)

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

[PATCH v4 1/3] ring-buffer: Flush and stop persistent ring buffer on panic

[Masami Hiramatsu (Google)]

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

On real hardware, panic and machine reboot may not flush hardware cache
to memory. This means the persistent ring buffer, which relies on a
coherent state of memory, may not have its events written to the buffer
and they may be lost. Moreover, there may be inconsistency with the
counters which are used for validation of the integrity of the
persistent ring buffer which may cause all data to be discarded.

To avoid this issue, stop recording of the ring buffer on panic and
flush the cache of the ring buffer's memory.

Fixes: e645535a954a ("tracing: Add option to use memmapped memory for trace boot instance")
Cc: stable@vger.kernel.org
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 Changes in v3:
   - update patch description.
---
 kernel/trace/ring_buffer.c |   21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index f16f053ef77d..3eb124c93d72 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -6,6 +6,7 @@
  */
 #include <linux/sched/isolation.h>
 #include <linux/trace_recursion.h>
+#include <linux/panic_notifier.h>
 #include <linux/trace_events.h>
 #include <linux/ring_buffer.h>
 #include <linux/trace_clock.h>
@@ -589,6 +590,7 @@ struct trace_buffer {
 
 	unsigned long			range_addr_start;
 	unsigned long			range_addr_end;
+	struct notifier_block		flush_nb;
 
 	struct ring_buffer_meta		*meta;
 
@@ -2471,6 +2473,16 @@ static void rb_free_cpu_buffer(struct ring_buffer_per_cpu *cpu_buffer)
 	kfree(cpu_buffer);
 }
 
+static int rb_flush_buffer_cb(struct notifier_block *nb, unsigned long event, void *data)
+{
+	struct trace_buffer *buffer = container_of(nb, struct trace_buffer, flush_nb);
+
+	ring_buffer_record_disable(buffer);
+	flush_kernel_vmap_range((void *)buffer->range_addr_start,
+				buffer->range_addr_end - buffer->range_addr_start);
+	return NOTIFY_DONE;
+}
+
 static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
 					 int order, unsigned long start,
 					 unsigned long end,
@@ -2590,6 +2602,12 @@ static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
 
 	mutex_init(&buffer->mutex);
 
+	/* Persistent ring buffer needs to flush cache before reboot. */
+	if (start & end) {
+		buffer->flush_nb.notifier_call = rb_flush_buffer_cb;
+		atomic_notifier_chain_register(&panic_notifier_list, &buffer->flush_nb);
+	}
+
 	return_ptr(buffer);
 
  fail_free_buffers:
@@ -2677,6 +2695,9 @@ ring_buffer_free(struct trace_buffer *buffer)
 {
 	int cpu;
 
+	if (buffer->range_addr_start && buffer->range_addr_end)
+		atomic_notifier_chain_unregister(&panic_notifier_list, &buffer->flush_nb);
+
 	cpuhp_state_remove_instance(CPUHP_TRACE_RB_PREPARE, &buffer->node);
 
 	irq_work_sync(&buffer->irq_work.work);

Re: [PATCH v4 1/3] ring-buffer: Flush and stop persistent ring buffer on panic

[Masami Hiramatsu]

On Tue, 24 Feb 2026 15:52:42 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> On real hardware, panic and machine reboot may not flush hardware cache
> to memory. This means the persistent ring buffer, which relies on a
> coherent state of memory, may not have its events written to the buffer
> and they may be lost. Moreover, there may be inconsistency with the
> counters which are used for validation of the integrity of the
> persistent ring buffer which may cause all data to be discarded.
> 
> To avoid this issue, stop recording of the ring buffer on panic and
> flush the cache of the ring buffer's memory.
> 
> Fixes: e645535a954a ("tracing: Add option to use memmapped memory for trace boot instance")
> Cc: stable@vger.kernel.org
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
>  Changes in v3:
>    - update patch description.
> ---
>  kernel/trace/ring_buffer.c |   21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index f16f053ef77d..3eb124c93d72 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -6,6 +6,7 @@
>   */
>  #include <linux/sched/isolation.h>
>  #include <linux/trace_recursion.h>
> +#include <linux/panic_notifier.h>
>  #include <linux/trace_events.h>
>  #include <linux/ring_buffer.h>
>  #include <linux/trace_clock.h>
> @@ -589,6 +590,7 @@ struct trace_buffer {
>  
>  	unsigned long			range_addr_start;
>  	unsigned long			range_addr_end;
> +	struct notifier_block		flush_nb;
>  
>  	struct ring_buffer_meta		*meta;
>  
> @@ -2471,6 +2473,16 @@ static void rb_free_cpu_buffer(struct ring_buffer_per_cpu *cpu_buffer)
>  	kfree(cpu_buffer);
>  }
>  
> +static int rb_flush_buffer_cb(struct notifier_block *nb, unsigned long event, void *data)
> +{
> +	struct trace_buffer *buffer = container_of(nb, struct trace_buffer, flush_nb);
> +
> +	ring_buffer_record_disable(buffer);

I found this was a wrong API. I have to use ring_buffer_record_off().

> +	flush_kernel_vmap_range((void *)buffer->range_addr_start,
> +				buffer->range_addr_end - buffer->range_addr_start);

This does nothing on arm64.



Thanks,

> +	return NOTIFY_DONE;
> +}
> +
>  static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
>  					 int order, unsigned long start,
>  					 unsigned long end,
> @@ -2590,6 +2602,12 @@ static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
>  
>  	mutex_init(&buffer->mutex);
>  
> +	/* Persistent ring buffer needs to flush cache before reboot. */
> +	if (start & end) {
> +		buffer->flush_nb.notifier_call = rb_flush_buffer_cb;
> +		atomic_notifier_chain_register(&panic_notifier_list, &buffer->flush_nb);
> +	}
> +
>  	return_ptr(buffer);
>  
>   fail_free_buffers:
> @@ -2677,6 +2695,9 @@ ring_buffer_free(struct trace_buffer *buffer)
>  {
>  	int cpu;
>  
> +	if (buffer->range_addr_start && buffer->range_addr_end)
> +		atomic_notifier_chain_unregister(&panic_notifier_list, &buffer->flush_nb);
> +
>  	cpuhp_state_remove_instance(CPUHP_TRACE_RB_PREPARE, &buffer->node);
>  
>  	irq_work_sync(&buffer->irq_work.work);
> 


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

[PATCH v4 2/3] ring-buffer: Handle RB_MISSED_* flags on commit field correctly

[Masami Hiramatsu (Google)]

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

Since the MSBs of rb_data_page::commit are used for storing
RB_MISSED_EVENTS and RB_MISSED_STORED, we need to mask out those bits
when it is used for finding the size of data pages.

Fixes: 5f3b6e839f3c ("ring-buffer: Validate boot range memory events")
Fixes: 5b7be9c709e1 ("ring-buffer: Add test to validate the time stamp deltas")
Cc: stable@vger.kernel.org
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 Changes in v4:
   - Fix to move rb_commit_index() after ring_buffer_per_cpu definition.
---
 kernel/trace/ring_buffer.c |   32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 3eb124c93d72..67bc652ba02a 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -395,6 +395,12 @@ static __always_inline unsigned int rb_page_commit(struct buffer_page *bpage)
 	return local_read(&bpage->page->commit);
 }
 
+/* Size is determined by what has been committed */
+static __always_inline unsigned rb_page_size(struct buffer_page *bpage)
+{
+	return rb_page_commit(bpage) & ~RB_MISSED_MASK;
+}
+
 static void free_buffer_page(struct buffer_page *bpage)
 {
 	/* Range pages are not to be freed */
@@ -614,6 +620,12 @@ struct ring_buffer_iter {
 	int				missed_events;
 };
 
+static __always_inline unsigned
+rb_commit_index(struct ring_buffer_per_cpu *cpu_buffer)
+{
+	return rb_page_commit(cpu_buffer->commit_page);
+}
+
 int ring_buffer_print_page_header(struct trace_buffer *buffer, struct trace_seq *s)
 {
 	struct buffer_data_page field;
@@ -1907,7 +1919,7 @@ static int rb_validate_buffer(struct buffer_data_page *dpage, int cpu)
 	u64 delta;
 	int tail;
 
-	tail = local_read(&dpage->commit);
+	tail = local_read(&dpage->commit) & ~RB_MISSED_MASK;
 	return rb_read_data_buffer(dpage, tail, cpu, &ts, &delta);
 }
 
@@ -1934,7 +1946,7 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
 		goto invalid;
 	}
 	entries += ret;
-	entry_bytes += local_read(&cpu_buffer->reader_page->page->commit);
+	entry_bytes += rb_page_size(cpu_buffer->reader_page);
 	local_set(&cpu_buffer->reader_page->entries, ret);
 
 	ts = head_page->page->time_stamp;
@@ -2054,7 +2066,7 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
 			local_inc(&cpu_buffer->pages_touched);
 
 		entries += ret;
-		entry_bytes += local_read(&head_page->page->commit);
+		entry_bytes += rb_page_size(head_page);
 		local_set(&cpu_buffer->head_page->entries, ret);
 
 		if (head_page == cpu_buffer->commit_page)
@@ -3257,18 +3269,6 @@ rb_iter_head_event(struct ring_buffer_iter *iter)
 	return NULL;
 }
 
-/* Size is determined by what has been committed */
-static __always_inline unsigned rb_page_size(struct buffer_page *bpage)
-{
-	return rb_page_commit(bpage) & ~RB_MISSED_MASK;
-}
-
-static __always_inline unsigned
-rb_commit_index(struct ring_buffer_per_cpu *cpu_buffer)
-{
-	return rb_page_commit(cpu_buffer->commit_page);
-}
-
 static __always_inline unsigned
 rb_event_index(struct ring_buffer_per_cpu *cpu_buffer, struct ring_buffer_event *event)
 {
@@ -4433,7 +4433,7 @@ static void check_buffer(struct ring_buffer_per_cpu *cpu_buffer,
 
 	if (tail == CHECK_FULL_PAGE) {
 		full = true;
-		tail = local_read(&bpage->commit);
+		tail = local_read(&bpage->commit) & ~RB_MISSED_MASK;
 	} else if (info->add_timestamp &
 		   (RB_ADD_STAMP_FORCE | RB_ADD_STAMP_ABSOLUTE)) {
 		/* Ignore events with absolute time stamps */

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

[Masami Hiramatsu (Google)]

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

Skip invalid sub-buffers when validating the persistent ring buffer
instead of discarding the entire ring buffer. Also, mark there are
missed events on the discarded 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. Only discard the
subbuffersa that ar found to be corrupted.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
  Changes in v3:
  - Record missed data event on commit.
---
 kernel/trace/ring_buffer.c |   21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 67bc652ba02a..72f82e2e6740 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -2058,17 +2058,18 @@ 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 += rb_page_size(head_page);
-		local_set(&cpu_buffer->head_page->entries, ret);
+			/* Instead of discard whole ring buffer, discard only this sub-buffer. */
+			local_set(&head_page->entries, 0);
+			local_set(&head_page->page->commit, RB_MISSED_EVENTS);
+		} else {
+			/* If the buffer has content, update pages_touched */
+			if (ret)
+				local_inc(&cpu_buffer->pages_touched);
 
+			entries += ret;
+			entry_bytes += rb_page_size(head_page);
+			local_set(&cpu_buffer->head_page->entries, ret);
+		}
 		if (head_page == cpu_buffer->commit_page)
 			break;
 	}

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

[Masami Hiramatsu]

On Tue, 24 Feb 2026 15:52:57 +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 discarding the entire ring buffer. Also, mark there are
> missed events on the discarded 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. Only discard the
> subbuffersa that ar found to be corrupted.
> 
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
>   Changes in v3:
>   - Record missed data event on commit.
> ---
>  kernel/trace/ring_buffer.c |   21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 67bc652ba02a..72f82e2e6740 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -2058,17 +2058,18 @@ 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);

This pr_info() should be removed too, because multiple pages
can be discarded.

Thanks,

> -			goto invalid;
> -		}
> -
> -		/* If the buffer has content, update pages_touched */
> -		if (ret)
> -			local_inc(&cpu_buffer->pages_touched);
> -
> -		entries += ret;
> -		entry_bytes += rb_page_size(head_page);
> -		local_set(&cpu_buffer->head_page->entries, ret);
> +			/* Instead of discard whole ring buffer, discard only this sub-buffer. */
> +			local_set(&head_page->entries, 0);
> +			local_set(&head_page->page->commit, RB_MISSED_EVENTS);
> +		} else {
> +			/* If the buffer has content, update pages_touched */
> +			if (ret)
> +				local_inc(&cpu_buffer->pages_touched);
>  
> +			entries += ret;
> +			entry_bytes += rb_page_size(head_page);
> +			local_set(&cpu_buffer->head_page->entries, ret);
> +		}
>  		if (head_page == cpu_buffer->commit_page)
>  			break;
>  	}
> 


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

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

[Steven Rostedt]

On Thu, 26 Feb 2026 21:58:33 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:

> > --- a/kernel/trace/ring_buffer.c
> > +++ b/kernel/trace/ring_buffer.c
> > @@ -2058,17 +2058,18 @@ 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);  
> 
> This pr_info() should be removed too, because multiple pages
> can be discarded.

I wouldn't discard it, but instead add a "once" variable, where it shows it
only once per CPU.

-- Steve

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

[Masami Hiramatsu]

On Thu, 26 Feb 2026 12:36:55 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Thu, 26 Feb 2026 21:58:33 +0900
> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> 
> > > --- a/kernel/trace/ring_buffer.c
> > > +++ b/kernel/trace/ring_buffer.c
> > > @@ -2058,17 +2058,18 @@ 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);  
> > 
> > This pr_info() should be removed too, because multiple pages
> > can be discarded.
> 
> I wouldn't discard it, but instead add a "once" variable, where it shows it
> only once per CPU.

OK, let me update it. 

Thanks!

> 
> -- Steve


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