Summary
Fixes PR 297976, "panic: flush_newblk_dep: Bad newblk", on UFS with soft updates.
flush_pagedep_deps() drops the soft updates lock while it obtains the vnode of a newly created directory with get_parent_vp(). The MKDIR_BODY dependency can complete during that window, which invalidates the diradd selected before the lock was dropped. Once the directory's allocdirect is retired, flush_newblk_dep() looks up the directory's first block by block number and can find an older dependency for a previous use of the same physical block.
The newblk hash is not unique by block number. When ffs_reallocblks() relocates a cluster, the completed allocindirs of the old blocks stay on the indirdep's ir_completehd until the indirect block pointer in the inode is written, while the old blocks are already free and can be allocated to a new directory. flush_newblk_dep() then finds a D_ALLOCINDIR where it expects a D_ALLOCDIRECT and panics.
The change:
- Keeps the vnode returned by get_parent_vp(), reacquires the soft updates lock and restarts dependency selection. The kept vnode is used only when the newly selected MKDIR_BODY dependency refers to the same inode.
- Enters flush_newblk_dep() with the soft updates lock held and passes it the associated diradd. MKDIR_BODY is rechecked after every operation that may drop the lock, so completion during the helper's retry loop cannot lead to another lookup of the retired allocation.
- Splits the work in two. flush_pagedep_deps() now only owns the kept vnode and the list of unfinished diradds. The dependency selection moves to a new flush_pagedep_deps1(), which never acquires or releases a vnode: it returns EJUSTRETURN with the inode number when it needs one, or with 0 when the kept vnode must be released first. Every call restarts the selection, so a vnode is only used for a diradd found while it was held. This also merges the two get_parent_vp() call sites into one.
- Asserts that flush_newblk_dep() is called with the vnode exclusively locked.
The original newblk dependency ordering is unchanged, and the existing assertion for an active MKDIR_BODY dependency that resolves to an unexpected dependency type is kept.
The second commit adds a stress2 reproducer, tools/test/stress2/misc/mkdir_blkreuse.sh, described under Test Plan.
Files: sys/ufs/ffs/ffs_softdep.c (+128 -40), tools/test/stress2/misc/mkdir_blkreuse.sh (new, 584 lines).
PR: 297976
Test Plan
All kernel testing was on releng/14.3, amd64 (AWS EC2, 4 vCPUs, 16 GB RAM), with INVARIANTS, INVARIANT_SUPPORT, WITNESS and DEBUG_VFS_LOCKS. The patch applies to main without changes. On main it was only compiled (the UFS module, with GENERIC options including INVARIANTS), not booted.
Reproducer
mkdir_blkreuse.sh runs on a small md(4)-backed UFS with soft updates and no journal. It runs three kinds of workers:
- Writers grow two interleaved files past UFS_NDADDR, so ffs_reallocblks() keeps relocating clusters referenced from indirect blocks.
- Directory workers create and remove subdirectories, driving ffs_vput_pair() into flush_pagedep_deps().
- Sync workers fsync() the directories while a flush is in progress.
The script header explains the setup details needed for a new directory to land on a just-freed block.
With dtrace=1 it also runs an oracle that counts the panic's precondition directly: a new directory's block 0 being allocated while the newblk hash still holds a dependency of a previous owner of that block. The count is also meaningful on a fixed kernel, where the precondition still happens but no longer panics.
To run it:
cd tools/test/stress2/misc testuser=nobody RUNTIME=10m dtrace=1 ./mkdir_blkreuse.sh
Results
| Kernel | Run | Result |
|---|---|---|
| releng/14.3, no fix | a few minutes | panic "flush_newblk_dep: Bad newblk" |
| main, no fix (reproduced independently by @pho) | panic, https://people.freebsd.org/~pho/stress/log/log0694.txt | |
| first revision of this fix | 2 min | no panic, about 7,000 precondition hits |
| this revision | 3 min, twice | no panic, 7,642 and 3,388 precondition hits |
@pho reproduced the panic independently on a main system with this reproducer, running its earlier write method: https://people.freebsd.org/~pho/stress/log/log0694.txt
The runs of this revision were done on a kernel that also carries the unrelated fdatasync()/O_DSYNC fixes mentioned below. Both panics without the fix were shown with the reproducer's earlier write method (see the note below), not with the current default.
Broader stress testing
The same kernel (this fix plus the fdatasync()/O_DSYNC fixes) also ran these stress2 tests. All passed. The ones that end with a file system check reported it clean, and there were no panics and no KASSERT failures:
- fdatasync.sh and fdatasync2.sh, each on UFS without soft updates, with soft updates, and with SU+J
- ftruncate2.sh
- fsync2.sh (soft updates and SU+J) and fsync3.sh
- gnop7.sh (soft updates and SU+J), gnop9.sh, gnop10.sh and fsck6.sh: forced unmounts and injected I/O errors, each followed by fsck
- 10-minute marcus.cfg loads on file systems mounted -o sync: soft updates, SU+J, and soft updates with a snapshot taken every 20 seconds
Note on the reproducer and fdatasync()
The writers need each new block on disk, with its dependency complete, before the block is relocated, while the file's inode stays unwritten. They used to get that from fdatasync(). That only worked because ffs_syncvnode() tested IN_SIZEMOD | IN_IBLKDATA against ip->i_flags (the chflags word) instead of ip->i_flag, so a data-only sync never wrote the inode.
Once that mix-up is fixed (D60134), fdatasync() writes the inode and the precondition stops occurring: 0 hits in 3 minutes. The reproducer therefore defaults to WALSYNC=direct: each 32 KB block is written as two 16 KB halves on an O_DIRECT descriptor. The first, partial write is sent to disk at once with bawrite(), which touches neither the inode nor the indirect block. The second write completes the block, which then goes through cluster_write() and ffs_reallocblks().
WALSYNC=fdatasync keeps the old method for kernels that still have the i_flag/i_flags mix-up. An attempt that let the syncer write the data, with kern.filedelay=1, produced no hits and was dropped.