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