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

Ethan Jackson ethan at nicira.com
Fri Jun 21 18:36:16 UTC 2013


The #include_next in sparse/pthread.h is a bit exotic.  Would you
please include a short comment explaining why it's necessary?

sparse/pthread.h has some trailing newlines at the end of the file.

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

Acked-by: Ethan Jackson <ethan at nicira.com>

On Wed, Jun 19, 2013 at 1:17 PM, Ben Pfaff <blp at nicira.com> wrote:
> 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>
> ---
>  include/sparse/automake.mk |    1 +
>  include/sparse/pthread.h   |   61 +++++++++++++++++++++++++
>  lib/automake.mk            |    2 +
>  lib/compiler.h             |   36 ++++++++++++++-
>  lib/ovs-thread.c           |  108 ++++++++++++++++++++++++++++++++++++++++++++
>  lib/ovs-thread.h           |   89 ++++++++++++++++++++++++++++++++++++
>  6 files changed, 296 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..1e54e95
> --- /dev/null
> +++ b/include/sparse/pthread.h
> @@ -0,0 +1,61 @@
> +/*
> + * 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
> +
> +#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..f48a659
> --- /dev/null
> +++ b/lib/ovs-thread.c
> @@ -0,0 +1,108 @@
> +/*
> + * 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_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 *);
> +
> +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_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_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
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list