booting ros is broken (was: fix broken work item impl.)

[Hartmut Birr]

Hi,

I've added the 'work item fix' to my source tree for testing. I'm not
able
to boot reactos. Ros hangs somewhere in ndis:

...
(ldr/loader.c:319) Could not open module file:
\SystemRoot\system32\drivers\DLKRTS.SYS
(io/pnpmgr.c:1521) Initialization of service DLKRTS failed (Status c0000001)
DriverBase for \SystemRoot\system32\drivers\pci.sys: 9cd89000
Peripheral Component Interconnect Bus Driver
(io/create.c:90) Parent is a Directory which is neither a file type nor a
device type
(io/pnpmgr.c:1521) Initialization of service Ne2000 failed (Status c0000001)
DriverBase for scsiport.sys: 9cd93000
DriverBase for atapi.sys: 9cd9f000
DriverBase for aic78u2.sys: 9cdc9000
DriverBase for class2.sys: 9ce6b000
DriverBase for disk.sys: 9ce75000
DriverBase for vfatfs.sys: 9ce7e000
DriverBase for bootvid.sys: 9ce93000
DriverBase for ndis.sys: 9ceb4000
(ndis/main.c:52)(DriverEntry) Called.
(ke/main.c:710) CommandLine: multi(0)disk(0)rdisk(0)partition(3)\ReactOS

Without the fix, booting continues after that point with:

DriverBase for \SystemRoot\system32\drivers\DLKRTS.SYS: 9cf09000
(ndis/miniport.c:1090)(NdisInitializeWrapper) Called.
(ndis/miniport.c:1631)(NdisMRegisterMiniport) Called.
(ndis/miniport.c:1577)(NdisIAddDevice) LinkageKey:
Class\{4D36E972-E325-11CE-BFC1-08002BE10318}\0002\Linkage.
(ndis/miniport.c:1374)(NdisIStartAdapter) creating device \Device\DLKRTS1
(ndis/miniport.c:1394)(NdisIStartAdapter) opened device reg key
(ndis/miniport.c:1396)(NdisIStartAdapter) acquiring miniport block lock
...

- Hartmut

[Gunnar Dalsnes]

Ok. So my fix wasn't the right fix either. I will revert my changes.

