Good point: Remember, we’re off in the weeds here and not talking about handling a power management IRP on the Disk stack at this point, right? Because, as SNoone so aptly pointed out, you CAN’T post that request to a worker thread in the first place. THAT violates the contract. And, you couldn’t block AT ALL if you were called at IRQL DISPATCH_LEVEL.
With those caveats, and given that we’re talking “in general” about request processing:
You’re absolutely correct: It’s not usually a good practice to block an arbitrary thread. In fact, it’s incredibly BAD practice.
On the other hand, there ARE cases where blocking in the dispatch routine while passing a request to a lower driver is OK. Consider, for example, how PnP requests are routinely processed (and IoForwardIrpSynchronously). We know we’re being called in the context of the system process at that point.
So, yes. Blocking in a dispatch routine is generally a bad idea. I’m not recommending it, for sure. Rather, I’m just striving trying to help folks understand how I/O completion works.
Thanks for pointing that out,
Peter
OSR
Adding to what’s said:
Any driver blocking in a power-up IRP dispatch routine is extremely wrong. If that driver (by virtue of PCIe slot location) is called ahead of the paging HBA port, this can cause a deadlock.
wrote in message news:xxxxx@ntdev…
> Ouch, Mr. Chappell… a bit touchy, are we?
No more than you look to be. I have looked around these newsgroups to gauge
what passes as appropriate tone, and paid special attention to your posts.
But I’ll try harder not to bridle a bit at being told I’m only sort-of
correct.
> Do you think I deliberately misinterpreted your post?
No, where do you get that from?
> In fact, the context in the “paragraph immediately before” includes the
> following sentence:
>
>
>
> (capitals of MUST mine)
>
> This is precisely the context I was using to interpret the follow-on
> paragraph,
> and what I was trying to clarify. You say “you must have your dispatch
> routine
> mark the IRP as pending”… but in fact there’s another alternative: Your
> dispatch routine could BLOCK. Your post did not account for this
> alternative.
But it’s not an alternative, let alone an unaccounted one. The only sensible
interpretation of “later” is “after your dispatch routine returns”, else in
what sense do you “hold on to the IRP”. If your dispatch routine blocks
until another thread has passed the IRP down and (presumably) has seen
IoCallDriver return, then there has been no holding on to the IRP. The IRP
is not passed down later. You’re still in your dispatch routine. When you
unblock, you’ll return from your dispatch routine. Someone beneath you may
have held on to the IRP, but you didn’t. What you have is just a special
case of passing an IRP down in the dispatch routine. That you call the lower
device’s dispatch routine from a different thread is immaterial to the OP’s
question - which is why I agree with you that what you call an “artificial
distinction” is artificial but is why I say it’s an artifice of your reading
not my writing.
> That’s all I was seeking to clarify Mr. Chappell. Maybe YOU understand
> this
> detail. But your post certainly does not make this clear.
Fair enough, if it wasn’t clear, it wasn’t clear. But you appear to have
some particular ways of thinking of these IRP mechanics, making distinctions
where none matter, and I really don’t think I could reasonably have
anticipated what you might read into what I wrote. That I should find this
on a relatively simple matter is indeed not a good sign. And it really is
simple, isn’t it? Once you “pend” an IRP, when you next see that IRP,
however you get it, you mustn’t pass it down just by using
IoSkipCurrentIrpStackLocation and IoCallDriver. Do we at least agree on
that?
> And my gentle attempt to clarify the situation and flesh-out the various
> alternatives
> certainly does not warrant a sarcastic rebuke.
There was no sarcasm. But I must confess that I took your message as
ending with sarcasm, so who’s to know?
Geoff.
OK, Mr. Chappell… It’s clear that you’re intent on making a big impression in your debut posts to the list. You’ve certainly accomplished that.
And I’ve clarified the technical issue for the archives, so I’m done.
Peter
OSR
>this is getting a bit tedious. you are not actually answering the questions
being asked, you are just describing the problem over and over. what device
stack are you in? what ddk sample is leading you supposedly in the wrong
direction?
I am referring to src/usb/Bulkusb sample. Refer to HandleDeviceSetPower function in the file that deals with power. Search for function HoldIoRequests. The scene is almost same in my driver. He has marked the irp as pending. Then when at DISPATCH_LEVEL he schedules a work item. The dispatch routine returns status pending. The work item sends it down later. The sending down will have IoCopy… and IoSetCompletion… I think it is okay to do a IoCopy… and not IoSkip… while you have marked the irp as pending. But Peter might disagree. My concern is that doing IoSkip… on an irp that is marked as pending.
I will find some time later to reply with the other questions that are asked, if any.
But a question to Maxim, in the reply [post 3] you suggest to disagree with idea of marking irp as pending and then do IoSkip… before send it down. But in post 15, you say you are okay with that. Am I reading it in a wrong way? And clearly MSDN says otherwise. [against what you said in post 15]
Please correct my if I am wrong.
> used by the lower device. Once you alter the contents of your stack
location, you have no business calling IoSkipCurrentIrpStackLocation, no
matter where you call it from.
Please name the exact bad thing which can arise from this.
One more perfectly legal thing:
IoMarkIrpPending
IoCopyCurrentIrpStackLocationToNext
IoSetCompletionRoutine
IoCallDriver
return STATUS_PENDING
If the completion routine is empty - then this is the same as I provided above.
Do you want to say that it is prohibited to send the IRP to lower drivers after calling IoMarkIrpPending? it is not so.
–
Maxim S. Shatskih
Windows DDK MVP
xxxxx@storagecraft.com
http://www.storagecraft.com
> IoMarkIrpPending(…);
Irp->IoStatus.Status = STATUS_SUCCESS;
Irp->IoStatus.Information = (something);
IoCompleteRequest(Irp, 0);
return STATUS_SUCCESS;
And I’m sure you agree that THIS formulation is NOT correct.
This is OK, since the STATUS_SUCCESS return value is ignored and is never seen by the IO manager.
Note (VOID) cast on IoCallDriver.
–
Maxim S. Shatskih
Windows DDK MVP
xxxxx@storagecraft.com
http://www.storagecraft.com
> Yes, you are wrong. Even when it works, you’re still wrong architecturally, and opening yourself up for
trouble.
Note that, if you’re correct here, then it is impossible to pass the IRP down to a lower driver from a work item, which is surely not the case. I think at least PoCallDriver in the OS can do something similar.
–
Maxim S. Shatskih
Windows DDK MVP
xxxxx@storagecraft.com
http://www.storagecraft.com
> But a question to Maxim, in the reply [post 3] you suggest to disagree with idea of marking irp as
The rule is: if IoMarkIrpPending is ever called, then the IO manager must see STATUS_PENDING return from the topmost dispatch routine.
Usually, IoSkip is done in the following way:
IoSkip
return IoCallDriver
In this case, IoMarkIrpPending before IoSkip will cause issues - see Peter’s email and note the fact that IoCallDriver’s return value will be passed up to the IO manager.
But surely it is OK in lots of code to a) first pend the IRP to some queue b) then, when some conditions will be met in some other context and the IRP will be dequeued, pass it down as:
IoSkip
(VOID)IoCallDriver
In this case, the IO manager never sees the return value from IoCallDriver. It only sees the return value from the first driver (from the path which pended the IRP to the queue), which must be STATUS_PENDING.
–
Maxim S. Shatskih
Windows DDK MVP
xxxxx@storagecraft.com
http://www.storagecraft.com
“The rule is: if IoMarkIrpPending is ever called, then the IO manager must see
STATUS_PENDING return from the topmost dispatch routine.”
That’s not exactly correct. Add to that “unless the completion was held off by STATUS_MORE_PROCESSING_REQUIRED and a secondary IoCompleteRequest was issued after that from the original dispatch call stack”.
Consider the scenario when you pass the IRP down synchronously (and could have peen pended by any lower driver), and then you do IoCompleteRequest for it. The IRP is not considered pended anymore. It’s completed in the original context.
> Consider the scenario when you pass the IRP down synchronously
Yes, this is about this code snippet:
if( Irp->PendingReturned )
IoMarkIrpPending(Irp);
in the completion routine.
In your case, there must NOT be any such snippet in the CR used to synchronize against the lower driver’s execution.
–
Maxim S. Shatskih
Windows DDK MVP
xxxxx@storagecraft.com
http://www.storagecraft.com
“The rule is: if IoMarkIrpPending is ever called, then the IO manager must see
STATUS_PENDING return from the topmost dispatch routine.”
Let’s reformulate that. The I/O manager expectation is that STATUS_PENDING returned from the topmost dispatch routine MUST match SL_PENDING_RETURNED in the topmost IO_STACK_LOCATION only. Note that it’s not formally required for the IRPs built by the drivers for their internal needs, but a lower driver usually doesn’t know how the IRP was built.
To maintain that expectation, STATUS_PENDING on every level of dispatch call stack should match SL_PENDING_RETURNED in its IO_STACK_LOCATION. That means, if you propagate the IoCallDriver status up, you propagate SL_STATUS_PENDING up also.
But all this reasoning whether IoMarkIrpPending before IoSkipCurrentIrpStackLocation can be used, becomes irrelevant, because IoSkipCurrentIrpStackLocation documentation EXPLICITLY forbids that. EXPLICITLY.
> “Maxim S. Shatskih” wrote in message
> news:xxxxx@ntdev…
> Please name the exact bad thing which can arise from this.
I thought I did. If the “this” is your sequence IoMarkIrpPending,
IoSkipCurrentIrpStackLocation, IoCallDriver, then the lower device will be
called as if it had already called IoMarkIrpPending at the very start of its
dispatch routine. Well, how do you know that the lower device means ever to
call IoMarkIrpPending? Whether the lower device will want to mark the IRP as
pending is its decision, not yours.
> One more perfectly legal thing:
>
> IoMarkIrpPending
> IoCopyCurrentIrpStackLocationToNext
> IoSetCompletionRoutine
> IoCallDriver
> return STATUS_PENDING
>
> If the completion routine is empty - then this is the same as I provided
> above.
It is perfectly legal, yes, but it is not the same as IoMarkIrpPending,
IoSkipCurrentIrpStackLocation and IoCallDriver. In the legal sequence, your
device and the lower device access different stack locations. Yours is
marked as pending. The lower device’s is not. (See that
IoCopyCurrentIrpStackLocationToNext clears the Control member in the next
stack location.) In the illegal sequence, your device and the lower access
the same stack location, which you have marked as pending.
> Do you want to say that it is prohibited to send the IRP to lower drivers
> after calling IoMarkIrpPending? it is not so.
No, of course not. As I told the OP, if he wants to do this, he’ll have to
use the IoCopyCurrentIrpStackLocationToNext sequence, not the apparently
easier IoSkipCurrentIrpStackLocation sequence.
Geoff.
> wrote in message news:xxxxx@ntdev…
> OK, Mr. Chappell… It’s clear that you’re intent on making a big
> impression in your debut posts to the list.
Right, I planned this. (Yes, that’s sarcastic.) A simple matter of IRP flow
is obviously the thing someone writes about when they set out to make an
impression. Do you not think that maybe you had a small role in blowing it
up?
You say something is “SORT of correct”, without actually showing how it’s
not correct. You say it’s “misleading” even though to be misled you’ve read
into the text some significance from who knows where and you present as
important some “unaccounted alternative” that’s actually just a needless
complication. And then you ascribe to the other person an intention to make
a big impression!
I don’t know about you - or wouldn’t have until now - but if I’m about to
use words like incorrect and misleading, then I recognise it’s my burden to
make sure the supposed mistake cannot just be my misinterpretation. Are you
so far from that that you find it easier to discern some intention of making
a big impression rather than see that people quite reasonably don’t like
being told they’re incorrect or misleading unless they truly are? If you
want to shift the responsibility on to others for getting “touchy” about
being corrected, then take more care to be unarguably correct when you
correct someone.
> And I’ve clarified the technical issue for the archives, so I’m done.
It’s all OK, then.
Geoff.
“Geoff Chappell” wrote in message news:xxxxx@ntdev…
I thought I did. If the “this” is your sequence IoMarkIrpPending,
IoSkipCurrentIrpStackLocation, IoCallDriver, then the lower device will be
called as if it had already called IoMarkIrpPending at the very start of
its dispatch routine. Well, how do you know that the lower device means
ever to call IoMarkIrpPending? Whether the lower device will want to mark
the IRP as pending is its decision, not yours.
As seen in the example Max provided, he isn’t arguing that this will work:
IoMarkIrpPending();
IoSkipCurrentIrpStackLocation();
return IoCallDriver();
We all agree that’s broken given the reason you cited. Instead, Max’s
contention is that *this* will work:
IoMarkIrpPending();
IoSkipCurrentIrpStackLocation();
(VOID)IoCallDriver();
return STATUS_PENDING;
Which takes more thinking to wrap your head around, but it does indeed work
given the current implementation. However, it doesn’t really matter given
that the docs indicate that it is also illegal.
-scott
–
Scott Noone
Consulting Associate and Chief System Problem Analyst
OSR Open Systems Resources, Inc.
http://www.osronline.com
Hope to see you at the next OSR kernel debugging class February 14th in
Columbia, MD!