diff mbox series

[ovs-dev] ovs-thread: Add pthread spin lock support.

Message ID 1561053656-23385-1-git-send-email-u9012063@gmail.com
State Superseded
Headers show
Series [ovs-dev] ovs-thread: Add pthread spin lock support. | expand

Commit Message

William Tu June 20, 2019, 6 p.m. UTC
The patch adds the basic spin lock functions:
ovs_spin_{lock, try_lock, unlock, init, destroy}.
I have some use cases using af_xdp, patches will
come later.

Signed-off-by: William Tu <u9012063@gmail.com>
---
 include/openvswitch/thread.h | 18 ++++++++++++++++++
 lib/ovs-thread.c             | 23 +++++++++++++++++++++++
 2 files changed, 41 insertions(+)

Comments

Li,Rongqing via dev June 26, 2019, 1:37 p.m. UTC | #1
Hi William,

On Thu, Jun 20, 2019 at 11:00:56AM -0700, William Tu wrote:
> The patch adds the basic spin lock functions:
> ovs_spin_{lock, try_lock, unlock, init, destroy}.
> I have some use cases using af_xdp, patches will
> come later.

Usually the new API is merged along with an user otherwise
it is difficult to evaluate what is really needed and etc...

It also avoids the issue of having to maintain dead code if
the API's user never comes or gets merged for whatever reason.

Does that make sense to you?

Thanks!
fbl

> 
> Signed-off-by: William Tu <u9012063@gmail.com>
> ---
>  include/openvswitch/thread.h | 18 ++++++++++++++++++
>  lib/ovs-thread.c             | 23 +++++++++++++++++++++++
>  2 files changed, 41 insertions(+)
> 
> diff --git a/include/openvswitch/thread.h b/include/openvswitch/thread.h
> index 2987db37c9dc..44cd5b77f087 100644
> --- a/include/openvswitch/thread.h
> +++ b/include/openvswitch/thread.h
> @@ -33,6 +33,11 @@ struct OVS_LOCKABLE ovs_mutex {
>      const char *where;          /* NULL if and only if uninitialized. */
>  };
>  
> +struct OVS_LOCKABLE ovs_spin {
> +    pthread_spinlock_t lock;
> +    const char *where;          /* NULL if and only if uninitialized. */
> +};
> +
>  /* "struct ovs_mutex" initializer. */
>  #ifdef PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP
>  #define OVS_MUTEX_INITIALIZER { PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP, \
> @@ -70,6 +75,19 @@ 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);
> +
> +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)
>  
>  /* Convenient once-only execution.
>   *
> diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c
> index 159d87e5b0ca..d6b4ed1d61a9 100644
> --- a/lib/ovs-thread.c
> +++ b/lib/ovs-thread.c
> @@ -75,6 +75,7 @@ static bool multithreaded;
>  LOCK_FUNCTION(mutex, lock);
>  LOCK_FUNCTION(rwlock, rdlock);
>  LOCK_FUNCTION(rwlock, wrlock);
> +LOCK_FUNCTION(spin, lock);
>  
>  #define TRY_LOCK_FUNCTION(TYPE, FUN) \
>      int \
> @@ -103,6 +104,7 @@ LOCK_FUNCTION(rwlock, wrlock);
>  TRY_LOCK_FUNCTION(mutex, trylock);
>  TRY_LOCK_FUNCTION(rwlock, tryrdlock);
>  TRY_LOCK_FUNCTION(rwlock, trywrlock);
> +TRY_LOCK_FUNCTION(spin, trylock);
>  
>  #define UNLOCK_FUNCTION(TYPE, FUN, WHERE) \
>      void \
> @@ -125,6 +127,8 @@ UNLOCK_FUNCTION(mutex, unlock, "<unlocked>");
>  UNLOCK_FUNCTION(mutex, destroy, NULL);
>  UNLOCK_FUNCTION(rwlock, unlock, "<unlocked>");
>  UNLOCK_FUNCTION(rwlock, destroy, NULL);
> +UNLOCK_FUNCTION(spin, unlock, "<unlocked>");
> +UNLOCK_FUNCTION(spin, destroy, NULL);
>  
>  #define XPTHREAD_FUNC1(FUNCTION, PARAM1)                \
>      void                                                \
> @@ -268,6 +272,25 @@ ovs_mutex_cond_wait(pthread_cond_t *cond, const struct ovs_mutex *mutex_)
>      }
>  }
>  
> +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);
> +}
> +
>  /* Initializes the 'barrier'.  'size' is the number of threads
>   * expected to hit the barrier. */
>  void
> -- 
> 2.7.4
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Ben Pfaff June 26, 2019, 2:08 p.m. UTC | #2
On Wed, Jun 26, 2019 at 10:37:49AM -0300, Flavio Leitner via dev wrote:
> 
> Hi William,
> 
> On Thu, Jun 20, 2019 at 11:00:56AM -0700, William Tu wrote:
> > The patch adds the basic spin lock functions:
> > ovs_spin_{lock, try_lock, unlock, init, destroy}.
> > I have some use cases using af_xdp, patches will
> > come later.
> 
> Usually the new API is merged along with an user otherwise
> it is difficult to evaluate what is really needed and etc...
> 
> It also avoids the issue of having to maintain dead code if
> the API's user never comes or gets merged for whatever reason.
> 
> Does that make sense to you?

