[ovs-dev] [threads 01/11] ovs-thread: New module, initially just with pthreads wrapper functions.

Ben Pfaff blp at nicira.com
Mon Jun 24 18:05:38 UTC 2013


On Fri, Jun 21, 2013 at 11:36:16AM -0700, Ethan Jackson wrote:
> The #include_next in sparse/pthread.h is a bit exotic.  Would you
> please include a short comment explaining why it's necessary?

OK, I added:
/* Get actual <pthread.h> definitions for us to annotate and build on. */

We have tons of #include_next in the datapath compat directory by the
way.

You realize that these definitions only get used with "sparse", right?
 
> sparse/pthread.h has some trailing newlines at the end of the file.

Thanks, removed.

> In ovs-thread.c any reason not to macro-ize the trylock functions as well?

I guess we might as well, there are three of them after all.

Here's an incremental followed by a full patch

diff --git a/include/sparse/pthread.h b/include/sparse/pthread.h
index 1e54e95..7ba6a05 100644
--- a/include/sparse/pthread.h
+++ b/include/sparse/pthread.h
@@ -18,6 +18,7 @@
 #error "Use this header only with sparse.  It is not a correct implementation."
 #endif
 
+/* Get actual <pthread.h> definitions for us to annotate and build on. */
 #include_next <pthread.h>
 
 #include "compiler.h"
@@ -57,5 +58,3 @@ int pthread_cond_wait(pthread_cond_t *, pthread_mutex_t *mutex)
         }                                               \
         retval;                                         \
     })
-
-
diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c
index f48a659..2322432 100644
--- a/lib/ovs-thread.c
+++ b/lib/ovs-thread.c
@@ -35,6 +35,16 @@
             ovs_abort(error, "%s failed", #FUNCTION);   \
         }                                               \
     }
