From: Mathieu Desnoyers Date: Thu, 17 Sep 2015 16:24:32 +0000 (-0400) Subject: relay: use urcu_ref_get_unless_zero X-Git-Tag: v2.11.0-rc1~569 X-Git-Url: https://git.lttng.org/?p=lttng-tools.git;a=commitdiff_plain;h=ce4d40839ac3beef1a58730d3636a522497bc60f relay: use urcu_ref_get_unless_zero 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 Signed-off-by: Jérémie Galarneau --- diff --git a/README.md b/README.md index 4d72ed43d..6dbf19f7f 100644 --- 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. - - **[`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` diff --git a/configure.ac b/configure.ac index 197288d9a..9239e3b0b 100644 --- a/configure.ac +++ b/configure.ac @@ -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 -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], [], @@ -410,6 +410,11 @@ AC_CHECK_DECL([cmm_smp_mb__before_uatomic_or], [], [AC_MSG_ERROR([WRONG_LIBURCU_MSG])], [[#include ]] ) +#Function added in urcu 0.9.0 +AC_CHECK_DECL([urcu_ref_get_unless_zero], [], + [AC_MSG_ERROR([WRONG_LIBURCU_MSG])], [[#include ]] +) + # Check kmod library AC_ARG_WITH(kmod-prefix, AS_HELP_STRING([--with-kmod-prefix=PATH], diff --git a/doc/relayd-architecture.txt b/doc/relayd-architecture.txt index 1491da90f..56c81241f 100644 --- a/doc/relayd-architecture.txt +++ b/doc/relayd-architecture.txt @@ -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 -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. @@ -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. -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 diff --git a/src/bin/lttng-relayd/connection.c b/src/bin/lttng-relayd/connection.c index f3d7e642e..e8c3d4bea 100644 --- a/src/bin/lttng-relayd/connection.c +++ b/src/bin/lttng-relayd/connection.c @@ -27,16 +27,7 @@ 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, @@ -75,7 +66,6 @@ struct relay_connection *connection_create(struct lttcomm_sock *sock, PERROR("zmalloc relay connection"); goto end; } - pthread_mutex_init(&conn->reflock, NULL); 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(); - pthread_mutex_lock(&conn->reflock); urcu_ref_put(&conn->ref, connection_release); - pthread_mutex_unlock(&conn->reflock); rcu_read_unlock(); } diff --git a/src/bin/lttng-relayd/connection.h b/src/bin/lttng-relayd/connection.h index 443863813..43e8a1de6 100644 --- a/src/bin/lttng-relayd/connection.h +++ b/src/bin/lttng-relayd/connection.h @@ -77,7 +77,6 @@ struct relay_connection { uint32_t minor; struct urcu_ref ref; - pthread_mutex_t reflock; bool version_check_done; diff --git a/src/bin/lttng-relayd/ctf-trace.c b/src/bin/lttng-relayd/ctf-trace.c index 27009efc3..4ca6dba1e 100644 --- a/src/bin/lttng-relayd/ctf-trace.c +++ b/src/bin/lttng-relayd/ctf-trace.c @@ -75,17 +75,7 @@ void ctf_trace_release(struct urcu_ref *ref) */ 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); - pthread_mutex_init(&trace->reflock, NULL); 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(); - pthread_mutex_lock(&trace->reflock); urcu_ref_put(&trace->ref, ctf_trace_release); - pthread_mutex_unlock(&trace->reflock); rcu_read_unlock(); } diff --git a/src/bin/lttng-relayd/ctf-trace.h b/src/bin/lttng-relayd/ctf-trace.h index d051f8083..9903d38e8 100644 --- a/src/bin/lttng-relayd/ctf-trace.h +++ b/src/bin/lttng-relayd/ctf-trace.h @@ -30,10 +30,6 @@ #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 */ diff --git a/src/bin/lttng-relayd/index.c b/src/bin/lttng-relayd/index.c index b15bbcd77..1b14e3e07 100644 --- a/src/bin/lttng-relayd/index.c +++ b/src/bin/lttng-relayd/index.c @@ -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); - pthread_mutex_init(&index->reflock, NULL); 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) { - 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); - /* 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. */ - pthread_mutex_lock(&index->reflock); assert(index->ref.refcount != 0); urcu_ref_put(&index->ref, index_release); - pthread_mutex_unlock(&index->reflock); rcu_read_unlock(); } diff --git a/src/bin/lttng-relayd/index.h b/src/bin/lttng-relayd/index.h index 80fe86ab0..dda5b910b 100644 --- a/src/bin/lttng-relayd/index.h +++ b/src/bin/lttng-relayd/index.h @@ -34,7 +34,6 @@ struct relay_index { /* * 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 */ diff --git a/src/bin/lttng-relayd/session.c b/src/bin/lttng-relayd/session.c index f76fb4a42..3ea8e50d6 100644 --- a/src/bin/lttng-relayd/session.c +++ b/src/bin/lttng-relayd/session.c @@ -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); - pthread_mutex_init(&session->reflock, NULL); 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) { - 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(); - pthread_mutex_lock(&session->reflock); urcu_ref_put(&session->ref, session_release); - pthread_mutex_unlock(&session->reflock); rcu_read_unlock(); } diff --git a/src/bin/lttng-relayd/session.h b/src/bin/lttng-relayd/session.h index 4c8f8a61f..2410fd483 100644 --- a/src/bin/lttng-relayd/session.h +++ b/src/bin/lttng-relayd/session.h @@ -54,8 +54,6 @@ struct relay_session { */ struct urcu_ref ref; - /* session reflock nests inside ctf_trace reflock. */ - pthread_mutex_t reflock; pthread_mutex_t lock; diff --git a/src/bin/lttng-relayd/stream.c b/src/bin/lttng-relayd/stream.c index e9c7ad172..04ff467de 100644 --- a/src/bin/lttng-relayd/stream.c +++ b/src/bin/lttng-relayd/stream.c @@ -32,16 +32,7 @@ /* 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); - pthread_mutex_init(&stream->reflock, NULL); 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 - * 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) { @@ -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. - * - * 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) { @@ -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); - /* - * Ensure existence of stream->reflock for stream unlock. - */ 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 @@ -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); - pthread_mutex_unlock(&stream->reflock); rcu_read_unlock(); } diff --git a/src/bin/lttng-relayd/stream.h b/src/bin/lttng-relayd/stream.h index d471c7b7f..e385032cb 100644 --- a/src/bin/lttng-relayd/stream.h +++ b/src/bin/lttng-relayd/stream.h @@ -37,12 +37,6 @@ 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; diff --git a/src/bin/lttng-relayd/viewer-stream.c b/src/bin/lttng-relayd/viewer-stream.c index 8a3b09a92..60aa4371d 100644 --- a/src/bin/lttng-relayd/viewer-stream.c +++ b/src/bin/lttng-relayd/viewer-stream.c @@ -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); - pthread_mutex_init(&vstream->reflock, NULL); 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) { - 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(); - pthread_mutex_lock(&vstream->reflock); urcu_ref_put(&vstream->ref, viewer_stream_release); - pthread_mutex_unlock(&vstream->reflock); rcu_read_unlock(); } diff --git a/src/bin/lttng-relayd/viewer-stream.h b/src/bin/lttng-relayd/viewer-stream.h index 2514b1722..3d9b076b7 100644 --- a/src/bin/lttng-relayd/viewer-stream.h +++ b/src/bin/lttng-relayd/viewer-stream.h @@ -45,7 +45,6 @@ struct relay_stream; */ struct relay_viewer_stream { struct urcu_ref ref; - pthread_mutex_t reflock; /* Back ref to stream. */ struct relay_stream *stream;