sysupgrade: quiesce the overlay before erasing the medium under it - #2301
Merged
Conversation
do_wipe_overlay hands flash_eraseall a partition that is still a mounted, read-write jffs2 with its garbage-collect thread running, so two agents erase and write the same medium at once. Both of them notice. The GC thread walks a filesystem being deleted underneath it: jffs2: Header CRC failed on REF_PRISTINE node at 0x00706874: Read 0xffffffff jffs2: notice: (573) jffs2_get_inode_nodes: Node header CRC failed at 0x886bac jffs2: Node totlen on flash (0xffffffff) != totlen from node ref (0x0000002c) jffs2: warning: (573) jffs2_do_read_inode_internal: no data nodes found for ino #20 and jffs2's own erase path reads back a block it has just erased and finds somebody else's data in it: jffs2: Newly-erased block contained word 0x20031985 at offset 0x003b0000 0x20031985 is a jffs2 cleanmarker -- magic 0x1985, nodetype 0x2003 -- the one `flash_eraseall -j` writes. The overlay cannot be unmounted. It is the upperdir and workdir of the overlayfs that is still root for init(1), majestic, dropbear and getty, so that superblock holds a reference to the mount for as long as they exist: umount is EBUSY by construction, and umount -l detaches it from our namespace while the superblock and the GC thread live on. pivot_root does not help and was never meant to -- it moves this script's own root off the partition it is about to overwrite, nobody else's, which is why enter_ramfs deliberately leaves the old root mounted at /mnt. Remounting it read-only is enough: jffs2_remount_fs() stops the garbage-collect thread on the way to ro, leaving flash_eraseall as the only thing touching the medium. Writes still arriving through the overlayfs get EROFS instead of reaching flash, which is what we want a second before a reboot that wipes it anyway. flash_eraseall writes the raw /dev/mtdN character device and never needed the mount. The mount point is looked up in /proc/mounts rather than assumed: after the pivot the overlay is at /mnt/overlay, and on the in-place fallback it is still at /overlay. A refused remount warns and erases live, which is exactly what this did before, so the fallback is never worse than the status quo. Measured on a lab hi3516ev300 with an overlay carrying real write history: 3 of 4 runs logged live-kernel jffs2 errors before, 0 of 5 after, with the reset itself still correct (overlay back to 328K, rw and the GC thread restored on the next boot, dmesg clean). An empty overlay is silent either way, which is why this hides on a bench that has just been reset. Refs #2300. Split out of #2298, which is about the unverified uImage read that actually bricks the camera; this is the other half and is independent.
PR Summary by Qodosysupgrade: remount overlay read-only before wiping rootfs_data
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
The mount-point lookup matched only /dev/mtdblockN, but general/overlay/init mounts the same partition as "ubi0:rootfs_data" on a NAND camera, so the quiesce silently did nothing on every UBI board while the script claimed otherwise. The race is the same there and so is the cure: ubifs_remount_ro() stops the background thread and commits, exactly as jffs2_remount_fs() stops the GC thread. Match both spellings, and cover the UBI one in the test suite -- it fails against the previous commit, so it pins the gap rather than restating it. Reported by Qodo on #2301.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2300.
do_wipe_overlay()handsflash_erasealla partition that is still a mounted, read-write jffs2 with its garbage-collect thread running. Two agents then erase and write the same medium at once, and both of them notice.The GC thread walks a filesystem being deleted underneath it:
and jffs2's own erase path reads back a block it has just erased and finds someone else's data in it:
0x20031985is a jffs2 cleanmarker — magic0x1985, nodetype0x2003— the oneflash_eraseall -jwrites.Why not just unmount it
It can't be. The jffs2 is the
upperdirandworkdirof the overlayfs that is still root for init(1), majestic, dropbear and getty:That superblock holds a reference to the mount for as long as those processes exist, so
umountisEBUSYby construction andumount -lonly detaches it from our namespace while the superblock and the GC thread live on.pivot_rootdoes not help here and was never meant to — it moves this script's root off the partition it is about to overwrite (#2251), nobody else's, which is whyenter_ramfs()deliberately leaves the old root mounted at/mnt. Nothing in the pivot changes in this PR.What this does instead
mount -o remount,roworks on it even with the overlayfs live on top, andjffs2_remount_fs()stops the garbage-collect thread on the way to read-only. Measured on the running camera:That leaves
flash_eraseallas the only thing touching the medium. Writes still arriving through the overlayfs getEROFSinstead of reaching flash, which is the outcome we want a second before a reboot that wipes it anyway.flash_eraseallwrites the raw/dev/mtdNcharacter device and never needed the mount.Shape notes:
/proc/mounts, not assumed: after the pivot the overlay is at/mnt/overlay, on the in-place fallback it is still at/overlay. A hardcoded path would silently remount nothing on one of the two paths. Confirmed on hardware — the patched run reports/mnt/overlay.awkandsedare already in the applet listenter_ramfs()stages, so this works after the pivot.rootfs_data, or a root that never mounted an overlay) → the lookup comes up empty and the function steps aside.Verification
On hardware — lab hi3516ev300, NOR, serial console,
ignore_loglevelon the cmdline (see below), the patched script pushed onto the camera andsysupgrade -n --webrun with an overlay pre-loaded with ~6 MB of write history each time:The reset itself is still correct after a patched run:
An empty overlay is silent either way — that is why a bare
flash_eraseallloop reproduces nothing and why this hides on a bench that has just been reset. It needs a populated one.In the test suite —
.github/scripts/test_sysupgrade.sh, 107 checks green. Five are new:Checked that they fail against the unpatched script rather than passing vacuously — four of the five do (the fifth is the no-op path, which is correct either way):
The harness gains a
remountbranch in themountstub (logged, withSTUB_REMOUNT_RCto inject a kernel that refuses), a jffs2 line in the fake/proc/mountsplusset_mounts no-overlayto remove it, and alogged_athelper so the ordering can be asserted rather than assumed.Scope
This does not touch #2298 — the camera that fails to come back after a reset does so because U-Boot boots an unverified uImage that occasionally has a bit flipped in it. One of the five runs above still needed a power cycle for that reason; its jffs2 side was clean. The two are independent.
Footnote
None of these kernel messages are visible on a stock camera:
/etc/sysctl.confsetskernel.printk = 3 3 1 3and S02sysctl applies it, but the live value is0 0 0 0on hi3516ev300, hi3516cv300 and gk7205v200 (ssc30kq keeps3 3 1 3).console_loglevel=0suppresses evenKERN_EMERG. Everything above neededignore_loglevelon the kernel cmdline to see at all. Worth fixing separately — a camera that panics is currently indistinguishable from one that hung.