From 45893984238b2e2c12fc0d84b90336c98a6d98c9 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Thu, 7 Mar 2013 12:05:35 -0500 Subject: [PATCH] Move ust channel registry inside session registry This is more logical with the way we do locking where a channel can not exist without a session so accessing a channel implies the session lock. Also, this patch makes the session registry a pointer that we allocate and free on destroy. Signed-off-by: David Goulet --- src/bin/lttng-sessiond/ust-app.c | 72 ++++++----- src/bin/lttng-sessiond/ust-app.h | 6 +- src/bin/lttng-sessiond/ust-consumer.c | 2 +- src/bin/lttng-sessiond/ust-registry.c | 172 +++++++++++++++++++++----- src/bin/lttng-sessiond/ust-registry.h | 26 ++-- 5 files changed, 201 insertions(+), 77 deletions(-) diff --git a/src/bin/lttng-sessiond/ust-app.c b/src/bin/lttng-sessiond/ust-app.c index 225d3f86a..cef02ae29 100644 --- a/src/bin/lttng-sessiond/ust-app.c +++ b/src/bin/lttng-sessiond/ust-app.c @@ -300,8 +300,8 @@ void delete_ust_app_channel(int sock, struct ust_app_channel *ua_chan, } lttng_ht_destroy(ua_chan->events); - /* Wipe and free registry. */ - ust_registry_channel_destroy(&ua_chan->session->registry, &ua_chan->registry); + /* Wipe and free registry from session registry. */ + ust_registry_channel_del_free(ua_chan->session->registry, ua_chan->key); if (ua_chan->obj != NULL) { /* Remove channel from application UST object descriptor. */ @@ -361,10 +361,11 @@ static int push_metadata(struct ust_app *app, struct ust_app_session *ua_sess) * ability to reorder the metadata it receives. */ pthread_mutex_lock(socket->lock); - pthread_mutex_lock(&ua_sess->registry.lock); + pthread_mutex_lock(&ua_sess->registry->lock); - offset = ua_sess->registry.metadata_len_sent; - len = ua_sess->registry.metadata_len - ua_sess->registry.metadata_len_sent; + offset = ua_sess->registry->metadata_len_sent; + len = ua_sess->registry->metadata_len - + ua_sess->registry->metadata_len_sent; if (len == 0) { DBG3("No metadata to push for session id %d", ua_sess->id); ret = 0; @@ -380,9 +381,9 @@ static int push_metadata(struct ust_app *app, struct ust_app_session *ua_sess) goto error_reg_unlock; } /* Copy what we haven't send out. */ - memcpy(metadata_str, ua_sess->registry.metadata + offset, len); + memcpy(metadata_str, ua_sess->registry->metadata + offset, len); - pthread_mutex_unlock(&ua_sess->registry.lock); + pthread_mutex_unlock(&ua_sess->registry->lock); ret = ust_consumer_push_metadata(socket, ua_sess, metadata_str, len, offset); @@ -392,9 +393,9 @@ static int push_metadata(struct ust_app *app, struct ust_app_session *ua_sess) } /* Update len sent of the registry. */ - pthread_mutex_lock(&ua_sess->registry.lock); - ua_sess->registry.metadata_len_sent += len; - pthread_mutex_unlock(&ua_sess->registry.lock); + pthread_mutex_lock(&ua_sess->registry->lock); + ua_sess->registry->metadata_len_sent += len; + pthread_mutex_unlock(&ua_sess->registry->lock); pthread_mutex_unlock(socket->lock); rcu_read_unlock(); @@ -402,7 +403,7 @@ static int push_metadata(struct ust_app *app, struct ust_app_session *ua_sess) return 0; error_reg_unlock: - pthread_mutex_unlock(&ua_sess->registry.lock); + pthread_mutex_unlock(&ua_sess->registry->lock); pthread_mutex_unlock(socket->lock); error_rcu_unlock: rcu_read_unlock(); @@ -488,7 +489,8 @@ void delete_ust_app_session(int sock, struct ust_app_session *ua_sess, } lttng_ht_destroy(ua_sess->channels); - ust_registry_session_destroy(&ua_sess->registry); + ust_registry_session_destroy(ua_sess->registry); + free(ua_sess->registry); if (ua_sess->handle != -1) { ret = ustctl_release_handle(sock, ua_sess->handle); @@ -652,6 +654,7 @@ struct ust_app_channel *alloc_ust_app_channel(char *name, ua_chan->enabled = 1; ua_chan->handle = -1; + ua_chan->session = ua_sess; ua_chan->key = get_next_channel_key(); ua_chan->ctx = lttng_ht_new(0, LTTNG_HT_TYPE_ULONG); ua_chan->events = lttng_ht_new(0, LTTNG_HT_TYPE_STRING); @@ -659,8 +662,10 @@ struct ust_app_channel *alloc_ust_app_channel(char *name, CDS_INIT_LIST_HEAD(&ua_chan->streams.head); - /* Initialize UST registry. */ - ust_registry_channel_init(&ua_sess->registry, &ua_chan->registry); + /* Add a channel registry to session. */ + if (ust_registry_channel_add(ua_sess->registry, ua_chan->key) < 0) { + goto error; + } /* Copy attributes */ if (attr) { @@ -1145,8 +1150,6 @@ static int create_ust_channel(struct ust_app *app, /* Flag the channel that it is sent to the application. */ ua_chan->is_sent = 1; - /* Assign session to channel. */ - ua_chan->session = ua_sess; /* Initialize ust objd object using the received handle and add it. */ lttng_ht_node_init_ulong(&ua_chan->ust_objd_node, ua_chan->handle); lttng_ht_add_unique_ulong(app->ust_objd, &ua_chan->ust_objd_node); @@ -3535,6 +3538,7 @@ static int reply_ust_register_channel(int sock, int sobjd, int cobjd, struct ust_app *app; struct ust_app_channel *ua_chan; struct ust_app_session *ua_sess; + struct ust_registry_channel *chan_reg; rcu_read_lock(); @@ -3552,11 +3556,13 @@ static int reply_ust_register_channel(int sock, int sobjd, int cobjd, assert(ua_chan); assert(ua_chan->session); ua_sess = ua_chan->session; - assert(ua_sess); - pthread_mutex_lock(&ua_sess->registry.lock); + pthread_mutex_lock(&ua_sess->registry->lock); + + chan_reg = ust_registry_channel_find(ua_sess->registry, ua_chan->key); + assert(chan_reg); - if (ust_registry_is_max_id(ua_chan->session->registry.used_channel_id)) { + if (ust_registry_is_max_id(ua_sess->registry->used_channel_id)) { ret_code = -1; chan_id = -1U; type = -1; @@ -3567,25 +3573,25 @@ static int reply_ust_register_channel(int sock, int sobjd, int cobjd, if (ua_chan->attr.type == LTTNG_UST_CHAN_METADATA) { chan_id = -1U; } else { - chan_id = ust_registry_get_next_chan_id(&ua_chan->session->registry); + chan_id = ust_registry_get_next_chan_id(ua_sess->registry); } - reg_count = ust_registry_get_event_count(&ua_chan->registry); + reg_count = ust_registry_get_event_count(chan_reg); if (reg_count < 31) { type = USTCTL_CHANNEL_HEADER_COMPACT; } else { type = USTCTL_CHANNEL_HEADER_LARGE; } - ua_chan->registry.nr_ctx_fields = nr_fields; - ua_chan->registry.ctx_fields = fields; - ua_chan->registry.chan_id = chan_id; - ua_chan->registry.header_type = type; + chan_reg->nr_ctx_fields = nr_fields; + chan_reg->ctx_fields = fields; + chan_reg->chan_id = chan_id; + chan_reg->header_type = type; /* Append to metadata */ if (!ret_code) { - ret_code = ust_metadata_channel_statedump(&ua_chan->session->registry, - &ua_chan->registry); + ret_code = ust_metadata_channel_statedump(ua_chan->session->registry, + chan_reg); if (ret_code) { ERR("Error appending channel metadata (errno = %d)", ret_code); goto reply; @@ -3607,7 +3613,7 @@ reply: } error: - pthread_mutex_unlock(&ua_sess->registry.lock); + pthread_mutex_unlock(&ua_sess->registry->lock); error_rcu_unlock: rcu_read_unlock(); return ret; @@ -3649,11 +3655,11 @@ static int add_event_ust_registry(int sock, int sobjd, int cobjd, char *name, assert(ua_chan->session); ua_sess = ua_chan->session; - pthread_mutex_lock(&ua_sess->registry.lock); + pthread_mutex_lock(&ua_sess->registry->lock); - ret_code = ust_registry_create_event(&ua_sess->registry, - &ua_chan->registry, sobjd, cobjd, name, sig, nr_fields, fields, - loglevel, model_emf_uri, &event_id); + ret_code = ust_registry_create_event(ua_sess->registry, ua_chan->key, + sobjd, cobjd, name, sig, nr_fields, fields, loglevel, + model_emf_uri, &event_id); /* * The return value is returned to ustctl so in case of an error, the @@ -3677,7 +3683,7 @@ static int add_event_ust_registry(int sock, int sobjd, int cobjd, char *name, DBG3("UST registry event %s has been added successfully", name); error: - pthread_mutex_unlock(&ua_sess->registry.lock); + pthread_mutex_unlock(&ua_sess->registry->lock); error_rcu_unlock: rcu_read_unlock(); return ret; diff --git a/src/bin/lttng-sessiond/ust-app.h b/src/bin/lttng-sessiond/ust-app.h index c6294d0a4..649c0a96d 100644 --- a/src/bin/lttng-sessiond/ust-app.h +++ b/src/bin/lttng-sessiond/ust-app.h @@ -141,10 +141,6 @@ struct ust_app_channel { struct ust_app_session *session; struct lttng_ht *ctx; struct lttng_ht *events; - /* - * UST event registry. The ONLY writer is the application thread. - */ - struct ust_registry_channel registry; /* * Node indexed by channel name in the channels' hash table of a session. */ @@ -170,7 +166,7 @@ struct ust_app_session { int handle; /* used has unique identifier for app session */ int id; /* session unique identifier */ struct ust_app_channel *metadata; - struct ust_registry_session registry; + struct ust_registry_session *registry; struct lttng_ht *channels; /* Registered channels */ struct lttng_ht_node_ulong node; char path[PATH_MAX]; diff --git a/src/bin/lttng-sessiond/ust-consumer.c b/src/bin/lttng-sessiond/ust-consumer.c index 33cb6ff37..b0264e4ba 100644 --- a/src/bin/lttng-sessiond/ust-consumer.c +++ b/src/bin/lttng-sessiond/ust-consumer.c @@ -133,7 +133,7 @@ static int ask_channel_creation(struct ust_app_session *ua_sess, ua_sess->gid, consumer->net_seq_index, ua_chan->key, - ua_sess->registry.uuid); + ua_sess->registry->uuid); health_code_update(); diff --git a/src/bin/lttng-sessiond/ust-registry.c b/src/bin/lttng-sessiond/ust-registry.c index 31d33df4c..b92345fec 100644 --- a/src/bin/lttng-sessiond/ust-registry.c +++ b/src/bin/lttng-sessiond/ust-registry.c @@ -165,17 +165,16 @@ end: * Should be called with session registry mutex held. */ int ust_registry_create_event(struct ust_registry_session *session, - struct ust_registry_channel *chan, - int session_objd, int channel_objd, char *name, char *sig, - size_t nr_fields, struct ustctl_field *fields, int loglevel, + uint64_t chan_key, int session_objd, int channel_objd, char *name, + char *sig, size_t nr_fields, struct ustctl_field *fields, int loglevel, char *model_emf_uri, uint32_t *event_id) { int ret; struct cds_lfht_node *nptr; struct ust_registry_event *event = NULL; + struct ust_registry_channel *chan; assert(session); - assert(chan); assert(name); assert(sig); @@ -188,17 +187,25 @@ int ust_registry_create_event(struct ust_registry_session *session, goto error; } + rcu_read_lock(); + + chan = ust_registry_channel_find(session, chan_key); + if (!chan) { + ret = -EINVAL; + goto error_unlock; + } + /* Check if we've reached the maximum possible id. */ if (ust_registry_is_max_id(chan->used_event_id)) { ret = -ENOENT; - goto error; + goto error_unlock; } event = alloc_event(session_objd, channel_objd, name, sig, nr_fields, fields, loglevel, model_emf_uri); if (!event) { ret = -ENOMEM; - goto error; + goto error_unlock; } event->id = ust_registry_get_next_event_id(chan); @@ -207,7 +214,6 @@ int ust_registry_create_event(struct ust_registry_session *session, "chan_objd: %u, sess_objd: %u", event->name, event->signature, event->id, event->channel_objd, event->session_objd); - rcu_read_lock(); /* * This is an add unique with a custom match function for event. The node * are matched using the event name and signature. @@ -227,15 +233,16 @@ int ust_registry_create_event(struct ust_registry_session *session, if (event_id) { *event_id = event->id; } - rcu_read_unlock(); /* Append to metadata */ ret = ust_metadata_event_statedump(session, chan, event); if (ret) { ERR("Error appending event metadata (errno = %d)", ret); + rcu_read_unlock(); return ret; } + rcu_read_unlock(); return 0; error_unlock: @@ -268,23 +275,6 @@ void ust_registry_destroy_event(struct ust_registry_channel *chan, return; } -/* - * Initialize registry with default values. - */ -void ust_registry_channel_init(struct ust_registry_session *session, - struct ust_registry_channel *chan) -{ - assert(chan); - - memset(chan, 0, sizeof(struct ust_registry_channel)); - - chan->ht = lttng_ht_new(0, LTTNG_HT_TYPE_STRING); - assert(chan->ht); - - /* Set custom match function. */ - chan->ht->match_fct = ht_match_event; -} - /* * Destroy every element of the registry and free the memory. This does NOT * free the registry pointer since it might not have been allocated before so @@ -292,8 +282,7 @@ void ust_registry_channel_init(struct ust_registry_session *session, * * This MUST be called within a RCU read side lock section. */ -void ust_registry_channel_destroy(struct ust_registry_session *session, - struct ust_registry_channel *chan) +static void destroy_channel(struct ust_registry_channel *chan) { struct lttng_ht_iter iter; struct ust_registry_event *event; @@ -306,12 +295,109 @@ void ust_registry_channel_destroy(struct ust_registry_session *session, ust_registry_destroy_event(chan, event); } lttng_ht_destroy(chan->ht); + + free(chan); } /* * Initialize registry with default values. */ -int ust_registry_session_init(struct ust_registry_session *session, +int ust_registry_channel_add(struct ust_registry_session *session, + uint64_t key) +{ + int ret = 0; + struct ust_registry_channel *chan; + + assert(session); + + chan = zmalloc(sizeof(*chan)); + if (!chan) { + PERROR("zmalloc ust registry channel"); + ret = -ENOMEM; + goto error; + } + + chan->ht = lttng_ht_new(0, LTTNG_HT_TYPE_STRING); + if (!chan->ht) { + ret = -ENOMEM; + goto error; + } + + /* Set custom match function. */ + chan->ht->match_fct = ht_match_event; + + rcu_read_lock(); + lttng_ht_node_init_u64(&chan->node, key); + lttng_ht_add_unique_u64(session->channels, &chan->node); + rcu_read_unlock(); + +error: + return ret; +} + +/* + * Find a channel in the given registry. RCU read side lock MUST be acquired + * before calling this function and as long as the event reference is kept by + * the caller. + * + * On success, the pointer is returned else NULL. + */ +struct ust_registry_channel *ust_registry_channel_find( + struct ust_registry_session *session, uint64_t key) +{ + struct lttng_ht_node_u64 *node; + struct lttng_ht_iter iter; + struct ust_registry_channel *chan = NULL; + + assert(session); + assert(session->channels); + + lttng_ht_lookup(session->channels, &key, &iter); + node = lttng_ht_iter_get_node_u64(&iter); + if (!node) { + goto end; + } + chan = caa_container_of(node, struct ust_registry_channel, node); + +end: + return chan; +} + +/* + * Remove channel using key from registry and free memory. + */ +void ust_registry_channel_del_free(struct ust_registry_session *session, + uint64_t key) +{ + struct lttng_ht_iter iter; + struct ust_registry_channel *chan; + + assert(session); + + rcu_read_lock(); + chan = ust_registry_channel_find(session, key); + if (!chan) { + goto end; + } + + iter.iter.node = &chan->node.node; + lttng_ht_del(session->channels, &iter); + + destroy_channel(chan); + +end: + rcu_read_unlock(); + return; +} + +/* + * Initialize registry with default values and set the newly allocated session + * pointer to sessionp. + * + * Return 0 on success and sessionp is set or else return -1 and sessionp is + * kept untouched. + */ +int ust_registry_session_init(struct ust_registry_session **sessionp, struct ust_app *app, uint32_t bits_per_long, uint32_t uint8_t_alignment, @@ -322,10 +408,16 @@ int ust_registry_session_init(struct ust_registry_session *session, int byte_order) { int ret; + struct ust_registry_session *session; - assert(session); + assert(sessionp); + assert(app); - memset(session, 0, sizeof(struct ust_registry_session)); + session = zmalloc(sizeof(*session)); + if (!session) { + PERROR("zmalloc ust registry session"); + goto error; + } pthread_mutex_init(&session->lock, NULL); session->bits_per_long = bits_per_long; @@ -336,6 +428,11 @@ int ust_registry_session_init(struct ust_registry_session *session, session->long_alignment = long_alignment; session->byte_order = byte_order; + session->channels = lttng_ht_new(0, LTTNG_HT_TYPE_U64); + if (!session->channels) { + goto error; + } + ret = lttng_uuid_generate(session->uuid); if (ret) { ERR("Failed to generate UST uuid (errno = %d)", ret); @@ -350,6 +447,8 @@ int ust_registry_session_init(struct ust_registry_session *session, goto error; } + *sessionp = session; + return 0; error: @@ -363,10 +462,23 @@ error: void ust_registry_session_destroy(struct ust_registry_session *reg) { int ret; + struct lttng_ht_iter iter; + struct ust_registry_channel *chan; /* On error, EBUSY can be returned if lock. Code flow error. */ ret = pthread_mutex_destroy(®->lock); assert(!ret); + rcu_read_lock(); + /* Destroy all event associated with this registry. */ + cds_lfht_for_each_entry(reg->channels->ht, &iter.iter, chan, node.node) { + /* Delete the node from the ht and free it. */ + ret = lttng_ht_del(reg->channels, &iter); + assert(!ret); + destroy_channel(chan); + } + lttng_ht_destroy(reg->channels); + rcu_read_unlock(); + free(reg->metadata); } diff --git a/src/bin/lttng-sessiond/ust-registry.h b/src/bin/lttng-sessiond/ust-registry.h index 5efa0828d..04bbaa0e9 100644 --- a/src/bin/lttng-sessiond/ust-registry.h +++ b/src/bin/lttng-sessiond/ust-registry.h @@ -62,9 +62,15 @@ struct ust_registry_session { size_t metadata_len, metadata_alloc_len; /* Length of bytes sent to the consumer. */ size_t metadata_len_sent; + /* + * Hash table containing channels sent by the UST tracer. MUST be accessed + * with a RCU read side lock acquired. + */ + struct lttng_ht *channels; }; struct ust_registry_channel { + uint64_t key; /* Id set when replying to a register channel. */ uint32_t chan_id; enum ustctl_channel_header header_type; @@ -84,6 +90,8 @@ struct ust_registry_channel { */ size_t nr_ctx_fields; struct ustctl_field *ctx_fields; + /* Hash table node for the session ht indexed by key. */ + struct lttng_ht_node_u64 node; }; /* @@ -168,12 +176,16 @@ static inline uint32_t ust_registry_get_event_count( return (uint32_t) uatomic_read(&r->used_event_id); } -void ust_registry_channel_init(struct ust_registry_session *session, - struct ust_registry_channel *chan); void ust_registry_channel_destroy(struct ust_registry_session *session, struct ust_registry_channel *chan); - -int ust_registry_session_init(struct ust_registry_session *session, +struct ust_registry_channel *ust_registry_channel_find( + struct ust_registry_session *session, uint64_t key); +int ust_registry_channel_add(struct ust_registry_session *session, + uint64_t key); +void ust_registry_channel_del_free(struct ust_registry_session *session, + uint64_t key); + +int ust_registry_session_init(struct ust_registry_session **sessionp, struct ust_app *app, uint32_t bits_per_long, uint32_t uint8_t_alignment, @@ -182,13 +194,11 @@ int ust_registry_session_init(struct ust_registry_session *session, uint32_t uint64_t_alignment, uint32_t long_alignment, int byte_order); - void ust_registry_session_destroy(struct ust_registry_session *session); int ust_registry_create_event(struct ust_registry_session *session, - struct ust_registry_channel *channel, - int session_objd, int channel_objd, char *name, char *sig, - size_t nr_fields, struct ustctl_field *fields, int loglevel, + uint64_t chan_key, int session_objd, int channel_objd, char *name, + char *sig, size_t nr_fields, struct ustctl_field *fields, int loglevel, char *model_emf_uri, uint32_t *event_id); struct ust_registry_event *ust_registry_find_event( struct ust_registry_channel *chan, char *name, char *sig); -- 2.34.1