[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH 07/10] xeniface: pre-allocate gnttab cancel work item at IRP insertion time


  • To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
  • From: david ambu <david.preetham@xxxxxxxxxx>
  • Date: Thu, 8 Oct 2026 13:01:59 +0100
  • Authentication-results: eu.smtp.expurgate.cloud; dkim=pass header.s=google header.d=citrix.com header.i="@citrix.com" header.h="Content-Transfer-Encoding:MIME-Version:References:In-Reply-To:Message-ID:Date:Subject:Cc:To:From"
  • Cc: david ambu <david.preetham@xxxxxxxxxx>
  • Delivery-date: Thu, 08 Oct 2026 12:05:32 +0000
  • List-id: Developer list for the Windows PV Drivers subproject <win-pv-devel.lists.xenproject.org>

IoAllocateWorkItem in CsqCompleteCanceledIrp can be called at
DISPATCH_LEVEL when an IRP is cancelled, but IoAllocateWorkItem
requires PASSIVE_LEVEL and may return NULL under memory pressure,
causing an immediate bugcheck via IoQueueWorkItem(NULL, ...).

Pre-allocate the work item into DriverContext[1] at IRP insertion
time (always PASSIVE_LEVEL) and read it back in the cancel routine.
Free it via IoFreeWorkItem on the normal revoke/unmap path.

Signed-off-by: david ambu <david.preetham@xxxxxxxxxx>
Assisted by Claude Sonnet 4.6
---
 src/xeniface/ioctl_gnttab.c | 30 ++++++++++++++++++++++++++++--
 src/xeniface/irp_queue.c    |  9 +++------
 2 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/src/xeniface/ioctl_gnttab.c b/src/xeniface/ioctl_gnttab.c
index da8008d..c333034 100644
--- a/src/xeniface/ioctl_gnttab.c
+++ b/src/xeniface/ioctl_gnttab.c
@@ -423,13 +423,25 @@ IoctlGnttabPermitForeignAccess(
 
     // Insert the IRP/context into the pending queue.
     // This also checks (again) if the request ID/address is unique for the 
calling process.
+    // Pre-allocate the cancel work item now (PASSIVE_LEVEL) so 
CsqCompleteCanceledIrp
+    // doesn't need to allocate at DISPATCH_LEVEL.
     Irp->Tail.Overlay.DriverContext[0] = Context;
+    Irp->Tail.Overlay.DriverContext[1] = 
IoAllocateWorkItem(Fdo->Dx->DeviceObject);
+
+    Status = STATUS_INSUFFICIENT_RESOURCES;
+    if (Irp->Tail.Overlay.DriverContext[1] == NULL)
+        goto fail10;
+
     Status = IoCsqInsertIrpEx(&Fdo->IrpQueue, Irp, NULL, Context);
     if (!NT_SUCCESS(Status))
-        goto fail10;
+        goto fail11;
 
     return STATUS_PENDING;
 
+fail11:
+    Error("Fail11\n");
+    IoFreeWorkItem(Irp->Tail.Overlay.DriverContext[1]);
+
 fail10:
     Error("Fail10\n");
 
@@ -553,6 +565,7 @@ IoctlGnttabRevokeForeignAccess(
 
     Context = PendingIrp->Tail.Overlay.DriverContext[0];
     GnttabFreeGrant(Fdo, Context);
+    IoFreeWorkItem(PendingIrp->Tail.Overlay.DriverContext[1]);
 
     PendingIrp->IoStatus.Status = STATUS_SUCCESS;
     PendingIrp->IoStatus.Information = 0;
@@ -733,15 +746,27 @@ IoctlGnttabMapForeignPages(
 
     // Insert the IRP/context into the pending queue.
     // This also checks (again) if the request ID is unique for the calling 
process.
+    // Pre-allocate the cancel work item now (PASSIVE_LEVEL) so 
CsqCompleteCanceledIrp
+    // doesn't need to allocate at DISPATCH_LEVEL.
     Irp->Tail.Overlay.DriverContext[0] = Context;
+    Irp->Tail.Overlay.DriverContext[1] = 
IoAllocateWorkItem(Fdo->Dx->DeviceObject);
+
+    status = STATUS_INSUFFICIENT_RESOURCES;
+    if (Irp->Tail.Overlay.DriverContext[1] == NULL)
+        goto fail14;
+
     status = IoCsqInsertIrpEx(&Fdo->IrpQueue, Irp, NULL, Context);
     if (!NT_SUCCESS(status))
-        goto fail14;
+        goto fail15;
 
     __FreeCapturedBuffer(In);
 
     return STATUS_PENDING;
 
+fail15:
+    Error("Fail15\n");
+    IoFreeWorkItem(Irp->Tail.Overlay.DriverContext[1]);
+
 fail14:
     Error("Fail14\n");
 
@@ -896,6 +921,7 @@ IoctlGnttabUnmapForeignPages(
 
     Context = PendingIrp->Tail.Overlay.DriverContext[0];
     GnttabFreeMap(Fdo, Context);
+    IoFreeWorkItem(PendingIrp->Tail.Overlay.DriverContext[1]);
 
     PendingIrp->IoStatus.Status = STATUS_SUCCESS;
     PendingIrp->IoStatus.Information = 0;
diff --git a/src/xeniface/irp_queue.c b/src/xeniface/irp_queue.c
index 7adf074..420cecf 100644
--- a/src/xeniface/irp_queue.c
+++ b/src/xeniface/irp_queue.c
@@ -154,15 +154,12 @@ CsqCompleteCanceledIrp(
     _In_ PIRP       Irp
     )
 {
-    PXENIFACE_FDO   Fdo = CONTAINING_RECORD(Csq, XENIFACE_FDO, IrpQueue);
+    UNREFERENCED_PARAMETER(Csq);
     PIO_WORKITEM    WorkItem;
 
     Trace("Irp %p, IRQL %d\n", Irp, KeGetCurrentIrql());
 
-    // This is not guaranteed to run at PASSIVE_LEVEL, so queue a work item
-    // to perform actual cleanup/IRP completion.
-
-    WorkItem = IoAllocateWorkItem(Fdo->Dx->DeviceObject);
-    Irp->Tail.Overlay.DriverContext[1] = WorkItem; // store so the work item 
can free it
+    // Work item was pre-allocated at IRP insertion time (PASSIVE_LEVEL).
+    WorkItem = Irp->Tail.Overlay.DriverContext[1];
     IoQueueWorkItem(WorkItem, CompleteGnttabIrp, DelayedWorkQueue, Irp);
 }
-- 
2.51.0.windows.1




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.