Feature: implement ticket spinlock

test config: ./tools/configure.sh -l qemu-armv8a:nsh_smp Pass ostest No matter big-endian or little-endian, ticket spinlock only check the next and the owner is equal or not. If they are equal, it means there is a task hold the lock or lock is free. Signed-off-by: TaiJu Wu <[email protected]> Co-authored-by: Xiang Xiao <[email protected]>
TaiJuWu · Oct 5, 2023 · b80e2a5 · b80e2a5
1 parent 7901ed0
commit b80e2a5
Show file tree

Hide file tree

Showing 9 changed files with 147 additions and 24 deletions.
diff --git a/arch/arm64/src/common/arm64_cpustart.c b/arch/arm64/src/common/arm64_cpustart.c
@@ -33,7 +33,7 @@
 #include <nuttx/sched_note.h>
 #include <sched/sched.h>
 #include <nuttx/cache.h>
-#include <arch/spinlock.h>
+#include <nuttx/spinlock.h>
 #include <nuttx/init.h>
 
 #include "init/init.h"

diff --git a/boards/arm64/qemu/qemu-armv8a/configs/nsh_smp/defconfig b/boards/arm64/qemu/qemu-armv8a/configs/nsh_smp/defconfig
@@ -28,6 +28,8 @@ CONFIG_DEFAULT_TASK_STACKSIZE=16384
 CONFIG_DEV_ZERO=y
 CONFIG_EXAMPLES_HELLO=y
 CONFIG_EXPERIMENTAL=y
+CONFIG_SPINLOCK=y
+CONFIG_TICKET_SPINLOCK=y
 CONFIG_FS_PROCFS=y
 CONFIG_FS_PROCFS_REGISTER=y
 CONFIG_FS_ROMFS=y

diff --git a/include/nuttx/spinlock.h b/include/nuttx/spinlock.h
@@ -39,6 +39,26 @@
 typedef uint8_t spinlock_t;
 #else
 
+#ifdef CONFIG_TICKET_SPINLOCK
+
+#  define SP_UNLOCKED 0  /* The Un-locked state */
+#  define SP_LOCKED   1  /* The Locked state */
+
+#  define INIT_SPINLOCK_VALUE {{SP_UNLOCKED}}
+
+union spinlock_u
+{
+  struct
+  {
+    uint16_t owner;
+    uint16_t next;
+  } tickets;
+  uint32_t value;
+};
+typedef union spinlock_u spinlock_t;
+
+#else
+
 /* The architecture specific spinlock.h header file must also provide the
  * following:
  *
@@ -48,9 +68,12 @@ typedef uint8_t spinlock_t;
  *
  * SP_LOCKED and SP_UNLOCKED must be constants of type spinlock_t.
  */
-
 #include <arch/spinlock.h>
 
+#define INIT_SPINLOCK_VALUE SP_UNLOCKED
+
+#endif
+
 /****************************************************************************
  * Pre-processor Definitions
  ****************************************************************************/
@@ -84,6 +107,10 @@ typedef uint8_t spinlock_t;
 #  define __SP_UNLOCK_FUNCTION 1
 #endif
 
+#ifdef CONFIG_TICKET_SPINLOCK
+#  define __SP_UNLOCK_FUNCTION 1
+#endif
+
 /****************************************************************************
  * Public Function Prototypes
  ****************************************************************************/
@@ -199,7 +226,7 @@ void spin_lock_wo_note(FAR volatile spinlock_t *lock);
  *
  ****************************************************************************/
 
-spinlock_t spin_trylock(FAR volatile spinlock_t *lock);
+bool spin_trylock(FAR volatile spinlock_t *lock);
 
 /****************************************************************************
  * Name: spin_trylock_wo_note
@@ -223,7 +250,7 @@ spinlock_t spin_trylock(FAR volatile spinlock_t *lock);
  *
  ****************************************************************************/
 
-spinlock_t spin_trylock_wo_note(FAR volatile spinlock_t *lock);
+bool spin_trylock_wo_note(FAR volatile spinlock_t *lock);
 
 /****************************************************************************
  * Name: spin_unlock
@@ -285,7 +312,11 @@ void spin_unlock_wo_note(FAR volatile spinlock_t *lock);
  ****************************************************************************/
 
 /* bool spin_islocked(FAR spinlock_t lock); */
-#define spin_islocked(l) (*(l) == SP_LOCKED)
+#ifdef CONFIG_TICKET_SPINLOCK
+#  define spin_islocked(l) ((*l).tickets.owner != (*l).tickets.next)
+#else
+#  define spin_islocked(l) (*(l) == SP_LOCKED)
+#endif
 
 /****************************************************************************
  * Name: spin_setbit
@@ -352,9 +383,11 @@ void spin_clrbit(FAR volatile cpu_set_t *set, unsigned int cpu,
  ****************************************************************************/
 
 /* void spin_initialize(FAR spinlock_t *lock, spinlock_t state); */
