From: Martin Pieuchot Subject: Re: Simply uvn_attach() & kill UVM_VNODE_ALOCK To: tech@openbsd.org Date: Mon, 10 Nov 2025 15:00:25 +0000 On 10/11/25(Mon) 10:57, Martin Pieuchot wrote: > On 09/11/25(Sun) 17:37, Martin Pieuchot wrote: > > By querying the size of the vnode before grabbing `vmobjlock' > > uvn_attach() can be greatly simplified. Since the lock is no > > longer released, there is no need for UVM_VNODE_ALOCK. > > As discussed in the hackroom, here's a simpler diff to review that do > not include the removal of UVM_VNODE_ALOCK. Here's the second part of the diff that removes duplicated code and now unused UVM_VNODE_ALOCK. ok? Index: uvm/uvm_vnode.c =================================================================== RCS file: /cvs/src/sys/uvm/uvm_vnode.c,v diff -u -p -r1.147 uvm_vnode.c --- uvm/uvm_vnode.c 10 Nov 2025 14:54:37 -0000 1.147 +++ uvm/uvm_vnode.c 10 Nov 2025 14:56:53 -0000 @@ -137,7 +137,7 @@ uvn_attach(struct vnode *vp, vm_prot_t a { struct uvm_vnode *uvn; struct vattr vattr; - int oldflags, result; + int result; struct partinfo pi; u_quad_t used_vnode_size = 0; @@ -202,8 +202,7 @@ uvn_attach(struct vnode *vp, vm_prot_t a /* * now uvn must not be in a blocked state. * first check to see if it is already active, in which case - * we can bump the reference count, check to see if we need to - * add it to the writeable list, and then return. + * we can bump the reference count. */ if (uvn->u_flags & UVM_VNODE_VALID) { /* already active? */ @@ -212,42 +211,31 @@ uvn_attach(struct vnode *vp, vm_prot_t a vref(vp); } uvn->u_obj.uo_refs++; /* bump uvn ref! */ + } else { + /* now set up the uvn. */ + KASSERT(uvn->u_obj.uo_refs == 0); + uvn->u_obj.uo_refs++; + uvn->u_flags = UVM_VNODE_VALID|UVM_VNODE_CANPERSIST; + uvn->u_nio = 0; + uvn->u_size = used_vnode_size; - /* check for new writeable uvn */ - if ((accessprot & PROT_WRITE) != 0 && - (uvn->u_flags & UVM_VNODE_WRITEABLE) == 0) { - uvn->u_flags |= UVM_VNODE_WRITEABLE; - KERNEL_ASSERT_LOCKED(); - LIST_INSERT_HEAD(&uvn_wlist, uvn, u_wlist); - } - - rw_exit(uvn->u_obj.vmobjlock); - return (&uvn->u_obj); + /* + * 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]. + */ + vref(vp); } - /* now set up the uvn. */ - 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_nio = 0; - uvn->u_size = used_vnode_size; - - /* - * 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]. - */ - vref(vp); - /* if write access, we need to add it to the wlist */ - if (accessprot & PROT_WRITE) { + if ((accessprot & PROT_WRITE) && + !(uvn->u_flags & UVM_VNODE_WRITEABLE)) { uvn->u_flags |= UVM_VNODE_WRITEABLE; /* we are on wlist! */ KERNEL_ASSERT_LOCKED(); LIST_INSERT_HEAD(&uvn_wlist, uvn, u_wlist); } - if (oldflags & UVM_VNODE_WANTED) + if (uvn->u_flags & UVM_VNODE_WANTED) wakeup(uvn); rw_exit(uvn->u_obj.vmobjlock); @@ -430,9 +418,7 @@ uvm_vnp_terminate(struct vnode *vp) /* * must be a valid uvn that is not already dying (because XLOCK - * protects us from that). the uvn can't in the ALOCK state - * because it is valid, and uvn's that are in the ALOCK state haven't - * been marked valid yet. + * protects us from that). */ #ifdef DEBUG /* @@ -1495,9 +1481,6 @@ uvm_vnp_sync(struct mount *mp) * If the vnode is "blocked" it means it must be dying, which * in turn means its in the process of being flushed out so * we can safely skip it. - * - * note that uvn must already be valid because we found it on - * the wlist (this also means it can't be ALOCK'd). */ if ((uvn->u_flags & UVM_VNODE_BLOCKED) != 0) continue; Index: uvm/uvm_vnode.h =================================================================== RCS file: /cvs/src/sys/uvm/uvm_vnode.h,v diff -u -p -r1.23 uvm_vnode.h --- uvm/uvm_vnode.h 8 Nov 2025 17:23:22 -0000 1.23 +++ uvm/uvm_vnode.h 10 Nov 2025 14:55:03 -0000 @@ -72,7 +72,6 @@ struct uvm_vnode { */ #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 */ #define UVM_VNODE_RELKILL 0x010 /* uvn should be killed by releasepg @@ -92,7 +91,7 @@ struct uvm_vnode { * UVM_VNODE_BLOCKED: any condition that should new processes from * touching the vnode [set WANTED and sleep to wait for it to clear] */ -#define UVM_VNODE_BLOCKED (UVM_VNODE_ALOCK|UVM_VNODE_DYING|UVM_VNODE_RELKILL) +#define UVM_VNODE_BLOCKED (UVM_VNODE_DYING|UVM_VNODE_RELKILL) struct uvm_object *uvn_attach(struct vnode *, vm_prot_t); void uvm_vnp_terminate(struct vnode *);