From c4546ea038a0282baace0e50fe8815edcc0e1a81 Mon Sep 17 00:00:00 2001 From: Travis Geiselbrecht Date: Mon, 18 Dec 2006 09:38:00 +0000 Subject: [PATCH] A fix for the double-fault on bootup: -Turns out the area removal routine had a massive race condition inside vm_put_area(). Basically the area was removed from the address space's area list before the pages were unmapped, so the vm could (and would) recycle the space before the pages were finally unmapped. It was completely reproducable on my machine during initialization of a bunch of storage drivers that were bringing the locked_pool module into and out of existence, which caused a thread to be spawned and stopped in rapid sucession. On a dual processor machine, it was possible for the new thread to be started up while the old one was still shutting down, and the kernel stack of the new one would get wiped out. Note, there still is a page ref counting problem with this area removal code. It doesn't decrement the ref count of the page as it unmaps it. Will have to figure that out. git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@19549 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- src/system/kernel/vm/vm.cpp | 36 +++++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/src/system/kernel/vm/vm.cpp b/src/system/kernel/vm/vm.cpp index 132952238d..df2b55c46a 100644 --- a/src/system/kernel/vm/vm.cpp +++ b/src/system/kernel/vm/vm.cpp @@ -1385,6 +1385,13 @@ _vm_put_area(vm_area *area, bool aspaceLocked) if (area->id == RESERVED_AREA_ID) return false; + addressSpace = area->address_space; + + // grab a write lock on the address space around the removal of the area + // from the global hash table to avoid a race with vm_soft_fault() + if (!aspaceLocked) + acquire_sem_etc(addressSpace->sem, WRITE_COUNT, 0, 0); + acquire_sem_etc(sAreaHashLock, WRITE_COUNT, 0, 0); if (atomic_add(&area->ref_count, -1) == 1) { hash_remove(sAreaHash, area); @@ -1392,10 +1399,22 @@ _vm_put_area(vm_area *area, bool aspaceLocked) } release_sem_etc(sAreaHashLock, WRITE_COUNT, 0); + if (!aspaceLocked) + release_sem_etc(addressSpace->sem, WRITE_COUNT, 0); + if (!removeit) return false; - addressSpace = area->address_space; + // at this point the area is removed from the global hash table, but still + // exists in the area list. it's ref_count is zero, and is guaranteed not to + // be incremented anymore (by a direct hash lookup, or vm_area_lookup()). + + // unmap the virtual address space the area occupied. any page faults at this + // point should fail in vm_area_lookup(). + vm_translation_map *map = &addressSpace->translation_map; + (*map->ops->lock)(map); + (*map->ops->unmap)(map, area->base, area->base + (area->size - 1)); + (*map->ops->unlock)(map); // ToDo: do that only for vnode stores vm_cache_write_modified(area->cache_ref, false); @@ -1406,11 +1425,6 @@ _vm_put_area(vm_area *area, bool aspaceLocked) vm_cache_remove_area(area->cache_ref, area); vm_cache_release_ref(area->cache_ref); - vm_translation_map *map = &addressSpace->translation_map; - (*map->ops->lock)(map); - (*map->ops->unmap)(map, area->base, area->base + (area->size - 1)); - (*map->ops->unlock)(map); - // now we can give up the area's reference to the address space vm_put_address_space(addressSpace); @@ -2826,6 +2840,7 @@ vm_soft_fault(addr_t originalAddress, bool isWrite, bool isUser) } +/* NOTE: must have the address space's sem held */ static vm_area * vm_area_lookup(vm_address_space *addressSpace, addr_t address) { @@ -2834,7 +2849,7 @@ vm_area_lookup(vm_address_space *addressSpace, addr_t address) // check the areas list first area = addressSpace->area_hint; if (area && area->base <= address && (area->base + area->size) > address) - return area; + goto found; for (area = addressSpace->areas; area != NULL; area = area->address_space_next) { if (area->id == RESERVED_AREA_ID) @@ -2844,8 +2859,15 @@ vm_area_lookup(vm_address_space *addressSpace, addr_t address) break; } +found: + // if the ref count is zero, the area is in the middle of being + // destroyed in _vm_put_area. pretend it doesn't exist. + if (area && area->ref_count == 0) + return NULL; + if (area) addressSpace->area_hint = area; + return area; }