[ros-dev] ARM Guy Broke BOOT on Real Hardware

[Alex Ionescu]

On 28-Jul-08, at 8:23 AM, tamlin at algonet.se wrote:

> Alex wrote:
>
>> It looks like a guarded mutex is being acquired at DPC level. That's
>> pretty bad.
>
> Well, at least it's redundant and useless. :-)

Correct.

>
>
>> Pushlocks shouldn't be acquired at DPC level either
>
> Indeed. If "you" are at DPC level, you have but one synchronization to
> care about - SMP (i.e. CPU<->CPU).

Half-correct.

The bigger issue is that you can't do a wait at DPC level, because the  
dispatcher itself runs at DPC level, so you'll basically deadlock by  
yielding because nobody will run to schedule you somewhere else.

>
>
>> , but there's no
>> ASSERTs in the pushlock code that check for that.
>
> An oversight that should be fixed, I'm sure (even if it happened to
> slow down the system by a whole order of magnitude, which it won't).


Half-correct.

Pushlocks are meant to be inlined, so the ASSERT there is probably a  
bad idea -- it could be added in the contended case, but that one  
isn't hit often.

>
>
>> MMProbeAndLockPages should never be called for paged pool addreses
>> while at DPC level,
>
> Should MmProbeAndLock ever be called at IRQL above APC level? I mean,
> what if the probing fails? Should the paging executive somehow preempt
> this call and page in the missing pages (or should it simply throw  a
> "page not present" exception - not too great to do at DPC level I'd
> say)?

Incorrect. If at DPC level, MmProbeAndLockPages graciously fails,  
without raising an exception.

However, the behavior you describe about "page in the missing pages"  
is what does happen, for paged-out pages (but of course, this  
shouldn't happen while at DPC level).

>
>
>> which means the driver probably called it for a
>> non-paged pool address.
>
> If my previous suspicion is correct, I build on that and suspect this
> is an error in itself. The ProbeAndLock call should be done at APC
> level (though I may be completely wrong, but do read my finishing
> line).

Incorrect...

>
>
>> In that case, the whole loop about checking if the page is present
> and
>> then faulting it in is irrelevant, and won't happen.
>
> Right. Page fault at DPC level == Bad Move(tm).

Correct.

Don't forget that MmProbeAndLockPages also *Locks* pages.

The intended behavior of using this call at DPC level is to do just  
that -- otherwise, another driver could de-allocate your allocation.  
So it's still a very useful call, just don't make it page stuff in.

Arty's fix is a step in the right direction, but not fully correct...  
now that I can finally build ReactOS thanks to Colin, I'll give a  
bugfix a spin.

>
>
> -- 
> Mike
> _______________________________________________
> Ros-dev mailing list
> Ros-dev at reactos.org
> http://www.reactos.org/mailman/listinfo/ros-dev

Best regards,
Alex Ionescu