Get rid of the extra vnode reference known as UVM_VNODE_CANPERSIST.

Back in the 4.4BSD days the VM subystem had a OBJ_CANPERSIST flag to
enter objects in a global cached list.  Some of this logic seem to have
been copied to UVM but without the global list.

Unfortunately keeping UVM vnode objects alive after munmap(2)ing the
corresponding region without incrementing the reference count of the
related vnode led to many bugs when the vnode was recycled and/or when
it data where written back to disk (via the page daemon).

The problem is that VM pages might have a non-accounted reference to a
vnode via `pg->uobject'.

Fix "vref used where vget required" panic reported by bluhm@, gkoehler@
and Andrew Krasavinseen on bugs@.

Thanks a lot to semarie@ for co-debugging this issue!

Tested by bluhm@, tb@, miod@.

ok kettenis@, semarie@

diff --git a/sys/uvm/uvm_vnode.c b/sys/uvm/uvm_vnode.c
index 28e51a2..1e38302 100644 (file)
--- a/sys/uvm/uvm_vnode.c
+++ b/sys/uvm/uvm_vnode.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: uvm_vnode.c,v 1.127 2022/08/31 09:07:35 gnezdo Exp $  */
+/*     $OpenBSD: uvm_vnode.c,v 1.128 2022/09/10 16:14:36 mpi Exp $     */
 /*     $NetBSD: uvm_vnode.c,v 1.36 2000/11/24 20:34:01 chs Exp $       */
 
 /*
@@ -163,11 +163,8 @@ uvn_attach(struct vnode *vp, vm_prot_t accessprot)
         */
        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 accessprot)
        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 accessprot)
        /*
         * 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);

diff --git a/sys/uvm/uvm_vnode.h b/sys/uvm/uvm_vnode.h
index 1716511..f132986 100644 (file)
--- a/sys/uvm/uvm_vnode.h
+++ b/sys/uvm/uvm_vnode.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: uvm_vnode.h,v 1.18 2021/10/12 07:37:42 mpi Exp $      */
+/*     $OpenBSD: uvm_vnode.h,v 1.19 2022/09/10 16:14:36 mpi Exp $      */
 /*     $NetBSD: uvm_vnode.h,v 1.9 2000/03/26 20:54:48 kleink Exp $     */
 
 /*
@@ -68,7 +68,6 @@ struct uvm_vnode {
  * u_flags values
  */
 #define UVM_VNODE_VALID                0x001   /* we are attached to the vnode */
-#define UVM_VNODE_CANPERSIST   0x002   /* we can persist after ref == 0 */
 #define UVM_VNODE_ALOCK                0x004   /* uvn_attach is locked out */
 #define UVM_VNODE_DYING                0x008   /* final detach/terminate in 
                                           progress */