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]>
