On Sun, 2 Dec 2007, Bruce Evans wrote:

The above seems to fix "mv a b", but for "rm a" it only restores the previous symptoms of the bug -- the "failed to flush worklist" error is gone but the "update error" is back.

The following simiilar patch seems to fix "rm a" in 5.2:

% Index: ufs_inode.c % =================================================================== % RCS file: /home/ncvs/src/sys/ufs/ufs/ufs_inode.c,v % retrieving revision 1.53 % diff -u -2 -r1.53 ufs_inode.c % --- ufs_inode.c 7 Apr 2004 03:47:20 -0000 1.53 % +++ ufs_inode.c 1 Dec 2007 17:26:30 -0000 % @@ -108,5 +111,5 @@ % ip->i_mode = 0; % DIP(ip, i_mode) = 0; % - ip->i_flag |= IN_CHANGE | IN_UPDATE; % + ip->i_flag |= IN_MODIFIED; % if (DOINGSOFTDEP(vp)) % softdep_change_linkcnt(ip);

This is in ufs_inactive(). Soft updates calls back here from the sync. softdep_change_linkcount() just creates a dependency and depends on its callers to set IN_* flags which will cause i/o to flush the dependency. Here the caller sets flags that had no effect in the MNT_RDONLY case, the same as in the previous patch. I removed the IN_CHANGE and IN_UPDATE flags completely since they just clobber the previous times (set by a non-delayed ufs_inactive but probably already clobbered by a delayed truncation). (We are clearing out parts of the inode, so the times don't really matter, but unless something else clears the times it is better to leave the non-delayed times which record when the update marks for the userland remove were converted to timestamps.)

Just before the above, the file is truncated. ffs_truncate() has the usual confusion between IN_CHANGE and IN_MODIFIED. It sets (IN_CHANGE | IN_UPDATE) in 7 different places, but never sets IN_MODIFIED. In my test case of "rm a; mount -u -o ro /f", this isn't a problem, because IN_MODIFIED is usually already set set ffs_truncate() doesn't forget to start i/o. ffs_truncate() calls ffs_update() with waitfor == 1 in many cases, but in my test case it always calls ffs_update() with waitfor == 0. Any call to ffs_update() in it cancels any previous setting of IN_MODIFIED when modifications are moved to the buffer. Thus when we forget to set the IN_MODIFIED in the above code, in the MNT_RDONLY case we always have inconsistencies: - non-soft updates case: we have at least a cleared i_rdev and i_mode with no means to write them. Is this enough to cause the original problem? - soft updates case: we also have a dependency with no means to flush it.

I think this change doesn't help for -current yet, since this code is unreachable when MNT_RDONLY is set. The truncation code is also unreachable. kib's patch makes both reachable again.

I observe/predict/explain the following error behaviours for soft updates (still some guesses): -current the first patch: "/f: update error: blocks 32 files 1" (because the truncation and other things aren't done, but worklist items aren't created for them so the problem isn't detected in softdep_waitidle). -5.2 with first patch: "/f: update error: blocks 0 files 1" (because the truncation works accidentally, and softdep_waitidle() doesn't exist to detect the unflushable worklist item created by the reachable softdep_change_linkcnt() call). Panic later (due to broken worklist after a buggy remount is permitted?). -current with first patch and kib's patch: "softdep_waitidle: ..." (because the bug fixed in the second patch is exposed again and softdep_waitidle() detects it).

Bruce