Oney's Book, HandleQueryStop(): Why is there no race condition?

Hello,

I erroneously posted this to microsoft.public.win32.programmer.kernel, I
thought I were in this group here, so please excuse if you get multiple
copies of this.

In Walter Oney's book, chapter 6, Walter shows a HandleQueryStop()
function which can be used by the PNP management stuff
(IRP_MN_QUERY_STOP_DEVICE). This can be found on page 323 (2nd edition)
or 241 (1st edition).

I hope I can give the code here:

(1) if (pdx->state != WORKING)
(2) return DefaultPnpHandler(fdo, Irp);
(3) if (!OkayToStop(pdx))
(4) return CompleteRequest(Irq, STATUS_UNSUCCESSFUL, 0);
(5) StallRequests(&pqx->dqReadWrite);
(6) WaitForCurrentIrp(&pdx->dqReadWrite);
(7) pdx->state = PENDINGSTOP;
(8) return DefaultPnpHandler(fdo, Irq);

I ask myself why (5) does not need to be issued before (3), and undone
if (4) is executed? An OkayToStop() function might return an Ok which is
not true anymore when 5 is executed, can't it?

I think about a scenario with two CPUs where one CPU has justed picked
an IRP to execute at the point of time when (3) is executed on the other
CPU. Before (5) can be executed, the 2nd CPU has begun executing the IRP
which results in putting the driver in a state where it cannot be
stopped afterwards (for example, a REWIND operation for a tape), where
another IRP has to be executed to undo this effect.

Am I missing something important here, or this this a bug in the code?

Kind regards,
Spiro.

--
Spiro R. Trikaliotis
http://www.trikaliotis.net/

Walter
Tue May 04 15:15:35 CDT 2004

Spiro Trikaliotis wrote:
> In Walter Oney's book, chapter 6, Walter shows a HandleQueryStop()
> function which can be used by the PNP management stuff
> (IRP_MN_QUERY_STOP_DEVICE). This can be found on page 323 (2nd edition)
> or 241 (1st edition).
>
> I hope I can give the code here:
>
> (1) if (pdx->state != WORKING)
> (2) return DefaultPnpHandler(fdo, Irp);
> (3) if (!OkayToStop(pdx))
> (4) return CompleteRequest(Irq, STATUS_UNSUCCESSFUL, 0);
> (5) StallRequests(&pqx->dqReadWrite);
> (6) WaitForCurrentIrp(&pdx->dqReadWrite);
> (7) pdx->state = PENDINGSTOP;
> (8) return DefaultPnpHandler(fdo, Irq);
>
> I ask myself why (5) does not need to be issued before (3), and undone
> if (4) is executed? An OkayToStop() function might return an Ok which is
> not true anymore when 5 is executed, can't it?

The way the DEVQUEUE code works, WaitForCurrentIrp actually waits for
someone to call StartNextPacket. If the queue were not already stalled,
StartNextPacket would then start a new IRP, which is not what we want
here.

--
Walter Oney, Consulting and Training
Basic and Advanced Driver Programming Seminars
Check out our schedule at http://www.oneysoft.com

Spiro
Wed May 05 13:18:32 CDT 2004

Hello Walter,

either you did not understand my question, or I did not understand your
answer. ;-)

Walter Oney <waltoney@oneysoft.com> schrieb:
> Spiro Trikaliotis wrote:
>> In Walter Oney's book, chapter 6, Walter shows a HandleQueryStop()
>> function which can be used by the PNP management stuff
>> (IRP_MN_QUERY_STOP_DEVICE). This can be found on page 323 (2nd edition)
>> or 241 (1st edition).
>>
>> I hope I can give the code here:
>>
>> (1) if (pdx->state != WORKING)
>> (2) return DefaultPnpHandler(fdo, Irp);
>> (3) if (!OkayToStop(pdx))
>> (4) return CompleteRequest(Irq, STATUS_UNSUCCESSFUL, 0);
>> (5) StallRequests(&pdx->dqReadWrite);
>> (6) WaitForCurrentIrp(&pdx->dqReadWrite);
>> (7) pdx->state = PENDINGSTOP;
>> (8) return DefaultPnpHandler(fdo, Irq);
>>
>> I ask myself why (5) does not need to be issued before (3), and undone
>> if (4) is executed? An OkayToStop() function might return an Ok which is
>> not true anymore when 5 is executed, can't it?
>
> The way the DEVQUEUE code works, WaitForCurrentIrp actually waits for
> someone to call StartNextPacket. If the queue were not already stalled,
> StartNextPacket would then start a new IRP, which is not what we want
> here.

