On Fri, 30 Nov 2007, Kostik Belousov wrote:

> On Fri, Nov 30, 2007 at 03:58:55PM +1100, Bruce Evans wrote:
>> On Fri, 30 Nov 2007, David Cecil wrote:

>...
>>> One more point to address Julian's question, the partition is not mounted
>>> with soft updates.

>>
>> Interesting. I saw no sign of the problem without soft updates except a
>> panic later after enabling soft updates. I was running fsck a lot but
>> may have forgotten one since no error was detected. The problem should
>> be easier to understand if it affects non-soft-updates.

>
> As a speculation, it might be that ufs_inactive() should conditionalize on
> fs_ronly instead of MNT_RDONLY. Then, VOP_INACTIVE() would set up the
> IN_CHANGE|IN_UPDATE and finally call the ffs_update() ?


Something like that seems to be right. The folowing hack in ufs_inactive()
seems to fix the problem with sift updates, as does unsetting MNT_RDONLY
for the whole VOP_SYNC() in ffs_mount().

% 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 30 Nov 2007 12:58:39 -0000
% @@ -59,4 +59,6 @@
% #endif
%
% +#include
% +
% /*
% * Last reference to an inode. If necessary, write or delete it.
% @@ -118,6 +120,15 @@
% ip->i_flag &= ~IN_ACCESS;
% } else {
% + int wasro = 0;
% +
% (void) vn_write_suspend_wait(vp, NULL, V_WAIT);
% + if (vp->v_mount->mnt_flag & MNT_RDONLY &&
% + ip->i_fs->fs_ronly == 0) {
% + vp->v_mount->mnt_flag &= ~MNT_RDONLY;
% + wasro = 1;
% + }
% UFS_UPDATE(vp, 0);
% + if (wasro)
% + vp->v_mount->mnt_flag |= MNT_RDONLY;
% }
% }

I didn't bother with correct locking here (only tested under UP).

With this change, the VOP_SYNC() in ffs_mount() for MNT_UPDATE seems
to flush everything in simple cases (with no open files), just like
the VOP_SYNC() in unmount() flushes everything before ffs_unmount() is
reached. OTOH, without a forced flush, soft updates takes a long
time to flush things -- more like 3 syncer periods than 1 for
non-waitfor syncs. With soft updates, the above is called from deep
in VOP_SYNC(). It's strange that the above non-waitfor UFS_UPDATE()
is used inside of waitfor syncs. It apparently works because the
waitfor syncs wait for it later, but only if it is non-null.

BTW, *_mount() has lots of bogusness related to string options. In
particular, ffs_mount() for update from r/w to r/o checks the "ro"
string option and sets MNT_RDONLY later, but MNT_RDONLY is already set
and other things depend on it being set early.

Bruce
_______________________________________________
freebsd-fs@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-fs
To unsubscribe, send any mail to "freebsd-fs-unsubscribe@freebsd.org"