DragonFlyBSD

This bug is not related to fat32. It can for example be reproduced with /proc:

cd /proc
umount -f /proc
ls (or any external command)

It can also be reproduced w/o force unmount:

sleep 30 &
cd /proc/$!
... wait for 30s or kill the sleep process ...
ls (or any external command)

When forking ls, fdcopy() calls vref() on fd_cdir but the latter was
inactivated earlier during unmounting (vflush()). This triggers the
assertion in vref().

I don't know how to fix this. As an experiment, I tried setting
fd_cdir to NULL if VINACTIVE is set but this triggers a crash at
boot-time. Maybe the assertion should be relaxed, for this case only,
to:

KKASSERT == 0);

Any advice?

Ideally, the umount shouldn't be able to succeed because it's still open
as CWD for someone.

Cheers,
--
Thomas E. Spanjaard
tgen@netphreax.net

Maybe but the bug also occurs w/o unmounting (cf my previous email).

Freebsd allows forced unmounts but does not crash. Linux on the other
hand behaves as you suggest (forced unmount fails).

:2007/10/21, Thomas E. Spanjaard <tgen@netphreax.net>:
:> > Any advice?
:>
:> Ideally, the umount shouldn't be able to succeed because it's still open
:> as CWD for someone.
:
:Maybe but the bug also occurs w/o unmounting (cf my previous email).
:
:Freebsd allows forced unmounts but does not crash. Linux on the other
:hand behaves as you suggest (forced unmount fails).

In our case we want the forced unmount to succeed, though personally
    speaking Linux is probably more correct simply because when we override
    vnodes in a forced unmount there is always the possibility of a race
    against something.  For now though I want forced unmounts to succeed.

I'll check it out.

-Matt
                    Matthew Dillon 
                    <dillon@backplane.com>

What about the following patch?

It fixes the assertion failure for the various cases listed in this
thread. When running ls from an unmounted /proc, cache_resolve()
complains but I reckon this is expected:

EXDEV case 1 0xcaba70c8
EXDEV case 1 0xcaba70c8
EXDEV case 1 0xcaba70c8
ls: .: cross-device link

===================================================================
RCS file: /home/dcvs/src/sys/kern/vfs_subr.c,v
retrieving revision 1.106
diff u -p -r1.106 vfs_subr.c
-- vfs_subr.c 31 Jul 2007 01:14:50 -0000 1.106
+++ vfs_subr.c 22 Oct 2007 09:33:59 -0000
@ -1108,7 +1108,7 @ vclean_vxlocked(struct vnode *vp, int fl
}

/*
-        * If the vnode has not be deactivated, deactivated it.  Deactivation
+        * If the vnode has not been deactivated, deactivate it.  Deactivation
        * can create new buffers and VM pages so we have to call vinvalbuf()
        * again to make sure they all get flushed.
        
@ -1148,6 +1148,17 @ vclean_vxlocked(struct vnode *vp, int fl
        vp->v_ops = &dead_vnode_vops_p;
        vn_pollgone(vp);
        vp->v_tag = VT_NON;

       /
+        * If the vnode was force-closed, it is still referenced and may be
+        * vref()'ed again.  vref() calls are allowed for active vnodes only so
+        * do as if this vnode is active in the "dead vnode" fs.  When its last
+        * reference goes away, it will be inactivated and reclaimed again in
+        * this dummy fs.
+        */
+       if (active && (flags & DOCLOSE)) {
+               vp->v_flag &= ~(VINACTIVE|VRECLAIMED);
+       }
 }

/*

:What about the following patch?
:
:It fixes the assertion failure for the various cases listed in this
:thread. When running ls from an unmounted /proc, cache_resolve()
:complains but I reckon this is expected:
:
:EXDEV case 1 0xcaba70c8
:EXDEV case 1 0xcaba70c8
:EXDEV case 1 0xcaba70c8
:ls: .: cross-device link

Yes, the patch looks pretty good.  I'll test it a bit today and
    check out the error case, then commit it.

-Matt
                    Matthew Dillon 
                    <dillon@backplane.com>

: vp->v_ops = &dead_vnode_vops_p;
: vn_pollgone(vp);
: vp->v_tag = VT_NON;
:+
:+ /*
:+ * If the vnode was force-closed, it is still referenced and may be
:+ * vref()'ed again. vref() calls are allowed for active vnodes only so
:+ * do as if this vnode is active in the "dead vnode" fs. When its last
:+ * reference goes away, it will be inactivated and reclaimed again in
:+ * this dummy fs.
:+ */
:+ if (active && (flags & DOCLOSE)) {
:+ vp->v_flag &= ~(VINACTIVE|VRECLAIMED);
:+ }
: }