Semaphores: Provide macros for sem_setprotobol() and sem_getprotocol() if priority inheritance is not enabled. More SEM_PRIO_* definitions to include/nuttx/semaphore.h

TODO

@@ -9,7 +9,7 @@ issues related to each board port.
 
 nuttx/:
 
- (13)  Task/Scheduler (sched/)
+ (14)  Task/Scheduler (sched/)
   (1)  Memory Management (mm/)
   (1)  Power Management (drivers/pm)
   (3)  Signals (sched/signal, arch/)
@@ -216,6 +216,55 @@ o Task/Scheduler (sched/)
   Status:      Open
   Priority:    Medium-ish
 
+  Title:       ISSUES WITH PRIORITY INHERITANCE WHEN SEMAPHORE USED AS IPC
+  Description: The semaphores have multiple uses. The typical usage is where
+               the semaphore is used as lock on one or more resources.  In
+               this typical case, priority inheritance works perfectly:  The
+               holder of a semaphore count must be remembered so that its
+               priority can be boosted if a higher priority task requires a
+               count from the semaphore.  It remains the holder until is
+               calls sem_post() to release the count on the semaphore.
+
+               But a different usage model for semaphores is for signalling
+               events.  In this case, the semaphore count is initialized to
+               zero and the receiving task calls sem_wait() to wait for the
+               next event of interest.  When an event of interest is
+               detected by another task (or even an interrupt handler),
+               sem_post() is called which increments the count to 1 and
+               wakes up the receiving task.
+
+               For example, in the following TASK A waits for events and
+               TASK B (or perhaps an interrupt handler) signals task A of
+               the occurence of the events by posting the semaphore:
+
+                 TASK A                 TASK B
+                 sem_init(sem, 0, 0);
+                 sem_wait(sem);
+                                        sem_post(sem);
+                 Awakens as holder
+
+               These two usage models are really very different and priority
+               inheritance simply does not apply when the semaphore is used for
+               signalling rather than locking.  In this signalling case
+               priority inheritance can interfere with the operation of the
+               semaphore.  The problem is that when TASK A is awakened it is
+               a holder of the semaphore.  Normally, a task is removed from
+               the holder list when it finally release the 
+              
+               However, TASK A never calls sem_post(sem) so it becomes
+               *permanently* a holder of the semaphore and may have its
+               priority boosted when any other task tries to acquire the
+               semaphore.
+
+               The fix is to call sem_setprotocol(SEM_PRIO_NONE) immediately
+               after the sem_init() call so that there will be no priority
+               inheritance operations on this semaphore used for signalling.
+  Status:      Open
+  Priority:    High.  If you have priority inheritance enabled and you use
+               semaphores for signalling events, then you *must* call
+               sem_setprotocol(SEM_PRIO_NONE) immediately after initializing
+               the semaphore.
+
   Title:       SCALABILITY
   Description: Task control information is retained in simple lists.  This
                is completely appropriate for small embedded systems where

include/nuttx/semaphore.h

@@ -51,6 +51,12 @@
  * Pre-processor Definitions
  ****************************************************************************/
 
+/* Values for protocol attribute */
+
+#define SEM_PRIO_NONE             0
+#define SEM_PRIO_INHERIT          1
+#define SEM_PRIO_PROTECT          2
+
 /****************************************************************************
  * Public Type Definitions
  ****************************************************************************/
@@ -67,9 +73,7 @@ struct nsem_inode_s
 
   sem_t ns_sem;                     /* The contained semaphore */
 
-  /* Inode payload unique to named semaphores.  ns_inode must appear first
+  /* Inode payload unique to named semaphores. */
-   * in this structure in order to support casting between type sem_t and
-   * types of struct nsem_inode_s. */
 
   FAR struct inode *ns_inode;       /* Containing inode */
 };
@@ -158,6 +162,8 @@ int sem_reset(FAR sem_t *sem, int16_t count);
 
 #ifdef CONFIG_PRIORITY_INHERITANCE
 int sem_getprotocol(FAR sem_t *sem, FAR int *protocol);
+#else
+#  define sem_getprotocol(s,p) do { *(p) == SEM_PRIO_NONE); } while (0)
 #endif
 
 /****************************************************************************
@@ -166,6 +172,25 @@ int sem_getprotocol(FAR sem_t *sem, FAR int *protocol);
  * Description:
  *    Set semaphore protocol attribute.
  *
+ *    One particularly important use of this function is when a semaphore
+ *    is used for inter-task communication like:
+ *
+ *      TASK A                 TASK B
+ *      sem_init(sem, 0, 0);
+ *      sem_wait(sem);
+ *                             sem_post(sem);
+ *      Awakens as holder
+ *
+ *    In this case priority inheritance can interfere with the operation of
+ *    the semaphore.  The problem is that when TASK A is restarted it is a
+ *    holder of the semaphore.  However, it never calls sem_post(sem) so it
+ *    becomes *permanently* a holder of the semaphore and may have its
+ *    priority boosted when any other task tries to acquire the semaphore.
+ *
+ *    The fix is to call sem_setprotocol(SEM_PRIO_NONE) immediately after
+ *    the sem_init() call so that there will be no priority inheritance
+ *    operations on this semaphore.
+ *
  * Parameters:
  *    sem      - A pointer to the semaphore whose attributes are to be
  *               modified
@@ -179,6 +204,8 @@ int sem_getprotocol(FAR sem_t *sem, FAR int *protocol);
 
 #ifdef CONFIG_PRIORITY_INHERITANCE
 int sem_setprotocol(FAR sem_t *sem, int protocol);
+#else
+#  define sem_setprotocol(s,p)  DEBUGASSERT((p) == SEM_PRIO_NONE);
 #endif
 
 #undef EXTERN

include/pthread.h

@@ -40,18 +40,19 @@
  * Included Files
  ********************************************************************************/
 
