[fireball] 37132: - Refactor timer code to share more macros (see http://www.dcl.hpi.uni-potsdam.de/research/WRK/?p=32). - Implement expiring timers when the user changes the system time. Previously these timers would never expire if the time was set past their expiration points. - Fix a bug in KiInsertTreeTimer where, if a timer expired while we inserted it, we kept its Inserted state to TRUE. - Fix a bug where, when changing the system time, the modifications were not done in the correct order
Author: fireball Date: Sat Nov 1 10:33:05 2008 New Revision: 37132
URL: http://svn.reactos.org/svn/reactos?rev=37132&view=rev Log: - Refactor timer code to share more macros (see http://www.dcl.hpi.uni-potsdam.de/research/WRK/?p=32). - Implement expiring timers when the user changes the system time. Previously these timers would never expire if the time was set past their expiration points. - Fix a bug in KiInsertTreeTimer where, if a timer expired while we inserted it, we kept its Inserted state to TRUE. - Fix a bug where, when changing the system time, the modifications were not done in the correct order, possibly resulting in a race condition happening if someone else was checking the time simultaneously. - Tested with winetest kernel32 timer. - Thanks to Alex and Stefan.
Modified: trunk/reactos/ntoskrnl/include/internal/ke.h trunk/reactos/ntoskrnl/include/internal/ke_x.h trunk/reactos/ntoskrnl/ke/clock.c trunk/reactos/ntoskrnl/ke/dpc.c trunk/reactos/ntoskrnl/ke/timerobj.c
Modified: trunk/reactos/ntoskrnl/include/internal/ke.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/k... ============================================================================== --- trunk/reactos/ntoskrnl/include/internal/ke.h [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/include/internal/ke.h [iso-8859-1] Sat Nov 1 10:33:05 2008 @@ -286,6 +286,13 @@ IN ULONG Hand );
+VOID +FASTCALL +KiTimerListExpire( + IN PLIST_ENTRY ExpiredListHead, + IN KIRQL OldIrql +); + BOOLEAN FASTCALL KiInsertTreeTimer(
Modified: trunk/reactos/ntoskrnl/include/internal/ke_x.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/k... ============================================================================== --- trunk/reactos/ntoskrnl/include/internal/ke_x.h [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/include/internal/ke_x.h [iso-8859-1] Sat Nov 1 10:33:05 2008 @@ -1100,6 +1100,37 @@ }
// +// Called from KiCompleteTimer, KiInsertTreeTimer, KeSetSystemTime +// to remove timer entries +// See Windows HPI blog for more information. +VOID +FORCEINLINE +KiRemoveEntryTimer(IN PKTIMER Timer) +{ + ULONG Hand; + PKTIMER_TABLE_ENTRY TableEntry; + + /* Remove the timer from the timer list and check if it's empty */ + Hand = Timer->Header.Hand; + if (RemoveEntryList(&Timer->TimerListEntry)) + { + /* Get the respective timer table entry */ + TableEntry = &KiTimerTableListHead[Hand]; + if (&TableEntry->Entry == TableEntry->Entry.Flink) + { + /* Set the entry to an infinite absolute time */ + TableEntry->Time.HighPart = 0xFFFFFFFF; + } + } + + /* Clear the list entries on dbg builds so we can tell the timer is gone */ +#if DBG + Timer->TimerListEntry.Flink = NULL; + Timer->TimerListEntry.Blink = NULL; +#endif +} + +// // Called by Wait and Queue code to insert a timer for dispatching. // Also called by KeSetTimerEx to insert a timer from the caller. // @@ -1125,6 +1156,57 @@ /* Do nothing, just release the lock */ KiReleaseTimerLock(LockQueue); } +} + +// +// Called by KeSetTimerEx and KiInsertTreeTimer to calculate Due Time +// See the Windows HPI Blog for more information +// +BOOLEAN +FORCEINLINE +KiComputeDueTime(IN PKTIMER Timer, + IN LARGE_INTEGER DueTime, + OUT PULONG Hand) +{ + LARGE_INTEGER InterruptTime, SystemTime, DifferenceTime; + + /* Convert to relative time if needed */ + Timer->Header.Absolute = FALSE; + if (DueTime.HighPart >= 0) + { + /* Get System Time */ + KeQuerySystemTime(&SystemTime); + + /* Do the conversion */ + DifferenceTime.QuadPart = SystemTime.QuadPart - DueTime.QuadPart; + + /* Make sure it hasn't already expired */ + Timer->Header.Absolute = TRUE; + if (DifferenceTime.HighPart >= 0) + { + /* Cancel everything */ + Timer->Header.SignalState = TRUE; + Timer->Header.Hand = 0; + Timer->DueTime.QuadPart = 0; + *Hand = 0; + return FALSE; + } + + /* Set the time as Absolute */ + DueTime = DifferenceTime; + } + + /* Get the Interrupt Time */ + InterruptTime.QuadPart = KeQueryInterruptTime(); + + /* Recalculate due time */ + Timer->DueTime.QuadPart = InterruptTime.QuadPart - DueTime.QuadPart; + + /* Get the handle */ + *Hand = KiComputeTimerTableIndex(Timer->DueTime.QuadPart); + Timer->Header.Hand = (UCHAR)*Hand; + Timer->Header.Inserted = TRUE; + return TRUE; }
//
Modified: trunk/reactos/ntoskrnl/ke/clock.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/clock.c?rev=371... ============================================================================== --- trunk/reactos/ntoskrnl/ke/clock.c [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/ke/clock.c [iso-8859-1] Sat Nov 1 10:33:05 2008 @@ -40,7 +40,6 @@ PKSPIN_LOCK_QUEUE LockQueue; LIST_ENTRY TempList, TempList2; ULONG Hand, i; - PKTIMER_TABLE_ENTRY TimerEntry;
/* Sanity checks */ ASSERT((NewTime->HighPart & 0xF0000000) == 0); @@ -57,10 +56,10 @@ /* Query the system time now */ KeQuerySystemTime(OldTime);
- /* Set the new system time */ + /* Set the new system time (ordering of these operations is critical) */ + SharedUserData->SystemTime.High2Time = NewTime->HighPart; SharedUserData->SystemTime.LowPart = NewTime->LowPart; SharedUserData->SystemTime.High1Time = NewTime->HighPart; - SharedUserData->SystemTime.High2Time = NewTime->HighPart;
/* Check if this was for the HAL and set the RTC time */ if (HalTime) ExCmosClockIsSane = HalSetRealTimeClock(&TimeFields); @@ -98,16 +97,7 @@ if (Timer->Header.Absolute) { /* Remove it from the timer list */ - if (RemoveEntryList(&Timer->TimerListEntry)) - { - /* Get the entry and check if it's empty */ - TimerEntry = &KiTimerTableListHead[Timer->Header.Hand]; - if (IsListEmpty(&TimerEntry->Entry)) - { - /* Clear the time then */ - TimerEntry->Time.HighPart = 0xFFFFFFFF; - } - } + KiRemoveEntryTimer(Timer);
/* Insert it into our temporary list */ InsertTailList(&TempList, &Timer->TimerListEntry); @@ -138,16 +128,7 @@ if (KiInsertTimerTable(Timer, Hand)) { /* Remove it from the timer list */ - if (RemoveEntryList(&Timer->TimerListEntry)) - { - /* Get the entry and check if it's empty */ - TimerEntry = &KiTimerTableListHead[Timer->Header.Hand]; - if (IsListEmpty(&TimerEntry->Entry)) - { - /* Clear the time then */ - TimerEntry->Time.HighPart = 0xFFFFFFFF; - } - } + KiRemoveEntryTimer(Timer);
/* Insert it into our temporary list */ InsertTailList(&TempList2, &Timer->TimerListEntry); @@ -157,8 +138,8 @@ KiReleaseTimerLock(LockQueue); }
- /* FIXME: Process expired timers! */ - KiReleaseDispatcherLock(OldIrql); + /* Process expired timers. This releases the dispatcher lock. */ + KiTimerListExpire(&TempList2, OldIrql);
/* Revert affinity */ KeRevertToUserAffinityThread();
Modified: trunk/reactos/ntoskrnl/ke/dpc.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/dpc.c?rev=37132... ============================================================================== --- trunk/reactos/ntoskrnl/ke/dpc.c [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/ke/dpc.c [iso-8859-1] Sat Nov 1 10:33:05 2008 @@ -86,7 +86,6 @@ LONG Limit, Index, i; ULONG Timers, ActiveTimers, DpcCalls; PLIST_ENTRY ListHead, NextEntry; - PKTIMER_TABLE_ENTRY TimerEntry; KIRQL OldIrql; PKTIMER Timer; PKDPC TimerDpc; @@ -147,16 +146,7 @@ { /* It's expired, remove it */ ActiveTimers--; - if (RemoveEntryList(&Timer->TimerListEntry)) - { - /* Get the entry and check if it's empty */ - TimerEntry = &KiTimerTableListHead[Timer->Header.Hand]; - if (IsListEmpty(&TimerEntry->Entry)) - { - /* Clear the time then */ - TimerEntry->Time.HighPart = 0xFFFFFFFF; - } - } + KiRemoveEntryTimer(Timer);
/* Make it non-inserted, unlock it, and signal it */ Timer->Header.Inserted = FALSE; @@ -192,6 +182,7 @@ }
/* Check if we have a DPC */ +#ifndef CONFIG_SMP if (TimerDpc) { /* Setup the DPC Entry */ @@ -199,7 +190,11 @@ DpcEntry[DpcCalls].Routine = TimerDpc->DeferredRoutine; DpcEntry[DpcCalls].Context = TimerDpc->DeferredContext; DpcCalls++; + ASSERT(DpcCalls < MAX_TIMER_DPCS); } +#else +#error MP Case: Need to check the DPC target CPU so see if we can piggyback +#endif
/* Check if we're done processing */ if (!(ActiveTimers) || !(Timers)) @@ -295,6 +290,108 @@
/* Lower IRQL if we need to */ if (OldIrql != DISPATCH_LEVEL) KeLowerIrql(OldIrql); + } + else + { + /* Unlock the dispatcher */ + KiReleaseDispatcherLock(OldIrql); + } +} + +VOID +FASTCALL +KiTimerListExpire(IN PLIST_ENTRY ExpiredListHead, + IN KIRQL OldIrql) +{ + ULARGE_INTEGER SystemTime; + LARGE_INTEGER Interval; + LONG i; + ULONG DpcCalls = 0; + PKTIMER Timer; + PKDPC TimerDpc; + ULONG Period; + DPC_QUEUE_ENTRY DpcEntry[MAX_TIMER_DPCS]; + + /* Query system */ + KeQuerySystemTime((PLARGE_INTEGER)&SystemTime); + + /* Loop expired list */ + while (ExpiredListHead->Flink != ExpiredListHead) + { + /* Get the current timer */ + Timer = CONTAINING_RECORD(ExpiredListHead->Flink, KTIMER, TimerListEntry); + + /* Remove it */ + RemoveEntryList(&Timer->TimerListEntry); + + /* Not inserted */ + Timer->Header.Inserted = FALSE; + + /* Signal it */ + Timer->Header.SignalState = 1; + + /* Get the DPC and period */ + TimerDpc = Timer->Dpc; + Period = Timer->Period; + + /* Check if there's any waiters */ + if (!IsListEmpty(&Timer->Header.WaitListHead)) + { + /* Check the type of event */ + if (Timer->Header.Type == TimerNotificationObject) + { + /* Unwait the thread */ + KxUnwaitThread(&Timer->Header, IO_NO_INCREMENT); + } + else + { + /* Otherwise unwait the thread and signal the timer */ + KxUnwaitThreadForEvent((PKEVENT)Timer, IO_NO_INCREMENT); + } + } + + /* Check if we have a period */ + if (Period) + { + /* Calculate the interval and insert the timer */ + Interval.QuadPart = Int32x32To64(Period, -10000); + while (!KiInsertTreeTimer(Timer, Interval)); + } + + /* Check if we have a DPC */ +#ifndef CONFIG_SMP + if (TimerDpc) + { + /* Setup the DPC Entry */ + DpcEntry[DpcCalls].Dpc = TimerDpc; + DpcEntry[DpcCalls].Routine = TimerDpc->DeferredRoutine; + DpcEntry[DpcCalls].Context = TimerDpc->DeferredContext; + DpcCalls++; + ASSERT(DpcCalls < MAX_TIMER_DPCS); + } +#else +#error MP Case: Need to check the DPC target CPU so see if we can piggyback +#endif + } + + /* Check if we still have DPC entries */ + if (DpcCalls) + { + /* Release the dispatcher while doing DPCs */ + KiReleaseDispatcherLock(DISPATCH_LEVEL); + + /* Start looping all DPC Entries */ + for (i = 0; DpcCalls; DpcCalls--, i++) + { + /* Call the DPC */ + DpcEntry[i].Routine(DpcEntry[i].Dpc, + DpcEntry[i].Context, + UlongToPtr(SystemTime.LowPart), + UlongToPtr(SystemTime.HighPart)); + } + + /* Lower IRQL */ + KeLowerIrql(OldIrql); } else {
Modified: trunk/reactos/ntoskrnl/ke/timerobj.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/timerobj.c?rev=... ============================================================================== --- trunk/reactos/ntoskrnl/ke/timerobj.c [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/ke/timerobj.c [iso-8859-1] Sat Nov 1 10:33:05 2008 @@ -29,71 +29,29 @@ BOOLEAN Inserted = FALSE; ULONG Hand = 0; PKSPIN_LOCK_QUEUE LockQueue; - LONGLONG DueTime; - LARGE_INTEGER InterruptTime, SystemTime, DifferenceTime; - PKTIMER_TABLE_ENTRY TimerEntry; - DPRINT("KiInsertTreeTimer(): Timer %p, Interval: %I64d\n", Timer, Interval.QuadPart);
- /* Convert to relative time if needed */ - Timer->Header.Absolute = FALSE; - if (Interval.HighPart >= 0) - { - /* Get System Time */ - KeQuerySystemTime(&SystemTime); - - /* Do the conversion */ - DifferenceTime.QuadPart = SystemTime.QuadPart - Interval.QuadPart; - - /* Make sure it hasn't already expired */ - Timer->Header.Absolute = TRUE; - if (DifferenceTime.HighPart >= 0) + /* Setup the timer's due time */ + if (KiComputeDueTime(Timer, Interval, &Hand)) + { + /* Acquire the lock */ + LockQueue = KiAcquireTimerLock(Hand); + + /* Insert the timer */ + if (KiInsertTimerTable(Timer, Hand)) { - /* Cancel everything */ - Timer->Header.SignalState = TRUE; - Timer->Header.Hand = 0; - Timer->DueTime.QuadPart = 0; - return FALSE; + /* It was already there, remove it */ + KiRemoveEntryTimer(Timer); + Timer->Header.Inserted = FALSE; } - - /* Set the time as Absolute */ - Interval = DifferenceTime; - } - - /* Get the Interrupt Time */ - InterruptTime.QuadPart = KeQueryInterruptTime(); - - /* Recalculate due time */ - DueTime = InterruptTime.QuadPart - Interval.QuadPart; - Timer->DueTime.QuadPart = DueTime; - - /* Get the handle */ - Hand = KiComputeTimerTableIndex(DueTime); - Timer->Header.Hand = (UCHAR)Hand; - Timer->Header.Inserted = TRUE; - - /* Acquire the lock */ - LockQueue = KiAcquireTimerLock(Hand); - - /* Insert the timer */ - if (KiInsertTimerTable(Timer, Hand)) - { - /* It was already there, remove it */ - if (RemoveEntryList(&Timer->TimerListEntry)) + else { - /* Get the entry and check if it's empty */ - TimerEntry = &KiTimerTableListHead[Hand]; - if (IsListEmpty(&TimerEntry->Entry)) - { - /* Clear the time then */ - TimerEntry->Time.HighPart = 0xFFFFFFFF; - } + /* Otherwise, we're now inserted */ + Inserted = TRUE; } - } - else - { - /* Otherwise, we're now inserted */ - Inserted = TRUE; + + /* Release the lock */ + KiReleaseTimerLock(LockQueue); }
/* Release the lock and return insert status */ @@ -110,7 +68,6 @@ BOOLEAN Expired = FALSE; PLIST_ENTRY ListHead, NextEntry; PKTIMER CurrentTimer; - DPRINT("KiInsertTimerTable(): Timer %p, Hand: %d\n", Timer, Hand);
/* Check if the period is zero */ @@ -160,7 +117,6 @@ PKDPC Dpc = Timer->Dpc; ULONG Period = Timer->Period; LARGE_INTEGER Interval, SystemTime; - DPRINT("KiSignalTimer(): Timer %p\n", Timer);
/* Set default values */ @@ -212,22 +168,11 @@ IN PKSPIN_LOCK_QUEUE LockQueue) { LIST_ENTRY ListHead; - PKTIMER_TABLE_ENTRY TimerEntry; BOOLEAN RequestInterrupt = FALSE; - DPRINT("KiCompleteTimer(): Timer %p, LockQueue: %p\n", Timer, LockQueue);
/* Remove it from the timer list */ - if (RemoveEntryList(&Timer->TimerListEntry)) - { - /* Get the entry and check if it's empty */ - TimerEntry = &KiTimerTableListHead[Timer->Header.Hand]; - if (IsListEmpty(&TimerEntry->Entry)) - { - /* Clear the time then */ - TimerEntry->Time.HighPart = 0xFFFFFFFF; - } - } + KiRemoveEntryTimer(Timer);
/* Link the timer list to our stack */ ListHead.Flink = &Timer->TimerListEntry; @@ -264,7 +209,6 @@ BOOLEAN Inserted; ASSERT_TIMER(Timer); ASSERT(KeGetCurrentIrql() <= DISPATCH_LEVEL); - DPRINT("KeCancelTimer(): Timer %p\n", Timer);
/* Lock the Database and Raise IRQL */ @@ -301,7 +245,8 @@ IN TIMER_TYPE Type) { DPRINT("KeInitializeTimerEx(): Timer %p, Type %s\n", - Timer, (Type == NotificationTimer) ? "NotificationTimer" : "SynchronizationTimer"); + Timer, (Type == NotificationTimer) ? + "NotificationTimer" : "SynchronizationTimer");
/* Initialize the Dispatch Header */ KeInitializeDispatcherHeader(&Timer->Header, @@ -352,13 +297,11 @@ KIRQL OldIrql; BOOLEAN Inserted; ULONG Hand = 0; - LARGE_INTEGER InterruptTime, SystemTime, DifferenceTime; BOOLEAN RequestInterrupt = FALSE; ASSERT_TIMER(Timer); ASSERT(KeGetCurrentIrql() <= DISPATCH_LEVEL); - DPRINT("KeSetTimerEx(): Timer %p, DueTime %I64d, Period %d, Dpc %p\n", - Timer, DueTime.QuadPart, Period, Dpc); + Timer, DueTime.QuadPart, Period, Dpc);
/* Lock the Database and Raise IRQL */ OldIrql = KiAcquireDispatcherLock(); @@ -370,59 +313,24 @@ /* Set Default Timer Data */ Timer->Dpc = Dpc; Timer->Period = Period; - - /* Convert to relative time if needed */ - Timer->Header.Absolute = FALSE; - if (DueTime.HighPart >= 0) - { - /* Get System Time */ - KeQuerySystemTime(&SystemTime); - - /* Do the conversion */ - DifferenceTime.QuadPart = SystemTime.QuadPart - DueTime.QuadPart; - - /* Make sure it hasn't already expired */ - Timer->Header.Absolute = TRUE; - if (DifferenceTime.HighPart >= 0) - { - /* Cancel everything */ - Timer->Header.SignalState = TRUE; - Timer->Header.Hand = 0; - Timer->DueTime.QuadPart = 0; - - /* Signal the timer */ - RequestInterrupt = KiSignalTimer(Timer); - - /* Release the dispatcher lock */ - KiReleaseDispatcherLockFromDpcLevel(); - - /* Check if we need to do an interrupt */ - if (RequestInterrupt) HalRequestSoftwareInterrupt(DISPATCH_LEVEL); - - /* Exit the dispatcher and return the old state */ - KiExitDispatcher(OldIrql); - return Inserted; - } - - /* Set the time as Absolute */ - DueTime = DifferenceTime; - } - - /* Get the Interrupt Time */ - InterruptTime.QuadPart = KeQueryInterruptTime(); - - /* Recalculate due time */ - Timer->DueTime.QuadPart = InterruptTime.QuadPart - DueTime.QuadPart; - - /* Get the handle */ - Hand = KiComputeTimerTableIndex(Timer->DueTime.QuadPart); - Timer->Header.Hand = (UCHAR)Hand; - Timer->Header.Inserted = TRUE; - - /* Insert the timer */ - Timer->Header.SignalState = FALSE; - KxInsertTimer(Timer, Hand); - + if (!KiComputeDueTime(Timer, DueTime, &Hand)) + { + /* Signal the timer */ + RequestInterrupt = KiSignalTimer(Timer); + + /* Release the dispatcher lock */ + KiReleaseDispatcherLockFromDpcLevel(); + + /* Check if we need to do an interrupt */ + if (RequestInterrupt) HalRequestSoftwareInterrupt(DISPATCH_LEVEL); + } + else + { + /* Insert the timer */ + Timer->Header.SignalState = FALSE; + KxInsertTimer(Timer, Hand); + } + /* Exit the dispatcher */ KiExitDispatcher(OldIrql);
-
fireball＠svn.reactos.org