Summary
On UFS, fdatasync(2) and writes to a descriptor opened with O_DSYNC could return success while the new file size and the block pointers to the new data were still only in memory. On soft updates file systems the same held for O_SYNC writes. A crash before the syncer caught up lost data the application had been told was on stable storage. Appending and then calling fdatasync(), as log writers and database WALs do, hits every one of these cases.
The series fixes four separate problems and adds a test. Git branch: https://github.com/sobomax/freebsd/tree/ufs-fdatasync-iflag
1. ufs: make fdatasync(2) and O_DSYNC writes update the inode
ffs_syncvnode() (the DATA_ONLY case) and ffs_write() (IO_DATASYNC) decide whether the inode must be written by testing IN_SIZEMOD | IN_IBLKDATA against ip->i_flags, the chflags(2) word, instead of ip->i_flag. Those bits alias UF_READONLY and UF_ARCHIVE:
- For ordinary files the test was always false, so the inode was never written.
- Files with urdonly or uarchive set got a synchronous inode write on every call instead.
journal_mount() has the same mix-up and sets IN_MODIFIED in i_flags. That only left UF_OPAQUE on the in-core .sujournal inode, because the ffs_update() right after it is synchronous anyway.
The fix is one letter in each of the three places. It uses UFS_INODE_SET_FLAG() in journal_mount().
Fixes: 7428630b757, 52488b51489, e7347be9e34, 113db2dddb7
2. ufs: make fdatasync(2) write dirty indirect blocks
A data-only ffs_syncvnode() skipped dirty indirect blocks without soft updates. With soft updates, it stopped making passes once only indirect blocks were left dirty.
Indirect blocks only become dirty when the block pointers in them change, and those pointers are what makes a new block reachable. So an append or hole fill beyond the direct blocks had its data and size on disk, but not the pointer to the new block.
The fix treats indirect blocks the same way in data-only and full syncs, as the loop did before 2f514f92cf0. Plain overwrites, the case data-only sync is meant to speed up, don't dirty indirect blocks, so they still write neither indirect blocks nor the inode.
Fixes: 2f514f92cf0
3. ufs: flush soft updates dependencies for O_SYNC and O_DSYNC writes
For an IO_SYNC write, ffs_write() writes the data with bwrite() and then the inode with ffs_update(vp, 1). With soft updates, a new block pointer (and, for an extending write, the size) is rolled back in the buffer being written until the block's dependencies, such as its cylinder group map, are on disk. ffs_update() still clears IN_SIZEMOD and IN_IBLKDATA. So the correct inode was only written later, by the syncer. The same happened to the pointer in an indirect block, which ffs_balloc() writes synchronously but rolled back.
With soft updates, a synchronous write now finishes with ffs_syncvnode(), as fsync(2) and fdatasync(2) do: DATA_ONLY for O_DSYNC, a full sync for O_SYNC. The inode flags can't be used to skip this, because allocating a block below an existing indirect block sets neither of them. ffs_syncvnode() only drops the vnode lock for directory dependencies, and a KASSERT checks that it never returns ERELOOKUP here.
4. ufs: do not let a rolled back inode write end a data-only sync
With soft updates, ffs_syncvnode() writes the inode between passes while buffers are still dirty. That write can go out with new block pointers rolled back, yet it clears IN_SIZEMOD and IN_IBLKDATA.
This is exposed by change 2 on an append that allocates a file's first indirect block. The inode goes out while the new indirect block is still unwritten, so it is written with the new pointer rolled back. The indirect block is written in a later pass. The final check then sees no flags set and skips the inode, which is never written with the real pointer.
The fix: note on entry whether a data-only sync has to write the inode, and do the final write in that case too.
5. tests: add UFS synchronous write durability tests
tests/sys/fs/ufs/sync_test (ATF, 48 cases) checks that once fsync() or fdatasync() returns, or a write with O_SYNC or O_DSYNC returns, the new data, the file size and every block pointer leading to the data are on disk.
The cases cover four scenarios, each with the four sync methods, on UFS without soft updates, with soft updates and with SU+J:
- appending a block
- filling a hole
- appending the first block that needs an indirect block
- appending below an existing indirect block
Each case runs on an md(4)-backed file system. Right after the call returns, it reads the inode, the indirect block and the data block straight from the device with libufs. There is no cache in front of the disk device, so this shows what would survive a crash at that moment without crashing anything.
Files: sys/ufs/ffs/ffs_vnops.c, sys/ufs/ffs/ffs_softdep.c (2 lines), tests/sys/fs/ufs/ (new), tests/sys/fs/Makefile, etc/mtree/BSD.tests.dist.
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, with the series cherry-picked. The changes apply to main without conflicts. On main, the UFS module was only compiled, not booted.
sync_test
sync_test passes all 48 cases with the full series. Adding the fixes one at a time shows each one's effect:
| Kernel | Passed | Still failing |
|---|---|---|
| change 1 only | 26/48 | O_SYNC/O_DSYNC on soft updates and SU+J (16); fdatasync() past the direct blocks on all three file system types (6) |
| changes 1 + 3 | 38/48 | fdatasync() and O_DSYNC past the direct blocks (10) |
| changes 1 + 3 + 2 | 46/48 | fdatasync() on soft updates and SU+J when the append allocates the first indirect block (2) |
| full series | 48/48 | none |
- Before the series: the original version of the test failed every fdatasync() and O_DSYNC case. The on-disk inode still had the old size and a zero pointer for the new block.
- Test controls: in every case the test first checks what it reads after a plain fsync(), so it can tell a written inode from an unwritten one. A build of the test with fdatasync() replaced by fsync() passed on the unfixed kernel.
- Repeats: the two cases change 4 fixes failed 5 out of 5 times before it and passed 5 out of 5 times after it.
- DTrace: it confirmed the mechanism behind change 3, the rolled-back O_DSYNC inode write (buf_start() in ffs_geom_strategy() replaces the new di_size in the outgoing buffer), and behind change 4, the intermediate inode write that clears the flags before the indirect block is written.
stress2
These all passed on a kernel with the full series. The ones that end with a file system check reported it clean, and there were no panics, KASSERT failures or new WITNESS warnings attributable to the series:
- fdatasync.sh and fdatasync2.sh, each on UFS without soft updates, with soft updates and with SU+J, each followed by a clean fsck_ffs -fn
- 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, where every write has IO_SYNC and takes the new path from change 3: soft updates, SU+J, and soft updates with a snapshot taken every 20 seconds. DTrace counted about 14,000 ffs_syncvnode() calls per second from ffs_write() on soft updates, which confirms that path was exercised.
WITNESS reported one snaplk -> bufwait lock order reversal during the snapshot run. Both stacks (ffs_snapshot() -> softdep_sync_metadata(), and a full fsync -> ffs_copyonwrite()) go through code this series does not change, so it appears to predate the series. It has not yet been checked on a kernel without the series.
Performance
Not benchmarked against a kernel without the series. Changes 2 and 3 add I/O by design:
- O_SYNC and O_DSYNC on soft updates: each write that allocates blocks now flushes its dependencies.
- fdatasync() of a large file after an append: it now writes the dirty indirect blocks.
On SU+J under the -o sync load, most synchronous writes took 8-64 us in ffs_syncvnode(), and about 8% took 4 ms or more, up to about 1 s, waiting for journal and cylinder group writes.
Not covered
- UFS1. The test's on-disk reader handles it, but no case creates a UFS1 file system.
- Double and triple indirect blocks.
- ffs_extwrite(), which has the same IO_SYNC -> ffs_update() pattern for extended attributes. It is not changed here.
Side effect on the PR 297976 reproducer
The mkdir_blkreuse.sh reproducer for PR 297976 relied on fdatasync() not writing the inode, which is the bug change 1 fixes. It has been reworked in that review to write blocks with O_DIRECT instead.