[cgutman] 56284: [NTOSKRNL] - The legacy ROS Mm uses structures known as page ops to track operations on a page such as page out, page in, and access fault. The idea is that each operation is force...

Author: cgutman
Date: Fri Mar 30 18:09:16 2012
New Revision: 56284

URL: http://svn.reactos.org/svn/reactos?rev=56284&view=rev
Log:
[NTOSKRNL]
- The legacy ROS Mm uses structures known as page ops to track operations on a page such
as page out, page in, and access fault. The idea is that each operation is forced to wait
until the page has completed all other pending operations to start its work. The problem
was that the page op's completion event was a NotificationEvent instead of a
SynchronizationEvent. This caused all operations to proceed at the same time if they were
waiting on a single page op to begin. Now that is fixed and page ops proceed one after
another as intended. This bug has been around since r9077.
- When waiting for sections to be unmapped or destroyed, the Mm waits for pending page
operations to complete. The problem is that MmUnmapViewOfSection had a critical bug in
which it forgot to dereference the page op it just retrieved. This caused zombie page ops
to be stuck to that particular address if there were any pending operations at the time of
the MmUnmapViewOfSection call. As a result, section destruction to bug check due to the
hung page op after waiting 10 seconds for the operation to complete. This bug has been
around since r18849.
- Due to the combined effects of the above bugs, sometimes unmapping or freeing a section
would hang the system or bug check the system (remember the "Failed to wait for page
op" messages?). This was evident in smiley_'s theme work when many applications
calling FreeLibrary at the same time would trigger this bug. There are likely many more
cases that triggered this series of events which either could never be replicated or
simply could not be explained.

Modified:
    trunk/reactos/ntoskrnl/mm/pageop.c
    trunk/reactos/ntoskrnl/mm/section.c

Modified: trunk/reactos/ntoskrnl/mm/pageop.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/pageop.c?rev=5…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/pageop.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/pageop.c [iso-8859-1] Fri Mar 30 18:09:16 2012
@@ -239,7 +239,7 @@
    PageOp->Status = STATUS_PENDING;
    PageOp->OpType = OpType;
    PageOp->MArea = MArea;
-   KeInitializeEvent(&PageOp->CompletionEvent, NotificationEvent, FALSE);
+   KeInitializeEvent(&PageOp->CompletionEvent, SynchronizationEvent, FALSE);
    MmPageOpHashTable[Hash] = PageOp;
    (void)InterlockedIncrementUL(&MArea->PageOpCount);
 

Modified: trunk/reactos/ntoskrnl/mm/section.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/section.c?rev=…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/section.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/section.c [iso-8859-1] Fri Mar 30 18:09:16 2012
@@ -4254,6 +4254,7 @@
                KeBugCheck(MEMORY_MANAGEMENT);
             }
             MmLockAddressSpace(AddressSpace);
+            MmspCompleteAndReleasePageOp(PageOp);
             MemoryArea = MmLocateMemoryAreaByAddress(AddressSpace,
                                                      BaseAddress);
             if (MemoryArea == NULL ||