+#define XPTHREAD_TRY_FUNC1(FUNCTION, PARAM1)            \
+    int                                                 \
+    x##FUNCTION(PARAM1 arg1)                            \
+    {                                                   \
+        int error = FUNCTION(arg1);                     \
+        if (OVS_UNLIKELY(error && error != EBUSY)) {    \
+            ovs_abort(error, "%s failed", #FUNCTION);   \
+        }                                               \
+        return error;                                   \
+    }
 #define XPTHREAD_FUNC2(FUNCTION, PARAM1, PARAM2)        \
     void                                                \
     x##FUNCTION(PARAM1 arg1, PARAM2 arg2)               \
@@ -48,42 +58,15 @@
 XPTHREAD_FUNC2(pthread_mutex_init, pthread_mutex_t *, pthread_mutexattr_t *);
 XPTHREAD_FUNC1(pthread_mutex_lock, pthread_mutex_t *);
 XPTHREAD_FUNC1(pthread_mutex_unlock, pthread_mutex_t *);
-
-int
-xpthread_mutex_trylock(pthread_mutex_t *mutex)
-{
-    int error = pthread_mutex_trylock(mutex);
-    if (OVS_UNLIKELY(error && error != EBUSY)) {
-        ovs_abort(error, "pthread_mutex_trylock failed");
-    }
-    return error;
-}
+XPTHREAD_TRY_FUNC1(pthread_mutex_trylock, pthread_mutex_t *);
 
 XPTHREAD_FUNC2(pthread_rwlock_init,
                pthread_rwlock_t *, pthread_rwlockattr_t *);
 XPTHREAD_FUNC1(pthread_rwlock_rdlock, pthread_rwlock_t *);
 XPTHREAD_FUNC1(pthread_rwlock_wrlock, pthread_rwlock_t *);
 XPTHREAD_FUNC1(pthread_rwlock_unlock, pthread_rwlock_t *);
-
-int
-xpthread_rwlock_tryrdlock(pthread_rwlock_t *rwlock)
-{
-    int error = pthread_rwlock_tryrdlock(rwlock);
-    if (OVS_UNLIKELY(error && error != EBUSY)) {
-        ovs_abort(error, "pthread_rwlock_tryrdlock failed");
-    }
-    return error;
-}
-
-int
-xpthread_rwlock_trywrlock(pthread_rwlock_t *rwlock)
-{
-    int error = pthread_rwlock_trywrlock(rwlock);
-    if (OVS_UNLIKELY(error && error != EBUSY)) {
-        ovs_abort(error, "pthread_rwlock_trywrlock failed");
-    }
-    return error;
-}
+XPTHREAD_TRY_FUNC1(pthread_rwlock_tryrdlock, pthread_rwlock_t *);
+XPTHREAD_TRY_FUNC1(pthread_rwlock_trywrlock, pthread_rwlock_t *);
 
 XPTHREAD_FUNC2(pthread_cond_init, pthread_cond_t *, pthread_condattr_t *);
 XPTHREAD_FUNC1(pthread_cond_signal, pthread_cond_t *);

--8<--------------------------cut here-------------------------->8--

From: Ben Pfaff <blp at nicira.com>
Date: Mon, 24 Jun 2013 11:05:10 -0700
Subject: [PATCH] ovs-thread: New module, initially just with pthreads wrapper functions.

The only tricky part here is that I'm throwing in annotations to allow
"sparse" to report unbalanced locking.

Signed-off-by: Ben Pfaff <blp at nicira.com>
Acked-by: Ethan Jackson <ethan at nicira.com>
---
 include/sparse/automake.mk |    1 +
 include/sparse/pthread.h   |   60 +++++++++++++++++++++++++++++
 lib/automake.mk            |    2 +
 lib/compiler.h             |   36 +++++++++++++++++-
 lib/ovs-thread.c           |   91 ++++++++++++++++++++++++++++++++++++++++++++
 lib/ovs-thread.h           |   89 +++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 278 insertions(+), 1 deletions(-)
 create mode 100644 include/sparse/pthread.h
 create mode 100644 lib/ovs-thread.c
 create mode 100644 lib/ovs-thread.h

diff --git a/include/sparse/automake.mk b/include/sparse/automake.mk
index 1a77500..45ae1f5 100644
--- a/include/sparse/automake.mk
+++ b/include/sparse/automake.mk
@@ -4,5 +4,6 @@ noinst_HEADERS += \
         include/sparse/math.h \
         include/sparse/netinet/in.h \
         include/sparse/netinet/ip6.h \
+        include/sparse/pthread.h \
         include/sparse/sys/socket.h \
         include/sparse/sys/wait.h
diff --git a/include/sparse/pthread.h b/include/sparse/pthread.h
new file mode 100644
index 0000000..7ba6a05
--- /dev/null
+++ b/include/sparse/pthread.h
@@ -0,0 +1,60 @@
+/*
+ * Copyright (c) 2013 Nicira, Inc.
+ *
+ * Licensed 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 __CHECKER__
+#error "Use this header only with sparse.  It is not a correct implementation."
+#endif
+
+/* Get actual <pthread.h> definitions for us to annotate and build on. */
+#include_next <pthread.h>
+
+#include "compiler.h"
+
+int pthread_mutex_lock(pthread_mutex_t *mutex) OVS_ACQUIRES(mutex);
+int pthread_mutex_unlock(pthread_mutex_t *mutex) OVS_RELEASES(mutex);
+
+int pthread_rwlock_rdlock(pthread_rwlock_t *rwlock) OVS_ACQUIRES(rwlock);
+int pthread_rwlock_wrlock(pthread_rwlock_t *rwlock) OVS_ACQUIRES(rwlock);
+int pthread_rwlock_unlock(pthread_rwlock_t *rwlock) OVS_RELEASES(rwlock);
+
+int pthread_cond_wait(pthread_cond_t *, pthread_mutex_t *mutex)
+    OVS_MUST_HOLD(mutex);
+
+#define pthread_mutex_trylock(MUTEX)                    \
+    ({                                                  \
+        int retval = pthread_mutex_trylock(mutex);      \
+        if (!retval) {                                  \
+            OVS_ACQUIRE(MUTEX);                         \
+        }                                               \
+        retval;                                         \
+    })
+
+#define pthread_rwlock_tryrdlock(RWLOCK)                \
+    ({                                                  \
+        int retval = pthread_rwlock_tryrdlock(rwlock);  \
+        if (!retval) {                                  \
+            OVS_ACQUIRE(RWLOCK);                        \
+        }                                               \
+        retval;                                         \
+    })
+#define pthread_rwlock_trywrlock(RWLOCK)                \
+    ({                                                  \
+        int retval = pthread_rwlock_trywrlock(rwlock);  \
+        if (!retval) {                                  \
+            OVS_ACQUIRE(RWLOCK);                        \
+        }                                               \
+        retval;                                         \
+    })
diff --git a/lib/automake.mk b/lib/automake.mk
index eec71dd..c6de4fe 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -121,6 +121,8 @@ lib_libopenvswitch_a_SOURCES = \
 	lib/ofp-version-opt.c \
 	lib/ofpbuf.c \
 	lib/ofpbuf.h \
+	lib/ovs-thread.c \
+	lib/ovs-thread.h \
 	lib/ovsdb-data.c \
 	lib/ovsdb-data.h \
 	lib/ovsdb-error.c \
diff --git a/lib/compiler.h b/lib/compiler.h
index 760389d..f3cbe96 100644
--- a/lib/compiler.h
+++ b/lib/compiler.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -41,6 +41,40 @@
 #define OVS_UNLIKELY(CONDITION) (!!(CONDITION))
 #endif
 
+#ifdef __CHECKER__
+/* "sparse" annotations for mutexes and mutex-like constructs.
+ *
+ * In a function prototype, OVS_ACQUIRES(MUTEX) indicates that the function
+ * must be called without MUTEX acquired and that it returns with MUTEX
+ * acquired.  OVS_RELEASES(MUTEX) indicates the reverse.  OVS_MUST_HOLD
+ * indicates that the function must be called with MUTEX acquired by the
+ * caller and that the function does not release MUTEX.
+ *
+ * In practice, sparse ignores the MUTEX argument.  It need not even be a
+ * valid expression.  It is meant to indicate to human readers what mutex is
+ * being acquired.
+ *
+ * Since sparse ignores MUTEX, it need not be an actual mutex.  It can be
+ * any construct on which paired acquire and release semantics make sense:
+ * read/write locks, temporary memory allocations, whatever.
+ *
+ * OVS_ACQUIRE, OVS_RELEASE, and OVS_HOLDS are suitable for use within macros,
+ * where there is no function prototype to annotate. */
+#define OVS_ACQUIRES(MUTEX) __attribute__((context(MUTEX, 0, 1)))
+#define OVS_RELEASES(MUTEX) __attribute__((context(MUTEX, 1, 0)))
+#define OVS_MUST_HOLD(MUTEX) __attribute__((context(MUTEX, 1, 1)))
+#define OVS_ACQUIRE(MUTEX) __context__(MUTEX, 0, 1)
+#define OVS_RELEASE(MUTEX) __context__(MUTEX, 1, 0)
+#define OVS_HOLDS(MUTEX) __context__(MUTEX, 1, 1)
+#else
+#define OVS_ACQUIRES(MUTEX)
+#define OVS_RELEASES(MUTEX)
+#define OVS_MUST_HOLD(MUTEX)
+#define OVS_ACQUIRE(MUTEX)
+#define OVS_RELEASE(MUTEX)
+#define OVS_HOLDS(MUTEX)
+#endif
+
 /* ISO C says that a C implementation may choose any integer type for an enum
  * that is sufficient to hold all of its values.  Common ABIs (such as the
  * System V ABI used on i386 GNU/Linux) always use a full-sized "int", even
diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c
new file mode 100644
index 0000000..2322432
--- /dev/null
+++ b/lib/ovs-thread.c
@@ -0,0 +1,91 @@
+/*
+ * Copyright (c) 2013 Nicira, Inc.
+ *
+ * Licensed 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.
+ */
+
+#include <config.h>
+#include "ovs-thread.h"
+#include <errno.h>
+#include "compiler.h"
+#include "util.h"
+
+#ifdef __CHECKER__
+/* Omit the definitions in this file because they are somewhat difficult to
+ * write without prompting "sparse" complaints, without ugliness or
+ * cut-and-paste.  Since "sparse" is just a checker, not a compiler, it
+ * doesn't matter that we don't define them. */
+#else
+#define XPTHREAD_FUNC1(FUNCTION, PARAM1)                \
+    void                                                \
+    x##FUNCTION(PARAM1 arg1)                            \
+    {                                                   \
+        int error = FUNCTION(arg1);                     \
+        if (OVS_UNLIKELY(error)) {                      \
+            ovs_abort(error, "%s failed", #FUNCTION);   \
+        }                                               \
+    }
+#define XPTHREAD_TRY_FUNC1(FUNCTION, PARAM1)            \
+    int                                                 \
+    x##FUNCTION(PARAM1 arg1)                            \
+    {                                                   \
+        int error = FUNCTION(arg1);                     \
+        if (OVS_UNLIKELY(error && error != EBUSY)) {    \
+            ovs_abort(error, "%s failed", #FUNCTION);   \
+        }                                               \
+        return error;                                   \
+    }
+#define XPTHREAD_FUNC2(FUNCTION, PARAM1, PARAM2)        \
+    void                                                \
+    x##FUNCTION(PARAM1 arg1, PARAM2 arg2)               \
+    {                                                   \
+        int error = FUNCTION(arg1, arg2);               \
+        if (OVS_UNLIKELY(error)) {                      \
+            ovs_abort(error, "%s failed", #FUNCTION);   \
+        }                                               \
+    }
+
+XPTHREAD_FUNC2(pthread_mutex_init, pthread_mutex_t *, pthread_mutexattr_t *);
+XPTHREAD_FUNC1(pthread_mutex_lock, pthread_mutex_t *);
+XPTHREAD_FUNC1(pthread_mutex_unlock, pthread_mutex_t *);
+XPTHREAD_TRY_FUNC1(pthread_mutex_trylock, pthread_mutex_t *);
+
+XPTHREAD_FUNC2(pthread_rwlock_init,
+               pthread_rwlock_t *, pthread_rwlockattr_t *);
+XPTHREAD_FUNC1(pthread_rwlock_rdlock, pthread_rwlock_t *);
+XPTHREAD_FUNC1(pthread_rwlock_wrlock, pthread_rwlock_t *);
+XPTHREAD_FUNC1(pthread_rwlock_unlock, pthread_rwlock_t *);
+XPTHREAD_TRY_FUNC1(pthread_rwlock_tryrdlock, pthread_rwlock_t *);
+XPTHREAD_TRY_FUNC1(pthread_rwlock_trywrlock, pthread_rwlock_t *);
+
+XPTHREAD_FUNC2(pthread_cond_init, pthread_cond_t *, pthread_condattr_t *);
+XPTHREAD_FUNC1(pthread_cond_signal, pthread_cond_t *);
+XPTHREAD_FUNC1(pthread_cond_broadcast, pthread_cond_t *);
+XPTHREAD_FUNC2(pthread_cond_wait, pthread_cond_t *, pthread_mutex_t *);
+
+typedef void destructor_func(void *);
+XPTHREAD_FUNC2(pthread_key_create, pthread_key_t *, destructor_func *);
+
+void
+xpthread_create(pthread_t *threadp, pthread_attr_t *attr,
+                void *(*start)(void *), void *arg)
+{
+    pthread_t thread;
+    int error;
+
+    error = pthread_create(threadp ? threadp : &thread, attr, start, arg);
+    if (error) {
+        ovs_abort(error, "pthread_create failed");
+    }
+}
+#endif
diff --git a/lib/ovs-thread.h b/lib/ovs-thread.h
new file mode 100644
index 0000000..cafeedf
--- /dev/null
+++ b/lib/ovs-thread.h
@@ -0,0 +1,89 @@
+/*
+ * Copyright (c) 2013 Nicira, Inc.
+ *
+ * Licensed 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 OVS_THREAD_H
+#define OVS_THREAD_H 1
+
+#include <pthread.h>
+#include "util.h"
+
+/* glibc has some non-portable mutex types and initializers:
+ *
+ *    - PTHREAD_MUTEX_ADAPTIVE_NP is a mutex type that works as a spinlock that
+ *      falls back to a mutex after spinning for some number of iterations.
+ *
+ *    - PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP is a non-portable initializer
+ *      for an error-checking mutex.
+ *
+ * We use these definitions to fall back to PTHREAD_MUTEX_NORMAL instead in
+ * these cases.
+ *
+ * (glibc has other non-portable initializers, but we can't reasonably
+ * substitute for them here.) */
+#ifdef PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP
+#define PTHREAD_MUTEX_ADAPTIVE PTHREAD_MUTEX_ADAPTIVE_NP
+#define PTHREAD_ADAPTIVE_MUTEX_INITIALIZER \
+    PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP
+#else
+#define PTHREAD_MUTEX_ADAPTIVE PTHREAD_MUTEX_NORMAL
+#define PTHREAD_ADAPTIVE_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER
+#endif
+
+#ifdef PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP
+#define PTHREAD_ERRORCHECK_MUTEX_INITIALIZER \
+    PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP
+#else
+#define PTHREAD_ERRORCHECK_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER
+#endif
+
+/* Simple wrappers for pthreads functions.  Most of these functions abort the
+ * process with an error message on any error.  The *_trylock() functions are
+ * exceptions: they pass through a 0 or EBUSY return value to the caller and
+ * abort on any other error. */
+void xpthread_mutex_init(pthread_mutex_t *, pthread_mutexattr_t *);
+void xpthread_mutex_lock(pthread_mutex_t *mutex) OVS_ACQUIRES(mutex);
+void xpthread_mutex_unlock(pthread_mutex_t *mutex) OVS_RELEASES(mutex);
+int xpthread_mutex_trylock(pthread_mutex_t *);
+
+void xpthread_rwlock_init(pthread_rwlock_t *, pthread_rwlockattr_t *);
+void xpthread_rwlock_rdlock(pthread_rwlock_t *rwlock) OVS_ACQUIRES(rwlock);
+void xpthread_rwlock_wrlock(pthread_rwlock_t *rwlock) OVS_ACQUIRES(rwlock);
+void xpthread_rwlock_unlock(pthread_rwlock_t *rwlock) OVS_RELEASES(rwlock);
+int xpthread_rwlock_tryrdlock(pthread_rwlock_t *);
+int xpthread_rwlock_trywrlock(pthread_rwlock_t *);
+
+void xpthread_cond_init(pthread_cond_t *, pthread_condattr_t *);
+void xpthread_cond_signal(pthread_cond_t *);
+void xpthread_cond_broadcast(pthread_cond_t *);
+void xpthread_cond_wait(pthread_cond_t *, pthread_mutex_t *mutex)
+    OVS_MUST_HOLD(mutex);
+
+#ifdef __CHECKER__
+/* Replace these functions by the macros already defined in the <pthread.h>
+ * annotations, because the macro definitions have correct semantics for the
+ * conditional acquisition that can't be captured in a function annotation.
+ * The difference in semantics from pthread_*() to xpthread_*() does not matter
+ * because sparse is not a compiler. */
+#define xpthread_mutex_trylock pthread_mutex_trylock
+#define xpthread_rwlock_tryrdlock pthread_rwlock_tryrdlock
+#define xpthread_rwlock_trywrlock pthread_rwlock_trywrlock
+#endif
+
+void xpthread_key_create(pthread_key_t *, void (*destructor)(void *));
+
+void xpthread_create(pthread_t *, pthread_attr_t *, void *(*)(void *), void *);
+
+#endif /* ovs-thread.h */
-- 
1.7.2.5




More information about the dev mailing list