It'd make sense to make this an early patch in a series for AF_XDP, if
AF_XDP uses it.
William Tu June 26, 2019, 2:29 p.m. UTC | #3
On Wed, Jun 26, 2019 at 7:08 AM Ben Pfaff <blp@ovn.org> wrote:
>
> On Wed, Jun 26, 2019 at 10:37:49AM -0300, Flavio Leitner via dev wrote:
> >
> > Hi William,
> >
> > On Thu, Jun 20, 2019 at 11:00:56AM -0700, William Tu wrote:
> > > The patch adds the basic spin lock functions:
> > > ovs_spin_{lock, try_lock, unlock, init, destroy}.
> > > I have some use cases using af_xdp, patches will
> > > come later.
> >
> > Usually the new API is merged along with an user otherwise
> > it is difficult to evaluate what is really needed and etc...
> >
> > It also avoids the issue of having to maintain dead code if
> > the API's user never comes or gets merged for whatever reason.
> >
> > Does that make sense to you?
>
> It'd make sense to make this an early patch in a series for AF_XDP, if
> AF_XDP uses it.

Thanks Ben and Flavio,
I will put this patch in the early series of AF_XDP patch.

William
diff mbox series

Patch

diff --git a/include/openvswitch/thread.h b/include/openvswitch/thread.h
index 2987db37c9dc..44cd5b77f087 100644
--- a/include/openvswitch/thread.h
+++ b/include/openvswitch/thread.h
@@ -33,6 +33,11 @@  struct OVS_LOCKABLE ovs_mutex {
     const char *where;          /* NULL if and only if uninitialized. */
 };
 
+struct OVS_LOCKABLE ovs_spin {
+    pthread_spinlock_t lock;
+    const char *where;          /* NULL if and only if uninitialized. */
+};
+
 /* "struct ovs_mutex" initializer. */
 #ifdef PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP
 #define OVS_MUTEX_INITIALIZER { PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP, \
@@ -70,6 +75,19 @@  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);
+
+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)
 
 /* Convenient once-only execution.
  *
diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c
index 159d87e5b0ca..d6b4ed1d61a9 100644
--- a/lib/ovs-thread.c
+++ b/lib/ovs-thread.c
@@ -75,6 +75,7 @@  static bool multithreaded;
 LOCK_FUNCTION(mutex, lock);
 LOCK_FUNCTION(rwlock, rdlock);
 LOCK_FUNCTION(rwlock, wrlock);
+LOCK_FUNCTION(spin, lock);
 
 #define TRY_LOCK_FUNCTION(TYPE, FUN) \
     int \
@@ -103,6 +104,7 @@  LOCK_FUNCTION(rwlock, wrlock);
 TRY_LOCK_FUNCTION(mutex, trylock);
 TRY_LOCK_FUNCTION(rwlock, tryrdlock);
 TRY_LOCK_FUNCTION(rwlock, trywrlock);
+TRY_LOCK_FUNCTION(spin, trylock);
 
 #define UNLOCK_FUNCTION(TYPE, FUN, WHERE) \
     void \
@@ -125,6 +127,8 @@  UNLOCK_FUNCTION(mutex, unlock, "<unlocked>");
 UNLOCK_FUNCTION(mutex, destroy, NULL);
 UNLOCK_FUNCTION(rwlock, unlock, "<unlocked>");
 UNLOCK_FUNCTION(rwlock, destroy, NULL);
+UNLOCK_FUNCTION(spin, unlock, "<unlocked>");
+UNLOCK_FUNCTION(spin, destroy, NULL);
 
 #define XPTHREAD_FUNC1(FUNCTION, PARAM1)                \
     void                                                \
@@ -268,6 +272,25 @@  ovs_mutex_cond_wait(pthread_cond_t *cond, const struct ovs_mutex *mutex_)
     }
 }
 
+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);
+}
+
 /* Initializes the 'barrier'.  'size' is the number of threads
  * expected to hit the barrier. */
 void