Date: Wed, 03 Oct 2001 21:05:36 -0400 From: Brian Fundakowski Feldman <green@FreeBSD.org> To: arch@FreeBSD.org Subject: sx-ifying src/sys/vm Message-ID: <200110040105.f9415av15297@green.bikeshed.org>
next in thread | raw e-mail | index | archive | help
I've taken a shot at changing the vm_map locking from using lockmgr() locks
(EAAARGHH...) to sx locks. Already I can tell that it 1. seems to work and
2. seems to not give me bogus lock ordering messages like lockmgr() used to.
I'd appreciate if people took a look at it and told me of any concerns,
whether it also looks fine to more than just myself, etc.
I added a few more vm_map locking operations than were there already for the
purpose of removing the hard-coded lockmgr() ops that were in a few places
previously. Instead of having a sx_locked() operation I used
sx_try_xlock(), and instead of recursion I saved the state of the one case
where a lock was attempted to be "recursed" upon, and removed the actual
recursion on the lock.
I also used EWOULDBLOCK instead of EBUSY as a return value for operations
that would block on attempting to acquire a lock, which I think makes more
sense, but in any case is not used currently in any of the code. I could go
either way on that :)
Here are all the changes for review:
Index: vm_fault.c
===================================================================
RCS file: /home/freebsd/ncvs/src/sys/vm/vm_fault.c,v
retrieving revision 1.125
diff -u -r1.125 vm_fault.c
--- vm_fault.c 2001/09/12 08:38:11 1.125
+++ vm_fault.c 2001/10/04 00:41:36
@@ -114,6 +114,7 @@
vm_map_entry_t entry;
int lookup_still_valid;
struct vnode *vp;
+ int lockheld;
};
static __inline void
@@ -127,7 +128,7 @@
static __inline void
unlock_map(struct faultstate *fs)
{
- if (fs->lookup_still_valid) {
+ if (fs->lookup_still_valid && !fs->lockheld) {
vm_map_lookup_done(fs->map, fs->entry);
fs->lookup_still_valid = FALSE;
}
@@ -213,12 +214,16 @@
int hardfault;
int faultcount;
struct faultstate fs;
+ boolean_t user_wire;
GIANT_REQUIRED;
cnt.v_vm_faults++;
hardfault = 0;
+ user_wire = (fault_flags & VM_FAULT_WIRE_MASK) == VM_FAULT_USER_WIRE;
+ fs.lockheld = user_wire;
+
RetryFault:;
/*
@@ -226,13 +231,11 @@
* search.
*/
fs.map = map;
- if ((result = vm_map_lookup(&fs.map, vaddr,
+ if ((result = vm_map_lookup2(&fs.map, vaddr,
fault_type, &fs.entry, &fs.first_object,
- &fs.first_pindex, &prot, &wired)) != KERN_SUCCESS) {
- if ((result != KERN_PROTECTION_FAILURE) ||
- ((fault_flags & VM_FAULT_WIRE_MASK) != VM_FAULT_USER_WIRE)) {
+ &fs.first_pindex, &prot, &wired, user_wire)) != KERN_SUCCESS) {
+ if (result != KERN_PROTECTION_FAILURE || !user_wire)
return result;
- }
/*
* If we are user-wiring a r/w segment, and it is COW, then
@@ -241,9 +244,10 @@
* to COW .text. We simply keep .text from ever being COW'ed
* and take the heat that one cannot debug wired .text sections.
*/
- result = vm_map_lookup(&fs.map, vaddr,
+ result = vm_map_lookup2(&fs.map, vaddr,
VM_PROT_READ|VM_PROT_WRITE|VM_PROT_OVERRIDE_WRITE,
- &fs.entry, &fs.first_object, &fs.first_pindex, &prot, &wired);
+ &fs.entry, &fs.first_object, &fs.first_pindex, &prot,
+ &wired, user_wire);
if (result != KERN_SUCCESS) {
return result;
}
@@ -666,7 +670,7 @@
* grab the lock if we need to
*/
(fs.lookup_still_valid ||
- lockmgr(&fs.map->lock, LK_EXCLUSIVE|LK_NOWAIT, (void *)0, curthread) == 0)
+ vm_map_lock_upgrade(fs.map) == 0)
) {
fs.lookup_still_valid = 1;
Index: vm_glue.c
===================================================================
RCS file: /home/freebsd/ncvs/src/sys/vm/vm_glue.c,v
retrieving revision 1.119
diff -u -r1.119 vm_glue.c
--- vm_glue.c 2001/09/12 08:38:11 1.119
+++ vm_glue.c 2001/10/04 00:41:37
@@ -577,9 +577,7 @@
* data structures there is a
* possible deadlock.
*/
- if (lockmgr(&vm->vm_map.lock,
- LK_EXCLUSIVE | LK_NOWAIT,
- NULL, curthread)) {
+ if (vm_map_trylock(&vm->vm_map)) {
vmspace_free(vm);
PROC_UNLOCK(p);
goto nextproc;
Index: vm_map.c
===================================================================
RCS file: /home/freebsd/ncvs/src/sys/vm/vm_map.c,v
retrieving revision 1.207
diff -u -r1.207 vm_map.c
--- vm_map.c 2001/09/12 08:38:11 1.207
+++ vm_map.c 2001/10/04 00:41:44
@@ -266,70 +266,59 @@
vm_map_lock(vm_map_t map)
{
vm_map_printf("locking map LK_EXCLUSIVE: %p\n", map);
- if (lockmgr(&map->lock, LK_EXCLUSIVE, NULL, curthread) != 0)
- panic("vm_map_lock: failed to get lock");
+ sx_xlock(&map->lock);
map->timestamp++;
}
+int
+vm_map_trylock(vm_map_t map)
+{
+ vm_map_printf("trying to lock map LK_EXCLUSIVE: %p\n", map);
+ if (sx_try_xlock(&map->lock)) {
+ map->timestamp++;
+ return (0);
+ }
+ return (EWOULDBLOCK);
+}
+
void
vm_map_unlock(vm_map_t map)
{
vm_map_printf("locking map LK_RELEASE: %p\n", map);
- lockmgr(&(map)->lock, LK_RELEASE, NULL, curthread);
+ sx_xunlock(&map->lock);
}
void
vm_map_lock_read(vm_map_t map)
{
vm_map_printf("locking map LK_SHARED: %p\n", map);
- lockmgr(&(map)->lock, LK_SHARED, NULL, curthread);
+ sx_slock(&map->lock);
}
void
vm_map_unlock_read(vm_map_t map)
{
vm_map_printf("locking map LK_RELEASE: %p\n", map);
- lockmgr(&(map)->lock, LK_RELEASE, NULL, curthread);
-}
-
-static __inline__ int
-_vm_map_lock_upgrade(vm_map_t map, struct thread *td) {
- int error;
-
- vm_map_printf("locking map LK_EXCLUPGRADE: %p\n", map);
- error = lockmgr(&map->lock, LK_EXCLUPGRADE, NULL, td);
- if (error == 0)
- map->timestamp++;
- return error;
+ sx_sunlock(&map->lock);
}
int
vm_map_lock_upgrade(vm_map_t map)
{
- return(_vm_map_lock_upgrade(map, curthread));
+ vm_map_printf("locking map LK_EXCLUPGRADE: %p\n", map);
+ if (sx_try_upgrade(&map->lock)) {
+ map->timestamp++;
+ return (0);
+ }
+ sx_sunlock(&map->lock);
+ return (EWOULDBLOCK);
}
void
vm_map_lock_downgrade(vm_map_t map)
{
vm_map_printf("locking map LK_DOWNGRADE: %p\n", map);
- lockmgr(&map->lock, LK_DOWNGRADE, NULL, curthread);
-}
-
-void
-vm_map_set_recursive(vm_map_t map)
-{
- mtx_lock((map)->lock.lk_interlock);
- map->lock.lk_flags |= LK_CANRECURSE;
- mtx_unlock((map)->lock.lk_interlock);
-}
-
-void
-vm_map_clear_recursive(vm_map_t map)
-{
- mtx_lock((map)->lock.lk_interlock);
- map->lock.lk_flags &= ~LK_CANRECURSE;
- mtx_unlock((map)->lock.lk_interlock);
+ sx_downgrade(&map->lock);
}
vm_offset_t
@@ -403,7 +392,7 @@
map->first_free = &map->header;
map->hint = &map->header;
map->timestamp = 0;
- lockinit(&map->lock, PVM, "thrd_sleep", 0, LK_NOPAUSE);
+ sx_init(&map->lock, "thrd_sleep");
}
void
@@ -411,7 +400,7 @@
struct vm_map *map;
{
GIANT_REQUIRED;
- lockdestroy(&map->lock);
+ sx_destroy(&map->lock);
}
/*
@@ -1490,7 +1479,6 @@
estart = entry->start;
/* First we need to allow map modifications */
- vm_map_set_recursive(map);
vm_map_lock_downgrade(map);
map->timestamp++;
@@ -1500,14 +1488,12 @@
entry->wired_count--;
entry->eflags &= ~MAP_ENTRY_USER_WIRED;
- vm_map_clear_recursive(map);
- vm_map_unlock(map);
+ vm_map_unlock_read(map);
(void) vm_map_user_pageable(map, start, entry->start, TRUE);
return rv;
}
- vm_map_clear_recursive(map);
if (vm_map_lock_upgrade(map)) {
vm_map_lock(map);
if (vm_map_lookup_entry(map, estart, &entry)
@@ -1748,14 +1734,20 @@
vm_map_lock(map);
}
if (rv) {
- vm_map_unlock(map);
+ if (vm_map_pmap(map) == kernel_pmap)
+ vm_map_unlock(map);
+ else
+ vm_map_unlock_read(map);
(void) vm_map_pageable(map, start, failed, TRUE);
return (rv);
}
vm_map_simplify_entry(map, start_entry);
}
- vm_map_unlock(map);
+ if (new_pageable || vm_map_pmap(map) == kernel_pmap)
+ vm_map_unlock(map);
+ else
+ vm_map_unlock_read(map);
return (KERN_SUCCESS);
}
@@ -2678,6 +2670,22 @@
vm_prot_t *out_prot, /* OUT */
boolean_t *wired) /* OUT */
{
+
+ return (vm_map_lookup2(var_map, vaddr, fault_typea, out_entry,
+ object, pindex, out_prot, wired, 0));
+}
+
+int
+vm_map_lookup2(vm_map_t *var_map, /* IN/OUT */
+ vm_offset_t vaddr,
+ vm_prot_t fault_typea,
+ vm_map_entry_t *out_entry, /* OUT */
+ vm_object_t *object, /* OUT */
+ vm_pindex_t *pindex, /* OUT */
+ vm_prot_t *out_prot, /* OUT */
+ boolean_t *wired, /* OUT */
+ boolean_t lockheld)
+{
vm_map_entry_t entry;
vm_map_t map = *var_map;
vm_prot_t prot;
@@ -2690,11 +2698,13 @@
* Lookup the faulting address.
*/
- vm_map_lock_read(map);
+ if (!lockheld)
+ vm_map_lock_read(map);
#define RETURN(why) \
{ \
- vm_map_unlock_read(map); \
+ if (!lockheld) \
+ vm_map_unlock_read(map); \
return(why); \
}
@@ -2730,7 +2740,8 @@
vm_map_t old_map = map;
*var_map = map = entry->object.sub_map;
- vm_map_unlock_read(old_map);
+ if (!lockheld)
+ vm_map_unlock_read(old_map);
goto RetryLookup;
}
Index: vm_map.h
===================================================================
RCS file: /home/freebsd/ncvs/src/sys/vm/vm_map.h,v
retrieving revision 1.67
diff -u -r1.67 vm_map.h
--- vm_map.h 2001/09/12 08:38:11 1.67
+++ vm_map.h 2001/10/04 00:41:44
@@ -71,7 +71,8 @@
#ifndef _VM_MAP_
#define _VM_MAP_
-#include <sys/lockmgr.h>
+#include <sys/lock.h>
+#include <sys/sx.h>
#ifdef MAP_LOCK_DIAGNOSTIC
#include <sys/systm.h>
@@ -154,7 +155,7 @@
*/
struct vm_map {
struct vm_map_entry header; /* List of entries */
- struct lock lock; /* Lock for map data */
+ struct sx lock; /* Lock for map data */
int nentries; /* Number of entries */
vm_size_t size; /* virtual size */
u_char system_map; /* Am I a system map? */
@@ -216,13 +217,12 @@
#endif
void vm_map_lock(vm_map_t map);
+int vm_map_trylock(vm_map_t map);
void vm_map_unlock(vm_map_t map);
void vm_map_lock_read(vm_map_t map);
void vm_map_unlock_read(vm_map_t map);
int vm_map_lock_upgrade(vm_map_t map);
void vm_map_lock_downgrade(vm_map_t map);
-void vm_map_set_recursive(vm_map_t map);
-void vm_map_clear_recursive(vm_map_t map);
vm_offset_t vm_map_min(vm_map_t map);
vm_offset_t vm_map_max(vm_map_t map);
struct pmap *vm_map_pmap(vm_map_t map);
@@ -272,6 +272,8 @@
int vm_map_insert (vm_map_t, vm_object_t, vm_ooffset_t, vm_offset_t, vm_offset_t, vm_prot_t, vm_prot_t, int);
int vm_map_lookup (vm_map_t *, vm_offset_t, vm_prot_t, vm_map_entry_t *, vm_object_t *,
vm_pindex_t *, vm_prot_t *, boolean_t *);
+int vm_map_lookup2 (vm_map_t *, vm_offset_t, vm_prot_t, vm_map_entry_t *, vm_object_t *,
+ vm_pindex_t *, vm_prot_t *, boolean_t *, boolean_t);
void vm_map_lookup_done (vm_map_t, vm_map_entry_t);
boolean_t vm_map_lookup_entry (vm_map_t, vm_offset_t, vm_map_entry_t *);
int vm_map_pageable (vm_map_t, vm_offset_t, vm_offset_t, boolean_t);
Index: vm_pageout.c
===================================================================
RCS file: /home/freebsd/ncvs/src/sys/vm/vm_pageout.c,v
retrieving revision 1.183
diff -u -r1.183 vm_pageout.c
--- vm_pageout.c 2001/09/12 08:38:11 1.183
+++ vm_pageout.c 2001/10/04 00:41:47
@@ -546,9 +546,8 @@
vm_object_t obj, bigobj;
GIANT_REQUIRED;
- if (lockmgr(&map->lock, LK_EXCLUSIVE | LK_NOWAIT, (void *)0, curthread)) {
+ if (vm_map_trylock(map))
return;
- }
bigobj = NULL;
Index: vm_zone.c
===================================================================
RCS file: /home/freebsd/ncvs/src/sys/vm/vm_zone.c,v
retrieving revision 1.48
diff -u -r1.48 vm_zone.c
--- vm_zone.c 2001/08/05 03:55:02 1.48
+++ vm_zone.c 2001/10/04 00:41:48
@@ -409,11 +409,12 @@
* map.
*/
mtx_unlock(&z->zmtx);
- if (lockstatus(&kernel_map->lock, NULL)) {
+ if (vm_map_trylock(kernel_map)) {
item = (void *) kmem_malloc(kmem_map, nbytes, M_WAITOK);
if (item != NULL)
atomic_add_int(&zone_kmem_pages, z->zalloc);
} else {
+ vm_map_unlock(kernel_map);
item = (void *) kmem_alloc(kernel_map, nbytes);
if (item != NULL)
atomic_add_int(&zone_kern_pages, z->zalloc);
--
Brian Fundakowski Feldman \ FreeBSD: The Power to Serve! /
green@FreeBSD.org `------------------------------'
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-arch" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200110040105.f9415av15297>