|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [RFC PATCH 8/9] ring: Defer destruction until backend grant release
If the backend is still accessing the ring page grants, we must defer
ring destruction until the backend has fully released them.
Implement a reclaim thread that polls for grant revocation. If grant
revocation ever fails, the reclaim thread will take care of polling for
grant release. Wait for the reclaim thread when transitioning to D3.
Assisted-by: LLM
Signed-off-by: Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>
---
src/xenvbd/ring.c | 306 ++++++++++++++++++++++++++++++++++++++++------
1 file changed, 270 insertions(+), 36 deletions(-)
diff --git a/src/xenvbd/ring.c b/src/xenvbd/ring.c
index 3ce3ba5..b0dd283 100644
--- a/src/xenvbd/ring.c
+++ b/src/xenvbd/ring.c
@@ -57,6 +57,14 @@
#define XENVBD_MAX_RING_PAGE_ORDER (4)
#define XENVBD_MAX_RING_PAGES (1 << XENVBD_MAX_RING_PAGE_ORDER)
+// Separate struct to be transferred to ReclaimThread on ring page reclaim.
+typedef struct _XENVBD_BLKIF_PAGES {
+ LIST_ENTRY ListEntry;
+ PMDL Mdl;
+ ULONG Count;
+ PXENBUS_GNTTAB_ENTRY Grants[XENVBD_MAX_RING_PAGES];
+} XENVBD_BLKIF_PAGES, *PXENVBD_BLKIF_PAGES;
+
typedef struct _XENVBD_BLKIF_RING {
PXENVBD_RING Ring;
ULONG Index;
@@ -64,10 +72,9 @@ typedef struct _XENVBD_BLKIF_RING {
PXENBUS_CACHE RequestCache;
PXENBUS_CACHE SegmentCache;
PXENBUS_CACHE IndirectCache;
- PMDL Mdl;
+ PXENVBD_BLKIF_PAGES Pages;
blkif_sring_t *Shared;
blkif_front_ring_t Front;
- PXENBUS_GNTTAB_ENTRY Grants[XENVBD_MAX_RING_PAGES];
PXENBUS_EVTCHN_CHANNEL Channel;
KDPC Dpc;
ULONG Dpcs;
@@ -84,6 +91,8 @@ typedef struct _XENVBD_BLKIF_RING {
LIST_ENTRY PreparedQueue;
LIST_ENTRY SubmittedList;
LIST_ENTRY ShutdownQueue;
+ // List of XENVBD_BLKIF_PAGES still being held by the backend
+ LIST_ENTRY DeferredPages;
ULONG SrbsQueued;
ULONG SrbsCompleted;
ULONG SrbsFailed;
@@ -119,6 +128,7 @@ struct _XENVBD_RING {
PXENBUS_DEBUG_CALLBACK DebugCallback;
ULONG Order;
PXENVBD_BLKIF_RING *Ring;
+ PXENVBD_THREAD ReclaimThread;
LONG Stats[XENVBD_STAT__MAX];
};
@@ -1754,6 +1764,215 @@ RingWatchdog(
return STATUS_SUCCESS;
}
+static PXENVBD_BLKIF_PAGES
+BlkifRingAllocatePages(
+ _In_ ULONG Count
+ )
+{
+ PXENVBD_BLKIF_PAGES Pages;
+
+ ASSERT3U(Count, <=, XENVBD_MAX_RING_PAGES);
+
+ Pages = __RingAllocate(sizeof(XENVBD_BLKIF_PAGES));
+ if (Pages == NULL)
+ goto fail1;
+
+ Pages->Mdl = __AllocatePages(Count, FALSE);
+ if (Pages->Mdl == NULL)
+ goto fail2;
+
+ Pages->Count = Count;
+
+ return Pages;
+
+fail2:
+ __RingFree(Pages);
+
+fail1:
+ return NULL;
+}
+
+static VOID
+__BlkifRingFreePages(
+ _In_ PXENVBD_BLKIF_PAGES Pages
+ )
+{
+ ULONG Index;
+
+ for (Index = 0; Index < Pages->Count; ++Index)
+ ASSERT3P(Pages->Grants[Index], ==, NULL);
+
+ __FreePages(Pages->Mdl);
+ Pages->Mdl = NULL;
+ Pages->Count = 0;
+
+ RtlZeroMemory(&Pages->ListEntry, sizeof(LIST_ENTRY));
+
+ ASSERT(IsZeroMemory(Pages, sizeof(XENVBD_BLKIF_PAGES)));
+ __RingFree(Pages);
+}
+
+_Check_return_
+static BOOLEAN
+__BlkifRingRevokePages(
+ _In_ PXENVBD_BLKIF_RING BlkifRing,
+ _Inout_ PXENVBD_BLKIF_PAGES Pages
+ )
+{
+ PXENVBD_RING Ring = BlkifRing->Ring;
+ PXENVBD_GRANTER Granter = FrontendGetGranter(Ring->Frontend);
+ ULONG Index;
+
+ for (Index = 0; Index < Pages->Count; ++Index) {
+ if (Pages->Grants[Index] == NULL)
+ continue;
+
+ if (!NT_SUCCESS(GranterTryPut(Granter, Pages->Grants[Index])))
+ return FALSE;
+
+ Pages->Grants[Index] = NULL;
+ }
+
+ return TRUE;
+}
+
+_IRQL_requires_(DISPATCH_LEVEL)
+static VOID
+BlkifRingReleasePages(
+ _Inout_ PXENVBD_BLKIF_RING BlkifRing
+ )
+{
+ PXENVBD_RING Ring = BlkifRing->Ring;
+ PXENVBD_BLKIF_PAGES Pages = BlkifRing->Pages;
+
+ BlkifRing->Shared = NULL;
+ BlkifRing->Pages = NULL;
+
+ if (__BlkifRingRevokePages(BlkifRing, Pages)) {
+ __BlkifRingFreePages(Pages);
+ return;
+ }
+
+ Warning("Target[%u][%u] : deferring ring pages\n",
+ FrontendGetTargetId(Ring->Frontend),
+ BlkifRing->Index);
+
+ __BlkifRingAcquireLock(BlkifRing);
+ InsertTailList(&BlkifRing->DeferredPages, &Pages->ListEntry);
+ __BlkifRingReleaseLock(BlkifRing);
+
+ ThreadWake(Ring->ReclaimThread);
+}
+
+static FORCEINLINE BOOLEAN
+__BlkifRingIsDeferring(
+ _In_ PXENVBD_BLKIF_RING BlkifRing
+ )
+{
+ return !IsListEmpty(&BlkifRing->DeferredPages);
+}
+
+_IRQL_requires_(DISPATCH_LEVEL)
+static VOID
+BlkifRingReclaim(
+ _Inout_ PXENVBD_BLKIF_RING BlkifRing
+ )
+{
+#if DBG
+ ASSERT3P(BlkifRing->LockThread, ==, KeGetCurrentThread());
+#endif
+
+ while (!IsListEmpty(&BlkifRing->DeferredPages)) {
+ PLIST_ENTRY ListEntry;
+ PXENVBD_BLKIF_PAGES Pages;
+
+ ListEntry = RemoveHeadList(&BlkifRing->DeferredPages);
+ Pages = CONTAINING_RECORD(ListEntry, XENVBD_BLKIF_PAGES, ListEntry);
+
+ if (!__BlkifRingRevokePages(BlkifRing, Pages)) {
+ // Move this reclaim failure to the back so that entries behind it
+ // are tried first on the next pass.
+ InsertTailList(&BlkifRing->DeferredPages, ListEntry);
+ break;
+ }
+
+ __BlkifRingFreePages(Pages);
+ }
+}
+
+#define XENVBD_RECLAIM_PERIOD TIME_MS(1000)
+#define XENVBD_RECLAIM_WARNING_PERIOD TIME_S(30)
+
+static NTSTATUS
+RingReclaim(
+ _In_ PXENVBD_THREAD Self,
+ _In_ PVOID Context
+ )
+{
+ PXENVBD_RING Ring = Context;
+ LARGE_INTEGER Timeout;
+ BOOLEAN Deferring;
+ ULONGLONG NextWarning;
+
+ Trace("====>\n");
+
+ Timeout.QuadPart = TIME_RELATIVE(XENVBD_RECLAIM_PERIOD);
+ Deferring = FALSE;
+ NextWarning = 0;
+
+ for (;;) {
+ PKEVENT Event;
+ ULONG Index;
+ KIRQL Irql;
+
+ Event = ThreadGetEvent(Self);
+
+ (VOID) KeWaitForSingleObject(Event,
+ Executive,
+ KernelMode,
+ FALSE,
+ Deferring ? &Timeout : NULL);
+ KeClearEvent(Event);
+
+ Deferring = FALSE;
+
+ KeRaiseIrql(DISPATCH_LEVEL, &Irql);
+
+ for (Index = 0; Index < FrontendGetMaxQueues(Ring->Frontend); ++Index)
{
+ PXENVBD_BLKIF_RING BlkifRing = Ring->Ring[Index];
+
+ __BlkifRingAcquireLock(BlkifRing);
+
+ BlkifRingReclaim(BlkifRing);
+
+ if (__BlkifRingIsDeferring(BlkifRing))
+ Deferring = TRUE;
+
+ __BlkifRingReleaseLock(BlkifRing);
+ }
+
+ KeLowerIrql(Irql);
+
+ if (!ThreadIsAlerted(Self))
+ continue;
+
+ // Only stop once the backend has released everything. Freeing pages
+ // that it may still access is worse than holding up D0->D3
+ if (!Deferring)
+ break;
+
+ if (KeQueryInterruptTime() >= NextWarning) {
+ Warning("Target[%u] : still waiting for backend to release
grants\n",
+ FrontendGetTargetId(Ring->Frontend));
+ NextWarning = KeQueryInterruptTime() +
XENVBD_RECLAIM_WARNING_PERIOD;
+ }
+ }
+
+ Trace("<====\n");
+
+ return STATUS_SUCCESS;
+}
+
static FORCEINLINE NTSTATUS
BlkifRingCacheCreate(
_Inout_ PXENVBD_BLKIF_RING BlkifRing
@@ -1940,6 +2159,7 @@ BlkifRingCreate(
InitializeListHead(&(*BlkifRing)->ShutdownQueue);
InitializeListHead(&(*BlkifRing)->SubmittedList);
InitializeListHead(&(*BlkifRing)->PreparedQueue);
+ InitializeListHead(&(*BlkifRing)->DeferredPages);
KeInitializeThreadedDpc(&(*BlkifRing)->Dpc, BlkifRingDpc, *BlkifRing);
@@ -1956,6 +2176,7 @@ fail4:
RtlZeroMemory(&(*BlkifRing)->Dpc, sizeof(KDPC));
+ RtlZeroMemory(&(*BlkifRing)->DeferredPages, sizeof(LIST_ENTRY));
RtlZeroMemory(&(*BlkifRing)->PreparedQueue, sizeof(LIST_ENTRY));
RtlZeroMemory(&(*BlkifRing)->SubmittedList, sizeof(LIST_ENTRY));
RtlZeroMemory(&(*BlkifRing)->ShutdownQueue, sizeof(LIST_ENTRY));
@@ -1988,8 +2209,12 @@ BlkifRingDestroy(
ThreadJoin(BlkifRing->WatchdogThread);
BlkifRing->WatchdogThread = NULL;
+ BUG_ON(__BlkifRingIsDeferring(BlkifRing));
+
RtlZeroMemory(&BlkifRing->Dpc, sizeof(KDPC));
+ RtlZeroMemory(&BlkifRing->DeferredPages, sizeof(LIST_ENTRY));
+
ASSERT(IsListEmpty(&BlkifRing->PreparedQueue));
RtlZeroMemory(&BlkifRing->PreparedQueue, sizeof(LIST_ENTRY));
@@ -2045,13 +2270,13 @@ BlkifRingConnect(
if (!NT_SUCCESS(status))
goto fail1;
- BlkifRing->Mdl = __AllocatePages(1 << Ring->Order, FALSE);
+ BlkifRing->Pages = BlkifRingAllocatePages(1ul << Ring->Order);
status = STATUS_NO_MEMORY;
- if (BlkifRing->Mdl == NULL)
+ if (BlkifRing->Pages == NULL)
goto fail2;
- BlkifRing->Shared = MmGetSystemAddressForMdlSafe(BlkifRing->Mdl,
+ BlkifRing->Shared = MmGetSystemAddressForMdlSafe(BlkifRing->Pages->Mdl,
NormalPagePriority);
ASSERT(BlkifRing->Shared != NULL);
@@ -2062,11 +2287,11 @@ BlkifRingConnect(
FRONT_RING_INIT(&BlkifRing->Front, BlkifRing->Shared, PAGE_SIZE <<
Ring->Order);
#pragma warning(pop)
- for (Index = 0; Index < (1ul << Ring->Order); ++Index) {
+ for (Index = 0; Index < BlkifRing->Pages->Count; ++Index) {
status = GranterGet(Granter,
- MmGetMdlPfnArray(BlkifRing->Mdl)[Index],
+ MmGetMdlPfnArray(BlkifRing->Pages->Mdl)[Index],
FALSE,
- &BlkifRing->Grants[Index]);
+ &BlkifRing->Pages->Grants[Index]);
if (!NT_SUCCESS(status))
goto fail3;
}
@@ -2121,19 +2346,9 @@ fail4:
Error("fail4\n");
fail3:
Error("fail3\n");
- for (Index = 0; Index < (1ul << Ring->Order); ++Index) {
- if (BlkifRing->Grants[Index] == NULL)
- continue;
-
- GranterPut(Granter, BlkifRing->Grants[Index]);
- BlkifRing->Grants[Index] = NULL;
- }
-
RtlZeroMemory(&BlkifRing->Front, sizeof(blkif_front_ring_t));
- __FreePages(BlkifRing->Mdl);
- BlkifRing->Shared = NULL;
- BlkifRing->Mdl = NULL;
+ BlkifRingReleasePages(BlkifRing);
fail2:
Error("fail2\n");
BlkifRingCacheDestroy(BlkifRing);
@@ -2177,13 +2392,13 @@ BlkifRingStoreWrite(
Path,
"ring-ref",
"%u",
- GranterReference(Granter, BlkifRing->Grants[0]));
+ GranterReference(Granter,
BlkifRing->Pages->Grants[0]));
if (!NT_SUCCESS(status))
goto fail2;
} else {
ULONG Index;
- for (Index = 0; Index < (1ul << Ring->Order); ++Index) {
+ for (Index = 0; Index < BlkifRing->Pages->Count; ++Index) {
CHAR Name[MAX_NAME_LEN + 1];
status = RtlStringCchPrintfA(Name,
@@ -2199,7 +2414,7 @@ BlkifRingStoreWrite(
Path,
Name,
"%u",
- GranterReference(Granter,
BlkifRing->Grants[Index]));
+ GranterReference(Granter,
BlkifRing->Pages->Grants[Index]));
if (!NT_SUCCESS(status))
goto fail4;
}
@@ -2331,8 +2546,6 @@ BlkifRingDisconnect(
)
{
PXENVBD_RING Ring = BlkifRing->Ring;
- PXENVBD_GRANTER Granter = FrontendGetGranter(Ring->Frontend);
- ULONG Index;
Trace("====> %u\n", BlkifRing->Index);
ASSERT(BlkifRing->Connected);
@@ -2347,19 +2560,15 @@ BlkifRingDisconnect(
BlkifRing->Channel);
BlkifRing->Channel = NULL;
- for (Index = 0; Index < (1ul << Ring->Order); ++Index) {
- if (BlkifRing->Grants[Index] == NULL)
- continue;
-
- GranterPut(Granter, BlkifRing->Grants[Index]);
- BlkifRing->Grants[Index] = NULL;
- }
+ // Normally the backend has closed by now, so ring pages left over from
+ // earlier connections should be revocable
+ __BlkifRingAcquireLock(BlkifRing);
+ BlkifRingReclaim(BlkifRing);
+ __BlkifRingReleaseLock(BlkifRing);
RtlZeroMemory(&BlkifRing->Front, sizeof(blkif_front_ring_t));
- __FreePages(BlkifRing->Mdl);
- BlkifRing->Shared = NULL;
- BlkifRing->Mdl = NULL;
+ BlkifRingReleasePages(BlkifRing);
BlkifRingCacheDestroy(BlkifRing);
@@ -2561,6 +2770,8 @@ RingDestroy(
{
LONG Index;
+ ASSERT3P(Ring->ReclaimThread, ==, NULL);
+
Index = (LONG) FrontendGetMaxQueues(Ring->Frontend);
ASSERT3U(Index, !=, 0);
@@ -2599,11 +2810,23 @@ RingD3ToD0(
_Inout_ PXENVBD_RING Ring
)
{
- UNREFERENCED_PARAMETER(Ring);
+ NTSTATUS status;
ASSERT3U(KeGetCurrentIrql(), ==, PASSIVE_LEVEL);
+ ASSERT3P(Ring->ReclaimThread, ==, NULL);
+
+ status = ThreadCreate(RingReclaim,
+ Ring,
+ &Ring->ReclaimThread);
+ if (!NT_SUCCESS(status))
+ goto fail1;
return STATUS_SUCCESS;
+
+fail1:
+ Error("fail1 (%08x)\n", status);
+
+ return status;
}
VOID
@@ -2611,9 +2834,20 @@ RingD0ToD3(
_Inout_ PXENVBD_RING Ring
)
{
- UNREFERENCED_PARAMETER(Ring);
+ ULONG Index;
ASSERT3U(KeGetCurrentIrql(), ==, PASSIVE_LEVEL);
+
+ // Grants are only handed over to the reclaim thread at disconnect, so a
+ // ring still connected here would never be drained
+ for (Index = 0; Index < FrontendGetMaxQueues(Ring->Frontend); ++Index)
+ ASSERT(!Ring->Ring[Index]->Connected);
+
+ // The reclaim thread only exits once nothing is deferred any more, so no
+ // grant (nor the gnttab reference behind it) survives into D3
+ ThreadAlert(Ring->ReclaimThread);
+ ThreadJoin(Ring->ReclaimThread);
+ Ring->ReclaimThread = NULL;
}
NTSTATUS
--
2.56.0.windows.3
--
Ngoc Tu Dinh | Vates XCP-ng Developer
XCP-ng & Xen Orchestra - Vates solutions
web: https://vates.tech
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |