Re: [PATCH v3 4/4] selftests/ftrace: Add persistent ring buffer testcases

Masami Hiramatsu (Google) <[email protected]>
Newsgroups dev.linux.lists.sashiko-reviews,org.kernel.vger.linux-trace-kernel
Message-ID <[email protected]>
On Wed, 12 Aug 2026 00:57:31 +0000
[email protected] wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Test enters an infinite reboot loop if the persistent trace feature fails.
> --
> 
> --- Patch [4]: [PATCH v3 4/4] selftests/ftrace: Add persistent ring buffer testcases ---
> commit d365a5483f67b1295e536e6d26fd4ee7166e27b3
> Author: Masami Hiramatsu (Google) <[email protected]>
> 
> selftests/ftrace: Add persistent ring buffer testcases
> 
> Add test cases for persistent ring buffer and backup instance across guest
> crash/reboot. These verify trace data retention in the boot_map instance and
> preservation in the backup instance on subsequent boot.
> 
> > diff --git a/tools/testing/selftests/ftrace/boottime/tests/persistent-01-reserve-mem.sh b/tools/testing/selftests/ftrace/boottime/tests/persistent-01-reserve-mem.sh
> > new file mode 100644
> > index 0000000000000..49abaf518481f
> > --- /dev/null
> > +++ b/tools/testing/selftests/ftrace/boottime/tests/persistent-01-reserve-mem.sh
> > @@ -0,0 +1,28 @@
> 
> [ ... ]
> 
> > +
> > +# Check if Boot 1 marker was already written
> > +if grep -q "BOOT1_MARKER" "$TRACEDIR/instances/boot_map/trace" 2>/dev/null; then
> > +	# Second boot: verify persistent ring buffer content from first boot
> > +	echo "PASS: persistent-01-reserve-mem"
> > +	exit 0
> > +fi
> > +
> > +# First boot: write marker to persistent buffer and trigger kernel crash/reboot
> > +echo "BOOT1_MARKER" > "$TRACEDIR/instances/boot_map/trace_marker"
> > +sync
> > +
> > +# Trigger reboot to restart into second boot
> > +echo b > /proc/sysrq-trigger 2>/dev/null || echo c > /proc/sysrq-trigger 2>/dev/null || true
> 
> [Severity: Medium]
> If the persistent trace feature fails to retain the BOOT1_MARKER across
> reboot, could this script enter an infinite reboot loop?
> 
> Since the test falls through to the first-boot logic when the marker is
> missing, it appears it will write the marker and reboot again indefinitely.
> This might cause the test to run until it is killed by a global timeout
> rather than failing cleanly.

Yes, that is why I made the timeout command required. Since the test uses
only initramfs, we can not save the state on it. Of course we can use
virtio-fs or something like it, but it is too much. If we can handle such
failure via timeout, it is good enough.

> 
> > +sleep 5
> > +echo "FAIL: reboot trigger failed on first boot"
> > +exit 1
> > diff --git a/tools/testing/selftests/ftrace/boottime/tests/persistent-02-backup-instance.sh b/tools/testing/selftests/ftrace/boottime/tests/persistent-02-backup-instance.sh
> > new file mode 100644
> > index 0000000000000..0b2a2f03ce4f2
> > --- /dev/null
> > +++ b/tools/testing/selftests/ftrace/boottime/tests/persistent-02-backup-instance.sh
> > @@ -0,0 +1,40 @@
> 
> [ ... ]
> 
> > +
> > +# Check if BOOT1_MARKER is in boot_map/trace (indicates second boot)
> > +if grep -q "BOOT1_MARKER" "$TRACEDIR/instances/boot_map/trace" 2>/dev/null; then
> > +	# Second boot: verify BOOT1_MARKER was copied into backup/trace from Boot 1
> > +	if grep -q "BOOT1_MARKER" "$TRACEDIR/instances/backup/trace" 2>/dev/null; then
> > +		echo "PASS: persistent-02-backup-instance"
> > +		exit 0
> > +	else
> > +		echo "FAIL: BOOT1_MARKER found in boot_map/trace" \
> > +			"but missing from backup/trace on second boot"
> > +		exit 1
> > +	fi
> > +fi
> > +
> > +# First boot: write BOOT1_MARKER to boot_map and reboot via sysrq-trigger
> > +echo "BOOT1_MARKER" > "$TRACEDIR/instances/boot_map/trace_marker"
> > +sync
> > +
> > +# Trigger reboot to restart into second boot
> > +echo b > /proc/sysrq-trigger 2>/dev/null || echo c > /proc/sysrq-trigger 2>/dev/null || true
> 
> [Severity: Medium]
> Similarly, does this script also risk an infinite reboot loop if the marker
> is missing from boot_map/trace on the second boot?
> 
> It looks like it would also fall back to triggering another reboot instead
> of reporting a failure.

Ditto. It is eventually handled by timeout failure.

Thank you,

-- 
Masami Hiramatsu (Google) <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.