* VMArea::Unwire(addr_t, size_t, bool): Don't delete the removed range, but

return it.
* lock_memory_etc(): On error the VMAreaWiredRange object could be leaked.
* [un]lock_memory_etc(): Call VMArea::Unwire() with the cache locked and
  explicitly delete the range object after unlocking the cache to avoid
  potential deadlocks.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@36035 a95241bf-73f2-0310-859d-f6bbb57e9c96

headers/private/kernel/vm/VMArea.h

@@ -95,7 +95,7 @@ struct VMArea {
 
 			void				Wire(VMAreaWiredRange* range);
 			void				Unwire(VMAreaWiredRange* range);
-			void				Unwire(addr_t base, size_t size, bool writable);
+			VMAreaWiredRange*	Unwire(addr_t base, size_t size, bool writable);
 
 			bool				AddWaiterIfWired(VMAreaUnwiredWaiter* waiter);
 			bool				AddWaiterIfWired(VMAreaUnwiredWaiter* waiter,

src/system/kernel/vm/VMArea.cpp

@@ -131,10 +131,12 @@ VMArea::Unwire(VMAreaWiredRange* range)
 
 /*!	Removes a wired range from this area.
 
-	Must balance a previous Wire() call.
+	Must balance a previous Wire() call. The first implicit range with matching
+	\a base, \a size, and \a writable attributes is removed and returned. It's
+	waiters are woken up as well.
 	The area's top cache must be locked.
 */
-void
+VMAreaWiredRange*
 VMArea::Unwire(addr_t base, size_t size, bool writable)
 {
 	for (VMAreaWiredRangeList::Iterator it = fWiredRanges.GetIterator();
@@ -142,13 +144,13 @@ VMArea::Unwire(addr_t base, size_t size, bool writable)
 		if (range->implicit && range->base == base && range->size == size
 				&& range->writable == writable) {
 			Unwire(range);
-			delete range;
-			return;
+			return range;
 		}
 	}
 
 	panic("VMArea::Unwire(%#" B_PRIxADDR ", %#" B_PRIxADDR ", %d): no such "
 		"range", base, size, writable);
+	return NULL;
 }
 
 

src/system/kernel/vm/vm.cpp

@@ -4762,6 +4762,9 @@ lock_memory_etc(team_id team, void* address, size_t numBytes, uint32 flags)
 	if (writable)
 		requiredProtection |= B_KERNEL_WRITE_AREA | (isUser ? B_WRITE_AREA : 0);
 
+	uint32 mallocFlags = isUser
+		? 0 : HEAP_DONT_WAIT_FOR_MEMORY | HEAP_DONT_LOCK_KERNEL_SPACE;
+
 	// get and read lock the address space
 	VMAddressSpace* addressSpace = NULL;
 	if (isUser) {
@@ -4794,8 +4797,6 @@ lock_memory_etc(team_id team, void* address, size_t numBytes, uint32 flags)
 
 		// allocate the wired range (do that before locking the cache to avoid
 		// deadlocks)
-		uint32 mallocFlags = isUser
-			? 0 : HEAP_DONT_WAIT_FOR_MEMORY | HEAP_DONT_LOCK_KERNEL_SPACE;
 		VMAreaWiredRange* range = new(malloc_flags(mallocFlags))
 			VMAreaWiredRange(areaStart, areaEnd - areaStart, writable, true);
 		if (range == NULL) {
@@ -4860,14 +4861,21 @@ lock_memory_etc(team_id team, void* address, size_t numBytes, uint32 flags)
 		}
 
 		map->Unlock();
-		cacheChainLocker.Unlock();
 
-		if (error != B_OK) {
+		if (error == B_OK) {
+			cacheChainLocker.Unlock();
+		} else {
 			// An error occurred, so abort right here. If the current address
 			// is the first in this area, unwire the area, since we won't get
 			// to it when reverting what we've done so far.
-			if (nextAddress == areaStart)
+			if (nextAddress == areaStart) {
 				area->Unwire(range);
+				cacheChainLocker.Unlock();
+				range->~VMAreaWiredRange();
+				free_etc(range, mallocFlags);
+			} else
+				cacheChainLocker.Unlock();
+
 			break;
 		}
 	}
@@ -4911,6 +4919,9 @@ unlock_memory_etc(team_id team, void* address, size_t numBytes, uint32 flags)
 	if (writable)
 		requiredProtection |= B_KERNEL_WRITE_AREA | (isUser ? B_WRITE_AREA : 0);
 
+	uint32 mallocFlags = isUser
+		? 0 : HEAP_DONT_WAIT_FOR_MEMORY | HEAP_DONT_LOCK_KERNEL_SPACE;
+
 	// get and read lock the address space
 	VMAddressSpace* addressSpace = NULL;
 	if (isUser) {
@@ -4951,8 +4962,16 @@ unlock_memory_etc(team_id team, void* address, size_t numBytes, uint32 flags)
 			|| area->cache_type == CACHE_TYPE_DEVICE
 			|| area->wiring == B_FULL_LOCK
 			|| area->wiring == B_CONTIGUOUS) {
+			// unwire the range (to avoid deadlocks we delete the range after
+			// unlocking the cache)
 			nextAddress = areaEnd;
-			area->Unwire(areaStart, areaEnd - areaStart, writable);
+			VMAreaWiredRange* range = area->Unwire(areaStart,
+				areaEnd - areaStart, writable);
+			cacheChainLocker.Unlock();
+			if (range != NULL) {
+				range->~VMAreaWiredRange();
+				free_etc(range, mallocFlags);
+			}
 			continue;
 		}
 
@@ -4984,10 +5003,18 @@ unlock_memory_etc(team_id team, void* address, size_t numBytes, uint32 flags)
 		}
 
 		map->Unlock();
+
+		// All pages are unwired. Remove the area's wired range as well (to
+		// avoid deadlocks we delete the range after unlocking the cache).
+		VMAreaWiredRange* range = area->Unwire(areaStart,
+			areaEnd - areaStart, writable);
+
 		cacheChainLocker.Unlock();
 
-		// all pages are unwired -- remove the area's wired range as well
-		area->Unwire(areaStart, areaEnd - areaStart, writable);
+		if (range != NULL) {
+			range->~VMAreaWiredRange();
+			free_etc(range, mallocFlags);
+		}
 
 		if (error != B_OK)
 			break;