[haiku-commits] r38895 - haiku/trunk/src/system/kernel/vm

From: axeld@xxxxxxxxxxxxxxxx
To: haiku-commits@xxxxxxxxxxxxx
Date: Fri, 8 Oct 2010 09:40:42 +0200 (CEST)
Author: axeld
Date: 2010-10-08 09:40:42 +0200 (Fri, 08 Oct 2010)
New Revision: 38895
Changeset: http://dev.haiku-os.org/changeset/38895

Modified:
   haiku/trunk/src/system/kernel/vm/vm_page.cpp
Log:
* Fixed a bug that would cause allocate_page_run() to be called with an out of
  bounds index - the system would overwrite memory then and eventually KDL.
  This could best be reproduced with overlays after a while.
* Added a TODO comment that explains why free_cached_pages() might fail even
  though the page is actually free now.
* Added an explanation of how the sFreePageQueuesLock is to be used, thanks to
  Ingo for explaining it to me in the first place :-)


Modified: haiku/trunk/src/system/kernel/vm/vm_page.cpp
===================================================================
--- haiku/trunk/src/system/kernel/vm/vm_page.cpp        2010-10-07 00:34:12 UTC 
(rev 38894)
+++ haiku/trunk/src/system/kernel/vm/vm_page.cpp        2010-10-08 07:40:42 UTC 
(rev 38895)
@@ -1,6 +1,6 @@
 /*
  * Copyright 2010, Ingo Weinhold, ingo_weinhold@xxxxxxx
- * Copyright 2002-2009, Axel Dörfler, axeld@xxxxxxxxxxxxxxxxx
+ * Copyright 2002-2010, Axel Dörfler, axeld@xxxxxxxxxxxxxxxxx
  * Distributed under the terms of the MIT License.
  *
  * Copyright 2001-2002, Travis Geiselbrecht. All rights reserved.
@@ -125,6 +125,10 @@
 static ConditionVariable sFreePageCondition;
 static mutex sPageDeficitLock = MUTEX_INITIALIZER("page deficit");
 
+// This lock must be used whenever the free or clear page queues are changed.
+// If you need to work on both queues at the same time, you need to hold a 
write
+// lock, otherwise, a read lock suffices (each queue still has a spinlock to
+// guard against concurrent changes).
 static rw_lock sFreePageQueuesLock
        = RW_LOCK_INITIALIZER("free/clear page queues");
 
@@ -3189,15 +3193,14 @@
                DEBUG_PAGE_ACCESS_END(page);
                sClearPageQueue.PrependUnlocked(page);
        }
-
 }
 
 
 /*!    Tries to allocate the a contiguous run of \a length pages starting at
        index \a start.
 
-       The must have write-locked the free/clear page queues. The function will
-       unlock regardless of whether it succeeds or fails.
+       The caller must have write-locked the free/clear page queues. The 
function
+       will unlock regardless of whether it succeeds or fails.
 
        If the function fails, it cleans up after itself, i.e. it will free all
        pages it managed to allocate.
@@ -3220,6 +3223,7 @@
        uint32 pageState = flags & VM_PAGE_ALLOC_STATE;
        ASSERT(pageState != PAGE_STATE_FREE);
        ASSERT(pageState != PAGE_STATE_CLEAR);
+       ASSERT(start + length <= sNumPages);
 
        TA(AllocatePageRun(length));
 
@@ -3298,8 +3302,11 @@
 
                        // free the page, if it is still cached
                        vm_page& page = sPages[nextIndex];
-                       if (!free_cached_page(&page, false))
+                       if (!free_cached_page(&page, false)) {
+                               // TODO: if the page turns out to have been 
freed already,
+                               // there would be no need to fail
                                break;
+                       }
 
                        page.SetState(flags & VM_PAGE_ALLOC_STATE);
                        page.busy = (flags & VM_PAGE_ALLOC_BUSY) != 0;
@@ -3382,6 +3389,7 @@
                end = std::max(restrictions->high_address / B_PAGE_SIZE,
                                sPhysicalPageOffset)
                        - sPhysicalPageOffset;
+               end = std::min(end, sNumPages);
        } else
                end = sNumPages;
[haiku-commits] r38895 - haiku/trunk/src/system/kernel/vm

Other related posts:
