[ion] 22724: [AUDIT] - Irq.c is another lame wrapper. - Remlock's code was written by Filip Navara and is clean. My additions are based on wdm.h and only add size checks and fix two visible bugs. - Add basic support/detection for debug I/O remove blocks (we bugcheck if we identify one though). - Simplify IoReleaseRemoveLockAndWaitEx - Remove locks are SYNCH events, not Notification events! - Make sure IoConnectInterrupt returns NULL to the caller on failure.

Author: ion
Date: Fri Jun 30 22:54:34 2006
New Revision: 22724

URL: http://svn.reactos.org/svn/reactos?rev=22724&view=rev
Log:
[AUDIT] - Irq.c is another lame wrapper.
        - Remlock's code was written by Filip Navara and is clean. My additions are
based on wdm.h and only add size checks and fix two visible bugs.
- Add basic support/detection for debug I/O remove blocks (we bugcheck if we identify one
though).
- Simplify IoReleaseRemoveLockAndWaitEx
- Remove locks are SYNCH events, not Notification events!
- Make sure IoConnectInterrupt returns NULL to the caller on failure.

Modified:
    trunk/reactos/ntoskrnl/include/internal/io.h
    trunk/reactos/ntoskrnl/io/irq.c   (contents, props changed)
    trunk/reactos/ntoskrnl/io/remlock.c   (contents, props changed)

Modified: trunk/reactos/ntoskrnl/include/internal/io.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/…
==============================================================================
--- trunk/reactos/ntoskrnl/include/internal/io.h (original)
+++ trunk/reactos/ntoskrnl/include/internal/io.h Fri Jun 30 22:54:34 2006
@@ -207,6 +207,27 @@
     PIO_WORKITEM_ROUTINE WorkerRoutine;
     PVOID Context;
 } IO_WORKITEM, *PIO_WORKITEM;
+
+//
+// I/O Wrapper around the Kernel Interrupt
+//
+typedef struct _IO_INTERRUPT
+{
+    KINTERRUPT FirstInterrupt;
+    PKINTERRUPT Interrupt[MAXIMUM_PROCESSORS];
+    KSPIN_LOCK SpinLock;
+} IO_INTERRUPT, *PIO_INTERRUPT;
+
+//
+// To simplify matters, the kernel is made to support both the checked and free
+// version of the I/O Remove Lock in the same binary. This structure includes
+// both, since the DDK has the structure with a compile-time #ifdef.
+//
+typedef struct _EXTENDED_IO_REMOVE_LOCK
+{
+    IO_REMOVE_LOCK_COMMON_BLOCK Common;
+    IO_REMOVE_LOCK_DBG_BLOCK Dbg;
+} EXTENDED_IO_REMOVE_LOCK, *PEXTENDED_IO_REMOVE_LOCK;
 
 //
 // Dummy File Object used inside the Open Packet so that OB knows how to

Modified: trunk/reactos/ntoskrnl/io/irq.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/io/irq.c?rev=2272…
==============================================================================
--- trunk/reactos/ntoskrnl/io/irq.c (original)
+++ trunk/reactos/ntoskrnl/io/irq.c Fri Jun 30 22:54:34 2006
@@ -1,11 +1,9 @@
-/* $Id$
- *
- * COPYRIGHT:       See COPYING in the top level directory
- * PROJECT:         ReactOS kernel
+/*
+ * PROJECT:         ReactOS Kernel
+ * LICENSE:         GPL - See COPYING in the top level directory
  * FILE:            ntoskrnl/io/irq.c
- * PURPOSE:         IRQ handling
- *
- * PROGRAMMERS:     David Welch (welch(a)mcmail.com)
+ * PURPOSE:         I/O Wrappers (called Completion Ports) for Kernel Queues
+ * PROGRAMMERS:     Alex Ionescu (alex.ionescu(a)reactos.org)
  */
 
 /* INCLUDES *****************************************************************/
@@ -14,58 +12,24 @@
 #define NDEBUG
 #include <internal/debug.h>
 
