[PATCH 1/4] ntoskrnl: Report IRP completion via get_next_device_request if possible.

Zebediah Figura zfigura at codeweavers.com
Fri Sep 10 00:15:31 CDT 2021


Although there is arguably an advantage to saving a server request, the impetus
for this patch is make it easier for the server to process the IRP return status
before (or at the same time as) it processes the IOSB status. This allows
simpler handling of the case where the IRP handler returns STATUS_PENDING but
completes the IRP before returning.

Signed-off-by: Zebediah Figura <zfigura at codeweavers.com>
---
 dlls/ntoskrnl.exe/ntoskrnl.c | 150 +++++++++++++++++++++++++----------
 server/device.c              |  21 ++---
 server/protocol.def          |   2 +
 3 files changed, 123 insertions(+), 50 deletions(-)

diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c
index f7a9d8e8efe..b4d038c299b 100644
--- a/dlls/ntoskrnl.exe/ntoskrnl.c
+++ b/dlls/ntoskrnl.exe/ntoskrnl.c
@@ -76,6 +76,14 @@ static void *ldr_notify_cookie;
 static PLOAD_IMAGE_NOTIFY_ROUTINE load_image_notify_routines[8];
 static unsigned int load_image_notify_routine_count;
 
+struct irp_data
+{
+    HANDLE handle;
+    IRP *irp;
+    BOOL async;
+    BOOL complete;
+};
+
 static int wine_drivers_rb_compare( const void *key, const struct wine_rb_entry *entry )
 {
     const struct wine_driver *driver = WINE_RB_ENTRY_VALUE( entry, const struct wine_driver, entry );
@@ -419,10 +427,23 @@ static void *create_file_object( HANDLE handle )
 
 DECLARE_CRITICAL_SECTION(irp_completion_cs);
 
+static void free_dispatch_irp( struct irp_data *irp_data )
+{
+    IRP *irp = irp_data->irp;
+
+    if (irp->UserBuffer != irp->AssociatedIrp.SystemBuffer)
+    {
+        HeapFree( GetProcessHeap(), 0, irp->UserBuffer );
+        irp->UserBuffer = NULL;
+    }
+
+    free( irp_data );
+}
+
 /* transfer result of IRP back to wineserver */
 static NTSTATUS WINAPI dispatch_irp_completion( DEVICE_OBJECT *device, IRP *irp, void *context )
 {
-    HANDLE irp_handle = context;
+    struct irp_data *irp_data = context;
     void *out_buff = irp->UserBuffer;
     NTSTATUS status;
 
@@ -431,9 +452,17 @@ static NTSTATUS WINAPI dispatch_irp_completion( DEVICE_OBJECT *device, IRP *irp,
 
     EnterCriticalSection( &irp_completion_cs );
 
+    irp_data->complete = TRUE;
+    if (!irp_data->async)
+    {
+        /* main loop will report completion via get_next_device_request */
+        LeaveCriticalSection( &irp_completion_cs );
+        return STATUS_MORE_PROCESSING_REQUIRED;
+    }
+
     SERVER_START_REQ( set_irp_result )
     {
-        req->handle   = wine_server_obj_handle( irp_handle );
+        req->handle   = wine_server_obj_handle( irp_data->handle );
         req->status   = irp->IoStatus.u.Status;
         req->size     = irp->IoStatus.Information;
         if (!NT_ERROR(irp->IoStatus.u.Status))
@@ -444,11 +473,7 @@ static NTSTATUS WINAPI dispatch_irp_completion( DEVICE_OBJECT *device, IRP *irp,
     }
     SERVER_END_REQ;
 
-    if (irp->UserBuffer != irp->AssociatedIrp.SystemBuffer)
-    {
-        HeapFree( GetProcessHeap(), 0, irp->UserBuffer );
-        irp->UserBuffer = NULL;
-    }
+    free_dispatch_irp( irp_data );
 
     LeaveCriticalSection( &irp_completion_cs );
     return status;
@@ -458,26 +483,36 @@ struct dispatch_context
 {
     irp_params_t params;
     HANDLE handle;
-    IRP   *irp;
+    struct irp_data *irp_data;
     ULONG  in_size;
     void  *in_buff;
 };
 
-static void dispatch_irp( DEVICE_OBJECT *device, IRP *irp, struct dispatch_context *context )
+static NTSTATUS dispatch_irp( DEVICE_OBJECT *device, IRP *irp, struct dispatch_context *context )
 {
+    struct irp_data *irp_data;
     LARGE_INTEGER count;
 
-    IoSetCompletionRoutine( irp, dispatch_irp_completion, context->handle, TRUE, TRUE, TRUE );
+    if (!(irp_data = malloc( sizeof(*irp_data) )))
+        return STATUS_NO_MEMORY;
+    irp_data->handle = context->handle;
+    irp_data->irp = irp;
+    irp_data->async = FALSE;
+    irp_data->complete = FALSE;
+
+    IoSetCompletionRoutine( irp, dispatch_irp_completion, irp_data, TRUE, TRUE, TRUE );
+    context->irp_data = irp_data;
     context->handle = 0;
 
     KeQueryTickCount( &count );  /* update the global KeTickCount */
 
-    context->irp = irp;
     device->CurrentIrp = irp;
     KeEnterCriticalRegion();
     IoCallDriver( device, irp );
     KeLeaveCriticalRegion();
     device->CurrentIrp = NULL;
+
+    return STATUS_SUCCESS;
 }
 
 /* process a create request for a given file */
@@ -520,9 +555,7 @@ static NTSTATUS dispatch_create( struct dispatch_context *context )
     irp->UserEvent = NULL;
 
     irp->Flags |= IRP_CREATE_OPERATION;
-    dispatch_irp( device, irp, context );
-
-    return STATUS_SUCCESS;
+    return dispatch_irp( device, irp, context );
 }
 
 /* process a close request for a given file */
@@ -558,9 +591,7 @@ static NTSTATUS dispatch_close( struct dispatch_context *context )
     irp->UserEvent = NULL;
 
     irp->Flags |= IRP_CLOSE_OPERATION;
-    dispatch_irp( device, irp, context );
-
-    return STATUS_SUCCESS;
+    return dispatch_irp( device, irp, context );
 }
 
 /* process a read request for a given device */
@@ -600,9 +631,7 @@ static NTSTATUS dispatch_read( struct dispatch_context *context )
 
     irp->Flags |= IRP_READ_OPERATION;
     irp->Flags |= IRP_DEALLOCATE_BUFFER;  /* deallocate out_buff */
-    dispatch_irp( device, irp, context );
-
-    return STATUS_SUCCESS;
+    return dispatch_irp( device, irp, context );
 }
 
 /* process a write request for a given device */
@@ -636,9 +665,7 @@ static NTSTATUS dispatch_write( struct dispatch_context *context )
 
     irp->Flags |= IRP_WRITE_OPERATION;
     irp->Flags |= IRP_DEALLOCATE_BUFFER;  /* deallocate in_buff */
-    dispatch_irp( device, irp, context );
-
-    return STATUS_SUCCESS;
+    return dispatch_irp( device, irp, context );
 }
 
 /* process a flush request for a given device */
