Re: [ros-dev] [ros-diffs] [jgardou] 66334: [NTOSKRNL/MM] - MiIsEntireRangeCommitted: Ensure the PTE we are checking is really faulted in. - Prefer MiPteToPde and MiPdeToPte (which should really be called MiFirstPteInPde) in...

[Timo Kreuzer]

Why does the PDE need to be faulted in? The function is called 
MiIsEntireRangeCommitted, not 
MiCheckIfEntireRangeIsCommittedAndMakePdeValid.
I also wonder why one would even walk the PTEs if the VAD is MEM_COMMIT.

Am 19.02.2015 um 11:19 schrieb Jérôme Gardou:
> If it takes me 6 months to understand why, then yes.
>
> Hint: you can make this delay shorter.
>
> Also the title is a bit misleading, it's the PDE that is not faulted in
> in case we don't terminate the loop iteration early.
>
> Le 19/02/2015 05:46, Alex Ionescu a écrit :
> > it might fix an assert but the patch is incorrect. will this also take 6
> > months to revert?
> >
> > Best regards,
> > Alex Ionescu
> >
> > On Tue, Feb 17, 2015 at 6:19 AM, [e-mail] wrote:
> > > Author: jgardou
> > > Date: Tue Feb 17 14:19:05 2015
> > > New Revision: 66334
> > >
> > > URL: http://svn.reactos.org/svn/reactos?rev=66334&view=rev
> > > Log:
> > > [NTOSKRNL/MM]
> > > MiIsEntireRangeCommitted: Ensure the PTE we are checking is really
> > >
> > > faulted in.
> > > Prefer MiPteToPde and MiPdeToPte (which should really be called
> > >
> > > MiFirstPteInPde) instead of MiAddressToPte and MiPteToAddress
> > > Fixes weird failed ASSERT in page fault handler when using DPH.
> > >
> > > Modified:
> > >      trunk/reactos/ntoskrnl/mm/ARM3/virtual.c
> > >
> > > Modified: trunk/reactos/ntoskrnl/mm/ARM3/virtual.c
> > > URL:
> > > http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/virtual.c?...
> > >
> > > ==============================================================================
> > > --- trunk/reactos/ntoskrnl/mm/ARM3/virtual.c    [iso-8859-1] (original)
> > > +++ trunk/reactos/ntoskrnl/mm/ARM3/virtual.c    [iso-8859-1] Tue Feb 17
> > > 14:19:05 2015
> > > @@ -1994,14 +1994,13 @@
> > >           if (OnBoundary)
> > >           {
> > >               /* Is this PDE demand zero? */
> > >        PointerPde = MiAddressToPte(PointerPte);
> > >        PointerPde = MiPteToPde(PointerPte);
> > >         if (PointerPde->u.Long != 0)
> > >         {
> > >             /* It isn't -- is it valid? */
> > >             if (PointerPde->u.Hard.Valid == 0)
> > >             {
> > >                 /* Nope, fault it in */
> > >                PointerPte = MiPteToAddress(PointerPde);
> > >                 MiMakeSystemAddressValid(PointerPte, Process);
> > >             }
> > >         }
> > >
> > > @@ -2009,13 +2008,13 @@
> > >               {
> > >                   /* The PTE was already valid, so move to the next one */
> > >                   PointerPde++;
> > >            PointerPte = MiPteToAddress(PointerPde);
> > >            PointerPte = MiPdeToPte(PointerPde);
> > >
> > >             /* Is the entire VAD committed? If not, fail */
> > >             if (!Vad->u.VadFlags.MemCommit) return FALSE;
> > >
> > >            /* Everything is committed so far past the range, return
> > >
> > > true */
> > >            if (PointerPte > LastPte) return TRUE;
> > >            /* New loop iteration with our new, on-boundary PTE. */
> > >            continue;
> > >         }
> > >     }
> >
> > Ros-dev mailing list
> > [e-mail]
> > http://www.reactos.org/mailman/listinfo/ros-dev
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev