Still some issues, but it seems to not crash anymore

diff --git a/Kernel/arch/x86/include/arch.h b/Kernel/arch/x86/include/arch.h
index b3f7545..0126be3 100644 (file)
--- a/Kernel/arch/x86/include/arch.h
+++ b/Kernel/arch/x86/include/arch.h
@@ -12,6 +12,7 @@
 
 // Allow nested spinlocks?
 #define STACKED_LOCKS  1
+#define LOCK_DISABLE_INTS      0
 
 // - Processor/Machine Specific Features
 #if ARCH != i386 && ARCH != i486 && ARCH != i586
@@ -38,7 +39,9 @@
  */
 struct sShortSpinlock {
        volatile int    Lock;   //!< Lock value
+       #if LOCK_DISABLE_INTS
         int    IF;     //!< Interrupt state on call to SHORTLOCK
+       #endif
        #if STACKED_LOCKS
         int    Depth;
        #endif
@@ -50,6 +53,16 @@ struct sShortSpinlock {
 static inline int IS_LOCKED(struct sShortSpinlock *Lock) {
        return !!Lock->Lock;
 }
+
+/**
+ * \brief Check if the current CPU has the lock
+ * \param Lock Lock pointer
+ */
+static inline int CPU_HAS_LOCK(struct sShortSpinlock *Lock) {
+       extern int      GetCPUNum(void);
+       return Lock->Lock == GetCPUNum() + 1;
+}
+
 /**
  * \brief Acquire a Short Spinlock
  * \param Lock Lock pointer
@@ -64,15 +77,19 @@ static inline int IS_LOCKED(struct sShortSpinlock *Lock) {
  */
 static inline void SHORTLOCK(struct sShortSpinlock *Lock) {
         int    v = 1;
+       #if LOCK_DISABLE_INTS
         int    IF;
+       #endif
        #if STACKED_LOCKS
        extern int      GetCPUNum(void);
         int    cpu = GetCPUNum() + 1;
        #endif
        
+       #if LOCK_DISABLE_INTS
        // Save interrupt state and clear interrupts
        __ASM__ ("pushf;\n\tpop %%eax\n\tcli" : "=a"(IF));
        IF &= 0x200;    // AND out all but the interrupt flag
+       #endif
        
        #if STACKED_LOCKS
        if( Lock->Lock == cpu ) {
@@ -87,16 +104,18 @@ static inline void SHORTLOCK(struct sShortSpinlock *Lock) {
                // CMPXCHG:
                //  If r/m32 == EAX, set ZF and set r/m32 = r32
                //  Else, clear ZF and set EAX = r/m32
-               __ASM__("lock cmpxchgl %%ecx, (%%edi)"
+               __ASM__("lock cmpxchgl %2, (%3)"
                        : "=a"(v)
-                       : "a"(0), "c"(cpu), "D"(&Lock->Lock)
+                       : "a"(0), "r"(cpu), "r"(&Lock->Lock)
                        );
                #else
                __ASM__("xchgl %%eax, (%%edi)":"=a"(v):"a"(1),"D"(&Lock->Lock));
                #endif
        }
        
+       #if LOCK_DISABLE_INTS
        Lock->IF = IF;
+       #endif
 }
 /**
  * \brief Release a short lock
@@ -109,6 +128,8 @@ static inline void SHORTREL(struct sShortSpinlock *Lock) {
                return ;
        }
        #endif
+       
+       #if LOCK_DISABLE_INTS
        // Lock->IF can change anytime once Lock->Lock is zeroed
        if(Lock->IF) {
                Lock->Lock = 0;
@@ -117,6 +138,9 @@ static inline void SHORTREL(struct sShortSpinlock *Lock) {
        else {
                Lock->Lock = 0;
        }
+       #else
+       Lock->Lock = 0;
+       #endif
 }
 
 // === MACROS ===

diff --git a/Kernel/threads.c b/Kernel/threads.c
index 2670a1d..aa550ab 100644 (file)
--- a/Kernel/threads.c
+++ b/Kernel/threads.c
@@ -43,7 +43,6 @@ void  Threads_Yield(void);
 void   Threads_Sleep(void);
  int   Threads_Wake(tThread *Thread);
 void   Threads_AddActive(tThread *Thread);
-void   Threads_int_AddActive(tThread *Thread);
 tThread        *Threads_RemActive(void);
  int   Threads_GetPID(void);
  int   Threads_GetTID(void);
@@ -565,15 +564,17 @@ int Threads_Wake(tThread *Thread)
                return -EALREADY;
        
        case THREAD_STAT_SLEEPING:
+               SHORTLOCK( &glThreadListLock );
                // Remove from sleeping queue
                prev = Threads_int_GetPrev(&gSleepingThreads, Thread);
                prev->Next = Thread->Next;
                
-               Threads_int_AddActive( Thread );
+               Threads_AddActive( Thread );
                
                #if DEBUG_TRACE_STATE
                Log("Threads_Sleep: %p (%i %s) woken", Thread, Thread->TID, Thread->ThreadName);
                #endif
+               SHORTREL( &glThreadListLock );
                return -EOK;
        
        case THREAD_STAT_WAITING:
@@ -601,9 +602,7 @@ int Threads_WakeTID(tTID TID)
         int    ret;
        if(!thread)
                return -ENOENT;
-       SHORTLOCK( &glThreadListLock );
        ret = Threads_Wake( thread );
-       SHORTREL( &glThreadListLock );
        //Log_Debug("Threads", "TID %i woke %i (%p)", Threads_GetTID(), TID, thread);
        return ret;
 }
@@ -614,16 +613,25 @@ int Threads_WakeTID(tTID TID)
 void Threads_AddActive(tThread *Thread)
 {
        SHORTLOCK( &glThreadListLock );
-       Threads_int_AddActive(Thread);
-       SHORTREL( &glThreadListLock );
-}
-
-/**
- * \brief Adds a thread to the active queue
- * \note This version MUST have the thread list lock held
- */
-void Threads_int_AddActive(tThread *Thread)
-{
+       
+       #if 1
+       {
+               tThread *t;
+               for( t = gActiveThreads; t; t = t->Next )
+               {
+                       if( t == Thread ) {
+                               Panic("Threads_AddActive: Attempting a double add of TID %i (0x%x)",
+                                       Thread->TID, __builtin_return_address(0));
+                       }
+                       
+                       if(t->Status != THREAD_STAT_ACTIVE) {
+                               Panic("Threads_AddActive: TID %i status != THREAD_STAT_ACTIVE",
+                                       Thread->TID);
+                       }
+               }
+       }
+       #endif
+       
        // Add to active list
        Thread->Next = gActiveThreads;
        gActiveThreads = Thread;
@@ -636,8 +644,10 @@ void Threads_int_AddActive(tThread *Thread)
        giFreeTickets += Thread->NumTickets;
        
        #if DEBUG_TRACE_TICKETS
-       Log("Threads_int_AddActive: new giFreeTickets = %i", giFreeTickets);
+       Log("Threads_AddActive: %p %i (%s) added, new giFreeTickets = %i",
+               Thread, Thread->TID, Thread->ThreadName, giFreeTickets);
        #endif
+       SHORTREL( &glThreadListLock );
 }
 
 /**
@@ -648,12 +658,30 @@ void Threads_int_AddActive(tThread *Thread)
 tThread *Threads_RemActive(void)
 {
        tThread *ret = Proc_GetCurThread();
-       tThread *prev = Threads_int_GetPrev(&gActiveThreads, ret);
-       if(!prev)       return NULL;
+       tThread *prev;
+       
+       SHORTLOCK( &glThreadListLock );
+       
+       prev = Threads_int_GetPrev(&gActiveThreads, ret);
+       if(!prev) {
+               SHORTREL( &glThreadListLock );
+               return NULL;
+       }
+       
        ret->Remaining = 0;
        ret->CurCPU = -1;
+       
        prev->Next = ret->Next;
        giNumActiveThreads --;
+       // no need to decrement tickets, scheduler did it for us
+       
+       #if DEBUG_TRACE_TICKETS
+       Log("Threads_RemActive: %p %i (%s) removed, giFreeTickets = %i",
+               ret, ret->TID, ret->ThreadName, giFreeTickets);
+       #endif
+       
+       SHORTREL( &glThreadListLock );
+       
        return ret;
 }
 
@@ -748,13 +776,14 @@ int Threads_SetGID(Uint *Errno, tGID ID)
 
 /**
  * \fn void Threads_Dump(void)
- * \brief Dums a list of currently running threads
+ * \brief Dumps a list of currently running threads
  */
 void Threads_Dump(void)
 {
        tThread *thread;
        
-       Log("Active Threads:");
+       Log("--- Thread Dump ---");
+       Log("Active Threads: (%i reported)", giNumActiveThreads);
        for(thread=gActiveThreads;thread;thread=thread->Next)
        {
                Log(" %i (%i) - %s (CPU %i)",
@@ -775,6 +804,24 @@ void Threads_Dump(void)
                Log("  KStack 0x%x", thread->KernelStack);
        }
 }
+/**
+ * \fn void Threads_Dump(void)
+ */
+void Threads_DumpActive(void)
+{
+       tThread *thread;
+       
+       Log("Active Threads:");
+       for(thread=gActiveThreads;thread;thread=thread->Next)
+       {
+               Log(" %i (%i) - %s (CPU %i)",
+                       thread->TID, thread->TGID, thread->ThreadName, thread->CurCPU);
+               if(thread->Status != THREAD_STAT_ACTIVE)
+                       Log("  ERROR State (%i) != THREAD_STAT_ACTIVE (%i)", thread->Status, THREAD_STAT_ACTIVE);
+               Log("  %i Tickets, Quantum %i", thread->NumTickets, thread->Quantum);
+               Log("  KStack 0x%x", thread->KernelStack);
+       }
+}
 
 /**
  * \brief Gets the next thread to run
@@ -787,6 +834,10 @@ tThread *Threads_GetNextToRun(int CPU, tThread *Last)
         int    ticket;
         int    number; 
        
+       // If this CPU has the lock, we must let it complete
+       if( CPU_HAS_LOCK( &glThreadListLock ) )
+               return Last;
+       
        // Lock thread list
        SHORTLOCK( &glThreadListLock );
        
@@ -824,9 +875,12 @@ tThread *Threads_GetNextToRun(int CPU, tThread *Last)
        if(giNumActiveThreads == 1) {
                if( gActiveThreads->CurCPU == -1 )
                        gActiveThreads->CurCPU = CPU;
+               
                SHORTREL( &glThreadListLock );
+               
                if( gActiveThreads->CurCPU == CPU )
                        return gActiveThreads;
+               
                return NULL;    // CPU has nothing to do
        }
        
@@ -841,15 +895,27 @@ tThread *Threads_GetNextToRun(int CPU, tThread *Last)
                }
                #if DEBUG_TRACE_TICKETS
                else
-                       LogF(" %p (%s)->Status = %i (Released)\n", Last, Last->ThreadName, Last->Status);
+                       LogF(" CPU %i released %p (%s)->Status = %i (Released)\n",
+                               CPU, Last, Last->ThreadName, Last->Status);
                #endif
                Last->CurCPU = -1;
        }
        
+       #if DEBUG_TRACE_TICKETS
+       //Threads_DumpActive();
+       #endif
+       
        #if 1
        number = 0;
        for(thread = gActiveThreads; thread; thread = thread->Next) {
                if(thread->CurCPU >= 0) continue;
+               if(thread->Status != THREAD_STAT_ACTIVE)
+                       Panic("Bookkeeping fail - %p %i(%s) is on the active queue with a status of %i",
+                               thread, thread->TID, thread->ThreadName, thread->Status);
+               if(thread->Next == thread) {
+                       Panic("Bookkeeping fail - %p %i(%s) loops back on itself",
+                               thread, thread->TID, thread->ThreadName, thread->Status);
+               }
                number += thread->NumTickets;
        }
        if(number != giFreeTickets) {
@@ -974,9 +1040,12 @@ void Mutex_Release(tMutex *Mutex)
        if( Mutex->Waiting ) {
                Mutex->Owner = Mutex->Waiting;  // Set owner
                Mutex->Waiting = Mutex->Waiting->Next;  // Next!
+               
                // Wake new owner
-               Threads_AddActive(Mutex->Owner);
-               //Log("Mutex %p Woke %p", Mutex, Mutex->Owner);
+               SHORTLOCK( &glThreadListLock );
+               if( Mutex->Owner->Status != THREAD_STAT_ACTIVE )
+                       Threads_AddActive(Mutex->Owner);
+               SHORTREL( &glThreadListLock );
        }
        else {
                Mutex->Owner = NULL;