Re: [ros-dev] Re: [ros-svn] [ion] 20554: - Fix shamefully dangerously broken Work Thread/Queue/Item implementation:

[Hartmut Birr]

WaxDragon wrote:
> On 1/7/06, Hartmut Birr [e-mail] wrote:
> > Alex Ionescu wrote:
> > > Hartmut Birr wrote:
> > > > The real problem is,
> > > > KiAbortWaitThread is called for a thread which does not waiting.
> > >
> > > Ok, since this happened after my worker thread patch and they use
> > > kernel queues, I reviewed their implementation and found a number of
> > > important flaws.. wether or not they cause this problem I can't tell
> > > for sure, but I've also added a debug print before the
> > > KeAbortWaitThread call... let me know if this patch fixes anything or
> > > if the dprint shoes that the thread isn't really waiting.
> > >
> > > Best regards,
> > > Alex Ionescu
> >
> > I've test your changes (r20579 with r20601,20605,20606).
> > KeAbortWaitThread is called for waiting threads only. But I'm running in
> > another problem. Compiling ros on ros (with the nice parameter '-j2')
> > hangs after some time.
>
> When I try to selfhost with a -O2 build, I see a hang about 90% into
> the build.  The hang I see is *hard*, at least under vmware, nothing
> is reponding.   I don't use "-j2".
> > If I look to taskmgr or ctm, only the idle thread
> > consumes cpu power.
>
> I have seen this once, I don't remember what I was doing at the time,
> it may have compiling.
> > Sometimes I can stop the compiling with Ctrl-C,
> > sometimes not. I wasn't able to compile ros on the smp machine. On the
> > up machine, one of four compile runs does finish.
> > Hartmut

Currently I'm on r20600. After fixing some bugs (timer.c, gate.c and
wait.c), I'm able to compile ros on ros. After updating to r20601, it is
broken again. I commit the fixes if I reach the head revision.

- Hartmut

Index: ntoskrnl/ke/timer.c
===================================================================
--- ntoskrnl/ke/timer.c	(Revision 20601)
+++ ntoskrnl/ke/timer.c	(Arbeitskopie)
@@ -165,6 +165,11 @@
     return KeSetTimerEx(Timer, DueTime, 0, Dpc);
 }

+ULONG _help1;
+ULONG _help2;
+ULONG _help3;
+ULONG _help4;
+
 /*
  * @implemented
  *
@@ -247,6 +252,7 @@
     /* Query Interrupt Times */
     InterruptTime = KeQueryInterruptTime();

+    _help1 = 1;
     /* Loop through the Timer List and remove Expired Timers. Insert them into the Expired Listhead */
     LIST_FOR_EACH_SAFE(Timer, tmp, &KiTimerListHead, KTIMER, TimerListEntry)
     {
@@ -259,7 +265,9 @@
         RemoveEntryList(&Timer->TimerListEntry);
         InsertTailList(&ExpiredTimerList, &Timer->TimerListEntry);
     }
+    _help1 = 0;

+    _help2 = 1;
     /* Expire the Timers */
     while (!IsListEmpty(&ExpiredTimerList)) {

@@ -267,11 +275,13 @@

/* Get the Timer */
         Timer = CONTAINING_RECORD(CurrentEntry, KTIMER, TimerListEntry);
+        Timer->Header.Inserted = FALSE;
         DPRINT("Expiring Timer: %x\n", Timer);

/* Expire it */
         KiHandleExpiredTimer(Timer);
     }
+    _help2 = 0;

DPRINT("Timers expired\n");

@@ -301,7 +311,9 @@
     /* Set it as Signaled */
     DPRINT("Setting Timer as Signaled\n");
     Timer->Header.SignalState = TRUE;
+    _help3 = 1;
     KiWaitTest(&Timer->Header, IO_NO_INCREMENT);
+    _help3 = 0;

/* If the Timer is periodic, reinsert the timer with the new due time */
     if (Timer->Period) {
@@ -321,10 +333,11 @@
         DPRINT("Timer->Dpc %x Timer->Dpc->DeferredRoutine %x\n", Timer->Dpc, Timer->Dpc->DeferredRoutine);

/* Insert the DPC */
+        _help4 = 1;
         KeInsertQueueDpc(Timer->Dpc,
                          NULL,
                          NULL);
-
+	_help4 = 0;
         DPRINT("Finished dpc routine\n");
     }
 }
Index: ntoskrnl/ke/gate.c
===================================================================
--- ntoskrnl/ke/gate.c	(Revision 20601)
+++ ntoskrnl/ke/gate.c	(Arbeitskopie)
@@ -136,7 +136,6 @@
     /* Reschedule the Thread */
     DPRINT("Unblocking the Thread\n");
     KiUnblockThread(WaitThread, &WaitStatus, EVENT_INCREMENT);
-    return;

quit:
     /* Release the Dispatcher Database Lock */
Index: ntoskrnl/ke/wait.c
===================================================================
--- ntoskrnl/ke/wait.c	(Revision 20601)
+++ ntoskrnl/ke/wait.c	(Arbeitskopie)
@@ -152,6 +152,7 @@
         {
             /* FIXME: The timer already expired, we should find a new ready thread */
             Status = STATUS_SUCCESS;
+            CHECKPOINT1;
             break;
         }

@@ -329,6 +330,7 @@
             {
                 /* Return a timeout if we couldn't insert the timer */
                 Status = STATUS_TIMEOUT;
+                CHECKPOINT1;
                 goto DontWait;
             }
         }
@@ -560,6 +562,7 @@
             {
                 /* Return a timeout */
                 Status = STATUS_TIMEOUT;
+                CHECKPOINT1;
                 goto DontWait;
             }

@@ -808,6 +811,10 @@
     {
         KiDispatchThreadNoLock(Ready);
     }
+    else
+    {
+        KeReleaseDispatcherDatabaseLockFromDpcLevel();    
+    }

/* Lower irql back */
     KeLowerIrql(OldIrql);
Index: ntoskrnl/include/internal/ke.h
===================================================================
--- ntoskrnl/include/internal/ke.h	(Revision 20601)
+++ ntoskrnl/include/internal/ke.h	(Arbeitskopie)
@@ -62,7 +62,6 @@
 #define KeReleaseDispatcherDatabaseLockFromDpcLevel() \
     KeReleaseSpinLockFromDpcLevel(&DispatcherDatabaseLock);
 #define KeReleaseDispatcherDatabaseLock(OldIrql) \
-    KeReleaseSpinLockFromDpcLevel(&DispatcherDatabaseLock); \
     KiExitDispatcher(OldIrql);
 #endif

@@ -675,7 +674,7 @@

VOID
 NTAPI
-KeApplicationProcessorInit(VOID);
+KeApplicationProcessorInit(ULONG);

VOID
 NTAPI