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 Mobile<https://aka.ms/blhgte>
From: Ros-dev on behalf of Alex Ionescu
Sent: Sunday, May 10, 2020 12:37:09 PM
To: ReactOS Development List ; Thomas Faber
Cc: Linda Wang
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 wrote:
>
> https://git.reactos.org/?p=reactos.git;a=commitdiff;h=25a5aee86f43e9f526d4d360e4e4f35feb9d22b4
>
> commit 25a5aee86f43e9f526d4d360e4e4f35feb9d22b4
> Author: Thomas Faber
> AuthorDate: Sat Feb 22 12:31:54 2020 +0100
> Commit: Thomas Faber
> 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(, FALSE);
>
> -/* Set the zero page event */
> -KeInitializeEvent(, SynchronizationEvent, FALSE);
> -MmZeroingPageThreadActive = FALSE;
> +/* Set up the zero page event */
> +KeInitializeEvent(, NotificationEvent, FALSE);
>
> /* Initialize the dead stack S-LIST */
> InitializeSListHead();
> 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(, 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();
> MiReleasePfnLock(OldIrql);
> break;
> }
>
___
Ros-dev mailing list
Ros-dev@reactos.org
http://reactos.org/mailman/listinfo/ros-dev
___
Ros-dev mailing list
Ros-dev@reactos.org
http://reactos.org/mailman/listinfo/ros-dev
