On Wed, 1 Apr 2026 18:40:55 -0400 Steven Rostedt <[email protected]> wrote:
> > I replied with mostly grammar fixes and some rewrites of text. Thanks for reviewing! Let me update it. > > On Tue, 31 Mar 2026 17:36:30 +0900 > "Masami Hiramatsu (Google)" <[email protected]> wrote: > > > From: Masami Hiramatsu (Google) <[email protected]> > > > > Add a self-destractive test for the persistent ring buffer. > > "self-destractive"? Do you mean "self-destructive"? > > Probably better to call it a "self-corrupting test". > > > > > This will inject erroneous value to some sub-buffer pages (where > > inject an erroneous > > > the index is even or multiples of 5) in the persistent ring buffer > > when kernel gets panic, and check whether the number of detected > > when the kernel panics, and checks whether > > > invalid pages and the total entry_bytes are the same as recorded > > same as the recorded > > > values after reboot. > > > > This can ensure the kernel correctly recover partially corrupted > > This ensures that the kernel can correctly recover a partially corrupted > > > persistent ring buffer when boot. > > after a reboot or panic. > > > > > The test only runs on the persistent ring buffer whose name is > > "ptracingtest". And user has to fill it up with events before > > The user has to fill it with events before a kernel panic. > > > kernel panics. > > > > To run the test, enable CONFIG_RING_BUFFER_PERSISTENT_INJECT > > and you have to setup the kernel cmdline; > > and add the following kernel cmdline: > > Note, it's more proper to use a colon (:) than a semi-colon (;) when > referencing an example on the next line. OK, > > > > > reserve_mem=20M:2M:trace trace_instance=ptracingtest^traceoff@trace > > panic=1 > > > > And run following commands after the 1st boot; > > Run the following commands after the 1st boot: > > > > > cd /sys/kernel/tracing/instances/ptracingtest > > echo 1 > tracing_on > > echo 1 > events/enable > > sleep 3 > > echo c > /proc/sysrq-trigger > > > > After panic message, the kernel will reboot and run the verification > > on the persistent ring buffer, e.g. > > > > Ring buffer meta [2] invalid buffer page detected > > Ring buffer meta [2] is from previous boot! (318 pages discarded) > > Ring buffer testing [2] invalid pages: PASSED (318/318) > > Ring buffer testing [2] entry_bytes: PASSED (1300476/1300476) > > > > Signed-off-by: Masami Hiramatsu (Google) <[email protected]> > > --- > > Changes in v15: > > - Use pr_warn() for test result. > > - Inject errors on the page index is multiples of 5 so that > > this can reproduce contiguous empty pages. > > Changes in v14: > > - Rename config to CONFIG_RING_BUFFER_PERSISTENT_INJECT. > > - Clear meta->nr_invalid/entry_bytes after testing. > > - Add test commands in config comment. > > Changes in v10: > > - Add entry_bytes test. > > - Do not compile test code if CONFIG_RING_BUFFER_PERSISTENT_SELFTEST=n. > > Changes in v9: > > - Test also reader pages. > > --- > > include/linux/ring_buffer.h | 1 + > > kernel/trace/Kconfig | 31 ++++++++++++++++++ > > kernel/trace/ring_buffer.c | 74 > > +++++++++++++++++++++++++++++++++++++++++++ > > kernel/trace/trace.c | 4 ++ > > 4 files changed, 110 insertions(+) > > > > diff --git a/include/linux/ring_buffer.h b/include/linux/ring_buffer.h > > index 994f52b34344..0670742b2d60 100644 > > --- a/include/linux/ring_buffer.h > > +++ b/include/linux/ring_buffer.h > > @@ -238,6 +238,7 @@ int ring_buffer_subbuf_size_get(struct trace_buffer > > *buffer); > > > > enum ring_buffer_flags { > > RB_FL_OVERWRITE = 1 << 0, > > + RB_FL_TESTING = 1 << 1, > > }; > > > > #ifdef CONFIG_RING_BUFFER > > diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig > > index e130da35808f..07305ed6d745 100644 > > --- a/kernel/trace/Kconfig > > +++ b/kernel/trace/Kconfig > > @@ -1202,6 +1202,37 @@ config RING_BUFFER_VALIDATE_TIME_DELTAS > > Only say Y if you understand what this does, and you > > still want it enabled. Otherwise say N > > > > +config RING_BUFFER_PERSISTENT_INJECT > > + bool "Enable persistent ring buffer error injection test" > > + depends on RING_BUFFER > > + help > > + Run a selftest on the persistent ring buffer which names > > + "ptracingtest" (and its backup) when panic_on_reboot by > > Does this do anything with the backup? This meant that if you make a backup of ptracingtest (instance=backup=ptracingtest) the check runs on backup instance too. But it may be redundant. I'll drop it. > > > + invalidating ring buffer pages. > > This option will have the kernel check if the persistent ring > buffer is named "ptracingtest", and if so, it will corrupt some > of its pages on a kernel panic. This is used to test if the > persistent ring buffer can recover from some of its sub-buffers > being corrupted. > > [space] > > > + To use this, boot kernel with "ptracingtest" persistent > > , boot a kernel with a "ptracingtest" persistent > > > + ring buffer, e.g. > > + > > + reserve_mem=20M:2M:trace trace_instance=ptracingtest@trace panic=1 > > + > > + And after the 1st boot, run test command, like; > > , run the following commands: > > > + > > + cd /sys/kernel/tracing/instances/ptracingtest > > + echo 1 > events/enable > > + echo 1 > tracing_on > > + sleep 3 > > + echo c > /proc/sysrq-trigger > > + > > + After panic message, the kernel reboots and show test results > > + on the boot log. > > After the panic message, the kernel will reboot and will show the > test results in the console output. > > > + > > + Note that user has to enable events on the persistent ring > > + buffer manually to fill up ring buffers before rebooting. > > Note that events for the ring buffer needs to be enabled prior to > crashing the kernel so that the ring buffer has content that the > test will corrupt. > > > + Since this invalidates the data on test target ring buffer, > > + "ptracingtest" persistent ring buffer must not be used for > > + actual tracing, but only for testing. > > As the test will corrupt events in the "ptracingtest" persistent > ring buffer, it should not be used for any other purpose other > than his test. > > > > + > > + If unsure, say N > > + > > config MMIOTRACE_TEST > > tristate "Test module for mmiotrace" > > depends on MMIOTRACE && m > > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c > > index 5ff632ca3858..fb098b0b4505 100644 > > --- a/kernel/trace/ring_buffer.c > > +++ b/kernel/trace/ring_buffer.c > > @@ -64,6 +64,10 @@ struct ring_buffer_cpu_meta { > > unsigned long commit_buffer; > > __u32 subbuf_size; > > __u32 nr_subbufs; > > +#ifdef CONFIG_RING_BUFFER_PERSISTENT_INJECT > > + __u32 nr_invalid; > > + __u32 entry_bytes; > > +#endif > > int buffers[]; > > }; > > > > @@ -2079,6 +2083,21 @@ static void rb_meta_validate_events(struct > > ring_buffer_per_cpu *cpu_buffer) if (discarded) > > pr_cont(" (%d pages discarded)", discarded); > > pr_cont("\n"); > > + > > +#ifdef CONFIG_RING_BUFFER_PERSISTENT_INJECT > > + if (meta->nr_invalid) > > + pr_warn("Ring buffer testing [%d] invalid pages: %s (%d/%d)\n", > > + cpu_buffer->cpu, > > + (discarded == meta->nr_invalid) ? "PASSED" : "FAILED", > > + discarded, meta->nr_invalid); > > + if (meta->entry_bytes) > > + pr_warn("Ring buffer testing [%d] entry_bytes: %s (%ld/%ld)\n", > > + cpu_buffer->cpu, > > + (entry_bytes == meta->entry_bytes) ? "PASSED" : > > "FAILED", > > + (long)entry_bytes, (long)meta->entry_bytes); > > + meta->nr_invalid = 0; > > + meta->entry_bytes = 0; > > +#endif > > return; > > > > invalid: > > @@ -2559,12 +2578,67 @@ static void rb_free_cpu_buffer(struct > > ring_buffer_per_cpu *cpu_buffer) kfree(cpu_buffer); > > } > > > > +#ifdef CONFIG_RING_BUFFER_PERSISTENT_INJECT > > +static void rb_test_inject_invalid_pages(struct trace_buffer *buffer) > > +{ > > + struct ring_buffer_per_cpu *cpu_buffer; > > + struct ring_buffer_cpu_meta *meta; > > + struct buffer_data_page *dpage; > > + u32 entry_bytes = 0; > > + unsigned long ptr; > > + int subbuf_size; > > + int invalid = 0; > > + int cpu; > > + int i; > > + > > + if (!(buffer->flags & RB_FL_TESTING)) > > + return; > > + > > + guard(preempt)(); > > + cpu = smp_processor_id(); > > + > > + cpu_buffer = buffer->buffers[cpu]; > > + meta = cpu_buffer->ring_meta; > > + ptr = (unsigned long)rb_subbufs_from_meta(meta); > > + subbuf_size = meta->subbuf_size; > > + > > + for (i = 0; i < meta->nr_subbufs; i++) { > > + int idx = meta->buffers[i]; > > + > > + dpage = (void *)(ptr + idx * subbuf_size); > > + /* Skip unused pages */ > > + if (!local_read(&dpage->commit)) > > + continue; > > + > > + /* > > + * Invalidate even pages or multiples of 5. This will lead 3 > > This will cause 3 OK, thanks for your comments. I'll update it according your review. Thank you, > > -- Steve > > > + * contiguous invalidated(empty) pages. > > + */ > > + if (!(i & 0x1) || !(i % 5)) { > > + local_add(subbuf_size + 1, &dpage->commit); > > + invalid++; > > + } else { > > + /* Count total commit bytes. */ > > + entry_bytes += local_read(&dpage->commit); > > + } > > + } > > + > > + pr_info("Inject invalidated %d pages on CPU%d, total size: %ld\n", > > + invalid, cpu, (long)entry_bytes); > > + meta->nr_invalid = invalid; > > + meta->entry_bytes = entry_bytes; > > +} -- Masami Hiramatsu (Google) <[email protected]>
