Author: cgutman
Date: Mon Jun 7 00:08:40 2010
New Revision: 47642
URL: http://svn.reactos.org/svn/reactos?rev=47642&view=rev
Log:
[NDIS]
- Hold the miniport lock when we work with the timer queue
- Use the return value of KeSetTimer(Ex) to determine whether we need to queue the timer in our queue, otherwise we just use the entry that is already there
- Add more assertions
Modified:
trunk/reactos/drivers/network/ndis/ndis/time.c
Modified: trunk/reactos/drivers/network/ndis/ndis/time.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/ndis/ndis/…
==============================================================================
--- trunk/reactos/drivers/network/ndis/ndis/time.c [iso-8859-1] (original)
+++ trunk/reactos/drivers/network/ndis/ndis/time.c [iso-8859-1] Mon Jun 7 00:08:40 2010
@@ -98,6 +98,8 @@
BOOLEAN DequeueMiniportTimer(PNDIS_MINIPORT_TIMER Timer)
{
PNDIS_MINIPORT_TIMER CurrentTimer;
+
+ ASSERT(KeGetCurrentIrql() == DISPATCH_LEVEL);
if (!Timer->Miniport->TimerQueue)
return FALSE;
@@ -143,13 +145,21 @@
* - call at IRQL <= DISPATCH_LEVEL
*/
{
+ KIRQL OldIrql;
+
ASSERT_IRQL(DISPATCH_LEVEL);
ASSERT(TimerCancelled);
ASSERT(Timer);
*TimerCancelled = KeCancelTimer (&Timer->Timer);
- DequeueMiniportTimer(Timer);
+ if (*TimerCancelled)
+ {
+ KeAcquireSpinLock(&Timer->Miniport->Lock, &OldIrql);
+ /* If it's somebody already dequeued it, something is wrong (maybe a double-cancel?) */
+ if (!DequeueMiniportTimer(Timer)) ASSERT(FALSE);
+ KeReleaseSpinLock(&Timer->Miniport->Lock, OldIrql);
+ }
}
VOID NTAPI
@@ -166,7 +176,13 @@
SystemArgument2);
/* Only dequeue if the timer has a period of 0 */
- if (!Timer->Timer.Period) DequeueMiniportTimer(Timer);
+ if (!Timer->Timer.Period)
+ {
+ KeAcquireSpinLockAtDpcLevel(&Timer->Miniport->Lock);
+ /* If someone already dequeued it, something is wrong (borked timer implementation?) */
+ if (!DequeueMiniportTimer(Timer)) ASSERT(FALSE);
+ KeReleaseSpinLockFromDpcLevel(&Timer->Miniport->Lock);
+ }
}
@@ -224,6 +240,7 @@
*/
{
LARGE_INTEGER Timeout;
+ KIRQL OldIrql;
ASSERT_IRQL(DISPATCH_LEVEL);
ASSERT(Timer);
@@ -231,14 +248,15 @@
/* relative delays are negative, absolute are positive; resolution is 100ns */
Timeout.QuadPart = Int32x32To64(MillisecondsPeriod, -10000);
- /* Dequeue the timer if it is queued already */
- DequeueMiniportTimer(Timer);
-
- /* Add the timer at the head of the timer queue */
- Timer->NextDeferredTimer = Timer->Miniport->TimerQueue;
- Timer->Miniport->TimerQueue = Timer;
-
- KeSetTimerEx (&Timer->Timer, Timeout, MillisecondsPeriod, &Timer->Dpc);
+ KeAcquireSpinLock(&Timer->Miniport->Lock, &OldIrql);
+ /* If KeSetTimer(Ex) returns FALSE then the timer is not in the system's queue (and not in ours either) */
+ if (!KeSetTimerEx(&Timer->Timer, Timeout, MillisecondsPeriod, &Timer->Dpc))
+ {
+ /* Add the timer at the head of the timer queue */
+ Timer->NextDeferredTimer = Timer->Miniport->TimerQueue;
+ Timer->Miniport->TimerQueue = Timer;
+ }
+ KeReleaseSpinLock(&Timer->Miniport->Lock, OldIrql);
}
@@ -262,6 +280,7 @@
*/
{
LARGE_INTEGER Timeout;
+ KIRQL OldIrql;
ASSERT_IRQL(DISPATCH_LEVEL);
ASSERT(Timer);
@@ -269,14 +288,15 @@
/* relative delays are negative, absolute are positive; resolution is 100ns */
Timeout.QuadPart = Int32x32To64(MillisecondsToDelay, -10000);
- /* Dequeue the timer if it is queued already */
- DequeueMiniportTimer(Timer);
-
- /* Add the timer at the head of the timer queue */
- Timer->NextDeferredTimer = Timer->Miniport->TimerQueue;
- Timer->Miniport->TimerQueue = Timer;
-
- KeSetTimer (&Timer->Timer, Timeout, &Timer->Dpc);
+ KeAcquireSpinLock(&Timer->Miniport->Lock, &OldIrql);
+ /* If KeSetTimer(Ex) returns FALSE then the timer is not in the system's queue (and not in ours either) */
+ if (!KeSetTimer(&Timer->Timer, Timeout, &Timer->Dpc))
+ {
+ /* Add the timer at the head of the timer queue */
+ Timer->NextDeferredTimer = Timer->Miniport->TimerQueue;
+ Timer->Miniport->TimerQueue = Timer;
+ }
+ KeReleaseSpinLock(&Timer->Miniport->Lock, OldIrql);
}
Author: cgutman
Date: Sun Jun 6 21:18:19 2010
New Revision: 47636
URL: http://svn.reactos.org/svn/reactos?rev=47636&view=rev
Log:
[NDIS]
- Only dequeue the timer in the DPC if the Period is 0 (which means that it's NOT a periodic timer so we only get called once)
- Attempt to dequeue the timer before inserting it so we don't end up with multiple copies of the same timer on the timer queue if somebody calls NdisMSet(Periodic)Timer twice
Modified:
trunk/reactos/drivers/network/ndis/ndis/time.c
Modified: trunk/reactos/drivers/network/ndis/ndis/time.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/ndis/ndis/…
==============================================================================
--- trunk/reactos/drivers/network/ndis/ndis/time.c [iso-8859-1] (original)
+++ trunk/reactos/drivers/network/ndis/ndis/time.c [iso-8859-1] Sun Jun 6 21:18:19 2010
@@ -95,7 +95,7 @@
KeInitializeDpc (&Timer->Dpc, (PKDEFERRED_ROUTINE)TimerFunction, FunctionContext);
}
-VOID DequeueMiniportTimer(PNDIS_MINIPORT_TIMER Timer)
+BOOLEAN DequeueMiniportTimer(PNDIS_MINIPORT_TIMER Timer)
{
PNDIS_MINIPORT_TIMER CurrentTimer;
@@ -104,6 +104,8 @@
if (Timer->Miniport->TimerQueue == Timer)
{
Timer->Miniport->TimerQueue = Timer->NextDeferredTimer;
+ Timer->NextDeferredTimer = NULL;
+ return TRUE;
}
else
{
@@ -113,11 +115,12 @@
if (CurrentTimer->NextDeferredTimer == Timer)
{
CurrentTimer->NextDeferredTimer = Timer->NextDeferredTimer;
- return;
+ Timer->NextDeferredTimer = NULL;
+ return TRUE;
}
CurrentTimer = CurrentTimer->NextDeferredTimer;
}
- ASSERT(FALSE);
+ return FALSE;
}
}
@@ -161,8 +164,8 @@
SystemArgument1,
SystemArgument2);
- /* FIXME: We can't call this if we have a periodic timer */
- //DequeueMiniportTimer(Timer);
+ /* Only dequeue if the timer has a period of 0 */
+ if (!Timer->Timer.Period) DequeueMiniportTimer(Timer);
}
@@ -227,6 +230,9 @@
/* relative delays are negative, absolute are positive; resolution is 100ns */
Timeout.QuadPart = Int32x32To64(MillisecondsPeriod, -10000);
+ /* Dequeue the timer if it is queued already */
+ DequeueMiniportTimer(Timer);
+
/* Add the timer at the head of the timer queue */
Timer->NextDeferredTimer = Timer->Miniport->TimerQueue;
Timer->Miniport->TimerQueue = Timer;
@@ -261,6 +267,9 @@
/* relative delays are negative, absolute are positive; resolution is 100ns */
Timeout.QuadPart = Int32x32To64(MillisecondsToDelay, -10000);
+
+ /* Dequeue the timer if it is queued already */
+ DequeueMiniportTimer(Timer);
/* Add the timer at the head of the timer queue */
Timer->NextDeferredTimer = Timer->Miniport->TimerQueue;