-/* TYPES ********************************************************************/
-typedef struct _IO_INTERRUPT
-{
-    KINTERRUPT FirstInterrupt;
-    PKINTERRUPT Interrupt[MAXIMUM_PROCESSORS];
-    KSPIN_LOCK SpinLock;
-} IO_INTERRUPT, *PIO_INTERRUPT;
-
 /* FUNCTIONS *****************************************************************/
 
 /*
- * FUNCTION: Registers a driver's isr to be called when its device interrupts
- * ARGUMENTS:
- *        InterruptObject (OUT) = Points to the interrupt object created on
- *                                return
- *        ServiceRoutine = Routine to be called when the device interrupts
- *        ServiceContext = Parameter to be passed to ServiceRoutine
- *        SpinLock = Initalized spinlock that will be used to synchronize
- *                   access between the isr and other driver routines. This is
- *                   required if the isr handles more than one vector or the
- *                   driver has more than one isr
- *        Vector = Interrupt vector to allocate
- *                 (returned from HalGetInterruptVector)
- *        Irql = DIRQL returned from HalGetInterruptVector
- *        SynchronizeIrql = DIRQL at which the isr will execute. This must
- *                          be the highest of all the DIRQLs returned from
- *                          HalGetInterruptVector if the driver has multiple
- *                          isrs
- *        InterruptMode = Specifies if the interrupt is LevelSensitive or
- *                        Latched
- *        ShareVector = Specifies if the vector can be shared
- *        ProcessorEnableMask = Processors on the isr can run
- *        FloatingSave = TRUE if the floating point stack should be saved when
- *                       the isr runs. Must be false for x86 drivers
- * RETURNS: Status
- * IRQL: PASSIVE_LEVEL
- *
  * @implemented
  */
