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