sched_note: Add spinlock instrumentation; In SMP configurations, select to log only notes from certain CPUs

include/nuttx/sched_note.h

@@ -50,6 +50,22 @@
 
 #ifdef CONFIG_SCHED_INSTRUMENTATION
 
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Provide defaults for some configuration settings (could be undefined with
+ * old configuration files)
+ */
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_CPUSET
+#  define CONFIG_SCHED_INSTRUMENTATION_CPUSET 0xffff
+#endif
+
+#ifdef CONFIG_SCHED_NOTE_BUFSIZE
+#  define CONFIG_SCHED_NOTE_BUFSIZE 2048
+#endif
+
 /****************************************************************************
  * Public Types
  ****************************************************************************/
@@ -81,6 +97,13 @@ enum note_type_e
   NOTE_CSECTION_ENTER,
   NOTE_CSECTION_LEAVE
 #endif
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
+  ,
+  NOTE_SPINLOCK_LOCK,
+  NOTE_SPINLOCK_LOCKED,
+  NOTE_SPINLOCK_UNLOCK,
+  NOTE_SPINLOCK_ABORT
+#endif
 };
 
 /* This structure provides the common header of each note */
@@ -182,6 +205,17 @@ struct note_csection_s
 #endif
 };
 #endif /* CONFIG_SCHED_INSTRUMENTATION_CSECTION */
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
+/* This is the specific form of the NOTE_SPINLOCK_LOCK/LOCKED/UNLOCK/ABORT note */
+
+struct note_spinlock_s
+{
+  struct note_common_s nsp_cmn; /* Common note parameters */
+  FAR void *nsp_spinlock;       /* Address of spinlock */
+  uint8_t nsp_value;            /* Value of spinlock */
+};
+#endif /* CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS */
 #endif /* CONFIG_SCHED_INSTRUMENTATION_BUFFER */
 
 /****************************************************************************
@@ -237,6 +271,18 @@ void sched_note_csection(FAR struct tcb_s *tcb, bool enter);
 #  define sched_note_csection(t,e)
 #endif
 
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
+void sched_note_spinlock(FAR struct tcb_s *tcb, FAR volatile void *spinlock);
+void sched_note_spinlocked(FAR struct tcb_s *tcb, FAR volatile void *spinlock);
+void sched_note_spinunlock(FAR struct tcb_s *tcb, FAR volatile void *spinlock);
+void sched_note_spinabort(FAR struct tcb_s *tcb, FAR volatile void *spinlock);
+#else
+#  define sched_note_spinlock(t,s)
+#  define sched_note_spinlocked(t,s)
+#  define sched_note_spinunlock(t,s)
+#  define sched_note_spinabort(t,s)
+#endif
+
 /****************************************************************************
  * Name: sched_note_get
  *
@@ -311,6 +357,10 @@ int note_register(void);
 #  define sched_note_cpu_resumed(t)
 #  define sched_note_premption(t,l)
 #  define sched_note_csection(t,e)
+#  define sched_note_spinlock(t,s)
+#  define sched_note_spinlocked(t,s)
+#  define sched_note_spinunlock(t,s)
+#  define sched_note_spinabort(t,s)
 
 #endif /* CONFIG_SCHED_INSTRUMENTATION */
 #endif /* __INCLUDE_NUTTX_SCHED_NOTE_H */

include/nuttx/spinlock.h

@@ -80,6 +80,10 @@
 #  define SP_DSB()
 #endif
 
