From 444eec451190d6edffbbb691d9a7b957fa714fa7 Mon Sep 17 00:00:00 2001 From: Cameron Gutman Date: Sun, 6 Jun 2010 22:08:40 +0000 Subject: [PATCH] [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 svn path=/trunk/; revision=47642 --- reactos/drivers/network/ndis/ndis/time.c | 56 ++++++++++++++++-------- 1 file changed, 38 insertions(+), 18 deletions(-) diff --git a/reactos/drivers/network/ndis/ndis/time.c b/reactos/drivers/network/ndis/ndis/time.c index 772557f5289..0e8cdd46473 100644 --- a/reactos/drivers/network/ndis/ndis/time.c +++ b/reactos/drivers/network/ndis/ndis/time.c @@ -99,6 +99,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 @@ NdisMCancelTimer( * - 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 @@ MiniTimerDpcFunction(PKDPC Dpc, 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 @@ NdisMSetPeriodicTimer( */ { LARGE_INTEGER Timeout; + KIRQL OldIrql; ASSERT_IRQL(DISPATCH_LEVEL); ASSERT(Timer); @@ -231,14 +248,15 @@ NdisMSetPeriodicTimer( /* 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 @@ NdisMSetTimer( */ { LARGE_INTEGER Timeout; + KIRQL OldIrql; ASSERT_IRQL(DISPATCH_LEVEL); ASSERT(Timer); @@ -269,14 +288,15 @@ NdisMSetTimer( /* 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); }