Page MenuHomeFreeBSD

ufs: undo the new parent's link when ufs_rename() cannot rewrite ".."
Needs ReviewPublic

Authored by sobomax on Mon, Oct 5, 3:58 AM.

Details

Reviewers
kib
pho
mckusick
Summary

Summary

When ufs_rename() moves a directory to a new parent, it first adds a link to the new parent for the directory's ... With SU+J it also sets up a journal record (jaddref) for that link with softdep_setup_dotdot_link(). Then it rewrites .. with ufs_dirrewrite().

If that rewrite fails, for example on an I/O error reading the directory block, ufs_dirrewrite() puts the old parent's link back, so .. still consistently names the old parent. But two things are left behind:

  • the extra link on the new parent
  • the journal record, which is neither written nor cancelled

The same happens if the update of the new parent fails before the rewrite is tried. That path also leaves through unlockout with the link and the record in place.

Without the journal, fsck later finds the new parent's link count one too high.

With SU+J the result is a livelock:

  • initiate_write_inodeblock_ufs2() rolls the link count back to that of the first unwritten journal record on the inodedep in every write of the inode. handle_written_inodeblock() then redirties the block. The leaked record is never written, so this never ends.
  • An unmount cannot suspend writes. A plain umount fails with EAGAIN once vn_fsync_buf() gives up on the block. The deferred forced unmount after I/O errors keeps retrying in vfs_write_suspend(), which leaves every writer stuck in suspfs, and every vfs_busy() of the mount stuck as well (mount -v, df, ...).

The fix: on either failure, take back the link added to the new parent (only when one was added, i.e. there was no existing target directory) and, with SU+J, cancel the journal record with softdep_revert_link(). Both paths go through a small helper, ufs_rename_dotdot_undo().

Before a179e72489f8, a failed rewrite was reported as "rename: missing .. entry" through ufs_dirbad(), which only prints. The rename then went on the same way. Those messages, often seen at forced unmounts in the gnop stress2 tests, were this leak.

How it was found

gnop9.sh (SU+J, forced unmounts with injected I/O errors) hung. The deferred unmount task spun in vfs_write_suspend() -> ffs_sync() -> vn_fsync_buf() for hours, rewriting the same 4 inode blocks about 475 times a second each. A live minidump (savecore -L) examined with kgdb showed:

  • 6 directories with 235 to 1028 links, each with a non-empty id_inoreflst. Their inodedeps were otherwise ALLCOMPLETE.
  • 111 D_JADDREF records on those lists, all in state ATTACHED only, with if_diroff 12 (..), no jseg and not on the journal pending list (sd_on_journal 0). if_ino was the new parent and if_parent the moved directory.

Test

New stress2 test rename19.sh, run on SU and on SU+J:

  1. It creates a/d and b on a gnop(8) device, remounts, and caches everything the rename needs except d's directory block.
  2. It makes all reads fail for the duration of mv a/d b/d, so the only read that fails is ufs_dirrewrite()'s.
  3. It unmounts, with a watchdog, and runs fsck_ffs -fn.
  4. The test requires the stale .. to be reported, which proves the rewrite failed, and nothing else to be reported.

Test Plan

Testing was on amd64 (AWS EC2, 4 vCPUs, 16 GB RAM), non-debug kernel.

Without the fix

rename19.sh fails on both file system types:

  • SU: fsck reports the stale .. (CURRENTLY POINTS TO I=... (/a), SHOULD POINT TO I=... (/b)) and LINK COUNT DIR I=... (/b) COUNT 3 SHOULD BE 2.
  • SU+J: umount fails with "Resource temporarily unavailable" (EAGAIN), and the file system cannot be unmounted at all.

With the fix

  • rename19.sh: passes on SU and on SU+J. The stale .. is the only problem fsck reports, and the unmount succeeds. DTrace showed ufs_dirrewrite() failing with EIO once per file system type, and softdep_revert_link() called once from ufs_rename(), on SU+J.
  • gnop9.sh: passed 4 times in a row.
  • Other stress2 tests: these passed on the same kernel, with no panics:
    • gnop10.sh, gnop7.sh (SU and SU+J) and fsck6.sh
    • fsync2.sh (SU and SU+J) and fsync3.sh
    • 10-minute marcus.cfg loads on -o sync mounts: SU, SU+J, and SU with snapshots
    • nlink6.sh

The tested kernel also carried other UFS changes under review (D60134, D60204), which do not touch ufs_rename().

Not covered

  • The failure of the new parent's update before the rewrite (the UFS_UPDATE() path) is not exercised by the test. It takes the same undo as the rewrite failure.
  • When the move replaces an existing target directory (tip != NULL), no link is added to the new parent, so only the journal record is cancelled. rename19.sh does not cover that case.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped