[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, <jgardou at svn.reactos.org> 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?rev=66334&r1=66333&r2=66334&view=diff
>>>
>>> ==============================================================================
>>> --- 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
>> Ros-dev at reactos.org
>> http://www.reactos.org/mailman/listinfo/ros-dev
>>
> _______________________________________________
> Ros-dev mailing list
> Ros-dev at reactos.org
> http://www.reactos.org/mailman/listinfo/ros-dev
>


-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 3683 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://www.reactos.org/pipermail/ros-dev/attachments/20150220/d7361b60/attachment.bin>