Newbie question about correct handling for pending requests

Hi again!

After digging through all the documentation and a few samples I’m still not sure if my queue/request handling is correct or
maybe has a flaw that will show up sooner or later…

In my KMDF driver I currently have one (sequential) queue for read and write requests and one manual
queue for pending read requests. (Writes will always succeed!)
All synchronization and execution levels are left at their default values.

Now my question:

In my read event handler I simply forward the request (if I cannot fulfill it immediately) like this:

status = WdfRequestForwardToIoQueue(Request, pDevContext->PendingReadQueue);

In my write handler - after having written to my memory I’m looping over the pending request queue like this:

// Check for pending reads… and move them to main queue
do
{
WDFREQUEST request;

status = WdfIoQueueRetrieveNextRequest(pDevContext->PendingReadQueue, &request);

// Found an entry?
if (NT_SUCCESS(status))
{
status = WdfRequestForwardToIoQueue(request, pDevContext->ReadQueue);
if (!NT_SUCCESS(status))
{
WdfRequestCompleteWithInformation(request, STATUS_UNSUCCESSFUL, 0);
return;
}
}
}
while (NT_SUCCESS(status));

Now my question: Although this works fine so far I’d like to ensure that this is a correct approach or if
someone sees future problems with this code?

You have a race between enqueue and dequeue in your read and write callbacks. Put both actions under a spinlock

d


From: xxxxx@frank-wolf.orgmailto:xxxxx
Sent: ?1/?23/?2013 7:24 AM
To: Windows System Software Devs Interest Listmailto:xxxxx
Subject: [ntdev] Newbie question about correct handling for pending requests

Hi again!

After digging through all the documentation and a few samples I’m still not sure if my queue/request handling is correct or
maybe has a flaw that will show up sooner or later…

In my KMDF driver I currently have one (sequential) queue for read and write requests and one manual
queue for pending read requests. (Writes will always succeed!)
All synchronization and execution levels are left at their default values.

Now my question:

In my read event handler I simply forward the request (if I cannot fulfill it immediately) like this:

status = WdfRequestForwardToIoQueue(Request, pDevContext->PendingReadQueue);

In my write handler - after having written to my memory I’m looping over the pending request queue like this:

// Check for pending reads… and move them to main queue
do
{
WDFREQUEST request;

status = WdfIoQueueRetrieveNextRequest(pDevContext->PendingReadQueue, &request);

// Found an entry?
if (NT_SUCCESS(status))
{
status = WdfRequestForwardToIoQueue(request, pDevContext->ReadQueue);
if (!NT_SUCCESS(status))
{
WdfRequestCompleteWithInformation(request, STATUS_UNSUCCESSFUL, 0);
return;
}
}
}
while (NT_SUCCESS(status));

Now my question: Although this works fine so far I’d like to ensure that this is a correct approach or if
someone sees future problems with this code?


NTDEV is sponsored by OSR

OSR is HIRING!! See http://www.osr.com/careers

For our schedule of WDF, WDM, debugging and other seminars visit:
http://www.osr.com/seminars

To unsubscribe, visit the List Server section of OSR Online at http://www.osronline.com/page.cfm?name=ListServer</mailto:xxxxx></mailto:xxxxx>

Hello Doron,
thanks for your answer. However I wonder at what time the race can happen:

The queue (though it’s called ReadQueue) is configured to handle
read and write requests with sequential dispatching. So it should never happen?

Do I understand right that KMDF queues are not thread safe (unlike the
Message queue in user mode land)?

I didn’t see that reads and writes were processed on the same sequential queue. In that case you are fine

d


From: xxxxx@frank-wolf.orgmailto:xxxxx
Sent: ?1/?23/?2013 8:59 AM
To: Windows System Software Devs Interest Listmailto:xxxxx
Subject: RE:[ntdev] Newbie question about correct handling for pending requests

Hello Doron,
thanks for your answer. However I wonder at what time the race can happen:

The queue (though it’s called ReadQueue) is configured to handle
read and write requests with sequential dispatching. So it should never happen?

Do I understand right that KMDF queues are not thread safe (unlike the
Message queue in user mode land)?


NTDEV is sponsored by OSR

OSR is HIRING!! See http://www.osr.com/careers