-NTSTATUS 
-STDCALL
-IoConnectInterrupt(PKINTERRUPT* InterruptObject,
-                   PKSERVICE_ROUTINE ServiceRoutine,
-                   PVOID ServiceContext,
-                   PKSPIN_LOCK SpinLock,
-                   ULONG Vector,
-                   KIRQL Irql,
-                   KIRQL SynchronizeIrql,
-                   KINTERRUPT_MODE InterruptMode,
-                   BOOLEAN ShareVector,
-                   KAFFINITY ProcessorEnableMask,
-                   BOOLEAN FloatingSave)
+NTSTATUS
+NTAPI
+IoConnectInterrupt(OUT PKINTERRUPT *InterruptObject,
+                   IN PKSERVICE_ROUTINE ServiceRoutine,
+                   IN PVOID ServiceContext,
+                   IN PKSPIN_LOCK SpinLock,
+                   IN ULONG Vector,
+                   IN KIRQL Irql,
+                   IN KIRQL SynchronizeIrql,
+                   IN KINTERRUPT_MODE InterruptMode,
+                   IN BOOLEAN ShareVector,
+                   IN KAFFINITY ProcessorEnableMask,
+                   IN BOOLEAN FloatingSave)
 {
     PKINTERRUPT Interrupt;
     PKINTERRUPT InterruptUsed;
@@ -74,10 +38,10 @@
     BOOLEAN FirstRun = TRUE;
     ULONG count;
     LONG i;
-    
     PAGED_CODE();
 
-    DPRINT("IoConnectInterrupt(Vector %x)\n",Vector);
+    /* Assume failure */
+    *InterruptObject = NULL;
 
     /* Convert the Mask */
     ProcessorEnableMask &= ((1 << KeNumberProcessors) - 1);
@@ -90,32 +54,25 @@
     {
         if (ProcessorEnableMask & (1 << i)) count++;
     }
-    
+
     /* Allocate the array of I/O Interrupts */
     IoInterrupt = ExAllocatePoolWithTag(NonPagedPool,
                                         (count - 1)* sizeof(KINTERRUPT) +
                                         sizeof(IO_INTERRUPT),
                                         TAG_KINTERRUPT);
-    if (!IoInterrupt) return(STATUS_INSUFFICIENT_RESOURCES);
+    if (!IoInterrupt) return STATUS_INSUFFICIENT_RESOURCES;
 
     /* Select which Spinlock to use */
-    if (SpinLock)
-    {
-        SpinLockUsed = SpinLock;
-    }
-    else
-    {
-        SpinLockUsed = &IoInterrupt->SpinLock;
-    }
-    
+    SpinLockUsed = SpinLock ? SpinLock : &IoInterrupt->SpinLock;
+
     /* We first start with a built-in Interrupt inside the I/O Structure */
     *InterruptObject = &IoInterrupt->FirstInterrupt;
     Interrupt = (PKINTERRUPT)(IoInterrupt + 1);
     FirstRun = TRUE;
-    
+
     /* Start with a fresh structure */
     RtlZeroMemory(IoInterrupt, sizeof(IO_INTERRUPT));
-    
+
     /* Now create all the interrupts */
     for (i = 0; i < KeNumberProcessors; i++)
     {
@@ -124,7 +81,7 @@
         {
             /* Check which one we will use */
             InterruptUsed = FirstRun ? &IoInterrupt->FirstInterrupt : Interrupt;
-            
+
             /* Initialize it */
             KeInitializeInterrupt(InterruptUsed,
                                   ServiceRoutine,
@@ -137,7 +94,7 @@
                                   ShareVector,
                                   i,
                                   FloatingSave);
-                                  
+
             /* Connect it */
             if (!KeConnectInterrupt(InterruptUsed))
             {
@@ -152,9 +109,11 @@
                     /* Far enough, so disconnect everything */
                     IoDisconnectInterrupt(&IoInterrupt->FirstInterrupt);
                 }
+
+                /* And fail */
                 return STATUS_INVALID_PARAMETER;
             }
-            
+
             /* Now we've used up our First Run */
             if (FirstRun)
             {
@@ -173,39 +132,35 @@
 }
 
 /*
- * FUNCTION: Releases a drivers isr
- * ARGUMENTS:
- *        InterruptObject = isr to release
- *
  * @implemented
  */
 VOID 
-STDCALL
+NTAPI
 IoDisconnectInterrupt(PKINTERRUPT InterruptObject)
-
 {
     LONG i;
     PIO_INTERRUPT IoInterrupt;
-    
     PAGED_CODE();
-    
+
     /* Get the I/O Interrupt */
-    IoInterrupt = CONTAINING_RECORD(InterruptObject, 
-                                    IO_INTERRUPT, 
+    IoInterrupt = CONTAINING_RECORD(InterruptObject,
+                                    IO_INTERRUPT,
                                     FirstInterrupt);
-                                    
+
     /* Disconnect the first one */
     KeDisconnectInterrupt(&IoInterrupt->FirstInterrupt);
 
     /* Now disconnect the others */
     for (i = 0; i < KeNumberProcessors; i++)
     {
+        /* Make sure one was registered */
         if (IoInterrupt->Interrupt[i])
         {
+            /* Disconnect it */
             KeDisconnectInterrupt(&InterruptObject[i]);
         }
     }
-    
+
     /* Free the I/O Interrupt */
     ExFreePool(IoInterrupt);
 }

Propchange: trunk/reactos/ntoskrnl/io/irq.c
------------------------------------------------------------------------------
--- svn:needs-lock (original)
+++ svn:needs-lock (removed)
@@ -1,1 +1,0 @@
-*

Modified: trunk/reactos/ntoskrnl/io/remlock.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/io/remlock.c?rev=…
==============================================================================
--- trunk/reactos/ntoskrnl/io/remlock.c (original)
+++ trunk/reactos/ntoskrnl/io/remlock.c Fri Jun 30 22:54:34 2006
@@ -1,17 +1,16 @@
-/* $Id$
- *
- * COPYRIGHT:       See COPYING in the top level directory
- * PROJECT:         ReactOS kernel
+/*
+ * PROJECT:         ReactOS Kernel
+ * LICENSE:         GPL - See COPYING in the top level directory
  * FILE:            ntoskrnl/io/remlock.c
- * PURPOSE:         Remove Lock functions
- *
- * PROGRAMMERS:     Filip Navara (xnavara(a)volny.cz)
+ * PURPOSE:         Remove Lock Support
+ * PROGRAMMERS:     Alex Ionescu (alex.ionescu(a)reactos.org)
+ *                  Filip Navara (navaraf(a)reactos.org)
  */
 
 /* INCLUDES ******************************************************************/
 
+#include <ntoskrnl.h>
 #define NDEBUG
-#include <ntoskrnl.h>
 #include <internal/debug.h>
 
 /* FUNCTIONS *****************************************************************/
@@ -20,81 +19,139 @@
  * @implemented
  */
 VOID
-STDCALL
-IoInitializeRemoveLockEx(
-  IN PIO_REMOVE_LOCK RemoveLock,
-  IN ULONG AllocateTag,
-  IN ULONG MaxLockedMinutes,
-  IN ULONG HighWatermark,
-  IN ULONG RemlockSize)
+NTAPI
+IoInitializeRemoveLockEx(IN PIO_REMOVE_LOCK RemoveLock,
+                         IN ULONG AllocateTag,
+                         IN ULONG MaxLockedMinutes,
+                         IN ULONG HighWatermark,
+                         IN ULONG RemlockSize)
 {
-  DPRINT("IoInitializeRemoveLockEx called\n");
-  RtlZeroMemory(RemoveLock, RemlockSize);
-  RemoveLock->Common.IoCount = 1;
-  KeInitializeEvent(&RemoveLock->Common.RemoveEvent, NotificationEvent, FALSE);
+    PEXTENDED_IO_REMOVE_LOCK Lock = (PEXTENDED_IO_REMOVE_LOCK)RemoveLock;
+    PAGED_CODE();
+
+    /* Check if this is a debug lock */
+    if (RemlockSize == sizeof(IO_REMOVE_LOCK_DBG_BLOCK))
+    {
+        /* Clear the lock */
+        RtlZeroMemory(Lock, RemlockSize);
+
+        /* Setup debug parameters */
+        Lock->Dbg.HighWatermark = HighWatermark;
+        Lock->Dbg.MaxLockedTicks = MaxLockedMinutes * 600000000;
+        Lock->Dbg.AllocateTag = AllocateTag;
+        KeInitializeSpinLock(&Lock->Dbg.Spin);
+    }
+    else
+    {
+        /* Otherwise, setup a free block */
+        Lock->Common.Removed = FALSE;
+        Lock->Common.IoCount = 1;
+        KeInitializeEvent(&Lock->Common.RemoveEvent,
+                          SynchronizationEvent,
+                          FALSE);
+    }
 }
 
 /*
  * @implemented
  */
 NTSTATUS
-STDCALL
-IoAcquireRemoveLockEx(
-  IN PIO_REMOVE_LOCK RemoveLock,
-  IN OPTIONAL PVOID Tag,
-  IN LPCSTR File,
-  IN ULONG Line,
-  IN ULONG RemlockSize)
+NTAPI
+IoAcquireRemoveLockEx(IN PIO_REMOVE_LOCK RemoveLock,
+                      IN OPTIONAL PVOID Tag,
+                      IN LPCSTR File,
+                      IN ULONG Line,
+                      IN ULONG RemlockSize)
 {
-  DPRINT("IoAcquireRemoveLockEx called\n");
-  InterlockedIncrement(&RemoveLock->Common.IoCount);
-  if (RemoveLock->Common.Removed)
-  {
-    if (InterlockedDecrement(&RemoveLock->Common.IoCount) == 0)
+    PEXTENDED_IO_REMOVE_LOCK Lock = (PEXTENDED_IO_REMOVE_LOCK)RemoveLock;
+
+    /* Increase the lock count */
+    InterlockedIncrement(&Lock->Common.IoCount);
+    if (!Lock->Common.Removed)
     {
-      KeSetEvent(&RemoveLock->Common.RemoveEvent, IO_NO_INCREMENT, FALSE);
+        /* Check what kind of lock this is */
+        if (RemlockSize == sizeof(IO_REMOVE_LOCK_DBG_BLOCK))
+        {
+            /* FIXME: Not yet supported */
+            DPRINT1("UNIMPLEMENTED\n");
+            KEBUGCHECK(0);
+        }
     }
-    return STATUS_DELETE_PENDING;
-  }
-  return STATUS_SUCCESS;
+    else
+    {
+        /* Otherwise, decrement the count and check if it's gone */
+        if (!InterlockedDecrement(&Lock->Common.IoCount))
+        {
+            /* Signal the event */
+            KeSetEvent(&Lock->Common.RemoveEvent, IO_NO_INCREMENT, FALSE);
+        }
+
+        /* Return pending delete */
+        return STATUS_DELETE_PENDING;
+    }
+
+    /* Otherwise, return success */
+    return STATUS_SUCCESS;
 }
 
 /*
  * @implemented
  */
 VOID
-STDCALL
-IoReleaseRemoveLockEx(
-  IN PIO_REMOVE_LOCK RemoveLock,
-  IN PVOID Tag,
-  IN ULONG RemlockSize)
+NTAPI
+IoReleaseRemoveLockEx(IN PIO_REMOVE_LOCK RemoveLock,
+                      IN PVOID Tag,
+                      IN ULONG RemlockSize)
 {
-  LONG IoCount;
+    PEXTENDED_IO_REMOVE_LOCK Lock = (PEXTENDED_IO_REMOVE_LOCK)RemoveLock;
 
-  DPRINT("IoReleaseRemoveLockEx called\n");
-  IoCount = InterlockedDecrement(&RemoveLock->Common.IoCount);
-  if (IoCount == 0)
-  {
-    KeSetEvent(&RemoveLock->Common.RemoveEvent, IO_NO_INCREMENT, FALSE);
-  }
+    /* Check what kind of lock this is */
+    if (RemlockSize == sizeof(IO_REMOVE_LOCK_DBG_BLOCK))
+    {
+        /* FIXME: Not yet supported */
+        DPRINT1("UNIMPLEMENTED\n");
+        KEBUGCHECK(0);
+    }
+
+    /* Decrement the lock count */
+    if (!InterlockedDecrement(&Lock->Common.IoCount));
+    {
+        /* Signal the event */
+        KeSetEvent(&Lock->Common.RemoveEvent, IO_NO_INCREMENT, FALSE);
+    }
 }
 
 /*
  * @implemented
  */
 VOID
-STDCALL
-IoReleaseRemoveLockAndWaitEx(
-  IN PIO_REMOVE_LOCK RemoveLock,
-  IN PVOID Tag,
-  IN ULONG RemlockSize)
+NTAPI
+IoReleaseRemoveLockAndWaitEx(IN PIO_REMOVE_LOCK RemoveLock,
+                             IN PVOID Tag,
+                             IN ULONG RemlockSize)
 {
-  DPRINT("IoReleaseRemoveLockAndWaitEx called\n");
-  RemoveLock->Common.Removed = TRUE;
-  InterlockedDecrement(&RemoveLock->Common.IoCount);
-  IoReleaseRemoveLockEx(RemoveLock, Tag, RemlockSize);
-  KeWaitForSingleObject(&RemoveLock->Common.RemoveEvent, Executive, KernelMode,
-    FALSE, NULL);
+    PEXTENDED_IO_REMOVE_LOCK Lock = (PEXTENDED_IO_REMOVE_LOCK)RemoveLock;
+    PAGED_CODE();
+
+    /* Remove the lock and decrement the count */
+    Lock->Common.Removed = TRUE;
+    if (InterlockedDecrement(&Lock->Common.IoCount) > 0)
+    {
+        /* Wait for it */
+        KeWaitForSingleObject(&Lock->Common.RemoveEvent,
+                              Executive,
+                              KernelMode,
+                              FALSE,
+                              NULL);
+    }
+
+    /* Check what kind of lock this is */
+    if (RemlockSize == sizeof(IO_REMOVE_LOCK_DBG_BLOCK))
+    {
+        /* FIXME: Not yet supported */
+        DPRINT1("UNIMPLEMENTED\n");
+        KEBUGCHECK(0);
+    }
 }
 
 /* EOF */

Propchange: trunk/reactos/ntoskrnl/io/remlock.c
------------------------------------------------------------------------------
--- svn:needs-lock (original)
+++ svn:needs-lock (removed)
@@ -1,1 +1,0 @@
-*