On Mon, Mar 28, 2022 at 01:35:48PM +0200, Alexander Bluhm wrote:
> Hi,
> 
> There was a discussion about file system bugs with macppc.  My dual
> core macppc never completed a make release.  I get various panics.
> One of them is below.
> 
> bluhm
> 
> panic: vref used where vget required
> Stopped at      db_enter+0x24:  lwz r11,12(r1)
>     TID    PID    UID     PRFLAGS     PFLAGS  CPU  COMMAND
>  192060  78628     21         0x2          0    0  c++
> *132472  74971      0     0x14000      0x200    1K pagedaemon
> db_enter() at db_enter+0x20
> panic(91373c) at panic+0x158
> vref(23b8fa20) at vref+0xac
> uvm_vnp_uncache(e7eb7c50) at uvm_vnp_uncache+0x88
> ffs_write(7ed2e423) at ffs_write+0x3b0
> VOP_WRITE(23b8fa20,e7eb7c50,40,1ff3f60) at VOP_WRITE+0x48
> uvn_io(7ed2e423,a93d0c,a93814,ffffffff,e4010000) at uvn_io+0x264
> uvn_put(414b673a,e7eb7dd4,24f00070,5326e90) at uvn_put+0x64
> uvm_pager_put(0,0,e7eb7d70,6ee0b8,2000000,80000000,0) at uvm_pager_put+0x15c
> uvmpd_scan_inactive(0) at uvmpd_scan_inactive+0x224
> uvmpd_scan() at uvmpd_scan+0x158
> uvm_pageout(7e932633) at uvm_pageout+0x398
> fork_trampoline() at fork_trampoline+0x14
> end trace frame: 0x0, count: 2
> https://www.openbsd.org/ddb.html describes the minimum info required in bug
> reports.  Insufficient info makes it difficult to find and fix bugs.
> 

could you try to reproduce with the following kernel diff ?

as first step, it adds some KERNEL_ASSERT_LOCKED() on v_usecount modification, 
and some vprint() inside uvn_io().

the vprint() inside uvn_io() should trigger only before the panic to occurs.

I am expecting the KERNEL_ASSERT_LOCKED() to not trigger, and vprint() to popup 
before the panic.

having one or two vprint() would be interesting to know.

please report the backtrace, the vprint() (if any), and ddb output for
"show vnode /f 0xXYZ"

Thanks.
-- 
Sebastien Marie


diff 34ceb667563449b0c2871b87a9bb5cb5ae05fa82 /home/semarie/repos/openbsd/src
blob - 13c70890ccc11f131013d14b853f4f06941924cc
file + sys/kern/vfs_subr.c
--- sys/kern/vfs_subr.c
+++ sys/kern/vfs_subr.c
@@ -673,7 +673,9 @@ vget(struct vnode *vp, int flags)
                splx(s);
        }
 
+       KERNEL_ASSERT_LOCKED();
        vp->v_usecount++;
