[ovs-dev] [sparse 2/3] sparse: Remove support for thread-safety annotations.

Ethan Jackson ethan at nicira.com
Tue Aug 13 00:51:15 UTC 2013


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

Agreed thank you.

Ethan

On Tue, Aug 13, 2013 at 6:54 AM, Ben Pfaff <blp at nicira.com> wrote:
> The Clang support for thread-safety annotations is much more effective
> than "sparse" support.  I found that I was unable to make the annotations
> warning-free under sparse.
>
> Signed-off-by: Ben Pfaff <blp at nicira.com>
> ---
>  include/sparse/pthread.h |   38 --------------------------------------
>  lib/compiler.h           |   29 +----------------------------
>  lib/ovs-thread.h         |    5 -----
>  ofproto/ofproto-dpif.c   |    1 +
>  ofproto/ofproto.c        |    3 ++-
>  5 files changed, 4 insertions(+), 72 deletions(-)
>
> diff --git a/include/sparse/pthread.h b/include/sparse/pthread.h
> index aa4652e..40c5ca3 100644
> --- a/include/sparse/pthread.h
> +++ b/include/sparse/pthread.h
> @@ -21,18 +21,6 @@
>  /* 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_ACQ_RDLOCK(rwlock);
> -int pthread_rwlock_wrlock(pthread_rwlock_t *rwlock) OVS_ACQ_WRLOCK(rwlock);
> -int pthread_rwlock_unlock(pthread_rwlock_t *rwlock) OVS_RELEASES(rwlock);
> -
> -int pthread_cond_wait(pthread_cond_t *, pthread_mutex_t *mutex)
> -    OVS_REQUIRES(mutex);
> -
>  /* Sparse complains about the proper PTHREAD_*_INITIALIZER definitions.
>   * Luckily, it's not a real compiler so we can overwrite it with something
>   * simple. */
> @@ -47,29 +35,3 @@ int pthread_cond_wait(pthread_cond_t *, pthread_mutex_t *mutex)
>
>  #undef PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP
>  #define PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP {}
> -
> -#define pthread_mutex_trylock(MUTEX)                    \
> -    ({                                                  \
> -        int retval = pthread_mutex_trylock(mutex);      \
> -        if (!retval) {                                  \
> -            OVS_MACRO_LOCK(MUTEX);                      \
> -        }                                               \
> -        retval;                                         \
> -    })
> -
> -#define pthread_rwlock_tryrdlock(RWLOCK)                \
> -    ({                                                  \
> -        int retval = pthread_rwlock_tryrdlock(rwlock);  \
> -        if (!retval) {                                  \
> -            OVS_MACRO_LOCK(RWLOCK);                     \
> -        }                                               \
> -        retval;                                         \
> -    })
> -#define pthread_rwlock_trywrlock(RWLOCK)                \
> -    ({                                                  \
> -        int retval = pthread_rwlock_trywrlock(rwlock);  \
> -        if (!retval) {                                  \
> -            OVS_MACRO_LOCK(RWLOCK);                     \
> -        }                                               \
> -        retval;                                         \
> -    })
> diff --git a/lib/compiler.h b/lib/compiler.h
> index 2ca81bd..519b832 100644
> --- a/lib/compiler.h
> +++ b/lib/compiler.h
> @@ -128,32 +128,7 @@
>  #define OVS_EXCLUDED(...) __attribute__((locks_excluded(__VA_ARGS__)))
>  #define OVS_ACQ_BEFORE(...) __attribute__((acquired_before(__VA_ARGS__)))
>  #define OVS_ACQ_AFTER(...) __attribute__((acquired_after(__VA_ARGS__)))
> -#elif __CHECKER__
> -/* "sparse" annotations for mutexes and mutex-like constructs.
> - *
> - * Change the thread-safety check annotations to use "context" attribute.
> - *
> - * OVS_MACRO_LOCK and OVS_MACRO_RELEASE are suitable for use within macros,
> - * where there is no function prototype to annotate. */
> -#define OVS_LOCKABLE
> -#define OVS_REQ_RDLOCK(...) __attribute__((context(MUTEX, 1, 1)))
> -#define OVS_ACQ_RDLOCK(...) __attribute__((context(MUTEX, 0, 1)))
> -#define OVS_REQ_WRLOCK(...) __attribute__((context(MUTEX, 1, 1)))
> -#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(RETVAL, ...)
> -#define OVS_TRY_RDLOCK(RETVAL, ...)
> -#define OVS_TRY_LOCK(REVAL, ...)
> -#define OVS_GUARDED
> -#define OVS_GUARDED_BY(...)
> -#define OVS_EXCLUDED(...)
> -#define OVS_RELEASES(...)   __attribute__((context(MUTEX, 1, 0)))
> -#define OVS_ACQ_BEFORE(...)
> -#define OVS_ACQ_AFTER(...)
> -#define OVS_MACRO_LOCK(...) __context__(MUTEX, 0, 1)
> -#define OVS_MACRO_RELEASE(...) __context__(MUTEX, 1, 0)
> -#else
> +#else  /* not Clang */
>  #define OVS_LOCKABLE
>  #define OVS_REQ_RDLOCK(...)
>  #define OVS_ACQ_RDLOCK(...)
> @@ -170,8 +145,6 @@
>  #define OVS_RELEASES(...)
>  #define OVS_ACQ_BEFORE(...)
>  #define OVS_ACQ_AFTER(...)
> -#define OVS_MACRO_LOCK(...)
> -#define OVS_MACRO_RELEASE(...)
>  #endif
>
>  /* ISO C says that a C implementation may choose any integer type for an enum
> diff --git a/lib/ovs-thread.h b/lib/ovs-thread.h
> index 3547686..5d3964a 100644
> --- a/lib/ovs-thread.h
> +++ b/lib/ovs-thread.h
> @@ -496,11 +496,6 @@ ovsthread_once_start(struct ovsthread_once *once)
>      return OVS_UNLIKELY(!ovsthread_once_is_done__(once)
>                          && !ovsthread_once_start__(once));
>  }
> -
> -#ifdef __CHECKER__
> -#define ovsthread_once_start(ONCE) \
> -    ((ONCE)->done ? false : ({ OVS_MACRO_LOCK((&ONCE->mutex)); true; }))
> -#endif
>
>  /* Thread ID.
>   *
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 050c818..fb43303 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -4756,6 +4756,7 @@ bool
>  rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto,
>                            const struct flow *flow, struct flow_wildcards *wc,
>                            uint8_t table_id, struct rule_dpif **rule)
> +    OVS_ACQ_RDLOCK((*rule)->up.evict)
>  {
>      struct cls_rule *cls_rule;
>      struct classifier *cls;
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index c7fc09b..cc9c800 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -5459,7 +5459,8 @@ oftable_enable_eviction(struct oftable *table,
>  /* Removes 'rule' from the oftable that contains it. */
>  static void
>  oftable_remove_rule__(struct ofproto *ofproto, struct classifier *cls,
> -                      struct rule *rule) OVS_REQ_WRLOCK(cls->rwlock)
> +                      struct rule *rule)
> +    OVS_REQ_WRLOCK(cls->rwlock) OVS_RELEASES(rule->evict)
>  {
>      classifier_remove(cls, &rule->cr);
>      if (rule->meter_id) {
> --
> 1.7.10.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
X-CudaMail-Whitelist-To: dev at openvswitch.org



More information about the dev mailing list