I wanted to put (5) before (3), that is:

(1) if (pdx->state != WORKING)
(2) return DefaultPnpHandler(fdo, Irp);
(5) StallRequests(&pdx->dqReadWrite); // <--- moved here
(6) WaitForCurrentIrp(&pdx->dqReadWrite); // <--- moved here
(3) if (!OkayToStop(pdx)) {
RestartRequest(&pdx->dqReadWrite, fdo); <--- new!
(4) return CompleteRequest(Irq, STATUS_UNSUCCESSFUL, 0);
}
/* (5) and (6) are missing here */
(7) pdx->state = PENDINGSTOP;
(8) return DefaultPnpHandler(fdo, Irq);

That is, I stall the queue *before* I check if it is OkayToStop(). I
have the impression that an IRP being executed on another processor
could do something that would result in OkayToStop() to return false
instead of true, but since OkayToStop() does not now that this is
executed, it might falsely return true.

Think about an IRP which changes a hardware register in such a way that
the device is not allowed to be stopped until another IRP is executed.

Kind regards,
Spiro.

--
Spiro R. Trikaliotis
http://www.trikaliotis.net/

Walter
Wed May 05 16:00:47 CDT 2004

Spiro Trikaliotis wrote:
> Think about an IRP which changes a hardware register in such a way that
> the device is not allowed to be stopped until another IRP is executed.

Yeah, I guess I see your point.

--
Walter Oney, Consulting and Training
Basic and Advanced Driver Programming Seminars
Check out our schedule at http://www.oneysoft.com

Spiro
Thu May 06 04:29:58 CDT 2004

Hello Walter,

Walter Oney <waltoney@oneysoft.com> schrieb:

> Yeah, I guess I see your point.

Well... Sorry to bother, but: What is your conclusion? Is there a race
condition, or am I wrong?

Kind regards,
Spiro.

--
Spiro R. Trikaliotis
http://www.trikaliotis.net/

Walter
Thu May 06 05:13:24 CDT 2004

Spiro Trikaliotis wrote:
> Well... Sorry to bother, but: What is your conclusion? Is there a race
> condition, or am I wrong?

I don't see how to solve the problem of StartNextPacket starting an IRP
that will put the device into the non-stoppable state you've posited
without doing something like you've suggested.

--
Walter Oney, Consulting and Training
Basic and Advanced Driver Programming Seminars
Check out our schedule at http://www.oneysoft.com

Spiro
Thu May 06 05:16:09 CDT 2004

Hello Walter,

Walter Oney <waltoney@oneysoft.com> wrote:

> I don't see how to solve the problem of StartNextPacket starting an
> IRP that will put the device into the non-stoppable state you've
> posited without doing something like you've suggested.

Ok, thank you for your help!

Kind regards,
Spiro.

--
Spiro R. Trikaliotis
http://www.trikaliotis.net/

Bill
Thu May 06 11:14:30 CDT 2004

Good catch! That is fairly subtle.

--
Bill McKenzie
Software Engineer - Prism 802.11 Wireless Solutions
Conexant Systems, Inc.


"Spiro Trikaliotis" <nntp+200405@trikaliotis.net> wrote in message
news:slrnc9k439.1mj.nntp+200405@news.trikaliotis.net...
> Hello Walter,
>
> Walter Oney <waltoney@oneysoft.com> wrote:
>
> > I don't see how to solve the problem of StartNextPacket starting an
> > IRP that will put the device into the non-stoppable state you've
> > posited without doing something like you've suggested.
>
> Ok, thank you for your help!
>
> Kind regards,
> Spiro.
>
> --
> Spiro R. Trikaliotis
> http://www.trikaliotis.net/

Pavel
Fri May 07 13:53:05 CDT 2004

Sorry for jumping in...
but what exactly means "non-stoppable state"?
If there is need to stop the device, the driver should be
able to stop it from any state?

Regards,
--PA


"Bill McKenzie" <bill.mckenzie@nospam.conexant.com> wrote in message
news:OWFNKV4MEHA.3572@tk2msftngp13.phx.gbl...
> Good catch! That is fairly subtle.
>
> --
> Bill McKenzie
> Software Engineer - Prism 802.11 Wireless Solutions
> Conexant Systems, Inc.
>
>
> "Spiro Trikaliotis" <nntp+200405@trikaliotis.net> wrote in message
> news:slrnc9k439.1mj.nntp+200405@news.trikaliotis.net...
>> Hello Walter,
>>
>> Walter Oney <waltoney@oneysoft.com> wrote:
>>
>> > I don't see how to solve the problem of StartNextPacket starting an
>> > IRP that will put the device into the non-stoppable state you've
>> > posited without doing something like you've suggested.
>>
>> Ok, thank you for your help!
>>
>> Kind regards,
>> Spiro.
>>
>> --
>> Spiro R. Trikaliotis
>> http://www.trikaliotis.net/
>
>

