[ros-diffs] [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, 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.
fireball at svn.reactos.org
fireball at svn.reactos.org
Sat Nov 1 16:33:06 CET 2008
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/ke.h?rev=37132&r1=37131&r2=37132&view=diff
==============================================================================
--- 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/ke_x.h?rev=37132&r1=37131&r2=37132&view=diff
==============================================================================
--- 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=37132&r1=37131&r2=37132&view=diff
==============================================================================
--- 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&r1=37131&r2=37132&view=diff
==============================================================================
--- 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=37132&r1=37131&r2=37132&view=diff
==============================================================================
--- 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);
More information about the Ros-diffs
mailing list