Cleanup Semaphore/SpinLock

src/kernel/exception/Bluescreen.h

@@ -18,5 +18,4 @@
 // dump blue screen (will not return)
 void bs_dump(uint8_t exceptionNr);
 
-
 #endif

src/lib/async/Semaphore.cc

@@ -5,7 +5,9 @@
 
 namespace Async {
 
-void Semaphore::p() {
+Semaphore::Semaphore(uint32_t c) : counter(c) {}
+
+void Semaphore::acquire() {
     // Lock to allow deterministic operations on counter/queue
     lock.acquire();
 
@@ -15,7 +17,7 @@ void Semaphore::p() {
         lock.release();
     } else {
         // Block and manage thread in semaphore queue until it's woken up by v() again
-        if (!wait_queue.initialized()) {  // TODO: I will replace this suboptimal datastructure in the future
+        if (!wait_queue.initialized()) {
             wait_queue.reserve();
         }
         auto &schedulerService = Kernel::System::getService<Kernel::SchedulerService>();
@@ -27,7 +29,7 @@ void Semaphore::p() {
     }
 }
 
-void Semaphore::v() {
+void Semaphore::release() {
     lock.acquire();
 
     if (!wait_queue.empty()) {

src/lib/async/Semaphore.h

@@ -1,41 +1,43 @@
-/*****************************************************************************
- *                                                                           *
- *                             S E M A P H O R E                             *
- *                                                                           *
- *---------------------------------------------------------------------------*
- * Beschreibung:    Implementierung von Sempahor-Objekten.                   *
- *                                                                           *
- * Autor:           Michael Schoettner, 2.9.2016                             *
- *****************************************************************************/
-
 #ifndef Semaphore_include__
 #define Semaphore_include__
 
-#include "kernel/process/Thread.h"
 #include "SpinLock.h"
-#include "lib/container//Vector.h"
+#include "kernel/process/Thread.h"
+#include "lib/container/Vector.h"
+#include "lib/util/RestrictedConstructors.h"
 
 namespace Async {
 
 class Semaphore {
-private:
-    // Queue fuer wartende Threads.
-    Container::Vector<unsigned int> wait_queue;
-    SpinLock lock;
-
-    int counter;
-
 public:
-    Semaphore(const Semaphore &copy) = delete;  // Verhindere Kopieren
+    /**
+     * Initialize the semaphore and it's counter.
+     *
+     * @param c The semaphore counter
+     */
+    explicit Semaphore(uint32_t c);
 
-    // Konstruktor: Initialisieren des Semaphorzaehlers
+    MakeUncopyable(Semaphore)
-    explicit Semaphore(int c) : counter(c) {}
 
-    // 'Passieren': Warten auf das Freiwerden eines kritischen Abschnitts.
+    MakeUnmovable(Semaphore)
-    void p();
 
-    // 'Vreigeben': Freigeben des kritischen Abschnitts.
+    ~Semaphore() = default;
-    void v();
+
+    /**
+     * Wait in queue until the semaphore can be acquired.
+     */
+    void acquire();
+
+    /**
+     * Release the semaphore.
+     */
+    void release();
+
+private:
+    SpinLock lock;
+    Container::Vector<unsigned int> wait_queue; // TODO: Replace with a queue (linked list)
+
+    uint32_t counter;
 };
 
 }

src/lib/async/SpinLock.cc

@@ -1,66 +1,28 @@
-/*****************************************************************************
- *                                                                           *
- *                              S P I N L O C K                              *
- *                                                                           *
- *---------------------------------------------------------------------------*
- * Beschreibung:    Implementierung eines Spinlocks mithilfe der cmpxchg     *
- *                  Instruktion.                                             *
- *                                                                           *
- * Autor:           Michael Schoettner, 2.2.2018                             *
- *****************************************************************************/
-
 #include "SpinLock.h"
 
 namespace Async {
 
-/*****************************************************************************
+// TODO: Outsource to Atomic lib
- * Methode:         CAS                                                      *
- *---------------------------------------------------------------------------*
- * Parameter:       *ptr    Adresse der Variable des Locks                   *
- *                   old    Wert gegen den verglichen wird                   *
- *                  _new    Wert der gesetzt werden soll                     *
- *                                                                           *
- * Beschreibung:    Semantik der Funktion CAS = Cmompare & Swap:             *
- *                      if old == *ptr then                                  *
- *                          *ptr := _new                                     *
- *                      return prev                                          *
- *****************************************************************************/
 static inline uint32_t CAS(const uint32_t *ptr) {
     uint32_t prev;
 
-    /*
+    asm volatile("lock;" // prevent race conditions with other cores
-        AT&T/UNIX assembly syntax
+                 "cmpxchg %1, %2;" // %1 = _new; %2 = *ptr
+            : "=a"(prev) // output: =a: RAX -> prev (%0))
+            : "r"(1), "m"(*ptr), "a"(0) // input = %1, %2, %3 (r=register, m=memory, a=accumlator = eax
+            : "memory"); // ensures assembly block will not be moved by gcc
 
-        The 'volatile' keyword after 'asm' indicates that the instruction 
+    return prev; // return pointer instead of prev to prevent unnecessary second call
-        has important side-effects. GCC will not delete a volatile asm if 
-        sit is reachable.
-     */
-    asm volatile("lock;"                           // prevent race conditions with other cores
-                 "cmpxchg %1, %2;"                 // %1 = _new; %2 = *ptr
-        // constraints
-            : "=a"(prev)                      // output: =a: RAX -> prev (%0))
-            : "r"(1), "m"(*ptr), "a"(0)  // input = %1, %2, %3 (r=register, m=memory, a=accumlator = eax
-            : "memory");                      // ensures assembly block will not be moved by gcc
-
-    return prev;  // return pointer instead of prev to prevent unnecessary second call
 }
 
-/*****************************************************************************
+SpinLock::SpinLock() : lock(0), ptr(&lock) {}
- * Methode:         SpinLock::acquire                                        *
+
- *---------------------------------------------------------------------------*
- * Beschreibung:    Lock belegen.                                            *
- *****************************************************************************/
 void SpinLock::acquire() {
     // If lock == 0 the SpinLock can be aquired without waiting
     // If lock == 1 the  while loop blocks until aquired
     while (CAS(ptr) != 0) {}
 }
 
-/*****************************************************************************
- * Methode:         SpinLock::release                                        *
- *---------------------------------------------------------------------------*
- * Beschreibung:    Lock freigeben.                                          *
- *****************************************************************************/
 void SpinLock::release() {
     lock = 0;
 }

src/lib/async/SpinLock.h

@@ -13,22 +13,33 @@
 #define SpinLock_include__
 
 #include <cstdint>
+#include "lib/util/RestrictedConstructors.h"
 
 namespace Async {
 
 class SpinLock {
+public:
+    SpinLock();
+
+    MakeUncopyable(SpinLock)
+
+    MakeUnmovable(SpinLock)
+
+    ~SpinLock() = default;
+
+    /**
+     * Acquire the spin lock.
+     */
+    void acquire();
+
+    /**
+     * Release the spin lock.
+     */
+    void release();
+
 private:
     uint32_t lock;
     uint32_t *ptr;
-
-public:
-    SpinLock(const SpinLock &copy) = delete;  // Verhindere Kopieren
-
-    SpinLock() : lock(0), ptr(&lock) {}
-
-    void acquire();
-
-    void release();
 };
 
 }

src/lib/stream/CGA_Stream.h

@@ -62,9 +62,9 @@ public:
 
     // ~CGA_Stream() override = default;
 
-    void lock() { sem.p(); }
+    void lock() { sem.acquire(); }
 
-    void unlock() { sem.v(); }
+    void unlock() { sem.release(); }
 
     // Methode zur Ausgabe des Pufferinhalts der Basisklasse StringBuffer.
     void flush() override;