-#include <nuttx/config.h>   /* Default settings */
+#include <nuttx/config.h>    /* Default settings */
-#include <nuttx/compiler.h> /* Compiler settings, noreturn_function */
+#include <nuttx/compiler.h>  /* Compiler settings, noreturn_function */
 
-#include <sys/types.h>      /* Needed for general types */
+#include <sys/types.h>       /* Needed for general types */
-#include <sys/prctl.h>      /* Needed by pthread_[set|get]name_np */
+#include <sys/prctl.h>       /* Needed by pthread_[set|get]name_np */
 
-#include <stdint.h>         /* C99 fixed width integer types */
+#include <stdint.h>          /* C99 fixed width integer types */
-#include <stdbool.h>        /* C99 boolean types */
+#include <stdbool.h>         /* C99 boolean types */
-#include <unistd.h>         /* For getpid */
+#include <unistd.h>          /* For getpid */
-#include <semaphore.h>      /* Needed for sem_t */
+#include <signal.h>          /* Needed for sigset_t, includes this file */
-#include <signal.h>         /* Needed for sigset_t, includes this file */
+#include <time.h>            /* Needed for struct timespec */
-#include <time.h>           /* Needed for struct timespec */
+
+#include <nuttx/semaphore.h> /* For sem_t and SEM_PRIO_* defines */
 
 /********************************************************************************
  * Pre-processor Definitions
@@ -112,9 +113,7 @@
 #define PTHREAD_INHERIT_SCHED         0
 #define PTHREAD_EXPLICIT_SCHED        1
 
-#define PTHREAD_PRIO_NONE             0
+/* Default priority */
-#define PTHREAD_PRIO_INHERIT          1
-#define PTHREAD_PRIO_PROTECT          2
 
 #define PTHREAD_DEFAULT_PRIORITY      100
 
@@ -135,7 +134,7 @@
 
 #define PTHREAD_BARRIER_SERIAL_THREAD 0x1000
 
-/* Values for protocol attribute */
+/* Values for protocol mutex attribute */
 
 #define PTHREAD_PRIO_NONE             SEM_PRIO_NONE
 #define PTHREAD_PRIO_INHERIT          SEM_PRIO_INHERIT

include/semaphore.h

@@ -45,24 +45,10 @@
 #include <stdint.h>
 #include <limits.h>
 
-#ifdef __cplusplus
-#define EXTERN extern "C"
-extern "C"
-{
-#else
-#define EXTERN extern
-#endif
-
 /****************************************************************************
  * Pre-processor Definitions
  ****************************************************************************/
 
-/* Values for protocol attribute */
-
-#define SEM_PRIO_NONE             0
-#define SEM_PRIO_INHERIT          1
-#define SEM_PRIO_PROTECT          2
-
 /* Bit definitions for the struct sem_s flags field */
 
 #define PRIOINHERIT_FLAGS_DISABLE (1 << 0)  /* Bit 0: Priority inheritance
@@ -130,6 +116,14 @@ typedef struct sem_s sem_t;
  * Public Data
  ****************************************************************************/
 
+#ifdef __cplusplus
+#define EXTERN extern "C"
+extern "C"
+{
+#else
+#define EXTERN extern
+#endif
+
 /****************************************************************************
  * Public Function Prototypes
  ****************************************************************************/

sched/semaphore/sem_setprotocol.c

@@ -58,6 +58,25 @@
  * Description:
  *    Set semaphore protocol attribute.
  *
+ *    One particularly important use of this furnction is when a semaphore
+ *    is used for inter-task communication like:
+ *
+ *      TASK A                 TASK B
+ *      sem_init(sem, 0, 0);
+ *      sem_wait(sem);
+ *                             sem_post(sem);
+ *      Awakens as holder
+ *
+ *    In this case priority inheritance can interfere with the operation of
+ *    the semaphore.  The problem is that when TASK A is restarted it is a
+ *    holder of the semaphore.  However, it never calls sem_post(sem) so it
+ *    becomes *permanently* a holder of the semaphore and may have its
+ *    priority boosted when any other task tries to acquire the semaphore.
+ *
+ *    The fix is to call sem_setprotocol(SEM_PRIO_NONE) immediately after
+ *    the sem_init() call so that there will be no priority inheritance
+ *    operations on this semaphore.
+ *
  * Parameters:
  *    sem      - A pointer to the semaphore whose attributes are to be
  *               modified