Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/sys Fix a corner case locking error, which could lead to map...
details: https://anonhg.NetBSD.org/src/rev/50d395591c91
branches: trunk
changeset: 474219:50d395591c91
user: thorpej <thorpej%NetBSD.org@localhost>
date: Thu Jul 01 20:07:05 1999 +0000
description:
Fix a corner case locking error, which could lead to map corruption in
SMP environments. See comments in <vm/vm_map.h> for details.
diffstat:
sys/uvm/uvm_map.c | 47 +++++++++-----
sys/uvm/uvm_map_i.h | 3 +-
sys/vm/vm_map.h | 160 +++++++++++++++++++++++++++++++++++++++++++--------
3 files changed, 164 insertions(+), 46 deletions(-)
diffs (truncated from 363 to 300 lines):
diff -r 8e006ba8b242 -r 50d395591c91 sys/uvm/uvm_map.c
--- a/sys/uvm/uvm_map.c Thu Jul 01 19:59:59 1999 +0000
+++ b/sys/uvm/uvm_map.c Thu Jul 01 20:07:05 1999 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: uvm_map.c,v 1.59 1999/06/18 05:13:46 thorpej Exp $ */
+/* $NetBSD: uvm_map.c,v 1.60 1999/07/01 20:07:05 thorpej Exp $ */
/*
* Copyright (c) 1997 Charles D. Cranor and Washington University.
@@ -191,23 +191,6 @@
* local inlines
*/
-/* XXX Should not exist! */
-#define vm_map_downgrade(map) \
- (void) lockmgr(&(map)->lock, LK_DOWNGRADE, NULL)
-
-/* XXX Should not exist! */
-#ifdef DIAGNOSTIC
-#define vm_map_upgrade(map) \
-do { \
- if (lockmgr(&(map)->lock, LK_UPGRADE, NULL) != 0) \
- panic("vm_map_upgrade: failed to upgrade lock"); \
-} while (0)
-#else
-#define vm_map_upgrade(map) \
- (void) lockmgr(&(map)->lock, LK_UPGRADE, NULL)
-#endif /* DIAGNOSTIC */
-
-
/*
* uvm_mapent_alloc: allocate a map entry
*
@@ -1997,6 +1980,9 @@
{
vm_map_entry_t entry, start_entry, failed_entry;
int rv;
+#ifdef DIAGNOSTIC
+ u_int timestamp_save;
+#endif
UVMHIST_FUNC("uvm_map_pageable"); UVMHIST_CALLED(maphist);
UVMHIST_LOG(maphist,"(map=0x%x,start=0x%x,end=0x%x,new_pageable=0x%x)",
map, start, end, new_pageable);
@@ -2140,6 +2126,10 @@
* Pass 2.
*/
+#ifdef DIAGNOSTIC
+ timestamp_save = map->timestamp;
+#endif
+ vm_map_busy(map);
vm_map_downgrade(map);
rv = 0;
@@ -2165,6 +2155,12 @@
* Get back to an exclusive (write) lock.
*/
vm_map_upgrade(map);
+ vm_map_unbusy(map);
+
+#ifdef DIAGNOSTIC
+ if (timestamp_save != map->timestamp)
+ panic("uvm_map_pageable: stale map");
+#endif
/*
* first drop the wiring count on all the entries
@@ -2193,6 +2189,7 @@
}
/* We are holding a read lock here. */
+ vm_map_unbusy(map);
vm_map_unlock_read(map);
UVMHIST_LOG(maphist,"<- done (OK WIRE)",0,0,0,0);
@@ -2217,6 +2214,9 @@
vm_map_entry_t entry, failed_entry;
vsize_t size;
int rv;
+#ifdef DIAGNOSTIC
+ u_int timestamp_save;
+#endif
UVMHIST_FUNC("uvm_map_pageable_all"); UVMHIST_CALLED(maphist);
UVMHIST_LOG(maphist,"(map=0x%x,flags=0x%x)", map, flags, 0, 0);
@@ -2345,6 +2345,10 @@
* Pass 3.
*/
+#ifdef DIAGNOSTIC
+ timestamp_save = map->timestamp;
+#endif
+ vm_map_busy(map);
vm_map_downgrade(map);
rv = KERN_SUCCESS;
@@ -2369,6 +2373,12 @@
* Get back an exclusive (write) lock.
*/
vm_map_upgrade(map);
+ vm_map_unbusy(map);
+
+#ifdef DIAGNOSTIC
+ if (timestamp_save != map->timestamp)
+ panic("uvm_map_pageable_all: stale map");
+#endif
/*
* first drop the wiring count on all the entries
@@ -2395,6 +2405,7 @@
}
/* We are holding a read lock here. */
+ vm_map_unbusy(map);
vm_map_unlock_read(map);
UVMHIST_LOG(maphist,"<- done (OK WIRE)",0,0,0,0);
diff -r 8e006ba8b242 -r 50d395591c91 sys/uvm/uvm_map_i.h
--- a/sys/uvm/uvm_map_i.h Thu Jul 01 19:59:59 1999 +0000
+++ b/sys/uvm/uvm_map_i.h Thu Jul 01 20:07:05 1999 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: uvm_map_i.h,v 1.15 1999/06/14 22:05:23 thorpej Exp $ */
+/* $NetBSD: uvm_map_i.h,v 1.16 1999/07/01 20:07:05 thorpej Exp $ */
/*
* Copyright (c) 1997 Charles D. Cranor and Washington University.
@@ -128,6 +128,7 @@
lockinit(&map->lock, PVM, "vmmaplk", 0, 0);
simple_lock_init(&map->ref_lock);
simple_lock_init(&map->hint_lock);
+ simple_lock_init(&map->flags_lock);
/*
* If the map is interrupt safe, place it on the list
diff -r 8e006ba8b242 -r 50d395591c91 sys/vm/vm_map.h
--- a/sys/vm/vm_map.h Thu Jul 01 19:59:59 1999 +0000
+++ b/sys/vm/vm_map.h Thu Jul 01 20:07:05 1999 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: vm_map.h,v 1.32 1999/06/16 17:43:49 thorpej Exp $ */
+/* $NetBSD: vm_map.h,v 1.33 1999/07/01 20:07:05 thorpej Exp $ */
/*
* Copyright (c) 1991, 1993
@@ -128,6 +128,52 @@
* by address. A single hint is provided to start
* searches again from the last successful search,
* insertion, or removal.
+ *
+ * LOCKING PROTOCOL NOTES:
+ * -----------------------
+ *
+ * VM map locking is a little complicated. There are both shared
+ * and exclusive locks on maps. However, it is sometimes required
+ * to downgrade an exclusive lock to a shared lock, and upgrade to
+ * an exclusive lock again (to perform error recovery). However,
+ * another thread *must not* queue itself to receive an exclusive
+ * lock while before we upgrade back to exclusive, otherwise the
+ * error recovery becomes extremely difficult, if not impossible.
+ *
+ * In order to prevent this scenario, we introduce the notion of
+ * a `busy' map. A `busy' map is read-locked, but other threads
+ * attempting to write-lock wait for this flag to clear before
+ * entering the lock manager. A map may only be marked busy
+ * when the map is write-locked (and then the map must be downgraded
+ * to read-locked), and may only be marked unbusy by the thread
+ * which marked it busy (holding *either* a read-lock or a
+ * write-lock, the latter being gained by an upgrade).
+ *
+ * Access to the map `flags' member is controlled by the `flags_lock'
+ * simple lock. Note that some flags are static (set once at map
+ * creation time, and never changed), and thus require no locking
+ * to check those flags. All flags which are r/w must be set or
+ * cleared while the `flags_lock' is asserted. Additional locking
+ * requirements are:
+ *
+ * VM_MAP_PAGEABLE r/o static flag; no locking required
+ *
+ * VM_MAP_INTRSAFE r/o static flag; no locking required
+ *
+ * VM_MAP_WIREFUTURE r/w; may only be set or cleared when
+ * 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.
*/
struct vm_map {
struct pmap * pmap; /* Physical map */
@@ -140,14 +186,8 @@
vm_map_entry_t hint; /* hint for quick lookups */
simple_lock_data_t hint_lock; /* lock for hint storage */
vm_map_entry_t first_free; /* First free space hint */
- /*
- * Locking note: read-only flags need not be locked to read
- * them; they are set once at map creation time, and never
- * changed again. Only read-write flags require that the
- * appropriate map lock be acquired before reading or writing
- * the flag.
- */
int flags; /* flags */
+ simple_lock_data_t flags_lock; /* Lock for flags field */
unsigned int timestamp; /* Version number */
#define min_offset header.start
#define max_offset header.end
@@ -157,6 +197,8 @@
#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 */
/*
* Interrupt-safe maps must also be kept on a special list,
@@ -211,6 +253,14 @@
*
* vm_map_unlock_read: release a shared lock on a map.
*
+ * vm_map_downgrade: downgrade an exclusive lock to a shared lock.
+ *
+ * vm_map_upgrade: upgrade a shared lock to an exclusive lock.
+ *
+ * vm_map_busy: mark a map as busy.
+ *
+ * vm_map_unbusy: clear busy status on a map.
+ *
* Note that "intrsafe" maps use only exclusive, spin locks. We simply
* use the sleep lock's interlock for this.
*/
@@ -218,9 +268,11 @@
#ifdef _KERNEL
/* XXX: clean up later */
#include <sys/time.h>
-#include <sys/proc.h> /* XXX for curproc and p_pid */
+#include <sys/proc.h> /* for tsleep(), wakeup() */
+#include <sys/systm.h> /* for panic() */
static __inline boolean_t vm_map_lock_try __P((vm_map_t));
+static __inline void vm_map_lock __P((vm_map_t));
static __inline boolean_t
vm_map_lock_try(map)
@@ -230,8 +282,15 @@
if (map->flags & VM_MAP_INTRSAFE)
rv = simple_lock_try(&map->lock.lk_interlock);
- else
- rv = (lockmgr(&map->lock, LK_EXCLUSIVE|LK_NOWAIT, NULL) == 0);
+ else {
+ simple_lock(&map->flags_lock);
+ if (map->flags & VM_MAP_BUSY) {
+ simple_unlock(&map->flags_lock);
+ return (FALSE);
+ }
+ rv = (lockmgr(&map->lock, LK_EXCLUSIVE|LK_NOWAIT|LK_INTERLOCK,
+ &map->flags_lock) == 0);
+ }
if (rv)
map->timestamp++;
@@ -239,25 +298,39 @@
return (rv);
}
+static __inline void
+vm_map_lock(map)
+ vm_map_t map;
+{
+ int error;
+
+ if (map->flags & VM_MAP_INTRSAFE) {
+ simple_lock(&map->lock.lk_interlock);
+ return;
+ }
+
+ try_again:
+ simple_lock(&map->flags_lock);
+ if (map->flags & VM_MAP_BUSY) {
+ map->flags |= VM_MAP_WANTLOCK;
+ simple_unlock(&map->flags_lock);
+ (void) tsleep(&map->flags, PVM, "vmmapbsy", 0);
+ goto try_again;
+ }
+
+ error = lockmgr(&map->lock, LK_EXCLUSIVE|LK_SLEEPFAIL|LK_INTERLOCK,
+ &map->flags_lock);
+
+ if (error) {
#ifdef DIAGNOSTIC
-#define _vm_map_lock(map) \
-do { \
- if (lockmgr(&(map)->lock, LK_EXCLUSIVE, NULL) != 0) \
- panic("vm_map_lock: failed to get lock"); \
Home |
Main Index |
Thread Index |
Old Index