ufs: report why rewriting ".." failed in rename

[sobomax]

Summary

Summary

When ufs_rename() moves a directory to a new parent and ufs_dirrewrite() fails to rewrite its ".." entry, it reports bad dir ino N at offset 12: rename: missing .. entry, whatever the error was. ufs_dirrewrite() never detects a missing ".." at all. It fails only in two cases:

• EIDRM: the ".." entry names an inode other than the expected one.
• Any other error: the directory block could not be read (UFS_BLKATOFF()) or written. This happens for every directory rename in progress when the device goes away under a forcibly unmounted file system, as in the stress2 gnop tests. The log then fills with reports of damaged directories that are intact on disk.

The change:

• ufs_dirrewrite() reports the EIDRM case itself with ufs_dirbad(), at the point where it returns EIDRM. All its callers get the same diagnostic: the two in ufs_rename() and the fsck sysctl in ffs_alloc.c.
• ufs_rename() no longer reports anything for the ".." rewrite, and ignores the result as before. Errors from the lower layers are not reported, as is usual for an I/O initiator.

Git branch: https://github.com/sobomax/freebsd/tree/ufs-rename-dirbad-msg
Files: sys/ufs/ufs/ufs_lookup.c (+2), sys/ufs/ufs/ufs_vnops.c (+1 -3).

Test Plan

• Motivation: stress2's gnop10.sh destroys the gnop(8) device under a mounted UFS with soft updates while a directory-rename load runs. That produced bursts of rename: missing .. entry messages. Every burst had the same timestamp as the UFS: forcibly unmounting message, and the test's fsck runs reported the file system clean.
• Previous revision: it printed the error instead. It ran in a kernel through gnop10.sh, which logged only rename: error 6 updating .. of ino N (ENXIO) at the forced unmounts, confirming that these failures were I/O errors from the vanished device, not bad directories.
• This revision: the UFS module builds with -Werror on main, with GENERIC options including INVARIANTS. With it, gnop10.sh's forced unmounts should log nothing for the ".." rewrite.

[kib]

sys/ufs/ufs/ufs_vnops.c

1754

We do not normally printf anything from kernel when initiator of an io request notes error from the lower layer. I do not see a need for the 'else' branch there.

EIDRM is indeed weird case. But due to weirdness, it makes sense to call ufs_dirbad() consistently, which basically means doing it at the place where EIDRM is returned. Then all users of ufs_dirrewrite() would do the consistent diagnostic.

[sobomax]

@kib Thanks, both done in the update:

• The else branch in ufs_rename() is gone. I/O errors from the lower layer are no longer reported there, so the forced-unmount case is silent again, but without the misleading "missing .. entry" line.
• ufs_dirbad() is now called in ufs_dirrewrite() itself, where EIDRM is returned, so all callers (ufs_rename() twice and the fsck sysctl in ffs_alloc.c) get the same diagnostic: rewrite: .. entry does not name the expected inode. ufs_rename() no longer reports anything for the ".." rewrite and ignores the result, as before this change.