[PATCH][RFC] POSIX barrier implementation, take 1

[PATCH][RFC] POSIX barrier implementation, take 1

[Václav Haisman]

[-- Attachment #1: Type: text/plain, Size: 232 bytes --]

Hi.

I am attaching a patch that adds (or tries to) POSIX barriers
implementation into Cygwin. I have compiled it but not actually tested
it, yet. I am dumping it here just in case I get run over by a bus on
my way home. :)

-- 
VH

[-- Attachment #2: barrier01.patch.txt --]
[-- Type: text/plain, Size: 12027 bytes --]

diff --git a/newlib/libc/include/sys/features.h b/newlib/libc/include/sys/features.h
index 4ad7fbd..0c6043c 100644
--- a/newlib/libc/include/sys/features.h
+++ b/newlib/libc/include/sys/features.h
@@ -118,10 +118,10 @@ extern "C" {
 
 #define _POSIX_ADVISORY_INFO			200112L
 /* #define _POSIX_ASYNCHRONOUS_IO		    -1 */
-/* #define _POSIX_BARRIERS			    -1 */
+#define _POSIX_BARRIERS				200112L
 #define _POSIX_CHOWN_RESTRICTED			     1
 #define _POSIX_CLOCK_SELECTION			200112L
-#define _POSIX_CPUTIME			    	200112L
+#define _POSIX_CPUTIME				200112L
 #define _POSIX_FSYNC				200112L
 #define _POSIX_IPV6				200112L
 #define _POSIX_JOB_CONTROL			     1
@@ -140,7 +140,7 @@ extern "C" {
 #define _POSIX_REGEXP				     1
 #define _POSIX_SAVED_IDS			     1
 #define _POSIX_SEMAPHORES			200112L
-#define _POSIX_SHARED_MEMORY_OBJECTS		200112L 
+#define _POSIX_SHARED_MEMORY_OBJECTS		200112L
 #define _POSIX_SHELL				     1
 /* #define _POSIX_SPAWN				    -1 */
 #define _POSIX_SPIN_LOCKS			    200112L
diff --git a/newlib/libc/include/sys/types.h b/newlib/libc/include/sys/types.h
index 5dd6c75..bfe93fa 100644
--- a/newlib/libc/include/sys/types.h
+++ b/newlib/libc/include/sys/types.h
@@ -431,6 +431,7 @@ typedef struct {
 
 /* POSIX Barrier Types */
 
+#if !defined(__CYGWIN__)
 #if defined(_POSIX_BARRIERS)
 typedef __uint32_t pthread_barrier_t;        /* POSIX Barrier Object */
 typedef struct {
@@ -440,6 +441,7 @@ typedef struct {
 #endif
 } pthread_barrierattr_t;
 #endif /* defined(_POSIX_BARRIERS) */
+#endif /* __CYGWIN__ */
 
 /* POSIX Spin Lock Types */
 
diff --git a/winsup/cygwin/include/cygwin/types.h b/winsup/cygwin/include/cygwin/types.h
index 85ee7c7..b01ae95 100644
--- a/winsup/cygwin/include/cygwin/types.h
+++ b/winsup/cygwin/include/cygwin/types.h
@@ -184,6 +184,8 @@ typedef struct __pthread_attr_t {char __dummy;} *pthread_attr_t;
 typedef struct __pthread_mutexattr_t {char __dummy;} *pthread_mutexattr_t;
 typedef struct __pthread_condattr_t {char __dummy;} *pthread_condattr_t;
 typedef struct __pthread_cond_t {char __dummy;} *pthread_cond_t;
+typedef struct __pthread_barrierattr_t {char __dummy;} *pthread_barrierattr_t;
+typedef struct __pthread_barrier_t {char __dummy;} *pthread_barrier_t;
 
   /* These variables are not user alterable. This means you!. */
 typedef struct
@@ -207,6 +209,8 @@ typedef class pthread_attr *pthread_attr_t;
 typedef class pthread_mutexattr *pthread_mutexattr_t;
 typedef class pthread_condattr *pthread_condattr_t;
 typedef class pthread_cond *pthread_cond_t;
+typedef class pthread_barrier *pthread_barrier_t;
+typedef class pthread_barrierattr *pthread_barrierattr_t;
 typedef class pthread_once pthread_once_t;
 typedef class pthread_spinlock *pthread_spinlock_t;
 typedef class pthread_rwlock *pthread_rwlock_t;
diff --git a/winsup/cygwin/include/pthread.h b/winsup/cygwin/include/pthread.h
index 9ad8b66..84e0a14 100644
--- a/winsup/cygwin/include/pthread.h
+++ b/winsup/cygwin/include/pthread.h
@@ -62,6 +62,7 @@ extern "C"
 /* process is the default */
 #define PTHREAD_SCOPE_PROCESS 0
 #define PTHREAD_SCOPE_SYSTEM 1
+#define PTHREAD_BARRIER_SERIAL_THREAD (-1)
 
 /* Register Fork Handlers */
 int pthread_atfork (void (*)(void), void (*)(void), void (*)(void));
@@ -133,6 +134,17 @@ int pthread_condattr_init (pthread_condattr_t *);
 int pthread_condattr_setclock (pthread_condattr_t *, clockid_t);
 int pthread_condattr_setpshared (pthread_condattr_t *, int);
 
+/* Barriers */
+int pthread_barrierattr_init (pthread_barrierattr_t *);
+int pthread_barrierattr_setpshared (pthread_barrierattr_t *, int);
+int pthread_barrierattr_getpshared (const pthread_barrierattr_t *, int *);
+int pthread_barrierattr_destroy (pthread_barrierattr_t *);
+int pthread_barrier_init (pthread_barrier_t *,
+                          const pthread_barrierattr_t *, unsigned);
+int pthread_barrier_destroy (pthread_barrier_t *);
+int pthread_barrier_wait (pthread_barrier_t *);
+
+/* Threads */
 int pthread_create (pthread_t *, const pthread_attr_t *,
 		    void *(*)(void *), void *);
 int pthread_detach (pthread_t);
diff --git a/winsup/cygwin/thread.cc b/winsup/cygwin/thread.cc
index 8f29900..794dd93 100644
--- a/winsup/cygwin/thread.cc
+++ b/winsup/cygwin/thread.cc
@@ -50,6 +50,17 @@ const pthread_t pthread_mutex::_new_mutex = (pthread_t) 1;
 const pthread_t pthread_mutex::_unlocked_mutex = (pthread_t) 2;
 const pthread_t pthread_mutex::_destroyed_mutex = (pthread_t) 3;
 
+
+template <typename T>
+inline
+void
+delete_and_clear (T * * const ptr)
+{
+  delete *ptr;
+  *ptr = 0;
+}
+
+
 inline bool
 pthread_mutex::no_owner()
 {
@@ -267,6 +278,23 @@ pthread_cond::is_initializer_or_object (pthread_cond_t const *cond)
   return true;
 }
 
+inline bool
+pthread_barrierattr::is_good_object (pthread_barrierattr_t const *cond)
+{
+  if (verifyable_object_isvalid (cond, PTHREAD_BARRIERATTR_MAGIC)
+      != VALID_OBJECT)
+    return false;
+  return true;
+}
+
+inline bool
+pthread_barrier::is_good_object (pthread_barrier_t const *cond)
+{
+  if (verifyable_object_isvalid (cond, PTHREAD_BARRIER_MAGIC) != VALID_OBJECT)
+    return false;
+  return true;
+}
+
 /* RW locks */
 inline bool
 pthread_rwlock::is_good_object (pthread_rwlock_t const *rwlock)
@@ -1300,6 +1328,25 @@ pthread_cond::_fixup_after_fork ()
     api_fatal ("pthread_cond::_fixup_after_fork () failed to recreate win32 semaphore");
 }
 
+pthread_barrierattr::pthread_barrierattr ()
+  : verifyable_object (PTHREAD_BARRIERATTR_MAGIC)
+  , shared (PTHREAD_PROCESS_PRIVATE)
+{
+}
+
+pthread_barrierattr::~pthread_barrierattr ()
+{
+}
+
+pthread_barrier::pthread_barrier ()
+  : verifyable_object (PTHREAD_BARRIER_MAGIC)
+{
+}
+
+pthread_barrier::~pthread_barrier ()
+{
+}
+
 pthread_rwlockattr::pthread_rwlockattr ():verifyable_object
   (PTHREAD_RWLOCKATTR_MAGIC), shared (PTHREAD_PROCESS_PRIVATE)
 {
@@ -3869,3 +3916,225 @@ pthread_null::getsequence_np ()
 }
 
 pthread_null pthread_null::_instance;
+
+
+
+#define LIKELY(X) __builtin_expect (!!(X), 1)
+#define UNLIKELY(X) __builtin_expect (!!(X), 0)
+
+
+extern "C"
+int
+pthread_barrierattr_init (pthread_barrierattr_t * battr)
+{
+  if (UNLIKELY (battr == NULL)
+      || pthread_barrierattr::is_good_object (battr))
+    return EINVAL;
+
+  *battr = new pthread_barrierattr;
+  (*battr)->shared = PTHREAD_PROCESS_PRIVATE;
+
+  return 0;
+}
+
+
+extern "C"
+int
+pthread_barrierattr_setpshared (pthread_barrierattr_t * battr, int shared)
+{
+  if (UNLIKELY (! pthread_barrierattr::is_good_object (battr)))
+    return EINVAL;
+
+  if (UNLIKELY (shared != PTHREAD_PROCESS_SHARED
+                && shared != PTHREAD_PROCESS_PRIVATE))
+    return EINVAL;
+
+  (*battr)->shared = shared;
+  return 0;
+}
+
+
+extern "C"
+int
+pthread_barrierattr_getpshared (const pthread_barrierattr_t * battr,
+                                int * shared)
+{
+  if (UNLIKELY (! pthread_barrierattr::is_good_object (battr)
+                || shared == NULL))
+    return EINVAL;
+
+  *shared = (*battr)->shared;
+  return 0;
+}
+
+
+extern "C"
+int
+pthread_barrierattr_destroy (pthread_barrierattr_t * battr)
+{
+  if (UNLIKELY (! pthread_barrierattr::is_good_object (battr)))
+    return EINVAL;
+
+  delete_and_clear (battr);
+  return 0;
+}
+
+
+extern "C"
+int
+pthread_barrier_init (pthread_barrier_t * bar,
+                      const pthread_barrierattr_t * attr, unsigned count)
+{
+  if (UNLIKELY (bar == NULL
+                || pthread_barrier::is_good_object (bar)))
+    return EINVAL;
+
+  *bar = new pthread_barrier;
+  return (*bar)->init (attr, count);
+}
+
+
+int
+pthread_barrier::init (const pthread_barrierattr_t * attr, unsigned count)
+{
+  pthread_mutex_t * mutex = NULL;
+
+  if (UNLIKELY ((attr != NULL
+                 || ! pthread_barrierattr::is_good_object (attr)
+                 || (*attr)->shared == PTHREAD_PROCESS_SHARED)
+                || count == 0))
+    return EINVAL;
+
+  int retval = pthread_mutex_init (&mtx, NULL);
+  if (UNLIKELY (retval != 0))
+    return retval;
+
+  retval = pthread_cond_init (&cond, NULL);
+  if (UNLIKELY (retval != 0))
+    {
+      int ret = pthread_mutex_destroy (mutex);
+      if (ret != 0)
+        api_fatal ("pthread_mutex_destroy (%p) = %d", mutex, ret);
+
+      mtx = NULL;
+      return retval;
+    }
+
+  cnt = count;
+  cyc = 0;
+  wt = 0;
+
+  return 0;
+}
+
+
+extern "C"
+int
+pthread_barrier_destroy (pthread_barrier_t * bar)
+{
+  if (UNLIKELY (! pthread_barrier::is_good_object (bar)))
+    return EINVAL;
+
+  int ret;
+  ret = (*bar)->destroy ();
+  if (ret == 0)
+    delete_and_clear (bar);
+
+  return ret;
+}
+
+
+int
+pthread_barrier::destroy ()
+{
+  if (UNLIKELY (wt != 0))
+    return EBUSY;
+
+  int retval = pthread_cond_destroy (&cond);
+  if (UNLIKELY (retval != 0))
+    return retval;
+  else
+    cond = NULL;
+
+  retval = pthread_mutex_destroy (&mtx);
+  if (UNLIKELY (retval != 0))
+    return retval;
+  else
+    mtx = NULL;
+
+  cnt = 0;
+  cyc = 0;
+  wt = 0;
+
+  return 0;
+}
+
+
+extern "C"
+int
+pthread_barrier_wait (pthread_barrier_t * bar)
+{
+  if (UNLIKELY (! pthread_barrier::is_good_object (bar)))
+    return EINVAL;
+
+  return (*bar)->wait ();
+}
+
+
+int
+pthread_barrier::wait ()
+{
+  int retval = pthread_mutex_lock (&mtx);
+  if (UNLIKELY (retval != 0))
+    return retval;
+
+  if (UNLIKELY (wt >= cnt))
+    {
+      api_fatal ("wt >= cnt (%u >= %u)", wt, cnt);
+      return EINVAL;
+    }
+
+  if (UNLIKELY (++wt == cnt))
+    {
+      ++cyc;
+      /* This is the last thread to reach the barrier. Signal the waiting
+         threads to wake up and continue.  */
+      retval = pthread_cond_broadcast (&cond);
+      if (UNLIKELY (retval != 0))
+        goto cond_error;
+
+      wt = 0;
+      retval = pthread_mutex_unlock (&mtx);
+      if (UNLIKELY (retval != 0))
+        abort ();
+
+      return PTHREAD_BARRIER_SERIAL_THREAD;
+    }
+  else
+    {
+      uint64_t cycle = cyc;
+      do
+        {
+          retval = pthread_cond_wait (&cond, &mtx);
+          if (UNLIKELY (retval != 0))
+            goto cond_error;
+        }
+      while (UNLIKELY (cycle == cyc));
+
+      retval = pthread_mutex_unlock (&mtx);
+      if (UNLIKELY (retval != 0))
+        api_fatal ("pthread_mutex_unlock (%p) = %d", &mtx, retval);
+
+      return 0;
+    }
+
+ cond_error:
+  {
+    --wt;
+    int ret = pthread_mutex_unlock (&mtx);
+    if (UNLIKELY (ret != 0))
+        api_fatal ("pthread_mutex_unlock (%p) = %d", &mtx, ret);
+
+    return retval;
+  }
+}
diff --git a/winsup/cygwin/thread.h b/winsup/cygwin/thread.h
index a6c7358..f7bce18 100644
--- a/winsup/cygwin/thread.h
+++ b/winsup/cygwin/thread.h
@@ -1,3 +1,4 @@
+// -*- C++ -*-
 /* thread.h: Locking and threading module definitions
 
    Copyright 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2007, 2008, 2009,
@@ -84,6 +85,8 @@ class pinfo;
 #define PTHREAD_RWLOCK_MAGIC PTHREAD_MAGIC+9
 #define PTHREAD_RWLOCKATTR_MAGIC PTHREAD_MAGIC+10
 #define PTHREAD_SPINLOCK_MAGIC PTHREAD_MAGIC+11
+#define PTHREAD_BARRIER_MAGIC PTHREAD_MAGIC+12
+#define PTHREAD_BARRIERATTR_MAGIC PTHREAD_MAGIC+13
 
 #define MUTEX_OWNER_ANONYMOUS ((pthread_t) -1)
 
@@ -520,6 +523,38 @@ private:
   static fast_mutex cond_initialization_lock;
 };
 
+
+class pthread_barrierattr: public verifyable_object
+{
+public:
+  static bool is_good_object(pthread_barrierattr_t const *);
+  int shared;
+
+  pthread_barrierattr ();
+  ~pthread_barrierattr ();
+};
+
+
+class pthread_barrier: public verifyable_object
+{
+public:
+  static bool is_good_object(pthread_barrier_t const *);
+
+  pthread_mutex_t mtx; /* Mutex protecting everything below. */
+  pthread_cond_t cond; /* Conditional variable to wait on. */
+  unsigned cnt; /* Barrier count. Threads to wait for. */
+  uint64_t cyc; /* Cycle count. */
+  unsigned wt; /* Already waiting threads count. */
+
+  int init (const pthread_barrierattr_t *, unsigned);
+  int wait();
+  int destroy ();
+
+  pthread_barrier ();
+  ~pthread_barrier ();
+};
+
+
 class pthread_rwlockattr: public verifyable_object
 {
 public:

Re: [PATCH][RFC] POSIX barrier implementation, take 1

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 903 bytes --]

Hi Václav,

On Feb 11 14:30, Václav Haisman wrote:
> Hi.
> 
> I am attaching a patch that adds (or tries to) POSIX barriers
> implementation into Cygwin. I have compiled it but not actually tested
> it, yet. I am dumping it here just in case I get run over by a bus on
> my way home. :)

Uh oh, please don't do that.

I took a quick glance and your code looks pretty well.  Your
introduction of LIKELY/UNLIKELY is a neat step as well.  Shouldn't that
go into some generic header rather than just into thread.cc so we can
use it more librally throughout?

Btw., I wouldn't be unhappy if you'd have a more stern look into the
pthread implementation apart from barriers.  There are probably more
shortcomings...


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]