|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [win-pv-devel] [PATCH v2 2/2] Update XENBUS_EVTCHN Wait method to avoid races
Use of the XENBUS_EVTCHN method is prone to races in that a client
usually wants to check that something that should be triggered by an
event and, if it has not occurred, wait for the next event.
The problem is that the client may makes its check, the event then occurs,
and then the client waits. Thus the event is missed and the client only
wakes up when the timeout expires.
This patch changes the Wait method to take an explicit event count to wait
for, and adds a method to sample the current event count. A client can
then avoid a race as described above by sampling the event count first,
making its check and then waiting for the event count to increment by one.
If the event occurred between the check and the wait, the wait will now
return immediately.
Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
---
include/evtchn_interface.h | 49 +++++++++++++++++++++++---
include/revision.h | 3 +-
src/xenbus/evtchn.c | 87 +++++++++++++++++++++++++++++++++++++++-------
src/xenbus/store.c | 11 ++++++
4 files changed, 133 insertions(+), 17 deletions(-)
diff --git a/include/evtchn_interface.h b/include/evtchn_interface.h
index 4e696c8..310850b 100644
--- a/include/evtchn_interface.h
+++ b/include/evtchn_interface.h
@@ -175,17 +175,39 @@ typedef VOID
IN PXENBUS_EVTCHN_CHANNEL Channel
);
+/*! \typedef XENBUS_EVTCHN_GET_COUNT
+ \brief Get the number of events received by the channel since it was opened
+
+ \param Interface The interface header
+ \param Channel The channel handle
+ \return The number of events
+*/
+typedef ULONG
+(*XENBUS_EVTCHN_GET_COUNT)(
+ IN PINTERFACE Interface,
+ IN PXENBUS_EVTCHN_CHANNEL Channel
+ );
+
+typedef NTSTATUS
+(*XENBUS_EVTCHN_WAIT_V5)(
+ IN PINTERFACE Interface,
+ IN PXENBUS_EVTCHN_CHANNEL Channel,
+ IN PLARGE_INTEGER Timeout OPTIONAL
+ );
+
/*! \typedef XENBUS_EVTCHN_WAIT
- \brief Wait for an event to the local end of the channel
+ \brief Wait for events to the local end of the channel
\param Interface The interface header
\param Channel The channel handle
+ \param Count The event count to wait for
\param Timeout An optional timeout value (similar to
KeWaitForSingleObject(), but non-zero values are allowed at DISPATCH_LEVEL).
*/
typedef NTSTATUS
(*XENBUS_EVTCHN_WAIT)(
IN PINTERFACE Interface,
IN PXENBUS_EVTCHN_CHANNEL Channel,
+ IN ULONG Count,
IN PLARGE_INTEGER Timeout OPTIONAL
);
@@ -248,7 +270,7 @@ struct _XENBUS_EVTCHN_INTERFACE_V5 {
XENBUS_EVTCHN_UNMASK EvtchnUnmask;
XENBUS_EVTCHN_SEND_V1 EvtchnSendVersion1;
XENBUS_EVTCHN_TRIGGER EvtchnTrigger;
- XENBUS_EVTCHN_WAIT EvtchnWait;
+ XENBUS_EVTCHN_WAIT_V5 EvtchnWaitVersion5;
XENBUS_EVTCHN_GET_PORT EvtchnGetPort;
XENBUS_EVTCHN_CLOSE EvtchnClose;
};
@@ -266,12 +288,31 @@ struct _XENBUS_EVTCHN_INTERFACE_V6 {
XENBUS_EVTCHN_UNMASK EvtchnUnmask;
XENBUS_EVTCHN_SEND EvtchnSend;
XENBUS_EVTCHN_TRIGGER EvtchnTrigger;
+ XENBUS_EVTCHN_WAIT_V5 EvtchnWaitVersion5;
+ XENBUS_EVTCHN_GET_PORT EvtchnGetPort;
+ XENBUS_EVTCHN_CLOSE EvtchnClose;
+};
+
+/*! \struct _XENBUS_EVTCHN_INTERFACE_V7
+ \brief EVTCHN interface version 7
+ \ingroup interfaces
+*/
+struct _XENBUS_EVTCHN_INTERFACE_V7 {
+ INTERFACE Interface;
+ XENBUS_EVTCHN_ACQUIRE EvtchnAcquire;
+ XENBUS_EVTCHN_RELEASE EvtchnRelease;
+ XENBUS_EVTCHN_OPEN EvtchnOpen;
+ XENBUS_EVTCHN_BIND EvtchnBind;
+ XENBUS_EVTCHN_UNMASK EvtchnUnmask;
+ XENBUS_EVTCHN_SEND EvtchnSend;
+ XENBUS_EVTCHN_TRIGGER EvtchnTrigger;
+ XENBUS_EVTCHN_GET_COUNT EvtchnGetCount;
XENBUS_EVTCHN_WAIT EvtchnWait;
XENBUS_EVTCHN_GET_PORT EvtchnGetPort;
XENBUS_EVTCHN_CLOSE EvtchnClose;
};
-typedef struct _XENBUS_EVTCHN_INTERFACE_V6 XENBUS_EVTCHN_INTERFACE,
*PXENBUS_EVTCHN_INTERFACE;
+typedef struct _XENBUS_EVTCHN_INTERFACE_V7 XENBUS_EVTCHN_INTERFACE,
*PXENBUS_EVTCHN_INTERFACE;
/*! \def XENBUS_EVTCHN
\brief Macro at assist in method invocation
@@ -282,7 +323,7 @@ typedef struct _XENBUS_EVTCHN_INTERFACE_V6
XENBUS_EVTCHN_INTERFACE, *PXENBUS_EVT
#endif // _WINDLL
#define XENBUS_EVTCHN_INTERFACE_VERSION_MIN 4
-#define XENBUS_EVTCHN_INTERFACE_VERSION_MAX 6
+#define XENBUS_EVTCHN_INTERFACE_VERSION_MAX 7
#endif // _XENBUS_EVTCHN_INTERFACE_H
diff --git a/include/revision.h b/include/revision.h
index 2aa7dc5..bc73be2 100644
--- a/include/revision.h
+++ b/include/revision.h
@@ -51,6 +51,7 @@
DEFINE_REVISION(0x0800000A, 1, 2, 5, 1, 1, 1, 1, 1, 1, 0, 1), \
DEFINE_REVISION(0x0800000B, 1, 2, 5, 1, 2, 1, 1, 2, 1, 0, 1), \
DEFINE_REVISION(0x09000000, 1, 2, 5, 1, 2, 1, 1, 2, 1, 0, 1), \
- DEFINE_REVISION(0x09000001, 1, 2, 6, 1, 2, 1, 1, 2, 1, 1, 1)
+ DEFINE_REVISION(0x09000001, 1, 2, 6, 1, 2, 1, 1, 2, 1, 1, 1), \
+ DEFINE_REVISION(0x09000002, 1, 2, 7, 1, 2, 1, 1, 2, 1, 1, 1)
#endif // _REVISION_H
diff --git a/src/xenbus/evtchn.c b/src/xenbus/evtchn.c
index 7546f19..f9f95c9 100644
--- a/src/xenbus/evtchn.c
+++ b/src/xenbus/evtchn.c
@@ -81,7 +81,7 @@ struct _XENBUS_EVTCHN_CHANNEL {
PKSERVICE_ROUTINE Callback;
PVOID Argument;
BOOLEAN Active; // Must be tested at >= DISPATCH_LEVEL
- ULONG Events;
+ ULONG Count;
XENBUS_EVTCHN_TYPE Type;
XENBUS_EVTCHN_PARAMETERS Parameters;
BOOLEAN Mask;
@@ -401,7 +401,7 @@ EvtchnReap(
Trace("%u\n", LocalPort);
- Channel->Events = 0;
+ Channel->Count = 0;
ASSERT(Channel->Closed);
Channel->Closed = FALSE;
@@ -507,7 +507,7 @@ EvtchnPoll(
KeMemoryBarrier();
if (!Channel->Closed) {
- Channel->Events++;
+ Channel->Count++;
RemoveEntryList(&Channel->PendingListEntry);
InitializeListHead(&Channel->PendingListEntry);
@@ -898,15 +898,26 @@ EvtchnGetPort(
return Channel->LocalPort;
}
+static ULONG
+EvtchnGetCount(
+ IN PINTERFACE Interface,
+ IN PXENBUS_EVTCHN_CHANNEL Channel
+ )
+{
+ UNREFERENCED_PARAMETER(Interface);
+
+ return Channel->Count;
+}
+
static NTSTATUS
EvtchnWait(
IN PINTERFACE Interface,
IN PXENBUS_EVTCHN_CHANNEL Channel,
+ IN ULONG Count,
IN PLARGE_INTEGER Timeout
)
{
KIRQL Irql;
- ULONG Events;
LARGE_INTEGER Start;
NTSTATUS status;
@@ -915,14 +926,13 @@ EvtchnWait(
ASSERT3U(KeGetCurrentIrql(), <=, DISPATCH_LEVEL);
KeRaiseIrql(DISPATCH_LEVEL, &Irql); // Prevent suspend
- Events = Channel->Events;
- KeMemoryBarrier();
-
KeQuerySystemTime(&Start);
for (;;) {
+ KeMemoryBarrier();
+
status = STATUS_SUCCESS;
- if (Channel->Events != Events)
+ if ((LONG64)Count - (LONG64)Channel->Count <= 0)
break;
if (Timeout != NULL) {
@@ -950,14 +960,35 @@ EvtchnWait(
}
_mm_pause();
- KeMemoryBarrier();
}
+ if (status == STATUS_TIMEOUT)
+ Info("TIMED OUT: Count = %08x Channel->Count = %08x\n",
+ Count,
+ Channel->Count);
+
KeLowerIrql(Irql);
return status;
}
+static NTSTATUS
+EvtchnWaitVersion5(
+ IN PINTERFACE Interface,
+ IN PXENBUS_EVTCHN_CHANNEL Channel,
+ IN PLARGE_INTEGER Timeout
+ )
+{
+ ULONG Count;
+
+ Count = EvtchnGetCount(Interface, Channel);
+
+ return EvtchnWait(Interface,
+ Channel,
+ Count + 1,
+ Timeout);
+}
+
static
_Function_class_(KSERVICE_ROUTINE)
__drv_requiresIRQL(HIGH_LEVEL)
@@ -1369,8 +1400,8 @@ EvtchnDebugCallback(
XENBUS_DEBUG(Printf,
&Context->DebugInterface,
- "Events = %lu\n",
- Channel->Events);
+ "Count = %lu\n",
+ Channel->Count);
}
}
}
@@ -1655,7 +1686,7 @@ static struct _XENBUS_EVTCHN_INTERFACE_V5
EvtchnInterfaceVersion5 = {
EvtchnUnmask,
EvtchnSendVersion1,
EvtchnTrigger,
- EvtchnWait,
+ EvtchnWaitVersion5,
EvtchnGetPort,
EvtchnClose,
};
@@ -1669,6 +1700,21 @@ static struct _XENBUS_EVTCHN_INTERFACE_V6
EvtchnInterfaceVersion6 = {
EvtchnUnmask,
EvtchnSend,
EvtchnTrigger,
+ EvtchnWaitVersion5,
+ EvtchnGetPort,
+ EvtchnClose,
+};
+
+static struct _XENBUS_EVTCHN_INTERFACE_V7 EvtchnInterfaceVersion7 = {
+ { sizeof (struct _XENBUS_EVTCHN_INTERFACE_V7), 7, NULL, NULL, NULL },
+ EvtchnAcquire,
+ EvtchnRelease,
+ EvtchnOpen,
+ EvtchnBind,
+ EvtchnUnmask,
+ EvtchnSend,
+ EvtchnTrigger,
+ EvtchnGetCount,
EvtchnWait,
EvtchnGetPort,
EvtchnClose,
@@ -1833,6 +1879,23 @@ EvtchnGetInterface(
status = STATUS_SUCCESS;
break;
}
+ case 7: {
+ struct _XENBUS_EVTCHN_INTERFACE_V7 *EvtchnInterface;
+
+ EvtchnInterface = (struct _XENBUS_EVTCHN_INTERFACE_V7 *)Interface;
+
+ status = STATUS_BUFFER_OVERFLOW;
+ if (Size < sizeof (struct _XENBUS_EVTCHN_INTERFACE_V7))
+ break;
+
+ *EvtchnInterface = EvtchnInterfaceVersion7;
+
+ ASSERT3U(Interface->Version, ==, Version);
+ Interface->Context = Context;
+
+ status = STATUS_SUCCESS;
+ break;
+ }
default:
status = STATUS_NOT_SUPPORTED;
break;
diff --git a/src/xenbus/store.c b/src/xenbus/store.c
index 86bbcb8..101dc40 100644
--- a/src/xenbus/store.c
+++ b/src/xenbus/store.c
@@ -900,6 +900,7 @@ StoreSubmitRequest(
{
PXENBUS_STORE_RESPONSE Response;
KIRQL Irql;
+ ULONG Count;
LARGE_INTEGER Timeout;
ASSERT3U(Request->State, ==, XENBUS_STORE_REQUEST_PREPARED);
@@ -913,6 +914,11 @@ StoreSubmitRequest(
InsertTailList(&Context->SubmittedList, &Request->ListEntry);
Request->State = XENBUS_STORE_REQUEST_SUBMITTED;
+
+ Count = XENBUS_EVTCHN(GetCount,
+ &Context->EvtchnInterface,
+ Context->Channel);
+
StorePollLocked(Context);
KeMemoryBarrier();
@@ -924,10 +930,15 @@ StoreSubmitRequest(
status = XENBUS_EVTCHN(Wait,
&Context->EvtchnInterface,
Context->Channel,
+ Count + 1,
&Timeout);
if (status == STATUS_TIMEOUT)
Warning("TIMED OUT\n");
+ Count = XENBUS_EVTCHN(GetCount,
+ &Context->EvtchnInterface,
+ Context->Channel);
+
StorePollLocked(Context);
KeMemoryBarrier();
}
--
2.5.3
_______________________________________________
win-pv-devel mailing list
win-pv-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/cgi-bin/mailman/listinfo/win-pv-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |