Thanks for the review.
On Fri, Jun 28, 2013 at 03:23:24PM -0700, Ethan Jackson wrote:
> Acked-by: Ethan Jackson <et...@nicira.com>
>
> On Wed, Jun 19, 2013 at 1:17 PM, Ben Pfaff <b...@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 <b...@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@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