+#if defined(CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS) && !defined(__SP_UNLOCK_FUNCTION)
+#  define __SP_UNLOCK_FUNCTION 1
+#endif
+
 /* If the target CPU supports a data cache then it may be necessary to
  * manage spinlocks in a special way, perhaps linking them all into a
  * special non-cacheable memory region.

sched/Kconfig

@@ -678,6 +678,13 @@ config SCHED_INSTRUMENTATION
 
 if SCHED_INSTRUMENTATION
 
+config SCHED_INSTRUMENTATION_CPUSET
+	hex "CPU bit set"
+	default 0xffff
+	depends on SMP
+	---help---
+		Monitor only CPUs in the bitset.  Bit 0=CPU0, Bit1=CPU1, etc.
+
 config SCHED_INSTRUMENTATION_PREEMPTION
 	bool "Preemption monitor hooks"
 	default n
@@ -687,10 +694,15 @@ config SCHED_INSTRUMENTATION_PREEMPTION
 
 			void sched_note_premption(FAR struct tcb_s *tcb, bool state);
 
+config SCHED_INSTRUMENTATION_FAUXPAS
+	bool
+	default y if !EXPERIMENTAL && SCHED_INSTRUMENTATION_BUFFER
+	default n if EXPERIMENTAL || !SCHED_INSTRUMENTATION_BUFFER
+
 config SCHED_INSTRUMENTATION_CSECTION
 	bool "Critical section monitor hooks"
 	default n
-	depends on EXPERIMENTAL || !SCHED_INSTRUMENTATION_BUFFER
+	depends on !SCHED_INSTRUMENTATION_FAUXPAS
 	---help---
 		Enables additional hooks for entry and exit from critical sections.
 		Interrupts are disabled while within a critical section.  Board-
@@ -706,6 +718,27 @@ config SCHED_INSTRUMENTATION_CSECTION
 		added from the note buffer in order to remove one entry.  Not
 		very useful in its current state!
 
+config SCHED_INSTRUMENTATION_SPINLOCK
+	bool "Spinlock monitor hooks"
+	default n
+	depends on SPINLOCK && (!SMP || !SCHED_INSTRUMENTATION_FAUXPAS)
+	---help---
+		Enables additional hooks for spinlock state.  Board-specific logic
+		must provide this additional logic.
+
+			void sched_note_spinlock(FAR struct tcb_s *tcb, bool state);
+			void sched_note_spinlocked(FAR struct tcb_s *tcb, bool state);
+			void sched_note_spinunlock(FAR struct tcb_s *tcb, bool state);
+			void sched_note_spinabort(FAR struct tcb_s *tcb, bool state);
+
+		NOTE: This option is marked EXPERIMENTAL because there is a logical
+		error in the design when this feature is used with
+		CONFIG_SCHED_INSTRUMENTATION_BUFFER.  That error is that
+		sched_note_get() calls enter_ and leave_critical_section which use
+		spinlocks in SMP mode.  That means that each call to sched_note_get()
+		causes several additional entries to be added from the note buffer in
+		order to remove one entry.  Not very useful in its current state!
+
 config SCHED_INSTRUMENTATION_BUFFER
 	bool "Buffer instrumentation data in memory"
 	default n

sched/irq/irq_csection.c

@@ -128,6 +128,14 @@ static uint8_t g_cpu_nestcount[CONFIG_SMP_NCPUS];
 #ifdef CONFIG_SMP
 static inline bool irq_waitlock(int cpu)
 {
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
+  FAR struct tcb_s *tcb = this_task();
+
+  /* Notify that we are waiting for a spinlock */
+
+  sched_note_spinlock(tcb, &g_cpu_irqlock);
+#endif
+
   /* Duplicate the spin_lock() logic from spinlock.c, but adding the check
    * for the deadlock condition.
    */
@@ -142,6 +150,12 @@ static inline bool irq_waitlock(int cpu)
            * Abort the wait and return false.
            */
 
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
+          /* Notify that we are waiting for a spinlock */
+
+          sched_note_spinabort(tcb, &g_cpu_irqlock);
+#endif
+
           return false;
         }
 
@@ -150,6 +164,12 @@ static inline bool irq_waitlock(int cpu)
 
   /* We have g_cpu_irqlock! */
 
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
+  /* Notify that we have the spinlock */
+
+  sched_note_spinlocked(tcb, &g_cpu_irqlock);
+#endif
+
   SP_DMB();
   return true;
 }

sched/sched/sched_note.c

@@ -46,8 +46,11 @@
 
 #include <nuttx/sched.h>
 #include <nuttx/clock.h>
+#include <nuttx/spinlock.h>
 #include <nuttx/sched_note.h>
 
