On Tue, Jul 30, 2013 at 03:31:48PM -0700, Alex Wang wrote:
> From: Ethan Jackson <et...@nicira.com>
>
> This commit adds annotations for thread safety check. And the
> check can be conducted by using -Wthread-safety flag in clang.
>
> Co-authored-by: Alex Wang <al...@nicira.com>
> Signed-off-by: Alex Wang <al...@nicira.com>
> Signed-off-by: Ethan Jackson <et...@nicira.com>
The lockfile logging change in this patch caused test failures for me.
I didn't see a reason for the logging change, so I just dropped it. We
can do it in a separate patch if there is a good reason..
I did a little editing of compiler and ovs-thread.[ch]. Diff appended.
Hope it's OK.
My only unresolved comment is that the 'where' member of the new struct
is write-only: nothing ever reads it. But this is important enough that
I'm willing to resolve that one way or another later.
Applied to master, thanks to both of you!
diff --git a/lib/compiler.h b/lib/compiler.h
index a95a065..1c01fd1 100644
--- a/lib/compiler.h
+++ b/lib/compiler.h
@@ -51,46 +51,49 @@
* OVS_LOCKABLE indicates that the struct contains mutex element
* which can be locked by functions like ovs_mutex_lock().
*
- * The word 'MUTEX' in the following text can stand for more than
- * one OVS_LOCKABLE structs. This means that we can require a function
- * to hold multiple locks while invoked.
- * e.g. some_function() OVS_REQ_WRLOCK(wrlock_file1, wrlock_file2)
+ * Below, the word MUTEX stands for the name of an object with an OVS_LOCKABLE
+ * struct type. It can also be a comma-separated list of multiple structs,
+ * e.g. to require a function to hold multiple locks while invoked.
*
*
- * In a global variable declaration:
+ * On a variable:
*
- * OVS_GUARDED_VAR indicates that the variable may only be accessed while
- * some MUTEX is acquired.
+ * - OVS_GUARDED indicates that the variable may only be accessed some mutex
+ * is held.
*
- * OVS_GUARDED_BY(...) indicates that the variable may only be accessed while
- * MUTEX is acquired.
+ * - OVS_GUARDED_BY(MUTEX) indicates that the variable may only be accessed
+ * while the specific MUTEX is held.
*
*
- * In a function prototype:
+ * On a function, the following attributes apply to mutexes:
*
- * OVS_ACQ_RDLOCK(...), OVS_ACQ_WRLOCK(...) and OVS_ACQUIRES(...) indicate
- * that the function may only be called without MUTEX acquired and that it
- * returns with MUTEX acquired. OVS_ACQUIRES(...) is used for plain MUTEX,
- * and the other two for readers-writer MUTEX.
+ * - OVS_ACQUIRES(MUTEX) indicate that the function must be called without
+ * holding MUTEX and that it returns holding MUTEX.
*
- * OVS_RELEASES(...) indicates that the function may only be called with
- * MUTEX acquired and that it returns with MUTEX released. It can be used
- * for all types of MUTEX.
+ * - OVS_RELEASES(MUTEX) indicates that the function may only be called with
+ * MUTEX held and that it returns with MUTEX released. It can be used for
+ * all types of MUTEX.
*
- * OVS_TRY_RDLOCK(...) and OVS_TRY_WRLOCK(...) and OVS_TRY_LOCK(...) indicate
- * that the function will try to acquire MUTEX. The macro takes one or more
- * arguments. The first argument is an interger or boolean value specifying
- * the return value of a successful lock acquisition. The remaining arguments
- * are MUTEX. OVS_TRY_LOCK(...) is used for plain MUTEX, and the other two
- * for readers-writer MUTEX.
+ * - OVS_TRY_LOCK(RETVAL, MUTEX) indicate that the function will try to
+ * acquire MUTEX. RETVAL is an integer or boolean value specifying the
+ * return value of a successful lock acquisition.
*
- * OVS_REQ_RDLOCK(...), OVS_REQ_WRLOCK(...) and OVS_REQUIRES(...) indicate
- * that the function may only be called with MUTEX acquired by the caller
- * and that the function does not release MUTEX. OVS_REQUIRES(...) is used
- * for plain MUTEX, and the other two for readers-writer MUTEX.
+ * - OVS_REQUIRES(MUTEX) indicate that the function may only be called with
+ * MUTEX held and that the function does not release MUTEX.
*
- * OVS_LOCKS_EXCLUDED(MUTEX) indicates that the function may only be called
- * without MUTEX acquired. */
+ * - OVS_LOCKS_EXCLUDED(MUTEX) indicates that the function may only be
+ * called when MUTEX is not held.
+ *
+ * The following variants, with the same syntax, apply to reader-writer locks:
+ *
+ * mutex rwlock, for reading rwlock, for writing
+ * ------------------- ------------------- -------------------
+ * OVS_ACQUIRES OVS_ACQ_RDLOCK OVS_ACQ_WRLOCK
+ * OVS_RELEASES OVS_RELEASES OVS_RELEASES
+ * OVS_TRY_LOCK OVS_TRY_RDLOCK OVS_TRY_WRLOCK
+ * OVS_REQUIRES OVS_REQ_RDLOCK OVS_REQ_WRLOCK
+ * OVS_LOCKS_EXCLUDED OVS_LOCKS_EXCLUDED OVS_LOCKS_EXCLUDED
+ */
#define OVS_LOCKABLE __attribute__((lockable))
#define OVS_REQ_RDLOCK(...) __attribute__((shared_locks_required(__VA_ARGS__)))
#define OVS_ACQ_RDLOCK(...) __attribute__((shared_lock_function(__VA_ARGS__)))
@@ -102,12 +105,12 @@
__attribute__((exclusive_locks_required(__VA_ARGS__)))
#define OVS_ACQUIRES(...) \
__attribute__((exclusive_lock_function(__VA_ARGS__)))
-#define OVS_TRY_WRLOCK(...) \
- __attribute__((exclusive_trylock_function(__VA_ARGS__)))
-#define OVS_TRY_RDLOCK(...) \
- __attribute__((shared_trylock_function(__VA_ARGS__)))
-#define OVS_TRY_LOCK(...) \
- __attribute__((exclusive_trylock_function(__VA_ARGS__)))
+#define OVS_TRY_WRLOCK(RETVAL, ...) \
+ __attribute__((exclusive_trylock_function(RETVAL, __VA_ARGS__)))
+#define OVS_TRY_RDLOCK(RETVAL, ...) \
+ __attribute__((shared_trylock_function(RETVAL, __VA_ARGS__)))
+#define OVS_TRY_LOCK(RETVAL, ...) \
+ __attribute__((exclusive_trylock_function(RETVAL, __VA_ARGS__)))
#define OVS_GUARDED __attribute__((guarded_var))
#define OVS_GUARDED_BY(...) __attribute__((guarded_by(__VA_ARGS__)))
#define OVS_RELEASES(...) __attribute__((unlock_function(__VA_ARGS__)))
@@ -126,9 +129,9 @@
#define OVS_ACQ_WRLOCK(...) __attribute__((context(MUTEX, 0, 1)))
#define OVS_REQUIRES(...) __attribute__((context(MUTEX, 1, 1)))
#define OVS_ACQUIRES(...) __attribute__((context(MUTEX, 0, 1)))
-#define OVS_TRY_WRLOCK(...)
-#define OVS_TRY_RDLOCK(...)
-#define OVS_TRY_LOCK(...)
+#define OVS_TRY_WRLOCK(RETVAL, ...)
+#define OVS_TRY_RDLOCK(RETVAL, ...)
+#define OVS_TRY_LOCK(REVAL, ...)
#define OVS_GUARDED
#define OVS_GUARDED_BY(...)
#define OVS_RELEASES(...) __attribute__((context(MUTEX, 1, 0)))
diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c
index 640cb4c..c8b2c15 100644
--- a/lib/ovs-thread.c
+++ b/lib/ovs-thread.c
@@ -131,14 +131,13 @@ void
ovs_mutex_init(const struct ovs_mutex *l_, int type)
{
struct ovs_mutex *l = CONST_CAST(struct ovs_mutex *, l_);
- int error;
pthread_mutexattr_t attr;
+ int error;
l->where = NULL;
xpthread_mutexattr_init(&attr);
xpthread_mutexattr_settype(&attr, type);
- error = pthread_mutex_init(&l->lock,
- CONST_CAST(const pthread_mutexattr_t *, &attr));
+ error = pthread_mutex_init(&l->lock, &attr);
if (OVS_UNLIKELY(error)) {
ovs_abort(error, "pthread_mutex_init failed");
}
diff --git a/lib/ovs-thread.h b/lib/ovs-thread.h
index 5c72d93..f5e171a 100644
--- a/lib/ovs-thread.h
+++ b/lib/ovs-thread.h
@@ -69,7 +69,7 @@ struct OVS_LOCKABLE ovs_mutex {
* Most of these functions abort the process with an error message on any
* error. ovs_mutex_trylock() is an exception: it passes through a 0 or EBUSY
* return value to the caller and aborts on any other error. */
-void ovs_mutex_init(const struct ovs_mutex *, int);
+void ovs_mutex_init(const struct ovs_mutex *, int type);
void ovs_mutex_destroy(const struct ovs_mutex *);
void ovs_mutex_unlock(const struct ovs_mutex *mutex) OVS_RELEASES(mutex);
void ovs_mutex_lock_at(const struct ovs_mutex *mutex, const char *where)
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