@@ -665,9 +692,7 @@ static NTSTATUS dispatch_flush( struct dispatch_context *context )
     irpsp = IoGetNextIrpStackLocation( irp );
     irpsp->FileObject = file;
 
-    dispatch_irp( device, irp, context );
-
-    return STATUS_SUCCESS;
+    return dispatch_irp( device, irp, context );
 }
 
 /* process an ioctl request for a given device */
@@ -680,6 +705,7 @@ static NTSTATUS dispatch_ioctl( struct dispatch_context *context )
     DEVICE_OBJECT *device;
     FILE_OBJECT *file = wine_server_get_ptr( context->params.ioctl.file );
     ULONG out_size = context->params.ioctl.out_size;
+    NTSTATUS status;
 
     if (!file) return STATUS_INVALID_HANDLE;
 
@@ -728,10 +754,10 @@ static NTSTATUS dispatch_ioctl( struct dispatch_context *context )
     context->in_buff = NULL;
 
     irp->Flags |= IRP_DEALLOCATE_BUFFER;  /* deallocate in_buff */
-    dispatch_irp( device, irp, context );
+    status = dispatch_irp( device, irp, context );
 
     HeapFree( GetProcessHeap(), 0, to_free );
-    return STATUS_SUCCESS;
+    return status;
 }
 
 /* process a volume information request for a given device */
@@ -778,9 +804,7 @@ static NTSTATUS dispatch_volume( struct dispatch_context *context )
     context->in_buff = NULL;
 
     irp->Flags |= IRP_DEALLOCATE_BUFFER;  /* deallocate out_buff */
-    dispatch_irp( device, irp, context );
-
-    return STATUS_SUCCESS;
+    return dispatch_irp( device, irp, context );
 }
 
 static NTSTATUS dispatch_free( struct dispatch_context *context )
@@ -874,16 +898,11 @@ PEPROCESS PsInitialSystemProcess = NULL;
 NTSTATUS CDECL wine_ntoskrnl_main_loop( HANDLE stop_event )
 {
     HANDLE manager = get_device_manager();
-    struct dispatch_context context;
+    struct dispatch_context context = {.in_size = 4096};
     NTSTATUS status = STATUS_SUCCESS;
     struct wine_driver *driver;
     HANDLE handles[2];
 
-    context.handle  = NULL;
-    context.irp     = NULL;
-    context.in_size = 4096;
-    context.in_buff = NULL;
-
     /* Set the system process global before setting up the request thread trickery  */
     PsInitialSystemProcess = IoGetCurrentProcess();
     request_thread = GetCurrentThreadId();
@@ -903,12 +922,47 @@ NTSTATUS CDECL wine_ntoskrnl_main_loop( HANDLE stop_event )
             goto done;
         }
 
