[ofw] [PATCHv2] WinVerbs: Make QP modification asynchronous

[Fab Tillier]

> This patch defers the QP modification to a work item if it can be
> allocated.  This is intended to serve as an example of how deferring
> work to a work item does not complicate locking, destruction handling,
> or device removal.
>
> Note that the patch has only been compile tested.  Note also that the
> choice of local variable names were selected to minimize the diffs, and
> so break with the normal syntax of all-lower-case local parameters.

Here's an updated patch that does the following:
- Only allocate the work item on Vista+, otherwise processes synchronously.  This has to do with IoQueueWorkItemEx being Vista+ only.  Uses #if WINVER checks, which uglifies the code, but it now will build for 32-bit XP.
- Moves the QP ID check into the IOCTL handler rather than the work item callback, so that invalid handle checks are done in the context of the calling thread.  This is more an exercise to demonstrate parameter validation separated from the work item processing, but does help clients in not receiving an asynchronous 'invalid handle' error.
- Removes the need to lookup the provider in the work item handler.

Signed-off-by: Fab Tillier <ftillier at microsoft.com>

Index: wv_driver.c
===================================================================
--- wv_driver.c (revision 1924)
+++ wv_driver.c (working copy)
@@ -47,7 +47,6 @@
 #include "wv_ep.h"

 WDF_DECLARE_CONTEXT_TYPE_WITH_NAME(WV_RDMA_DEVICE, WvRdmaDeviceGetContext)
-WDF_DECLARE_CONTEXT_TYPE_WITH_NAME(WV_PROVIDER, WvProviderGetContext)

 WDFDEVICE                              ControlDevice;
 static LIST_ENTRY              DevList;
Index: wv_provider.h
===================================================================
--- wv_provider.h       (revision 1924)
+++ wv_provider.h       (working copy)
@@ -66,6 +66,8 @@ typedef struct _WV_PROVIDER

 }      WV_PROVIDER;

+WDF_DECLARE_CONTEXT_TYPE_WITH_NAME(WV_PROVIDER, WvProviderGetContext)
+
 void WvProviderGet(WV_PROVIDER *pProvider);
 void WvProviderPut(WV_PROVIDER *pProvider);
 void WvProviderInit(WDFDEVICE Device, WV_PROVIDER *pProvider);
Index: wv_qp.c
===================================================================
--- wv_qp.c     (revision 1924)
+++ wv_qp.c     (working copy)
@@ -28,6 +28,7 @@
  */

 #include "wv_qp.h"
+#include "wv_provider.h"
 #include "wv_ioctl.h"

 typedef struct _WV_MULTICAST
@@ -584,8 +585,9 @@ complete:
        WdfRequestCompleteWithInformation(Request, status, len);
 }

-void WvQpModify(WV_PROVIDER *pProvider, WDFREQUEST Request)
+static void WvDoQpModify( __in DEVICE_OBJECT* pDevObj, __in void* Context, __in PIO_WORKITEM pWorkItem )
 {
+       WDFREQUEST                              Request = (WDFREQUEST)Context;
        WV_IO_QP_ATTRIBUTES             *pattr;
        UINT8                                   *out;
        size_t                                  outlen, len = 0;
@@ -594,22 +596,19 @@ void WvQpModify(WV_PROVIDER *pProvider,
        NTSTATUS                                status;
        ib_api_status_t                 ib_status;

+       UNREFERENCED_PARAMETER( pDevObj );
+
        status = WdfRequestRetrieveInputBuffer(Request, sizeof(WV_IO_QP_ATTRIBUTES),
                                                                                   &pattr, NULL);
-       if (!NT_SUCCESS(status)) {
-               goto complete;
-       }
+       // Input buffer was validated before the work item was queued.
+       ASSERT(NT_SUCCESS(status));
        status = WdfRequestRetrieveOutputBuffer(Request, 0, &out, &outlen);
        if (!NT_SUCCESS(status) && status != STATUS_BUFFER_TOO_SMALL) {
                goto complete;
        }

        WvVerbsConvertAttr(&attr, pattr);
-       qp = WvQpAcquire(pProvider, pattr->Id.Id);
-       if (qp == NULL) {
-               status = STATUS_NOT_FOUND;
-               goto complete;
-       }
+       qp = ((WV_QUEUE_PAIR*)(ULONG_PTR)pattr->Id.Id);

        ib_status = qp->pVerbs->ndi_modify_qp(qp->hVerbsQp, &attr, NULL, (UINT32) outlen, out);
        if (ib_status != IB_SUCCESS) {
@@ -620,6 +619,48 @@ void WvQpModify(WV_PROVIDER *pProvider,

 complete:
        WdfRequestCompleteWithInformation(Request, status, len);
+       if (pWorkItem) {
+               IoFreeWorkItem(pWorkItem);
+       }
+}
+
+void WvQpModify(WV_PROVIDER *pProvider, WDFREQUEST Request)
+{
+       WV_IO_QP_ATTRIBUTES             *pattr;
+       NTSTATUS                                status;
+#if WINVER >= 0x0600
+       WDFFILEOBJECT                   file;
+       WDFDEVICE                               device;
+       PIO_WORKITEM                    workitem;
+#endif
+
+       status = WdfRequestRetrieveInputBuffer(Request, sizeof(WV_IO_QP_ATTRIBUTES),
+                                                                                  &pattr, NULL);
+       if (!NT_SUCCESS(status)) {
+               WdfRequestCompleteWithInformation(Request, status, 0);
+               return;
+       }
+
+       pattr->Id.Id = (ULONG_PTR)WvQpAcquire(pProvider, pattr->Id.Id);
+       if (pattr->Id.Id == 0) {
+               WdfRequestCompleteWithInformation(Request, STATUS_NOT_FOUND, 0);
+               return;
+       }
+
+#if WINVER >= 0x0600
+       file = WdfRequestGetFileObject(Request);
+       device = WdfFileObjectGetDevice(file);
+
+       workitem = IoAllocateWorkItem( WdfDeviceWdmGetDeviceObject(device) );
+       if (workitem == NULL) {
+#endif
+               // No work item, just execute the modify synchronously.
+               WvDoQpModify(NULL, Request, NULL);
+#if WINVER >= 0x0600
+       } else {
+               IoQueueWorkItemEx(workitem, WvDoQpModify, DelayedWorkQueue, Request);
+       }
+#endif
 }

 void WvQpAttach(WV_PROVIDER *pProvider, WDFREQUEST Request)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: wv_qp_modify.2.patch
Type: application/octet-stream
Size: 3824 bytes
Desc: wv_qp_modify.2.patch
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20090211/cace6ba3/attachment.obj>