relay: use urcu_ref_get_unless_zero
authorMathieu Desnoyers <mathieu.desnoyers@efficios.com>
Thu, 17 Sep 2015 16:24:32 +0000 (12:24 -0400)
committerJérémie Galarneau <jeremie.galarneau@efficios.com>
Mon, 22 May 2017 15:57:01 +0000 (11:57 -0400)
This allows removing the reflock be performing this check and increment
atomically.

The minimum version of userspace-rcu is bumped to 0.9.0 as
urcu_ref_get_unless_zero() was introduced as part of that
release.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
15 files changed:
README.md
configure.ac
doc/relayd-architecture.txt
src/bin/lttng-relayd/connection.c
src/bin/lttng-relayd/connection.h
src/bin/lttng-relayd/ctf-trace.c
src/bin/lttng-relayd/ctf-trace.h
src/bin/lttng-relayd/index.c
src/bin/lttng-relayd/index.h
src/bin/lttng-relayd/session.c
src/bin/lttng-relayd/session.h
src/bin/lttng-relayd/stream.c
src/bin/lttng-relayd/stream.h
src/bin/lttng-relayd/viewer-stream.c
src/bin/lttng-relayd/viewer-stream.h

index 4d72ed43d9c893d1bf6e2358d1a8f0b086412404..6dbf19f7f12f5b19813614d9c4c216a6c0c422c3 100644 (file)
--- a/README.md
+++ b/README.md
@@ -23,7 +23,7 @@ components:
     that, the kernel version may probably be older, but we can't provide
     any guarantee. Please let us know if you are able to go lower
     without any problems.
     that, the kernel version may probably be older, but we can't provide
     any guarantee. Please let us know if you are able to go lower
     without any problems.
