From 68a717d4d752a8037dcf165753e4eb662a37e11e Mon Sep 17 00:00:00 2001 From: David Gwynne Date: Sun, 20 Oct 2024 11:28:18 +0000 Subject: try to simplify the locking code around busy maps. vm_maps have a "feature" where they can mark that they're being operated on by a specific proc, and then release the rwlock protecting their state. to relock, you have to be the same proc that marked it busy. this diff tries to simplify it a bit. it basically has threads check the busy field up front and rechecks the busy field inside the rwlock. if you can sleep, it will sleep up front for the busy field to become clear, rather than sleep on either the busy field or the rwlock. some code paths clear the busy field without holding the rwlock, so it doesn't make sense to me to be waiting for the busy field but sleeping somewhere else. ok claudio@ mpi@ --- sys/uvm/uvm_map.c | 96 ++++++++++++++++++++++++++++++------------------------- sys/uvm/uvm_map.h | 21 +++--------- 2 files changed, 57 insertions(+), 60 deletions(-) (limited to 'sys/uvm') diff --git a/sys/uvm/uvm_map.c b/sys/uvm/uvm_map.c index 5fbcfe5f6e3..9de86fa2a7e 100644 --- a/sys/uvm/uvm_map.c +++ b/sys/uvm/uvm_map.c @@ -1,4 +1,4 @@ -/* $OpenBSD: uvm_map.c,v 1.330 2024/07/24 12:17:31 mpi Exp $ */ +/* $OpenBSD: uvm_map.c,v 1.331 2024/10/20 11:28:17 dlg Exp $ */ /* $NetBSD: uvm_map.c,v 1.86 2000/11/27 08:40:03 chs Exp $ */ /* @@ -5201,68 +5201,77 @@ out: boolean_t vm_map_lock_try_ln(struct vm_map *map, char *file, int line) { - boolean_t rv; + int rv; if (map->flags & VM_MAP_INTRSAFE) { - rv = mtx_enter_try(&map->mtx); + if (!mtx_enter_try(&map->mtx)) + return FALSE; } else { + struct proc *busy; + mtx_enter(&map->flags_lock); - if ((map->flags & VM_MAP_BUSY) && (map->busy != curproc)) { - mtx_leave(&map->flags_lock); - return (FALSE); - } + busy = map->busy; mtx_leave(&map->flags_lock); - rv = (rw_enter(&map->lock, RW_WRITE|RW_NOSLEEP) == 0); - /* 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) && - (map->busy != curproc)) { - rw_exit(&map->lock); - rv = FALSE; - } - mtx_leave(&map->flags_lock); + if (busy != NULL && busy != curproc) + return FALSE; + + rv = rw_enter(&map->lock, RW_WRITE|RW_NOSLEEP); + if (rv != 0) + return FALSE; + + /* to be sure, to be sure */ + mtx_enter(&map->flags_lock); + busy = map->busy; + mtx_leave(&map->flags_lock); + if (busy != NULL && busy != curproc) { + rw_exit(&map->lock); + return FALSE; } } - if (rv) { - 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); - } + 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); - return (rv); + return TRUE; } void vm_map_lock_ln(struct vm_map *map, char *file, int line) { if ((map->flags & VM_MAP_INTRSAFE) == 0) { - do { - mtx_enter(&map->flags_lock); -tryagain: - while ((map->flags & VM_MAP_BUSY) && - (map->busy != curproc)) { - map->flags |= VM_MAP_WANTLOCK; - msleep_nsec(&map->flags, &map->flags_lock, + mtx_enter(&map->flags_lock); + for (;;) { + while (map->busy != NULL && map->busy != curproc) { + map->nbusy++; + msleep_nsec(&map->busy, &map->mtx, PVM, vmmapbsy, INFSLP); + map->nbusy--; } mtx_leave(&map->flags_lock); - } 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) && (map->busy != curproc)) { - rw_exit(&map->lock); - goto tryagain; + + rw_enter_write(&map->lock); + + /* to be sure, to be sure */ + mtx_enter(&map->flags_lock); + if (map->busy != NULL && map->busy != curproc) { + /* go around again */ + rw_exit_write(&map->lock); + } else { + /* we won */ + break; + } } mtx_leave(&map->flags_lock); } else { mtx_enter(&map->mtx); } - if (map->busy != curproc) + if (map->busy != curproc) { + KASSERT(map->busy == NULL); 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); @@ -5314,25 +5323,24 @@ vm_map_busy_ln(struct vm_map *map, char *file, int line) mtx_enter(&map->flags_lock); map->busy = curproc; - map->flags |= VM_MAP_BUSY; mtx_leave(&map->flags_lock); } void vm_map_unbusy_ln(struct vm_map *map, char *file, int line) { - int oflags; + unsigned int nbusy; KASSERT((map->flags & VM_MAP_INTRSAFE) == 0); KASSERT(map->busy == curproc); mtx_enter(&map->flags_lock); - oflags = map->flags; + nbusy = map->nbusy; map->busy = NULL; - map->flags &= ~(VM_MAP_BUSY|VM_MAP_WANTLOCK); mtx_leave(&map->flags_lock); - if (oflags & VM_MAP_WANTLOCK) - wakeup(&map->flags); + + if (nbusy > 0) + wakeup(&map->busy); } void diff --git a/sys/uvm/uvm_map.h b/sys/uvm/uvm_map.h index 3a47d6dd899..9f8775f74ea 100644 --- a/sys/uvm/uvm_map.h +++ b/sys/uvm/uvm_map.h @@ -1,4 +1,4 @@ -/* $OpenBSD: uvm_map.h,v 1.90 2024/06/18 12:37:29 jsg Exp $ */ +/* $OpenBSD: uvm_map.h,v 1.91 2024/10/20 11:28:17 dlg Exp $ */ /* $NetBSD: uvm_map.h,v 1.24 2001/02/18 21:19:08 chs Exp $ */ /* @@ -214,17 +214,6 @@ RBT_PROTOTYPE(uvm_map_addr, vm_map_entry, daddrs.addr_entry, * map is write-locked. may be tested * without asserting `flags_lock'. * - * VM_MAP_BUSY r/w; may only be set when map is - * write-locked, may only be cleared by - * thread which set it, map read-locked - * or write-locked. must be tested - * while `flags_lock' is asserted. - * - * VM_MAP_WANTLOCK r/w; may only be set when the map - * is busy, and thread is attempting - * to write-lock. must be tested - * while `flags_lock' is asserted. - * * VM_MAP_GUARDPAGES r/o; must be specified at map * initialization time. * If set, guards will appear between @@ -257,6 +246,7 @@ RBT_PROTOTYPE(uvm_map_addr, vm_map_entry, daddrs.addr_entry, * a atomic operations * I immutable after creation or exec(2) * v `vm_map_lock' (this map `lock' or `mtx') + * f flags_lock */ struct vm_map { struct pmap *pmap; /* [I] Physical map */ @@ -266,9 +256,10 @@ struct vm_map { vsize_t size; /* virtual size */ int ref_count; /* [a] Reference count */ - int flags; /* flags */ + int flags; /* [f] flags */ unsigned int timestamp; /* Version number */ - struct proc *busy; /* [v] thread holding map busy*/ + struct proc *busy; /* [f] thread holding map busy*/ + unsigned int nbusy; /* [f] waiters for busy */ vaddr_t min_offset; /* [I] First address in map. */ vaddr_t max_offset; /* [I] Last address in map. */ @@ -323,8 +314,6 @@ struct vm_map { #define VM_MAP_PAGEABLE 0x01 /* ro: entries are pageable */ #define VM_MAP_INTRSAFE 0x02 /* ro: interrupt safe map */ #define VM_MAP_WIREFUTURE 0x04 /* rw: wire future mappings */ -#define VM_MAP_BUSY 0x08 /* rw: map is busy */ -#define VM_MAP_WANTLOCK 0x10 /* rw: want to write-lock */ #define VM_MAP_GUARDPAGES 0x20 /* rw: add guard pgs to map */ #define VM_MAP_ISVMSPACE 0x40 /* ro: map is a vmspace */ #define VM_MAP_PINSYSCALL_ONCE 0x100 /* rw: pinsyscall done */ -- cgit v1.2.3