Still, the old impl. is broken and bsod for me:-( I guess a temporary dirty
hack could be to increase the semaphore limit? Any objections?

Alex's suggestion to use kernel queues is the right fix i guess, but this is
somewhere in the future.

Gunnar

>   -----Original Message-----
>  From: ros-dev-bounces(a)reactos.com 
>  [mailto:[e-mail]] On Behalf Of Hartmut Birr
>  Sent: Wednesday, November 17, 2004 8:33 PM
>  To: ReactOS Development List
>  Subject: [ros-dev] booting ros is broken (was: fix broken 
>  work item impl.)
>  
>  
>  Hi,
>  
>  I've added the 'work item fix' to my source tree for testing. 
>  I'm not able
>  to boot reactos. Ros hangs somewhere in ndis:
>  
>  ...
>  (ldr/loader.c:319) Could not open module file:
>  \SystemRoot\system32\drivers\DLKRTS.SYS
>  (io/pnpmgr.c:1521) Initialization of service DLKRTS failed 
>  (Status c0000001)
>  DriverBase for \SystemRoot\system32\drivers\pci.sys: 9cd89000
>  Peripheral Component Interconnect Bus Driver
>  (io/create.c:90) Parent is a Directory which is neither a 
>  file type nor a
>  device type
>  (io/pnpmgr.c:1521) Initialization of service Ne2000 failed 
>  (Status c0000001)
>  DriverBase for scsiport.sys: 9cd93000
>  DriverBase for atapi.sys: 9cd9f000
>  DriverBase for aic78u2.sys: 9cdc9000
>  DriverBase for class2.sys: 9ce6b000
>  DriverBase for disk.sys: 9ce75000
>  DriverBase for vfatfs.sys: 9ce7e000
>  DriverBase for bootvid.sys: 9ce93000
>  DriverBase for ndis.sys: 9ceb4000
>  (ndis/main.c:52)(DriverEntry) Called.
>  (ke/main.c:710) CommandLine: 
>  multi(0)disk(0)rdisk(0)partition(3)\ReactOS
>  
>  Without the fix, booting continues after that point with:
>  
>  DriverBase for \SystemRoot\system32\drivers\DLKRTS.SYS: 9cf09000
>  (ndis/miniport.c:1090)(NdisInitializeWrapper) Called.
>  (ndis/miniport.c:1631)(NdisMRegisterMiniport) Called.
>  (ndis/miniport.c:1577)(NdisIAddDevice) LinkageKey:
>  Class\{4D36E972-E325-11CE-BFC1-08002BE10318}\0002\Linkage.
>  (ndis/miniport.c:1374)(NdisIStartAdapter) creating device 
>  \Device\DLKRTS1
>  (ndis/miniport.c:1394)(NdisIStartAdapter) opened device reg key
>  (ndis/miniport.c:1396)(NdisIStartAdapter) acquiring miniport 
>  block lock
>  ...
>  
>  - Hartmut
>  
>  
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.com
>  http://reactos.com:8080/mailman/listinfo/ros-dev
>  
>  
>  
>

[Alex Ionescu]

Gunnar Dalsnes wrote:

>  Ok. So my fix wasn't the right fix either. I will
> revert my changes.
>
> Still, the old impl. is broken and bsod for me:-( I guess a temporary dirty
> hack could be to increase the semaphore limit? Any objections?
>   
>  
I agree, it's better then wasting time in trying to fix into more detail 
code which shouldn't behave that way.

>  Alex's suggestion to use kernel queues is the right
> fix i guess, but this is
> somewhere in the future.
>
>   
>  
I've added it to my timeline. As soon as I'm done testing the new DPC 
rules I'll check out KQUEUES ;)
Seems to be easy since your current code really seems solid.

Best regards,
Alex Ionescu

>  _______________________________________________
> Ros-dev mailing list
> Ros-dev(a)reactos.com
> http://reactos.com:8080/mailman/listinfo/ros-dev
>
>   
>

[Hartmut Birr]

>   -----Original Message-----
>  From: ros-dev-bounces(a)reactos.com 
>  [mailto:[e-mail]] On Behalf Of Gunnar Dalsnes
>  Sent: Thursday, November 18, 2004 12:05 AM
>  To: 'ReactOS Development List'
>  Subject: RE: [ros-dev] booting ros is broken (was: fix broken 
>  work item impl.)
>  
>  
>  
>  Ok. So my fix wasn't the right fix either. I will revert my changes.
>  
>  Still, the old impl. is broken and bsod for me:-( I guess a 
>  temporary dirty
>  hack could be to increase the semaphore limit? Any objections? 
If 256 work items are not enough, something else is wrong. You should change
the worker code a little bit, that the caller of each request is stored in
the work item. If the number of items reach a limit (10..20), the code
should print the items and callers. This will not work with non ros drivers
if the caller is stored in WORK_QUEUE_ITEM.

- Hartmut

[Filip Navara]

Attached is an experimental implementation of work queues using KQUEUEs. 
It depends on the KQUEUE patch I sent...

Regards,
Filip

Index: ntoskrnl/ex/work.c
===================================================================
RCS file: /CVS/ReactOS/reactos/ntoskrnl/ex/work.c,v
retrieving revision 1.22
diff -u -r1.22 work.c
--- ntoskrnl/ex/work.c	17 Nov 2004 23:55:36 -0000	1.22
+++ ntoskrnl/ex/work.c	21 Nov 2004 13:26:34 -0000
@@ -24,19 +24,9 @@
 typedef struct _WORK_QUEUE
 {
    /*
-    * PURPOSE: Head of the list of waiting work items
+    * PURPOSE: The actual queue for the work items
     */
-   LIST_ENTRY Head;
-   
-   /*
-    * PURPOSE: Sychronize access to the work queue
-    */
-   KSPIN_LOCK Lock;
-   
-   /*
-    * PURPOSE: Worker threads with nothing to do wait on this event
-    */
-   KSEMAPHORE Sem;
+   KQUEUE WorkerQueue;
    
    /*
     * PURPOSE: Thread associated with work queue
@@ -75,22 +65,12 @@
    
    for(;;)
      {
-	current = ExInterlockedRemoveHeadList(&queue->Head,
-					      &queue->Lock);
+	current = KeRemoveQueue(&queue->WorkerQueue, KernelMode, NULL);
 	if (current!=NULL)
 	  {
 	     item = CONTAINING_RECORD(current,WORK_QUEUE_ITEM,List);
 	     item->WorkerRoutine(item->Parameter);
 	  }
-	else
-	  {
-	     KeWaitForSingleObject((PVOID)&queue->Sem,
-				   Executive,
-				   KernelMode,
-				   FALSE,
-				   NULL);
-	     DPRINT("Woke from wait\n");
-	  }
      }
 }
 
@@ -100,11 +80,7 @@
    ULONG i;
    PETHREAD Thread;
    
-   InitializeListHead(&WorkQueue->Head);
-   KeInitializeSpinLock(&WorkQueue->Lock);
-   KeInitializeSemaphore(&WorkQueue->Sem,
-			 0,
-			 256);
+   KeInitializeQueue(&WorkQueue->WorkerQueue, NUMBER_OF_WORKER_THREADS);
    for (i=0; i<NUMBER_OF_WORKER_THREADS; i++)
      {
 	PsCreateSystemThread(&WorkQueue->Thread[i],
@@ -154,47 +130,26 @@
     ASSERT(WorkItem!=NULL);
     ASSERT_IRQL(DISPATCH_LEVEL);
    
-   /*
-    * Insert the item in the appropiate queue and wake up any thread
-    * waiting for something to do
-    */
+    /*
+     * Insert the item in the appropiate queue and wake up any thread
+     * waiting for something to do
+     */
     switch(QueueType)
     {
-    case DelayedWorkQueue:
-	    ExInterlockedInsertTailList(&EiNormalWorkQueue.Head,
-				    &WorkItem->List,
-				    &EiNormalWorkQueue.Lock);
-	    KeReleaseSemaphore(&EiNormalWorkQueue.Sem,
-			   IO_NO_INCREMENT,
-			   1,
-			   FALSE);
-	break;
+        case DelayedWorkQueue:
+           KeInsertQueue(&EiNormalWorkQueue.WorkerQueue,
+                         &WorkItem->List);
+           break;
 	
-    case CriticalWorkQueue:
-        ExInterlockedInsertTailList(&EiCriticalWorkQueue.Head,
-				    &WorkItem->List,
-				    &EiCriticalWorkQueue.Lock);
-        KeReleaseSemaphore(&EiCriticalWorkQueue.Sem,
-	       	  	   IO_NO_INCREMENT,
-	       		   1,
-	       		   FALSE);
-	    break;
-
-    case HyperCriticalWorkQueue:
-	    ExInterlockedInsertTailList(&EiHyperCriticalWorkQueue.Head,
-				    &WorkItem->List,
-				    &EiHyperCriticalWorkQueue.Lock);
-	    KeReleaseSemaphore(&EiHyperCriticalWorkQueue.Sem,
-			   IO_NO_INCREMENT,
-			   1,
-			   FALSE);
-    	break;
-
-#ifdef __USE_W32API
-	case MaximumWorkQueue:
-	   // Unimplemented
-	   break;
-#endif
+        case CriticalWorkQueue:
+           KeInsertQueue(&EiCriticalWorkQueue.WorkerQueue,
+                         &WorkItem->List);
+           break;
+
+        case HyperCriticalWorkQueue:
+           KeInsertQueue(&EiHyperCriticalWorkQueue.WorkerQueue,
+                         &WorkItem->List);
+           break;
     }
 }