-
-#define spin_initialize(l,s) do { *(l) = (s); } while (0)
-
+#ifdef CONFIG_TICKET_SPINLOCK
+#  define spin_initialize(l,s) do { (*l).value = (s); } while (0)
+#else
+#  define spin_initialize(l,s) do { *(l) = (s); } while (0)
+#endif
 /****************************************************************************
  * Name: spin_lock_irqsave
  *

diff --git a/sched/Kconfig b/sched/Kconfig
@@ -308,6 +308,16 @@ config SPINLOCK
 		CONFIG_ARCH_HAVE_MULTICPU.  This permits the use of spinlocks in
 		other novel architectures.
 
+if SPINLOCK
+
+config TICKET_SPINLOCK
+	bool "Use ticket Spinlocks"
+	default n
+	---help---
+		Use ticket spinlock algorithm.
+
+endif # SPINLOCK
+
 config IRQCHAIN
 	bool "Enable multi handler sharing a IRQ"
 	default n

diff --git a/sched/irq/irq_csection.c b/sched/irq/irq_csection.c
@@ -46,7 +46,7 @@
  * disabled.
  */
 
-volatile spinlock_t g_cpu_irqlock = SP_UNLOCKED;
+volatile spinlock_t g_cpu_irqlock = INIT_SPINLOCK_VALUE;
 
 /* Used to keep track of which CPU(s) hold the IRQ lock. */
 

diff --git a/sched/irq/irq_spinlock.c b/sched/irq/irq_spinlock.c
@@ -39,7 +39,7 @@
 
 /* Used for access control */
 
-static volatile spinlock_t g_irq_spin = SP_UNLOCKED;
+static volatile spinlock_t g_irq_spin = INIT_SPINLOCK_VALUE;
 
 /* Handles nested calls to spin_lock_irqsave and spin_unlock_irqrestore */
 

diff --git a/sched/sched/sched_lock.c b/sched/sched/sched_lock.c
@@ -97,7 +97,7 @@
  *    least one CPU has pre-emption disabled.
  */
 
-volatile spinlock_t g_cpu_schedlock = SP_UNLOCKED;
+volatile spinlock_t g_cpu_schedlock = INIT_SPINLOCK_VALUE;
 
 /* Used to keep track of which CPU(s) hold the IRQ lock. */
 

diff --git a/sched/semaphore/CMakeLists.txt b/sched/semaphore/CMakeLists.txt
@@ -40,4 +40,8 @@ if(CONFIG_SPINLOCK)
   list(APPEND CSRCS spinlock.c)
 endif()
 
+if(CONFIG_TICKET_SPINLOCK)
+  list(APPEND CSRCS spinlock.c)
+endif()
+
 target_sources(sched PRIVATE ${CSRCS})
diff --git a/sched/semaphore/spinlock.c b/sched/semaphore/spinlock.c
@@ -32,9 +32,13 @@
 #include <nuttx/sched_note.h>
 #include <arch/irq.h>
 
+#ifdef CONFIG_TICKET_SPINLOCK
+#  include <stdatomic.h>
+#endif
+
 #include "sched/sched.h"
 
-#ifdef CONFIG_SPINLOCK
+#if defined(CONFIG_SPINLOCK) || defined(CONFIG_TICKET_SPINLOCK)
 
 /****************************************************************************
  * Public Functions
@@ -71,7 +75,14 @@ void spin_lock(FAR volatile spinlock_t *lock)
   sched_note_spinlock(this_task(), lock, NOTE_SPINLOCK_LOCK);
 #endif
 
+#ifdef CONFIG_TICKET_SPINLOCK
+
+  uint16_t ticket = atomic_fetch_add(&lock->tickets.next, 1);
+  while (atomic_load(&lock->tickets.owner) != ticket)
+
+#else // CONFIG_SPINLOCK
   while (up_testset(lock) == SP_LOCKED)
+#endif
     {
       SP_DSB();
       SP_WFE();
@@ -109,7 +120,14 @@ void spin_lock(FAR volatile spinlock_t *lock)
 
 void spin_lock_wo_note(FAR volatile spinlock_t *lock)
 {
+#ifdef CONFIG_TICKET_SPINLOCK
+
+  uint16_t ticket = atomic_fetch_add(&lock->tickets.next, 1);
+  while (atomic_load(&lock->tickets.owner) != ticket)
+
+#else /* CONFIG_TICKET_SPINLOCK */
   while (up_testset(lock) == SP_LOCKED)
