Re: [ros-diffs] [ion] 19352: - Reimplement Fast Mutex implementation in HAL/NT to be compatible with the real implementation. (Fast Mutex needs to raise IRQL).

[Hartmut Birr]

ion(a)svn.reactos.com wrote:
>   - Reimplement Fast Mutex implementation in HAL/NT to
> be compatible with the real implementation. (Fast Mutex needs to raise IRQL).
>  - Implement ExEnterCriticalRegionAndAcquireFastMutexUnsafe and
> ExReleaseFastMutexUnsafeAndLeaveCriticalRegion.
>  - Make win32k use those two new functions so that it can continue running at
> PASSIVE_LEVEL.
>  - Remove CcBrokenMutex and use the new APIs instead.
>  - Implement and export ntoskrnl version of Fast Mutex
>  - Update headers for new fast-mutex definition and API exports.
>  - Fix RemoveEntryList in NDK.
>  - Add exfuncs.h to NDK.
>  - Fix path in mmtypes.h in NDK to be compatible to how it shoudl be included.
>  *Modified: trunk/reactos/hal/halx86/generic/fmutex.c*
>  --- trunk/reactos/hal/halx86/generic/fmutex.c	2005-11-19 21:07:25 UTC (rev 19351)
>  +++ trunk/reactos/hal/halx86/generic/fmutex.c	2005-11-19 22:13:35 UTC (rev 19352)
>  +VOID
>  +FASTCALL
>  +ExAcquireFastMutex(PFAST_MUTEX FastMutex)
>    
>   {
>    
>  -   KeEnterCriticalRegion();
>  -   ExAcquireFastMutexUnsafe(FastMutex);
>    
>  +    KIRQL OldIrql;
>  +
>  +    /* Raise IRQL to APC */
>  +    OldIrql = KfRaiseIrql(APC_LEVEL);
>  +
>  +    /* Decrease the count */
>  +    if (InterlockedDecrement(&FastMutex->Count))
>  +    {
>  +        /* Someone is still holding it, use slow path */
>  +        FastMutex->Contention++;
>  +        KeWaitForSingleObject(&FastMutex->Gate,
>  +                              WrExecutive,
>  +                              WaitAny,
>  +                              FALSE,
>  +                              NULL);
>  +    }
>  +
>  +    /* Set the owner and IRQL */
>  +    FastMutex->Owner = KeGetCurrentThread();
>  +    FastMutex->OldIrql = OldIrql;
>    
>   }
>     
Hi,

this piece of  code is wrong. If we terminating a thread, which waits on
something, we do unblock the thread. The thread must check, if it got
the lock, if not it must wait again.

- Hartmut

[Alex Ionescu]

Hartmut Birr wrote:

>  
> Hi,
>
> this piece of  code is wrong. If we terminating a thread, which waits on
> something, we do unblock the thread. The thread must check, if it got
> the lock, if not it must wait again.
>
> - Hartmut
>   
>  
Hi,

I'm afraid that's incorrect; if the wait gets satisfied you -will- get 
the fast mutex.

Best regards,
Alex Ionescu

[Alex Ionescu]

Hartmut Birr wrote:

>  Hi,
>
> this piece of  code is wrong. If we terminating a thread, which waits on
> something, we do unblock the thread. The thread must check, if it got
> the lock, if not it must wait again.
>
> - Hartmut
>
>   
>  
Hi,

Forgot to add in my previous email. You can't terminate a thread that's 
running at APC_LEVEL because the kill APC won't execute. Hence, it's 
impossible for the scenario you mentionned to occur.

Best regards,
Alex Ionescu

[Hartmut Birr]

Alex Ionescu wrote:
>   Hartmut Birr wrote:
>
> >   Hi,
> >
> >  this piece of  code is wrong. If we terminating a thread, which waits on
> >  something, we do unblock the thread. The thread must check, if it got
> >  the lock, if not it must wait again.
> >
> >  - Hartmut
> >
> >   
> >  
>  Hi,
>
>  Forgot to add in my previous email. You can't terminate a thread
>  that's running at APC_LEVEL because the kill APC won't execute. Hence,
>  it's impossible for the scenario you mentionned to occur.
>
>  Best regards,
>  Alex Ionescu 
The thread does wait at passive level. It has called
ExEnterCriticalRegionAndAcquireFastMutexUnsafe. A other thread tries to
terminate the waiting thread. KiInsertQueueApc does unwait the waiting
thread, because the exit APC is a special APC.

- Hartmut

[Alex Ionescu]

Hartmut Birr wrote:

>  
> The thread does wait at passive level. It has called
> ExEnterCriticalRegionAndAcquireFastMutexUnsafe. A other thread tries to
> terminate the waiting thread. KiInsertQueueApc does unwait the waiting
> thread, because the exit APC is a special APC.
>
> - Hartmut
> ________________
>   
>  
Hi,

The exit APC is a normal APC. Here is a very document on MSDN you can 
read, it answered a lot of my questions about locking, APCs and threads:
http://msdn.microsoft.com/library/en-us/dndevice/html/Locks_Sync.asp

Best regards,
Alex Ionescu