Do not grab the `vmmaplk' recursively, prevent a self-deadlock.

Change the semantic of vm_map_busy() to be able to completely unlock the
`vmmaplk' instead of downgrading it to a read lock in mlock(2).  This is
necessary because uvm_fault_wire() tries to re-grab the same lock.

We now keep track of the thread currently holding the vmmap busy to ensure
it can relock & unbusy the vmmap.  The new pattern becomes:

....vm_map_lock(map);
....vm_map_busy(map); /* prevent other threads to grab an exclusive lock */
....vm_map_unlock(map);
....
..../*
.... * Do some stuff generally requiring a tsleep(9).
.... */
....
....vm_map_lock(map);
....vm_map_unbusy(map); /* allow other threads to make progress after unlock */
....vm_map_unlock(map);

Fix adapted from NetBSD's r1.249 of uvm/uvm_map.c.  Issue reported by
Jacqueline Jolicoeur exposed by a "wallet refresh" of the Monero App.
Panic hand-copied below:

sleep_finish()
rw_enter()
uvmfault_lookup()
uvm_fault_check()
uvm_fault()
uvm_fault_wire()
uvm_map_pageable_wire()
sys_mlock()

This version skips bumping the map's timestamp if the lock is acquired by the
thread marked the VM map busy.  This prevents a KASSERT() reported by bluhm@
triggered by regress/misc/posixtestsuite conformance/interfaces/mmap/18-1

ok kettenis@

2 files changed, 27 insertions, 28 deletions

