[ion] 17057: - Optimize locking so that the Wake Lock is only entered after a first attempt determines the timer is a wake timer.
- Optimize locking so that the Wake Lock is only entered after a first attempt determines the timer is a wake timer. After the lock, a second check is done to make sure. However, since the first check will fail 99% of times, we will not acquire/release a spinlock, and thus decrease contention. - Fix Object Type Initializer to actually use ExpTimerDelete when the object is deleted. - Fix Initializer to report correct memory usage of timer object. - Fix Initializer to report OBJ_OPENLINK as an invalid attribute for timer objects. - Use correct access masks when modifying or querying timer objects. - Handle wake timers in NtCancelTimer. - Return warning NTSTATUS code if a wake timer is requested but not supported by the system (default on ROS). - Check for valid timer type in NtCreateTimer. - Check for valid period in NtSetTimer. - Don't dereference the timer in NtSetTimer three times. - Return the correct Due Time in NtQueryTimer by substracting the stable interrupt time. - Harmonize formatting, fix some comments. Modified: trunk/reactos/ntoskrnl/ex/timer.c _____
Modified: trunk/reactos/ntoskrnl/ex/timer.c --- trunk/reactos/ntoskrnl/ex/timer.c 2005-08-05 06:13:43 UTC (rev 17056) +++ trunk/reactos/ntoskrnl/ex/timer.c 2005-08-05 06:53:56 UTC (rev 17057) @@ -2,23 +2,23 @@
* COPYRIGHT: See COPYING in the top level directory * PROJECT: ReactOS kernel * FILE: ntoskrnl/ex/timer.c - * PURPOSE: User-mode timers + * PURPOSE: Executive Timer Implementation * - * PROGRAMMERS: Alex Ionescu (alex@relsoft.net) - Reimplemented + * PROGRAMMERS: Alex Ionescu (alex@relsoft.net) * David Welch (welch@mcmail.com) */
/* INCLUDES *****************************************************************/
#include <ntoskrnl.h> - #define NDEBUG #include <internal/debug.h>
/* TYPES ********************************************************************/
/* Executive Timer Object */ -typedef struct _ETIMER { +typedef struct _ETIMER +{ KTIMER KeTimer; KAPC TimerApc; KDPC TimerDpc; @@ -38,7 +38,8 @@ LIST_ENTRY ExpWakeList;
/* Timer Mapping */ -static GENERIC_MAPPING ExpTimerMapping = { +static GENERIC_MAPPING ExpTimerMapping = +{ STANDARD_RIGHTS_READ | TIMER_QUERY_STATE, STANDARD_RIGHTS_WRITE | TIMER_MODIFY_STATE, STANDARD_RIGHTS_EXECUTE | SYNCHRONIZE, @@ -46,8 +47,8 @@ };
/* Timer Information Classes */ -static const INFORMATION_CLASS_INFO ExTimerInfoClass[] = { - +static const INFORMATION_CLASS_INFO ExTimerInfoClass[] = +{ /* TimerBasicInformation */ ICI_SQ_SAME( sizeof(TIMER_BASIC_INFORMATION), sizeof(ULONG), ICIF_QUERY ), }; @@ -68,42 +69,42 @@
while (!IsListEmpty(&Thread->ActiveTimerListHead)) { - /* Remove a Timer */ - CurrentEntry = RemoveTailList(&Thread->ActiveTimerListHead); + CurrentEntry = RemoveTailList(&Thread->ActiveTimerListHead);
/* Get the Timer */ Timer = CONTAINING_RECORD(CurrentEntry, ETIMER, ActiveTimerListEntry); + DPRINT("Timer, ThreadList: 0x%p, 0x%p\n", Timer, Thread);
+ /* Mark it as deassociated */ ASSERT (Timer->ApcAssociated); - Timer->ApcAssociated = FALSE; + Timer->ApcAssociated = FALSE;
- DPRINT("Timer, ThreadList: 0x%p, 0x%p\n", Timer, Thread); - /* Unlock the list */ KeReleaseSpinLockFromDpcLevel(&Thread->ActiveTimerListLock);
/* Lock the Timer */ KeAcquireSpinLockAtDpcLevel(&Timer->Lock);
- ASSERT (&Thread->Tcb == Timer->TimerApc.Thread); - + /* Cancel the timer and remove its DPC and APC */ + ASSERT(&Thread->Tcb == Timer->TimerApc.Thread); KeCancelTimer(&Timer->KeTimer); KeRemoveQueueDpc(&Timer->TimerDpc); KeRemoveQueueApc(&Timer->TimerApc);
- /* Unlock the Timer */ KeReleaseSpinLock(&Timer->Lock, OldIrql);
- /* Dereference it, if needed */ + /* Dereference it */ ObDereferenceObject(Timer);
/* Loop again */ KeAcquireSpinLock(&Thread->ActiveTimerListLock, &OldIrql); }
+ /* Release lock and return */ KeReleaseSpinLock(&Thread->ActiveTimerListLock, OldIrql); + return; }
VOID @@ -112,24 +113,27 @@ { KIRQL OldIrql; PETIMER Timer = ObjectBody; - DPRINT("ExpDeleteTimer(Timer: 0x%p)\n", Timer);
- /* Lock the Wake List */ - KeAcquireSpinLock(&ExpWakeListLock, &OldIrql); - /* Check if it has a Wait List */ - if (Timer->WakeTimer) { + if (Timer->WakeTimer) + { + /* Lock the Wake List */ + KeAcquireSpinLock(&ExpWakeListLock, &OldIrql);
- /* Remove it from the Wait List */ - DPRINT("Removing wake list\n"); - RemoveEntryList(&Timer->WakeTimerListEntry); - Timer->WakeTimer = FALSE; + /* Check again, since it might've changed before we locked */ + if (Timer->WakeTimer) + { + /* Remove it from the Wait List */ + DPRINT("Removing wake list\n"); + RemoveEntryList(&Timer->WakeTimerListEntry); + Timer->WakeTimer = FALSE; + } + + /* Release the Wake List */ + KeReleaseSpinLock(&ExpWakeListLock, OldIrql); }
- /* Release the Wake List */ - KeReleaseSpinLock(&ExpWakeListLock, OldIrql); - /* Tell the Kernel to cancel the Timer */ DPRINT("Cancelling Timer\n"); KeCancelTimer(&Timer->KeTimer); @@ -154,8 +158,8 @@ KeAcquireSpinLock(&Timer->Lock, &OldIrql);
/* Queue the APC */ - if(Timer->ApcAssociated) { - + if(Timer->ApcAssociated) + { DPRINT("Queuing APC\n"); KeInsertQueueApc(&Timer->TimerApc, SystemArgument1, @@ -165,6 +169,7 @@
/* Release the Timer */ KeReleaseSpinLock(&Timer->Lock, OldIrql); + return; }
@@ -196,8 +201,8 @@ */ if ((Timer->ApcAssociated) && (&CurrentThread->Tcb == Timer->TimerApc.Thread) && - (!Timer->KeTimer.Period)) { - + (!Timer->KeTimer.Period)) + { /* Remove it from the Active Timers List */ DPRINT("Removing Timer\n"); RemoveEntryList(&Timer->ActiveTimerListEntry); @@ -233,11 +238,12 @@ RtlZeroMemory(&ObjectTypeInitializer, sizeof(ObjectTypeInitializer)); RtlInitUnicodeString(&Name, L"Timer"); ObjectTypeInitializer.Length = sizeof(ObjectTypeInitializer); - ObjectTypeInitializer.DefaultNonPagedPoolCharge = sizeof(KTIMER); + ObjectTypeInitializer.InvalidAttributes = OBJ_OPENLINK; + ObjectTypeInitializer.DefaultNonPagedPoolCharge = sizeof(ETIMER); ObjectTypeInitializer.GenericMapping = ExpTimerMapping; ObjectTypeInitializer.PoolType = NonPagedPool; ObjectTypeInitializer.ValidAccessMask = TIMER_ALL_ACCESS; - ObjectTypeInitializer.UseDefaultObject = TRUE; + ObjectTypeInitializer.DeleteProcedure = ExpDeleteTimer; ObpCreateTypeObject(&ObjectTypeInitializer, &Name, &ExTimerType);
/* Initialize the Wait List and Lock */ @@ -245,7 +251,6 @@ InitializeListHead(&ExpWakeList); }
- NTSTATUS STDCALL NtCancelTimer(IN HANDLE TimerHandle, @@ -258,44 +263,46 @@ PETHREAD TimerThread; BOOLEAN KillTimer = FALSE; NTSTATUS Status = STATUS_SUCCESS; - PAGED_CODE(); DPRINT("NtCancelTimer(0x%p, 0x%x)\n", TimerHandle, CurrentState);
/* Check Parameter Validity */ - if(CurrentState != NULL && PreviousMode != KernelMode) { - _SEH_TRY { + if(CurrentState && PreviousMode != KernelMode) + { + _SEH_TRY + { ProbeForWrite(CurrentState, sizeof(BOOLEAN), sizeof(BOOLEAN)); - } _SEH_EXCEPT(_SEH_ExSystemExceptionFilter) { + } + _SEH_EXCEPT(_SEH_ExSystemExceptionFilter) + { Status = _SEH_GetExceptionCode(); - } _SEH_END; + } + _SEH_END;
- if(!NT_SUCCESS(Status)) { - return Status; - } + if(!NT_SUCCESS(Status)) return Status; }
/* Get the Timer Object */ Status = ObReferenceObjectByHandle(TimerHandle, - TIMER_ALL_ACCESS, + TIMER_MODIFY_STATE, ExTimerType, PreviousMode, (PVOID*)&Timer, NULL);
/* Check for success */ - if(NT_SUCCESS(Status)) { - + if(NT_SUCCESS(Status)) + { DPRINT("Timer Referenced: 0x%p\n", Timer);
/* Lock the Timer */ KeAcquireSpinLock(&Timer->Lock, &OldIrql);
/* Check if it's enabled */ - if (Timer->ApcAssociated) { - + if (Timer->ApcAssociated) + { /* * First, remove it from the Thread's Active List * Get the Thread. @@ -318,39 +325,58 @@ KeRemoveQueueApc(&Timer->TimerApc); Timer->ApcAssociated = FALSE; KillTimer = TRUE; - - } else { - + } + else + { /* If timer was disabled, we still need to cancel it */ DPRINT("APC was not Associated. Cancelling Timer\n"); KeCancelTimer(&Timer->KeTimer); }
+ /* Handle a Wake Timer */ + if (Timer->WakeTimer) + { + /* Lock the Wake List */ + KeAcquireSpinLockAtDpcLevel(&ExpWakeListLock); + + /* Check again, since it might've changed before we locked */ + if (Timer->WakeTimer) + { + /* Remove it from the Wait List */ + DPRINT("Removing wake list\n"); + RemoveEntryList(&Timer->WakeTimerListEntry); + Timer->WakeTimer = FALSE; + } + + /* Release the Wake List */ + KeReleaseSpinLockFromDpcLevel(&ExpWakeListLock); + } + + /* Unlock the Timer */ + KeReleaseSpinLock(&Timer->Lock, OldIrql); + /* Read the old State */ State = KeReadStateTimer(&Timer->KeTimer);
/* Dereference the Object */ ObDereferenceObject(Timer);
- /* Unlock the Timer */ - KeReleaseSpinLock(&Timer->Lock, OldIrql); - /* Dereference if it was previously enabled */ if (KillTimer) ObDereferenceObject(Timer); DPRINT1("Timer disabled\n");
/* Make sure it's safe to write to the handle */ - if(CurrentState != NULL) { - - _SEH_TRY { - + if(CurrentState) + { + _SEH_TRY + { *CurrentState = State; - - } _SEH_EXCEPT(_SEH_ExSystemExceptionFilter) { - + } + _SEH_EXCEPT(_SEH_ExSystemExceptionFilter) + { Status = _SEH_GetExceptionCode(); - - } _SEH_END; + } + _SEH_END; } }
@@ -358,7 +384,6 @@ return Status; }
- NTSTATUS STDCALL NtCreateTimer(OUT PHANDLE TimerHandle, @@ -370,41 +395,48 @@ HANDLE hTimer; KPROCESSOR_MODE PreviousMode = ExGetPreviousMode(); NTSTATUS Status = STATUS_SUCCESS; - PAGED_CODE(); DPRINT("NtCreateTimer(Handle: 0x%p, Type: %d)\n", TimerHandle, TimerType);
/* Check Parameter Validity */ - if (PreviousMode != KernelMode) { - - _SEH_TRY { - + if (PreviousMode != KernelMode) + { + _SEH_TRY + { ProbeForWrite(TimerHandle, sizeof(HANDLE), sizeof(ULONG)); - } _SEH_EXCEPT(_SEH_ExSystemExceptionFilter) { - + } + _SEH_EXCEPT(_SEH_ExSystemExceptionFilter) + { Status = _SEH_GetExceptionCode(); + } + _SEH_END;
- } _SEH_END; - if(!NT_SUCCESS(Status)) return Status; }
+ /* Check for correct timer type */ + if ((TimerType != NotificationTimer) && (TimerType != SynchronizationTimer)) + { + DPRINT1("Invalid Timer Type!\n"); + return STATUS_INVALID_PARAMETER_4; + } + /* Create the Object */ Status = ObCreateObject(PreviousMode, - ExTimerType, - ObjectAttributes, - PreviousMode, - NULL, - sizeof(ETIMER), - 0, - 0, - (PVOID*)&Timer); + ExTimerType, + ObjectAttributes, + PreviousMode, + NULL, + sizeof(ETIMER), + 0, + 0, + (PVOID*)&Timer);
/* Check for Success */ - if(NT_SUCCESS(Status)) { - + if(NT_SUCCESS(Status)) + { /* Initialize the Kernel Timer */ DPRINT("Initializing Timer: 0x%p\n", Timer); KeInitializeTimerEx(&Timer->KeTimer, TimerType); @@ -428,24 +460,22 @@ &hTimer); DPRINT("Timer Inserted\n");
- /* Make sure it's safe to write to the handle */ - _SEH_TRY { - + _SEH_TRY + { *TimerHandle = hTimer; - - } _SEH_EXCEPT(_SEH_ExSystemExceptionFilter) { - + } + _SEH_EXCEPT(_SEH_ExSystemExceptionFilter) + { Status = _SEH_GetExceptionCode(); - - } _SEH_END; + } + _SEH_END; }
/* Return to Caller */ return Status; }
- NTSTATUS STDCALL NtOpenTimer(OUT PHANDLE TimerHandle, @@ -455,25 +485,24 @@ HANDLE hTimer; KPROCESSOR_MODE PreviousMode = ExGetPreviousMode(); NTSTATUS Status = STATUS_SUCCESS; - PAGED_CODE(); DPRINT("NtOpenTimer(TimerHandle: 0x%p)\n", TimerHandle);
/* Check Parameter Validity */ - if (PreviousMode != KernelMode) { - - _SEH_TRY { - + if (PreviousMode != KernelMode) + { + _SEH_TRY + { ProbeForWrite(TimerHandle, sizeof(HANDLE), sizeof(ULONG)); - - } _SEH_EXCEPT(_SEH_ExSystemExceptionFilter) { - + } + _SEH_EXCEPT(_SEH_ExSystemExceptionFilter) + { Status = _SEH_GetExceptionCode(); + } + _SEH_END;
- } _SEH_END; - if(!NT_SUCCESS(Status)) return Status; }
@@ -487,18 +516,18 @@ &hTimer);
/* Check for success */ - if(NT_SUCCESS(Status)) { - + if(NT_SUCCESS(Status)) + { /* Make sure it's safe to write to the handle */ - _SEH_TRY { - + _SEH_TRY + { *TimerHandle = hTimer; - - } _SEH_EXCEPT(_SEH_ExSystemExceptionFilter) { - + } + _SEH_EXCEPT(_SEH_ExSystemExceptionFilter) + { Status = _SEH_GetExceptionCode(); - - } _SEH_END; + } + _SEH_END; }
/* Return to Caller */ @@ -518,7 +547,6 @@ KPROCESSOR_MODE PreviousMode = ExGetPreviousMode(); NTSTATUS Status = STATUS_SUCCESS; PTIMER_BASIC_INFORMATION BasicInfo = (PTIMER_BASIC_INFORMATION)TimerInformation; - PAGED_CODE(); DPRINT("NtQueryTimer(TimerHandle: 0x%p, Class: %d)\n", TimerHandle, TimerInformationClass);
@@ -530,8 +558,8 @@ ReturnLength, PreviousMode, &Status); - if(!NT_SUCCESS(Status)) { - + if(!NT_SUCCESS(Status)) + { DPRINT1("NtQueryTimer() failed, Status: 0x%x\n", Status); return Status; } @@ -545,24 +573,30 @@ NULL);
/* Check for Success */ - if(NT_SUCCESS(Status)) { - + if(NT_SUCCESS(Status)) + { /* Return the Basic Information */ - _SEH_TRY { + _SEH_TRY + { + /* Return the remaining time, corrected */ + BasicInfo->TimeRemaining.QuadPart = Timer->KeTimer.DueTime.QuadPart - + KeQueryInterruptTime();
- /* FIXME: Interrupt correction based on Interrupt Time */ - DPRINT("Returning Information for Timer: 0x%p. Time Remaining: %I64x\n", Timer, Timer->KeTimer.DueTime.QuadPart); - BasicInfo->TimeRemaining.QuadPart = Timer->KeTimer.DueTime.QuadPart; + /* Return the current state */ BasicInfo->SignalState = KeReadStateTimer(&Timer->KeTimer);
+ /* Return the buffer length if requested */ if(ReturnLength != NULL) *ReturnLength = sizeof(TIMER_BASIC_INFORMATION);
- } _SEH_EXCEPT(_SEH_ExSystemExceptionFilter) { - + DPRINT("Returning Information for Timer: 0x%p. Time Remaining: %I64x\n", + Timer, BasicInfo->TimeRemaining.QuadPart); + } + _SEH_EXCEPT(_SEH_ExSystemExceptionFilter) + { Status = _SEH_GetExceptionCode(); + } + _SEH_END;
- } _SEH_END; - /* Dereference Object */ ObDereferenceObject(Timer); } @@ -590,54 +624,69 @@ PETHREAD TimerThread; BOOLEAN KillTimer = FALSE; NTSTATUS Status = STATUS_SUCCESS; - PAGED_CODE(); - DPRINT("NtSetTimer(TimerHandle: 0x%p, DueTime: %I64x, Apc: 0x%p, Period: %d)\n", TimerHandle, DueTime->QuadPart, TimerApcRoutine, Period); + DPRINT("NtSetTimer(TimerHandle: 0x%p, DueTime: %I64x, Apc: 0x%p, Period: %d)\n", + TimerHandle, DueTime->QuadPart, TimerApcRoutine, Period);
/* Check Parameter Validity */ - if (PreviousMode != KernelMode) { - - _SEH_TRY { - + if (PreviousMode != KernelMode) + { + _SEH_TRY + { ProbeForRead(DueTime, sizeof(LARGE_INTEGER), sizeof(ULONG)); TimerDueTime = *DueTime;
- if(PreviousState != NULL) { - + if(PreviousState) + { ProbeForWrite(PreviousState, sizeof(BOOLEAN), sizeof(BOOLEAN)); } - - } _SEH_EXCEPT(_SEH_ExSystemExceptionFilter) { - + } + _SEH_EXCEPT(_SEH_ExSystemExceptionFilter) + { Status = _SEH_GetExceptionCode(); + } + _SEH_END;
- } _SEH_END; - if(!NT_SUCCESS(Status)) return Status; }
+ /* Check for a valid Period */ + if (Period < 0) + { + DPRINT1("Invalid Period for timer\n"); + return STATUS_INVALID_PARAMETER_6; + } + /* Get the Timer Object */ Status = ObReferenceObjectByHandle(TimerHandle, - TIMER_ALL_ACCESS, + TIMER_MODIFY_STATE, ExTimerType, PreviousMode, (PVOID*)&Timer, NULL);
+ /* + * Tell the user we don't support Wake Timers... + * when we have the ability to use/detect the Power Management + * functionatliy required to support them, make this check dependent + * on the actual PM capabilities + */ + if (WakeTimer) Status = STATUS_TIMER_RESUME_IGNORED; + /* Check status */ - if (NT_SUCCESS(Status)) { - + if (NT_SUCCESS(Status)) + { /* Lock the Timer */ DPRINT("Timer Referencced: 0x%p\n", Timer); KeAcquireSpinLock(&Timer->Lock, &OldIrql);
/* Cancel Running Timer */ - if (Timer->ApcAssociated) { - + if (Timer->ApcAssociated) + { /* * First, remove it from the Thread's Active List * Get the Thread. @@ -674,13 +723,14 @@ /* Handle Wake Timers */ DPRINT("Doing Wake Semantics\n"); KeAcquireSpinLockAtDpcLevel(&ExpWakeListLock); - if (WakeTimer && !Timer->WakeTimer) { - + if (WakeTimer && !Timer->WakeTimer) + { /* Insert it into the list */ Timer->WakeTimer = TRUE; InsertTailList(&ExpWakeList, &Timer->WakeTimerListEntry); - } else if (!WakeTimer && Timer->WakeTimer) { - + } + else if (!WakeTimer && Timer->WakeTimer) + { /* Remove it from the list */ RemoveEntryList(&Timer->WakeTimerListEntry); Timer->WakeTimer = FALSE; @@ -688,8 +738,8 @@ KeReleaseSpinLockFromDpcLevel(&ExpWakeListLock);
/* Set up the APC Routine if specified */ - if (TimerApcRoutine) { - + if (TimerApcRoutine) + { /* Initialize the APC */ DPRINT("Initializing APC: 0x%p\n", Timer->TimerApc); KeInitializeApc(&Timer->TimerApc, @@ -720,26 +770,23 @@ /* Unlock the Timer */ KeReleaseSpinLock(&Timer->Lock, OldIrql);
- /* Dereference the Object */ - ObDereferenceObject(Timer); - /* Dereference if it was previously enabled */ if (!TimerApcRoutine) ObDereferenceObject(Timer); if (KillTimer) ObDereferenceObject(Timer); DPRINT("Finished Setting the Timer\n");
/* Make sure it's safe to write to the handle */ - if(PreviousState != NULL) { - - _SEH_TRY { - + if(PreviousState != NULL) + { + _SEH_TRY + { *PreviousState = State; - - } _SEH_EXCEPT(_SEH_ExSystemExceptionFilter) { - + } + _SEH_EXCEPT(_SEH_ExSystemExceptionFilter) + { Status = _SEH_GetExceptionCode(); - - } _SEH_END; + } + _SEH_END; } }
-
ion＠svn.reactos.com