+
        if (flags & LK_TYPE_MASK) {
                if ((error = vn_lock(vp, flags)) != 0) {
                        vp->v_usecount--;
@@ -761,8 +763,10 @@ vput(struct vnode *vp)
                panic("vput: ref cnt");
        }
 #endif
+       KERNEL_ASSERT_LOCKED();
        vp->v_usecount--;
        KASSERT(vp->v_usecount > 0 || vp->v_uvcount == 0);
+
        if (vp->v_usecount > 0) {
                VOP_UNLOCK(vp);
                return;
@@ -801,7 +806,9 @@ vrele(struct vnode *vp)
                panic("vrele: ref cnt");
        }
 #endif
+       KERNEL_ASSERT_LOCKED();
        vp->v_usecount--;
+
        if (vp->v_usecount > 0) {
                return (0);
        }
@@ -999,8 +1017,10 @@ vclean(struct vnode *vp, int flags, struct proc *p)
         * so that its count cannot fall to zero and generate a
         * race against ourselves to recycle it.
         */
-       if ((active = vp->v_usecount) != 0)
+       if ((active = vp->v_usecount) != 0) {
+               KERNEL_ASSERT_LOCKED();
                vp->v_usecount++;
+       }
 
        /*
         * Prevent the vnode from being recycled or
@@ -1201,6 +1221,11 @@ vgonel(struct vnode *vp, struct proc *p)
                if (vp->v_holdcnt > 0)
                        panic("vgonel: not clean");
 
+#ifdef DIAGNOSTIC
+               if (!(vp->v_bioflag & VBIOONFREELIST))
+                       panic("vgonel: not on freelist");
+#endif
+
                if (TAILQ_FIRST(&vnode_free_list) != vp) {
                        TAILQ_REMOVE(&vnode_free_list, vp, v_freelist);
                        TAILQ_INSERT_HEAD(&vnode_free_list, vp, v_freelist);
@@ -2251,7 +2275,7 @@ vfs_vnode_print(void *v, int full,
 {
        struct vnode *vp = v;
 
-       (*pr)("tag %s(%d) type %s(%d) mount %p typedata %p\n",
+       (*pr)("%p tag %s(%d) type %s(%d) mount %p typedata %p\n", v,
              (u_int)vp->v_tag >= nitems(vtags)? "<unk>":vtags[vp->v_tag],
              vp->v_tag,
              (u_int)vp->v_type >= nitems(vtypes)? "<unk>":vtypes[vp->v_type],
@@ -2261,6 +2285,9 @@ vfs_vnode_print(void *v, int full,
              vp->v_data, vp->v_usecount, vp->v_writecount,
              vp->v_holdcnt, vp->v_numoutput);
 
+       (*pr)("flag 0x%x lflag 0x%x bioflag 0x%x\n",
+           vp->v_flag, vp->v_lflag, vp->v_bioflag);
+       
        /* uvm_object_printit(&vp->v_uobj, full, pr); */
 
        if (full) {
blob - 87182a180504da6342a7a6dfebb3d2e5dc499194
file + sys/kern/vfs_vnops.c
--- sys/kern/vfs_vnops.c
+++ sys/kern/vfs_vnops.c
@@ -566,6 +566,19 @@ vn_lock(struct vnode *vp, int flags)
        int error, xlocked, do_wakeup;
 
        do {
+#ifdef DIAGNOSTIC
+               /*
+                * The vn_lock() function must not be called when the
+                * vnode's reference count is zero. but vrele() will
+                * call it with v_usecount == 0. so just ensure the
+                * vnode isn't on freelist.
+                */
+               if (vp->v_usecount == 0 && vp->v_bioflag & VBIOONFREELIST) {
+                       vprint("vn_lock: vnode on freelist", vp);
+                       panic("%s: vp on freelist", __func__);
+               }
+#endif
+
                mtx_enter(&vnode_mtx);
                if (vp->v_lflag & VXLOCK) {
                        vp->v_lflag |= VXWANT;
blob - c9e7dd6328876cec5a6c0418c67a5d0d5a8df676
file + sys/uvm/uvm_vnode.c
--- sys/uvm/uvm_vnode.c
+++ sys/uvm/uvm_vnode.c
@@ -1147,6 +1147,11 @@ uvn_io(struct uvm_vnode *uvn, vm_page_t *pps, int npag
        vn = uvn->u_vnode;
        file_offset = pps[0]->offset;
 
+#ifndef SMALL_KERNEL
+       if (vn->v_usecount == 0)
+               vprint("uvn_io: start", vn);
+#endif
+
        /* check for sync'ing I/O. */
        while (uvn->u_flags & UVM_VNODE_IOSYNC) {
                if (waitf == M_NOWAIT) {
@@ -1225,6 +1230,11 @@ uvn_io(struct uvm_vnode *uvn, vm_page_t *pps, int npag
        if ((uvn->u_flags & UVM_VNODE_VNISLOCKED) == 0)
                result = vn_lock(vn, LK_EXCLUSIVE | LK_RECURSEFAIL | lkflags);
        if (result == 0) {
+#ifndef SMALL_KERNEL
+               if (vn->v_usecount == 0)
+                       vprint("uvn_io: after lock", vn);
+#endif
+
                /* NOTE: vnode now locked! */
                if (rw == UIO_READ)
                        result = VOP_READ(vn, &uio, 0, curproc->p_ucred);

Reply via email to