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
This commit is contained in:
Travis Geiselbrecht 2006-12-18 09:38:00 +00:00
parent 9bc7a58b1d
commit c4546ea038

View File

@ -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;
}