File remove problem

[David Cecil]

Hi Bruce,

I tried the change and it doesn't fix the non-sofdep problem.

Looking at that code got me thinking though.  I thought that a workable 
solution for us short term might be to call UFS_UPDATE with waitfor == 1 
if the filesystem is mounted synchronous.  So, the scenario is:
mount -u -w -o sync /
remove file
mount -u -o ro /

I observed that the second call to ffs_update with the offending vnode 
caused bwrite to be called, though the first call was still bdwrite.  
Anyhow, the problem still occurred.  I tried clearing the B_ASYNC flag 
in ffs_update if bwrite is called, but still the problem occurs.  
Obviously I'm missing something.

Dave

ext Bruce Evans wrote:
> 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 <ufs/ffs/fs.h>
> % +
> %  /*
> %   * 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
>