summaryrefslogtreecommitdiff
path: root/sys/uvm/uvm_map.c
diff options
context:
space:
mode:
authorMartin Pieuchot <mpi@cvs.openbsd.org>2023-04-25 12:36:31 +0000
committerMartin Pieuchot <mpi@cvs.openbsd.org>2023-04-25 12:36:31 +0000
commiteba21f3d8d841dbd7d6a710260d947da9c161550 (patch)
treea76d2e31f92885408d5d66c6ff1f5ba875d2b241 /sys/uvm/uvm_map.c
parent3b62579e949755a87ca795e4b45821d5b4975a74 (diff)
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() ok kettenis@
Diffstat (limited to 'sys/uvm/uvm_map.c')
-rw-r--r--sys/uvm/uvm_map.c49
1 files changed, 23 insertions, 26 deletions
diff --git a/sys/uvm/uvm_map.c b/sys/uvm/uvm_map.c
index d73835e0067..794095b96e3 100644
--- a/sys/uvm/uvm_map.c
+++ b/sys/uvm/uvm_map.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: uvm_map.c,v 1.315 2023/04/13 15:23:23 miod Exp $ */
+/* $OpenBSD: uvm_map.c,v 1.316 2023/04/25 12:36:30 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;
}
@@ -5386,6 +5375,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 +5428,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 +5443,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)