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

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. :-)

>  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).

>  , 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).

>  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)?

>  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).

>  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).

-- 
Mike

[Alex Ionescu]

On 28-Jul-08, at 8:23 AM, tamlin(a)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(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev 
Best regards,
Alex Ionescu