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 *);