For our schedule of WDF, WDM, debugging and other seminars visit:
http://www.osr.com/seminars

To unsubscribe, visit the List Server section of OSR Online at http://www.osronline.com/page.cfm?name=ListServer</mailto:xxxxx></mailto:xxxxx>

Great!
Just in case I’m going to separate the read and write queues at a later time.

Would it also be possible to avoid the race by just giving the manual queue
a certain Synchronizationlevel at creation or do I have to use my own Spinlock?

The theoretical sync level would have to be at the call sites changing the queue (ie forwarding into it, retrieving from it). That is why the same sequential queue doing both operations is safe. The sync level on the queue only helps dictate the queue’s callback synchronization.

-----Original Message-----
From: xxxxx@lists.osr.com [mailto:xxxxx@lists.osr.com] On Behalf Of xxxxx@frank-wolf.org
Sent: Wednesday, January 23, 2013 9:39 AM
To: Windows System Software Devs Interest List
Subject: RE:[ntdev] Newbie question about correct handling for pending requests

Great!
Just in case I’m going to separate the read and write queues at a later time.

Would it also be possible to avoid the race by just giving the manual queue a certain Synchronizationlevel at creation or do I have to use my own Spinlock?


NTDEV is sponsored by OSR

OSR is HIRING!! See http://www.osr.com/careers

For our schedule of WDF, WDM, debugging and other seminars visit:
http://www.osr.com/seminars

To unsubscribe, visit the List Server section of OSR Online at http://www.osronline.com/page.cfm?name=ListServer

xxxxx@frank-wolf.org wrote:

Hello Doron,
thanks for your answer. However I wonder at what time the race can happen:

The queue (though it’s called ReadQueue) is configured to handle
read and write requests with sequential dispatching. So it should never happen?

So, you have reads and writes going into a single sequential queue.
Writes apparently supply data that will be read by the reads. If there
is no data available, reads go into a manual queue. When a write
happens, they succeed, and you pump ALL of the manual read requests back
into the sequential queue, where they might get moved over yet again.
Is that it?

Queue manipulations are pretty speedy, but if it were me, I’d skip that
“copy back” step. Just have the write handler pop a request off the
manual queue, process the read, and complete it, until you are out of data.

On the other hand, I don’t think there’s anything fundamentally wrong
with your approach now.


Tim Roberts, xxxxx@probo.com
Providenza & Boekelheide, Inc.

> Is that it?

Yes!

Queue manipulations are pretty speedy, but if it were me, I’d skip that “copy back” step.
Just have the write handler pop a request off the manual queue, process the read,
and complete it, until you are out of data.

Very good point and I was indeed thinking about that optimization step once the driver
works as intended; i.e. has all features and is stable for in-house testing.

To get the job done I was just going the safe route at first! :slight_smile:

However one question comes to my mind:

I’m popping requests of the manual queue in a loop until no more requests are there.
What happens if I have to push a request pack into the queue? Do I have an endless loop?
In case I have: Is there an easy KMDF way to solve that problem?

Yup.

Well, “don’t do it” comes to mind (sorry). It’s not a very good design.

I guess you could save the handle before you insert the request, and after you remove an entry from the last compare it to that of the entry you removed. If they match, you’ve reached your exit condition. Yuck…

Peter
OSR

You’re probably right… indeed I never had such a use case in
“user mode land” programs.

xxxxx@frank-wolf.org wrote:

I’m popping requests of the manual queue in a loop until no more requests are there.
What happens if I have to push a request pack into the queue? Do I have an endless loop?
In case I have: Is there an easy KMDF way to solve that problem?

I think you can convince yourself that the loop will end eventually.
Remember that a request can only be in one queue at a time, and remember
that your main queue is sequential, so only one thing will be popped at
a time.

So, you’re thinking of a case where you pop from the manual queue, push
onto the sequential queue, where it then gets popped, dispatched,
processed, and pushed back on the manual queue before the original
enumeration completes. That’s possible, of course, but the queue
enumeration is going to run pretty quickly, so typically the dispatch
processing will take longer. That means, sooner or later, the
enumeration will finish first.

If you’re worried about it, you can certainly add a lock.


Tim Roberts, xxxxx@probo.com
Providenza & Boekelheide, Inc.