[ovs-dev] [threads 04/11] ovs-thread: Add support for convenient once-only initializers.

Ben Pfaff blp at nicira.com
Fri Jun 28 23:02:06 UTC 2013


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



More information about the dev mailing list