Cleanup Scheduler

src/kernel/interrupt/IntDispatcher.cc

@@ -25,7 +25,7 @@ int IntDispatcher::assign(uint8_t vector, ISR &isr) {
     /* hier muss Code eingefuegt werden */
 
     handlerMap[vector] = &isr;
-    log.info() << "Registered ISR for vector " << dec << vector << endl;
+    log.info() << "Registered ISR for vector " << dec << vector << endl; // TODO: Numbers don't show up in log
 
     return 0;
 }
@@ -37,7 +37,7 @@ int IntDispatcher::dispatch(uint8_t vector) {
     ISR *isr = handlerMap[vector];
 
     if (isr == nullptr) {
-        log.error() << "No ISR registered for vector " << vector << endl;
+        log.error() << "No ISR registered for vector " << vector << endl; // TODO: Numbers don't show up in log
 
         // TODO: Throw exception
 

src/kernel/process/Scheduler.cc

@@ -9,47 +9,33 @@ Scheduler::Scheduler() {
     // TODO: Cleanup thread
 }
 
-void Scheduler::start(Container::Vector<Memory::unique_ptr<Thread>>::iterator next) {
-    active = next;
-    if (active >= ready_queue.end()) {
-        active = ready_queue.begin();
-    }
-    (*active)->start();  // First dereference the Iterator, then the unique_ptr to get Thread
-}
-
-void Scheduler::switch_to(Thread *prev_raw, Container::Vector<Memory::unique_ptr<Thread>>::iterator next) {
-    active = next;
-    if (active >= ready_queue.end()) {
-        active = ready_queue.begin();
-        // log.debug() << "Scheduler::switch_to started different thread than passed" << endl;
-    }
-    prev_raw->switchTo(**active);
-}
-
-/*****************************************************************************
- * Methode:         Scheduler::ready                                         *
- *---------------------------------------------------------------------------*
- * Beschreibung:    Thread in readyQueue eintragen.                          *
- *****************************************************************************/
 void Scheduler::ready(Memory::unique_ptr<Thread> &&thread) {
     Device::CPU::disable_int();
     ready_queue.push_back(std::move(thread));
     Device::CPU::enable_int();
 }
 
-/*****************************************************************************
+void Scheduler::start(Container::Vector<Memory::unique_ptr<Thread>>::iterator next) {
- * Methode:         Scheduler::exit                                          *
+    active = next;
- *---------------------------------------------------------------------------*
+    if (active >= ready_queue.end()) {
- * Beschreibung:    Thread ist fertig und terminiert sich selbst. Hier muss  *
+        // By jumping to the start of the queue we can always start the next ready thread by
- *                  nur auf den naechsten Thread mithilfe des Dispatchers    *
+        // incrementing the "active" iterator
- *                  umgeschaltet werden. Der aktuell laufende Thread ist     *
+        active = ready_queue.begin();
- *                  nicht in der readyQueue.                                 *
+    }
- *****************************************************************************/
+    (*active)->start(); // First dereference the Iterator, then the unique_ptr to get the Thread
-void Scheduler::exit() {
+}
 
-    /* hier muss Code eingefuegt werden */
+// Use a raw pointer for the previous thread because the unique_ptr of that thread is still in the ready_queue,
+// this way we don't need to take care of moving it to this function and then back to the ready_queue
+void Scheduler::start(Thread *prev_raw, Container::Vector<Memory::unique_ptr<Thread>>::iterator next) {
+    active = next;
+    if (active >= ready_queue.end()) {
+        active = ready_queue.begin();
+    }
+    prev_raw->switchTo(**active); // First dereference the Iterator, then the unique_ptr to get the Thread
+}
 
-    // Thread-Wechsel durch PIT verhindern
+void Scheduler::yield() {
     Device::CPU::disable_int();
 
     if (ready_queue.size() == 1) {
@@ -57,7 +43,53 @@ void Scheduler::exit() {
         return;
     }
 
-    start(ready_queue.erase(active));  // erase returns the next iterator after the erased element
+    start((*active).get(), active + 1); // prev_raw is valid since no thread was killed/deleted
+}
+
+void Scheduler::block() {
+    Device::CPU::disable_int();
+
+    if (ready_queue.size() == 1) {
+        Device::CPU::enable_int();
+        return;
+    }
+
+    Thread *prev_raw = (*active).get(); // Has to be done before erasing/adding the thread to the block_queue
+    std::size_t pos = distance(ready_queue.begin(), active); // Current thread's index in ready_queue
+    block_queue.push_back(std::move(ready_queue[pos])); // Leaves an "empty shell" in the ready_queue
+
+    start(prev_raw, ready_queue.erase(active)); // prev_raw is valid as it was taken before vector move/erase
+}
+
+void Scheduler::deblock(uint16_t tid) {
+    Device::CPU::disable_int();
+
+    for (Container::Vector<Memory::unique_ptr<Thread>>::iterator it = block_queue.begin();
+         it != block_queue.end(); ++it) {
+        if ((*it)->tid == tid) {
+            // Found thread with correct tid
+
+            std::size_t pos = distance(block_queue.begin(), it);
+            ready_queue.insert(active + 1, std::move(block_queue[pos]));  // We insert the thread after the active
+            // thread to prefer deblocked threads
+            block_queue.erase(it);
+            Device::CPU::enable_int();
+            return;
+        }
+    }
+
+    Device::CPU::enable_int();
+}
+
+void Scheduler::exit() {
+    Device::CPU::disable_int();
+
+    if (ready_queue.size() == 1) {
+        Device::CPU::enable_int();
+        return;
+    }
+
+    start(ready_queue.erase(active)); // erase returns the next iterator after the erased element
     // cannot use switch_to here as the previous thread no longer
     // exists (was deleted by erase)
 
@@ -65,25 +97,13 @@ void Scheduler::exit() {
     // dispatch kehrt nicht zurueck
 }
 
-/*****************************************************************************
- * Methode:         Scheduler::kill                                          *
- *---------------------------------------------------------------------------*
- * Beschreibung:    Thread mit 'Gewalt' terminieren. Er wird aus der         *
- *                  readyQueue ausgetragen und wird dann nicht mehr aufge-   *
- *                  rufen. Der Aufrufer dieser Methode muss ein anderer      *
- *                  Thread sein.                                             *
- *                                                                           *
- * Parameter:                                                                *
- *      that        Zu terminierender Thread                                 *
- *****************************************************************************/
 void Scheduler::kill(uint16_t tid, Memory::unique_ptr<Thread> *ptr) {
     Device::CPU::disable_int();
 
     uint16_t prev_tid = (*active)->tid;
 
     // Block queue, can always kill
-    for (Container::Vector<Memory::unique_ptr<Thread>>::iterator it = block_queue.begin();
+    for (auto it = block_queue.begin(); it != block_queue.end(); ++it) {
-         it != block_queue.end(); ++it) {
         if ((*it)->tid == tid) {
             // Found thread to kill
 
@@ -136,7 +156,11 @@ void Scheduler::kill(uint16_t tid, Memory::unique_ptr<Thread> *ptr) {
     Device::CPU::enable_int();
 }
 
-// TODO: Can't retrive the thread right now because it's not clear when it's finished,
+void Scheduler::kill(uint16_t tid) {
+    kill(tid, nullptr);
+}
+
+// TODO: Can't retrieve the thread right now because it's not clear when it's finished,
 //       maybe introduce a exited_queue and get it from there
 void Scheduler::nice_kill(uint16_t tid, Memory::unique_ptr<Thread> *ptr) {
     Device::CPU::disable_int();
@@ -161,91 +185,8 @@ void Scheduler::nice_kill(uint16_t tid, Memory::unique_ptr<Thread> *ptr) {
     Device::CPU::enable_int();
 }
 
-/*****************************************************************************
+void Scheduler::nice_kill(uint16_t tid) {
- * Methode:         Scheduler::yield                                         *
+    nice_kill(tid, nullptr);
- *---------------------------------------------------------------------------*
- * Beschreibung:    CPU::freiwillig abgeben und Auswahl des naechsten Threads.*
- *                  Naechsten Thread aus der readyQueue holen, den aktuellen *
- *                  in die readyQueue wieder eintragen. Das Umschalten soll  *
- *                  mithilfe des Dispatchers erfolgen.                       *
- *                                                                           *
- *                  Achtung: Falls nur der Idle-Thread läuft, so ist die     *
- *                           readyQueue leer.                                *
- *****************************************************************************/
-void Scheduler::yield() {
-
-    /* hier muss Code eingefuegt werden */
-
-    // Thread-Wechsel durch PIT verhindern
-    Device::CPU::disable_int();
-
-    if (ready_queue.size() == 1) {
-        Device::CPU::enable_int();
-        return;
-    }
-    switch_to((*active).get(), active + 1);  // prev_raw is valid since no thread was killed/deleted
-}
-
-/*****************************************************************************
- * Methode:         Scheduler::block                                         *
- *---------------------------------------------------------------------------*
- * Beschreibung:    Aufrufer ist blockiert. Es soll auf den naechsten Thread *
- *                  umgeschaltet werden. Der Aufrufer soll nicht in die      *
- *                  readyQueue eingefuegt werden und wird extern verwaltet.  *
- *                  Wird bei uns nur fuer Semaphore verwendet. Jede Semaphore*
- *                  hat eine Warteschlange wo der Thread dann verwaltet wird.*
- *                  Die Methode kehrt nicht zurueck, sondern schaltet um.    *
- *****************************************************************************/
-void Scheduler::block() {
-
-    /* hier muss Code eingefuegt werden */
-
-    Device::CPU::disable_int();
-
-    if (ready_queue.size() == 1) {
-        Device::CPU::enable_int();
-        return;
-    }
-
-    Thread *prev_raw = (*active).get();
-    std::size_t pos = distance(ready_queue.begin(), active);
-    block_queue.push_back(std::move(ready_queue[pos]));
-
-    switch_to(prev_raw, ready_queue.erase(active));  // prev_raw is valid as thread was moved before vector erase
-}
-
-/*****************************************************************************
- * Methode:         Scheduler::deblock                                       *
- *---------------------------------------------------------------------------*
- * Beschreibung:    Thread 'that' deblockieren. 'that' wird nur in die       *
- *                  readyQueue eingefuegt und dann zurueckgekehrt. In der    *
- *                  einfachsten Form entspricht diese Funktion exakt 'ready' *
- *                  Man koennte alternativ aber den deblockierten Thread auch*
- *                  am Anfang der readyQueue einfuegen, um ihn zu beorzugen. *
- *                                                                           *
- * Parameter:       that:  Thread der deblockiert werden soll.               *
- *****************************************************************************/
-void Scheduler::deblock(uint16_t tid) {
-
-    /* hier muss Code eingefuegt werden */
-
-    Device::CPU::disable_int();
-
-    for (Container::Vector<Memory::unique_ptr<Thread>>::iterator it = block_queue.begin();
-         it != block_queue.end(); ++it) {
-        if ((*it)->tid == tid) {
-            // Found thread with correct tid
-
-            std::size_t pos = distance(block_queue.begin(), it);
-            ready_queue.insert(active + 1, std::move(block_queue[pos]));  // We insert the thread after the active
-            // thread to prefer deblocked threads
-            block_queue.erase(it);
-            Device::CPU::enable_int();
-            return;
-        }
-    }
-
-    Device::CPU::enable_int();
 }
 
 }

src/kernel/process/Scheduler.h

@@ -1,84 +1,109 @@
-/*****************************************************************************
- *                                                                           *
- *                          S C H E D U L E R                                *
- *                                                                           *
- *---------------------------------------------------------------------------*
- * Beschreibung:    Implementierung eines einfachen Zeitscheiben-Schedulers. *
- *                  Rechenbereite Threads werden in 'readQueue' verwaltet.   *
- *                                                                           *
- * Autor:           Michael, Schoettner, HHU, 22.8.2016                      *
- *****************************************************************************/
-
 #ifndef Scheduler_include__
 #define Scheduler_include__
 
 #include "Thread.h"
 #include "lib/memory/UniquePointer.h"
-#include "lib/stream/Logger.h"
 #include "lib/container/Vector.h"
+#include "lib/util/RestrictedConstructors.h"
 
 namespace Kernel {
 
+/**
+ * This class implements a simple round-robin scheduler.
+ */
 class Scheduler {
     friend class SchedulerService;
 
-    friend class IdleThread;
-
 public:
     /**
      * Constructor. Initializes the IdleThread.
      */
     Scheduler();
 
-    Scheduler(const Scheduler &copy) = delete;  // Verhindere Kopieren
+    MakeUncopyable(Scheduler)
 
-    // TODO: Rest of constructors
+    MakeUnmovable(Scheduler)
+
+    ~Scheduler() = default;
 
 private:
-    [[nodiscard]] uint16_t get_active() const {
+    /**
-        return (*active)->tid;
+     * Place a thread in the scheduler's ready_queue.
-    }
+     *
-
+     * @param thread The thread to add to the ready_queue
+     */
     void ready(Memory::unique_ptr<Thread> &&thread);
 
-    // Roughly the old dispatcher functionality
+    /**
-    void start(Container::Vector<Memory::unique_ptr<Thread>>::iterator next); // Start next without prev
+     * Start/continue a thread independently of the previous running thread.
+     * Is applicable when the previous thread terminated/was killed.
+     *
+     * @param next The next thread to run
+     */
+    void start(Container::Vector<Memory::unique_ptr<Thread>>::iterator next);
 
-    // Switch from prev to next
+    /**
-    void switch_to(Thread *prev_raw, Container::Vector<Memory::unique_ptr<Thread>>::iterator next);
+     * Start/continue a thread.
+     * Is applicable when the previous thread wants to continue execution at a later point.
+     *
+     * @param prev_raw The raw pointer to the previous running thread
+     * @param next The next thread to run
+     */
+    void start(Thread *prev_raw, Container::Vector<Memory::unique_ptr<Thread>>::iterator next);
 
-    // CPU freiwillig abgeben und Auswahl des naechsten Threads
+    /**
-    void yield(); // Returns when only the idle thread runs
+     * Give CPU time to the next waiting thread from the ready_queue.
+     */
+    void yield();
 
-    // Blocks current thread (move to block_queue)
+    /**
-    void block(); // Returns on error because we don't have exceptions
+     * Block a thread from receiving CPU time.
+     * Blocked threads are managed in the block_queue.
+     */
+    void block();
 
-    // Deblock by tid (move to ready_queue)
+    /**
+     * Allow a blocked thread to receive CPU time again.
+     *
+     * @param tid
+     */
     void deblock(uint16_t tid);
 
-    // Thread terminiert sich selbst
+    /**
-    // NOTE: When a thread exits itself it will disappear...
+     * Remove a thread from the ready_queue and give CPU time to the next waiting thread.
-    //       Maybe put exited threads in an exited queue?
+     * Exited threads are managed in the exit_queue.
-    //       Then they would have to be acquired from there to exit...
+     */
-    void exit();  // Returns on error because we don't have exceptions
+    void exit();
 
-    // Thread mit 'Gewalt' terminieren
+    /**
+     * Forcefully exit a thread and retrieve it.
+     *
+     * @param tid The thread to exit
+     * @param ptr The pointer to the killed thread for external use
+     */
     void kill(uint16_t tid, Memory::unique_ptr<Thread> *ptr);
 
-    void kill(uint16_t tid) { kill(tid, nullptr); }
+    /**
+     * Forcefully exit a thread and discard it.
+     *
+     * @param tid The thread to exit
+     */
+    void kill(uint16_t tid);
 
+    // TODO: Remove this
     // Asks thread to exit
     // NOTE: I had many problems with killing threads that were stuck in some semaphore
     //       or were involved in any locking mechanisms, so with this a thread can make sure
     //       to "set things right" before exiting itself (but could also be ignored)
+    // TODO: "Setting things right" should be done in the thread's destructor
     void nice_kill(uint16_t tid, Memory::unique_ptr<Thread> *ptr);
 
-    void nice_kill(uint16_t tid) { nice_kill(tid, nullptr); }
+    void nice_kill(uint16_t tid);
 
 private:
     Container::Vector<Memory::unique_ptr<Thread>> ready_queue;
     Container::Vector<Memory::unique_ptr<Thread>> block_queue;
-    Container::Vector<Memory::unique_ptr<Thread>> exited; // TODO: Manage exited threads
+    Container::Vector<Memory::unique_ptr<Thread>> exit_queue; // TODO: Manage exited threads
 
     // It makes sense to keep track of the active thread through this as it makes handling the
     // unique_ptr easier and reduces the copying in the vector when cycling through the threads

src/kernel/process/Thread.h

@@ -33,24 +33,23 @@
 namespace Kernel {
 
 class Thread {
-    friend class SchedulerService;
-
     friend class Scheduler;
 
-protected:
+public:
-    // TODO: Remove this
-    bool running = true; // For soft exit, if thread uses infinite loop inside run(), use this as condition
-
-    // TODO: Public this
     uint16_t tid; // Thread-ID (wird im Konstruktor vergeben)
 
-public:
+    /**
-    // TODO: Is this a good idea?
+     * Reserved thread ids.
+     */
     enum Threads : uint16_t {
         IDLE = 0,
         CLEANUP = 1 // TODO: Can cleanup be done in a thread?
     };
 
+protected:
+    // TODO: Remove this
+    bool running = true; // For soft exit, if thread uses infinite loop inside run(), use this as condition
+
 public:
     Thread();
 
@@ -63,6 +62,10 @@ public:
         delete[] stack;
     }
 
+    // Methode des Threads, muss in Sub-Klasse implementiert werden
+    virtual void run() = 0;
+
+private:
     // Thread aktivieren
     void start() const;
 
@@ -72,9 +75,6 @@ public:
     // Ask thread to terminate itself
     void suicide() { running = false; }
 
-    // Methode des Threads, muss in Sub-Klasse implementiert werden
-    virtual void run() = 0;
-
 private:
     uint32_t *stack;
     uint32_t esp;

src/kernel/service/InterruptService.h

@@ -7,9 +7,14 @@
 
 namespace Kernel {
 
+// NOTE: I mainly have this to be able to implement additional interrupt controllers easily
+
+/**
+ * This class implements the interrupt system service.
+ */
 class InterruptService : public Service {
 public:
-    static const constexpr uint8_t ID = 0;
+    static const constexpr uint8_t ID = Service::INTERRUPT;
 
 public:
     InterruptService() = default;

src/kernel/service/SchedulerService.cpp

@@ -7,7 +7,7 @@ void SchedulerService::startScheduling() {
 }
 
 uint16_t SchedulerService::active() const {
-    return scheduler.get_active();
+    return (*scheduler.active)->tid;
 }
 
 void SchedulerService::yield() {

src/kernel/service/SchedulerService.h

@@ -8,6 +8,8 @@
 
 namespace Kernel {
 
+// NOTE: I mainly have this to be able to implement additional schedulers easily
+
 /**
  * This class implements the scheduling system service.
  */
@@ -49,7 +51,7 @@ public:
 
     void exit();
 
-    void suicide(uint16_t tid);
+    void suicide(uint16_t tid); // TODO: Remove
 
     void kill(uint16_t tid);
 

src/kernel/service/Service.h

@@ -5,6 +5,11 @@
 
 namespace Kernel {
 
+/**
+ * Implements the super class of all system services.
+ * Services abstract certain systems/devices so the used implementation
+ * can be changed easily. Every subclass needs a constexpr member ID.
+ */
 class Service {
 public:
     Service() = default;
@@ -20,6 +25,18 @@ public:
     Service &operator=(Service &&move) = delete;
 
     virtual ~Service() = default;
+
+protected:
+    /**
+     * Assigns each service implementation a unique ID.
+     */
+    enum Services : uint8_t {
+        INTERRUPT = 0,
+        SCHEDULER = 1,
+        MEMORY = 2,
+        EVENT = 3,
+        TIME = 4
+    };
 };
 
 }