+        EnterCriticalSection( &irp_completion_cs );
+
         SERVER_START_REQ( get_next_device_request )
         {
             req->manager  = wine_server_obj_handle( manager );
             req->prev     = wine_server_obj_handle( context.handle );
-            req->user_ptr = wine_server_client_ptr( context.irp );
-            req->status   = status;
+
+            if (context.irp_data)
+            {
+                IRP *irp = context.irp_data->irp;
+
+                req->user_ptr = wine_server_client_ptr( irp );
+
+                if (context.irp_data->complete)
+                {
+                    /* IRP completed even before we got here; we can report completion now */
+                    void *out_buff = irp->UserBuffer;
+
+                    if (irp->Flags & IRP_WRITE_OPERATION)
+                        out_buff = NULL;  /* do not transfer back input buffer */
+
+                    req->prev      = wine_server_obj_handle( context.irp_data->handle );
+                    req->status    = irp->IoStatus.u.Status;
+                    req->result    = irp->IoStatus.Information;
+                    if (!NT_ERROR(irp->IoStatus.u.Status) && out_buff)
+                        wine_server_add_data( req, out_buff, irp->IoStatus.Information );
+                }
+                else
+                {
+                    if (status == STATUS_SUCCESS)
+                        status = STATUS_PENDING;
+
+                    req->status    = status;
+                }
+            }
+            else
+            {
+                req->user_ptr = 0;
+                req->status   = status;
+            }
+
             wine_server_set_reply( req, context.in_buff, context.in_size );
             if (!(status = wine_server_call( req )))
             {
@@ -924,10 +978,26 @@ NTSTATUS CDECL wine_ntoskrnl_main_loop( HANDLE stop_event )
                 if (status == STATUS_BUFFER_OVERFLOW)
                     context.in_size = reply->in_size;
             }
-            context.irp = NULL;
         }
         SERVER_END_REQ;
 
+        if (context.irp_data)
+        {
+            if (context.irp_data->complete)
+            {
+                IoCompleteRequest( context.irp_data->irp, IO_NO_INCREMENT );
+                free_dispatch_irp( context.irp_data );
+            }
+            else
+            {
+                context.irp_data->async = TRUE;
+            }
+        }
+
+        LeaveCriticalSection( &irp_completion_cs );
+
+        context.irp_data = NULL;
+
         switch (status)
         {
         case STATUS_SUCCESS:
diff --git a/server/device.c b/server/device.c
index a2383ecdc7c..b0e417a6473 100644
--- a/server/device.c
+++ b/server/device.c
@@ -946,22 +946,25 @@ DECL_HANDLER(get_next_device_request)
                                                              0, &device_manager_ops )))
         return;
 
-    if (req->prev) close_handle( current->process, req->prev );  /* avoid an extra round-trip for close */
-
     /* process result of previous call */
     if (manager->current_call)
     {
         irp = manager->current_call;
         irp->user_ptr = req->user_ptr;
 
-        if (req->status)
-            set_irp_result( irp, req->status, NULL, 0, 0 );
-        if (irp->canceled)
-            /* if it was canceled during dispatch, we couldn't queue cancel call without client pointer,
-             * so we need to do it now */
-            cancel_irp_call( irp );
+        if (req->prev)
+        {
+            set_irp_result( irp, req->status, get_req_data(), get_req_data_size(), req->result );
+            close_handle( current->process, req->prev );  /* avoid an extra round-trip for close */
+        }
         else if (irp->async)
+        {
             set_async_pending( irp->async );
+            if (irp->canceled)
+                /* if it was canceled during dispatch, we couldn't queue cancel call without client pointer,
+                 * so we need to do it now */
+                cancel_irp_call( irp );
+        }
 
         free_irp_params( irp );
         release_object( irp );
@@ -1019,8 +1022,6 @@ DECL_HANDLER(set_irp_result)
     if ((irp = (struct irp_call *)get_handle_obj( current->process, req->handle, 0, &irp_call_ops )))
     {
         set_irp_result( irp, req->status, get_req_data(), get_req_data_size(), req->size );
-        /* we may be still dispatching the IRP. don't bother queuing cancel if it's already complete */
-        irp->canceled = 0;
         close_handle( current->process, req->handle );  /* avoid an extra round-trip for close */
         release_object( irp );
     }
diff --git a/server/protocol.def b/server/protocol.def
index 133d6ad0552..fec212a3ea3 100644
--- a/server/protocol.def
+++ b/server/protocol.def
@@ -3368,6 +3368,8 @@ struct handle_info
     obj_handle_t prev;            /* handle to the previous irp */
     unsigned int status;          /* status of the previous irp */
     client_ptr_t user_ptr;        /* user pointer of the previous irp */
+    data_size_t  result;          /* IOSB result of the previous irp */
+    VARARG(data,bytes);           /* output data of the previous irp */
 @REPLY
     irp_params_t params;          /* irp parameters */
     obj_handle_t next;            /* handle to the next irp */
-- 
2.33.0




More information about the wine-devel mailing list