Discussion:
[win-pv-devel] [PATCH xenvif 4/5] Allow FrontendIncrementStatistic() to be called at < DISPATCH_LEVEL
Paul Durrant
2018-09-18 13:56:16 UTC
Permalink
Swap the ASSERTion for a KeRaiseIrql().

Signed-off-by: Paul Durrant <***@citrix.com>
---
src/xenvif/frontend.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/xenvif/frontend.c b/src/xenvif/frontend.c
index 9949357..142b2d4 100644
--- a/src/xenvif/frontend.c
+++ b/src/xenvif/frontend.c
@@ -1634,10 +1634,11 @@ FrontendIncrementStatistic(
{
ULONG Index;
PXENVIF_FRONTEND_STATISTICS Statistics;
+ KIRQL Irql;

ASSERT(Name < XENVIF_VIF_STATISTIC_COUNT);

- ASSERT3U(KeGetCurrentIrql(), ==, DISPATCH_LEVEL);
+ KeRaiseIrql(DISPATCH_LEVEL, &Irql);

Index = KeGetCurrentProcessorNumberEx(NULL);

@@ -1645,6 +1646,8 @@ FrontendIncrementStatistic(
Statistics = &Frontend->Statistics[Index];

Statistics->Value[Name] += Delta;
+
+ KeLowerIrql(Irql);
}

static FORCEINLINE const CHAR *
--
2.5.3
Paul Durrant
2018-09-18 13:56:17 UTC
Permalink
To avoid problems with DPC timeouts move the majority of the receiver's
work, and interaction with the network stack, into a threaded DPC. This
leaves the poll entry point (called from the now non-threaded poller DPC)
to simply service responses and build a local packet queue that is then
drained by the threaded DPC.

Signed-off-by: Paul Durrant <***@citrix.com>
---
src/xenvif/receiver.c | 358 +++++++++++++++++++++++++++++---------------------
src/xenvif/vif.c | 7 +
2 files changed, 214 insertions(+), 151 deletions(-)

diff --git a/src/xenvif/receiver.c b/src/xenvif/receiver.c
index 3ee5a06..f5d572b 100644
--- a/src/xenvif/receiver.c
+++ b/src/xenvif/receiver.c
@@ -98,7 +98,10 @@ typedef struct _XENVIF_RECEIVER_RING {
ULONG BackfillSize;
PXENBUS_DEBUG_CALLBACK DebugCallback;
PXENVIF_THREAD WatchdogThread;
- LIST_ENTRY PacketList;
+ PLIST_ENTRY PacketQueue;
+ KDPC Dpc;
+ ULONG Dpcs;
+ LIST_ENTRY PacketComplete;
XENVIF_RECEIVER_HASH Hash;
} XENVIF_RECEIVER_RING, *PXENVIF_RECEIVER_RING;

@@ -921,10 +924,22 @@ fail1:
}

static VOID
+ReceiverRingCompletePacket(
+ IN PXENVIF_RECEIVER_RING Ring,
+ IN PXENVIF_RECEIVER_PACKET Packet
+ )
+{
+ ReceiverRingProcessTag(Ring, Packet);
+ ReceiverRingProcessChecksum(Ring, Packet);
+
+ ASSERT(IsZeroMemory(&Packet->ListEntry, sizeof (LIST_ENTRY)));
+ InsertTailList(&Ring->PacketComplete, &Packet->ListEntry);
+}
+
+static VOID
ReceiverRingProcessLargePacket(
IN PXENVIF_RECEIVER_RING Ring,
- IN PXENVIF_RECEIVER_PACKET Packet,
- OUT PLIST_ENTRY List
+ IN PXENVIF_RECEIVER_PACKET Packet
)
{
PXENVIF_RECEIVER Receiver;
@@ -1014,8 +1029,7 @@ ReceiverRingProcessLargePacket(
ASSERT3U(Length, >=, SegmentSize);
Length -= SegmentSize;

- ASSERT(IsZeroMemory(&Segment->ListEntry, sizeof (LIST_ENTRY)));
- InsertTailList(List, &Segment->ListEntry);
+ ReceiverRingCompletePacket(Ring, Segment);

if (Offload) {
ASSERT(Ring->OffloadOptions.NeedLargePacketSplit != 0);
@@ -1064,8 +1078,7 @@ ReceiverRingProcessLargePacket(
if (Receiver->AlwaysPullup != 0)
__ReceiverRingPullupPacket(Ring, Packet);

- ASSERT(IsZeroMemory(&Packet->ListEntry, sizeof (LIST_ENTRY)));
- InsertTailList(List, &Packet->ListEntry);
+ ReceiverRingCompletePacket(Ring, Packet);
} else {
__ReceiverRingPutPacket(Ring, Packet, TRUE);
}
@@ -1102,8 +1115,7 @@ fail1:
static VOID
ReceiverRingProcessStandardPacket(
IN PXENVIF_RECEIVER_RING Ring,
- IN PXENVIF_RECEIVER_PACKET Packet,
- OUT PLIST_ENTRY List
+ IN PXENVIF_RECEIVER_PACKET Packet
)
{
PXENVIF_RECEIVER Receiver;
@@ -1175,9 +1187,7 @@ ReceiverRingProcessStandardPacket(
Packet->Mdl.Next = Mdl;
}

- ASSERT(IsZeroMemory(&Packet->ListEntry, sizeof (LIST_ENTRY)));
- InsertTailList(List, &Packet->ListEntry);
-
+ ReceiverRingCompletePacket(Ring, Packet);
return;

fail2:
@@ -1210,8 +1220,7 @@ fail1:
static VOID
ReceiverRingProcessPacket(
IN PXENVIF_RECEIVER_RING Ring,
- IN PXENVIF_RECEIVER_PACKET Packet,
- OUT PLIST_ENTRY List
+ IN PXENVIF_RECEIVER_PACKET Packet
)
{
PXENVIF_RECEIVER Receiver;
@@ -1299,9 +1308,9 @@ ReceiverRingProcessPacket(
goto fail3;

if (Packet->MaximumSegmentSize != 0)
- ReceiverRingProcessLargePacket(Ring, Packet, List);
+ ReceiverRingProcessLargePacket(Ring, Packet);
else
- ReceiverRingProcessStandardPacket(Ring, Packet, List);
+ ReceiverRingProcessStandardPacket(Ring, Packet);

return;

@@ -1334,63 +1343,8 @@ fail1:
1);
}

-static VOID
-ReceiverRingProcessPackets(
- IN PXENVIF_RECEIVER_RING Ring,
- OUT PLIST_ENTRY List,
- OUT PULONG Count
- )
-{
- PLIST_ENTRY ListEntry;
-
- while (!IsListEmpty(&Ring->PacketList)) {
- PXENVIF_RECEIVER_PACKET Packet;
-
- ListEntry = RemoveHeadList(&Ring->PacketList);
- ASSERT3P(ListEntry, !=, &Ring->PacketList);
-
- RtlZeroMemory(ListEntry, sizeof (LIST_ENTRY));
-
- Packet = CONTAINING_RECORD(ListEntry, XENVIF_RECEIVER_PACKET, ListEntry);
- ReceiverRingProcessPacket(Ring, Packet, List);
- }
-
- for (ListEntry = List->Flink;
- ListEntry != List;
- ListEntry = ListEntry->Flink) {
- PXENVIF_RECEIVER_PACKET Packet;
-
- Packet = CONTAINING_RECORD(ListEntry, XENVIF_RECEIVER_PACKET, ListEntry);
-
- ReceiverRingProcessTag(Ring, Packet);
- ReceiverRingProcessChecksum(Ring, Packet);
-
- (*Count)++;
- }
-}
-
-static FORCEINLINE VOID
-__drv_requiresIRQL(DISPATCH_LEVEL)
-__ReceiverRingAcquireLock(
- IN PXENVIF_RECEIVER_RING Ring
- )
-{
- ASSERT3U(KeGetCurrentIrql(), ==, DISPATCH_LEVEL);
-
- KeAcquireSpinLockAtDpcLevel(&Ring->Lock);
-}
-
-static DECLSPEC_NOINLINE VOID
-ReceiverRingAcquireLock(
- IN PXENVIF_RECEIVER_RING Ring
- )
-{
- __ReceiverRingAcquireLock(Ring);
-}
-
static FORCEINLINE VOID
-__drv_requiresIRQL(DISPATCH_LEVEL)
-__ReceiverRingReleaseLock(
+__ReceiverRingSwizzle(
IN PXENVIF_RECEIVER_RING Ring
)
{
@@ -1398,33 +1352,44 @@ __ReceiverRingReleaseLock(
PXENVIF_FRONTEND Frontend;
PXENVIF_VIF_CONTEXT Context;
LIST_ENTRY List;
- ULONG Count;
- BOOLEAN More;
-
- ASSERT3U(KeGetCurrentIrql(), ==, DISPATCH_LEVEL);
+ PLIST_ENTRY ListEntry;

Receiver = Ring->Receiver;
Frontend = Receiver->Frontend;
Context = PdoGetVifContext(FrontendGetPdo(Frontend));

InitializeListHead(&List);
- Count = 0;

- ReceiverRingProcessPackets(Ring, &List, &Count);
- ASSERT(EQUIV(IsListEmpty(&List), Count == 0));
- ASSERT(IsListEmpty(&Ring->PacketList));
+ ListEntry = InterlockedExchangePointer(&Ring->PacketQueue, NULL);

- // We need to bump Loaned before dropping the lock to avoid VifDisable()
- // returning prematurely.
- __InterlockedAdd(&Receiver->Loaned, Count);
+ // Packets are held in the queue in reverse order so that the most
+ // recent is always head of the list. This is necessary to allow
+ // addition to the list to be done atomically.

-#pragma prefast(disable:26110)
- KeReleaseSpinLockFromDpcLevel(&Ring->Lock);
+ while (ListEntry != NULL) {
+ PLIST_ENTRY NextEntry;
+
+ NextEntry = ListEntry->Blink;
+ ListEntry->Flink = ListEntry->Blink = ListEntry;
+
+ InsertHeadList(&List, ListEntry);
+
+ ListEntry = NextEntry;
+ }
+
+ while (!IsListEmpty(&List)) {
+ PXENVIF_RECEIVER_PACKET Packet;

- More = !IsListEmpty(&List) ? TRUE : FALSE;
+ ListEntry = RemoveHeadList(&List);
+ ASSERT3P(ListEntry, !=, &List);

- while (More) {
- PLIST_ENTRY ListEntry;
+ RtlZeroMemory(ListEntry, sizeof (LIST_ENTRY));
+
+ Packet = CONTAINING_RECORD(ListEntry, XENVIF_RECEIVER_PACKET, ListEntry);
+ ReceiverRingProcessPacket(Ring, Packet);
+ }
+
+ while (!IsListEmpty(&Ring->PacketComplete)) {
PXENVIF_RECEIVER_PACKET Packet;
PXENVIF_PACKET_INFO Info;
PUCHAR BaseVa;
@@ -1432,14 +1397,11 @@ __ReceiverRingReleaseLock(
PETHERNET_ADDRESS DestinationAddress;
ETHERNET_ADDRESS_TYPE Type;

- ListEntry = RemoveHeadList(&List);
- ASSERT3P(ListEntry, !=, &List);
+ ListEntry = RemoveHeadList(&Ring->PacketComplete);
+ ASSERT3P(ListEntry, !=, &Ring->PacketComplete);

RtlZeroMemory(ListEntry, sizeof (LIST_ENTRY));

- ASSERT(More);
- More = !IsListEmpty(&List) ? TRUE : FALSE;
-
Packet = CONTAINING_RECORD(ListEntry,
XENVIF_RECEIVER_PACKET,
ListEntry);
@@ -1530,55 +1492,57 @@ __ReceiverRingReleaseLock(
XENVIF_RECEIVER_UDP_PACKETS,
1);

- if (Packet->MaximumSegmentSize != 0)
+ if (Packet->MaximumSegmentSize != 0)
FrontendIncrementStatistic(Frontend,
XENVIF_RECEIVER_GSO_PACKETS,
1);

- if (Packet->Flags.IpChecksumSucceeded != 0)
- FrontendIncrementStatistic(Frontend,
- XENVIF_RECEIVER_IPV4_CHECKSUM_SUCCEEDED,
- 1);
-
- if (Packet->Flags.IpChecksumFailed != 0)
- FrontendIncrementStatistic(Frontend,
- XENVIF_RECEIVER_IPV4_CHECKSUM_FAILED,
- 1);
-
- if (Packet->Flags.IpChecksumNotValidated != 0)
- FrontendIncrementStatistic(Frontend,
- XENVIF_RECEIVER_IPV4_CHECKSUM_NOT_VALIDATED,
- 1);
-
- if (Packet->Flags.TcpChecksumSucceeded != 0)
- FrontendIncrementStatistic(Frontend,
- XENVIF_RECEIVER_TCP_CHECKSUM_SUCCEEDED,
- 1);
-
- if (Packet->Flags.TcpChecksumFailed != 0)
- FrontendIncrementStatistic(Frontend,
- XENVIF_RECEIVER_TCP_CHECKSUM_FAILED,
- 1);
-
- if (Packet->Flags.TcpChecksumNotValidated != 0)
- FrontendIncrementStatistic(Frontend,
- XENVIF_RECEIVER_TCP_CHECKSUM_NOT_VALIDATED,
- 1);
-
- if (Packet->Flags.UdpChecksumSucceeded != 0)
- FrontendIncrementStatistic(Frontend,
- XENVIF_RECEIVER_UDP_CHECKSUM_SUCCEEDED,
- 1);
-
- if (Packet->Flags.UdpChecksumFailed != 0)
- FrontendIncrementStatistic(Frontend,
- XENVIF_RECEIVER_UDP_CHECKSUM_FAILED,
- 1);
-
- if (Packet->Flags.UdpChecksumNotValidated != 0)
- FrontendIncrementStatistic(Frontend,
- XENVIF_RECEIVER_UDP_CHECKSUM_NOT_VALIDATED,
- 1);
+ if (Packet->Flags.IpChecksumSucceeded != 0)
+ FrontendIncrementStatistic(Frontend,
+ XENVIF_RECEIVER_IPV4_CHECKSUM_SUCCEEDED,
+ 1);
+
+ if (Packet->Flags.IpChecksumFailed != 0)
+ FrontendIncrementStatistic(Frontend,
+ XENVIF_RECEIVER_IPV4_CHECKSUM_FAILED,
+ 1);
+
+ if (Packet->Flags.IpChecksumNotValidated != 0)
+ FrontendIncrementStatistic(Frontend,
+ XENVIF_RECEIVER_IPV4_CHECKSUM_NOT_VALIDATED,
+ 1);
+
+ if (Packet->Flags.TcpChecksumSucceeded != 0)
+ FrontendIncrementStatistic(Frontend,
+ XENVIF_RECEIVER_TCP_CHECKSUM_SUCCEEDED,
+ 1);
+
+ if (Packet->Flags.TcpChecksumFailed != 0)
+ FrontendIncrementStatistic(Frontend,
+ XENVIF_RECEIVER_TCP_CHECKSUM_FAILED,
+ 1);
+
+ if (Packet->Flags.TcpChecksumNotValidated != 0)
+ FrontendIncrementStatistic(Frontend,
+ XENVIF_RECEIVER_TCP_CHECKSUM_NOT_VALIDATED,
+ 1);
+
+ if (Packet->Flags.UdpChecksumSucceeded != 0)
+ FrontendIncrementStatistic(Frontend,
+ XENVIF_RECEIVER_UDP_CHECKSUM_SUCCEEDED,
+ 1);
+
+ if (Packet->Flags.UdpChecksumFailed != 0)
+ FrontendIncrementStatistic(Frontend,
+ XENVIF_RECEIVER_UDP_CHECKSUM_FAILED,
+ 1);
+
+ if (Packet->Flags.UdpChecksumNotValidated != 0)
+ FrontendIncrementStatistic(Frontend,
+ XENVIF_RECEIVER_UDP_CHECKSUM_NOT_VALIDATED,
+ 1);
+
+ (VOID) InterlockedIncrement(&Receiver->Loaned);

VifReceiverQueuePacket(Context,
Ring->Index,
@@ -1590,13 +1554,40 @@ __ReceiverRingReleaseLock(
Packet->TagControlInformation,
&Packet->Info,
&Packet->Hash,
- More,
+ !IsListEmpty(&Ring->PacketComplete) ? TRUE : FALSE,
Packet);
-
- --Count;
}
+}

- ASSERT3U(Count, ==, 0);
+static FORCEINLINE VOID
+__drv_requiresIRQL(DISPATCH_LEVEL)
+__ReceiverRingAcquireLock(
+ IN PXENVIF_RECEIVER_RING Ring
+ )
+{
+ ASSERT3U(KeGetCurrentIrql(), ==, DISPATCH_LEVEL);
+
+ KeAcquireSpinLockAtDpcLevel(&Ring->Lock);
+}
+
+static DECLSPEC_NOINLINE VOID
+ReceiverRingAcquireLock(
+ IN PXENVIF_RECEIVER_RING Ring
+ )
+{
+ __ReceiverRingAcquireLock(Ring);
+}
+
+static FORCEINLINE VOID
+__drv_requiresIRQL(DISPATCH_LEVEL)
+__ReceiverRingReleaseLock(
+ IN PXENVIF_RECEIVER_RING Ring
+ )
+{
+ ASSERT3U(KeGetCurrentIrql(), ==, DISPATCH_LEVEL);
+
+#pragma prefast(disable:26110)
+ KeReleaseSpinLockFromDpcLevel(&Ring->Lock);
}

static DECLSPEC_NOINLINE VOID
@@ -1607,6 +1598,29 @@ ReceiverRingReleaseLock(
__ReceiverRingReleaseLock(Ring);
}

+__drv_functionClass(KDEFERRED_ROUTINE)
+__drv_maxIRQL(DISPATCH_LEVEL)
+__drv_minIRQL(PASSIVE_LEVEL)
+__drv_sameIRQL
+static VOID
+ReceiverRingDpc(
+ IN PKDPC Dpc,
+ IN PVOID Context,
+ IN PVOID Argument1,
+ IN PVOID Argument2
+ )
+{
+ PXENVIF_RECEIVER_RING Ring = Context;
+
+ UNREFERENCED_PARAMETER(Dpc);
+ UNREFERENCED_PARAMETER(Argument1);
+ UNREFERENCED_PARAMETER(Argument2);
+
+ ASSERT(Ring != NULL);
+
+ __ReceiverRingSwizzle(Ring);
+}
+
static FORCEINLINE VOID
__ReceiverRingStop(
IN PXENVIF_RECEIVER_RING Ring
@@ -1892,6 +1906,11 @@ ReceiverRingDebugCallback(
(Ring->Enabled) ? "ENABLED" : "DISABLED",
(__ReceiverRingIsStopped(Ring)) ? "STOPPED" : "RUNNING");

+ XENBUS_DEBUG(Printf,
+ &Receiver->DebugInterface,
+ "Dpcs = %lu\n",
+ Ring->Dpcs);
+
// Dump front ring
XENBUS_DEBUG(Printf,
&Receiver->DebugInterface,
@@ -1918,6 +1937,26 @@ ReceiverRingDebugCallback(
Ring->ResponsesProcessed);
}

+static FORCEINLINE VOID
+__ReceiverRingQueuePacket(
+ IN PXENVIF_RECEIVER_RING Ring,
+ IN PXENVIF_RECEIVER_PACKET Packet
+ )
+{
+ PLIST_ENTRY ListEntry;
+ PLIST_ENTRY Old;
+ PLIST_ENTRY New;
+
+ ListEntry = &Packet->ListEntry;
+
+ do {
+ Old = Ring->PacketQueue;
+
+ ListEntry->Blink = Ring->PacketQueue;
+ New = ListEntry;
+ } while (InterlockedCompareExchangePointer(&Ring->PacketQueue, (PVOID)New, (PVOID)Old) != Old);
+}
+
static DECLSPEC_NOINLINE BOOLEAN
ReceiverRingPoll(
IN PXENVIF_RECEIVER_RING Ring
@@ -2133,7 +2172,7 @@ ReceiverRingPoll(
Packet->Flags.Value = flags;

ASSERT(IsZeroMemory(&Packet->ListEntry, sizeof (LIST_ENTRY)));
- InsertTailList(&Ring->PacketList, &Packet->ListEntry);
+ __ReceiverRingQueuePacket(Ring, Packet);
}

if (rsp_cons - Ring->Front.rsp_cons > XENVIF_RECEIVER_BATCH(Ring))
@@ -2166,6 +2205,10 @@ ReceiverRingPoll(
if (!__ReceiverRingIsStopped(Ring))
ReceiverRingFill(Ring);

+ if (Ring->PacketQueue != NULL &&
+ KeInsertQueueDpc(&Ring->Dpc, NULL, NULL))
+ Ring->Dpcs++;
+
done:
return Retry;

@@ -2301,7 +2344,7 @@ __ReceiverRingInitialize(
if ((*Ring)->Path == NULL)
goto fail2;

- InitializeListHead(&(*Ring)->PacketList);
+ InitializeListHead(&(*Ring)->PacketComplete);

status = RtlStringCbPrintfA(Name,
sizeof (Name),
@@ -2359,6 +2402,8 @@ __ReceiverRingInitialize(
if (!NT_SUCCESS(status))
goto fail7;

+ KeInitializeThreadedDpc(&(*Ring)->Dpc, ReceiverRingDpc, *Ring);
+
return STATUS_SUCCESS;

fail7:
@@ -2386,7 +2431,7 @@ fail4:
fail3:
Error("fail3\n");

- RtlZeroMemory(&(*Ring)->PacketList, sizeof (LIST_ENTRY));
+ RtlZeroMemory(&(*Ring)->PacketComplete, sizeof (LIST_ENTRY));

FrontendFreePath(Frontend, (*Ring)->Path);
(*Ring)->Path = NULL;
@@ -2419,6 +2464,7 @@ __ReceiverRingConnect(
PFN_NUMBER Pfn;
CHAR Name[MAXNAMELEN];
ULONG Index;
+ PROCESSOR_NUMBER ProcNumber;
NTSTATUS status;

Receiver = Ring->Receiver;
@@ -2495,6 +2541,11 @@ __ReceiverRingConnect(
if (!NT_SUCCESS(status))
goto fail6;

+ status = KeGetProcessorNumberFromIndex(Ring->Index, &ProcNumber);
+ ASSERT(NT_SUCCESS(status));
+
+ KeSetTargetProcessorDpcEx(&Ring->Dpc, &ProcNumber);
+
return STATUS_SUCCESS;

fail6:
@@ -2643,6 +2694,9 @@ __ReceiverRingDisable(
Ring->Enabled = FALSE;
Ring->Stopped = FALSE;

+ if (KeInsertQueueDpc(&Ring->Dpc, NULL, NULL))
+ Ring->Dpcs++;
+
__ReceiverRingReleaseLock(Ring);

Info("%s[%u]: <====\n",
@@ -2661,6 +2715,8 @@ __ReceiverRingDisconnect(
Receiver = Ring->Receiver;
Frontend = Receiver->Frontend;

+ Ring->Dpcs = 0;
+
__ReceiverRingEmpty(Ring);

ASSERT(Ring->Connected);
@@ -2714,6 +2770,9 @@ __ReceiverRingTeardown(
Ring->BackfillSize = 0;
Ring->OffloadOptions.Value = 0;

+ KeFlushQueuedDpcs();
+ RtlZeroMemory(&Ring->Dpc, sizeof (KDPC));
+
ThreadAlert(Ring->WatchdogThread);
ThreadJoin(Ring->WatchdogThread);
Ring->WatchdogThread = NULL;
@@ -2728,8 +2787,8 @@ __ReceiverRingTeardown(
Ring->PacketCache);
Ring->PacketCache = NULL;

- ASSERT(IsListEmpty(&Ring->PacketList));
- RtlZeroMemory(&Ring->PacketList, sizeof (LIST_ENTRY));
+ ASSERT(IsListEmpty(&Ring->PacketComplete));
+ RtlZeroMemory(&Ring->PacketComplete, sizeof (LIST_ENTRY));

FrontendFreePath(Frontend, Ring->Path);
Ring->Path = NULL;
@@ -3510,16 +3569,13 @@ ReceiverWaitForPackets(
LARGE_INTEGER Timeout;

ASSERT3U(KeGetCurrentIrql(), <, DISPATCH_LEVEL);
+ KeFlushQueuedDpcs();

Frontend = Receiver->Frontend;

Trace("%s: ====>\n", FrontendGetPath(Frontend));

Returned = Receiver->Returned;
-
- // Make sure Loaned is not sampled before Returned
- KeMemoryBarrier();
-
Loaned = Receiver->Loaned;
ASSERT3S(Loaned - Returned, >=, 0);

diff --git a/src/xenvif/vif.c b/src/xenvif/vif.c
index ffdec50..69ced78 100644
--- a/src/xenvif/vif.c
+++ b/src/xenvif/vif.c
@@ -1161,6 +1161,7 @@ __VifReceiverQueuePacket(
Hash,
More,
Cookie);
+
}

VOID
@@ -1179,6 +1180,10 @@ VifReceiverQueuePacket(
IN PVOID Cookie
)
{
+ KIRQL Irql;
+
+ KeRaiseIrql(DISPATCH_LEVEL, &Irql);
+
switch (Context->Version) {
case 6:
__VifReceiverQueuePacketVersion6(Context,
@@ -1229,6 +1234,8 @@ VifReceiverQueuePacket(
ASSERT(FALSE);
break;
}
+
+ KeLowerIrql(Irql);
}

VOID
--
2.5.3
Paul Durrant
2018-09-18 13:56:13 UTC
Permalink
The threaded DPC was introduced by commit bc722edd "Don't use KTIMERs in
receive path" but it appears to have too much of an impact on performance.
This patch reverts the poller to a normal DPC but does not introduce any
DPC timeout mitigation. That will be done by a subsequent patch.

Signed-off-by: Paul Durrant <***@citrix.com>
---
src/xenvif/poller.c | 44 +++++++++++++++++++-------------------------
1 file changed, 19 insertions(+), 25 deletions(-)

diff --git a/src/xenvif/poller.c b/src/xenvif/poller.c
index cc7e68f..7f18d13 100644
--- a/src/xenvif/poller.c
+++ b/src/xenvif/poller.c
@@ -157,38 +157,42 @@ fail1:
return status;
}

-static VOID
+static BOOLEAN
PollerChannelSetPending(
IN PXENVIF_POLLER_CHANNEL Channel
)
{
PXENVIF_POLLER_INSTANCE Instance;
+ ULONG Set;

Instance = Channel->Instance;

switch (Channel->Type)
{
case XENVIF_POLLER_CHANNEL_RECEIVER:
- (VOID) InterlockedBitTestAndSet(&Instance->Pending,
- XENVIF_POLLER_EVENT_RECEIVE);
+ Set = InterlockedBitTestAndSet(&Instance->Pending,
+ XENVIF_POLLER_EVENT_RECEIVE);
break;

case XENVIF_POLLER_CHANNEL_TRANSMITTER:
- (VOID) InterlockedBitTestAndSet(&Instance->Pending,
- XENVIF_POLLER_EVENT_TRANSMIT);
+ Set = InterlockedBitTestAndSet(&Instance->Pending,
+ XENVIF_POLLER_EVENT_TRANSMIT);
break;

case XENVIF_POLLER_CHANNEL_COMBINED:
- (VOID) InterlockedBitTestAndSet(&Instance->Pending,
- XENVIF_POLLER_EVENT_RECEIVE);
- (VOID) InterlockedBitTestAndSet(&Instance->Pending,
+ Set = InterlockedBitTestAndSet(&Instance->Pending,
+ XENVIF_POLLER_EVENT_RECEIVE);
+ Set |= InterlockedBitTestAndSet(&Instance->Pending,
XENVIF_POLLER_EVENT_TRANSMIT);
break;

default:
ASSERT(FALSE);
+ Set = 0;
break;
}
+
+ return (Set != 0) ? FALSE : TRUE;
}

static FORCEINLINE BOOLEAN
@@ -256,9 +260,8 @@ PollerChannelEvtchnCallback(

Channel->Events++;

- PollerChannelSetPending(Channel);
-
- if (KeInsertQueueDpc(&Instance->Dpc, NULL, NULL))
+ if (PollerChannelSetPending(Channel) &&
+ KeInsertQueueDpc(&Instance->Dpc, NULL, NULL))
Instance->Dpcs++;

return TRUE;
@@ -412,7 +415,7 @@ PollerChannelUnmask(
FALSE,
FALSE);
if (Pending)
- PollerChannelSetPending(Channel);
+ (VOID) PollerChannelSetPending(Channel);
}

static VOID
@@ -538,7 +541,7 @@ done:

__drv_functionClass(KDEFERRED_ROUTINE)
__drv_maxIRQL(DISPATCH_LEVEL)
-__drv_minIRQL(PASSIVE_LEVEL)
+__drv_minIRQL(DISPATCH_LEVEL)
__drv_sameIRQL
static VOID
PollerInstanceDpc(
@@ -570,11 +573,10 @@ PollerInstanceDpc(
for (;;) {
BOOLEAN NeedReceiverPoll;
BOOLEAN NeedTransmitterPoll;
- KIRQL Irql;

- KeAcquireSpinLock(&Instance->Lock, &Irql);
+ KeAcquireSpinLockAtDpcLevel(&Instance->Lock);
Enabled = Instance->Enabled;
- KeReleaseSpinLock(&Instance->Lock, Irql);
+ KeReleaseSpinLockFromDpcLevel(&Instance->Lock);

if (!Enabled)
break;
@@ -596,8 +598,6 @@ PollerInstanceDpc(

if (NeedReceiverPoll)
{
- KeRaiseIrql(DISPATCH_LEVEL, &Irql);
-
ReceiverRetry = ReceiverPoll(FrontendGetReceiver(Frontend),
Instance->Index);

@@ -607,14 +607,10 @@ PollerInstanceDpc(
(VOID) InterlockedBitTestAndSet(&Instance->Pending,
XENVIF_POLLER_EVENT_RECEIVE);
}
-
- KeLowerIrql(Irql);
}

if (NeedTransmitterPoll)
{
- KeRaiseIrql(DISPATCH_LEVEL, &Irql);
-
TransmitterRetry = TransmitterPoll(FrontendGetTransmitter(Frontend),
Instance->Index);

@@ -624,8 +620,6 @@ PollerInstanceDpc(
(VOID) InterlockedBitTestAndSet(&Instance->Pending,
XENVIF_POLLER_EVENT_TRANSMIT);
}
-
- KeLowerIrql(Irql);
}
}

@@ -672,7 +666,7 @@ PollerInstanceInitialize(

KeInitializeSpinLock(&(*Instance)->Lock);

- KeInitializeThreadedDpc(&(*Instance)->Dpc, PollerInstanceDpc, *Instance);
+ KeInitializeDpc(&(*Instance)->Dpc, PollerInstanceDpc, *Instance);

return STATUS_SUCCESS;
--
2.5.3
Paul Durrant
2018-09-18 13:56:14 UTC
Permalink
There was a flaw in commit 5932938b "Don't bump the receiver event counter
if the poller is going to retry" in that it is possible to drop out of
poll without ever updating the event counter (if one attempt requests a
retry and the next attempt finds nothing to do). This patch fixes the
issue by using the RING_FINAL_CHECK_FOR_RESPONSES macro to update the
event counter, which checks for a race with new responses.

Signed-off-by: Paul Durrant <***@citrix.com>
---
src/xenvif/receiver.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/xenvif/receiver.c b/src/xenvif/receiver.c
index 1f64fab..3ee5a06 100644
--- a/src/xenvif/receiver.c
+++ b/src/xenvif/receiver.c
@@ -1969,8 +1969,13 @@ ReceiverRingPoll(

KeMemoryBarrier();

- if (rsp_cons == rsp_prod)
- break;
+ if (rsp_cons == rsp_prod) {
+ RING_IDX WorkToDo;
+
+ RING_FINAL_CHECK_FOR_RESPONSES(&Ring->Front, WorkToDo);
+ if (!WorkToDo)
+ break;
+ }

while (rsp_cons != rsp_prod && !Retry) {
netif_rx_response_t *rsp;
@@ -2156,9 +2161,6 @@ ReceiverRingPoll(
KeMemoryBarrier();

Ring->Front.rsp_cons = rsp_cons;
- if (!Retry)
- Ring->Shared->rsp_event = rsp_cons + 1;
-
}

if (!__ReceiverRingIsStopped(Ring))
--
2.5.3
Paul Durrant
2018-09-18 13:56:15 UTC
Permalink
Each receiver thread needs to call MacApplyFilters() but the implementation
uses a normal kernel spin lock and hence the threads will all contend the
lock. Switch the mac code to use a reader/writer lock so that
MacApplyFilters() can acquire the lock as a reader and thus avoid causing
the contention.

Signed-off-by: Paul Durrant <***@citrix.com>
---
src/xenvif/mac.c | 139 +++++++++++++++++++++++++++++++++++++++++++------------
1 file changed, 110 insertions(+), 29 deletions(-)

diff --git a/src/xenvif/mac.c b/src/xenvif/mac.c
index 3110dde..428200a 100644
--- a/src/xenvif/mac.c
+++ b/src/xenvif/mac.c
@@ -49,7 +49,7 @@ typedef struct _XENVIF_MAC_MULTICAST {

struct _XENVIF_MAC {
PXENVIF_FRONTEND Frontend;
- KSPIN_LOCK Lock;
+ EX_SPIN_LOCK Lock;
BOOLEAN Connected;
BOOLEAN Enabled;
ULONG MaximumFrameSize;
@@ -214,7 +214,6 @@ MacInitialize(
if (*Mac == NULL)
goto fail1;

- KeInitializeSpinLock(&(*Mac)->Lock);
InitializeListHead(&(*Mac)->MulticastList);

FdoGetDebugInterface(PdoGetFdo(FrontendGetPdo(Frontend)),
@@ -233,6 +232,52 @@ fail1:
return status;
}

+static FORCEINLINE VOID
+__drv_requiresIRQL(DISPATCH_LEVEL)
+__MacAcquireLockExclusive(
+ IN PXENVIF_MAC Mac
+ )
+{
+ ASSERT3U(KeGetCurrentIrql(), ==, DISPATCH_LEVEL);
+
+ ExAcquireSpinLockExclusiveAtDpcLevel(&Mac->Lock);
+}
+
+static FORCEINLINE VOID
+__drv_requiresIRQL(DISPATCH_LEVEL)
+__MacReleaseLockExclusive(
+ IN PXENVIF_MAC Mac
+ )
+{
+ ASSERT3U(KeGetCurrentIrql(), ==, DISPATCH_LEVEL);
+
+#pragma prefast(disable:26110)
+ ExReleaseSpinLockExclusiveFromDpcLevel(&Mac->Lock);
+}
+
+static FORCEINLINE VOID
+__drv_requiresIRQL(DISPATCH_LEVEL)
+__MacAcquireLockShared(
+ IN PXENVIF_MAC Mac
+ )
+{
+ ASSERT3U(KeGetCurrentIrql(), ==, DISPATCH_LEVEL);
+
+ ExAcquireSpinLockSharedAtDpcLevel(&Mac->Lock);
+}
+
+static FORCEINLINE VOID
+__drv_requiresIRQL(DISPATCH_LEVEL)
+__MacReleaseLockShared(
+ IN PXENVIF_MAC Mac
+ )
+{
+ ASSERT3U(KeGetCurrentIrql(), ==, DISPATCH_LEVEL);
+
+#pragma prefast(disable:26110)
+ ExReleaseSpinLockSharedFromDpcLevel(&Mac->Lock);
+}
+
static NTSTATUS
MacDumpAddressTable(
IN PXENVIF_MAC Mac
@@ -250,7 +295,8 @@ MacDumpAddressTable(

Frontend = Mac->Frontend;

- KeAcquireSpinLock(&Mac->Lock, &Irql);
+ KeRaiseIrql(DISPATCH_LEVEL, &Irql);
+ __MacAcquireLockShared(Mac);

status = STATUS_UNSUCCESSFUL;
if (!Mac->Connected)
@@ -284,7 +330,8 @@ MacDumpAddressTable(

ASSERT3U(Index, ==, Count);

- KeReleaseSpinLock(&Mac->Lock, Irql);
+ __MacReleaseLockShared(Mac);
+ KeLowerIrql(Irql);

(VOID) XENBUS_STORE(Remove,
&Mac->StoreInterface,
@@ -328,7 +375,8 @@ fail2:
fail1:
Error("fail1 (%08x)\n", status);

- KeReleaseSpinLock(&Mac->Lock, Irql);
+ __MacReleaseLockExclusive(Mac);
+ KeLowerIrql(Irql);

return status;
}
@@ -344,6 +392,8 @@ MacConnect(
ULONG64 Mtu;
NTSTATUS status;

+ ASSERT3U(KeGetCurrentIrql(), ==, DISPATCH_LEVEL);
+
Frontend = Mac->Frontend;

status = XENBUS_DEBUG(Acquire, &Mac->DebugInterface);
@@ -399,10 +449,12 @@ MacConnect(
if (!NT_SUCCESS(status))
goto fail5;

- KeAcquireSpinLockAtDpcLevel(&Mac->Lock);
+ __MacAcquireLockExclusive(Mac);
+
ASSERT(!Mac->Connected);
Mac->Connected = TRUE;
- KeReleaseSpinLockFromDpcLevel(&Mac->Lock);
+
+ __MacReleaseLockExclusive(Mac);

(VOID) MacDumpAddressTable(Mac);

@@ -456,7 +508,8 @@ MacEnable(
Frontend = Mac->Frontend;

ASSERT3U(KeGetCurrentIrql(), ==, DISPATCH_LEVEL);
- KeAcquireSpinLockAtDpcLevel(&Mac->Lock);
+
+ __MacAcquireLockExclusive(Mac);

Thread = VifGetMacThread(PdoGetVifContext(FrontendGetPdo(Frontend)));

@@ -472,7 +525,7 @@ MacEnable(
ASSERT(!Mac->Enabled);
Mac->Enabled = TRUE;

- KeReleaseSpinLockFromDpcLevel(&Mac->Lock);
+ __MacReleaseLockExclusive(Mac);

Trace("<====\n");
return STATUS_SUCCESS;
@@ -480,7 +533,7 @@ MacEnable(
fail1:
Error("fail1 (%08x)\n");

- KeReleaseSpinLockFromDpcLevel(&Mac->Lock);
+ __MacReleaseLockExclusive(Mac);

return status;
}
@@ -497,7 +550,8 @@ MacDisable(
Frontend = Mac->Frontend;

ASSERT3U(KeGetCurrentIrql(), ==, DISPATCH_LEVEL);
- KeAcquireSpinLockAtDpcLevel(&Mac->Lock);
+
+ __MacAcquireLockExclusive(Mac);

ASSERT(Mac->Enabled);
Mac->Enabled = FALSE;
@@ -507,7 +561,7 @@ MacDisable(
Mac->Watch);
Mac->Watch = NULL;

- KeReleaseSpinLockFromDpcLevel(&Mac->Lock);
+ __MacReleaseLockExclusive(Mac);

Trace("<====\n");
}
@@ -521,10 +575,14 @@ MacDisconnect(

Frontend = Mac->Frontend;

- KeAcquireSpinLockAtDpcLevel(&Mac->Lock);
+ ASSERT3U(KeGetCurrentIrql(), ==, DISPATCH_LEVEL);
+
+ __MacAcquireLockExclusive(Mac);
+
ASSERT(Mac->Connected);
Mac->Connected = FALSE;
- KeReleaseSpinLockFromDpcLevel(&Mac->Lock);
+
+ __MacReleaseLockExclusive(Mac);

XENBUS_DEBUG(Deregister,
&Mac->DebugInterface,
@@ -584,7 +642,7 @@ MacTeardown(
RtlZeroMemory(&Mac->DebugInterface,
sizeof (XENBUS_DEBUG_INTERFACE));

- RtlZeroMemory(&Mac->Lock, sizeof (KSPIN_LOCK));
+ Mac->Lock = 0;

ASSERT(IsZeroMemory(Mac, sizeof (XENVIF_MAC)));
__MacFree(Mac);
@@ -710,10 +768,14 @@ MacAddMulticastAddress(

Multicast->Address = *Address;

- KeAcquireSpinLock(&Mac->Lock, &Irql);
+ KeRaiseIrql(DISPATCH_LEVEL, &Irql);
+ __MacAcquireLockExclusive(Mac);
+
InsertTailList(&Mac->MulticastList, &Multicast->ListEntry);
Mac->MulticastCount++;
- KeReleaseSpinLock(&Mac->Lock, Irql);
+
+ __MacReleaseLockExclusive(Mac);
+ KeLowerIrql(Irql);

(VOID) MacDumpAddressTable(Mac);

@@ -748,7 +810,8 @@ MacRemoveMulticastAddress(

Frontend = Mac->Frontend;

- KeAcquireSpinLock(&Mac->Lock, &Irql);
+ KeRaiseIrql(DISPATCH_LEVEL, &Irql);
+ __MacAcquireLockExclusive(Mac);

for (ListEntry = Mac->MulticastList.Flink;
ListEntry != &Mac->MulticastList;
@@ -773,7 +836,8 @@ found:
RemoveEntryList(&Multicast->ListEntry);
__MacFree(Multicast);

- KeReleaseSpinLock(&Mac->Lock, Irql);
+ __MacReleaseLockExclusive(Mac);
+ KeLowerIrql(Irql);

(VOID) MacDumpAddressTable(Mac);

@@ -791,7 +855,8 @@ found:
fail1:
Error("fail1 (%08x)\n", status);

- KeReleaseSpinLock(&Mac->Lock, Irql);
+ __MacReleaseLockExclusive(Mac);
+ KeLowerIrql(Irql);

return status;
}
@@ -807,7 +872,8 @@ MacQueryMulticastAddresses(
KIRQL Irql;
NTSTATUS status;

- KeAcquireSpinLock(&Mac->Lock, &Irql);
+ KeRaiseIrql(DISPATCH_LEVEL, &Irql);
+ __MacAcquireLockShared(Mac);

status = STATUS_BUFFER_OVERFLOW;
if (Address == NULL || *Count < Mac->MulticastCount)
@@ -827,14 +893,16 @@ MacQueryMulticastAddresses(
}
ASSERT3U(*Count, ==, Mac->MulticastCount);

- KeReleaseSpinLock(&Mac->Lock, Irql);
+ __MacReleaseLockShared(Mac);
+ KeLowerIrql(Irql);

return STATUS_SUCCESS;

fail1:
*Count = Mac->MulticastCount;

- KeReleaseSpinLock(&Mac->Lock, Irql);
+ __MacReleaseLockExclusive(Mac);
+ KeLowerIrql(Irql);

return status;
}
@@ -862,21 +930,25 @@ MacSetFilterLevel(
if (Type >= ETHERNET_ADDRESS_TYPE_COUNT)
goto fail1;

- KeAcquireSpinLock(&Mac->Lock, &Irql);
+ KeRaiseIrql(DISPATCH_LEVEL, &Irql);
+ __MacAcquireLockExclusive(Mac);

status = STATUS_INVALID_PARAMETER;
if (Level > XENVIF_MAC_FILTER_ALL || Level < XENVIF_MAC_FILTER_NONE)
goto fail2;

Mac->FilterLevel[Type] = Level;
- KeReleaseSpinLock(&Mac->Lock, Irql);
+
+ __MacReleaseLockExclusive(Mac);
+ KeLowerIrql(Irql);

return STATUS_SUCCESS;

fail2:
Error("fail2\n");

- KeReleaseSpinLock(&Mac->Lock, Irql);
+ __MacReleaseLockExclusive(Mac);
+ KeLowerIrql(Irql);

fail1:
Error("fail1 (%08x)\n", status);
@@ -891,14 +963,21 @@ MacQueryFilterLevel(
OUT PXENVIF_MAC_FILTER_LEVEL Level
)
{
+ KIRQL Irql;
NTSTATUS status;

status = STATUS_INVALID_PARAMETER;
if (Type >= ETHERNET_ADDRESS_TYPE_COUNT)
goto fail1;

+ KeRaiseIrql(DISPATCH_LEVEL, &Irql);
+ __MacAcquireLockShared(Mac);
+
*Level = Mac->FilterLevel[Type];

+ __MacReleaseLockShared(Mac);
+ KeLowerIrql(Irql);
+
return STATUS_SUCCESS;

fail1:
@@ -915,12 +994,13 @@ MacApplyFilters(
{
ETHERNET_ADDRESS_TYPE Type;
BOOLEAN Allow;
+ KIRQL Irql;

Type = GET_ETHERNET_ADDRESS_TYPE(DestinationAddress);
Allow = FALSE;

- ASSERT3U(KeGetCurrentIrql(), ==, DISPATCH_LEVEL);
- KeAcquireSpinLockAtDpcLevel(&Mac->Lock);
+ KeRaiseIrql(DISPATCH_LEVEL, &Irql);
+ __MacAcquireLockShared(Mac);

switch (Type) {
case ETHERNET_ADDRESS_UNICAST:
@@ -1014,7 +1094,8 @@ MacApplyFilters(
break;
}

- KeReleaseSpinLockFromDpcLevel(&Mac->Lock);
+ __MacReleaseLockShared(Mac);
+ KeLowerIrql(Irql);

return Allow;
}
--
2.5.3
Loading...