On Wed, Apr 06, 2022 at 03:23:55PM +0200, Sebastien Marie wrote:
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);
Thank you for your reply! Yes, of course, I will do that soon and post the results here. -- Wbr, Andrew Krasavin