diff --git a/sys/uvm/uvm_map.c b/sys/uvm/uvm_map.c
index 8c496669909..3951513654a 100644
--- a/sys/uvm/uvm_map.c
+++ b/sys/uvm/uvm_map.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uvm_map.c,v 1.317 2023/04/26 12:25:12 bluhm Exp $	*/
+/*	$OpenBSD: uvm_map.c,v 1.318 2023/05/20 12:48:36 mpi Exp $	*/
 /*	$NetBSD: uvm_map.c,v 1.86 2000/11/27 08:40:03 chs Exp $	*/
 
 /*
@@ -2144,15 +2144,8 @@ uvm_map_pageable_wire(struct vm_map *map, struct vm_map_entry *first,
 	 *    to be created.  then we clip each map entry to the region to
 	 *    be wired and increment its wiring count.
 	 *
-	 * 2: we downgrade to a read lock, and call uvm_fault_wire to fault
+	 * 2: we mark the map busy, unlock it and call uvm_fault_wire to fault
 	 *    in the pages for any newly wired area (wired_count == 1).
-	 *
-	 *    downgrading to a read lock for uvm_fault_wire avoids a possible
-	 *    deadlock with another thread that may have faulted on one of
-	 *    the pages to be wired (it would mark the page busy, blocking
-	 *    us, then in turn block on the map lock that we hold).
-	 *    because we keep the read lock on the map, the copy-on-write
-	 *    status of the entries we modify here cannot change.
 	 */
 	for (iter = first; iter != end;
 	    iter = RBT_NEXT(uvm_map_addr, iter)) {
@@ -2185,7 +2178,7 @@ uvm_map_pageable_wire(struct vm_map *map, struct vm_map_entry *first,
 	timestamp_save = map->timestamp;
 #endif
 	vm_map_busy(map);
-	vm_map_downgrade(map);
+	vm_map_unlock(map);
 
 	error = 0;
 	for (iter = first; error == 0 && iter != end;
@@ -2198,14 +2191,10 @@ uvm_map_pageable_wire(struct vm_map *map, struct vm_map_entry *first,
 		    iter->protection);
 	}
 
+	vm_map_lock(map);
+	vm_map_unbusy(map);
+
 	if (error) {
-		/*
-		 * uvm_fault_wire failure
-		 *
-		 * Reacquire lock and undo our work.
-		 */
-		vm_map_upgrade(map);
-		vm_map_unbusy(map);
 #ifdef DIAGNOSTIC
 		if (timestamp_save != map->timestamp)
 			panic("uvm_map_pageable_wire: stale map");
@@ -2244,13 +2233,10 @@ uvm_map_pageable_wire(struct vm_map *map, struct vm_map_entry *first,
 		return error;
 	}
 
-	/* We are currently holding a read lock. */
+
 	if ((lockflags & UVM_LK_EXIT) == 0) {
-		vm_map_unbusy(map);
-		vm_map_unlock_read(map);
+		vm_map_unlock(map);
 	} else {
-		vm_map_upgrade(map);
-		vm_map_unbusy(map);
 #ifdef DIAGNOSTIC
 		if (timestamp_save != map->timestamp)
 			panic("uvm_map_pageable_wire: stale map");
@@ -2495,6 +2481,7 @@ uvm_map_setup(struct vm_map *map, pmap_t pmap, vaddr_t min, vaddr_t max,
 	map->s_start = map->s_end = 0; /* Empty stack area by default. */
 	map->flags = flags;
 	map->timestamp = 0;
+	map->busy = NULL;
 	if (flags & VM_MAP_ISVMSPACE)
 		rw_init_flags(&map->lock, "vmmaplk", RWL_DUPOK);
 	else
@@ -5313,7 +5300,7 @@ vm_map_lock_try_ln(struct vm_map *map, char *file, int line)
 		rv = mtx_enter_try(&map->mtx);
 	} else {
 		mtx_enter(&map->flags_lock);
-		if (map->flags & VM_MAP_BUSY) {
+		if ((map->flags & VM_MAP_BUSY) && (map->busy != curproc)) {
 			mtx_leave(&map->flags_lock);
 			return (FALSE);
 		}
@@ -5322,7 +5309,8 @@ vm_map_lock_try_ln(struct vm_map *map, char *file, int line)
 		/* check if the lock is busy and back out if we won the race */
 		if (rv) {
 			mtx_enter(&map->flags_lock);
-			if (map->flags & VM_MAP_BUSY) {
+			if ((map->flags & VM_MAP_BUSY) &&
+			    (map->busy != curproc)) {
 				rw_exit(&map->lock);
 				rv = FALSE;
 			}
@@ -5347,7 +5335,8 @@ vm_map_lock_ln(struct vm_map *map, char *file, int line)
 		do {
 			mtx_enter(&map->flags_lock);
 tryagain:
-			while (map->flags & VM_MAP_BUSY) {
+			while ((map->flags & VM_MAP_BUSY) &&
+			    (map->busy != curproc)) {
 				map->flags |= VM_MAP_WANTLOCK;
 				msleep_nsec(&map->flags, &map->flags_lock,
 				    PVM, vmmapbsy, INFSLP);
@@ -5356,7 +5345,7 @@ tryagain:
 		} while (rw_enter(&map->lock, RW_WRITE|RW_SLEEPFAIL) != 0);
 		/* check if the lock is busy and back out if we won the race */
 		mtx_enter(&map->flags_lock);
-		if (map->flags & VM_MAP_BUSY) {
+		if ((map->flags & VM_MAP_BUSY) && (map->busy != curproc)) {
 			rw_exit(&map->lock);
 			goto tryagain;
 		}
@@ -5365,7 +5354,8 @@ tryagain:
 		mtx_enter(&map->mtx);
 	}
 
-	map->timestamp++;
+	if (map->busy != curproc)
+		map->timestamp++;
 	LPRINTF(("map   lock: %p (at %s %d)\n", map, file, line));
 	uvm_tree_sanity(map, file, line);
 	uvm_tree_size_chk(map, file, line);
@@ -5386,6 +5376,7 @@ vm_map_lock_read_ln(struct vm_map *map, char *file, int line)
 void
 vm_map_unlock_ln(struct vm_map *map, char *file, int line)
 {
+	KASSERT(map->busy == NULL || map->busy == curproc);
 	uvm_tree_sanity(map, file, line);
 	uvm_tree_size_chk(map, file, line);
 	LPRINTF(("map unlock: %p (at %s %d)\n", map, file, line));
@@ -5438,7 +5429,11 @@ void
 vm_map_busy_ln(struct vm_map *map, char *file, int line)
 {
 	KASSERT((map->flags & VM_MAP_INTRSAFE) == 0);
+	KASSERT(rw_write_held(&map->lock));
+	KASSERT(map->busy == NULL);
+
 	mtx_enter(&map->flags_lock);
+	map->busy = curproc;
 	map->flags |= VM_MAP_BUSY;
 	mtx_leave(&map->flags_lock);
 }
@@ -5449,8 +5444,11 @@ vm_map_unbusy_ln(struct vm_map *map, char *file, int line)
 	int oflags;
 
 	KASSERT((map->flags & VM_MAP_INTRSAFE) == 0);
+	KASSERT(map->busy == curproc);
+
 	mtx_enter(&map->flags_lock);
 	oflags = map->flags;
+	map->busy = NULL;
 	map->flags &= ~(VM_MAP_BUSY|VM_MAP_WANTLOCK);
 	mtx_leave(&map->flags_lock);
 	if (oflags & VM_MAP_WANTLOCK)
diff --git a/sys/uvm/uvm_map.h b/sys/uvm/uvm_map.h
index 58847fc45e1..935333a2331 100644
--- a/sys/uvm/uvm_map.h
+++ b/sys/uvm/uvm_map.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uvm_map.h,v 1.85 2023/04/26 12:25:12 bluhm Exp $	*/
+/*	$OpenBSD: uvm_map.h,v 1.86 2023/05/20 12:48:36 mpi Exp $	*/
 /*	$NetBSD: uvm_map.h,v 1.24 2001/02/18 21:19:08 chs Exp $	*/
 
 /*
@@ -274,6 +274,7 @@ struct vm_map {
 	int			ref_count;	/* [a] Reference count */
 	int			flags;		/* flags */
 	unsigned int		timestamp;	/* Version number */
+	struct proc		*busy;		/* [v] thread holding map busy*/
 
 	vaddr_t			min_offset;	/* [I] First address in map. */
 	vaddr_t			max_offset;	/* [I] Last address in map. */