+#include "sched/sched.h"
+
 #ifdef CONFIG_SCHED_INSTRUMENTATION_BUFFER
 
 /****************************************************************************
@@ -122,8 +125,10 @@ static inline unsigned int note_next(unsigned int ndx, unsigned int offset)
  *   Fill in some of the common fields in the note structure.
  *
  * Input Parameters:
- *   tcb  - The TCB containing the information
+ *   tcb    - The TCB containing the information
- *   note - The common note structure to use
+ *   note   - The common note structure to use
+ *   length - The total lengthof the note structure
+ *   type   - The type of the note
  *
  * Returned Value:
  *   None
@@ -154,6 +159,39 @@ static void note_common(FAR struct tcb_s *tcb, FAR struct note_common_s *note,
   note->nc_systime[3] = (uint8_t)((systime >> 24) & 0xff);
 }
 
+/****************************************************************************
+ * Name: note_spincommon
+ *
+ * Description:
+ *   Common logic for NOTE_SPINLOCK, NOTE_SPINLOCKED, and NOTE_SPINUNLOCK
+ *
+ * Input Parameters:
+ *   tcb  - The TCB containing the information
+ *   note - The common note structure to use
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
+void note_spincommon(FAR struct tcb_s *tcb, FAR volatile spinlock_t *spinlock,
+                     int type)
+{
+  struct note_spinlock_s note;
+
+  /* Format the note */
+
+  note_common(tcb, &note.nsp_cmn, sizeof(struct note_spinlock_s), type);
+  note.nsp_spinlock = (FAR void *)spinlock;
+  note.nsp_value    = (uint8_t)*spinlock;
+
+  /* Add the note to circular buffer */
+
+  note_add((FAR const uint8_t *)&note, sizeof(struct note_spinlock_s));
+}
+#endif
+
 /****************************************************************************
  * Name: note_length
  *
@@ -244,6 +282,17 @@ static void note_add(FAR const uint8_t *note, uint8_t notelen)
   unsigned int head;
   unsigned int next;
 
+#ifdef CONFIG_SMP
+  /* Ignore notes that are not in the set of monitored CPUs */
+
+  if ((CONFIG_SCHED_INSTRUMENTATION_CPUSET & (1 << this_cpu())) == 0)
+    {
+      /* Not in the set of monitored CPUs.  Do not log the note. */
+
+      return;
+    }
+#endif
+
   /* Get the index to the head of the circular buffer */
 
   DEBUGASSERT(note != NULL && notelen < CONFIG_SCHED_NOTE_BUFSIZE);
@@ -463,6 +512,27 @@ void sched_note_csection(FAR struct tcb_s *tcb, bool enter)
 }
 #endif
 
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
+void sched_note_spinlock(FAR struct tcb_s *tcb, FAR volatile void *spinlock)
+{
+  note_spincommon(tcb, spinlock, NOTE_SPINLOCK_LOCK)
+}
+
+void sched_note_spinlocked(FAR struct tcb_s *tcb, FAR volatile void *spinlock);
+{
+  note_spincommon(tcb, spinlock, NOTE_SPINLOCK_LOCKED)
+}
+
+void sched_note_spinunlock(FAR struct tcb_s *tcb, FAR volatile void *spinlock);
+{
+  note_spincommon(tcb, spinlock, NOTE_SPINLOCK_UNLOCK)
+}
+void sched_note_spinabort(FAR struct tcb_s *tcb, FAR volatile void *spinlock);
+{
+  note_spincommon(tcb, spinlock, NOTE_SPINLOCK_ABORT)
+}
+#endif
+
 /****************************************************************************
  * Name: sched_note_get
  *

sched/semaphore/spinlock.c

@@ -44,6 +44,7 @@
 #include <assert.h>
 
 #include <nuttx/spinlock.h>
+#include <nuttx/sched_note.h>
 #include <arch/irq.h>
 
 #include "sched/sched.h"
@@ -119,11 +120,22 @@ void spin_initializer(FAR struct spinlock_s *lock)
 
 void spin_lock(FAR volatile spinlock_t *lock)
 {
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
+  /* Notify that we are waiting for a spinlock */
+
+  sched_note_spinlock(this_task(), lock);
+#endif
+
   while (up_testset(lock) == SP_LOCKED)
     {
       SP_DSB();
     }
 
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
+  /* Notify that we have the spinlock */
+
+  sched_note_spinlocked(this_task(), lock);
+#endif
   SP_DMB();
 }
 
@@ -147,6 +159,12 @@ void spin_lock(FAR volatile spinlock_t *lock)
 #ifdef __SP_UNLOCK_FUNCTION
 void spin_unlock(FAR volatile spinlock_t *lock)
 {
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
+  /* Notify that we are unlocking the spinlock */
+
+  sched_note_spinunlock(this_task(), lock);
+#endif
+
   *lock = SP_UNLOCKED;
   SP_DMB();
 }