Bill
Fri May 07 23:14:56 CDT 2004

> If there is need to stop the device, the driver should be
> able to stop it from any state?

Not true or there would be no reason to allow for the rejection of a stop
through the query. Example: the best I can think of at the moment is a data
collection or frame grabber type device in an industrial setting which does
not wish to allow stops while (lengthy) data transfers are in progress.
This would seem a perfectly valid instance, albeit somewhat rare, in which
to reject a stop query.

There are probably plenty of industrial devices which take a significant
amount of time to stop once set in motion, although a really good example
escapes me at the moment.

--
Bill McKenzie
Software Engineer - Prism 802.11 Wireless Solutions
Conexant Systems, Inc.


"Pavel A." <pavel_a@geeklife.com> wrote in message
news:O0FK2RGNEHA.2736@TK2MSFTNGP11.phx.gbl...
> Sorry for jumping in...
> but what exactly means "non-stoppable state"?
> If there is need to stop the device, the driver should be
> able to stop it from any state?
>
> Regards,
> --PA
>
>
> "Bill McKenzie" <bill.mckenzie@nospam.conexant.com> wrote in message
> news:OWFNKV4MEHA.3572@tk2msftngp13.phx.gbl...
> > Good catch! That is fairly subtle.
> >
> > --
> > Bill McKenzie
> > Software Engineer - Prism 802.11 Wireless Solutions
> > Conexant Systems, Inc.
> >
> >
> > "Spiro Trikaliotis" <nntp+200405@trikaliotis.net> wrote in message
> > news:slrnc9k439.1mj.nntp+200405@news.trikaliotis.net...
> >> Hello Walter,
> >>
> >> Walter Oney <waltoney@oneysoft.com> wrote:
> >>
> >> > I don't see how to solve the problem of StartNextPacket starting an
> >> > IRP that will put the device into the non-stoppable state you've
> >> > posited without doing something like you've suggested.
> >>
> >> Ok, thank you for your help!
> >>
> >> Kind regards,
> >> Spiro.
> >>
> >> --
> >> Spiro R. Trikaliotis
> >> http://www.trikaliotis.net/
> >
> >
>
>

Spiro
Sat May 08 01:43:06 CDT 2004

Hello,

Bill McKenzie <bill.mckenzie@nospam.conexant.com> schrieb:

> There are probably plenty of industrial devices which take a significant
> amount of time to stop once set in motion, although a really good example
> escapes me at the moment.

Once I read the following example somewhere, but I can't remember where:

You have a tape drive which you handle with IRPs, one to start
rewinding, and one to stop it. The old QIC40/80 tape drives are good
examples. If the rewind is started, the computer has to make sure the
rewind is stopped; if you don't, the tape might rip.

So, issuing the start rewind IRP, your device is not allowed to stop.
So, this IRP might come in race with the IsOkayToStop() call, because it
might have been issued on another processor.

Walter tells us the example of a driver for a storage device which holds
the pagefile, this is not allowed to be stopped. I don't know how a
driver is informed that it holds the pagefile. If this is done via an
IRP, there might be a race condition for this, too.

Kind regards,
Spiro.

--
Spiro R. Trikaliotis
http://www.trikaliotis.net/

Maxim
Sat May 08 08:10:56 CDT 2004

> examples. If the rewind is started, the computer has to make sure the
> rewind is stopped; if you don't, the tape might rip.

Look at the code in the provided CdRom/ClassPnP source which shuts the CDAudio
music off on powering down. A similar thing.

--
Maxim Shatskih, Windows DDK MVP
StorageCraft Corporation
maxim@storagecraft.com
http://www.storagecraft.com

Spiro
Sun May 09 02:48:46 CDT 2004

Hello Maxim,

Maxim S. Shatskih <maxim@storagecraft.com> schrieb:

>> examples. If the rewind is started, the computer has to make sure the
>> rewind is stopped; if you don't, the tape might rip.
>
> Look at the code in the provided CdRom/ClassPnP source which shuts the CDAudio
> music off on powering down. A similar thing.