-  - **[`liburcu`](http://www.liburcu.org/) >= 0.8.0**: userspace RCU library,
+  - **[`liburcu`](http://www.liburcu.org/) >= 0.9.0**: userspace RCU library,
     by Mathieu Desnoyers and Paul E. McKenney.
   - **`libpopt` >= 1.13**:  command line arguments parsing library.
     - Debian/Ubuntu package: `libpopt-dev`
     by Mathieu Desnoyers and Paul E. McKenney.
   - **`libpopt` >= 1.13**:  command line arguments parsing library.
     - Debian/Ubuntu package: `libpopt-dev`
index 197288d9af88ae194f4e2e80882e6c16b9654a24..9239e3b0b0f8afcc34d080917fab4333ad6ed99b 100644 (file)
@@ -385,7 +385,7 @@ AM_CONDITIONAL([LTTNG_BUILD_WITH_LIBC_UUID], [test "x$link_with_libc_uuid" = "xy
 AC_CHECK_FUNC([clock_gettime], [AC_DEFINE_UNQUOTED([LTTNG_HAVE_CLOCK_GETTIME], 1, [Has clock_gettime() support.])])
 
 # URCU library version needed or newer
 AC_CHECK_FUNC([clock_gettime], [AC_DEFINE_UNQUOTED([LTTNG_HAVE_CLOCK_GETTIME], 1, [Has clock_gettime() support.])])
 
 # URCU library version needed or newer
-m4_define([WRONG_LIBURCU_MSG], [Userspace RCU (liburcu) >= 0.8.0 is needed])
+m4_define([WRONG_LIBURCU_MSG], [Userspace RCU (liburcu) >= 0.9.0 is needed])
 
 # Check liburcu needed function calls
 AC_CHECK_DECL([cds_list_add], [],
 
 # Check liburcu needed function calls
 AC_CHECK_DECL([cds_list_add], [],
@@ -410,6 +410,11 @@ AC_CHECK_DECL([cmm_smp_mb__before_uatomic_or], [],
         [AC_MSG_ERROR([WRONG_LIBURCU_MSG])], [[#include <urcu.h>]]
 )
 
         [AC_MSG_ERROR([WRONG_LIBURCU_MSG])], [[#include <urcu.h>]]
 )
 
+#Function added in urcu 0.9.0
+AC_CHECK_DECL([urcu_ref_get_unless_zero], [],
+        [AC_MSG_ERROR([WRONG_LIBURCU_MSG])], [[#include <urcu/ref.h>]]
+)
+
 # Check kmod library
 AC_ARG_WITH(kmod-prefix,
   AS_HELP_STRING([--with-kmod-prefix=PATH],
 # Check kmod library
 AC_ARG_WITH(kmod-prefix,
   AS_HELP_STRING([--with-kmod-prefix=PATH],
index 1491da90fb3f3b213a158211c48b2b869d7079fd..56c81241fa6b363e18361780b367ba3056ea8320 100644 (file)
@@ -44,10 +44,7 @@ A RCU lookup+refcounting scheme has been introduced for all objects
 scheme allows looking up the objects or doing a traversal on the RCU
 linked list or hash table in combination with a getter on the object.
 This getter validates that there is still at least one reference to the
 scheme allows looking up the objects or doing a traversal on the RCU
 linked list or hash table in combination with a getter on the object.
 This getter validates that there is still at least one reference to the
-object, else the lookup acts just as if the object does not exist. This
-scheme is protected by a "reflock" mutex in each object. "reflock"
-mutexes can be nested from the innermost object to the outermost object.
-IOW, the session reflock can nest within the ctf-trace reflock.
+object, else the lookup acts just as if the object does not exist.
 
 The relay_connection (connection between the sessiond/consumer and the
 relayd) is the outermost object of its hierarchy.
 
 The relay_connection (connection between the sessiond/consumer and the
 relayd) is the outermost object of its hierarchy.
@@ -61,8 +58,6 @@ live.c client thread) when objects are shared. Locks can be nested from
 the outermost object to the innermost object. IOW, the ctf-trace lock can
 nest within the session lock.
 
 the outermost object to the innermost object. IOW, the ctf-trace lock can
 nest within the session lock.
 
-A "lock" should never nest within a "reflock".
-
 RCU linked lists are used to iterate using RCU, and are protected by
 their own mutex for modifications. Iterations should be confirmed using
 the object "getter" to ensure its refcount is not 0 (except in cases
 RCU linked lists are used to iterate using RCU, and are protected by
 their own mutex for modifications. Iterations should be confirmed using
 the object "getter" to ensure its refcount is not 0 (except in cases
index f3d7e642ef2aa0139bce4a1f927d8a3ee56f1f77..e8c3d4bea5a50f2a28d69d88a94fb7b4744c168d 100644 (file)
 
 bool connection_get(struct relay_connection *conn)
 {
 
 bool connection_get(struct relay_connection *conn)
 {
-       bool has_ref = false;
-
-       pthread_mutex_lock(&conn->reflock);
-       if (conn->ref.refcount != 0) {
-               has_ref = true;
-               urcu_ref_get(&conn->ref);
-       }
-       pthread_mutex_unlock(&conn->reflock);
-
-       return has_ref;
+       return urcu_ref_get_unless_zero(&conn->ref);
 }
 
 struct relay_connection *connection_get_by_sock(struct lttng_ht *relay_connections_ht,
 }
 
 struct relay_connection *connection_get_by_sock(struct lttng_ht *relay_connections_ht,
@@ -75,7 +66,6 @@ struct relay_connection *connection_create(struct lttcomm_sock *sock,
                PERROR("zmalloc relay connection");
                goto end;
        }
                PERROR("zmalloc relay connection");
                goto end;
        }
-       pthread_mutex_init(&conn->reflock, NULL);
        urcu_ref_init(&conn->ref);
        conn->type = type;
        conn->sock = sock;
        urcu_ref_init(&conn->ref);
        conn->type = type;
        conn->sock = sock;
@@ -131,9 +121,7 @@ static void connection_release(struct urcu_ref *ref)
 void connection_put(struct relay_connection *conn)
 {
        rcu_read_lock();
 void connection_put(struct relay_connection *conn)
 {
        rcu_read_lock();
-       pthread_mutex_lock(&conn->reflock);
        urcu_ref_put(&conn->ref, connection_release);
        urcu_ref_put(&conn->ref, connection_release);
-       pthread_mutex_unlock(&conn->reflock);
        rcu_read_unlock();
 }
 
        rcu_read_unlock();
 }
 
index 44386381325e531fb45ea45f233e6f7f3a2ef946..43e8a1de641411ace69a9222c2815c43285a3b49 100644 (file)
@@ -77,7 +77,6 @@ struct relay_connection {
        uint32_t minor;
 
        struct urcu_ref ref;
        uint32_t minor;
 
        struct urcu_ref ref;
-       pthread_mutex_t reflock;
 
        bool version_check_done;
 
 
        bool version_check_done;
 
index 27009efc38270bd0e46adc6b046f59134c8b5b12..4ca6dba1e40c489bdd92f6a0a90636d5841dd94e 100644 (file)
@@ -75,17 +75,7 @@ void ctf_trace_release(struct urcu_ref *ref)
  */
 bool ctf_trace_get(struct ctf_trace *trace)
 {
  */
 bool ctf_trace_get(struct ctf_trace *trace)
 {
-       bool has_ref = false;
-
-       /* Confirm that the trace refcount has not reached 0. */
-       pthread_mutex_lock(&trace->reflock);
-       if (trace->ref.refcount != 0) {
-               has_ref = true;
-               urcu_ref_get(&trace->ref);
-       }
-       pthread_mutex_unlock(&trace->reflock);
-
-       return has_ref;
+       return urcu_ref_get_unless_zero(&trace->ref);
 }
 
 /*
 }
 
 /*
@@ -123,7 +113,6 @@ static struct ctf_trace *ctf_trace_create(struct relay_session *session,
        trace->session = session;
        urcu_ref_init(&trace->ref);
        pthread_mutex_init(&trace->lock, NULL);
        trace->session = session;
        urcu_ref_init(&trace->ref);
        pthread_mutex_init(&trace->lock, NULL);
-       pthread_mutex_init(&trace->reflock, NULL);
        pthread_mutex_init(&trace->stream_list_lock, NULL);
        lttng_ht_add_str(session->ctf_traces_ht, &trace->node);
 
        pthread_mutex_init(&trace->stream_list_lock, NULL);
        lttng_ht_add_str(session->ctf_traces_ht, &trace->node);
 
@@ -168,9 +157,7 @@ end:
 void ctf_trace_put(struct ctf_trace *trace)
 {
        rcu_read_lock();
 void ctf_trace_put(struct ctf_trace *trace)
 {
        rcu_read_lock();
-       pthread_mutex_lock(&trace->reflock);
        urcu_ref_put(&trace->ref, ctf_trace_release);
        urcu_ref_put(&trace->ref, ctf_trace_release);
-       pthread_mutex_unlock(&trace->reflock);
        rcu_read_unlock();
 }
 
        rcu_read_unlock();
 }
 
index d051f80837e4bcbbae88a7a12c12b6504968789a..9903d38e874a4e86760e25ee01a7f2605cc789b8 100644 (file)
 #include "viewer-stream.h"
 
 struct ctf_trace {
 #include "viewer-stream.h"
 
 struct ctf_trace {
-       /*
-        * The ctf_trace reflock nests inside the stream reflock.
-        */
-       pthread_mutex_t reflock;        /* Protects refcounting */
        struct urcu_ref ref;            /* Every stream has a ref on the trace. */
        struct relay_session *session;  /* Back ref to trace session */
 
        struct urcu_ref ref;            /* Every stream has a ref on the trace. */
        struct relay_session *session;  /* Back ref to trace session */
 
index b15bbcd7702954e675a98c06e02ab31058ff7499..1b14e3e078e1d96fb0d88f93301fac8e0c3e2d13 100644 (file)
@@ -58,7 +58,6 @@ static struct relay_index *relay_index_create(struct relay_stream *stream,
 
        lttng_ht_node_init_u64(&index->index_n, net_seq_num);
        pthread_mutex_init(&index->lock, NULL);
 
        lttng_ht_node_init_u64(&index->index_n, net_seq_num);
        pthread_mutex_init(&index->lock, NULL);
-       pthread_mutex_init(&index->reflock, NULL);
        urcu_ref_init(&index->ref);
 
 end:
        urcu_ref_init(&index->ref);
 
 end:
@@ -98,21 +97,11 @@ static struct relay_index *relay_index_add_unique(struct relay_stream *stream,
  */
 static bool relay_index_get(struct relay_index *index)
 {
  */
 static bool relay_index_get(struct relay_index *index)
 {
-       bool has_ref = false;
-
        DBG2("index get for stream id %" PRIu64 " and seqnum %" PRIu64 " refcount %d",
                        index->stream->stream_handle, index->index_n.key,
                        (int) index->ref.refcount);
 
        DBG2("index get for stream id %" PRIu64 " and seqnum %" PRIu64 " refcount %d",
                        index->stream->stream_handle, index->index_n.key,
                        (int) index->ref.refcount);
 
-       /* Confirm that the index refcount has not reached 0. */
-       pthread_mutex_lock(&index->reflock);
-       if (index->ref.refcount != 0) {
-               has_ref = true;
-               urcu_ref_get(&index->ref);
-       }
-       pthread_mutex_unlock(&index->reflock);
-
-       return has_ref;
+       return urcu_ref_get_unless_zero(&index->ref);
 }
 
 /*
 }
 
 /*
@@ -265,10 +254,8 @@ void relay_index_put(struct relay_index *index)
         * Index lock ensures that concurrent test and update of stream
         * ref is atomic.
         */
         * Index lock ensures that concurrent test and update of stream
         * ref is atomic.
         */
-       pthread_mutex_lock(&index->reflock);
        assert(index->ref.refcount != 0);
        urcu_ref_put(&index->ref, index_release);
        assert(index->ref.refcount != 0);
        urcu_ref_put(&index->ref, index_release);
-       pthread_mutex_unlock(&index->reflock);
        rcu_read_unlock();
 }
 
        rcu_read_unlock();
 }
 
index 80fe86ab024f8e668e51a1553d52385e1c666ad2..dda5b910b5b8c8df0b88dcd1bda2fd50f9be8c66 100644 (file)
@@ -34,7 +34,6 @@ struct relay_index {
        /*
         * index lock nests inside stream lock.
         */
        /*
         * index lock nests inside stream lock.
         */
-       pthread_mutex_t reflock;        /* Protects refcounting. */
        struct urcu_ref ref;            /* Reference from getters. */
        struct relay_stream *stream;    /* Back ref to stream */
 
        struct urcu_ref ref;            /* Reference from getters. */
        struct relay_stream *stream;    /* Back ref to stream */
 
index f76fb4a42e7606b8ebfea427033b00e3de192020..3ea8e50d6f9aaf60d64789069636c9ea13736e50 100644 (file)
@@ -69,7 +69,6 @@ struct relay_session *session_create(const char *session_name,
        urcu_ref_init(&session->ref);
        CDS_INIT_LIST_HEAD(&session->recv_list);
        pthread_mutex_init(&session->lock, NULL);
        urcu_ref_init(&session->ref);
        CDS_INIT_LIST_HEAD(&session->recv_list);
        pthread_mutex_init(&session->lock, NULL);
-       pthread_mutex_init(&session->reflock, NULL);
        pthread_mutex_init(&session->recv_list_lock, NULL);
 
        session->live_timer = live_timer;
        pthread_mutex_init(&session->recv_list_lock, NULL);
 
        session->live_timer = live_timer;
@@ -86,16 +85,7 @@ error:
 /* Should be called with RCU read-side lock held. */
 bool session_get(struct relay_session *session)
 {
 /* Should be called with RCU read-side lock held. */
 bool session_get(struct relay_session *session)
 {
-       bool has_ref = false;
-
-       pthread_mutex_lock(&session->reflock);
-       if (session->ref.refcount != 0) {
-               has_ref = true;
-               urcu_ref_get(&session->ref);
-       }
-       pthread_mutex_unlock(&session->reflock);
-
-       return has_ref;
+       return urcu_ref_get_unless_zero(&session->ref);
 }
 
 /*
 }
 
 /*
@@ -178,9 +168,7 @@ void session_release(struct urcu_ref *ref)
 void session_put(struct relay_session *session)
 {
        rcu_read_lock();
 void session_put(struct relay_session *session)
 {
        rcu_read_lock();
-       pthread_mutex_lock(&session->reflock);
        urcu_ref_put(&session->ref, session_release);
        urcu_ref_put(&session->ref, session_release);
-       pthread_mutex_unlock(&session->reflock);
        rcu_read_unlock();
 }
 
        rcu_read_unlock();
 }
 
index 4c8f8a61f402dcd98f36fbba780a86137fb02ad0..2410fd483ad8ae74f82902c8aafac152573f506d 100644 (file)
@@ -54,8 +54,6 @@ struct relay_session {
         */
 
        struct urcu_ref ref;
         */
 
        struct urcu_ref ref;
-       /* session reflock nests inside ctf_trace reflock. */
-       pthread_mutex_t reflock;
 
        pthread_mutex_t lock;
 
 
        pthread_mutex_t lock;
 
index e9c7ad172bc0d2f0e44ff3925ae929bd0b8e1398..04ff467de95b637fda1f2943c3042c591426d2a1 100644 (file)
 /* Should be called with RCU read-side lock held. */
 bool stream_get(struct relay_stream *stream)
 {
 /* Should be called with RCU read-side lock held. */
 bool stream_get(struct relay_stream *stream)
 {
-       bool has_ref = false;
-
-       pthread_mutex_lock(&stream->reflock);
-       if (stream->ref.refcount != 0) {
-               has_ref = true;
-               urcu_ref_get(&stream->ref);
-       }
-       pthread_mutex_unlock(&stream->reflock);
-
-       return has_ref;
+       return urcu_ref_get_unless_zero(&stream->ref);
 }
 
 /*
 }
 
 /*
@@ -100,7 +91,6 @@ struct relay_stream *stream_create(struct ctf_trace *trace,
        stream->channel_name = channel_name;
        lttng_ht_node_init_u64(&stream->node, stream->stream_handle);
        pthread_mutex_init(&stream->lock, NULL);
        stream->channel_name = channel_name;
        lttng_ht_node_init_u64(&stream->node, stream->stream_handle);
        pthread_mutex_init(&stream->lock, NULL);
-       pthread_mutex_init(&stream->reflock, NULL);
        urcu_ref_init(&stream->ref);
        ctf_trace_get(trace);
        stream->trace = trace;
        urcu_ref_init(&stream->ref);
        ctf_trace_get(trace);
        stream->trace = trace;
@@ -227,8 +217,7 @@ unlock:
 
 /*
  * Stream must be protected by holding the stream lock or by virtue of being
 
 /*
  * Stream must be protected by holding the stream lock or by virtue of being
- * called from stream_destroy, in which case it is guaranteed to be accessed
- * from a single thread by the reflock.
+ * called from stream_destroy.
  */
 static void stream_unpublish(struct relay_stream *stream)
 {
  */
 static void stream_unpublish(struct relay_stream *stream)
 {
@@ -278,9 +267,6 @@ static void stream_destroy_rcu(struct rcu_head *rcu_head)
 /*
  * No need to take stream->lock since this is only called on the final
  * stream_put which ensures that a single thread may act on the stream.
 /*
  * No need to take stream->lock since this is only called on the final
  * stream_put which ensures that a single thread may act on the stream.
- *
- * At that point, the object is also protected by the reflock which
- * guarantees that no other thread may share ownership of this stream.
  */
 static void stream_release(struct urcu_ref *ref)
 {
  */
 static void stream_release(struct urcu_ref *ref)
 {
@@ -321,15 +307,7 @@ static void stream_release(struct urcu_ref *ref)
 void stream_put(struct relay_stream *stream)
 {
        DBG("stream put for stream id %" PRIu64, stream->stream_handle);
 void stream_put(struct relay_stream *stream)
 {
        DBG("stream put for stream id %" PRIu64, stream->stream_handle);
-       /*
-        * Ensure existence of stream->reflock for stream unlock.
-        */
        rcu_read_lock();
        rcu_read_lock();
-       /*
-        * Stream reflock ensures that concurrent test and update of
-        * stream ref is atomic.
-        */
-       pthread_mutex_lock(&stream->reflock);
        assert(stream->ref.refcount != 0);
        /*
         * Wait until we have processed all the stream packets before
        assert(stream->ref.refcount != 0);
        /*
         * Wait until we have processed all the stream packets before
@@ -339,7 +317,6 @@ void stream_put(struct relay_stream *stream)
                        stream->stream_handle,
                        (int) stream->ref.refcount);
        urcu_ref_put(&stream->ref, stream_release);
                        stream->stream_handle,
                        (int) stream->ref.refcount);
        urcu_ref_put(&stream->ref, stream_release);
-       pthread_mutex_unlock(&stream->reflock);
        rcu_read_unlock();
 }
 
        rcu_read_unlock();
 }
 
index d471c7b7f49561c7c1882b5c584822daf037fcb5..e385032cb71f98d4994dfad241e915f61e0cb28a 100644 (file)
 struct relay_stream {
        uint64_t stream_handle;
 
 struct relay_stream {
        uint64_t stream_handle;
 
-       /*
-        * reflock used to synchronize the closing of this stream.
-        * stream reflock nests inside viewer stream reflock.
-        * stream reflock nests inside index reflock.
-        */
-       pthread_mutex_t reflock;
        struct urcu_ref ref;
        /* Back reference to trace. Protected by refcount on trace object. */
        struct ctf_trace *trace;
        struct urcu_ref ref;
        /* Back reference to trace. Protected by refcount on trace object. */
        struct ctf_trace *trace;
index 8a3b09a92060ef25e9f9f8f50b2d36d15c2de159..60aa4371d5cc446bb4dac4bdd654e13d45adc9ea 100644 (file)
@@ -145,7 +145,6 @@ struct relay_viewer_stream *viewer_stream_create(struct relay_stream *stream,
        lttng_ht_node_init_u64(&vstream->stream_n, stream->stream_handle);
        lttng_ht_add_unique_u64(viewer_streams_ht, &vstream->stream_n);
 
        lttng_ht_node_init_u64(&vstream->stream_n, stream->stream_handle);
        lttng_ht_add_unique_u64(viewer_streams_ht, &vstream->stream_n);
 
-       pthread_mutex_init(&vstream->reflock, NULL);
        urcu_ref_init(&vstream->ref);
 
        return vstream;
        urcu_ref_init(&vstream->ref);
 
        return vstream;
@@ -198,16 +197,7 @@ static void viewer_stream_release(struct urcu_ref *ref)
 /* Must be called with RCU read-side lock held. */
 bool viewer_stream_get(struct relay_viewer_stream *vstream)
 {
 /* Must be called with RCU read-side lock held. */
 bool viewer_stream_get(struct relay_viewer_stream *vstream)
 {
-       bool has_ref = false;
-
-       pthread_mutex_lock(&vstream->reflock);
-       if (vstream->ref.refcount != 0) {
-               has_ref = true;
-               urcu_ref_get(&vstream->ref);
-       }
-       pthread_mutex_unlock(&vstream->reflock);
-
-       return has_ref;
+       return urcu_ref_get_unless_zero(&vstream->ref);
 }
 
 /*
 }
 
 /*
@@ -240,9 +230,7 @@ end:
 void viewer_stream_put(struct relay_viewer_stream *vstream)
 {
        rcu_read_lock();
 void viewer_stream_put(struct relay_viewer_stream *vstream)
 {
        rcu_read_lock();
-       pthread_mutex_lock(&vstream->reflock);
        urcu_ref_put(&vstream->ref, viewer_stream_release);
        urcu_ref_put(&vstream->ref, viewer_stream_release);
-       pthread_mutex_unlock(&vstream->reflock);
        rcu_read_unlock();
 }
 
        rcu_read_unlock();
 }
 
index 2514b172214ac02d6929042180cc15b7215e94b9..3d9b076b75f533bf849e6eb6104fa33307d11f32 100644 (file)
@@ -45,7 +45,6 @@ struct relay_stream;
  */
 struct relay_viewer_stream {
        struct urcu_ref ref;
  */
 struct relay_viewer_stream {
        struct urcu_ref ref;
-       pthread_mutex_t reflock;
 
        /* Back ref to stream. */
        struct relay_stream *stream;
 
        /* Back ref to stream. */
        struct relay_stream *stream;
This page took 0.034383 seconds and 5 git commands to generate.