From 6b59b543c840ac82acc4d57f865c0b5e331ac590 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Fri, 21 Feb 2014 10:53:49 -0800 Subject: [PATCH] ovs-thread: Use fair (but nonrecursive) rwlocks on glibc. glibc supports two kinds of rwlocks: - The default kind of rwlock always allows recursive read-locks to succeed, but threads blocked on acquiring the write-lock are treated unfairly, causing them to be delayed indefinitely as long as new readers continue to come along. - An alternative "writer nonrecursive" rwlock allows recursive read-locks to succeed only if there are no threads waiting for the write-lock. Otherwise, recursive read-lock attempts deadlock in the presence of blocking write-lock attempts. However, this kind of rwlock is fair to writer. POSIX allows the latter behavior, which essentially means that any portable pthread program cannot try to take read-locks recursively. Since that's true, we might as well use the latter kind of rwlock with glibc and get the benefit of fairness of writers. Signed-off-by: Ben Pfaff Acked-by: Joe Stringer --- lib/ovs-thread.c | 14 ++++++++++++++ lib/ovs-thread.h | 25 ++++++++++++++++++++++++- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c index f0b1e9e81..4dfccaf3e 100644 --- a/lib/ovs-thread.c +++ b/lib/ovs-thread.c @@ -125,6 +125,12 @@ XPTHREAD_FUNC1(pthread_mutexattr_destroy, pthread_mutexattr_t *); XPTHREAD_FUNC2(pthread_mutexattr_settype, pthread_mutexattr_t *, int); XPTHREAD_FUNC2(pthread_mutexattr_gettype, pthread_mutexattr_t *, int *); +XPTHREAD_FUNC1(pthread_rwlockattr_init, pthread_rwlockattr_t *); +XPTHREAD_FUNC1(pthread_rwlockattr_destroy, pthread_rwlockattr_t *); +#ifdef PTHREAD_RWLOCK_WRITER_NONRECURSIVE_INITIALIZER_NP +XPTHREAD_FUNC2(pthread_rwlockattr_setkind_np, pthread_rwlockattr_t *, int); +#endif + XPTHREAD_FUNC2(pthread_cond_init, pthread_cond_t *, pthread_condattr_t *); XPTHREAD_FUNC1(pthread_cond_destroy, pthread_cond_t *); XPTHREAD_FUNC1(pthread_cond_signal, pthread_cond_t *); @@ -183,13 +189,21 @@ void ovs_rwlock_init(const struct ovs_rwlock *l_) { struct ovs_rwlock *l = CONST_CAST(struct ovs_rwlock *, l_); + pthread_rwlockattr_t attr; int error; l->where = NULL; + + xpthread_rwlockattr_init(&attr); +#ifdef PTHREAD_RWLOCK_WRITER_NONRECURSIVE_INITIALIZER_NP + xpthread_rwlockattr_setkind_np( + &attr, PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP); +#endif error = pthread_rwlock_init(&l->lock, NULL); if (OVS_UNLIKELY(error)) { ovs_abort(error, "pthread_rwlock_init failed"); } + xpthread_rwlockattr_destroy(&attr); } void diff --git a/lib/ovs-thread.h b/lib/ovs-thread.h index 955560638..2e9a937f5 100644 --- a/lib/ovs-thread.h +++ b/lib/ovs-thread.h @@ -77,14 +77,30 @@ void xpthread_mutexattr_destroy(pthread_mutexattr_t *); void xpthread_mutexattr_settype(pthread_mutexattr_t *, int type); void xpthread_mutexattr_gettype(pthread_mutexattr_t *, int *typep); -/* Read-write lock. */ +/* Read-write lock. + * + * An ovs_rwlock does not support recursive readers, because POSIX allows + * taking the reader lock recursively to deadlock when a thread is waiting on + * the write-lock. (NetBSD does deadlock.) glibc rwlocks in their default + * configuration do not deadlock, but ovs_rwlock_init() initializes rwlocks as + * non-recursive (which will deadlock) for two reasons: + * + * - glibc only provides fairness to writers in this mode. + * + * - It's better to find bugs in the primary Open vSwitch target rather + * than exposing them only to porters. */ struct OVS_LOCKABLE ovs_rwlock { pthread_rwlock_t lock; const char *where; }; /* Initializer. */ +#ifdef PTHREAD_RWLOCK_WRITER_NONRECURSIVE_INITIALIZER_NP +#define OVS_RWLOCK_INITIALIZER \ + { PTHREAD_RWLOCK_WRITER_NONRECURSIVE_INITIALIZER_NP, NULL } +#else #define OVS_RWLOCK_INITIALIZER { PTHREAD_RWLOCK_INITIALIZER, NULL } +#endif /* ovs_rwlock functions analogous to pthread_rwlock_*() functions. * @@ -95,6 +111,13 @@ void ovs_rwlock_init(const struct ovs_rwlock *); void ovs_rwlock_destroy(const struct ovs_rwlock *); void ovs_rwlock_unlock(const struct ovs_rwlock *rwlock) OVS_RELEASES(rwlock); +/* Wrappers for pthread_rwlockattr_*() that abort the process on any error. */ +void xpthread_rwlockattr_init(pthread_rwlockattr_t *); +void xpthread_rwlockattr_destroy(pthread_rwlockattr_t *); +#ifdef PTHREAD_RWLOCK_WRITER_NONRECURSIVE_INITIALIZER_NP +void xpthread_rwlockattr_setkind_np(pthread_rwlockattr_t *, int kind); +#endif + void ovs_rwlock_wrlock_at(const struct ovs_rwlock *rwlock, const char *where) OVS_ACQ_WRLOCK(rwlock); #define ovs_rwlock_wrlock(rwlock) \ -- 2.43.0