From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 1D2DDE88D72 for ; Sat, 4 Apr 2026 00:32:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:Mime-Version:References:In-Reply-To:Message-Id:Subject:Cc:To: From:Date:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=BRJjEXigRr6fkKAxI3DyGsJb3Hr0VzxHpLRFoxtKVVo=; b=xL1Nx+uytsZBYi8HkE6yeW8Vh1 lEfTkVzjueK1+8UkynB2Phs2kwl6f+zQYpPMOQKH/+xyHdTUc5DIqkvFe+CiFHoYdVei5IoKsj4Cp Q5w4QMnNVliXQ8iNKCjLwnwq4cZF44m8aIc2cSVYsCfCvItf5GcraRBqjUvMCp5Gr+sgWhXtmgyM4 ACJdDEUutTcAihbE/lH84v8agQKrfXjQQ4uGE1/vm0aZaHdGAHF3s+FHpsDGxD8kGP51w5rNNc2Ld VivLiLbfx4YB5SlXmbQEOAcnDi3sg3CCwhbnVFjemoqArNLJ3NmoLRDVfZfl5WAojBMwkRFMVtQkh zIxedCWw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1w8owH-00000002mJN-3L6X; Sat, 04 Apr 2026 00:32:37 +0000 Received: from sea.source.kernel.org ([172.234.252.31]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1w8owF-00000002mJ1-3by3 for linux-arm-kernel@lists.infradead.org; Sat, 04 Apr 2026 00:32:37 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sea.source.kernel.org (Postfix) with ESMTP id AEAA540421; Sat, 4 Apr 2026 00:32:34 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 535E5C4CEF7; Sat, 4 Apr 2026 00:32:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1775262754; bh=NYESnWA8nyBfPWTrYfHvv3+N0G9f3yRvmIQsHVnbpNU=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=iSqwot39hb91fr0oK/3w8vmv9eltkDrgkhpICT0zlmWq9jeCsjuAPIxMB2h/wE9/+ EBjYW3VR8mYxr/D7q2/LFD0G7oiiK9GtMi8+HRjovNkMakiEKrWH36arOqcWcypaeh dKHBNXhMsCJm4x0dK7G5zDuvHmVwggiADsc4wFY+loAmTLa2iMrRLGXRdqpSXvCcan 0xGgltOmU4QeRoJNoaloE6Ck92HRzSEJaZvVsdL1HRPi2Ee05rmhPOe+e4c/eNM/Y+ 70MSpBmOtaQXyxuadU9oZmNsSsNn/uSrChpkmjsQCswZi+tCeORSEqjtBKLL2Np6UX xyavJB2j72kfQ== Date: Sat, 4 Apr 2026 09:32:25 +0900 From: Masami Hiramatsu (Google) To: Steven Rostedt Cc: Catalin Marinas , Will Deacon , Mathieu Desnoyers , linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org, Ian Rogers , linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v15 4/5] ring-buffer: Add persistent ring buffer invalid-page inject test Message-Id: <20260404093225.213ca848800cb47d388edf05@kernel.org> In-Reply-To: <20260401184055.2f932674@gandalf.local.home> References: <177494615421.71933.3679132057004156013.stgit@mhiramat.tok.corp.google.com> <177494619065.71933.9842685686800241005.stgit@mhiramat.tok.corp.google.com> <20260401184055.2f932674@gandalf.local.home> X-Mailer: Sylpheed 3.8.0beta1 (GTK+ 2.24.33; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20260403_173235_951787_6E5809DC X-CRM114-Status: GOOD ( 55.93 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, 1 Apr 2026 18:40:55 -0400 Steven Rostedt 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)" wrote: > > > From: Masami Hiramatsu (Google) > > > > 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) > > --- > > 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)