[ros-dev] [ros-diffs] [cgutman] 42400: - Partial rewrite of recursive mutex code - Makes the recursive mutex faster and smaller - Fixes several unprotected accesses to recursive mutex
Alex Ionescu
ionucu at videotron.ca
Wed Aug 5 06:58:08 CEST 2009
Mutexes are already recursive, while fast-mutexes are not so I still
don't understand the point of this library. Just use a KMUTEX.
On 4-Aug-09, at 4:51 PM, cgutman at svn.reactos.org wrote:
> Author: cgutman
> Date: Wed Aug 5 01:51:39 2009
> New Revision: 42400
>
> URL: http://svn.reactos.org/svn/reactos?rev=42400&view=rev
> Log:
> - Partial rewrite of recursive mutex code
> - Makes the recursive mutex faster and smaller
> - Fixes several unprotected accesses to recursive mutex
>
> Modified:
> trunk/reactos/drivers/network/tcpip/include/lock.h
> trunk/reactos/drivers/network/tcpip/recmutex/recmutex.c
> trunk/reactos/drivers/network/tcpip/recmutex/recmutex.h
> trunk/reactos/drivers/network/tcpip/tcpip/lock.c
>
> Modified: trunk/reactos/drivers/network/tcpip/include/lock.h
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/tcpip/include/lock.h?rev=42400&r1=42399&r2=42400&view=diff
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- trunk/reactos/drivers/network/tcpip/include/lock.h [iso-8859-1]
> (original)
> +++ trunk/reactos/drivers/network/tcpip/include/lock.h [iso-8859-1]
> Wed Aug 5 01:51:39 2009
> @@ -13,7 +13,7 @@
> extern VOID TcpipAcquireFastMutex( PFAST_MUTEX Mutex );
> extern VOID TcpipReleaseFastMutex( PFAST_MUTEX Mutex );
> extern VOID TcpipRecursiveMutexInit( PRECURSIVE_MUTEX RecMutex );
> -extern UINT TcpipRecursiveMutexEnter( PRECURSIVE_MUTEX RecMutex,
> +extern VOID TcpipRecursiveMutexEnter( PRECURSIVE_MUTEX RecMutex,
> BOOLEAN ToWrite );
> extern VOID TcpipRecursiveMutexLeave( PRECURSIVE_MUTEX RecMutex );
>
>
> Modified: trunk/reactos/drivers/network/tcpip/recmutex/recmutex.c
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/tcpip/recmutex/recmutex.c?rev=42400&r1=42399&r2=42400&view=diff
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- trunk/reactos/drivers/network/tcpip/recmutex/recmutex.c
> [iso-8859-1] (original)
> +++ trunk/reactos/drivers/network/tcpip/recmutex/recmutex.c
> [iso-8859-1] Wed Aug 5 01:51:39 2009
> @@ -3,7 +3,6 @@
>
> VOID RecursiveMutexInit( PRECURSIVE_MUTEX RecMutex ) {
> RtlZeroMemory( RecMutex, sizeof(*RecMutex) );
> - KeInitializeSpinLock( &RecMutex->SpinLock );
> ExInitializeFastMutex( &RecMutex->Mutex );
> KeInitializeEvent( &RecMutex->StateLockedEvent,
> NotificationEvent, FALSE );
> @@ -11,76 +10,53 @@
>
> /* NOTE: When we leave, the FAST_MUTEX must have been released. The
> result
> * is that we always exit in the same irql as entering */
> -SIZE_T RecursiveMutexEnter( PRECURSIVE_MUTEX RecMutex, BOOLEAN
> ToWrite ) {
> +VOID RecursiveMutexEnter( PRECURSIVE_MUTEX RecMutex ) {
> NTSTATUS Status = STATUS_SUCCESS;
> PVOID CurrentThread = KeGetCurrentThread();
> - KIRQL CurrentIrql;
> + KIRQL CurrentIrql = KeGetCurrentIrql();
> +
> + ASSERT(RecMutex);
> + ASSERT(CurrentIrql <= APC_LEVEL);
>
> /* Wait for the previous user to unlock the RecMutex state.
> There might be
> * multiple waiters waiting to change the state. We need to
> check each
> * time we get the event whether somebody still has the state
> locked */
>
> - if( !RecMutex ) return FALSE;
> + ExAcquireFastMutex( &RecMutex->Mutex );
>
> - if( CurrentThread == RecMutex->CurrentThread ||
> - (!ToWrite && !RecMutex->Writer) ) {
> + if( CurrentThread == RecMutex->CurrentThread ) {
> RecMutex->LockCount++;
> - return TRUE;
> + ExReleaseFastMutex( &RecMutex->Mutex );
> + return;
> }
>
> - CurrentIrql = KeGetCurrentIrql();
> -
> - ASSERT(CurrentIrql <= DISPATCH_LEVEL);
> -
> - if( CurrentIrql <= APC_LEVEL ) {
> - ExAcquireFastMutex( &RecMutex->Mutex );
> - while( RecMutex->Locked ) {
> - ExReleaseFastMutex( &RecMutex->Mutex );
> - Status = KeWaitForSingleObject( &RecMutex-
> >StateLockedEvent,
> - UserRequest,
> - KernelMode,
> - FALSE,
> - NULL );
> - ExAcquireFastMutex( &RecMutex->Mutex );
> - }
> - RecMutex->OldIrql = CurrentIrql;
> - RecMutex->Locked = TRUE;
> - RecMutex->Writer = ToWrite;
> - RecMutex->CurrentThread = CurrentThread;
> - RecMutex->LockCount++;
> - ExReleaseFastMutex( &RecMutex->Mutex );
> - } else {
> - KeAcquireSpinLockAtDpcLevel( &RecMutex->SpinLock );
> - RecMutex->OldIrql = DISPATCH_LEVEL;
> - RecMutex->Locked = TRUE;
> - RecMutex->Writer = ToWrite;
> - RecMutex->CurrentThread = CurrentThread;
> - RecMutex->LockCount++;
> + while( RecMutex->LockCount != 0 ) {
> + ExReleaseFastMutex( &RecMutex->Mutex );
> + Status = KeWaitForSingleObject( &RecMutex->StateLockedEvent,
> + UserRequest,
> + KernelMode,
> + FALSE,
> + NULL );
> + ExAcquireFastMutex( &RecMutex->Mutex );
> }
> -
> - return TRUE;
> + RecMutex->CurrentThread = CurrentThread;
> + RecMutex->LockCount++;
> + ExReleaseFastMutex( &RecMutex->Mutex );
> }
>
> VOID RecursiveMutexLeave( PRECURSIVE_MUTEX RecMutex ) {
> + ASSERT(RecMutex);
> +
> + ExAcquireFastMutex( &RecMutex->Mutex );
>
> ASSERT(RecMutex->LockCount > 0);
> RecMutex->LockCount--;
>
> if( !RecMutex->LockCount ) {
> - RecMutex->CurrentThread = NULL;
> - if( RecMutex->OldIrql <= APC_LEVEL ) {
> - ExAcquireFastMutex( &RecMutex->Mutex );
> - RecMutex->Locked = FALSE;
> - RecMutex->Writer = FALSE;
> - ExReleaseFastMutex( &RecMutex->Mutex );
> - } else {
> - RecMutex->Locked = FALSE;
> - RecMutex->Writer = FALSE;
> - KeReleaseSpinLockFromDpcLevel( &RecMutex->SpinLock );
> - }
> -
> KePulseEvent( &RecMutex->StateLockedEvent,
> IO_NETWORK_INCREMENT,
> FALSE );
> }
> +
> + ExReleaseFastMutex( &RecMutex->Mutex );
> }
>
>
> Modified: trunk/reactos/drivers/network/tcpip/recmutex/recmutex.h
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/tcpip/recmutex/recmutex.h?rev=42400&r1=42399&r2=42400&view=diff
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- trunk/reactos/drivers/network/tcpip/recmutex/recmutex.h
> [iso-8859-1] (original)
> +++ trunk/reactos/drivers/network/tcpip/recmutex/recmutex.h
> [iso-8859-1] Wed Aug 5 01:51:39 2009
> @@ -10,20 +10,12 @@
> PVOID CurrentThread;
> /* Notification event which signals that another thread can take
> over */
> KEVENT StateLockedEvent;
> - /* IRQL from spin lock */
> - KIRQL OldIrql;
> - /* Is Locked */
> - BOOLEAN Locked;
> - /* Is reader or writer phase */
> - BOOLEAN Writer;
> - /* Spin lock needed for */
> - KSPIN_LOCK SpinLock;
> } RECURSIVE_MUTEX, *PRECURSIVE_MUTEX;
>
> extern VOID RecursiveMutexInit( PRECURSIVE_MUTEX RecMutex );
> -extern SIZE_T RecursiveMutexEnter( PRECURSIVE_MUTEX RecMutex,
> BOOLEAN ToRead );
> +extern VOID RecursiveMutexEnter( PRECURSIVE_MUTEX RecMutex );
> extern VOID RecursiveMutexLeave( PRECURSIVE_MUTEX RecMutex );
>
> -#define ASSERT_LOCKED(x) ASSERT((x)->Locked)
> +#define ASSERT_LOCKED(x)
>
> #endif/*_ROSRTL_RECMUTEX_H*/
>
> Modified: trunk/reactos/drivers/network/tcpip/tcpip/lock.c
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/network/tcpip/tcpip/lock.c?rev=42400&r1=42399&r2=42400&view=diff
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- trunk/reactos/drivers/network/tcpip/tcpip/lock.c [iso-8859-1]
> (original)
> +++ trunk/reactos/drivers/network/tcpip/tcpip/lock.c [iso-8859-1]
> Wed Aug 5 01:51:39 2009
> @@ -48,11 +48,9 @@
> RecursiveMutexInit( RecMutex );
> }
>
> -UINT TcpipRecursiveMutexEnter( PRECURSIVE_MUTEX RecMutex, BOOLEAN
> ToWrite ) {
> - UINT Ret;
> +VOID TcpipRecursiveMutexEnter( PRECURSIVE_MUTEX RecMutex, BOOLEAN
> ToWrite ) {
> //TI_DbgPrint(DEBUG_LOCK,("Locking\n"));
> - Ret = RecursiveMutexEnter( RecMutex, ToWrite );
> - return Ret;
> + RecursiveMutexEnter( RecMutex );
> }
>
> VOID TcpipRecursiveMutexLeave( PRECURSIVE_MUTEX RecMutex ) {
>
>
Best regards,
Alex Ionescu
More information about the Ros-dev
mailing list