[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openvswitch-dev
Subject:    [ovs-dev] [threads 04/11] ovs-thread: Add support for convenient once-only initializers.
From:       blp () nicira ! com (Ben Pfaff)
Date:       2013-06-28 23:02:06
Message-ID: 20130628230206.GE32128 () nicira ! com
[Download RAW message or body]

Thanks for the review.

On Fri, Jun 28, 2013 at 03:23:24PM -0700, Ethan Jackson wrote:
> Acked-by: Ethan Jackson <ethan at nicira.com>
> 
> On Wed, Jun 19, 2013 at 1:17 PM, Ben Pfaff <blp at nicira.com> wrote:
> > pthread_once() is portable but it does not allow passing any parameters to
> > the initialization function, which is often inconvenient, because it means
> > that the function can only access data declared at file scope.  This commit
> > introduces an alternative with a more convenient interface.
> > 
> > Signed-off-by: Ben Pfaff <blp at nicira.com>
> > ---
> > Makefile.am      |   12 ++++----
> > lib/ovs-thread.c |   18 +++++++++++++
> > lib/ovs-thread.h |   76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 100 insertions(+), 6 deletions(-)
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index 193b19e..488aed2 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -185,17 +185,17 @@ config-h-check:
> > fi
> > .PHONY: config-h-check
> > 
> > -# Check that "struct vlog_ratelimit" is always declared "static".
> > -ALL_LOCAL += rate-limit-check
> > -rate-limit-check:
> > +# Check that certain data structures are always declared "static".
> > +ALL_LOCAL += static-check
> > +static-check:
> > @if test -e $(srcdir)/.git && (git --version) >/dev/null 2>&1 && \
> > -           git --no-pager grep -n -E '^[       ]+struct vlog_rate_limit.*=' \
> > $(srcdir); \ +           git --no-pager grep -n -E '^[       ]+(struct \
> > vlog_rate_limit|pthread_once_t|struct ovsthread_once).*=' $(srcdir); \ then \
> > echo "See above for list of violations of the rule that "; \
> > -           echo "'struct vlog_rate_limit' must always be 'static'"; \
> > +           echo "certain data structures must always be 'static'"; \
> > exit 1; \
> > fi
> > -.PHONY: rate-limit-check
> > +.PHONY: static-check
> > 
> > # Check that assert.h is not used outside a whitelist of files.
> > ALL_LOCAL += check-assert-h-usage
> > diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c
> > index f48a659..d69ec5e 100644
> > --- a/lib/ovs-thread.c
> > +++ b/lib/ovs-thread.c
> > @@ -105,4 +105,22 @@ xpthread_create(pthread_t *threadp, pthread_attr_t *attr,
> > ovs_abort(error, "pthread_create failed");
> > }
> > }
> > +
> > +bool
> > +ovsthread_once_start__(struct ovsthread_once *once)
> > +{
> > +    xpthread_mutex_lock(&once->mutex);
> > +    if (!ovsthread_once_is_done__(once)) {
> > +        return false;
> > +    }
> > +    xpthread_mutex_unlock(&once->mutex);
> > +    return true;
> > +}
> > +
> > +void OVS_RELEASES(once)
> > +ovsthread_once_done(struct ovsthread_once *once)
> > +{
> > +    atomic_store(&once->done, true);
> > +    xpthread_mutex_unlock(&once->mutex);
> > +}
> > #endif
> > diff --git a/lib/ovs-thread.h b/lib/ovs-thread.h
> > index 752e44f..4779030 100644
> > --- a/lib/ovs-thread.h
> > +++ b/lib/ovs-thread.h
> > @@ -18,6 +18,7 @@
> > #define OVS_THREAD_H 1
> > 
> > #include <pthread.h>
> > +#include "ovs-atomic.h"
> > #include "util.h"
> > 
> > /* glibc has some non-portable mutex types and initializers:
> > @@ -279,5 +280,80 @@ void xpthread_create(pthread_t *, pthread_attr_t *, void \
> > *(*)(void *), void *); return NAME##_set__(value);                     \
> > }
> > 
> > +/* Convenient once-only execution.
> > + *
> > + *
> > + * Problem
> > + * =======
> > + *
> > + * POSIX provides pthread_once_t and pthread_once() as primitives for running a
> > + * set of code only once per process execution.  They are used like this:
> > + *
> > + *     static void run_once(void) { ...initialization... }
> > + *     static pthread_once_t once = PTHREAD_ONCE_INIT;
> > + * ...
> > + *     pthread_once(&once, run_once);
> > + *
> > + * pthread_once() does not allow passing any parameters to the initialization
> > + * function, which is often inconvenient, because it means that the function
> > + * can only access data declared at file scope.
> > + *
> > + *
> > + * Solution
> > + * ========
> > + *
> > + * Use ovsthread_once, like this, instead:
> > + *
> > + *     static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
> > + *
> > + *     if (ovsthread_once_start(&once)) {
> > + *         ...initialization...
> > + *         ovsthread_once_done(&once);
> > + *     }
> > + */
> > +
> > +struct ovsthread_once {
> > +    atomic_bool done;
> > +    pthread_mutex_t mutex;
> > +};
> > +
> > +#define OVSTHREAD_ONCE_INITIALIZER              \
> > +    {                                           \
> > +        ATOMIC_VAR_INIT(false),                 \
> > +        PTHREAD_ADAPTIVE_MUTEX_INITIALIZER,     \
> > +    }
> > +
> > +static inline bool ovsthread_once_start(struct ovsthread_once *);
> > +void ovsthread_once_done(struct ovsthread_once *once) OVS_RELEASES(once);
> > +
> > +bool ovsthread_once_start__(struct ovsthread_once *);
> > +
> > +static inline bool
> > +ovsthread_once_is_done__(const struct ovsthread_once *once)
> > +{
> > +    bool done;
> > +
> > +    atomic_read_explicit(&once->done, &done, memory_order_relaxed);
> > +    return done;
> > +}
> > +
> > +/* Returns true if this is the first call to ovsthread_once_start() for
> > + * 'once'.  In this case, the caller should perform whatever initialization
> > + * actions it needs to do, then call ovsthread_once_done() for 'once'.
> > + *
> > + * Returns false if this is not the first call to ovsthread_once_start() for
> > + * 'once'.  In this case, the call will not return until after
> > + * ovsthread_once_done() has been called. */
> > +static inline bool
> > +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_ACQUIRE(ONCE); true; }))
> > +#endif
> > 
> > #endif /* ovs-thread.h */
> > --
> > 1.7.2.5
> > 
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev


[prev in list] [next in list] [prev in thread] [next in thread]