Message ID | 1562975456-97888-1-git-send-email-u9012063@gmail.com |
---|---|
State | Changes Requested |
Headers | show |
Series | [ovs-dev,PATCHv16,1/2] ovs-thread: Add pthread spin lock support. | expand |
On 13.07.2019 2:50, William Tu wrote: > The patch adds the basic spin lock functions: > ovs_spin_{lock, try_lock, unlock, init, destroy}. > OSX does not support pthread spin lock, so make it > linux only. IIUC, pthread spinlock requires some specific glibc verions (>= 2.2) so it could be not supported even on Linux. Instead of checking _POSIX_C_SOURCE version, I think it's better to just check for pthread_spin_lock function in configure script with AC_CHECK_FUNC and check for the resulted macro. Additionally we could check for this macro while checking AF_XDP support to not enable it if we have no spinlocks. What do you think? Best regards, Ilya Maximets. > > Signed-off-by: William Tu <u9012063@gmail.com> > --- > include/openvswitch/thread.h | 22 ++++++++++++++++++++++ > lib/ovs-thread.c | 31 +++++++++++++++++++++++++++++++ > 2 files changed, 53 insertions(+) > > diff --git a/include/openvswitch/thread.h b/include/openvswitch/thread.h > index 2987db37c9dc..14cc9ad73900 100644 > --- a/include/openvswitch/thread.h > +++ b/include/openvswitch/thread.h > @@ -33,6 +33,13 @@ struct OVS_LOCKABLE ovs_mutex { > const char *where; /* NULL if and only if uninitialized. */ > }; > > +#ifdef __linux__ > +struct OVS_LOCKABLE ovs_spin { > + pthread_spinlock_t lock; > + const char *where; /* NULL if and only if uninitialized. */ > +}; > +#endif > + > /* "struct ovs_mutex" initializer. */ > #ifdef PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP > #define OVS_MUTEX_INITIALIZER { PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP, \ > @@ -70,6 +77,21 @@ int ovs_mutex_trylock_at(const struct ovs_mutex *mutex, const char *where) > > void ovs_mutex_cond_wait(pthread_cond_t *, const struct ovs_mutex *mutex) > OVS_REQUIRES(mutex); > + > +#ifdef __linux__ > +void ovs_spin_init(const struct ovs_spin *); > +void ovs_spin_destroy(const struct ovs_spin *); > +void ovs_spin_unlock(const struct ovs_spin *spin) OVS_RELEASES(spin); > +void ovs_spin_lock_at(const struct ovs_spin *spin, const char *where) > + OVS_ACQUIRES(spin); > +#define ovs_spin_lock(spin) \ > + ovs_spin_lock_at(spin, OVS_SOURCE_LOCATOR) > + > +int ovs_spin_trylock_at(const struct ovs_spin *spin, const char *where) > + OVS_TRY_LOCK(0, spin); > +#define ovs_spin_trylock(spin) \ > + ovs_spin_trylock_at(spin, OVS_SOURCE_LOCATOR) > +#endif > > /* Convenient once-only execution. > * > diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c > index 159d87e5b0ca..29a0b9e57acd 100644 > --- a/lib/ovs-thread.c > +++ b/lib/ovs-thread.c > @@ -75,6 +75,9 @@ static bool multithreaded; > LOCK_FUNCTION(mutex, lock); > LOCK_FUNCTION(rwlock, rdlock); > LOCK_FUNCTION(rwlock, wrlock); > +#ifdef __linux__ > +LOCK_FUNCTION(spin, lock); > +#endif > > #define TRY_LOCK_FUNCTION(TYPE, FUN) \ > int \ > @@ -103,6 +106,9 @@ LOCK_FUNCTION(rwlock, wrlock); > TRY_LOCK_FUNCTION(mutex, trylock); > TRY_LOCK_FUNCTION(rwlock, tryrdlock); > TRY_LOCK_FUNCTION(rwlock, trywrlock); > +#ifdef __linux__ > +TRY_LOCK_FUNCTION(spin, trylock); > +#endif > > #define UNLOCK_FUNCTION(TYPE, FUN, WHERE) \ > void \ > @@ -125,6 +131,10 @@ UNLOCK_FUNCTION(mutex, unlock, "<unlocked>"); > UNLOCK_FUNCTION(mutex, destroy, NULL); > UNLOCK_FUNCTION(rwlock, unlock, "<unlocked>"); > UNLOCK_FUNCTION(rwlock, destroy, NULL); > +#ifdef __linux__ > +UNLOCK_FUNCTION(spin, unlock, "<unlocked>"); > +UNLOCK_FUNCTION(spin, destroy, NULL); > +#endif > > #define XPTHREAD_FUNC1(FUNCTION, PARAM1) \ > void \ > @@ -268,6 +278,27 @@ ovs_mutex_cond_wait(pthread_cond_t *cond, const struct ovs_mutex *mutex_) > } > } > > +#ifdef __linux__ > +static void > +ovs_spin_init__(const struct ovs_spin *l_, int pshared) > +{ > + struct ovs_spin *l = CONST_CAST(struct ovs_spin *, l_); > + int error; > + > + l->where = "<unlocked>"; > + error = pthread_spin_init(&l->lock, pshared); > + if (OVS_UNLIKELY(error)) { > + ovs_abort(error, "pthread_spin_failed"); > + } > +} > + > +void > +ovs_spin_init(const struct ovs_spin *spin) > +{ > + ovs_spin_init__(spin, PTHREAD_PROCESS_PRIVATE); > +} > +#endif > + > /* Initializes the 'barrier'. 'size' is the number of threads > * expected to hit the barrier. */ > void >
On Mon, Jul 15, 2019 at 05:28:44PM +0300, Ilya Maximets wrote: > On 13.07.2019 2:50, William Tu wrote: > > The patch adds the basic spin lock functions: > > ovs_spin_{lock, try_lock, unlock, init, destroy}. > > OSX does not support pthread spin lock, so make it > > linux only. > > IIUC, pthread spinlock requires some specific glibc verions (>= 2.2) > so it could be not supported even on Linux. Instead of checking > _POSIX_C_SOURCE version, I think it's better to just check for > pthread_spin_lock function in configure script with AC_CHECK_FUNC and > check for the resulted macro. > Additionally we could check for this macro while checking AF_XDP support > to not enable it if we have no spinlocks. glibc 2.2 was released in 2000. I don't think that it is worth worrying that a user might have such an ancient version of glibc. However, I think that feature checks are generally better, so I support the change.
On Mon, Jul 15, 2019 at 11:54 AM Ben Pfaff <blp@ovn.org> wrote: > > On Mon, Jul 15, 2019 at 05:28:44PM +0300, Ilya Maximets wrote: > > On 13.07.2019 2:50, William Tu wrote: > > > The patch adds the basic spin lock functions: > > > ovs_spin_{lock, try_lock, unlock, init, destroy}. > > > OSX does not support pthread spin lock, so make it > > > linux only. > > > > IIUC, pthread spinlock requires some specific glibc verions (>= 2.2) > > so it could be not supported even on Linux. Instead of checking > > _POSIX_C_SOURCE version, I think it's better to just check for > > pthread_spin_lock function in configure script with AC_CHECK_FUNC and > > check for the resulted macro. > > Additionally we could check for this macro while checking AF_XDP support > > to not enable it if we have no spinlocks. > > glibc 2.2 was released in 2000. I don't think that it is worth worrying > that a user might have such an ancient version of glibc. > > However, I think that feature checks are generally better, so I support > the change. Thanks Ben and Ilya. I will add AC_CHECK_FUNC for checking the existence of pthread_spin_lock. If not exist, this patch won't get compile in and the following AF_XDP will be disable. Feature Test Macro Requirements for pthread_spin_lock _POSIX_C_SOURCE >= 200112L Since it's been a while, I won't add the _POSIX_C_SOURCE check. William
On 13.07.2019 2:50, William Tu wrote: > The patch adds the basic spin lock functions: > ovs_spin_{lock, try_lock, unlock, init, destroy}. > OSX does not support pthread spin lock, so make it > linux only. > > Signed-off-by: William Tu <u9012063@gmail.com> > --- > include/openvswitch/thread.h | 22 ++++++++++++++++++++++ > lib/ovs-thread.c | 31 +++++++++++++++++++++++++++++++ > 2 files changed, 53 insertions(+) > > diff --git a/include/openvswitch/thread.h b/include/openvswitch/thread.h > index 2987db37c9dc..14cc9ad73900 100644 > --- a/include/openvswitch/thread.h > +++ b/include/openvswitch/thread.h > @@ -33,6 +33,13 @@ struct OVS_LOCKABLE ovs_mutex { > const char *where; /* NULL if and only if uninitialized. */ > }; > > +#ifdef __linux__ > +struct OVS_LOCKABLE ovs_spin { > + pthread_spinlock_t lock; > + const char *where; /* NULL if and only if uninitialized. */ > +}; > +#endif > + > /* "struct ovs_mutex" initializer. */ > #ifdef PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP > #define OVS_MUTEX_INITIALIZER { PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP, \ > @@ -70,6 +77,21 @@ int ovs_mutex_trylock_at(const struct ovs_mutex *mutex, const char *where) > > void ovs_mutex_cond_wait(pthread_cond_t *, const struct ovs_mutex *mutex) > OVS_REQUIRES(mutex); > + > +#ifdef __linux__ > +void ovs_spin_init(const struct ovs_spin *); > +void ovs_spin_destroy(const struct ovs_spin *); > +void ovs_spin_unlock(const struct ovs_spin *spin) OVS_RELEASES(spin); > +void ovs_spin_lock_at(const struct ovs_spin *spin, const char *where) > + OVS_ACQUIRES(spin); > +#define ovs_spin_lock(spin) \ > + ovs_spin_lock_at(spin, OVS_SOURCE_LOCATOR) > + > +int ovs_spin_trylock_at(const struct ovs_spin *spin, const char *where) > + OVS_TRY_LOCK(0, spin); > +#define ovs_spin_trylock(spin) \ > + ovs_spin_trylock_at(spin, OVS_SOURCE_LOCATOR) > +#endif > > /* Convenient once-only execution. > * > diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c > index 159d87e5b0ca..29a0b9e57acd 100644 > --- a/lib/ovs-thread.c > +++ b/lib/ovs-thread.c > @@ -75,6 +75,9 @@ static bool multithreaded; > LOCK_FUNCTION(mutex, lock); > LOCK_FUNCTION(rwlock, rdlock); > LOCK_FUNCTION(rwlock, wrlock); > +#ifdef __linux__ > +LOCK_FUNCTION(spin, lock); > +#endif > > #define TRY_LOCK_FUNCTION(TYPE, FUN) \ > int \ > @@ -103,6 +106,9 @@ LOCK_FUNCTION(rwlock, wrlock); > TRY_LOCK_FUNCTION(mutex, trylock); > TRY_LOCK_FUNCTION(rwlock, tryrdlock); > TRY_LOCK_FUNCTION(rwlock, trywrlock); > +#ifdef __linux__ > +TRY_LOCK_FUNCTION(spin, trylock); > +#endif > > #define UNLOCK_FUNCTION(TYPE, FUN, WHERE) \ > void \ > @@ -125,6 +131,10 @@ UNLOCK_FUNCTION(mutex, unlock, "<unlocked>"); > UNLOCK_FUNCTION(mutex, destroy, NULL); > UNLOCK_FUNCTION(rwlock, unlock, "<unlocked>"); > UNLOCK_FUNCTION(rwlock, destroy, NULL); > +#ifdef __linux__ > +UNLOCK_FUNCTION(spin, unlock, "<unlocked>"); > +UNLOCK_FUNCTION(spin, destroy, NULL); > +#endif > > #define XPTHREAD_FUNC1(FUNCTION, PARAM1) \ > void \ > @@ -268,6 +278,27 @@ ovs_mutex_cond_wait(pthread_cond_t *cond, const struct ovs_mutex *mutex_) > } > } > > +#ifdef __linux__ > +static void > +ovs_spin_init__(const struct ovs_spin *l_, int pshared) > +{ > + struct ovs_spin *l = CONST_CAST(struct ovs_spin *, l_); > + int error; > + > + l->where = "<unlocked>"; > + error = pthread_spin_init(&l->lock, pshared); > + if (OVS_UNLIKELY(error)) { > + ovs_abort(error, "pthread_spin_failed"); s/pthread_spin_failed/pthread_spin_init failed/ > + } > +} > + > +void > +ovs_spin_init(const struct ovs_spin *spin) > +{ > + ovs_spin_init__(spin, PTHREAD_PROCESS_PRIVATE); > +} > +#endif > + > /* Initializes the 'barrier'. 'size' is the number of threads > * expected to hit the barrier. */ > void >
diff --git a/include/openvswitch/thread.h b/include/openvswitch/thread.h index 2987db37c9dc..14cc9ad73900 100644 --- a/include/openvswitch/thread.h +++ b/include/openvswitch/thread.h @@ -33,6 +33,13 @@ struct OVS_LOCKABLE ovs_mutex { const char *where; /* NULL if and only if uninitialized. */ }; +#ifdef __linux__ +struct OVS_LOCKABLE ovs_spin { + pthread_spinlock_t lock; + const char *where; /* NULL if and only if uninitialized. */ +}; +#endif + /* "struct ovs_mutex" initializer. */ #ifdef PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP #define OVS_MUTEX_INITIALIZER { PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP, \ @@ -70,6 +77,21 @@ int ovs_mutex_trylock_at(const struct ovs_mutex *mutex, const char *where) void ovs_mutex_cond_wait(pthread_cond_t *, const struct ovs_mutex *mutex) OVS_REQUIRES(mutex); + +#ifdef __linux__ +void ovs_spin_init(const struct ovs_spin *); +void ovs_spin_destroy(const struct ovs_spin *); +void ovs_spin_unlock(const struct ovs_spin *spin) OVS_RELEASES(spin); +void ovs_spin_lock_at(const struct ovs_spin *spin, const char *where) + OVS_ACQUIRES(spin); +#define ovs_spin_lock(spin) \ + ovs_spin_lock_at(spin, OVS_SOURCE_LOCATOR) + +int ovs_spin_trylock_at(const struct ovs_spin *spin, const char *where) + OVS_TRY_LOCK(0, spin); +#define ovs_spin_trylock(spin) \ + ovs_spin_trylock_at(spin, OVS_SOURCE_LOCATOR) +#endif /* Convenient once-only execution. * diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c index 159d87e5b0ca..29a0b9e57acd 100644 --- a/lib/ovs-thread.c +++ b/lib/ovs-thread.c @@ -75,6 +75,9 @@ static bool multithreaded; LOCK_FUNCTION(mutex, lock); LOCK_FUNCTION(rwlock, rdlock); LOCK_FUNCTION(rwlock, wrlock); +#ifdef __linux__ +LOCK_FUNCTION(spin, lock); +#endif #define TRY_LOCK_FUNCTION(TYPE, FUN) \ int \ @@ -103,6 +106,9 @@ LOCK_FUNCTION(rwlock, wrlock); TRY_LOCK_FUNCTION(mutex, trylock); TRY_LOCK_FUNCTION(rwlock, tryrdlock); TRY_LOCK_FUNCTION(rwlock, trywrlock); +#ifdef __linux__ +TRY_LOCK_FUNCTION(spin, trylock); +#endif #define UNLOCK_FUNCTION(TYPE, FUN, WHERE) \ void \ @@ -125,6 +131,10 @@ UNLOCK_FUNCTION(mutex, unlock, "<unlocked>"); UNLOCK_FUNCTION(mutex, destroy, NULL); UNLOCK_FUNCTION(rwlock, unlock, "<unlocked>"); UNLOCK_FUNCTION(rwlock, destroy, NULL); +#ifdef __linux__ +UNLOCK_FUNCTION(spin, unlock, "<unlocked>"); +UNLOCK_FUNCTION(spin, destroy, NULL); +#endif #define XPTHREAD_FUNC1(FUNCTION, PARAM1) \ void \ @@ -268,6 +278,27 @@ ovs_mutex_cond_wait(pthread_cond_t *cond, const struct ovs_mutex *mutex_) } } +#ifdef __linux__ +static void +ovs_spin_init__(const struct ovs_spin *l_, int pshared) +{ + struct ovs_spin *l = CONST_CAST(struct ovs_spin *, l_); + int error; + + l->where = "<unlocked>"; + error = pthread_spin_init(&l->lock, pshared); + if (OVS_UNLIKELY(error)) { + ovs_abort(error, "pthread_spin_failed"); + } +} + +void +ovs_spin_init(const struct ovs_spin *spin) +{ + ovs_spin_init__(spin, PTHREAD_PROCESS_PRIVATE); +} +#endif + /* Initializes the 'barrier'. 'size' is the number of threads * expected to hit the barrier. */ void
The patch adds the basic spin lock functions: ovs_spin_{lock, try_lock, unlock, init, destroy}. OSX does not support pthread spin lock, so make it linux only. Signed-off-by: William Tu <u9012063@gmail.com> --- include/openvswitch/thread.h | 22 ++++++++++++++++++++++ lib/ovs-thread.c | 31 +++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+)