+#endif
     {
       SP_DSB();
       SP_WFE();
@@ -137,32 +155,52 @@ void spin_lock_wo_note(FAR volatile spinlock_t *lock)
  *
  ****************************************************************************/
 
-spinlock_t spin_trylock(FAR volatile spinlock_t *lock)
+bool spin_trylock(FAR volatile spinlock_t *lock)
 {
 #ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
   /* Notify that we are waiting for a spinlock */
 
   sched_note_spinlock(this_task(), lock, NOTE_SPINLOCK_LOCK);
 #endif
 
-  if (up_testset(lock) == SP_LOCKED)
+#ifdef CONFIG_TICKET_SPINLOCK
+  uint16_t ticket = atomic_load(&lock->tickets.next);
+
+  spinlock_t old =
+    {
+    {
+        ticket, ticket
+    }
+    };
+
+  spinlock_t new =
+    {
+    {
+        ticket, ticket + 1
+    }
+    };
+
+  if (atomic_compare_exchange_strong(&lock->value, &old.value, new.value))
+#else /* CONFIG_TICKET_SPINLOCK */
+  if (up_testset(lock) == SP_UNLOCKED)
+#endif /* CONFIG_TICKET_SPINLOCK */ 
     {
 #ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
       /* Notify that we abort for a spinlock */
 
       sched_note_spinlock(this_task(), lock, NOTE_SPINLOCK_ABORT);
 #endif
-      SP_DSB();
-      return SP_LOCKED;
+      SP_DMB();
+      return SP_UNLOCKED;
     }
 
 #ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
   /* Notify that we have the spinlock */
 
   sched_note_spinlock(this_task(), lock, NOTE_SPINLOCK_LOCKED);
 #endif
-  SP_DMB();
-  return SP_UNLOCKED;
+  SP_DSB();
+  return SP_LOCKED;
 }
 
 /****************************************************************************
@@ -187,16 +225,36 @@ spinlock_t spin_trylock(FAR volatile spinlock_t *lock)
  *
  ****************************************************************************/
 
-spinlock_t spin_trylock_wo_note(FAR volatile spinlock_t *lock)
+bool spin_trylock_wo_note(FAR volatile spinlock_t *lock)
 {
-  if (up_testset(lock) == SP_LOCKED)
+#ifdef CONFIG_TICKET_SPINLOCK
+  uint16_t ticket = atomic_load(&lock->tickets.next);
+
+  spinlock_t old =
     {
-      SP_DSB();
-      return SP_LOCKED;
+    {
+      ticket, ticket
     }
+    };
 
-  SP_DMB();
-  return SP_UNLOCKED;
+  spinlock_t new =
+    {
+    {
+      ticket, ticket + 1
+    }
+    };
+
+  if (atomic_compare_exchange_strong(&lock->value, &old.value, new.value))
+#else /* CONFIG_TICKET_SPINLOCK */
+  if (up_testset(lock) == SP_UNLOCKED)
+#endif /* CONFIG_TICKET_SPINLOCK */ 
+    {
+      SP_DMB();
+      return SP_UNLOCKED;
+    }
+
+  SP_DSB();
+  return SP_LOCKED;
 }
 
 /****************************************************************************
@@ -226,7 +284,11 @@ void spin_unlock(FAR volatile spinlock_t *lock)
 #endif
 
   SP_DMB();
+#ifdef CONFIG_TICKET_SPINLOCK
+  atomic_fetch_add(&lock->tickets.owner, 1);
+#else
   *lock = SP_UNLOCKED;
+#endif
   SP_DSB();
   SP_SEV();
 }
@@ -255,7 +317,11 @@ void spin_unlock(FAR volatile spinlock_t *lock)
 void spin_unlock_wo_note(FAR volatile spinlock_t *lock)
 {
   SP_DMB();
+#ifdef CONFIG_TICKET_SPINLOCK
+  atomic_fetch_add(&lock->tickets.owner, 1);
+#else
   *lock = SP_UNLOCKED;
+#endif
   SP_DSB();
   SP_SEV();
 }
@@ -303,7 +369,11 @@ void spin_setbit(FAR volatile cpu_set_t *set, unsigned int cpu,
   prev    = *set;
 #endif
   *set   |= (1 << cpu);
+#ifdef CONFIG_TICKET_SPINLOCK
+  orlock->value = SP_LOCKED;
+#else
   *orlock = SP_LOCKED;
+#endif
 
 #ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
   if (prev == 0)
@@ -366,7 +436,11 @@ void spin_clrbit(FAR volatile cpu_set_t *set, unsigned int cpu,
   prev    = *set;
 #endif
   *set   &= ~(1 << cpu);
+#ifdef CONFIG_TICKET_SPINLOCK
+  orlock->value = (*set != 0) ? SP_LOCKED : SP_UNLOCKED;
+#else
   *orlock = (*set != 0) ? SP_LOCKED : SP_UNLOCKED;
+#endif
 
 #ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
   if (prev != 0 && *set == 0)