From a0d7a72c1a59859136ea6626f6c74795edee2839 Mon Sep 17 00:00:00 2001 From: Alex Ionescu Date: Mon, 22 Jan 2007 06:47:44 +0000 Subject: [PATCH] - Implement a cute little hack called DEFINE_WAIT_BLOCK which makes pushlocks work on GCC 3.4.5 as well as 4.1.2+ (with no perf-hit on the latter). - Implement ExWaitForUnblockPushLock (just a wrapper around ExTimedWaitForUnblockPushLock). - Simplfy ExBlockPushLock and fix some bugs. - Fix a bug in ExfReleasePushLockExclusive when we have to wake the lock. - Fix a bug in ExfUnblockPushLock which was touching the wrong pointer. - Fix ExWaitOnPushLock to verify that the pushlock is actually locked. svn path=/trunk/; revision=25584 --- reactos/ntoskrnl/ex/init.c | 14 +-- reactos/ntoskrnl/ex/pushlock.c | 164 +++++++++++++++---------- reactos/ntoskrnl/include/internal/ex.h | 61 ++++++++- 3 files changed, 161 insertions(+), 78 deletions(-) diff --git a/reactos/ntoskrnl/ex/init.c b/reactos/ntoskrnl/ex/init.c index 0b0efd029c8..ef8d6cb8f19 100644 --- a/reactos/ntoskrnl/ex/init.c +++ b/reactos/ntoskrnl/ex/init.c @@ -1,11 +1,11 @@ /* -* PROJECT: ReactOS Kernel -* LICENSE: GPL - See COPYING in the top level directory -* FILE: ntoskrnl/ex/init.c -* PURPOSE: Executive Initialization Code -* PROGRAMMERS: Alex Ionescu (alex.ionescu@reactos.org) -* Eric Kohl (ekohl@rz-online.de) -*/ + * PROJECT: ReactOS Kernel + * LICENSE: GPL - See COPYING in the top level directory + * FILE: ntoskrnl/ex/init.c + * PURPOSE: Executive Initialization Code + * PROGRAMMERS: Alex Ionescu (alex.ionescu@reactos.org) + * Eric Kohl (ekohl@rz-online.de) + */ /* INCLUDES ******************************************************************/ diff --git a/reactos/ntoskrnl/ex/pushlock.c b/reactos/ntoskrnl/ex/pushlock.c index 80103eaa68b..f2a943b7209 100644 --- a/reactos/ntoskrnl/ex/pushlock.c +++ b/reactos/ntoskrnl/ex/pushlock.c @@ -341,6 +341,33 @@ ExTimedWaitForUnblockPushLock(IN PEX_PUSH_LOCK PushLock, return Status; } +/*++ + * @name ExWaitForUnblockPushLock + * + * The ExWaitForUnblockPushLock routine waits for a pushlock + * to be unblocked, for a specified internal. + * + * @param PushLock + * Pointer to a pushlock whose waiter list needs to be optimized. + * + * @param WaitBlock + * Pointer to the pushlock's wait block. + * + * @return STATUS_SUCCESS is the pushlock is now unblocked, otherwise the error + * code returned by KeWaitForSingleObject. + * + * @remarks If the wait fails, then a manual unblock is attempted. + * + *--*/ +VOID +FASTCALL +ExWaitForUnblockPushLock(IN PEX_PUSH_LOCK PushLock, + IN PEX_PUSH_LOCK_WAIT_BLOCK WaitBlock) +{ + /* Call the timed function with no timeout */ + ExTimedWaitForUnblockPushLock(PushLock, WaitBlock, NULL); +} + /*++ * @name ExBlockPushLock * @@ -360,25 +387,33 @@ ExTimedWaitForUnblockPushLock(IN PEX_PUSH_LOCK PushLock, VOID FASTCALL ExBlockPushLock(PEX_PUSH_LOCK PushLock, - PVOID WaitBlock) + PVOID pWaitBlock) { - PVOID NewValue, OldValue; + PEX_PUSH_LOCK_WAIT_BLOCK WaitBlock = pWaitBlock; + EX_PUSH_LOCK NewValue, OldValue; + + /* Detect invalid wait block alignment */ + ASSERT((ULONG_PTR)pWaitBlock & 0x10); /* Set the waiting bit */ - ((PEX_PUSH_LOCK_WAIT_BLOCK)WaitBlock)->Flags |= EX_PUSH_LOCK_FLAGS_WAIT; + WaitBlock->Flags = EX_PUSH_LOCK_FLAGS_WAIT; + + /* Get the old value */ + OldValue = *PushLock; - /* Link the wait blocks */ - ((PEX_PUSH_LOCK_WAIT_BLOCK)WaitBlock)->Next = PushLock->Ptr; - - /* Try to set this one as the wait block now */ - NewValue = PushLock->Ptr; + /* Start block loop */ for (;;) { + /* Link the wait blocks */ + WaitBlock->Next = OldValue.Ptr; + /* Set the new wait block value */ - OldValue = InterlockedCompareExchangePointer(&PushLock->Ptr, - WaitBlock, - NewValue); - if (OldValue == NewValue) break; + NewValue.Ptr = InterlockedCompareExchangePointer(&PushLock->Ptr, + WaitBlock, + OldValue.Ptr); + if (OldValue.Ptr == NewValue.Ptr) break; + + /* Try again with the new value */ NewValue = OldValue; } } @@ -404,7 +439,7 @@ VOID FASTCALL ExfAcquirePushLockExclusive(PEX_PUSH_LOCK PushLock) { - EX_PUSH_LOCK_WAIT_BLOCK WaitBlock; + DEFINE_WAIT_BLOCK(WaitBlock); EX_PUSH_LOCK OldValue = *PushLock, NewValue, TempValue; BOOLEAN NeedWake; ULONG i; @@ -435,23 +470,23 @@ ExfAcquirePushLockExclusive(PEX_PUSH_LOCK PushLock) else { /* We'll have to create a Waitblock */ - WaitBlock.Flags = EX_PUSH_LOCK_FLAGS_EXCLUSIVE | - EX_PUSH_LOCK_FLAGS_WAIT; - WaitBlock.Previous = NULL; + WaitBlock->Flags = EX_PUSH_LOCK_FLAGS_EXCLUSIVE | + EX_PUSH_LOCK_FLAGS_WAIT; + WaitBlock->Previous = NULL; NeedWake = FALSE; /* Check if there is already a waiter */ if (OldValue.Waiting) { /* Nobody is the last waiter yet */ - WaitBlock.Last = NULL; + WaitBlock->Last = NULL; /* We are an exclusive waiter */ - WaitBlock.ShareCount = 0; + WaitBlock->ShareCount = 0; /* Set the current Wait Block pointer */ - WaitBlock.Next = (PEX_PUSH_LOCK_WAIT_BLOCK)((ULONG_PTR) - OldValue.Ptr &~ EX_PUSH_LOCK_PTR_BITS); + WaitBlock->Next = (PEX_PUSH_LOCK_WAIT_BLOCK)((ULONG_PTR) + OldValue.Ptr &~ EX_PUSH_LOCK_PTR_BITS); /* Point to ours */ NewValue.Value = (OldValue.Value & EX_PUSH_LOCK_MULTIPLE_SHARED) | @@ -466,10 +501,10 @@ ExfAcquirePushLockExclusive(PEX_PUSH_LOCK PushLock) else { /* We are the first waiter, so loop the wait block */ - WaitBlock.Last = &WaitBlock; + WaitBlock->Last = WaitBlock; /* Set the share count */ - WaitBlock.ShareCount = OldValue.Shared; + WaitBlock->ShareCount = OldValue.Shared; /* Check if someone is sharing this pushlock */ if (OldValue.Shared > 1) @@ -483,7 +518,7 @@ ExfAcquirePushLockExclusive(PEX_PUSH_LOCK PushLock) else { /* No shared count */ - WaitBlock.ShareCount = 0; + WaitBlock->ShareCount = 0; /* Point to our wait block */ NewValue.Value = EX_PUSH_LOCK_LOCK | @@ -494,10 +529,10 @@ ExfAcquirePushLockExclusive(PEX_PUSH_LOCK PushLock) #if DBG /* Setup the Debug Wait Block */ - WaitBlock.Signaled = 0; - WaitBlock.OldValue = OldValue; - WaitBlock.NewValue = NewValue; - WaitBlock.PushLock = PushLock; + WaitBlock->Signaled = 0; + WaitBlock->OldValue = OldValue; + WaitBlock->NewValue = NewValue; + WaitBlock->PushLock = PushLock; #endif /* Sanity check */ @@ -524,26 +559,26 @@ ExfAcquirePushLockExclusive(PEX_PUSH_LOCK PushLock) } /* Set up the Wait Gate */ - KeInitializeGate(&WaitBlock.WakeGate); + KeInitializeGate(&WaitBlock->WakeGate); /* Now spin on the push lock if necessary */ i = ExPushLockSpinCount; - if ((i) && (WaitBlock.Flags & EX_PUSH_LOCK_WAITING)) + if ((i) && (WaitBlock->Flags & EX_PUSH_LOCK_WAITING)) { /* Spin */ while (--i) YieldProcessor(); } /* Now try to remove the wait bit */ - if (InterlockedBitTestAndReset(&WaitBlock.Flags, 1)) + if (InterlockedBitTestAndReset(&WaitBlock->Flags, 1)) { /* Nobody removed it already, let's do a full wait */ - KeWaitForGate(&WaitBlock.WakeGate, WrPushLock, KernelMode); - ASSERT(WaitBlock.Signaled); + KeWaitForGate(&WaitBlock->WakeGate, WrPushLock, KernelMode); + ASSERT(WaitBlock->Signaled); } /* We shouldn't be shared anymore */ - ASSERT((WaitBlock.ShareCount == 0)); + ASSERT((WaitBlock->ShareCount == 0)); /* Loop again */ OldValue = NewValue; @@ -570,7 +605,7 @@ VOID FASTCALL ExfAcquirePushLockShared(PEX_PUSH_LOCK PushLock) { - EX_PUSH_LOCK_WAIT_BLOCK WaitBlock; + DEFINE_WAIT_BLOCK(WaitBlock); EX_PUSH_LOCK OldValue = *PushLock, NewValue; BOOLEAN NeedWake; ULONG i; @@ -614,20 +649,20 @@ ExfAcquirePushLockShared(PEX_PUSH_LOCK PushLock) else { /* We'll have to create a Waitblock */ - WaitBlock.Flags = EX_PUSH_LOCK_FLAGS_WAIT; - WaitBlock.ShareCount = 0; + WaitBlock->Flags = EX_PUSH_LOCK_FLAGS_WAIT; + WaitBlock->ShareCount = 0; NeedWake = FALSE; - WaitBlock.Previous = NULL; + WaitBlock->Previous = NULL; /* Check if there is already a waiter */ if (OldValue.Waiting) { /* Set the current Wait Block pointer */ - WaitBlock.Next = (PEX_PUSH_LOCK_WAIT_BLOCK)((ULONG_PTR) - OldValue.Ptr &~ EX_PUSH_LOCK_PTR_BITS); + WaitBlock->Next = (PEX_PUSH_LOCK_WAIT_BLOCK)((ULONG_PTR) + OldValue.Ptr &~ EX_PUSH_LOCK_PTR_BITS); /* Nobody is the last waiter yet */ - WaitBlock.Last = NULL; + WaitBlock->Last = NULL; /* Point to ours */ NewValue.Value = (OldValue.Value & (EX_PUSH_LOCK_MULTIPLE_SHARED | @@ -642,7 +677,7 @@ ExfAcquirePushLockShared(PEX_PUSH_LOCK PushLock) else { /* We are the first waiter, so loop the wait block */ - WaitBlock.Last = &WaitBlock; + WaitBlock->Last = WaitBlock; /* Point to our wait block */ NewValue.Value = (OldValue.Value & (EX_PUSH_LOCK_MULTIPLE_SHARED | @@ -656,10 +691,10 @@ ExfAcquirePushLockShared(PEX_PUSH_LOCK PushLock) #if DBG /* Setup the Debug Wait Block */ - WaitBlock.Signaled = 0; - WaitBlock.OldValue = OldValue; - WaitBlock.NewValue = NewValue; - WaitBlock.PushLock = PushLock; + WaitBlock->Signaled = 0; + WaitBlock->OldValue = OldValue; + WaitBlock->NewValue = NewValue; + WaitBlock->PushLock = PushLock; #endif /* Write the new value */ @@ -683,26 +718,26 @@ ExfAcquirePushLockShared(PEX_PUSH_LOCK PushLock) } /* Set up the Wait Gate */ - KeInitializeGate(&WaitBlock.WakeGate); + KeInitializeGate(&WaitBlock->WakeGate); /* Now spin on the push lock if necessary */ i = ExPushLockSpinCount; - if ((i) && (WaitBlock.Flags & EX_PUSH_LOCK_WAITING)) + if ((i) && (WaitBlock->Flags & EX_PUSH_LOCK_WAITING)) { /* Spin */ while (--i) YieldProcessor(); } /* Now try to remove the wait bit */ - if (InterlockedBitTestAndReset(&WaitBlock.Flags, 1)) + if (InterlockedBitTestAndReset(&WaitBlock->Flags, 1)) { /* Fast-path did not work, we need to do a full wait */ - KeWaitForGate(&WaitBlock.WakeGate, WrPushLock, KernelMode); - ASSERT(WaitBlock.Signaled); + KeWaitForGate(&WaitBlock->WakeGate, WrPushLock, KernelMode); + ASSERT(WaitBlock->Signaled); } /* We shouldn't be shared anymore */ - ASSERT((WaitBlock.ShareCount == 0)); + ASSERT((WaitBlock->ShareCount == 0)); } } } @@ -1004,7 +1039,7 @@ VOID FASTCALL ExfReleasePushLockExclusive(PEX_PUSH_LOCK PushLock) { - EX_PUSH_LOCK NewValue; + EX_PUSH_LOCK NewValue, WakeValue; EX_PUSH_LOCK OldValue = *PushLock; /* Loop until we can change */ @@ -1024,7 +1059,8 @@ ExfReleasePushLockExclusive(PEX_PUSH_LOCK PushLock) /* Sanity check */ ASSERT(NewValue.Waking && !NewValue.Locked); - /* Write the New Value */ + /* Write the New Value. Save our original value for waking */ + WakeValue = NewValue; NewValue.Ptr = InterlockedCompareExchangePointer(PushLock, NewValue.Ptr, OldValue.Ptr); @@ -1032,14 +1068,10 @@ ExfReleasePushLockExclusive(PEX_PUSH_LOCK PushLock) /* Check if the value changed behind our back */ if (NewValue.Value != OldValue.Value) { - /* Loop again */ - OldValue = NewValue; - continue; + /* Wake the Pushlock */ + ExfWakePushLock(PushLock, WakeValue); + break; } - - /* Wake the Pushlock */ - ExfWakePushLock(PushLock, NewValue); - break; } else { @@ -1056,10 +1088,10 @@ ExfReleasePushLockExclusive(PEX_PUSH_LOCK PushLock) /* Check if the value changed behind our back */ if (NewValue.Value == OldValue.Value) break; - - /* Loop again */ - OldValue = NewValue; } + + /* Loop again */ + OldValue = NewValue; } } @@ -1128,7 +1160,7 @@ ExfUnblockPushLock(PEX_PUSH_LOCK PushLock, KIRQL OldIrql = DISPATCH_LEVEL; /* Get the wait block and erase the previous one */ - WaitBlock = InterlockedExchangePointer(PushLock->Ptr, 0); + WaitBlock = InterlockedExchangePointer(&PushLock->Ptr, NULL); if (WaitBlock) { /* Check if there is a linked pushlock and raise IRQL appropriately */ @@ -1144,7 +1176,7 @@ ExfUnblockPushLock(PEX_PUSH_LOCK PushLock, if (InterlockedBitTestAndReset(&WaitBlock->Flags, 1)) { /* Nobody removed the flag before us, so signal the event */ - KeSetEventBoostPriority(&WaitBlock->WakeEvent, IO_NO_INCREMENT); + KeSetEventBoostPriority(&WaitBlock->WakeEvent, NULL); } /* Check if there was a next block */ @@ -1161,6 +1193,6 @@ ExfUnblockPushLock(PEX_PUSH_LOCK PushLock, EX_PUSH_LOCK_FLAGS_WAIT)) { /* Wait for the pushlock to be unblocked */ - ExTimedWaitForUnblockPushLock(PushLock, CurrentWaitBlock, NULL); + ExWaitForUnblockPushLock(PushLock, CurrentWaitBlock); } } diff --git a/reactos/ntoskrnl/include/internal/ex.h b/reactos/ntoskrnl/include/internal/ex.h index 6581d013486..7951e525ff0 100644 --- a/reactos/ntoskrnl/include/internal/ex.h +++ b/reactos/ntoskrnl/include/internal/ex.h @@ -68,6 +68,36 @@ typedef struct #define ExRundownCompleted _ExRundownCompleted #define ExGetPreviousMode KeGetPreviousMode +// +// Detect GCC 4.1.2+ +// +#if (__GNUC__ * 10000 + __GNUC_MINOR__ * 100 + __GNUC_PATCHLEVEL__) < 40102 + +// +// Broken GCC with Alignment Bug. We'll do alignment ourselves at higher cost. +// +#define DEFINE_WAIT_BLOCK(x) \ + struct _AlignHack \ + { \ + UCHAR Hack[15]; \ + EX_PUSH_LOCK_WAIT_BLOCK UnalignedBlock; \ + } WaitBlockBuffer; \ + PEX_PUSH_LOCK_WAIT_BLOCK x = (PEX_PUSH_LOCK_WAIT_BLOCK) \ + ((ULONG_PTR)&WaitBlockBuffer.UnalignedBlock &~ 0xF); + +#else + +// +// This is only for compatibility; the compiler will optimize the extra +// local variable (the actual pointer) away, so we don't take any perf hit +// by doing this. +// +#define DEFINE_WAIT_BLOCK(x) \ + EX_PUSH_LOCK_WAIT_BLOCK WaitBlockBuffer; \ + PEX_PUSH_LOCK_WAIT_BLOCK x = &WaitBlockBuffer; + +#endif + /* INITIALIZATION FUNCTIONS *************************************************/ VOID @@ -596,6 +626,23 @@ _ExRundownCompleted(IN PEX_RUNDOWN_REF RunRef) /* PUSHLOCKS *****************************************************************/ +/* FIXME: VERIFY THESE! */ + +VOID +FASTCALL +ExBlockPushLock(PEX_PUSH_LOCK PushLock, + PVOID WaitBlock); + +VOID +FASTCALL +ExfUnblockPushLock(PEX_PUSH_LOCK PushLock, + PVOID CurrentWaitBlock); + +VOID +FASTCALL +ExWaitForUnblockPushLock(IN PEX_PUSH_LOCK PushLock, + IN PEX_PUSH_LOCK_WAIT_BLOCK WaitBlock); + /*++ * @name ExInitializePushLock * INTERNAL MACRO @@ -751,12 +798,16 @@ VOID FORCEINLINE ExWaitOnPushLock(PEX_PUSH_LOCK PushLock) { - /* Acquire the lock */ - ExfAcquirePushLockExclusive(PushLock); - ASSERT(PushLock->Locked); + /* Check if we're locked */ + if (PushLock->Locked) + { + /* Acquire the lock */ + ExfAcquirePushLockExclusive(PushLock); + ASSERT(PushLock->Locked); - /* Release it */ - ExfReleasePushLockExclusive(PushLock); + /* Release it */ + ExfReleasePushLockExclusive(PushLock); + } } /*++