***SPAM*** RE: [ofw] RE: NetworkDirect over WinVerbs

[Fab Tillier]

>> You know what, I'll put together a patch that will move the QP
>> modification to a work item context and prove that it doesn't
>> complicate locking, destruction handling, or device removal.
>
> Yes - that patch maintains the same locking as the current code.

Note that the patch would need #ifdefs due to IoQueueWorkItemEx being Vista+ only.  I can rework it so that it builds for < Vista if you'd like.

>> In the CM callback you have a WDFREQUEST, so you can do these exact
>> steps.  The only issue is related to the CM callback being invoked at
>> DISPATCH_LEVEL, and the QP Modify being a synchronous call.  That
>> problem would go away if the HCA driver model didn't suck.  In the mean
>> time you can allocate a work item and queue it for processing at
>> passive level.  You still have the IRP outstanding, so the app context
>> won't go away.
>
> As long as being at dispatch doesn't require changing the
> synchronization in
> other areas to using spinlocks, this sounds fine.

If the only thing you do at dispatch is allocate/queue a work item, then you wouldn't need spinlocks.  In fact, you could acquire the QP when the IOCTL is initially processed, in the context of the user thread, and store the QP object into the QpId member of the input buffer to avoid the lookup later - that way all parameter validation is performed in the original caller's thread context.  The drawback would be delaying HCA removal, until the DREQ completes - not a big deal if the DREQ gets aborted when the HCA gets removed.

>> STDMETHOD(DisconnectAndFlush)(
>>        THIS_
>>        __in IVWConnectQueuePair* pQp,
>>        __in_opt OVERLAPPED* pOverlapped
>>        ) PURE;
>>  There's no more dependency between the two objects in the kernel code
>> than exists today with Connect and Accept.
>  I don't have an objection to this if it can be implemented cleanly. The
> difference between this call and Connect or Accept is that the QP object
> is accessed from a different thread.

True, but Connect and Accept could be deferred similarly to WvQpModify, to return to the user faster.

>> You can control whether I/O requests complete to the IOCP or not by
>> setting the lowest bit of the event handle in the OVERLAPPED structure
>> that you specify in your overlapped operation.  From the
>> GetQueuedCompletionStatus docs:
>
> That is not the obvious place to document this...

Yeah, it took me a while to find it...

>> "Even if you have passed the function a file handle associated with a
>> completion port and a valid OVERLAPPED structure, an application can
>> prevent completion port notification. This is done by specifying a
>> valid event handle for the hEvent member of the OVERLAPPED structure,
>> and setting its low-order bit. A valid event handle whose low-order bit
>> is set keeps I/O completion from being queued to the completion port."
>
> So, the winverbs ND provider could implement this with:
>
> Disconnect()
> NotifyDisconnect(internal overlap)
> wait on internal overlap using internal thread
> Modify(users overlap)

Yes, but then you add threads to the provider and that adds complexity with managing thread lifetimes with respect to DLL lifetime.  Another advantage of the work item is that the system thread pool is shared between all applications and grows and shrinks on demand, so you use up less system resources.

> Which doesn't seem that much different than scheduling Modify to a work
> item. Maybe slightly less efficient, but I think it's doable.

When you create a thread in a DLL, you need to take a reference on the DLL (via LoadLibrary) so that it doesn't get unloaded while the thread runs.  Then you need a mechanism to signal the threads to exit when the app is done with it, as well as calling FreeLibraryAndExitThread from the threads to properly unwind out of the DLL code and release their reference on the DLL so that it can unload.

Definitely more complicated than the work item.  Plus if QP modify ever becomes truly an async verb in the kernel, you can eliminate the PASSIVE_LEVEL requirement along with the work item without having to change user-mode code.

I'll code up a patch for DisconnectAndFlush...

-Fab