pthread: remove enter_critical_section in pthread_barrier_wait

include/nuttx/spinlock.h

@@ -41,6 +41,8 @@
 #  include <nuttx/atomic.h>
 #endif
 
+#include <nuttx/spinlock_type.h>
+
 #undef EXTERN
 #if defined(__cplusplus)
 #define EXTERN extern "C"
@@ -50,50 +52,6 @@ extern "C"
 #define EXTERN extern
 #endif
 
-#if defined(CONFIG_RW_SPINLOCK)
-typedef int rwlock_t;
-#  define RW_SP_UNLOCKED      0
-#  define RW_SP_READ_LOCKED   1
-#  define RW_SP_WRITE_LOCKED -1
-#endif
-
-#ifndef CONFIG_SPINLOCK
-#  define SP_UNLOCKED 0  /* The Un-locked state */
-#  define SP_LOCKED   1  /* The Locked state */
-
-typedef uint8_t spinlock_t;
-#elif defined(CONFIG_TICKET_SPINLOCK)
-
-union spinlock_u
-{
-  struct
-  {
-    unsigned short owner;
-    unsigned short next;
-  } tickets;
-  unsigned int value;
-};
-typedef union spinlock_u spinlock_t;
-
-#  define SP_UNLOCKED (union spinlock_u){{0, 0}}
-#  define SP_LOCKED (union spinlock_u){{0, 1}}
-
-#else
-
-/* The architecture specific spinlock.h header file must also provide the
- * following:
- *
- *   SP_LOCKED   - A definition of the locked state value (usually 1)
- *   SP_UNLOCKED - A definition of the unlocked state value (usually 0)
- *   spinlock_t  - The type of a spinlock memory object.
- *
- * SP_LOCKED and SP_UNLOCKED must be constants of type spinlock_t.
- */
-
-#include <arch/spinlock.h>
-
-#endif /* CONFIG_SPINLOCK */
-
 /****************************************************************************
  * Pre-processor Definitions
  ****************************************************************************/

include/nuttx/spinlock_type.h

@@ -0,0 +1,90 @@
+/****************************************************************************
+ * include/nuttx/spinlock_type.h
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_NUTTX_SPINLOCK_TYPE_H
+#define __INCLUDE_NUTTX_SPINLOCK_TYPE_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#undef EXTERN
+#if defined(__cplusplus)
+#define EXTERN extern "C"
+extern "C"
+{
+#else
+#define EXTERN extern
+#endif
+
+#if defined(CONFIG_RW_SPINLOCK)
+typedef int rwlock_t;
+#  define RW_SP_UNLOCKED      0
+#  define RW_SP_READ_LOCKED   1
+#  define RW_SP_WRITE_LOCKED -1
+#endif
+
+#ifndef CONFIG_SPINLOCK
+#  define SP_UNLOCKED 0  /* The Un-locked state */
+#  define SP_LOCKED   1  /* The Locked state */
+
+typedef uint8_t spinlock_t;
+#elif defined(CONFIG_TICKET_SPINLOCK)
+
+union spinlock_u
+{
+  struct
+  {
+    unsigned short owner;
+    unsigned short next;
+  } tickets;
+  unsigned int value;
+};
+typedef union spinlock_u spinlock_t;
+
+#  define SP_UNLOCKED (union spinlock_u){{0, 0}}
+#  define SP_LOCKED (union spinlock_u){{0, 1}}
+
+#else
+
+/* The architecture specific spinlock.h header file must also provide the
+ * following:
+ *
+ *   SP_LOCKED   - A definition of the locked state value (usually 1)
+ *   SP_UNLOCKED - A definition of the unlocked state value (usually 0)
+ *   spinlock_t  - The type of a spinlock memory object.
+ *
+ * SP_LOCKED and SP_UNLOCKED must be constants of type spinlock_t.
+ */
+
+#include <arch/spinlock.h>
+
+#endif /* CONFIG_SPINLOCK */
+
+#undef EXTERN
+#if defined(__cplusplus)
+}
+#endif
+
+#endif /* __INCLUDE_NUTTX_SPINLOCK_TYPE_H */

