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

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

On Wed, 12 Aug 2026 00:57:31 +0000
sashiko-bot@kernel.org 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) <mhiramat@kernel.org>
> 
> 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) <mhiramat@kernel.org>