|David Cecil||Nov 29, 2007 4:01 pm|
|Bill Vermillion||Nov 29, 2007 4:27 pm|
|David Cecil||Nov 29, 2007 4:30 pm|
|Julian Elischer||Nov 29, 2007 4:49 pm|
|Bruce Evans||Nov 29, 2007 5:22 pm|
|David Cecil||Nov 29, 2007 5:38 pm|
|Matthew D. Fuller||Nov 29, 2007 8:05 pm|
|Bruce Evans||Nov 29, 2007 8:58 pm|
|David Cecil||Nov 29, 2007 9:26 pm|
|Bruce Evans||Nov 29, 2007 9:44 pm|
|Bruce Evans||Nov 29, 2007 10:02 pm|
|Kostik Belousov||Nov 29, 2007 10:03 pm|
|David Cecil||Nov 29, 2007 11:14 pm|
|Bruce Evans||Nov 30, 2007 8:33 am|
|Matthew D. Fuller||Nov 30, 2007 9:44 am|
|David Cecil||Nov 30, 2007 4:42 pm|
|Bruce Evans||Nov 30, 2007 6:01 pm|
|Bruce Evans||Nov 30, 2007 6:23 pm|
|David Cecil||Nov 30, 2007 8:27 pm|
|Don Lewis||Nov 30, 2007 11:26 pm|
|Don Lewis||Nov 30, 2007 11:26 pm|
|Kostik Belousov||Dec 1, 2007 12:07 am|
|Bruce Evans||Dec 1, 2007 3:34 am|
|Bruce Evans||Dec 1, 2007 8:07 am|
|Bruce Evans||Dec 1, 2007 10:33 am|
|Don Lewis||Dec 1, 2007 2:07 pm|
|Don Lewis||Dec 1, 2007 2:14 pm|
|Bruce Evans||Dec 1, 2007 7:56 pm|
|Kostik Belousov||Dec 1, 2007 10:14 pm|
|Bruce Evans||Dec 2, 2007 12:35 am|
|Bruce Evans||Dec 2, 2007 1:07 am|
|David Cecil||Dec 2, 2007 2:10 pm|
|Don Lewis||Dec 2, 2007 2:53 pm|
|Julian Elischer||Dec 2, 2007 3:49 pm|
|Bruce Evans||Dec 2, 2007 8:03 pm|
|Don Lewis||Dec 2, 2007 9:17 pm|
|Bruce Evans||Dec 3, 2007 2:11 am|
|Subject:||File remove problem|
|From:||Bruce Evans (br...@optusnet.com.au)|
|Date:||Dec 1, 2007 10:33:25 am|
On Sun, 2 Dec 2007, Bruce Evans wrote:
Here is a non-hackish patch which explains why ignoring MNT_RDONLY in the above or in ffs_mount() helps. It just fixes the confusion between IN_MODIFIED and IN_CHANGE in critical places.
% Index: ffs_softdep.c ...
Without this change, soft updates depends on IN_CHANGE being converted to IN_MODIFIED by ufs_itimes(), but this conversion doesn't happen when MNT_RDONLY is set. With soft updates, changes are often delayed until sync time, and when the sync is for mount-update it is done after setting MNT_RDONLY so the above doesn't work. ... Without the above patch, after "mv a b", "fsync b" is insufficient to make "mount -u -o ro /f" not corrupt the fs (because the mv generates 3 or 4 entries in the worklist, and the fsync generates 0 or 1 more and always ends up with 1 or 2 not handled, and then the sync for mount-update usually doesn't make any more progress). I didn't check if a short delay after the fsync is sufficient, but expect that it is. With the above patch, a loop doing "mv a b; mount -u -o ro /f; fsck -n /f; mount -u -o rw /f; mv b a" shows no problems. However, the original problem was without soft updates, so there must be more bugs here.
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).