I know I probably don't have much weight anymore but, I re-iterate that one of the goals of the project should be to make things as similar as possible for debugging/troubleshooting and learning as Server 2003... which does have this variable. I don't think it's the end of the world to keep it around. Stuff like this also tends to break kdexts.dll.
Best regards, Alex Ionescu
On Mon, Apr 6, 2020 at 5:15 AM Thomas Faber thomas.faber@reactos.org wrote:
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=25a5aee86f43e9f526d4d3...
commit 25a5aee86f43e9f526d4d360e4e4f35feb9d22b4 Author: Thomas Faber thomas.faber@reactos.org AuthorDate: Sat Feb 22 12:31:54 2020 +0100 Commit: Thomas Faber thomas.faber@reactos.org CommitDate: Mon Apr 6 11:13:55 2020 +0200
[NTOS:MM] Get rid of unnecessary MmZeroingPageThreadActive.
ntoskrnl/mm/ARM3/miarm.h | 1 - ntoskrnl/mm/ARM3/mminit.c | 5 ++--- ntoskrnl/mm/ARM3/pfnlist.c | 3 +-- ntoskrnl/mm/ARM3/zeropage.c | 3 +-- 4 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/ntoskrnl/mm/ARM3/miarm.h b/ntoskrnl/mm/ARM3/miarm.h index c242b795af6..9d85c42afff 100644 --- a/ntoskrnl/mm/ARM3/miarm.h +++ b/ntoskrnl/mm/ARM3/miarm.h @@ -639,7 +639,6 @@ extern PMMPDE MiHighestUserPde; extern PFN_NUMBER MmSystemPageDirectory[PPE_PER_PAGE]; extern PMMPTE MmSharedUserDataPte; extern LIST_ENTRY MmProcessList; -extern BOOLEAN MmZeroingPageThreadActive; extern KEVENT MmZeroingPageEvent; extern ULONG MmSystemPageColor; extern ULONG MmProcessColorSeed; diff --git a/ntoskrnl/mm/ARM3/mminit.c b/ntoskrnl/mm/ARM3/mminit.c index 7cd7af10838..d72e7186193 100644 --- a/ntoskrnl/mm/ARM3/mminit.c +++ b/ntoskrnl/mm/ARM3/mminit.c @@ -2181,9 +2181,8 @@ MmArmInitSystem(IN ULONG Phase, /* Initialize the Loader Lock */ KeInitializeMutant(&MmSystemLoadLock, FALSE);
/* Set the zero page event */KeInitializeEvent(&MmZeroingPageEvent, SynchronizationEvent, FALSE);MmZeroingPageThreadActive = FALSE;
/* Set up the zero page event */KeInitializeEvent(&MmZeroingPageEvent, NotificationEvent, FALSE); /* Initialize the dead stack S-LIST */ InitializeSListHead(&MmDeadStackSListHead);diff --git a/ntoskrnl/mm/ARM3/pfnlist.c b/ntoskrnl/mm/ARM3/pfnlist.c index eac3a043418..f175541a500 100644 --- a/ntoskrnl/mm/ARM3/pfnlist.c +++ b/ntoskrnl/mm/ARM3/pfnlist.c @@ -701,10 +701,9 @@ MiInsertPageInFreeList(IN PFN_NUMBER PageFrameIndex) ColorTable->Count++;
/* Notify zero page thread if enough pages are on the free list now */
- if ((ListHead->Total >= 8) && !(MmZeroingPageThreadActive))
- if (ListHead->Total >= 8) { /* Set the event */
}MmZeroingPageThreadActive = TRUE; KeSetEvent(&MmZeroingPageEvent, IO_NO_INCREMENT, FALSE);diff --git a/ntoskrnl/mm/ARM3/zeropage.c b/ntoskrnl/mm/ARM3/zeropage.c index 6d859f6806f..5fe22294199 100644 --- a/ntoskrnl/mm/ARM3/zeropage.c +++ b/ntoskrnl/mm/ARM3/zeropage.c @@ -17,7 +17,6 @@
/* GLOBALS ********************************************************************/
-BOOLEAN MmZeroingPageThreadActive; KEVENT MmZeroingPageEvent;
/* PRIVATE FUNCTIONS **********************************************************/ @@ -73,7 +72,7 @@ MmZeroPageThread(VOID) { if (!MmFreePageListHead.Total) {
MmZeroingPageThreadActive = FALSE;
KeClearEvent(&MmZeroingPageEvent); MiReleasePfnLock(OldIrql); break; }
I agree with Alex's reasoning, plus in a multiprocessor environment such page zeroing may be done asynchronously to other cores or processors attempting to acquire use of that page while its state is inconsistent. Removing it counts as a security hole, in other words, except for single core processors that have interrupts disabled, that I see.
Sent from Outlook Mobilehttps://aka.ms/blhgte
________________________________ From: Ros-dev ros-dev-bounces@reactos.org on behalf of Alex Ionescu ionucu@videotron.ca Sent: Sunday, May 10, 2020 12:37:09 PM To: ReactOS Development List ros-dev@reactos.org; Thomas Faber thomas.faber@reactos.org Cc: Linda Wang ros-diffs@reactos.org Subject: Re: [ros-dev] [ros-diffs] [reactos] 02/02: [NTOS:MM] Get rid of unnecessary MmZeroingPageThreadActive.
I know I probably don't have much weight anymore but, I re-iterate that one of the goals of the project should be to make things as similar as possible for debugging/troubleshooting and learning as Server 2003... which does have this variable. I don't think it's the end of the world to keep it around. Stuff like this also tends to break kdexts.dll.
Best regards, Alex Ionescu
On Mon, Apr 6, 2020 at 5:15 AM Thomas Faber thomas.faber@reactos.org wrote:
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=25a5aee86f43e9f526d4d3...
commit 25a5aee86f43e9f526d4d360e4e4f35feb9d22b4 Author: Thomas Faber thomas.faber@reactos.org AuthorDate: Sat Feb 22 12:31:54 2020 +0100 Commit: Thomas Faber thomas.faber@reactos.org CommitDate: Mon Apr 6 11:13:55 2020 +0200
[NTOS:MM] Get rid of unnecessary MmZeroingPageThreadActive.
ntoskrnl/mm/ARM3/miarm.h | 1 - ntoskrnl/mm/ARM3/mminit.c | 5 ++--- ntoskrnl/mm/ARM3/pfnlist.c | 3 +-- ntoskrnl/mm/ARM3/zeropage.c | 3 +-- 4 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/ntoskrnl/mm/ARM3/miarm.h b/ntoskrnl/mm/ARM3/miarm.h index c242b795af6..9d85c42afff 100644 --- a/ntoskrnl/mm/ARM3/miarm.h +++ b/ntoskrnl/mm/ARM3/miarm.h @@ -639,7 +639,6 @@ extern PMMPDE MiHighestUserPde; extern PFN_NUMBER MmSystemPageDirectory[PPE_PER_PAGE]; extern PMMPTE MmSharedUserDataPte; extern LIST_ENTRY MmProcessList; -extern BOOLEAN MmZeroingPageThreadActive; extern KEVENT MmZeroingPageEvent; extern ULONG MmSystemPageColor; extern ULONG MmProcessColorSeed; diff --git a/ntoskrnl/mm/ARM3/mminit.c b/ntoskrnl/mm/ARM3/mminit.c index 7cd7af10838..d72e7186193 100644 --- a/ntoskrnl/mm/ARM3/mminit.c +++ b/ntoskrnl/mm/ARM3/mminit.c @@ -2181,9 +2181,8 @@ MmArmInitSystem(IN ULONG Phase, /* Initialize the Loader Lock */ KeInitializeMutant(&MmSystemLoadLock, FALSE);
/* Set the zero page event */KeInitializeEvent(&MmZeroingPageEvent, SynchronizationEvent, FALSE);MmZeroingPageThreadActive = FALSE;
/* Set up the zero page event */KeInitializeEvent(&MmZeroingPageEvent, NotificationEvent, FALSE); /* Initialize the dead stack S-LIST */ InitializeSListHead(&MmDeadStackSListHead);diff --git a/ntoskrnl/mm/ARM3/pfnlist.c b/ntoskrnl/mm/ARM3/pfnlist.c index eac3a043418..f175541a500 100644 --- a/ntoskrnl/mm/ARM3/pfnlist.c +++ b/ntoskrnl/mm/ARM3/pfnlist.c @@ -701,10 +701,9 @@ MiInsertPageInFreeList(IN PFN_NUMBER PageFrameIndex) ColorTable->Count++;
/* Notify zero page thread if enough pages are on the free list now */
- if ((ListHead->Total >= 8) && !(MmZeroingPageThreadActive))
- if (ListHead->Total >= 8) { /* Set the event */
}MmZeroingPageThreadActive = TRUE; KeSetEvent(&MmZeroingPageEvent, IO_NO_INCREMENT, FALSE);diff --git a/ntoskrnl/mm/ARM3/zeropage.c b/ntoskrnl/mm/ARM3/zeropage.c index 6d859f6806f..5fe22294199 100644 --- a/ntoskrnl/mm/ARM3/zeropage.c +++ b/ntoskrnl/mm/ARM3/zeropage.c @@ -17,7 +17,6 @@
/* GLOBALS ********************************************************************/
-BOOLEAN MmZeroingPageThreadActive; KEVENT MmZeroingPageEvent;
/* PRIVATE FUNCTIONS **********************************************************/ @@ -73,7 +72,7 @@ MmZeroPageThread(VOID) { if (!MmFreePageListHead.Total) {
MmZeroingPageThreadActive = FALSE;
KeClearEvent(&MmZeroingPageEvent); MiReleasePfnLock(OldIrql); break; }
_______________________________________________ Ros-dev mailing list Ros-dev@reactos.org http://reactos.org/mailman/listinfo/ros-dev