@@ -208,6 +226,13 @@ void spin_lockr(FAR struct spinlock_s *lock)
 
 #  warning Missing logic
 #endif
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
+      /* Notify that we are waiting for a spinlock */
+
+      sched_note_spinlock(this_task(), &lock->sp_lock);
+#endif
+
       /* Take the lock.  REVISIT:  We should set an indication in the TCB
        * that the thread is spinning.  This might be useful in determining
        * some scheduling actions?
@@ -221,6 +246,12 @@ void spin_lockr(FAR struct spinlock_s *lock)
           SP_DSB();
         }
 
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
+      /* Notify that we have thespinlock */
+
+      sched_note_spinlocked(this_task(), &lock->sp_lock);
+#endif
+
       SP_DMB();
 
       /* Take one count on the lock */
@@ -233,6 +264,12 @@ void spin_lockr(FAR struct spinlock_s *lock)
 
 #else /* CONFIG_SMP */
 
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
+  /* Notify that we are waiting for a spinlock */
+
+  sched_note_spinlock(this_task(), &lock->sp_lock);
+#endif
+
   /* Take the lock.  REVISIT:  We should set an indication in the TCB that
    * the thread is spinning.  This might be useful in determining some
    * scheduling actions?
@@ -244,6 +281,12 @@ void spin_lockr(FAR struct spinlock_s *lock)
       SP_DSB()
     }
 
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
+  /* Notify that we have thespinlock */
+
+  sched_note_spinlocked(this_task(), &lock->sp_lock);
+#endif
+
   SP_DMB();
 #endif /* CONFIG_SMP */
 }
@@ -303,6 +346,11 @@ void spin_unlockr(FAR struct spinlock_s *lock)
 
       if (lock->sp_count <= 1)
         {
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
+          /* Notify that we are unlocking the spinlock */
+
+          sched_note_spinunlock(this_task(), &lock->sp_lock);
+#endif
           /* The count must decremented to zero */
 
           lock->sp_count = 0;
@@ -318,6 +366,13 @@ void spin_unlockr(FAR struct spinlock_s *lock)
   up_irq_restore(flags);
 
 #else /* CONFIG_SMP */
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
+  /* Notify that we are unlocking the spinlock */
+
+  sched_note_spinunlock(this_task(), &lock->sp_lock);
+#endif
+
   /* Just mark the spinlock unlocked */
 
   DEBUGASSERT(lock != NULL && lock->sp_lock == SP_LOCKED);
@@ -347,15 +402,31 @@ void spin_setbit(FAR volatile cpu_set_t *set, unsigned int cpu,
                  FAR volatile spinlock_t *setlock,
                  FAR volatile spinlock_t *orlock)
 {
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
+  cpu_set_t prev;
+#endif
+
   /* First, get the 'setlock' spinlock */
 
   spin_lock(setlock);
 
   /* Then set the bit and mark the 'orlock' as locked */
 
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
+  prev    = *set;
+#endif
   *set   |= (1 << cpu);
   *orlock = SP_LOCKED;
 
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
+  if (prev == 0)
+    {
+      /* Notify that we have locked the spinlock */
+
+      sched_note_spinlocked(this_task(), orlock);
+    }
+#endif
+
   /* Release the 'setlock' */
 
   spin_unlock(setlock);
@@ -382,6 +453,10 @@ void spin_clrbit(FAR volatile cpu_set_t *set, unsigned int cpu,
                  FAR volatile spinlock_t *setlock,
                  FAR volatile spinlock_t *orlock)
 {
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
+  cpu_set_t prev;
+#endif
+
   /* First, get the 'setlock' spinlock */
 
   spin_lock(setlock);
@@ -390,9 +465,21 @@ void spin_clrbit(FAR volatile cpu_set_t *set, unsigned int cpu,
    * upon the resulting state of the CPU set.
    */
 
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
+  prev    = *set;
+#endif
   *set   &= ~(1 << cpu);
   *orlock = (*set != 0) ? SP_LOCKED : SP_UNLOCKED;
 
+#ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
+  if (prev != 0 && *set == 0)
+    {
+      /* Notify that we have unlocked the spinlock */
+
+      sched_note_spinunlock(this_task(), orlock);
+    }
+#endif
+
   /* Release the 'setlock' */
 
   spin_unlock(setlock);