From: Mathieu Desnoyers Date: Wed, 19 Aug 2015 07:29:52 +0000 (-0700) Subject: Fix: reference counting of consumer output X-Git-Tag: v2.6.1~59 X-Git-Url: http://git.liburcu.org/?a=commitdiff_plain;h=7ec4dd5ae0d04278b110f5ba9f37ffb8704e1605;p=lttng-tools.git Fix: reference counting of consumer output The UST app session has a reference on the consumer output object, but it belongs to the UST session. Implement a refcounting scheme to ensure it is not freed before all users are done using it. Signed-off-by: Mathieu Desnoyers Signed-off-by: Jérémie Galarneau --- diff --git a/src/bin/lttng-sessiond/consumer.c b/src/bin/lttng-sessiond/consumer.c index 2eaca237f..ea760000c 100644 --- a/src/bin/lttng-sessiond/consumer.c +++ b/src/bin/lttng-sessiond/consumer.c @@ -472,6 +472,7 @@ struct consumer_output *consumer_create_output(enum consumer_dst_type type) output->enabled = 1; output->type = type; output->net_seq_index = (uint64_t) -1ULL; + urcu_ref_init(&output->ref); output->socks = lttng_ht_new(0, LTTNG_HT_TYPE_ULONG); @@ -506,11 +507,10 @@ void consumer_destroy_output_sockets(struct consumer_output *obj) * * Should *NOT* be called with RCU read-side lock held. */ -void consumer_destroy_output(struct consumer_output *obj) +static void consumer_release_output(struct urcu_ref *ref) { - if (obj == NULL) { - return; - } + struct consumer_output *obj = + caa_container_of(ref, struct consumer_output, ref); consumer_destroy_output_sockets(obj); @@ -522,6 +522,27 @@ void consumer_destroy_output(struct consumer_output *obj) free(obj); } +/* + * Get the consumer_output object. + */ +void consumer_output_get(struct consumer_output *obj) +{ + urcu_ref_get(&obj->ref); +} + +/* + * Put the consumer_output object. + * + * Should *NOT* be called with RCU read-side lock held. + */ +void consumer_output_put(struct consumer_output *obj) +{ + if (!obj) { + return; + } + urcu_ref_put(&obj->ref, consumer_release_output); +} + /* * Copy consumer output and returned the newly allocated copy. * @@ -530,33 +551,28 @@ void consumer_destroy_output(struct consumer_output *obj) struct consumer_output *consumer_copy_output(struct consumer_output *obj) { int ret; - struct lttng_ht *tmp_ht_ptr; struct consumer_output *output; assert(obj); output = consumer_create_output(obj->type); if (output == NULL) { - goto error; + goto end; } - /* Avoid losing the HT reference after the memcpy() */ - tmp_ht_ptr = output->socks; - - memcpy(output, obj, sizeof(struct consumer_output)); - - /* Putting back the HT pointer and start copying socket(s). */ - output->socks = tmp_ht_ptr; - + output->enabled = obj->enabled; + output->net_seq_index = obj->net_seq_index; + memcpy(output->subdir, obj->subdir, PATH_MAX); + output->snapshot = obj->snapshot; + memcpy(&output->dst, &obj->dst, sizeof(output->dst)); ret = consumer_copy_sockets(output, obj); if (ret < 0) { - goto malloc_error; + goto error_put; } - -error: +end: return output; -malloc_error: - consumer_destroy_output(output); +error_put: + consumer_output_put(output); return NULL; } diff --git a/src/bin/lttng-sessiond/consumer.h b/src/bin/lttng-sessiond/consumer.h index a9266d5b9..ebff43f9d 100644 --- a/src/bin/lttng-sessiond/consumer.h +++ b/src/bin/lttng-sessiond/consumer.h @@ -21,6 +21,7 @@ #include #include #include +#include #include "snapshot.h" @@ -140,6 +141,8 @@ struct consumer_net { * Consumer output object describing where and how to send data. */ struct consumer_output { + struct urcu_ref ref; /* Refcount */ + /* If the consumer is enabled meaning that should be used */ unsigned int enabled; enum consumer_dst_type type; @@ -192,7 +195,8 @@ int consumer_socket_recv(struct consumer_socket *socket, void *msg, struct consumer_output *consumer_create_output(enum consumer_dst_type type); struct consumer_output *consumer_copy_output(struct consumer_output *obj); -void consumer_destroy_output(struct consumer_output *obj); +void consumer_output_get(struct consumer_output *obj); +void consumer_output_put(struct consumer_output *obj); int consumer_set_network_uri(struct consumer_output *obj, struct lttng_uri *uri); int consumer_send_fds(struct consumer_socket *sock, int *fds, size_t nb_fd); diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c index 6545a5ef2..19283d439 100644 --- a/src/bin/lttng-sessiond/main.c +++ b/src/bin/lttng-sessiond/main.c @@ -2634,7 +2634,7 @@ static int copy_session_consumer(int domain, struct ltt_session *session) * domain. */ if (session->kernel_session->consumer) { - consumer_destroy_output(session->kernel_session->consumer); + consumer_output_put(session->kernel_session->consumer); } session->kernel_session->consumer = consumer_copy_output(session->consumer); @@ -2647,7 +2647,7 @@ static int copy_session_consumer(int domain, struct ltt_session *session) case LTTNG_DOMAIN_UST: DBG3("Copying tracing session consumer output in UST session"); if (session->ust_session->consumer) { - consumer_destroy_output(session->ust_session->consumer); + consumer_output_put(session->ust_session->consumer); } session->ust_session->consumer = consumer_copy_output(session->consumer); diff --git a/src/bin/lttng-sessiond/session.c b/src/bin/lttng-sessiond/session.c index cd59cb7ab..452438952 100644 --- a/src/bin/lttng-sessiond/session.c +++ b/src/bin/lttng-sessiond/session.c @@ -195,7 +195,7 @@ int session_destroy(struct ltt_session *session) del_session_list(session); pthread_mutex_destroy(&session->lock); - consumer_destroy_output(session->consumer); + consumer_output_put(session->consumer); snapshot_destroy(&session->snapshot); free(session); diff --git a/src/bin/lttng-sessiond/snapshot.c b/src/bin/lttng-sessiond/snapshot.c index e5660dac7..07a31e080 100644 --- a/src/bin/lttng-sessiond/snapshot.c +++ b/src/bin/lttng-sessiond/snapshot.c @@ -222,7 +222,7 @@ void snapshot_output_destroy(struct snapshot_output *obj) if (obj->consumer) { consumer_output_send_destroy_relayd(obj->consumer); - consumer_destroy_output(obj->consumer); + consumer_output_put(obj->consumer); } free(obj); } diff --git a/src/bin/lttng-sessiond/trace-kernel.c b/src/bin/lttng-sessiond/trace-kernel.c index 093297cb0..9b4fc9015 100644 --- a/src/bin/lttng-sessiond/trace-kernel.c +++ b/src/bin/lttng-sessiond/trace-kernel.c @@ -108,14 +108,6 @@ struct ltt_kernel_session *trace_kernel_create_session(void) goto error; } - /* - * The tmp_consumer stays NULL until a set_consumer_uri command is - * executed. At this point, the consumer should be nullify until an - * enable_consumer command. This assignment is symbolic since we've zmalloc - * the struct. - */ - lks->tmp_consumer = NULL; - return lks; error: @@ -515,8 +507,7 @@ void trace_kernel_destroy_session(struct ltt_kernel_session *session) } /* Wipe consumer output object */ - consumer_destroy_output(session->consumer); - consumer_destroy_output(session->tmp_consumer); + consumer_output_put(session->consumer); free(session); } diff --git a/src/bin/lttng-sessiond/trace-kernel.h b/src/bin/lttng-sessiond/trace-kernel.h index db91b2ed2..d7763f656 100644 --- a/src/bin/lttng-sessiond/trace-kernel.h +++ b/src/bin/lttng-sessiond/trace-kernel.h @@ -100,14 +100,7 @@ struct ltt_kernel_session { /* UID/GID of the user owning the session */ uid_t uid; gid_t gid; - /* - * Two consumer_output object are needed where one is needed for the - * current output object and the second one is the temporary object used to - * store URI being set by the lttng_set_consumer_uri call. Once - * lttng_enable_consumer is called, the two pointers are swapped. - */ struct consumer_output *consumer; - struct consumer_output *tmp_consumer; /* Tracing session id */ uint64_t id; /* Session is active or not meaning it has been started or stopped. */ diff --git a/src/bin/lttng-sessiond/trace-ust.c b/src/bin/lttng-sessiond/trace-ust.c index 5f41bd764..c84b0edba 100644 --- a/src/bin/lttng-sessiond/trace-ust.c +++ b/src/bin/lttng-sessiond/trace-ust.c @@ -281,14 +281,6 @@ struct ltt_ust_session *trace_ust_create_session(uint64_t session_id) goto error_consumer; } - /* - * The tmp_consumer stays NULL until a set_consumer_uri command is - * executed. At this point, the consumer should be nullify until an - * enable_consumer command. This assignment is symbolic since we've zmalloc - * the struct. - */ - lus->tmp_consumer = NULL; - DBG2("UST trace session create successful"); return lus; @@ -781,8 +773,7 @@ void trace_ust_destroy_session(struct ltt_ust_session *session) buffer_reg_uid_destroy(reg, session->consumer); } - consumer_destroy_output(session->consumer); - consumer_destroy_output(session->tmp_consumer); + consumer_output_put(session->consumer); free(session); } diff --git a/src/bin/lttng-sessiond/trace-ust.h b/src/bin/lttng-sessiond/trace-ust.h index 795389e9d..674c2e292 100644 --- a/src/bin/lttng-sessiond/trace-ust.h +++ b/src/bin/lttng-sessiond/trace-ust.h @@ -85,14 +85,7 @@ struct ltt_ust_session { gid_t gid; /* Is the session active meaning has is been started or stopped. */ unsigned int active:1; - /* - * Two consumer_output object are needed where one is for the current - * output object and the second one is the temporary object used to store - * URI being set by the lttng_set_consumer_uri call. Once - * lttng_enable_consumer is called, the two pointers are swapped. - */ struct consumer_output *consumer; - struct consumer_output *tmp_consumer; /* Sequence number for filters so the tracer knows the ordering. */ uint64_t filter_seq_num; /* This indicates which type of buffer this session is set for. */ diff --git a/src/bin/lttng-sessiond/ust-app.c b/src/bin/lttng-sessiond/ust-app.c index 0ee7790b9..c1fe2f2a5 100644 --- a/src/bin/lttng-sessiond/ust-app.c +++ b/src/bin/lttng-sessiond/ust-app.c @@ -721,6 +721,8 @@ void delete_ust_app_session(int sock, struct ust_app_session *ua_sess, } pthread_mutex_unlock(&ua_sess->lock); + consumer_output_put(ua_sess->consumer); + call_rcu(&ua_sess->rcu_head, delete_ust_app_session_rcu); } @@ -1645,8 +1647,11 @@ static void shadow_copy_session(struct ust_app_session *ua_sess, ua_sess->egid = usess->gid; ua_sess->buffer_type = usess->buffer_type; ua_sess->bits_per_long = app->bits_per_long; + /* There is only one consumer object per session possible. */ + consumer_output_get(usess->consumer); ua_sess->consumer = usess->consumer; + ua_sess->output_traces = usess->output_traces; ua_sess->live_timer_interval = usess->live_timer_interval; copy_channel_attr_to_ustctl(&ua_sess->metadata_attr, @@ -1701,9 +1706,10 @@ static void shadow_copy_session(struct ust_app_session *ua_sess, lttng_ht_add_unique_str(ua_sess->channels, &ua_chan->node); } + return; error: - return; + consumer_output_put(ua_sess->consumer); } /*