where can I find this source? I haved checked twice, but could not find
it.

Anyway, I think this is a different issue (without looking at the code),
since a CDAudio music does not harm if anything fails, but a ripped tape
is a problem. Furthermore, if the system is powered down, the CD will
stop, too. ;-)

Kind regards,
Spiro.

--
Spiro R. Trikaliotis
http://www.trikaliotis.net/

Maxim
Sun May 09 09:02:31 CDT 2004

> > Look at the code in the provided CdRom/ClassPnP source which shuts the
CDAudio
> > music off on powering down. A similar thing.
> > where can I find this source? I haved checked twice, but could not find

DDK\SRC\STORAGE

--
Maxim Shatskih, Windows DDK MVP
StorageCraft Corporation
maxim@storagecraft.com
http://www.storagecraft.com

Mark
Sun May 09 10:41:57 CDT 2004

Over on the storage side of things, those of us with multipath storage
drivers (two paths to one disk) have to deal with this race condition on a
regular basis. We want to allow Q-remove or Q-stop on one path if another
path is available, but we want to fail the query if there is no other
available path. As 'available path' is an asynchronous state(1), managing
this is a bit tricky. The simplest approach is to just deny the query.
Unfortunately this results in the hideous 'reboot required' popup anytime a
new device instance enters the system. Other less conservative approaches
allow the query but leave the system vulnerable as the race condition cannot
be eliminated.

(1) for example somebody pulls a hotplug disk out of its slot.
--

=====================
Mark Roddy
Windows 2003/XP/2000 Consulting
Hollis Technology Solutions 603-321-1032
www.hollistech.com
markr@hollistech.com


"Bill McKenzie" <bill.mckenzie@nospam.conexant.com> wrote in message
news:eIIHcMLNEHA.556@tk2msftngp13.phx.gbl...
> > If there is need to stop the device, the driver should be
> > able to stop it from any state?
>
> Not true or there would be no reason to allow for the rejection of a stop
> through the query. Example: the best I can think of at the moment is a
data
> collection or frame grabber type device in an industrial setting which
does
> not wish to allow stops while (lengthy) data transfers are in progress.
> This would seem a perfectly valid instance, albeit somewhat rare, in which
> to reject a stop query.
>
> There are probably plenty of industrial devices which take a significant
> amount of time to stop once set in motion, although a really good example
> escapes me at the moment.
>
> --
> Bill McKenzie
> Software Engineer - Prism 802.11 Wireless Solutions
> Conexant Systems, Inc.
>
>
> "Pavel A." <pavel_a@geeklife.com> wrote in message
> news:O0FK2RGNEHA.2736@TK2MSFTNGP11.phx.gbl...
> > Sorry for jumping in...
> > but what exactly means "non-stoppable state"?
> > If there is need to stop the device, the driver should be
> > able to stop it from any state?
> >
> > Regards,
> > --PA
> >
> >
> > "Bill McKenzie" <bill.mckenzie@nospam.conexant.com> wrote in message
> > news:OWFNKV4MEHA.3572@tk2msftngp13.phx.gbl...
> > > Good catch! That is fairly subtle.
> > >
> > > --
> > > Bill McKenzie
> > > Software Engineer - Prism 802.11 Wireless Solutions
> > > Conexant Systems, Inc.
> > >
> > >
> > > "Spiro Trikaliotis" <nntp+200405@trikaliotis.net> wrote in message
> > > news:slrnc9k439.1mj.nntp+200405@news.trikaliotis.net...
> > >> Hello Walter,
> > >>
> > >> Walter Oney <waltoney@oneysoft.com> wrote:
> > >>
> > >> > I don't see how to solve the problem of StartNextPacket starting an
> > >> > IRP that will put the device into the non-stoppable state you've
> > >> > posited without doing something like you've suggested.
> > >>
> > >> Ok, thank you for your help!
> > >>
> > >> Kind regards,
> > >> Spiro.
> > >>
> > >> --
> > >> Spiro R. Trikaliotis
> > >> http://www.trikaliotis.net/
> > >
> > >
> >
> >
>
>

Spiro
Thu May 13 05:46:16 CDT 2004

Hello Maxim,

Maxim S. Shatskih <maxim@storagecraft.com> schrieb:

> DDK\SRC\STORAGE

ah... yes, of course. I did not even think about the DDK, I searched in
Oney's book only. ;-)

Thank you,
Spiro.

--
Spiro R. Trikaliotis
http://www.trikaliotis.net/