4 Jun
2011
4 Jun
'11
1:52 p.m.
Excellent, thank you!
Small nit, KeMemoryBarrierWithoutFence() is the "more correct" way of doing this
in kernel code -- it expands to _ReadWriteBarrier(), but it's the
"kernel-friendly" macro the WDK suggests.
--
Best regards,
Alex Ionescu
On 2011-06-04, at 8:33 AM, tkreuzer(a)svn.reactos.org wrote:
Author: tkreuzer
Date: Sat Jun 4 12:33:54 2011
New Revision: 52078
URL: http://svn.reactos.org/svn/reactos?rev=52078&view=rev
Log:
[NTOSKRNL/HAL]
- Add explicit memory barriers to KxAcquireSpinLock, KxReleaseSpinLock inline functions
and KeTryToAcquireQueuedSpinLock, KeTryToAcquireQueuedSpinLockRaiseToSynch in the UP case.
This will prevent the compiler from reordering memory access instructions across the
boundaries of these functions, even when being inlined.
- Use the inline functions in x64 spinlock functions, too
Modified:
trunk/reactos/hal/halx86/generic/spinlock.c
trunk/reactos/ntoskrnl/include/internal/spinlock.h
trunk/reactos/ntoskrnl/ke/amd64/spinlock.c
Modified: trunk/reactos/hal/halx86/generic/spinlock.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/hal/halx86/generic/spinloc…
==============================================================================
--- trunk/reactos/hal/halx86/generic/spinlock.c [iso-8859-1] (original)
+++ trunk/reactos/hal/halx86/generic/spinlock.c [iso-8859-1] Sat Jun 4 12:33:54 2011
@@ -188,6 +188,10 @@
/* Simply raise to synch */
KeRaiseIrql(SYNCH_LEVEL, OldIrql);
+ /* Add an explicit memory barrier to prevent the compiler from reordering
+ memory accesses across the borders of spinlocks */
+ _ReadWriteBarrier();
+
/* Always return true on UP Machines */
return TRUE;
}
@@ -208,6 +212,10 @@
/* Simply raise to dispatch */
KeRaiseIrql(DISPATCH_LEVEL, OldIrql);
+ /* Add an explicit memory barrier to prevent the compiler from reordering
+ memory accesses across the borders of spinlocks */
+ _ReadWriteBarrier();
+
/* Always return true on UP Machines */
return TRUE;
}
Modified: trunk/reactos/ntoskrnl/include/internal/spinlock.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/…
==============================================================================
--- trunk/reactos/ntoskrnl/include/internal/spinlock.h [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/include/internal/spinlock.h [iso-8859-1] Sat Jun 4 12:33:54
2011
@@ -21,6 +21,10 @@
{
/* On UP builds, spinlocks don't exist at IRQL >= DISPATCH */
UNREFERENCED_PARAMETER(SpinLock);
+
+ /* Add an explicit memory barrier to prevent the compiler from reordering
+ memory accesses across the borders of spinlocks */
+ _ReadWriteBarrier();
}
//
@@ -32,6 +36,10 @@
{
/* On UP builds, spinlocks don't exist at IRQL >= DISPATCH */
UNREFERENCED_PARAMETER(SpinLock);
+
+ /* Add an explicit memory barrier to prevent the compiler from reordering
+ memory accesses across the borders of spinlocks */
+ _ReadWriteBarrier();
}
#else
Modified: trunk/reactos/ntoskrnl/ke/amd64/spinlock.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/amd64/spinlock…
==============================================================================
--- trunk/reactos/ntoskrnl/ke/amd64/spinlock.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/ke/amd64/spinlock.c [iso-8859-1] Sat Jun 4 12:33:54 2011
@@ -23,14 +23,14 @@
KIRQL
KeAcquireSpinLockRaiseToSynch(PKSPIN_LOCK SpinLock)
{
-#ifndef CONFIG_SMP
- KIRQL OldIrql;
- /* Simply raise to dispatch */
- KeRaiseIrql(DISPATCH_LEVEL, &OldIrql);
- return OldIrql;
-#else
- UNIMPLEMENTED;
-#endif
+ KIRQL OldIrql;
+
+ /* Raise to sync */
+ KeRaiseIrql(SYNCH_LEVEL, &OldIrql);
+
+ /* Acquire the lock and return */
+ KxAcquireSpinLock(SpinLock);
+ return OldIrql;
}
/*
@@ -40,14 +40,14 @@
NTAPI
KeAcquireSpinLockRaiseToDpc(PKSPIN_LOCK SpinLock)
{
-#ifndef CONFIG_SMP
- KIRQL OldIrql;
- /* Simply raise to dispatch */
+ KIRQL OldIrql;
+
+ /* Raise to dispatch */
KeRaiseIrql(DISPATCH_LEVEL, &OldIrql);
- return OldIrql;
-#else
- UNIMPLEMENTED;
-#endif
+
+ /* Acquire the lock and return */
+ KxAcquireSpinLock(SpinLock);
+ return OldIrql;
}
/*
@@ -58,12 +58,9 @@
KeReleaseSpinLock(PKSPIN_LOCK SpinLock,
KIRQL OldIrql)
{
-#ifndef CONFIG_SMP
- /* Simply lower IRQL back */
+ /* Release the lock and lower IRQL back */
+ KxReleaseSpinLock(SpinLock);
KeLowerIrql(OldIrql);
-#else
- UNIMPLEMENTED;
-#endif
}
/*
@@ -72,14 +69,14 @@
KIRQL
KeAcquireQueuedSpinLock(IN KSPIN_LOCK_QUEUE_NUMBER LockNumber)
{
-#ifndef CONFIG_SMP
- KIRQL OldIrql;
- /* Simply raise to dispatch */
+ KIRQL OldIrql;
+
+ /* Raise to dispatch */
KeRaiseIrql(DISPATCH_LEVEL, &OldIrql);
- return OldIrql;
-#else
- UNIMPLEMENTED;
-#endif
+
+ /* Acquire the lock */
+ KxAcquireSpinLock(KeGetCurrentPrcb()->LockQueue[LockNumber].Lock); // HACK
+ return OldIrql;
}
/*
@@ -88,14 +85,14 @@
KIRQL
KeAcquireQueuedSpinLockRaiseToSynch(IN KSPIN_LOCK_QUEUE_NUMBER LockNumber)
{
-#ifndef CONFIG_SMP
- KIRQL OldIrql;
- /* Simply raise to dispatch */
- KeRaiseIrql(DISPATCH_LEVEL, &OldIrql);
- return OldIrql;
-#else
- UNIMPLEMENTED;
-#endif
+ KIRQL OldIrql;
+
+ /* Raise to synch */
+ KeRaiseIrql(SYNCH_LEVEL, &OldIrql);
+
+ /* Acquire the lock */
+ KxAcquireSpinLock(KeGetCurrentPrcb()->LockQueue[LockNumber].Lock); // HACK
+ return OldIrql;
}
/*
@@ -105,13 +102,17 @@
KeAcquireInStackQueuedSpinLock(IN PKSPIN_LOCK SpinLock,
IN PKLOCK_QUEUE_HANDLE LockHandle)
{
-#ifndef CONFIG_SMP
- /* Simply raise to dispatch */
+ /* Set up the lock */
+ LockHandle->LockQueue.Next = NULL;
+ LockHandle->LockQueue.Lock = SpinLock;
+
+ /* Raise to dispatch */
KeRaiseIrql(DISPATCH_LEVEL, &LockHandle->OldIrql);
-#else
- UNIMPLEMENTED;
-#endif
-}
+
+ /* Acquire the lock */
+ KxAcquireSpinLock(LockHandle->LockQueue.Lock); // HACK
+}
+
/*
* @implemented
@@ -120,13 +121,17 @@
KeAcquireInStackQueuedSpinLockRaiseToSynch(IN PKSPIN_LOCK SpinLock,
IN PKLOCK_QUEUE_HANDLE LockHandle)
{
-#ifndef CONFIG_SMP
- /* Simply raise to synch */
+ /* Set up the lock */
+ LockHandle->LockQueue.Next = NULL;
+ LockHandle->LockQueue.Lock = SpinLock;
+
+ /* Raise to synch */
KeRaiseIrql(SYNCH_LEVEL, &LockHandle->OldIrql);
-#else
- UNIMPLEMENTED;
-#endif
-}
+
+ /* Acquire the lock */
+ KxAcquireSpinLock(LockHandle->LockQueue.Lock); // HACK
+}
+
/*
* @implemented
@@ -135,27 +140,25 @@
KeReleaseQueuedSpinLock(IN KSPIN_LOCK_QUEUE_NUMBER LockNumber,
IN KIRQL OldIrql)
{
-#ifndef CONFIG_SMP
+ /* Release the lock */
+ KxReleaseSpinLock(KeGetCurrentPrcb()->LockQueue[LockNumber].Lock); // HACK
+
+ /* Lower IRQL back */
+ KeLowerIrql(OldIrql);
+}
+
+
+/*
+ * @implemented
+ */
+VOID
+KeReleaseInStackQueuedSpinLock(IN PKLOCK_QUEUE_HANDLE LockHandle)
+{
/* Simply lower IRQL back */
- KeLowerIrql(OldIrql);
-#else
- UNIMPLEMENTED;
-#endif
-}
-
-/*
- * @implemented
- */
-VOID
-KeReleaseInStackQueuedSpinLock(IN PKLOCK_QUEUE_HANDLE LockHandle)
-{
-#ifndef CONFIG_SMP
- /* Simply lower IRQL back */
+ KxReleaseSpinLock(LockHandle->LockQueue.Lock); // HACK
KeLowerIrql(LockHandle->OldIrql);
-#else
- UNIMPLEMENTED;
-#endif
-}
+}
+
/*
* @implemented
@@ -167,11 +170,16 @@
#ifndef CONFIG_SMP
/* Simply raise to dispatch */
KeRaiseIrql(DISPATCH_LEVEL, OldIrql);
+
+ /* Add an explicit memory barrier to prevent the compiler from reordering
+ memory accesses across the borders of spinlocks */
+ _ReadWriteBarrier();
/* Always return true on UP Machines */
return TRUE;
#else
UNIMPLEMENTED;
+ ASSERT(FALSE);
#endif
}
@@ -185,11 +193,16 @@
#ifndef CONFIG_SMP
/* Simply raise to dispatch */
KeRaiseIrql(DISPATCH_LEVEL, OldIrql);
+
+ /* Add an explicit memory barrier to prevent the compiler from reordering
+ memory accesses across the borders of spinlocks */
+ _ReadWriteBarrier();
/* Always return true on UP Machines */
return TRUE;
#else
UNIMPLEMENTED;
+ ASSERT(FALSE);
#endif
}