On 09/09/22(Fri) 12:25, Theo Buehler wrote: > > Yesterday gnezdo@ fixed a race in uvn_attach() that lead to the same > > assert. Here's an rebased diff for the bug discussed in this thread, > > could you try again and let us know? Thanks! > > This seems to be stable now. It's been running for nearly 5 days. > Without gnezdo's fix it would blow up within at most 2 days.
Thanks! I'm looking for oks then. Index: uvm/uvm_vnode.c =================================================================== RCS file: /cvs/src/sys/uvm/uvm_vnode.c,v retrieving revision 1.127 diff -u -p -r1.127 uvm_vnode.c --- uvm/uvm_vnode.c 31 Aug 2022 09:07:35 -0000 1.127 +++ uvm/uvm_vnode.c 1 Sep 2022 12:54:27 -0000 @@ -163,11 +163,8 @@ uvn_attach(struct vnode *vp, vm_prot_t a */ rw_enter(uvn->u_obj.vmobjlock, RW_WRITE); if (uvn->u_flags & UVM_VNODE_VALID) { /* already active? */ + KASSERT(uvn->u_obj.uo_refs > 0); - /* regain vref if we were persisting */ - if (uvn->u_obj.uo_refs == 0) { - vref(vp); - } uvn->u_obj.uo_refs++; /* bump uvn ref! */ rw_exit(uvn->u_obj.vmobjlock); @@ -235,7 +232,7 @@ uvn_attach(struct vnode *vp, vm_prot_t a KASSERT(uvn->u_obj.uo_refs == 0); uvn->u_obj.uo_refs++; oldflags = uvn->u_flags; - uvn->u_flags = UVM_VNODE_VALID|UVM_VNODE_CANPERSIST; + uvn->u_flags = UVM_VNODE_VALID; uvn->u_nio = 0; uvn->u_size = used_vnode_size; @@ -248,7 +245,7 @@ uvn_attach(struct vnode *vp, vm_prot_t a /* * add a reference to the vnode. this reference will stay as long * as there is a valid mapping of the vnode. dropped when the - * reference count goes to zero [and we either free or persist]. + * reference count goes to zero. */ vref(vp); if (oldflags & UVM_VNODE_WANTED) @@ -321,16 +318,6 @@ uvn_detach(struct uvm_object *uobj) */ vp->v_flag &= ~VTEXT; - /* - * we just dropped the last reference to the uvn. see if we can - * let it "stick around". - */ - if (uvn->u_flags & UVM_VNODE_CANPERSIST) { - /* won't block */ - uvn_flush(uobj, 0, 0, PGO_DEACTIVATE|PGO_ALLPAGES); - goto out; - } - /* its a goner! */ uvn->u_flags |= UVM_VNODE_DYING; @@ -380,7 +367,6 @@ uvn_detach(struct uvm_object *uobj) /* wake up any sleepers */ if (oldflags & UVM_VNODE_WANTED) wakeup(uvn); -out: rw_exit(uobj->vmobjlock); /* drop our reference to the vnode. */ @@ -496,8 +482,8 @@ uvm_vnp_terminate(struct vnode *vp) } /* - * done. now we free the uvn if its reference count is zero - * (true if we are zapping a persisting uvn). however, if we are + * done. now we free the uvn if its reference count is zero. + * however, if we are * terminating a uvn with active mappings we let it live ... future * calls down to the vnode layer will fail. */ @@ -505,14 +491,14 @@ uvm_vnp_terminate(struct vnode *vp) if (uvn->u_obj.uo_refs) { /* * uvn must live on it is dead-vnode state until all references - * are gone. restore flags. clear CANPERSIST state. + * are gone. restore flags. */ uvn->u_flags &= ~(UVM_VNODE_DYING|UVM_VNODE_VNISLOCKED| - UVM_VNODE_WANTED|UVM_VNODE_CANPERSIST); + UVM_VNODE_WANTED); } else { /* * free the uvn now. note that the vref reference is already - * gone [it is dropped when we enter the persist state]. + * gone. */ if (uvn->u_flags & UVM_VNODE_IOSYNCWANTED) panic("uvm_vnp_terminate: io sync wanted bit set"); @@ -1349,46 +1335,14 @@ uvm_vnp_uncache(struct vnode *vp) } /* - * we have a valid, non-blocked uvn. clear persist flag. + * we have a valid, non-blocked uvn. * if uvn is currently active we can return now. */ - uvn->u_flags &= ~UVM_VNODE_CANPERSIST; if (uvn->u_obj.uo_refs) { rw_exit(uobj->vmobjlock); return FALSE; } - /* - * uvn is currently persisting! we have to gain a reference to - * it so that we can call uvn_detach to kill the uvn. - */ - vref(vp); /* seems ok, even with VOP_LOCK */ - uvn->u_obj.uo_refs++; /* value is now 1 */ - rw_exit(uobj->vmobjlock); - -#ifdef VFSLCKDEBUG - /* - * carry over sanity check from old vnode pager: the vnode should - * be VOP_LOCK'd, and we confirm it here. - */ - if ((vp->v_flag & VLOCKSWORK) && !VOP_ISLOCKED(vp)) - panic("uvm_vnp_uncache: vnode not locked!"); -#endif - - /* - * now drop our reference to the vnode. if we have the sole - * reference to the vnode then this will cause it to die [as we - * just cleared the persist flag]. we have to unlock the vnode - * while we are doing this as it may trigger I/O. - * - * XXX: it might be possible for uvn to get reclaimed while we are - * unlocked causing us to return TRUE when we should not. we ignore - * this as a false-positive return value doesn't hurt us. - */ - VOP_UNLOCK(vp); - uvn_detach(&uvn->u_obj); - vn_lock(vp, LK_EXCLUSIVE | LK_RETRY); - return TRUE; } @@ -1483,11 +1437,9 @@ uvm_vnp_sync(struct mount *mp) continue; /* - * gain reference. watch out for persisting uvns (need to - * regain vnode REF). + * gain reference. */ - if (uvn->u_obj.uo_refs == 0) - vref(vp); + KASSERT(uvn->u_obj.uo_refs > 0); uvn->u_obj.uo_refs++; SIMPLEQ_INSERT_HEAD(&uvn_sync_q, uvn, u_syncq);