include/pthread.h

@@ -38,7 +38,6 @@
 #include <signal.h>          /* Needed for sigset_t, includes this file */
 #include <time.h>            /* Needed for struct timespec */
 
-#ifdef CONFIG_PTHREAD_SPINLOCKS
 /* The architecture specific spinlock.h header file must provide the
  * following:
  *
@@ -49,8 +48,7 @@
  * SP_LOCKED and SP_UNLOCKED must constants of type spinlock_t.
  */
 
-#  include <arch/spinlock.h>
-#endif
+#include <nuttx/spinlock_type.h>
 
 /****************************************************************************
  * Pre-processor Definitions
@@ -368,6 +366,8 @@ struct pthread_barrier_s
 {
   sem_t        sem;
   unsigned int count;
+  volatile int16_t wait_count;
+  volatile spinlock_t wait_lock;
 };
 
 #ifndef __PTHREAD_BARRIER_T_DEFINED

libs/libc/pthread/pthread_barrierinit.c

@@ -87,6 +87,8 @@ int pthread_barrier_init(FAR pthread_barrier_t *barrier,
     {
       sem_init(&barrier->sem, 0, 0);
       barrier->count = count;
+      barrier->wait_count = 0;
+      barrier->wait_lock = SP_UNLOCKED;
     }
 
   return ret;

sched/pthread/pthread_barrierwait.c

@@ -28,6 +28,8 @@
 
 #include <nuttx/irq.h>
 #include <nuttx/semaphore.h>
+#include <nuttx/spinlock.h>
+
 #include <pthread.h>
 #include <errno.h>
 #include <debug.h>
@@ -83,66 +85,36 @@
 int pthread_barrier_wait(FAR pthread_barrier_t *barrier)
 {
   irqstate_t flags;
-  int semcount;
-  int ret;
 
   if (barrier == NULL)
     {
       return EINVAL;
     }
 
-  /* Disable pre-emption throughout the following */
-
-  flags = enter_critical_section();
-
-  /* Find out how many threads are already waiting at the barrier */
-
-  ret = nxsem_get_value(&barrier->sem, &semcount);
-  if (ret != OK)
-    {
-      leave_critical_section(flags);
-      return -ret;
-    }
-
   /* If the number of waiters would be equal to the count, then we are done */
 
-  if ((1 - semcount) >= (int)barrier->count)
+  flags = spin_lock_irqsave(&barrier->wait_lock);
+
+  if ((1 - barrier->wait_count) >= (int)barrier->count)
     {
       /* Free all of the waiting threads */
 
-      while (semcount < 0)
+      while (barrier->wait_count < 0)
         {
+          barrier->wait_count++;
           nxsem_post(&barrier->sem);
-          nxsem_get_value(&barrier->sem, &semcount);
         }
 
       /* Then return PTHREAD_BARRIER_SERIAL_THREAD to the final thread */
 
-      leave_critical_section(flags);
+      spin_unlock_irqrestore(&barrier->wait_lock, flags);
+
       return PTHREAD_BARRIER_SERIAL_THREAD;
     }
 
-  /* Otherwise, this thread must wait as well */
-
-  while ((ret = nxsem_wait(&barrier->sem)) != OK)
-    {
-      /* If the thread is awakened by a signal, just continue to wait */
-
-      if (ret != -EINTR)
-        {
-          /* If it is awakened by some other error, then there is a
-           * problem
-           */
-
-          break;
-        }
-    }
+  barrier->wait_count--;
 
-  /* We will only get here when we are one of the N-1 threads that were
-   * waiting for the final thread at the barrier.  We just need to return
-   * zero.
-   */
+  spin_unlock_irqrestore(&barrier->wait_lock, flags);
 
-  leave_critical_section(flags);
-  return -ret;
+  return -nxsem_wait_uninterruptible(&barrier->sem);
 }